Created attachment 147585 [details] [review]
Patch

Looks like you forgot the part of the patch that adds the function?

Created attachment 148232 [details] [review]
Updated patch

I'm trying to test this, but I can't find any UI anywhere for how to actually *set* the primary monitor. :/

Created attachment 148234 [details] [review]
Add support to GdkDisplay::monitors-changed signal

Okay, I figured out how to test using xrandr command-line utility and the patch worked as expected.  For my machine the commands to switch between the two different monitors were:
  xrandr --output LVDS1 --primary
  xrandr --output VGA1 --primary

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).

testxinerama is also updated to display the primary monitor, and to listen to "monitors-changed" and show the primary monitor.

Created attachment 148238 [details] [review]
Win32 implementation (untested)

I don't have my Win32 build setup anymore, so I can't test this right now.  But here it is if there's anyone who's interested.

Does this actually ever set primary_monitor to something other than 0 ?

I thought Xrandr was supposed to sort the primary monitory first ?

When I plug my Thinkpad into my LCD, I can set the primary monitor to either VGA1 or LVDS1 and this function will return either 0 or 1.  Try running testxinerama and modify your primary display as I commented above and you'll see the number change.

The only thing is that the Gnome display setup util doesn't have any UI for setting the primary monitor, so to be honest I'm not sure if this will ever return other than 0 in practice as things stand right now.

My motivation for looking into this is that on Ubuntu we have this stupid splash screen and the login sequence looks kind of weird:
 1/ splash screen on monitor #0
 2/ gdm greeter on monitor #1
 3/ splash screen on monitor #0
 4/ desktop starts, panel on monitor #0

I haven't looked into why gdm is starting on #1 yet, but it seemed like it would be nice to have some real API that can be used by any apps that might need it (such as gnome-shell, gnome-panel, gdm greeter, xsplash, etc) and then separately add support to the display util for setting the value.

It looks like on NVIDIA drivers it will report multiple monitors through the API, but the command-line xrandr utility only shows one so there is not any obvious way to set the primary monitor to something other than 0 as far as I can tell.

Relevant bugs related to this worth tracking:

https://bugzilla.gnome.org/show_bug.cgi?id=595531
https://bugzilla.gnome.org/show_bug.cgi?id=564713
https://bugzilla.gnome.org/show_bug.cgi?id=562944

+  primary_output = XRRGetOutputPrimary (screen_x11->xdisplay,
+                                        screen_x11->xroot_window);

XRRGetOutputPrimary is a 1.3 addition, but this is probably ok since we only define HAVE_XRANDR if we have at least 1.3 client-side.

Other than that, looks good to commit. Might be good to add a sentence to the docs explaining what 'primary' means... if you have any idea what to say there.

I'll let tml review the win32 implementation.

Oh, one thing I just notice now that I'm rereading what the randr spec says about 'primary': XRRGetOutputPrimary may return None. Does your implementation handle that, and is it documented ?

Right now screen_x11->primary_monitor gets initialized to 0, and if it discovers that there is a different monitor which is primary then it sets it.  So if XRRGetOutputPrimary is None, that will never happen and the return value of this will be 0.

Does that seem reasonable, or do you think we should actually return a different value if there is no default monitor?  I kind of hate to do something like return -1 for no default though.  If we want to provide the ability to differentiate between "primary is 0" and "no primary" then maybe we could do:

gboolean gdk_screen_get_primary_monitor (GdkScreen *, int *);

And have the return value specify whether the int* has any particular meaning.

The problem with all this, in my opinion, is that if you know there are multiple monitors and you are an app like a panel that cares what the primary monitor is, then xrandr is basically shifting the decision of "what is the primary monitor?" to you by not having a value.  I kind of feel like GTK+ could make an opinionated choice here and say that if there is no default monitor set by xrandr, then monitor 0 is considered the primary one.

I think -1 would be consitent with for example gtk_menu_get_monitor which does return -1. How useful is returning 0 if you have no idea what monitor that is? On my Intel card, 0 has a different meaning depending on whether I boot with or without an external monitor.

Created attachment 148976 [details] [review]
Sets primary_monitor = -1 unless it's set otherwise by xrandr. 

I still don't really like the idea of returning -1 if the primary monitor isn't set.  As I said before, I think this is basically just shifting the responsibility back to the application developer to determine what monitor to use.  Anyone using this API would now have to check gdk_screen_get_primary_display(), and if the return value is -1 then use 0 instead.

Another option could be gboolean gdk_screen_has_primary_monitor()?  I don't envision that being very useful, but at least it would resolve this issue.

So, looking about this again, I think returning 0 in the 'no primary monitor' case is fine. But it should be documented.

Returns: the index of the primary monitor, or 0, if no primary monitor has been set.

Or so. Also, adding one sentence about the purpose of 'primary monitor' would be good. Something like:

The primary monitor is the one where the 'main desktop' user interface is
displayed. The primary monitor is the best monitor to display a new window on,
if you have no other clues where the window should be shown.

Can you commit the version that always returns a valid monitor, with the doc improvements ? 

Still waiting for tml to review the win32 part.

Thanks, pushed to master.