|Summary:||XRRSelectInput() doesn't deliver all the events that randrproto.txt advertises|
|Product:||xorg||Reporter:||Federico Mena-Quintero <federico>|
|Component:||Server/General||Assignee:||Jeremy Huddleston Sequoia <jeremyhu>|
|Status:||RESOLVED FIXED||QA Contact:||Xorg Project Team <xorg-team>|
|i915 platform:||i915 features:|
|Bug Depends on:|
Description Federico Mena-Quintero 2009-05-15 17:29:16 UTC
While looking at exactly what the RRNotify_* event subcodes mean, I caught this in randrproto.txt (section 7 - Extension requests) where it talks about RRSelectInput: RRCrtcChangeNotify may also be sent when this request executes if the CRTC configuration has changed since the client connected, to avoid race conditions. And similarly for RROutputChangeNotify. However, xserver/randr/rrdispatch.c:ProcRRSelectInput() and it only checks for RRScreenChangeNotifyMask. It doesn't check for RRCrtcChangeMask nor RROutputChangeMask. Am I misreading something, or is this just an oversight in that function? Also, randrproto.txt doesn't say anything about RROutputPropertyNotifyMask within RRSelectInput, but it seems that it may as well send the corresponding event if the properties changed since the client connected.
Comment 1 Adam Jackson 2009-06-12 10:32:16 UTC
I have no idea what X source you're looking at: http://cgit.freedesktop.org/xorg/xserver/tree/randr/rrdispatch.c#n87 if (stuff->enable & (RRScreenChangeNotifyMask| RRCrtcChangeNotifyMask| RROutputChangeNotifyMask| RROutputPropertyNotifyMask))
Comment 2 Adam Jackson 2009-06-12 10:34:04 UTC
Oh, wait, I see what you're saying. Yeah, probably just an oversight.
Comment 3 Federico Mena-Quintero 2009-06-12 13:13:14 UTC
Created attachment 26738 [details] [review] xserver-bfo21760-RRSelectInput-missing-events.diff
Comment 4 Jeremy Huddleston Sequoia 2011-04-11 14:13:40 UTC
Federico, have you been living on this patch? Does it still apply to ToT? Please followup by sending the patch for review to the xorg-devel mailing list for review.
Comment 5 Federico Mena-Quintero 2011-04-12 20:16:58 UTC
Sorry, I don't have an xorg build environment anymore to test this. This didn't come about from a real, observed bug, but rather from reading the source and the randrproto spec together. It seemed odd that events weren't being generated as advertised. Could you or someone else, that is closer than myself to xorg development, please send the patch to the list for consideration? I don't think any clients do so fine-grained matching of events that they would be affected by the current behavior (i.e. they use a blunt hammer and refresh everything on every event), but it would be nice to have the code match the spec.
Comment 6 Jeremy Huddleston Sequoia 2011-04-13 21:51:52 UTC
It's on master: http://cgit.freedesktop.org/xorg/xserver/commit/?id=b2997431fd426ab318bc5dfd2cd43956d733ebec Leaving open to track merging into 1.10 post 1.10.1. Sorry, but it looks like Keith's git-am gave me credit instead of you for authorship.
Comment 7 Federico Mena-Quintero 2011-04-14 10:48:38 UTC
No problem. Thanks again for babysitting my patch :)
Comment 8 Jeremy Huddleston Sequoia 2011-05-23 14:02:53 UTC
Sorry, I dropped the ball merging this into 1.10.2. I'll get it into 1.10.3.
Comment 9 Jeremy Huddleston Sequoia 2011-05-28 17:26:30 UTC
d784fd0..613e0e9 server-1.10-branch -> server-1.10-branch