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 705670 - Add a DBus API for display configuration under X
Add a DBus API for display configuration under X
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
wayland
: 705513 (view as bug list)
Depends on:
Blocks: 705510 706208
 
 
Reported: 2013-08-08 13:04 UTC by Giovanni Campagna
Modified: 2013-08-17 22:57 UTC
See Also:
GNOME target: 3.10
GNOME version: ---


Attachments
Rework and consolidate monitor handling in MetaScreen (21.06 KB, patch)
2013-08-08 13:05 UTC, Giovanni Campagna
reviewed Details | Review
Split monitor handling into an helper object (25.56 KB, patch)
2013-08-08 13:05 UTC, Giovanni Campagna
none Details | Review
Introduce a new DBus interface for display configuration (36.96 KB, patch)
2013-08-08 13:05 UTC, Giovanni Campagna
none Details | Review
Add the write side of the DBus protocol too (3.52 KB, patch)
2013-08-08 13:05 UTC, Giovanni Campagna
none Details | Review
Extend the DBus XRandR protocol to expose cloning restriction (6.95 KB, patch)
2013-08-08 13:05 UTC, Giovanni Campagna
none Details | Review
DisplayConfig: add the write side of the API (18.48 KB, patch)
2013-08-08 13:05 UTC, Giovanni Campagna
none Details | Review
Reverse handling of XRandR events between Screen and MonitorManager (14.28 KB, patch)
2013-08-08 13:06 UTC, Giovanni Campagna
none Details | Review
DisplayConfig: make the dummy backend writable (13.03 KB, patch)
2013-08-08 13:06 UTC, Giovanni Campagna
none Details | Review
default plugin: add a random color background on each monitor (3.10 KB, patch)
2013-08-08 13:06 UTC, Giovanni Campagna
none Details | Review
MonitorManager: fix handling of output transform (7.03 KB, patch)
2013-08-08 13:06 UTC, Giovanni Campagna
none Details | Review
MonitorManager: inherit directly from DisplayConfig instead of handling signals (4.75 KB, patch)
2013-08-08 13:06 UTC, Giovanni Campagna
none Details | Review
MonitorManager: add support for DPMS levels (9.25 KB, patch)
2013-08-08 13:06 UTC, Giovanni Campagna
none Details | Review
MonitorManager: add support for persistent monitor configurations (45.34 KB, patch)
2013-08-08 13:06 UTC, Giovanni Campagna
none Details | Review
MonitorConfig: add CRTC assignment (30.49 KB, patch)
2013-08-08 13:06 UTC, Giovanni Campagna
none Details | Review
MonitorConfig: add support for default configurations (15.51 KB, patch)
2013-08-08 13:06 UTC, Giovanni Campagna
none Details | Review
MonitorManager: store the presentation mode bit in XRandR (3.86 KB, patch)
2013-08-08 13:06 UTC, Giovanni Campagna
none Details | Review
MonitorManager: further extend the dummy backend (6.25 KB, patch)
2013-08-08 13:07 UTC, Giovanni Campagna
none Details | Review
MonitorManager: add support for backlight (11.76 KB, patch)
2013-08-08 13:07 UTC, Giovanni Campagna
none Details | Review
MonitorManager: ignore configuration changes that disable all outputs (3.55 KB, patch)
2013-08-08 13:07 UTC, Giovanni Campagna
none Details | Review
MonitorManager: add gamma support (9.51 KB, patch)
2013-08-08 13:07 UTC, Giovanni Campagna
none Details | Review
MonitorConfig: handle changes in the laptop lid (9.42 KB, patch)
2013-08-08 13:07 UTC, Giovanni Campagna
none Details | Review
MetaPlugin: add a UI hook for confirming display changes (11.58 KB, patch)
2013-08-08 13:07 UTC, Giovanni Campagna
none Details | Review
MonitorManager: split the XRandR parts in a subclass (72.81 KB, patch)
2013-08-08 13:07 UTC, Giovanni Campagna
none Details | Review
MonitorManager: add EDID properties to the output DBus description (6.54 KB, patch)
2013-08-08 13:07 UTC, Giovanni Campagna
none Details | Review
Rework and consolidate monitor handling in MetaScreen (21.06 KB, patch)
2013-08-16 15:41 UTC, Giovanni Campagna
reviewed Details | Review
Split monitor handling into an helper object (25.56 KB, patch)
2013-08-16 15:41 UTC, Giovanni Campagna
reviewed Details | Review
Introduce a new DBus interface for display configuration (36.96 KB, patch)
2013-08-16 15:41 UTC, Giovanni Campagna
reviewed Details | Review
Add the write side of the DBus protocol too (3.52 KB, patch)
2013-08-16 15:41 UTC, Giovanni Campagna
reviewed Details | Review
Extend the DBus XRandR protocol to expose cloning restriction (6.95 KB, patch)
2013-08-16 15:42 UTC, Giovanni Campagna
accepted-commit_now Details | Review
DisplayConfig: add the write side of the API (18.48 KB, patch)
2013-08-16 15:42 UTC, Giovanni Campagna
reviewed Details | Review
Reverse handling of XRandR events between Screen and MonitorManager (14.28 KB, patch)
2013-08-16 15:42 UTC, Giovanni Campagna
accepted-commit_now Details | Review
DisplayConfig: make the dummy backend writable (13.03 KB, patch)
2013-08-16 15:42 UTC, Giovanni Campagna
accepted-commit_now Details | Review
default plugin: add a random color background on each monitor (3.10 KB, patch)
2013-08-16 15:42 UTC, Giovanni Campagna
accepted-commit_now Details | Review
MonitorManager: fix handling of output transform (7.03 KB, patch)
2013-08-16 15:42 UTC, Giovanni Campagna
reviewed Details | Review
MonitorManager: inherit directly from DisplayConfig instead of handling signals (4.75 KB, patch)
2013-08-16 15:42 UTC, Giovanni Campagna
accepted-commit_now Details | Review
MonitorManager: add support for DPMS levels (9.25 KB, patch)
2013-08-16 15:42 UTC, Giovanni Campagna
accepted-commit_now Details | Review
MonitorManager: add support for persistent monitor configurations (45.34 KB, patch)
2013-08-16 15:42 UTC, Giovanni Campagna
accepted-commit_now Details | Review
MonitorConfig: add CRTC assignment (30.49 KB, patch)
2013-08-16 15:42 UTC, Giovanni Campagna
accepted-commit_now Details | Review
MonitorConfig: add support for default configurations (15.63 KB, patch)
2013-08-16 15:42 UTC, Giovanni Campagna
reviewed Details | Review
MonitorManager: store the presentation mode bit in XRandR (3.86 KB, patch)
2013-08-16 15:42 UTC, Giovanni Campagna
none Details | Review
MonitorManager: further extend the dummy backend (6.25 KB, patch)
2013-08-16 15:43 UTC, Giovanni Campagna
accepted-commit_now Details | Review
MonitorManager: add support for backlight (11.76 KB, patch)
2013-08-16 15:43 UTC, Giovanni Campagna
reviewed Details | Review
MonitorManager: ignore configuration changes that disable all outputs (3.55 KB, patch)
2013-08-16 15:43 UTC, Giovanni Campagna
accepted-commit_now Details | Review
MonitorManager: add gamma support (9.51 KB, patch)
2013-08-16 15:43 UTC, Giovanni Campagna
reviewed Details | Review
MonitorConfig: handle changes in the laptop lid (9.42 KB, patch)
2013-08-16 15:43 UTC, Giovanni Campagna
reviewed Details | Review
MetaPlugin: add a UI hook for confirming display changes (11.58 KB, patch)
2013-08-16 15:43 UTC, Giovanni Campagna
accepted-commit_now Details | Review
MonitorManager: split the XRandR parts in a subclass (72.81 KB, patch)
2013-08-16 15:43 UTC, Giovanni Campagna
none Details | Review
MonitorManager: add EDID properties to the output DBus description (6.88 KB, patch)
2013-08-16 15:43 UTC, Giovanni Campagna
accepted-commit_now Details | Review
MonitorXrandr: use XIDs, not API ids, in the warning (1.13 KB, patch)
2013-08-16 15:43 UTC, Giovanni Campagna
accepted-commit_now Details | Review
MonitorXrandr: implement correct EDID parsing (6.62 KB, patch)
2013-08-16 15:43 UTC, Giovanni Campagna
needs-work Details | Review
Monitor: restore correct display name handling (3.57 KB, patch)
2013-08-16 15:44 UTC, Giovanni Campagna
reviewed Details | Review
MonitorXrandr: resize the framebuffer prior to setting the CRTC configuration (2.19 KB, patch)
2013-08-16 15:44 UTC, Giovanni Campagna
accepted-commit_now Details | Review
MonitorXrandr: expose the subpixel order from X11 (2.12 KB, patch)
2013-08-16 17:09 UTC, Giovanni Campagna
rejected Details | Review
MonitorXrandr: follow the right order in applying the new configuration (4.20 KB, patch)
2013-08-16 17:09 UTC, Giovanni Campagna
accepted-commit_now Details | Review
Rework and consolidate monitor handling in MetaScreen (32.79 KB, patch)
2013-08-17 14:58 UTC, Giovanni Campagna
committed Details | Review
Introduce a new DBus interface for display configuration (37.20 KB, patch)
2013-08-17 14:59 UTC, Giovanni Campagna
committed Details | Review
Add the write side of the DBus protocol too (3.44 KB, patch)
2013-08-17 14:59 UTC, Giovanni Campagna
committed Details | Review
Extend the DBus XRandR protocol to expose cloning restriction (6.95 KB, patch)
2013-08-17 14:59 UTC, Giovanni Campagna
committed Details | Review
DisplayConfig: add the write side of the API (18.42 KB, patch)
2013-08-17 14:59 UTC, Giovanni Campagna
committed Details | Review
Reverse handling of XRandR events between Screen and MonitorManager (14.28 KB, patch)
2013-08-17 14:59 UTC, Giovanni Campagna
committed Details | Review
DisplayConfig: make the dummy backend writable (13.03 KB, patch)
2013-08-17 14:59 UTC, Giovanni Campagna
committed Details | Review
default plugin: add a random color background on each monitor (3.06 KB, patch)
2013-08-17 14:59 UTC, Giovanni Campagna
committed Details | Review
MonitorManager: fix handling of output transform (10.12 KB, patch)
2013-08-17 14:59 UTC, Giovanni Campagna
committed Details | Review
MonitorManager: inherit directly from DisplayConfig instead of handling signals (4.75 KB, patch)
2013-08-17 14:59 UTC, Giovanni Campagna
committed Details | Review
MonitorManager: add support for DPMS levels (9.25 KB, patch)
2013-08-17 15:00 UTC, Giovanni Campagna
committed Details | Review
MonitorManager: add support for persistent monitor configurations (45.48 KB, patch)
2013-08-17 15:00 UTC, Giovanni Campagna
committed Details | Review
MonitorConfig: add CRTC assignment (31.28 KB, patch)
2013-08-17 15:00 UTC, Giovanni Campagna
committed Details | Review
MonitorConfig: add support for default configurations (15.61 KB, patch)
2013-08-17 15:00 UTC, Giovanni Campagna
committed Details | Review
MonitorManager: store the presentation mode bit in XRandR (3.86 KB, patch)
2013-08-17 15:00 UTC, Giovanni Campagna
committed Details | Review
MonitorManager: further extend the dummy backend (6.25 KB, patch)
2013-08-17 15:00 UTC, Giovanni Campagna
committed Details | Review
MonitorManager: add support for backlight (11.75 KB, patch)
2013-08-17 15:00 UTC, Giovanni Campagna
committed Details | Review
MonitorManager: ignore configuration changes that disable all outputs (2.64 KB, patch)
2013-08-17 15:01 UTC, Giovanni Campagna
committed Details | Review
MonitorManager: add gamma support (9.48 KB, patch)
2013-08-17 15:01 UTC, Giovanni Campagna
committed Details | Review
MonitorConfig: handle changes in the laptop lid (9.42 KB, patch)
2013-08-17 15:01 UTC, Giovanni Campagna
committed Details | Review
MetaPlugin: add a UI hook for confirming display changes (11.58 KB, patch)
2013-08-17 15:01 UTC, Giovanni Campagna
committed Details | Review
MonitorManager: split the XRandR parts in a subclass (73.48 KB, patch)
2013-08-17 15:01 UTC, Giovanni Campagna
committed Details | Review
MonitorManager: add EDID properties to the output DBus description (6.88 KB, patch)
2013-08-17 15:01 UTC, Giovanni Campagna
committed Details | Review
MonitorXrandr: implement correct EDID parsing (24.66 KB, patch)
2013-08-17 15:01 UTC, Giovanni Campagna
committed Details | Review
Monitor: restore correct display name handling (3.57 KB, patch)
2013-08-17 15:01 UTC, Giovanni Campagna
committed Details | Review
MonitorXrandr: resize the framebuffer prior to setting the CRTC configuration (2.47 KB, patch)
2013-08-17 15:01 UTC, Giovanni Campagna
committed Details | Review
MonitorXrandr: follow the right order in applying the new configuration (5.01 KB, patch)
2013-08-17 15:02 UTC, Giovanni Campagna
committed Details | Review
MonitorXrandr: update the internal data structures after applying (4.63 KB, patch)
2013-08-17 17:20 UTC, Giovanni Campagna
committed Details | Review

Description Giovanni Campagna 2013-08-08 13:04:59 UTC
This bug is about implementing the proposed DBus display configuration
API in the master branch, using XRandR as the backend.

Patches are above master, not wip/wayland. Wayland stuff is in bug 705513
(or wip/wayland-display really)
Comment 1 Giovanni Campagna 2013-08-08 13:05:02 UTC
Created attachment 251157 [details] [review]
Rework and consolidate monitor handling in MetaScreen

Consolidate all places that deal with output configuration in
MetaScreen, which gets it either from XRandR or from a dummy static configuration.
We still need to read the Xinerama config, even when running xwayland,
because we need the indices for _NET_WM_FULLSCREEN_MONITORS, but
now we do it only when needed.
Comment 2 Giovanni Campagna 2013-08-08 13:05:06 UTC
Created attachment 251158 [details] [review]
Split monitor handling into an helper object

Create a new singleton object, MetaMonitorManager, which deals
with reading the XRandR configuration and in the future applying
the new one.
This is required because xwayland will not bind the xserver interface
until he has seen the current wl_outputs, so we can't wait
until MetaScreen is built to expose them.
Comment 3 Giovanni Campagna 2013-08-08 13:05:10 UTC
Created attachment 251159 [details] [review]
Introduce a new DBus interface for display configuration

This new interface will be used by the control center and possibly
the settings daemon to configure the screens. It is designed to
resemble a simplified XRandR, while still exposing all the quirks
of the hardware, so that the panel can limit the user choices
appropriately.

To do so, MetaMonitorMode needs to track CRTCs, outputs and modes,
so the low level objects have been decoupled from the high-level
MetaMonitorInfo, which is used by core and API and offers a simplified
view of HW, that hides away the details of what is cloned and how.
This is still not efficient as it should be, because on every
HW change we drop all data structures and rebuild them from scratch
(which is not expensive because there aren't many of them, but
at least in the XRandR path it involves a few sync X calls)
Comment 4 Giovanni Campagna 2013-08-08 13:05:14 UTC
Created attachment 251160 [details] [review]
Add the write side of the DBus protocol too

This is just in the documentation for now, to attract wider feedback
before we start looking at how to implement this for real.
Comment 5 Giovanni Campagna 2013-08-08 13:05:18 UTC
Created attachment 251161 [details] [review]
Extend the DBus XRandR protocol to expose cloning restriction

Turns out that even if two outputs say that they can be controlled
by a given CRTC, you can't configure them in the same CRTC unless
they are marked as "possible clones" one of the other.
This can further restrict the configuration options, so we need
to expose this limitation in the DBus API.
Comment 6 Giovanni Campagna 2013-08-08 13:05:21 UTC
Created attachment 251162 [details] [review]
DisplayConfig: add the write side of the API

Implement ApplyConfiguration in terms of XRandR calls.
Error checking is done before actually committing the configuration.

If mutter is using one of the other monitor config backends, an
error is reported and nothing happens.
Comment 7 Giovanni Campagna 2013-08-08 13:06:07 UTC
Created attachment 251163 [details] [review]
Reverse handling of XRandR events between Screen and MonitorManager

Now MonitorManager does its own handling of XRandR events, which
means we no longer handle ConfigureNotify on the root window.
MetaScreen reacts to MonitorManager::monitor-changed and updates
its internal state, including the new size.

This paves the way for doing display configuration using only
the dummy backend, which would allow testing wl_output interfaces.
Comment 8 Giovanni Campagna 2013-08-08 13:06:11 UTC
Created attachment 251164 [details] [review]
DisplayConfig: make the dummy backend writable

Add a number of dummy outputs and modes to the dummy backend,
and implement the writing bits.
The only visible effect is that you can change the screen size,
which resizes the output window.
Comment 9 Giovanni Campagna 2013-08-08 13:06:15 UTC
Created attachment 251165 [details] [review]
default plugin: add a random color background on each monitor

Instead of a full white background, make one with a random color.
This way the different "monitors" are visible and it's easier
to debug the DBus API.
Comment 10 Giovanni Campagna 2013-08-08 13:06:19 UTC
Created attachment 251166 [details] [review]
MonitorManager: fix handling of output transform

Read the current transform from XRandR, and expose the transforms
that are really supported on the bus.
The dummy backend now advertises all transforms, since it doesn't
actually apply them.
Comment 11 Giovanni Campagna 2013-08-08 13:06:24 UTC
Created attachment 251167 [details] [review]
MonitorManager: inherit directly from DisplayConfig instead of handling signals

This way we can handle properties too.
Comment 12 Giovanni Campagna 2013-08-08 13:06:27 UTC
Created attachment 251168 [details] [review]
MonitorManager: add support for DPMS levels

To the XRandR and dummy backend (and as usual the dummy backend
has no effect)
Comment 13 Giovanni Campagna 2013-08-08 13:06:32 UTC
Created attachment 251169 [details] [review]
MonitorManager: add support for persistent monitor configurations

Add a new object, MetaMonitorConfig, that takes care of converting
between the logical configurations stored in monitors.xml and
the HW resources exposed by MonitorManager.
This commit includes loading and saving of configurations, but
still missing is the actual CRTC assignments and a default
configuration when none is found in the file.
Comment 14 Giovanni Campagna 2013-08-08 13:06:36 UTC
Created attachment 251170 [details] [review]
MonitorConfig: add CRTC assignment

Ripped off libgnome-desktop, trimming the parts that checked
that the configuration was plausible, as that should be done
in gnome-control-center before asking mutter for a change.
Comment 15 Giovanni Campagna 2013-08-08 13:06:40 UTC
Created attachment 251171 [details] [review]
MonitorConfig: add support for default configurations

Activate the presentation bit on new hotplugged monitors, while
making a fully extended setup when running for the first time.
Comment 16 Giovanni Campagna 2013-08-08 13:06:45 UTC
Created attachment 251172 [details] [review]
MonitorManager: store the presentation mode bit in XRandR

Use a private output property to store if the output is in
presentation mode or not, so that this information is not lost
after the configuration read back from the server.
Comment 17 Giovanni Campagna 2013-08-08 13:07:07 UTC
Created attachment 251173 [details] [review]
MonitorManager: further extend the dummy backend

The default configuration is extended, which is only possible
if there are as many CRTCs as outputs, so make sure that's true.

Also, add more and bigger modes, so that different sizes will
be chosen for the three outputs.
A nice side effect of this is that with a real 1920x1080 + 1600x900
layout, if you disable the VGA you get a stage that matches the
screen size, which triggers the legacy fullscreen path in the
outside mutter.
Comment 18 Giovanni Campagna 2013-08-08 13:07:12 UTC
Created attachment 251174 [details] [review]
MonitorManager: add support for backlight

GnomeRR needs that too.
The backlight is exported as a normalized 0-100 value, or -1 if not
supported. Clamping to HW limits is handled by the backend.
Changing backlight uses a different method call, to avoid recomputing
the full display configuration every time the user presses the
backlight keys.
Comment 19 Giovanni Campagna 2013-08-08 13:07:16 UTC
Created attachment 251175 [details] [review]
MonitorManager: ignore configuration changes that disable all outputs

If we compute a screen size of 0 in either direction, we crash
later on, as it is invalid for clutter, cogl and X.
Comment 20 Giovanni Campagna 2013-08-08 13:07:21 UTC
Created attachment 251176 [details] [review]
MonitorManager: add gamma support

Add GetCrtcGamma() and SetCrtcGamma(), that wrap the similarly
named XRandR API. These are used by GnomeRR inside the color
plugin of the control center (and may go away if the color
plugin decides to do something different under wayland)
Comment 21 Giovanni Campagna 2013-08-08 13:07:25 UTC
Created attachment 251177 [details] [review]
MonitorConfig: handle changes in the laptop lid

This way we don't need to track the current and previous
configuration in gnome-settings-daemon, when we already do so
in mutter.
Comment 22 Giovanni Campagna 2013-08-08 13:07:29 UTC
Created attachment 251178 [details] [review]
MetaPlugin: add a UI hook for confirming display changes

We want to show a dialog when a display change happens from the
control center. To do so, add a new vfunc to MetaPlugin and
call it when a configuration change is requested via DBus.
Comment 23 Giovanni Campagna 2013-08-08 13:07:34 UTC
Created attachment 251179 [details] [review]
MonitorManager: split the XRandR parts in a subclass

Instead of keeping a forest of if backend else ..., use a subclass
and virtual functions to discriminate between XRandR and the
dummy backend (which lives in the parent class togheter with the
common code)
Comment 24 Giovanni Campagna 2013-08-08 13:07:39 UTC
Created attachment 251180 [details] [review]
MonitorManager: add EDID properties to the output DBus description

Add "edid-file", if we have one (in the KMS case, where we can point
people to the right sysfs file), or "edid" with inline data.
These are needed by colord to build the default ICC profile for
uncalibrated displays.
Comment 25 Matthias Clasen 2013-08-12 15:16:02 UTC
*** Bug 705513 has been marked as a duplicate of this bug. ***
Comment 26 drago01 2013-08-16 15:01:08 UTC
Review of attachment 251157 [details] [review]:

::: src/core/screen-private.h
@@ +51,3 @@
+  int width_mm;
+  int height_mm;
+  CoglSubpixelOrder subpixel_order;

What is that one for? You only seem to set it to COGL_SUBPIXEL_ORDER_UNKNOWN but never do anything else with it.
If we don't need it just remove it.

@@ +64,3 @@
+
+  /* The primary or first output for this crtc, 0 if we can't figure out.
+     This is a XID when using XRandR, otherwise a KMS id (not implemented) */

Well I'd just say 0 if we can't figure out otherwise it is an implementation detail of the backend (XID, KMS id, no meaning in the dummy backend).

::: src/core/screen.c
@@ +352,3 @@
 
+static gboolean
+has_dummy_output (void)

What's the point of this? It always returns FALSE so could as well not exist? (and seems like it does not in your branch).

@@ +506,2 @@
+    resources = XRRGetScreenResourcesCurrent (screen->display->xdisplay, screen->xroot);
+    if (!resources)

When / why would this happen? Can we really continue with just the dummy values?
Comment 27 drago01 2013-08-16 15:07:51 UTC
Review of attachment 251158 [details] [review]:

Partial can you please reattach the current branch?

::: src/core/display.c
@@ +518,3 @@
   if (meta_is_syncing ())
     XSynchronize (xdisplay, True);
+

Unrelated.

::: src/core/monitor-private.h
@@ +54,3 @@
+  int width_mm;
+  int height_mm;
+  CoglSubpixelOrder subpixel_order;

Again not needed / used -> kill it.

::: src/core/monitor.c
@@ +28,3 @@
+#include "config.h"
+
+#include <clutter/clutter.h>

Why?

@@ +225,3 @@
+ */
+static gboolean
+has_dummy_output (void)

Hmm again?

@@ +239,3 @@
+{
+  if (has_dummy_output ())
+    return make_dummy_monitor_config (manager);

It never gets there.
Comment 28 Giovanni Campagna 2013-08-16 15:41:40 UTC
Created attachment 251870 [details] [review]
Rework and consolidate monitor handling in MetaScreen

Consolidate all places that deal with output configuration in
MetaScreen, which gets it either from XRandR or from a dummy static configuration.
We still need to read the Xinerama config, even when running xwayland,
because we need the indices for _NET_WM_FULLSCREEN_MONITORS, but
now we do it only when needed.
Comment 29 Giovanni Campagna 2013-08-16 15:41:45 UTC
Created attachment 251871 [details] [review]
Split monitor handling into an helper object

Create a new singleton object, MetaMonitorManager, which deals
with reading the XRandR configuration and in the future applying
the new one.
This is required because xwayland will not bind the xserver interface
until he has seen the current wl_outputs, so we can't wait
until MetaScreen is built to expose them.
Comment 30 Giovanni Campagna 2013-08-16 15:41:51 UTC
Created attachment 251872 [details] [review]
Introduce a new DBus interface for display configuration

This new interface will be used by the control center and possibly
the settings daemon to configure the screens. It is designed to
resemble a simplified XRandR, while still exposing all the quirks
of the hardware, so that the panel can limit the user choices
appropriately.

To do so, MetaMonitorMode needs to track CRTCs, outputs and modes,
so the low level objects have been decoupled from the high-level
MetaMonitorInfo, which is used by core and API and offers a simplified
view of HW, that hides away the details of what is cloned and how.
This is still not efficient as it should be, because on every
HW change we drop all data structures and rebuild them from scratch
(which is not expensive because there aren't many of them, but
at least in the XRandR path it involves a few sync X calls)
Comment 31 Giovanni Campagna 2013-08-16 15:41:56 UTC
Created attachment 251873 [details] [review]
Add the write side of the DBus protocol too

This is just in the documentation for now, to attract wider feedback
before we start looking at how to implement this for real.
Comment 32 Giovanni Campagna 2013-08-16 15:42:02 UTC
Created attachment 251874 [details] [review]
Extend the DBus XRandR protocol to expose cloning restriction

Turns out that even if two outputs say that they can be controlled
by a given CRTC, you can't configure them in the same CRTC unless
they are marked as "possible clones" one of the other.
This can further restrict the configuration options, so we need
to expose this limitation in the DBus API.
Comment 33 Giovanni Campagna 2013-08-16 15:42:07 UTC
Created attachment 251875 [details] [review]
DisplayConfig: add the write side of the API

Implement ApplyConfiguration in terms of XRandR calls.
Error checking is done before actually committing the configuration.

If mutter is using one of the other monitor config backends, an
error is reported and nothing happens.
Comment 34 Giovanni Campagna 2013-08-16 15:42:12 UTC
Created attachment 251876 [details] [review]
Reverse handling of XRandR events between Screen and MonitorManager

Now MonitorManager does its own handling of XRandR events, which
means we no longer handle ConfigureNotify on the root window.
MetaScreen reacts to MonitorManager::monitor-changed and updates
its internal state, including the new size.

This paves the way for doing display configuration using only
the dummy backend, which would allow testing wl_output interfaces.
Comment 35 Giovanni Campagna 2013-08-16 15:42:18 UTC
Created attachment 251877 [details] [review]
DisplayConfig: make the dummy backend writable

Add a number of dummy outputs and modes to the dummy backend,
and implement the writing bits.
The only visible effect is that you can change the screen size,
which resizes the output window.
Comment 36 Giovanni Campagna 2013-08-16 15:42:23 UTC
Created attachment 251878 [details] [review]
default plugin: add a random color background on each monitor

Instead of a full white background, make one with a random color.
This way the different "monitors" are visible and it's easier
to debug the DBus API.
Comment 37 Giovanni Campagna 2013-08-16 15:42:28 UTC
Created attachment 251879 [details] [review]
MonitorManager: fix handling of output transform

Read the current transform from XRandR, and expose the transforms
that are really supported on the bus.
The dummy backend now advertises all transforms, since it doesn't
actually apply them.
Comment 38 Giovanni Campagna 2013-08-16 15:42:33 UTC
Created attachment 251880 [details] [review]
MonitorManager: inherit directly from DisplayConfig instead of handling signals

This way we can handle properties too.
Comment 39 Giovanni Campagna 2013-08-16 15:42:38 UTC
Created attachment 251881 [details] [review]
MonitorManager: add support for DPMS levels

To the XRandR and dummy backend (and as usual the dummy backend
has no effect)
Comment 40 Giovanni Campagna 2013-08-16 15:42:44 UTC
Created attachment 251882 [details] [review]
MonitorManager: add support for persistent monitor configurations

Add a new object, MetaMonitorConfig, that takes care of converting
between the logical configurations stored in monitors.xml and
the HW resources exposed by MonitorManager.
This commit includes loading and saving of configurations, but
still missing is the actual CRTC assignments and a default
configuration when none is found in the file.
Comment 41 Giovanni Campagna 2013-08-16 15:42:49 UTC
Created attachment 251883 [details] [review]
MonitorConfig: add CRTC assignment

Ripped off libgnome-desktop, trimming the parts that checked
that the configuration was plausible, as that should be done
in gnome-control-center before asking mutter for a change.
Comment 42 Giovanni Campagna 2013-08-16 15:42:54 UTC
Created attachment 251884 [details] [review]
MonitorConfig: add support for default configurations

Activate the presentation bit on new hotplugged monitors, while
making a fully extended setup when running for the first time.
Comment 43 Giovanni Campagna 2013-08-16 15:42:59 UTC
Created attachment 251885 [details] [review]
MonitorManager: store the presentation mode bit in XRandR

Use a private output property to store if the output is in
presentation mode or not, so that this information is not lost
after the configuration read back from the server.
Comment 44 Giovanni Campagna 2013-08-16 15:43:05 UTC
Created attachment 251886 [details] [review]
MonitorManager: further extend the dummy backend

The default configuration is extended, which is only possible
if there are as many CRTCs as outputs, so make sure that's true.

Also, add more and bigger modes, so that different sizes will
be chosen for the three outputs.
A nice side effect of this is that with a real 1920x1080 + 1600x900
layout, if you disable the VGA you get a stage that matches the
screen size, which triggers the legacy fullscreen path in the
outside mutter.
Comment 45 Giovanni Campagna 2013-08-16 15:43:10 UTC
Created attachment 251887 [details] [review]
MonitorManager: add support for backlight

GnomeRR needs that too.
The backlight is exported as a normalized 0-100 value, or -1 if not
supported. Clamping to HW limits is handled by the backend.
Changing backlight uses a different method call, to avoid recomputing
the full display configuration every time the user presses the
backlight keys.
Comment 46 Giovanni Campagna 2013-08-16 15:43:15 UTC
Created attachment 251888 [details] [review]
MonitorManager: ignore configuration changes that disable all outputs

If we compute a screen size of 0 in either direction, we crash
later on, as it is invalid for clutter, cogl and X.
Comment 47 Giovanni Campagna 2013-08-16 15:43:21 UTC
Created attachment 251889 [details] [review]
MonitorManager: add gamma support

Add GetCrtcGamma() and SetCrtcGamma(), that wrap the similarly
named XRandR API. These are used by GnomeRR inside the color
plugin of the control center (and may go away if the color
plugin decides to do something different under wayland)
Comment 48 Giovanni Campagna 2013-08-16 15:43:26 UTC
Created attachment 251890 [details] [review]
MonitorConfig: handle changes in the laptop lid

This way we don't need to track the current and previous
configuration in gnome-settings-daemon, when we already do so
in mutter.
Comment 49 Giovanni Campagna 2013-08-16 15:43:32 UTC
Created attachment 251891 [details] [review]
MetaPlugin: add a UI hook for confirming display changes

We want to show a dialog when a display change happens from the
control center. To do so, add a new vfunc to MetaPlugin and
call it when a configuration change is requested via DBus.
Comment 50 Giovanni Campagna 2013-08-16 15:43:40 UTC
Created attachment 251892 [details] [review]
MonitorManager: split the XRandR parts in a subclass

Instead of keeping a forest of if backend else ..., use a subclass
and virtual functions to discriminate between XRandR and the
dummy backend (which lives in the parent class togheter with the
common code)
Comment 51 Giovanni Campagna 2013-08-16 15:43:46 UTC
Created attachment 251893 [details] [review]
MonitorManager: add EDID properties to the output DBus description

Add "edid-file", if we have one (in the KMS case, where we can point
people to the right sysfs file), or "edid" with inline data.
These are needed by colord to build the default ICC profile for
uncalibrated displays.
Comment 52 Giovanni Campagna 2013-08-16 15:43:51 UTC
Created attachment 251894 [details] [review]
MonitorXrandr: use XIDs, not API ids, in the warning

More useful to debug mutter.
Comment 53 Giovanni Campagna 2013-08-16 15:43:57 UTC
Created attachment 251895 [details] [review]
MonitorXrandr: implement correct EDID parsing

To provide valid values for the vendor, product and serial fields
we need to read the EDID and parse it.
Parser kindly provided by gnome-desktop.
Comment 54 Giovanni Campagna 2013-08-16 15:44:02 UTC
Created attachment 251896 [details] [review]
Monitor: restore correct display name handling

Now that we have the right values from the EDID, we can load
the PNP database and find the proper vendor name, to show in
the control center UI.
Comment 55 Giovanni Campagna 2013-08-16 15:44:08 UTC
Created attachment 251897 [details] [review]
MonitorXrandr: resize the framebuffer prior to setting the CRTC configuration

Otherwise X11 will trim the new configuration and disable outputs
outside the screen.
Comment 56 drago01 2013-08-16 15:50:26 UTC
Review of attachment 251870 [details] [review]:

Same things still apply modulo the dummy thing.
Comment 57 Giovanni Campagna 2013-08-16 16:13:01 UTC
(In reply to comment #26)
> Review of attachment 251157 [details] [review]:
> 
> ::: src/core/screen-private.h
> @@ +51,3 @@
> +  int width_mm;
> +  int height_mm;
> +  CoglSubpixelOrder subpixel_order;
> 
> What is that one for? You only seem to set it to COGL_SUBPIXEL_ORDER_UNKNOWN
> but never do anything else with it.
> If we don't need it just remove it.

It's needed in the wayland branch (exposed to wayland clients).
I have a patch to set it sensibly in the X11 path too, I don't know how useful that would be...

> ::: src/core/screen.c
> @@ +352,3 @@
> 
> +static gboolean
> +has_dummy_output (void)
> 
> What's the point of this? It always returns FALSE so could as well not exist?
> (and seems like it does not in your branch).

It has more use in the wayland branch (in that case, it returns true when running as a non-kms wayland server). I left it here to make rebasing/merging easier.

> @@ +506,2 @@
> +    resources = XRRGetScreenResourcesCurrent (screen->display->xdisplay,
> screen->xroot);
> +    if (!resources)
> 
> When / why would this happen? Can we really continue with just the dummy
> values?

Good question, I don't know when the call can fail, but the value was checked in the previous xrandr mutter code as well as gnomerr.

(In reply to comment #27)
> ::: src/core/monitor.c
> @@ +28,3 @@
> +#include "config.h"
> +
> +#include <clutter/clutter.h>
> 
> Why?

Uhm... for the cogl configuration backend?
(That is later removed...)

> @@ +239,3 @@
> +{
> +  if (has_dummy_output ())
> +    return make_dummy_monitor_config (manager);
> 
> It never gets there.

Yeah, but it reduces the diff with the wayland branch a little.
Comment 58 drago01 2013-08-16 16:41:56 UTC
Review of attachment 251871 [details] [review]:

::: src/core/display.c
@@ +518,3 @@
   if (meta_is_syncing ())
     XSynchronize (xdisplay, True);
+

Unrelated.

::: src/core/monitor.c
@@ +222,3 @@
+ *
+ * Returns TRUE if the only available monitor is the dummy one
+ * backing the ClutterStage window.

That's not what the code does. It always returns false. If it is going to be made useful later add a comment.
Comment 59 drago01 2013-08-16 16:42:14 UTC
Review of attachment 251871 [details] [review]:

::: src/core/display.c
@@ +518,3 @@
   if (meta_is_syncing ())
     XSynchronize (xdisplay, True);
+

Unrelated.

::: src/core/monitor.c
@@ +222,3 @@
+ *
+ * Returns TRUE if the only available monitor is the dummy one
+ * backing the ClutterStage window.

That's not what the code does. It always returns false. If it is going to be made useful later add a comment.
Comment 60 drago01 2013-08-16 16:47:34 UTC
Review of attachment 251872 [details] [review]:

::: src/Makefile.am
@@ +335,3 @@
+		--c-namespace MetaDBus							\
+		--generate-c-code meta-dbus-xrandr					\
+		xrandr.xml

Not sure the "xrandr" makes sense if it is an abstraction for different backends but otoh it is based in the xrandr API ...

::: src/core/monitor.c
@@ +227,3 @@
+            meta_output->output_id = resources->outputs[i];
+            meta_output->name = g_strdup (output->name);
+            meta_output->vendor = g_strdup ("unknown");

Add a comment that we don't have EDID parsing yet.

@@ +514,3 @@
 }
 
+static const double known_diagonals[] = {

Rationale for this? Or was this like that in gnome-rr as well?

@@ +702,1 @@
 static MetaMonitorManager *global_manager;

global_monitor_manager? Don't know why but "global_manager" sounds like "manages globals" to me which just sounds weird.
Comment 61 drago01 2013-08-16 16:47:40 UTC
Review of attachment 251871 [details] [review]:

::: src/core/display.c
@@ +518,3 @@
   if (meta_is_syncing ())
     XSynchronize (xdisplay, True);
+

Unrelated.

::: src/core/monitor.c
@@ +222,3 @@
+ *
+ * Returns TRUE if the only available monitor is the dummy one
+ * backing the ClutterStage window.

That's not what the code does. It always returns false. If it is going to be made useful later add a comment.
Comment 62 drago01 2013-08-16 16:48:07 UTC
Review of attachment 251871 [details] [review]:

(make splinter happy by typing something here)

::: src/core/display.c
@@ +518,3 @@
   if (meta_is_syncing ())
     XSynchronize (xdisplay, True);
+

Unrelated.

::: src/core/monitor.c
@@ +222,3 @@
+ *
+ * Returns TRUE if the only available monitor is the dummy one
+ * backing the ClutterStage window.

That's not what the code does. It always returns false. If it is going to be made useful later add a comment.
Comment 63 drago01 2013-08-16 16:53:19 UTC
Review of attachment 251871 [details] [review]:

(make splinter happy by typing something here)

::: src/core/display.c
@@ +518,3 @@
   if (meta_is_syncing ())
     XSynchronize (xdisplay, True);
+

Unrelated.

::: src/core/monitor.c
@@ +222,3 @@
+ *
+ * Returns TRUE if the only available monitor is the dummy one
+ * backing the ClutterStage window.

That's not what the code does. It always returns false. If it is going to be made useful later add a comment.
Comment 64 drago01 2013-08-16 16:53:19 UTC
Review of attachment 251871 [details] [review]:

(make splinter happy by typing something here)

::: src/core/display.c
@@ +518,3 @@
   if (meta_is_syncing ())
     XSynchronize (xdisplay, True);
+

Unrelated.

::: src/core/monitor.c
@@ +222,3 @@
+ *
+ * Returns TRUE if the only available monitor is the dummy one
+ * backing the ClutterStage window.

That's not what the code does. It always returns false. If it is going to be made useful later add a comment.
Comment 65 drago01 2013-08-16 16:53:19 UTC
Review of attachment 251871 [details] [review]:

(make splinter happy by typing something here)

::: src/core/display.c
@@ +518,3 @@
   if (meta_is_syncing ())
     XSynchronize (xdisplay, True);
+

Unrelated.

::: src/core/monitor.c
@@ +222,3 @@
+ *
+ * Returns TRUE if the only available monitor is the dummy one
+ * backing the ClutterStage window.

That's not what the code does. It always returns false. If it is going to be made useful later add a comment.
Comment 66 drago01 2013-08-16 16:53:19 UTC
Review of attachment 251871 [details] [review]:

(make splinter happy by typing something here)

::: src/core/display.c
@@ +518,3 @@
   if (meta_is_syncing ())
     XSynchronize (xdisplay, True);
+

Unrelated.

::: src/core/monitor.c
@@ +222,3 @@
+ *
+ * Returns TRUE if the only available monitor is the dummy one
+ * backing the ClutterStage window.

That's not what the code does. It always returns false. If it is going to be made useful later add a comment.
Comment 67 drago01 2013-08-16 16:53:19 UTC
Review of attachment 251871 [details] [review]:

(make splinter happy by typing something here)

::: src/core/display.c
@@ +518,3 @@
   if (meta_is_syncing ())
     XSynchronize (xdisplay, True);
+

Unrelated.

::: src/core/monitor.c
@@ +222,3 @@
+ *
+ * Returns TRUE if the only available monitor is the dummy one
+ * backing the ClutterStage window.

That's not what the code does. It always returns false. If it is going to be made useful later add a comment.
Comment 68 drago01 2013-08-16 16:53:19 UTC
Review of attachment 251871 [details] [review]:

(make splinter happy by typing something here)

::: src/core/display.c
@@ +518,3 @@
   if (meta_is_syncing ())
     XSynchronize (xdisplay, True);
+

Unrelated.

::: src/core/monitor.c
@@ +222,3 @@
+ *
+ * Returns TRUE if the only available monitor is the dummy one
+ * backing the ClutterStage window.

That's not what the code does. It always returns false. If it is going to be made useful later add a comment.
Comment 69 drago01 2013-08-16 16:54:09 UTC
Crap blame splinter for not submitting / giving any feedback.
Comment 70 drago01 2013-08-16 16:54:24 UTC
Review of attachment 251873 [details] [review]:

::: src/xrandr.xml
@@ +138,3 @@
+
+	@serial must match the serial from the last GetResources() call,
+	or org.freedesktop.DBus.AccessDenied will be generated.

Sounds racy what if some client just grabs the information and while another one wants to change the configuration?

@@ +139,3 @@
+	@serial must match the serial from the last GetResources() call,
+	or org.freedesktop.DBus.AccessDenied will be generated.
+	(XXX: a better error maybe?)

InvalidArgs ? The serial is invalid because it got invalidated by the newer GetResources call? dunno.

@@ +143,3 @@
+	If @persistent is true, mutter will attempt to replicate this
+	configuration the next time this HW layout appears.
+	(XXX: or is this gnome-settings-daemon role?)

Good question.
Comment 71 drago01 2013-08-16 16:56:11 UTC
Review of attachment 251874 [details] [review]:

OK.
Comment 72 drago01 2013-08-16 17:04:29 UTC
Review of attachment 251875 [details] [review]:

::: src/core/monitor.c
@@ +180,3 @@
+
+  manager->max_screen_width = manager->modes[0].width;
+  manager->max_screen_height = manager->modes[0].height;

As said on IRC we should limit that to the maximum texture size for now (can be done in a later patch though).

@@ +961,3 @@
+      guint output_id;
+
+      if (crtc_id < 0 || crtc_id >= manager->n_crtcs)

The first condition cannot happen crtc_id is unsigned.

@@ +1005,3 @@
+          MetaOutput *output;
+
+          if (output_id < 0 || output_id >= manager->n_outputs)

output_id is unsigned as well.

@@ +1048,3 @@
+  while (g_variant_iter_loop (&output_iter, "(ua{sv})", &output_id, NULL))
+    {
+      if (output_id < 0 || output_id >= manager->n_outputs)

Again first condition cannot happen.
Comment 73 drago01 2013-08-16 17:06:53 UTC
Review of attachment 251876 [details] [review]:

OK.

::: src/core/display.c
@@ +2832,3 @@
                                                     &event->xconfigure);
             }
+

Unrelated whitespace change.
Comment 74 drago01 2013-08-16 17:08:45 UTC
Review of attachment 251877 [details] [review]:

OK.
Comment 75 Giovanni Campagna 2013-08-16 17:09:35 UTC
Created attachment 251916 [details] [review]
MonitorXrandr: expose the subpixel order from X11

Might be inaccurate in the face of rotation. Cogl has some code
to account for that, but
a) it's not clear which value we should expose in the wayland protocol
b) the Cogl code is obviously wrong in that it just discards the
   X value
c) the subpixel order is only used by wayland, so in the end...
   why bother?
Comment 76 Giovanni Campagna 2013-08-16 17:09:41 UTC
Created attachment 251917 [details] [review]
MonitorXrandr: follow the right order in applying the new configuration

First disable CRTCs that should be off in the new configuration,
then resize the framebuffer, then enable the new CRTCs.
If we don't do that, and we're making the screen smaller, X complains
with a BadMatch.
Comment 77 drago01 2013-08-16 17:10:13 UTC
Review of attachment 251878 [details] [review]:

OK.

::: src/compositor/plugins/default.c
@@ +30,1 @@
+#include <stdlib.h>

Why?
Comment 78 drago01 2013-08-16 17:12:34 UTC
Review of attachment 251879 [details] [review]:

::: src/core/monitor-private.h
@@ +49,3 @@
+
+#ifndef HAVE_WAYLAND
+enum wl_output_transform {

Hmm thinking about this some more .. do we really want to mix wayland constants into the other backends? Wouldn't it make more sense to have a meta_output_transform ? Or is this API supposed to be implemented by someone else other then mutter?
Comment 79 drago01 2013-08-16 17:19:40 UTC
Review of attachment 251880 [details] [review]:

OK.
Comment 80 drago01 2013-08-16 17:23:35 UTC
Review of attachment 251881 [details] [review]:

OK.
Comment 81 drago01 2013-08-16 17:51:35 UTC
Review of attachment 251882 [details] [review]:

OK.
Comment 82 drago01 2013-08-16 18:20:35 UTC
Review of attachment 251883 [details] [review]:

OK.
Comment 83 drago01 2013-08-16 18:23:13 UTC
Review of attachment 251884 [details] [review]:

::: src/core/monitor-config.c
@@ +852,3 @@
+ * best resolution
+ *
+ * Input assertions: there is at least one output

Why no g_return_if_fail or g_assert then?
Comment 84 drago01 2013-08-16 18:24:43 UTC
Review of attachment 251886 [details] [review]:

OK
Comment 85 drago01 2013-08-16 18:28:40 UTC
Review of attachment 251887 [details] [review]:

OK, note: did not test on a device that actually has back light control.

::: src/core/monitor.c
@@ +437,3 @@
+
+  if (actual_type != XA_INTEGER || actual_format != 32 ||
+      nitems < 1)

nitems is unsigned so that is impossible.

@@ +1799,3 @@
+    }
+
+  if (output_id < 0 || output_id >= manager->n_outputs)

You are doing a "guint < 0" again.
Comment 86 drago01 2013-08-16 18:29:11 UTC
Review of attachment 251888 [details] [review]:

OK.
Comment 87 drago01 2013-08-16 18:31:39 UTC
Review of attachment 251889 [details] [review]:

OK, needs fixes for the unsigned < 0 checks.

::: src/core/monitor.c
@@ +1913,3 @@
+    }
+
+  if (crtc_id < 0 || crtc_id >= manager->n_crtcs)

Again that can't be true.

@@ +2000,3 @@
+    }
+
+  if (crtc_id < 0 || crtc_id >= manager->n_crtcs)

Same here.
Comment 88 drago01 2013-08-16 18:36:21 UTC
Review of attachment 251890 [details] [review]:

OK, but note did test on a desktop so could not test this as well.
Comment 89 drago01 2013-08-16 18:47:35 UTC
Review of attachment 251891 [details] [review]:

Do we need a gnome-shell change for this?
Comment 90 drago01 2013-08-16 18:50:13 UTC
Review of attachment 251892 [details] [review]:

Hmm .. I'd rather have an abstract class and have the dummy backend subclass it as well.
Comment 91 drago01 2013-08-16 18:52:04 UTC
Review of attachment 251893 [details] [review]:

OK.
Comment 92 drago01 2013-08-16 18:55:40 UTC
Review of attachment 251894 [details] [review]:

Makes sense, maybe squash into the other patch?
Comment 93 drago01 2013-08-16 18:57:18 UTC
Review of attachment 251895 [details] [review]:

Files are missing. Forgot to git-add ?
Comment 94 drago01 2013-08-16 18:58:38 UTC
Review of attachment 251896 [details] [review]:

OK.
Comment 95 drago01 2013-08-16 18:59:57 UTC
Review of attachment 251897 [details] [review]:

OK.

::: src/core/monitor-xrandr.c
@@ +48,3 @@
+ * http://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/xsettings/gsd-xsettings-manager.c
+ * for the reasoning */
+#define DPI_FALLBACK 96.0

But but the internets told me that this is wrong ;)
Comment 96 drago01 2013-08-16 19:01:28 UTC
Review of attachment 251916 [details] [review]:

"why bother?" ... why add code that serves no real purpose then?
Comment 97 drago01 2013-08-16 19:04:55 UTC
Review of attachment 251917 [details] [review]:

OK.
Comment 98 Giovanni Campagna 2013-08-17 14:14:08 UTC
(In reply to comment #78)
> Review of attachment 251879 [details] [review]:
> 
> ::: src/core/monitor-private.h
> @@ +49,3 @@
> +
> +#ifndef HAVE_WAYLAND
> +enum wl_output_transform {
> 
> Hmm thinking about this some more .. do we really want to mix wayland constants
> into the other backends? Wouldn't it make more sense to have a
> meta_output_transform ? Or is this API supposed to be implemented by someone
> else other then mutter?

Well, it's either that or XRandR constants - I don't want to have another enum just to translate back and forth.
XRandR Rotation is slightly more complicate to understand (it's bit mask of a rotation flag and one reflection flag), so I chose wl_output_transform.
I thought of having a MetaOutputTransform that was bit-compatible with wl_output_transform, but what's the point? The enum is always there anyway - gtk is compiled with wayland support these days.

(In reply to comment #83)
> Review of attachment 251884 [details] [review]:
> 
> ::: src/core/monitor-config.c
> @@ +852,3 @@
> + * best resolution
> + *
> + * Input assertions: there is at least one output
> 
> Why no g_return_if_fail or g_assert then?

Because if the assertion is violated, it returns NULL and crashes immediately after, so it's equivalent to a g_assert (except it's segv rather than abrt...)

(In reply to comment #85)
> Review of attachment 251887 [details] [review]:
> 
> OK, note: did not test on a device that actually has back light control.
> 
> ::: src/core/monitor.c
> @@ +437,3 @@
> +
> +  if (actual_type != XA_INTEGER || actual_format != 32 ||
> +      nitems < 1)
> 
> nitems is unsigned so that is impossible.

I believe 0 is a valid value for unsigned :)

> ::: src/core/monitor.c
> @@ +222,3 @@
> + *
> + * Returns TRUE if the only available monitor is the dummy one
> + * backing the ClutterStage window.
> 
> That's not what the code does. It always returns false. If it is going to be
> made useful later add a comment.

Well, not later, just in a different branch.
(Also, if I remove it, suddenly the dummy backend becomes unused code and triggers warnings, but I really don't want to remove it or postpone the addition to the patch adding the env var)

(In reply to comment #70)
> Review of attachment 251873 [details] [review]:
> 
> ::: src/xrandr.xml
> @@ +138,3 @@
> +
> +    @serial must match the serial from the last GetResources() call,
> +    or org.freedesktop.DBus.AccessDenied will be generated.
> 
> Sounds racy what if some client just grabs the information and while another
> one wants to change the configuration?

The serial is only incremented when something happens upstream (X emitted an event), so if a client calls GetResources() while another calls ApplyConfiguration(), the Apply calls succeeds, but the client's information immediately becomes stale.
The querying client will at some point get XRRScreenNotify or wl_output_geometry, and then call GetResources(). If that doesn't happen, and it calls ApplyConfiguration too, it will fail, which is safe.
In practice, there are only two clients of the API: gnome-settings-daemon and gnome-control-center, so the risk of races is just theoretical.

> @@ +139,3 @@
> +    @serial must match the serial from the last GetResources() call,
> +    or org.freedesktop.DBus.AccessDenied will be generated.
> +    (XXX: a better error maybe?)
> 
> InvalidArgs ? The serial is invalid because it got invalidated by the newer
> GetResources call? dunno.

I wanted to reserve InvalidArgs for errors that can be prevented by good client side coding.

> @@ +143,3 @@
> +    If @persistent is true, mutter will attempt to replicate this
> +    configuration the next time this HW layout appears.
> +    (XXX: or is this gnome-settings-daemon role?)
> 
> Good question.

Well, it's answered by later patches - g-s-d's xrandr plugin will die, but 3.10 or 3.12.

(In reply to comment #89)
> Review of attachment 251891 [details] [review]:
> 
> Do we need a gnome-shell change for this?

Definitely. And I need to write it...

(In reply to comment #90)
> Review of attachment 251892 [details] [review]:
> 
> Hmm .. I'd rather have an abstract class and have the dummy backend subclass it
> as well.

What would it buy us, beyond more GObject boilerplate in the header file and more casts?
Comment 99 Giovanni Campagna 2013-08-17 14:20:24 UTC
(In reply to comment #60)
> Review of attachment 251872 [details] [review]:
> 
> ::: src/Makefile.am
> @@ +335,3 @@
> +        --c-namespace MetaDBus                            \
> +        --generate-c-code meta-dbus-xrandr                    \
> +        xrandr.xml
> 
> Not sure the "xrandr" makes sense if it is an abstraction for different
> backends but otoh it is based in the xrandr API ...

Yeah, originally the DBus API was called org.gnome.Mutter.XRandR, but gdbus-codegen would generate a terrible name (xrand_r), so I switched to DisplayConfig.
This is "XRandR for wayland" after all.

> @@ +514,3 @@
>  }
> 
> +static const double known_diagonals[] = {
> 
> Rationale for this? Or was this like that in gnome-rr as well?

Copy pasted for gnome-desktop.
Comment 100 Giovanni Campagna 2013-08-17 14:58:58 UTC
Created attachment 252023 [details] [review]
Rework and consolidate monitor handling in MetaScreen

Consolidate all places that deal with output configuration in
MetaScreen, which gets it either from XRandR or from a dummy static configuration.
We still need to read the Xinerama config, even when running xwayland,
because we need the indices for _NET_WM_FULLSCREEN_MONITORS, but
now we do it only when needed.
Comment 101 Giovanni Campagna 2013-08-17 14:59:03 UTC
Created attachment 252024 [details] [review]
Introduce a new DBus interface for display configuration

This new interface will be used by the control center and possibly
the settings daemon to configure the screens. It is designed to
resemble a simplified XRandR, while still exposing all the quirks
of the hardware, so that the panel can limit the user choices
appropriately.

To do so, MetaMonitorMode needs to track CRTCs, outputs and modes,
so the low level objects have been decoupled from the high-level
MetaMonitorInfo, which is used by core and API and offers a simplified
view of HW, that hides away the details of what is cloned and how.
This is still not efficient as it should be, because on every
HW change we drop all data structures and rebuild them from scratch
(which is not expensive because there aren't many of them, but
at least in the XRandR path it involves a few sync X calls)
Comment 102 Giovanni Campagna 2013-08-17 14:59:09 UTC
Created attachment 252025 [details] [review]
Add the write side of the DBus protocol too

This is just in the documentation for now, to attract wider feedback
before we start looking at how to implement this for real.
Comment 103 Giovanni Campagna 2013-08-17 14:59:16 UTC
Created attachment 252026 [details] [review]
Extend the DBus XRandR protocol to expose cloning restriction

Turns out that even if two outputs say that they can be controlled
by a given CRTC, you can't configure them in the same CRTC unless
they are marked as "possible clones" one of the other.
This can further restrict the configuration options, so we need
to expose this limitation in the DBus API.
Comment 104 Giovanni Campagna 2013-08-17 14:59:22 UTC
Created attachment 252027 [details] [review]
DisplayConfig: add the write side of the API

Implement ApplyConfiguration in terms of XRandR calls.
Error checking is done before actually committing the configuration.

If mutter is using one of the other monitor config backends, an
error is reported and nothing happens.
Comment 105 Giovanni Campagna 2013-08-17 14:59:30 UTC
Created attachment 252028 [details] [review]
Reverse handling of XRandR events between Screen and MonitorManager

Now MonitorManager does its own handling of XRandR events, which
means we no longer handle ConfigureNotify on the root window.
MetaScreen reacts to MonitorManager::monitor-changed and updates
its internal state, including the new size.

This paves the way for doing display configuration using only
the dummy backend, which would allow testing wl_output interfaces.
Comment 106 Giovanni Campagna 2013-08-17 14:59:38 UTC
Created attachment 252029 [details] [review]
DisplayConfig: make the dummy backend writable

Add a number of dummy outputs and modes to the dummy backend,
and implement the writing bits.
The only visible effect is that you can change the screen size,
which resizes the output window.
Comment 107 Giovanni Campagna 2013-08-17 14:59:44 UTC
Created attachment 252030 [details] [review]
default plugin: add a random color background on each monitor

Instead of a full white background, make one with a random color.
This way the different "monitors" are visible and it's easier
to debug the DBus API.
Comment 108 Giovanni Campagna 2013-08-17 14:59:49 UTC
Created attachment 252031 [details] [review]
MonitorManager: fix handling of output transform

Read the current transform from XRandR, and expose the transforms
that are really supported on the bus.
The dummy backend now advertises all transforms, since it doesn't
actually apply them.
Comment 109 Giovanni Campagna 2013-08-17 14:59:56 UTC
Created attachment 252032 [details] [review]
MonitorManager: inherit directly from DisplayConfig instead of handling signals

This way we can handle properties too.
Comment 110 Giovanni Campagna 2013-08-17 15:00:03 UTC
Created attachment 252033 [details] [review]
MonitorManager: add support for DPMS levels

To the XRandR and dummy backend (and as usual the dummy backend
has no effect)
Comment 111 Giovanni Campagna 2013-08-17 15:00:11 UTC
Created attachment 252034 [details] [review]
MonitorManager: add support for persistent monitor configurations

Add a new object, MetaMonitorConfig, that takes care of converting
between the logical configurations stored in monitors.xml and
the HW resources exposed by MonitorManager.
This commit includes loading and saving of configurations, but
still missing is the actual CRTC assignments and a default
configuration when none is found in the file.
Comment 112 Giovanni Campagna 2013-08-17 15:00:21 UTC
Created attachment 252035 [details] [review]
MonitorConfig: add CRTC assignment

Ripped off libgnome-desktop, trimming the parts that checked
that the configuration was plausible, as that should be done
in gnome-control-center before asking mutter for a change.
Comment 113 Giovanni Campagna 2013-08-17 15:00:33 UTC
Created attachment 252036 [details] [review]
MonitorConfig: add support for default configurations

Activate the presentation bit on new hotplugged monitors, while
making a fully extended setup when running for the first time.
Comment 114 Giovanni Campagna 2013-08-17 15:00:40 UTC
Created attachment 252037 [details] [review]
MonitorManager: store the presentation mode bit in XRandR

Use a private output property to store if the output is in
presentation mode or not, so that this information is not lost
after the configuration read back from the server.
Comment 115 Giovanni Campagna 2013-08-17 15:00:52 UTC
Created attachment 252038 [details] [review]
MonitorManager: further extend the dummy backend

The default configuration is extended, which is only possible
if there are as many CRTCs as outputs, so make sure that's true.

Also, add more and bigger modes, so that different sizes will
be chosen for the three outputs.
A nice side effect of this is that with a real 1920x1080 + 1600x900
layout, if you disable the VGA you get a stage that matches the
screen size, which triggers the legacy fullscreen path in the
outside mutter.
Comment 116 Giovanni Campagna 2013-08-17 15:00:57 UTC
Created attachment 252039 [details] [review]
MonitorManager: add support for backlight

GnomeRR needs that too.
The backlight is exported as a normalized 0-100 value, or -1 if not
supported. Clamping to HW limits is handled by the backend.
Changing backlight uses a different method call, to avoid recomputing
the full display configuration every time the user presses the
backlight keys.
Comment 117 Giovanni Campagna 2013-08-17 15:01:05 UTC
Created attachment 252040 [details] [review]
MonitorManager: ignore configuration changes that disable all outputs

If we compute a screen size of 0 in either direction, we crash
later on, as it is invalid for clutter, cogl and X.
Comment 118 Giovanni Campagna 2013-08-17 15:01:12 UTC
Created attachment 252041 [details] [review]
MonitorManager: add gamma support

Add GetCrtcGamma() and SetCrtcGamma(), that wrap the similarly
named XRandR API. These are used by GnomeRR inside the color
plugin of the control center (and may go away if the color
plugin decides to do something different under wayland)
Comment 119 Giovanni Campagna 2013-08-17 15:01:17 UTC
Created attachment 252042 [details] [review]
MonitorConfig: handle changes in the laptop lid

This way we don't need to track the current and previous
configuration in gnome-settings-daemon, when we already do so
in mutter.
Comment 120 Giovanni Campagna 2013-08-17 15:01:24 UTC
Created attachment 252043 [details] [review]
MetaPlugin: add a UI hook for confirming display changes

We want to show a dialog when a display change happens from the
control center. To do so, add a new vfunc to MetaPlugin and
call it when a configuration change is requested via DBus.
Comment 121 Giovanni Campagna 2013-08-17 15:01:30 UTC
Created attachment 252044 [details] [review]
MonitorManager: split the XRandR parts in a subclass

Instead of keeping a forest of if backend else ..., use a subclass
and virtual functions to discriminate between XRandR and the
dummy backend (which lives in the parent class togheter with the
common code)
Comment 122 Giovanni Campagna 2013-08-17 15:01:36 UTC
Created attachment 252045 [details] [review]
MonitorManager: add EDID properties to the output DBus description

Add "edid-file", if we have one (in the KMS case, where we can point
people to the right sysfs file), or "edid" with inline data.
These are needed by colord to build the default ICC profile for
uncalibrated displays.
Comment 123 Giovanni Campagna 2013-08-17 15:01:42 UTC
Created attachment 252046 [details] [review]
MonitorXrandr: implement correct EDID parsing

To provide valid values for the vendor, product and serial fields
we need to read the EDID and parse it.
Parser kindly provided by gnome-desktop.
Comment 124 Giovanni Campagna 2013-08-17 15:01:48 UTC
Created attachment 252047 [details] [review]
Monitor: restore correct display name handling

Now that we have the right values from the EDID, we can load
the PNP database and find the proper vendor name, to show in
the control center UI.
Comment 125 Giovanni Campagna 2013-08-17 15:01:56 UTC
Created attachment 252048 [details] [review]
MonitorXrandr: resize the framebuffer prior to setting the CRTC configuration

Otherwise X11 will trim the new configuration and disable outputs
outside the screen.
Comment 126 Giovanni Campagna 2013-08-17 15:02:02 UTC
Created attachment 252049 [details] [review]
MonitorXrandr: follow the right order in applying the new configuration

First disable CRTCs that should be off in the new configuration,
then resize the framebuffer, then enable the new CRTCs.
If we don't do that, and we're making the screen smaller, X complains
with a BadMatch.
Comment 127 Giovanni Campagna 2013-08-17 15:04:13 UTC
Note: there are some changes in this stack related to transform handling (namely, we needed to swap width/height in some cases and we weren't doing it)
Also, this is a lot more tested now.

I'll restore acn status based on the previous review.
Comment 128 drago01 2013-08-17 15:17:33 UTC
(In reply to comment #98)
> (In reply to comment #78)
> > Review of attachment 251879 [details] [review] [details]:
> > 
> > ::: src/core/monitor-private.h
> > @@ +49,3 @@
> > +
> > +#ifndef HAVE_WAYLAND
> > +enum wl_output_transform {
> > 
> > Hmm thinking about this some more .. do we really want to mix wayland constants
> > into the other backends? Wouldn't it make more sense to have a
> > meta_output_transform ? Or is this API supposed to be implemented by someone
> > else other then mutter?
> 
> Well, it's either that or XRandR constants - I don't want to have another enum
> just to translate back and forth.
> XRandR Rotation is slightly more complicate to understand (it's bit mask of a
> rotation flag and one reflection flag), so I chose wl_output_transform.
> I thought of having a MetaOutputTransform that was bit-compatible with
> wl_output_transform, but what's the point? The enum is always there anyway -
> gtk is compiled with wayland support these days.

OK.

> (In reply to comment #83)
> > Review of attachment 251884 [details] [review] [details]:
> > 
> > ::: src/core/monitor-config.c
> > @@ +852,3 @@
> > + * best resolution
> > + *
> > + * Input assertions: there is at least one output
> > 
> > Why no g_return_if_fail or g_assert then?
> 
> Because if the assertion is violated, it returns NULL and crashes immediately
> after, so it's equivalent to a g_assert (except it's segv rather than abrt...)

Not really the same. Don't segfault abort(). It is not only "cleaner" but is more obvious why it crashed. And it is not that much of extra code ;)

> (In reply to comment #85)
> > Review of attachment 251887 [details] [review] [details]:
> > 
> > OK, note: did not test on a device that actually has back light control.
> > 
> > ::: src/core/monitor.c
> > @@ +437,3 @@
> > +
> > +  if (actual_type != XA_INTEGER || actual_format != 32 ||
> > +      nitems < 1)
> > 
> > nitems is unsigned so that is impossible.
> 
> I believe 0 is a valid value for unsigned :)

Oops ... apparently I stopped reading after the "<" ;)

> > ::: src/core/monitor.c
> > @@ +222,3 @@
> > + *
> > + * Returns TRUE if the only available monitor is the dummy one
> > + * backing the ClutterStage window.
> > 
> > That's not what the code does. It always returns false. If it is going to be
> > made useful later add a comment.
> 
> Well, not later, just in a different branch.
> (Also, if I remove it, suddenly the dummy backend becomes unused code and
> triggers warnings, but I really don't want to remove it or postpone the
> addition to the patch adding the env var)

OK, adding a comment still wouldn't hurt.

> (In reply to comment #70)
> > Review of attachment 251873 [details] [review] [details]:
> > 
> > ::: src/xrandr.xml
> > @@ +138,3 @@
> > +
> > +    @serial must match the serial from the last GetResources() call,
> > +    or org.freedesktop.DBus.AccessDenied will be generated.
> > 
> > Sounds racy what if some client just grabs the information and while another
> > one wants to change the configuration?
> 
> The serial is only incremented when something happens upstream (X emitted an
> event), so if a client calls GetResources() while another calls
> ApplyConfiguration(), the Apply calls succeeds, but the client's information
> immediately becomes stale.
> The querying client will at some point get XRRScreenNotify or
> wl_output_geometry, and then call GetResources(). If that doesn't happen, and
> it calls ApplyConfiguration too, it will fail, which is safe.
> In practice, there are only two clients of the API: gnome-settings-daemon and
> gnome-control-center, so the risk of races is just theoretical.

OK, fair enough.

> > @@ +139,3 @@
> > +    @serial must match the serial from the last GetResources() call,
> > +    or org.freedesktop.DBus.AccessDenied will be generated.
> > +    (XXX: a better error maybe?)
> > 
> > InvalidArgs ? The serial is invalid because it got invalidated by the newer
> > GetResources call? dunno.
> 
> I wanted to reserve InvalidArgs for errors that can be prevented by good client
> side coding.

OK.

> > @@ +143,3 @@
> > +    If @persistent is true, mutter will attempt to replicate this
> > +    configuration the next time this HW layout appears.
> > +    (XXX: or is this gnome-settings-daemon role?)
> > 
> > Good question.
> 
> Well, it's answered by later patches - g-s-d's xrandr plugin will die, but 3.10
> or 3.12.

"but 3.10 or 3.12" what?

> (In reply to comment #89)
> > Review of attachment 251891 [details] [review] [details]:
> > 
> > Do we need a gnome-shell change for this?
> 
> Definitely. And I need to write it...

OK

> (In reply to comment #90)
> > Review of attachment 251892 [details] [review] [details]:
> > 
> > Hmm .. I'd rather have an abstract class and have the dummy backend subclass it
> > as well.
> 
> What would it buy us, beyond more GObject boilerplate in the header file and
> more casts?

A "cleaner design" but you have a point ...
Comment 129 Giovanni Campagna 2013-08-17 17:20:42 UTC
Created attachment 252056 [details] [review]
MonitorXrandr: update the internal data structures after applying

We were relying on the XRandR events from the X server to update
the configuration, but we were calling meta_monitor_config_update_current()
immediately after, so the MonitorConfig would be updated with the
old configuration (and we would save that to disk!)
Comment 130 drago01 2013-08-17 19:27:05 UTC
Review of attachment 252023 [details] [review]:

OK, with the comment added.

::: src/core/monitor.c
@@ +225,3 @@
+ */
+static gboolean
+has_dummy_output (void)

As I said in the other comment please add a comment why we have a function that always returns FALSE.
Comment 131 drago01 2013-08-17 19:28:47 UTC
Review of attachment 252024 [details] [review]:

OK.
Comment 132 drago01 2013-08-17 19:29:40 UTC
Review of attachment 252025 [details] [review]:

OK.
Comment 133 drago01 2013-08-17 19:33:46 UTC
Review of attachment 252027 [details] [review]:

OK, with var renamed.

::: src/core/monitor-private.h
@@ +97,3 @@
+
+  /* Used when changing configuration */
+  gboolean dirty;

s/dirty/is_dirty/ (yeah somehow forgot to note that in the previous review).
Comment 134 drago01 2013-08-17 19:35:36 UTC
Review of attachment 252031 [details] [review]:

OK.
Comment 135 drago01 2013-08-17 19:37:20 UTC
Review of attachment 252036 [details] [review]:

OK.
Comment 136 drago01 2013-08-17 19:38:23 UTC
Review of attachment 252037 [details] [review]:

OK.
Comment 137 drago01 2013-08-17 19:41:18 UTC
Review of attachment 252039 [details] [review]:

OK, with style fixes (still didn't test I assume you did).

::: src/core/monitor.c
@@ +414,3 @@
+                     int         hw_value)
+{
+  return round((double)(hw_value - output->backlight_min) /

Missing whitespace.

@@ +1787,3 @@
+  int hw_value;
+
+  hw_value = round((double)value / 100.0 * output->backlight_max + output->backlight_min);

Missing whitespace.
Comment 138 drago01 2013-08-17 19:42:23 UTC
Review of attachment 252041 [details] [review]:

OK.
Comment 139 drago01 2013-08-17 19:45:14 UTC
Review of attachment 252042 [details] [review]:

OK, with style fixed (still didn't test on a laptop though).

::: src/core/monitor-config.c
@@ +1268,3 @@
+{
+  if (self->previous)
+    {

Remove for one line if.
Comment 140 drago01 2013-08-17 20:07:30 UTC
Review of attachment 252044 [details] [review]:

OK.

::: src/core/monitor-private.h
@@ +215,3 @@
+  /* XXX: this structure is very badly
+     packed, but I like the logical organization
+     of fields */

Just kill this comment.
Comment 141 drago01 2013-08-17 20:09:57 UTC
Review of attachment 252046 [details] [review]:

Broken coding style please fix.

::: src/core/edid-parse.c
@@ +78,3 @@
+    switch (edid[0x10])
+    {
+    case 0x00:

Broken indentation.

@@ +93,3 @@
+
+    if (is_model_year)
+    {

Same here .. I know it is copy and pasted but still should match our coding style.

::: src/core/monitor-xrandr.c
@@ +264,3 @@
+    }
+  else
+    {

Remove braces.
Comment 142 drago01 2013-08-17 20:10:32 UTC
Review of attachment 252047 [details] [review]:

OK.
Comment 143 drago01 2013-08-17 20:11:13 UTC
Review of attachment 252049 [details] [review]:

OK.
Comment 144 drago01 2013-08-17 20:11:52 UTC
Review of attachment 252056 [details] [review]:

OK.
Comment 145 Giovanni Campagna 2013-08-17 22:54:09 UTC
Pushed with the suggested changes. Now let's wait for the regressions! :)

Attachment 252023 [details] pushed as 214f312 - Rework and consolidate monitor handling in MetaScreen
Attachment 252024 [details] pushed as 3bb33d3 - Introduce a new DBus interface for display configuration
Attachment 252025 [details] pushed as 7e1d100 - Add the write side of the DBus protocol too
Attachment 252026 [details] pushed as dc242e4 - Extend the DBus XRandR protocol to expose cloning restriction
Attachment 252027 [details] pushed as 57d0837 - DisplayConfig: add the write side of the API
Attachment 252028 [details] pushed as bf40409 - Reverse handling of XRandR events between Screen and MonitorManager
Attachment 252029 [details] pushed as c354e7e - DisplayConfig: make the dummy backend writable
Attachment 252030 [details] pushed as fc67c70 - default plugin: add a random color background on each monitor
Attachment 252031 [details] pushed as 522542c - MonitorManager: fix handling of output transform
Attachment 252032 [details] pushed as dbd8d4d - MonitorManager: inherit directly from DisplayConfig instead of handling signals
Attachment 252033 [details] pushed as e039add - MonitorManager: add support for DPMS levels
Attachment 252034 [details] pushed as 8f46212 - MonitorManager: add support for persistent monitor configurations
Attachment 252035 [details] pushed as d0529b7 - MonitorConfig: add CRTC assignment
Attachment 252036 [details] pushed as 764c472 - MonitorConfig: add support for default configurations
Attachment 252037 [details] pushed as 5c27a91 - MonitorManager: store the presentation mode bit in XRandR
Attachment 252038 [details] pushed as 849050b - MonitorManager: further extend the dummy backend
Attachment 252039 [details] pushed as 8b52782 - MonitorManager: add support for backlight
Attachment 252040 [details] pushed as cd20f1b - MonitorManager: ignore configuration changes that disable all outputs
Attachment 252041 [details] pushed as 3b61b85 - MonitorManager: add gamma support
Attachment 252042 [details] pushed as bbbcd8c - MonitorConfig: handle changes in the laptop lid
Attachment 252043 [details] pushed as 5086626 - MetaPlugin: add a UI hook for confirming display changes
Attachment 252044 [details] pushed as 46de0ed - MonitorManager: split the XRandR parts in a subclass
Attachment 252045 [details] pushed as 5707743 - MonitorManager: add EDID properties to the output DBus description
Attachment 252046 [details] pushed as 6946784 - MonitorXrandr: implement correct EDID parsing
Attachment 252047 [details] pushed as 3bb5086 - Monitor: restore correct display name handling
Attachment 252048 [details] pushed as 0986b66 - MonitorXrandr: resize the framebuffer prior to setting the CRTC configuration
Attachment 252049 [details] pushed as 3528b06 - MonitorXrandr: follow the right order in applying the new configuration
Attachment 252056 [details] pushed as 3112794 - MonitorXrandr: update the internal data structures after applying