Bug 21760

Summary: XRRSelectInput() doesn't deliver all the events that randrproto.txt advertises
Product: xorg Reporter: Federico Mena-Quintero <federico>
Component: Server/GeneralAssignee: Jeremy Huddleston Sequoia <jeremyhu>
Status: RESOLVED FIXED QA Contact: Xorg Project Team <xorg-team>
Severity: normal    
Priority: medium Keywords: patch
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Bug Depends on:    
Bug Blocks: 31018    
Attachments:
Description Flags
xserver-bfo21760-RRSelectInput-missing-events.diff none

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

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.