After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 643216 - Extraneous emits of GdkScreen::monitors-changed
Extraneous emits of GdkScreen::monitors-changed
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: X11
3.0.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2011-02-24 18:44 UTC by Alexander Larsson
Modified: 2011-02-24 18:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Avoid spurious emissions of monitors-changed (2.69 KB, patch)
2011-02-24 18:46 UTC, Alexander Larsson
committed Details | Review

Description Alexander Larsson 2011-02-24 18:44:02 UTC
The monitor change detection code in _gdk_x11_screen_size_changed() and process_monitors_change() goes to some length to make sure its only emitted when there is an actual change to the data visible via the GdkScreen monitors api.

However, commit 662e69ad added some code that always emits "monitors-changed" in _gdk_x11_screen_size_changed when we have randr13 and get a ConfigureNotify on the root window (even though we may already have emitted it in the RRScreenChangesNotify event!). 

As far as I can tell this is due to a comment in the bug referenced by the commit (https://bugzilla.gnome.org/show_bug.cgi?id=601712#c4) where it says:

  This version of the patch changes GdkDisplay to emit "monitors-changed" when
  the primary monitor changes (see the change in _gdk_x11_screen_size_changed)."

And, if you remove this part of the change the signal is not emitted when just the primary is changed. However, this is not really the right approach. We should just also check for if the primary changes in process_monitors_change() to avoid spurious signal emissions.
Comment 1 Alexander Larsson 2011-02-24 18:46:14 UTC
Created attachment 181859 [details] [review]
Avoid spurious emissions of monitors-changed

The monitor change detection code in _gdk_x11_screen_size_changed() and
process_monitors_change() goes to some length to make sure its only emitted
when there is an actual change to the data visible via the GdkScreen monitors
api.

However, commit 662e69ad added some code that always emits "monitors-changed"
in _gdk_x11_screen_size_changed when we have randr13 and get a ConfigureNotify
on the root window (even though we may already have emitted it in the
RRScreenChangesNotify event!).

As far as I can tell this is due to a comment in the bug referenced by the
commit (https://bugzilla.gnome.org/show_bug.cgi?id=601712#c4) where it says:

  This version of the patch changes GdkDisplay to emit "monitors-changed" when
  the primary monitor changes (see the change in _gdk_x11_screen_size_changed).

And, if you remove this part of the change the signal is not emitted when just
the primary is changed. However, this is not really the right approach. We
should just also check for if the primary changes in process_monitors_change()
to avoid spurious signal emissions.
Comment 2 Matthias Clasen 2011-02-24 18:51:33 UTC
Review of attachment 181859 [details] [review]:

Looks good to me.