Created attachment 250981 [details] [review]
Adapt to GnomeRR DBus rewrite

A number of APIs were changed to adapt GnomeRR to the new
mutter DBus API.

Created attachment 250982 [details] [review]
CcRRLabeler: don't crash when not running under X

If running under wayland, we can't use an X property notify
to look at workarea changes.
Currently, workarea is not part of the wayland protocol, so don't
bother and just don't crash.

Review of attachment 250981 [details] [review]:

Separate patches for the 2 panels please.

Rest looks great.

Review of attachment 250982 [details] [review]:

::: panels/display/cc-rr-labeler.c
@@ +53,1 @@
 	Atom        workarea_atom;

Do we have an equivalent to this in Wayland? How do we avoid covering the panels?

I think this code should be made to use gdk_screen_get_monitor_workarea() and recheck the values every time the screen configuration changes. That would make it work automatically on Wayland once we implement it in GTK+/gdk.

(In reply to comment #4)
> Review of attachment 250982 [details] [review]:
> 
> ::: panels/display/cc-rr-labeler.c
> @@ +53,1 @@
>      Atom        workarea_atom;
> 
> Do we have an equivalent to this in Wayland? How do we avoid covering the
> panels?
> 
> I think this code should be made to use gdk_screen_get_monitor_workarea() and
> recheck the values every time the screen configuration changes. That would make
> it work automatically on Wayland once we implement it in GTK+/gdk.

There is no workarea in wl_shell (wayland 1.12), but there might be in xdg_shell or in gnome_shell (wayland 1.14+?), so it doesn't really matter right now
On the other hand, in X the workarea can change without XRandR changes, for example if you open a dock or panel.

I think that replacing that code with GDK code is "good enough" for GNOME, and means that we'll be able to add Wayland support to the toolkit once something is figured out.

(In reply to comment #6)
> I think that replacing that code with GDK code is "good enough" for GNOME, and
> means that we'll be able to add Wayland support to the toolkit once something
> is figured out.

The patch still uses get_monitor_workarea(), and that's as much GDK as we can get, I think.

Created attachment 251150 [details] [review]
display: adapt to the new model for display configuration

The way we apply the new configuration changed, in that we are
expected to call the apply() method of GnomeRRConfig ourselves,
and that takes care to call to mutter, show the confirmation
dialog, etc.

Created attachment 251151 [details] [review]
wacom: adapt to GnomeRR API changes

GnomeRR now returns the EDID IDs as strings directly, so we don't
need to convert them to compare to the settings. Also, we don't
need to filter disconnected outputs, it's done by mutter now.

Created attachment 251152 [details] [review]
CcRRLabeler: don't crash when not running under X

If running under wayland, we can't use an X property notify
to look at workarea changes.
Currently, workarea is not part of the wayland protocol, so don't
bother and just don't crash.

Review of attachment 251150 [details] [review]:

Looks good otherwise.

::: panels/display/cc-display-panel.c
@@ +2320,2 @@
+  error = NULL;
+  gnome_rr_config_apply_persistent (self->priv->current_configuration,

That should be giving you back a retval.

@@ +2324,1 @@
+  if (error)

if (!retval)
...

Review of attachment 251151 [details] [review]:

++

Review of attachment 251152 [details] [review]:

Fine then.

Attachment 251150 [details] pushed as 150466c - display: adapt to the new model for display configuration
Attachment 251151 [details] pushed as 77c72eb - wacom: adapt to GnomeRR API changes
Attachment 251152 [details] pushed as 4c85135 - CcRRLabeler: don't crash when not running under X