Xrandr, Handle RRScreenChangeNotify in src/screen.cpp

[Karl Hegbloom]

Binary package hint: compiz

Release: Ubuntu "Natty Narwhal", current as of today (2011/03/05)
Package Version: 1:0.9.4-0ubuntu3

Summary: When using the 'gnome-display-properties' tool or the 'xrandr' command line tool to change the display configuration from single to dual monitor and etc., compiz did not properly handle that event. The attached quilt patch fixes this deficiency.

I have a laptop with a 1400x1050 display, and an LCD monitor with a 1680x1050 display. When I start with only the laptop screen enabled, and use the 'gnome-display-properties' to turn on the external display, it defaults to placing the external display to the right of the laptop panel. When I turn it on with that configuration, and then move the external monitor over to the left of the laptop panel, which is where it really is, physically, and then click apply, compiz did not update it's idea of the output dimensions. The external display will be on the left, but only 1400x1050 of it is used, and windows on the laptop panel will maximize to 1680x1050, overlapping the real screen onto the external monitor.

After applying this patch, it works as expected.

Also, I noticed that nowhere in core or any plugins was compiz actually asking the X Server to provide it with RRScreenChangeNotify events, despite the fact that the event handler in plugins/composite/screen.cpp has a switch case looking for them. It could not ever have updated the display refresh rates without having selected those events. This patch does that, and so I think that code can run now, when a new monitor or projector gets plugged in.

What remains is to write a plugin that handles screen rotation on tablet computers, syncing the rotation setting with the wacom / touchscreen rotation and sizes. I might try and write that. Any ideas?

[Sebastien Bacher (seb128)]

Thank you for your work there, Sam could you review the patch?

[Sam Spilsbury (smspillaz)]

Hi Karl

Sorry for getting back to you so late - as discussed on the compiz mailing list the general idea behind your patch is OK and I'm generally in favor of including it. I'd just like to run some ideas past Danny so that everyone knows what is going on (I re-read your explanation and can now see a usecase, where eg the screen layout changes between two different output geometries but the bounding geometry around both outputs (eg the root window) doesn't actually change size so we never get a ConfigureNotify for that.

[Sam Spilsbury (smspillaz)]

On Tue, Mar 8, 2011 at 12:24 AM, Danny Baumann <email address hidden> wrote:
> Hi,
>
>> It will now update the screen info, then detect and update output
>> devices just like it does in CompScreen::init. This allows compiz to
>> do the right thing when an external monitor or projector is plugged in
>> and enabled using the 'gnome-display-properties' dialog or the
>> 'xrandr' command line tool.
>>
>> Call XRRSelectInput in CompScreen::init, asking for
>> RRScreenChangeNotify events. Without this those events are never given
>> to compiz. That means that the switch case for it in
>> plugins/composite/screen.cpp was never triggered, until now.
>
> What is the exact problem this patch is trying to solve? I never had a
> problem with dynamically adding/removing monitors, because compiz is
> listening for ConfigureNotify on the root window, and some esoterical
> corner cases aside, the root window will change its size anyway when
> hotplugging monitors.
>

I re-read some of the explanations here [1], it looks like this is
specifically to handle the case where you don't get a ConfigureNotify
event on the root window because only the layout of the two monitors
is actually changing (eg the bounding rect of both monitors is the
same).

Although I'm worried about some potential doubling-up of handing
screen resizes - since it is possible for us to get both an
RRScreenChangeNotify and ConfigureNotify on the root window for the
same resolution change. Perhaps it would be better to check for the
existence of XRandR in this case and *only* handle the
RRScreenChangeNotify event if XRandR is available. According to the
documentation anyways it looks like you'll get RRScreenChangeNotify
for pretty much anything regarding the size of the screen [2]

Cheers,

Sam

[1] https://bugs.launchpad.net/ubuntu/+source/compiz/+bug/729903
[2] http://keithp.com/~keithp/talks/randr/protocol.txt

> I don't see a problem in adding that patch (it's obviously correct), I
> just want to understand the reasoning.
>
> Regards,
>
> Danny

[Karl Hegbloom (karl.hegbloom)]

On 03/10/2011 05:06 AM, Sam "SmSpillaz" Spilsbury wrote:
> [...] where eg the screen layout changes between two
> different output geometries but the bounding geometry around both
> outputs (eg the root window) doesn't actually change size so we never
> get a ConfigureNotify for that.

Sure. If use the gnome-display-properties to shut off the larger
external monitor that shows up on the right by default, and apply...
Then move it over to the left, and turn it back on, and apply, Compiz
already works fine with no patch. But if I simply move the monitor over
to the left and click apply, it does not pick up the change without that
patch.

[Eleni Maria Stea (hikiko)]

We didn't manage to reproduce this bug, plus it's quite old and Natty is not supported anymore. Therefore I changed the status to Won't Fix. If you are still having the problem in one of the supported Ubuntu releases please feel free to re-open it and update the description. Thank you!