Merge lp:~vanvugt/unity/fix-742544-alt-trunk into lp:unity

Proposed by Daniel van Vugt on 2011-12-23
Status: Rejected
Rejected by: Daniel van Vugt on 2012-06-11
Proposed branch: lp:~vanvugt/unity/fix-742544-alt-trunk
Merge into: lp:unity
Diff against target: 75 lines (+22/-26)
1 file modified
plugins/unityshell/src/UScreen.cpp (+22/-26)
To merge this branch: bzr merge lp:~vanvugt/unity/fix-742544-alt-trunk
Reviewer Review Type Date Requested Status
Marco Trevisan (Treviño) Disapprove on 2012-03-26
Sam Spilsbury (community) 2011-12-23 Abstain on 2012-01-20
Review via email: mp+86779@code.launchpad.net

Description of the change

The primary monitor (the one the launcher appears on) should always be chosen by the user (LP: #742544)

You select the primary monitor in the Gnome Displays panel, or the NVIDIA control panel for the nvidia driver.

To post a comment you must log in.
Daniel van Vugt (vanvugt) wrote :

This branch removes the fallback to the left-most monitor that was introduced late just before oneiric was released.

If you still want the old (oneiric-only) behaviour then see the more complex version of this fix:
https://code.launchpad.net/~vanvugt/unity/fix-742544-trunk/+merge/79492

Sam Spilsbury (smspillaz) :
review: Approve
Neil J. Patel (njpatel) wrote :

The launcher is shown on the left-most screen for some user-experience reasons, specifically because the launcher, in it's default auto-hide mode, cannot be shown via the mouse if it's not on the left-most monitor easily. Most people just assume it's broken. Jason Smith is working on a fix for this for 5.0, and that will solve this bug. For 4.0, we can't make such a big change that Jason is doing, so I don't think this should be applied to it either.

Sam Spilsbury (smspillaz) :
review: Abstain
Daniel van Vugt (vanvugt) wrote :

I know the justification but take a look at how many people this is upsetting: bug 742544

Also, if Jason is working on it then why wasn't the bug updated to show that? It's been assigned to me for 3 months and not a single comment from Jason or yourself to say any Unity devs were working on it.

Tim Penhey (thumper) wrote :

Hi Daniel,

With the new multi-monitor work and a launcher on each monitor, this change doesn't really make sense any more does it?

Tim

Daniel van Vugt (vanvugt) wrote :

Unity's inability to detect the correct primary monitor might become irrelevant by the sound of it. In which case, this proposal may become redundant. Though I haven't tested the new multi-monitor support to confirm.

Alex Launi (alexlauni) wrote :

Is this proposal still relevant or can it be dismissed?

Daniel van Vugt (vanvugt) wrote :

This proposal is still relevant as seen in bug 961281. I would not dismiss it until I'm sure this or something equivalent has been merged.

Marco Trevisan (Treviño) (3v1n0) wrote :

Daniel in current UScreen we alredy use gdk_screen_get_primary_monitor to get the primary monitor and we don't override that anymore.
So the only improvement that should be done on this side is supporting the NVIDIA drivers in some way to get a notification when the primary monitor has been changed for them.

review: Disapprove

Unmerged revisions

1796. By Daniel van Vugt on 2011-12-23

The primary monitor should always be the one chosen by the user (LP: #742544)

You select the primary monitor in the Gnome Displays panel, or the NVIDIA
control panel for the nvidia driver. Don't try to override what the user has
chosen or they will be unhappy.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/unityshell/src/UScreen.cpp'
2--- plugins/unityshell/src/UScreen.cpp 2011-09-23 23:53:13 +0000
3+++ plugins/unityshell/src/UScreen.cpp 2011-12-23 07:01:26 +0000
4@@ -16,6 +16,7 @@
5 * Authored by: Neil Jagdish Patel <neil.patel@canonical.com>
6 */
7
8+#define G_LOG_DOMAIN "UScreen"
9 #include "UScreen.h"
10
11 static UScreen* _default_screen = NULL;
12@@ -92,17 +93,18 @@
13 void
14 UScreen::Refresh()
15 {
16- GdkScreen* screen;
17- nux::Geometry last_geo(0, 0, 1, 1);
18-
19- screen = gdk_screen_get_default();
20-
21- _monitors.erase(_monitors.begin(), _monitors.end());
22-
23- g_print("\nScreen geometry changed:\n");
24-
25- int lowest_x = std::numeric_limits<int>::max();
26- int highest_y = std::numeric_limits<int>::min();
27+ GdkScreen* screen = gdk_screen_get_default();
28+ _monitors.clear();
29+ g_message("Screen geometry changed:\n");
30+
31+ /*
32+ * The primary monitor MUST be the one chosen by the user. You select the
33+ * primary monitor in the Gnome Displays panel, or the NVIDIA control
34+ * panel for the nvidia driver.
35+ * Don't try to override what the user has chosen or they will be unhappy.
36+ */
37+ primary_ = gdk_screen_get_primary_monitor(screen);
38+
39 for (int i = 0; i < gdk_screen_get_n_monitors(screen); i++)
40 {
41 GdkRectangle rect = { 0 };
42@@ -111,24 +113,18 @@
43
44 nux::Geometry geo(rect.x, rect.y, rect.width, rect.height);
45
46- // Check for mirrored displays
47- if (geo == last_geo)
48- continue;
49- last_geo = geo;
50-
51 _monitors.push_back(geo);
52
53- if (geo.x < lowest_x || (geo.x == lowest_x && geo.y > highest_y))
54- {
55- lowest_x = geo.x;
56- highest_y = geo.y;
57- primary_ = i;
58- }
59-
60- g_print(" %dx%dx%dx%d\n", geo.x, geo.y, geo.width, geo.height);
61+ gchar *name = gdk_screen_get_monitor_plug_name(screen, i);
62+ g_message(" #%d: %s %dx%d +%d+%d %s\n",
63+ i,
64+ name == NULL ? "?" : name,
65+ geo.width, geo.height,
66+ geo.x, geo.y,
67+ i == primary_ ? "(primary)" : "");
68+ if (name)
69+ g_free(name);
70 }
71
72- g_print("\n");
73-
74 changed.emit(primary_, _monitors);
75 }