First batch landed:

https://git.gnome.org/browse/mutter/log/?qt=range&q=9b64e09a24643264a537970572deaabe1c1098b7..6fa8238ebf5f1a1854b748533ad99ea15d647896

Current state:

* Disabled by default
* Missing D-Bus API (read part available on https://github.com/jadahl/mutter/commits/wip/monitor-config )
* Doesn't write config to disk
* Configuring transforms (rotation) missing
* Configuring scale (available on https://github.com/jadahl/mutter/commits/wip/monitor-config )

fwiw I filed bug 779001 because this braks mutter standalone for me.

Is far as I read your commits of today, my issue #771559 seems resolved if control-center gets enabled to control the scaling factor (via your "int->float" commit) appropriately, am I right?

If mutter is able to scale monitors individually, that would be really great, I'd need this at work! :D

(In reply to Frank from comment #3)
> Is far as I read your commits of today, my issue #771559 seems resolved if
> control-center gets enabled to control the scaling factor (via your
> "int->float" commit) appropriately, am I right?

The compositor will report what scales are supported. Fractional scaling is not supported yet, its only the API that has been adapted to eventually make it possible.

> 
> If mutter is able to scale monitors individually, that would be really
> great, I'd need this at work! :D

You can try that by enabling the relevant experimental features (see the gscheme file). Note that things like the shell chrome won't be displayed properly yet, it is still in the experimental phase.

(Second batch landed last week: https://git.gnome.org/browse/mutter/log/?qt=range&q=53a93deafc36d5a352447744783a3849b3c21ff3..c214eb15bf836e41c788d709b852004200cd88f2 )

Created attachment 349759 [details] [review]
monitor-config-manager: Handle comparing different sized config keys

The guard for handling size differences between keys were broken, it
only checked if the key passed by the second argument ended up being
shorter.

Created attachment 349760 [details] [review]
monitor-config-store: Replace key when replacing config

g_hash_table_insert() doesn't replace the key. This was a problem
because the key was owned by the value inserted into the hash table, so
when a value was removed, the key was freed, meaning that the key in
the hash table was no pointing to freed memory. Fix this by using
g_hash_table_replace() instead, which work the same except that it
replaces the key with the one passed. This means that the key of a
value in the hash table is always the key owned by the value.

Review of attachment 349759 [details] [review]:

++

Review of attachment 349760 [details] [review]:

right, g_hash_table_insert() strikes again

Attachment 349759 [details] pushed as 3f107da - monitor-config-manager: Handle comparing different sized config keys
Attachment 349760 [details] pushed as 0608ae2 - monitor-config-store: Replace key when replacing config

Created attachment 351028 [details] [review]
backend: Make the getter use the cached ui scale

The cached ui scale is kept up to date, so don't recalculate it
everytime meta_backend_get_ui_scaling() is called.

Created attachment 351029 [details] [review]
monitor-manager/xrandr: Use xcb API to configure CRTC

Use xcb-randr instead of libXrandr to set the CRTC configuration. This
is needed because data from the reply will later be used.

Created attachment 351030 [details] [review]
backend: Make X11 display opened function explicitly named

It didn't say anything about being the X11 display, so make it say so.

Created attachment 351031 [details] [review]
backend: Move settings into a new MetaSettings object

Introduce MetaSettings and add the settings managed by MetaBackend into
the new object. These settings include: experimental-features and UI
scaling factor.

Created attachment 351032 [details] [review]
monitor-manager/kms: Move global ui scaling setting to MetaSettings

It'll be used elsewhere, so shouldn't be in MetaMonitorManagerKms.

Created attachment 351033 [details] [review]
monitor-manager/kms: Move scale calculation to MetaMonitor

The scale calculation doesn't really have anything to do with KMS, and
eventually we'll want to have mutter calculate the monitor scale for
non-KMS backends too, so move the scale calculation to MetaMonitor.

Created attachment 351034 [details] [review]
window/wayland: Don't try to resize window on tear down

When exiting mutter, don't try to resize maximized windows when
unmanaging, as at this point, they will have no MetaWaylandSurface.
Originally this was done instead of setting the net_wm_state to not
mess with future window managers, but when we're a Wayland compositor,
this does not matter.

Created attachment 351035 [details] [review]
gschema: Add description for "monitor-config-manager"

Created attachment 351036 [details] [review]
monitor-manager/xrandr: Allow configuring scales on X11 too

This commit makes it possible to configure logical monitor scale also
when running on top of an X11 server using Xrandr. An extra property
'requires-globla-scale' is added to the D-Bus API is added to instruct
a configuration application to only allow setting a global logical
monitor scale.

This is needed to let gsd-xsettings use the configured state to set a
XSettings state that respects the explicit monitor configuration.

Created attachment 351037 [details] [review]
(gnome-shell) shell-global: Use MetaSettings for settings

Scale settings were moved from MetaBackend to a new MetaSettings.

Comment on attachment 351034 [details] [review]
window/wayland: Don't try to resize window on tear down

Marking "window/wayland: Don't try to resize window on tear down" as obsolete. It is unrelated, and I'll attach that to a separate bug.

Review of attachment 351028 [details] [review]:

Looks good. The usage in experimental_features_changed() seems like it could be removed since the notification should be done automatically

Review of attachment 351030 [details] [review]:

::: src/Makefile.am
@@ +83,3 @@
 	backends/meta-backend.c			\
 	meta/meta-backend.h			\
+	meta/meta-settings.h			\

this seems unrelated

::: src/backends/meta-backend-private.h
@@ +39,3 @@
 #include "backends/meta-pointer-constraint.h"
 #include "backends/meta-renderer.h"
+#include "backends/meta-settings-private.h"

unrelated

Review of attachment 351030 [details] [review]:

I see you just added those makefile/include changes in the wrong patch, they just need to be moved to the following one

Review of attachment 351031 [details] [review]:

looks fine

::: src/backends/meta-monitor.c
@@ +26,3 @@
 #include "backends/meta-backend-private.h"
 #include "backends/meta-monitor-manager-private.h"
+#include "backends/meta-settings-private.h"

?

::: src/backends/meta-settings.c
@@ +256,3 @@
+#if 0
+void
+meta_settings_register_proxy (MetaSettings      *settings,

do we need to keep this #if 0'd code around?

Review of attachment 351032 [details] [review]:

::: src/backends/meta-settings.c
@@ +356,3 @@
+  /* Chain up inter-dependent settings. */
+  g_signal_connect (settings, "global-scaling-factor-changed",
+                    G_CALLBACK (meta_settings_update_ui_scaling_factor), NULL);

connecting to our own signals looks strange, why not call this directly when needed?

Review of attachment 351033 [details] [review]:

Ah, I see how that meta-settings-private.h header showed up in meta-monitor.c in a previous commit :-)

Review of attachment 351035 [details] [review]:

I think we should also enable this by default in master as we talked about on IRC

Review of attachment 351037 [details] [review]:

ok

Review of attachment 351036 [details] [review]:

seems right but didn't test this yet

::: src/backends/x11/meta-monitor-manager-xrandr.c
@@ +1120,3 @@
 
+static gboolean
+is_crtc_assignment_unchanged (MetaCrtc      *crtc,

all the double negations caused by these functions being _unchanged instead of _changed make reading the logic harder than it could be

Review of attachment 351029 [details] [review]:

seems fine otherwise

::: src/backends/x11/meta-monitor-manager-xrandr.c
@@ +996,2 @@
     case META_MONITOR_TRANSFORM_FLIPPED:
+      return RR_Reflect_X | XCB_RANDR_ROTATION_ROTATE_0;

missed this RR_Reflect_X

(In reply to Rui Matos from comment #25)
> Review of attachment 351031 [details] [review] [review]:
> 
> looks fine
> 
> ::: src/backends/meta-monitor.c
> @@ +26,3 @@
>  #include "backends/meta-backend-private.h"
>  #include "backends/meta-monitor-manager-private.h"
> +#include "backends/meta-settings-private.h"
> 
> ?
> 
> ::: src/backends/meta-settings.c
> @@ +256,3 @@
> +#if 0
> +void
> +meta_settings_register_proxy (MetaSettings      *settings,
> 
> do we need to keep this #if 0'd code around?

No. These "register" things was part of a preparation for attaching an XSettings "proxy" to MetaSettings, but discarded that idea for now, but obviously did a bad job at git history surgery, here and on other commits :P

(In reply to Rui Matos from comment #26)
> Review of attachment 351032 [details] [review] [review]:
> 
> ::: src/backends/meta-settings.c
> @@ +356,3 @@
> +  /* Chain up inter-dependent settings. */
> +  g_signal_connect (settings, "global-scaling-factor-changed",
> +                    G_CALLBACK (meta_settings_update_ui_scaling_factor),
> NULL);
> 
> connecting to our own signals looks strange, why not call this directly when
> needed?

Just thought it would be nice to have inter-dependencies listed like this, making the "meta_update_configuration()" just update the configuration. I can change it to add "meta_update_other_configuration()" to "meta_update_configuration()" if you prefer.

Created attachment 352476 [details] [review]
monitor-manager/xrandr: Allow configuring scales on X11 too

This commit makes it possible to configure logical monitor scale also
when running on top of an X11 server using Xrandr. An extra property
'requires-globla-scale' is added to the D-Bus API is added to instruct
a configuration application to only allow setting a global logical
monitor scale.

This is needed to let gsd-xsettings use the configured state to set a
XSettings state that respects the explicit monitor configuration.

-----------

This changes ".._unchanged()" to "..._changed()" as suggested.

Review of attachment 351032 [details] [review]:

(In reply to Jonas Ådahl from comment #33)
> Just thought it would be nice to have inter-dependencies listed like this,
> making the "meta_update_configuration()" just update the configuration.

Ok, makes sense. It caught my eye because it's not a common idiom

Review of attachment 352476 [details] [review]:

lgtm

Attachment 351028 [details] pushed as 43cdf81 - backend: Make the getter use the cached ui scale
Attachment 351029 [details] pushed as df39a7d - monitor-manager/xrandr: Use xcb API to configure CRTC
Attachment 351030 [details] pushed as be17555 - backend: Make X11 display opened function explicitly named
Attachment 351031 [details] pushed as 2718699 - backend: Move settings into a new MetaSettings object
Attachment 351032 [details] pushed as 0bc312a - monitor-manager/kms: Move global ui scaling setting to MetaSettings
Attachment 351033 [details] pushed as 3b097c7 - monitor-manager/kms: Move scale calculation to MetaMonitor
Attachment 351035 [details] pushed as 05bc2e2 - gschema: Add description for "monitor-config-manager"
Attachment 352476 [details] pushed as 1bb0e18 - monitor-manager/xrandr: Allow configuring scales on X11 too

Created attachment 357383 [details] [review]
monitor-manager/xrandr: Fix name inconsistencies

MetaOutputs should be "output" and external types should be named as
such, i.e. XRandr outputs are renamed to "xrandr_output".

Created attachment 357384 [details] [review]
Remove old monitor configuration system

Remove the old MetaMonitorConfig system and mark the new one as
non-experimental. This also removes the D-Bus property.

Created attachment 357385 [details] [review]
monitor-manager/x11: Don't complain about 'normal' transform

The 'normal' transform has the value 0, so the g_warn_if_fail()
expression failed. Correct it so that it doesn't complain when no
transform is checked.

Created attachment 357386 [details] [review]
monitor-manager: Fix output variable naming

Fix the last case of using the variable name "meta_output" for a
MetaOutput.

Created attachment 357387 [details] [review]
monitor-manager/kms: Use connector id to find old output

The zero-initialized winsys id was incorrectly used as the key to find
the old output to base active/primary state from, which would never
succeed unless the winsys id happened to be 0. Fix this by using the
winsys id that will be used, i.e. the connector id.

Created attachment 357388 [details] [review]
Migrate old monitor configuration files to new system

This commit changes the new configuration system to use monitors.xml
instead of monitors-experimental.xml. When starting up and the
monitors.xml file is loaded, if a legacy monitors.xml file is
discovered (it has the version number 1), an attempt is made to migrate
the stored configuration onto the new system.

This is done in two steps:

1) Parsing and translation of the old configuration. This works by
parsing file using the mostly the old parser, but then translating the
resulting configuration structs into the new configuration system. As
the legacy configuration system doesn't carry over some state (such as
tiling and scale used), some things are not available. For tiling, the
migration paths makes an attempt to discover tiled monitors by
comparing EDID data, and guessing what the main tile is. Determination
of the scale of a migrated configuration is postponed until the
configuration is actually applied. This works by flagging the
configuration as 'migrated'.

2) Finishing the migration when applying. When a configuration with the
'migrated' flag is retrieved from the configuration store, the final
step of the migration is taken place. This involves calculating the
preferred scale given the mode configured, while making sure this
doesn't result in any overlapping logical monitor regions etc.

Created attachment 357389 [details] [review]
tests: Add monitor config migration tests

So far some basic testing, including:

 * Test that the migrated configuration is applicable
 * Test that a monitors.xml with multiple configurations are translated
 * Test rotation
 * Test tiled monitor discovery (well, test a made up tiled monitor
   configuration since I don't have a real one)

Created attachment 357390 [details] [review]
tests: Add rotated multi head monitor config migration test

Created attachment 357391 [details] [review]
monitor-config-manager: Keep short history of configurations

In order to go back in monitor configurations, save them to a history.
The history is implemented as a max 3 element long queue, where newly
set configurations are pushed to the head, and old are popped from the
tail.

The difference between using a single previous config reference and a
queue is that we can now remember the configuration used prior to a
D-Bus triggered configuration when the user discarded the configuration.

This will later be used to restore a previous configuration when a
laptop lid is opened.

Created attachment 357392 [details] [review]
monitor-manager: Try to restore previous config before regenerating

When opening a laptop lid, one will likely want to restore the
configuration one had prior to closing it, so when ensuring monitor
configuration, first try to see if the previously set configuration is
both complete (all connected monitors are configured) and applicable
(it is a valid configuration) and only try to generate a new from
scratch if that failed.

Created attachment 357393 [details] [review]
[gnome-control-center] panels/display: Remove support for old config system

As mutter now has removed the legacy monitor config system and the
associated property on org.gnome.Mutter.DisplayConfig, also remove the
support from gnome-control-center.

Created attachment 357394 [details] [review]
[gnome-settings-daemon] xsettings: Only support the new mutter D-Bus DisplayConfig API

The legacy API is no longer supported by mutter, so lets remove it from
gnome-settings-daemon too.

Review of attachment 357383 [details] [review]:

LGTM

Review of attachment 357384 [details] [review]:

::: src/backends/meta-monitor-config-manager.c
@@ +1148,3 @@
 
+void
+meta_crtc_info_free (MetaCrtcInfo *info)

It's a bit odd to the the declarations in monitor-manager-private, and the definitions in monitor-config-manager.c (although that was already true for monitor-config.c of course). I don't see those functions used anywhere else now, so maybe make them static and remove the declarations altogether?

::: src/tests/meta-monitor-manager-test.c
@@ +118,3 @@
   config = meta_monitor_manager_ensure_configured (manager);
 
+  if (meta_is_stage_views_enabled ())

Mmmh, this is unexpected - shouldn't the conditional be removed altogether now?

Review of attachment 357385 [details] [review]:

Makes sense to me

Review of attachment 357386 [details] [review]:

Love the commit message ;-)

Review of attachment 357387 [details] [review]:

OK

Review of attachment 357384 [details] [review]:

::: data/org.gnome.mutter.gschema.xml.in
@@ -115,3 @@
         Currently possible keywords:
 
-        • “monitor-config-manager”    — use the new monitor configuration

we should remove it from the default value too

Review of attachment 357388 [details] [review]:

looks good

::: src/backends/meta-monitor-config-migration.c
@@ +1147,3 @@
+    }
+
+  configs = load_config_file (user_file, error);

is this needed?

Review of attachment 357391 [details] [review]:

sure

Review of attachment 357392 [details] [review]:

lgtm

Review of attachment 357393 [details] [review]:

looks great

Review of attachment 357388 [details] [review]:

::: src/backends/meta-monitor-config-migration.c
@@ +1147,3 @@
+    }
+
+  configs = load_config_file (user_file, error);

Hmm, looks like I changed to use the meta_migrate_old_monitors_config() (below) but didn't remove the old load thingy.

Review of attachment 357394 [details] [review]:

We can now drop all usage of libgnome-desktop here by connecting directly to the MonitorsChanged mutter DBus signal. I can do that on a follow up patch afterwards

Didn't check the tests thoroughly but all seems sane to me

(In reply to Florian Müllner from comment #51)
> Review of attachment 357384 [details] [review] [review]:
> 
> ::: src/backends/meta-monitor-config-manager.c
> @@ +1148,3 @@
>  
> +void
> +meta_crtc_info_free (MetaCrtcInfo *info)
> 
> It's a bit odd to the the declarations in monitor-manager-private, and the
> definitions in monitor-config-manager.c (although that was already true for
> monitor-config.c of course). I don't see those functions used anywhere else
> now, so maybe make them static and remove the declarations altogether?

Sounds good.

> 
> ::: src/tests/meta-monitor-manager-test.c
> @@ +118,3 @@
>    config = meta_monitor_manager_ensure_configured (manager);
>  
> +  if (meta_is_stage_views_enabled ())
> 
> Mmmh, this is unexpected - shouldn't the conditional be removed altogether
> now?

I have some ideas of being able to run the tests with stage views disabled to mimic how X11 handles things, which is why I left them as is.

Attachment 357383 [details] pushed as 9ac87b3 - monitor-manager/xrandr: Fix name inconsistencies
Attachment 357384 [details] pushed as e8a6286 - Remove old monitor configuration system
Attachment 357385 [details] pushed as ab04286 - monitor-manager/x11: Don't complain about 'normal' transform
Attachment 357386 [details] pushed as 88f2441 - monitor-manager: Fix output variable naming
Attachment 357387 [details] pushed as 27a4f9f - monitor-manager/kms: Use connector id to find old output
Attachment 357388 [details] pushed as bc31624 - Migrate old monitor configuration files to new system
Attachment 357389 [details] pushed as 6195075 - tests: Add monitor config migration tests
Attachment 357390 [details] pushed as 6c0f107 - tests: Add rotated multi head monitor config migration test
Attachment 357391 [details] pushed as b140e7f - monitor-config-manager: Keep short history of configurations
Attachment 357392 [details] pushed as 35c9280 - monitor-manager: Try to restore previous config before regenerating

The g-c-c and g-s-d was pushed too, so I guess we can close this one now.