GNOME Bugzilla – Bug 668908
Allow buttons to switch assigned monitor
Last modified: 2012-08-21 15:30:53 UTC
From http://www.youtube.com/watch?v=607uIdBmozU&feature=youtu.be " Will be also a support for the Dual screen( 'monitor 1' 'monitor 2' button)? "
Created attachment 217528 [details] [review] Proposed patch Patch to implement the feature in gnome-settigns-daemon (applies to g-s-d) This patch goes on top of the patch for OSD window, ie bug 676170
Created attachment 217923 [details] [review] Updated patch Updated patch after commit ecb0e2d0 (bug 668547) and new patch for bug 679062
Is that blocking on bug 679062 or is it ready to go in for GNOME 3.6?
(In reply to comment #3) > Is that blocking on bug 679062 or is it ready to go in for GNOME 3.6? 2 different functionalities, it's just that the code shares the same portion so the patch depends on the patch from bug 679062 but that's not a real dependency. Will post an updated patch without the need for the patch from bug 679062
Created attachment 221986 [details] [review] Updated patch without the need for the patch from bug 679062 That patch should apply cleanly on current got head.
Review of attachment 221986 [details] [review]: Looks good apart from the current_monitor calculation. ::: plugins/wacom/gsd-wacom-manager.c @@ +1070,3 @@ + return; + + current_monitor = gsd_wacom_device_get_display_monitor (device); You should probably check for -1 as the current_monitor, otherwise you'll go: -1 (first monitor) 0 (first monitor) 1 (second monitor) etc. @@ +1072,3 @@ + current_monitor = gsd_wacom_device_get_display_monitor (device); + + if (++current_monitor >= n_monitors) Can you do the increment and the comparison separately please?
(In reply to comment #6) > Review of attachment 221986 [details] [review]: > > Looks good apart from the current_monitor calculation. > > ::: plugins/wacom/gsd-wacom-manager.c > @@ +1070,3 @@ > + return; > + > + current_monitor = gsd_wacom_device_get_display_monitor (device); > > You should probably check for -1 as the current_monitor, otherwise you'll go: > -1 (first monitor) > 0 (first monitor) > 1 (second monitor) No, this is on purpose. -1 does not mean the first monitor but entire screen, so do: -1 (all screen) 0 (first monitor) 1 (second monitor) and so on That's how the Windows impl works iirc. > @@ +1072,3 @@ > + current_monitor = gsd_wacom_device_get_display_monitor (device); > + > + if (++current_monitor >= n_monitors) > > Can you do the increment and the comparison separately please? OK, will do.
Created attachment 222004 [details] [review] Updated patch Adds a define for "-1" which means "entire screen, all monitors" and do the do the increment and the comparison separately as per review (comment #6)
Patch pushed to git