GNOME Bugzilla – Bug 702804
Reconfigure only on actual plug/unplug
Last modified: 2013-10-30 16:54:50 UTC
According to current code in on_randr_event() function, if (config_timestamp > change_timestamp) then it is assumed that the screen got reconfigured because of hotplug/unplug. But X-server may send RRScreenChangeNotify with new configTimestamp when list of possible frame-buffers changes. This event actually intended for RandR 1.0 client; client *must* call RRGetScreenInfo to update its view of possible screen configurations, to have a correct view of possible screen organizations. Here we are using the RandR 1.2 interface, which means that we need to confirm whether an actual output plug or unplug has happened.
Created attachment 247424 [details] [review] Proposed patch
can somebody confirm that, whether this enhancement make sense or not? Please
Review of attachment 247424 [details] [review]: What happens if the screen changes wasn't in response to a hotplug, and the output got manually probed? Will we still receive the "output-connected" signal? ::: plugins/xrandr/gsd-xrandr-manager.c @@ +126,3 @@ + /* Keep track of output plug/unplug events; see on_randr_event() */ + gboolean hot_plug_unplug; hotplug_event_occurred? @@ +1788,3 @@ +on_hot_plug_unplug_event (GnomeRRScreen *screen, GnomeRROutput *output, gpointer data) +{ + GsdXrandrManager *manager = GSD_XRANDR_MANAGER (data); You can use GsdXrandrManager *manager directly in the arguments, the function is cast anyway. @@ +1791,3 @@ + GsdXrandrManagerPrivate *priv = manager->priv; + + priv->hot_plug_unplug = TRUE; manager->priv->hotplug_event_occurred = TRUE; (no need for priv, it's used only once). @@ +1814,3 @@ + /* Reconfigure only on actual plug/unplug + * + * X-server may send RRScreenChangeNotify with new configTimestamp, "X server" @@ +1816,3 @@ + * X-server may send RRScreenChangeNotify with new configTimestamp, + * when list of possible frame-buffers changes. + * This event actually intended for RandR 1.0 client; client *must* gnome-desktop, which has the code to check for XRandR versions requires 1.3, not 1.0. Furthermore, receiving RRScreenChangeNotify happens directly in gnome-desktop, and never in gnome-settings-daemon. Should this happen in gnome-desktop instead? @@ +1834,3 @@ + use_stored_configuration_or_auto_configure_outputs (manager, config_timestamp); + + priv->hot_plug_unplug = FALSE; Should this always be reset in this function, rather than just in that condition's branch?
Thanks Bastien, I am not much familiar with gnome code; but I am trying to understand it in better way. After your review, I have again gone through code and think little bit more. 'RRScreenChangeNotify' event gets generated on every configuration change which include both, when list of possible frame-buffers changes and output plug/unplug. I don't think there is way to distinguish between these two cases. What do you think if we take track of connected list of output across configuration change and reconfigure screen only if connected output list is changed.
Code has moved to mutter for wayland support in GNOME 3.10. Reassigning.
Hey, sorry that your patch can't be applied any more, as the code you were patching no longer exists. I'm trying to understand what you're asking for. Basically, you're saying that we should reconfigure only when the layout of outputs (ie which ports are connected) changes, right? In what other cases can we get a Notify event, except for some other client asking explicitly for a different configuration?
For the record, the new code (meta_monitor_manager_handle_xevent(), mutter/src/core/monitor.c:1497) does: if (new layout matches previous intended configuration) rebuild data structures and emit signals else apply stored configuration Where matches means "it includes the same outputs, as (connector, vendor, product, serial) quadruples" Do you think that's enough?
Sounds fixed. Re-open if it's not.
I was reading that part of mutter code that mention this bug, so I thought it was best to comment here. With some qxl DRM driver fixes (in drm-next), arbitrary resize of client works nicely in 3.8, unless you have a configuration saved. With older xorg driver, the Spice agent was changing the resolution manually, to follow the client, so that's how it used to work pre-drm / pre-3.10 (I think the EDID "hack" was mostly used to prevent g-s-d from resetting it, although that edid code has been removed anyways) With the 3.10 and DRM driver, it requires a cooperative mutter. Currently, there are 2 things that prevents arbitrary resize to work out of the box: 1) the config_match_current() 2) the saved configuration Possible solution is to have a special connector property, like the one Dave Airlie proposed (but for wrong reason): http://lists.freedesktop.org/archives/dri-devel/2013-October/047069.html Any other idea? gsettings? Dbus? Agent talking to mutter? Agent doing switch himself?