Merge lp:~nick-dedekind/qtmir/miral-DisplayConfigurationStorage into lp:~unity-team/qtmir/miral-qt-integration

[Nick Dedekind]

Added interface for saving/loading display configuration output options.

Pass responsibility for display configuration storage to an overridable interface.

[lp:~nick-dedekind/qtmir/miral-DisplayConfigurationStorage]

595. By Nick Dedekind on 2016-12-08

better storage options

596. By Nick Dedekind on 2016-12-08

cleanup

597. By Nick Dedekind on 2016-12-08

clone_of->clone_of_output_index

598. By Nick Dedekind on 2016-12-08

clone_of_output_index->clone_output_index

599. By Nick Dedekind on 2016-12-09

merged edid

[Alan Griffiths (alan-griffiths)]

+ conf.for_each_output([this, &conf](mg::UserDisplayConfigurationOutput& output) {
...
default_policy.apply_to(conf);

First apply any saved configuration and then overwrite that with the default policy. Is this the right behaviour?

review: Needs Information

[Alan Griffiths (alan-griffiths)]

- miral::Application applicationFor(qtmir::PromptSession const &promptSession) const;
+ ::miral::Application applicationFor(qtmir::PromptSession const &promptSession) const;

This (and similar changes elsewhere) shouldn't be needed now.

[lp:~nick-dedekind/qtmir/miral-DisplayConfigurationStorage]

600. By Nick Dedekind on 2016-12-14

merged parent

601. By Nick Dedekind on 2016-12-14

moved default apply before load

602. By Nick Dedekind on 2016-12-14

removed ::miral

[Alan Griffiths (alan-griffiths)]

The target branch has landed, the pre-requisite has been rebased.

review: Needs Resubmitting

[lp:~nick-dedekind/qtmir/miral-DisplayConfigurationStorage]

603. By Nick Dedekind on 2017-01-06

merged pre-req

604. By Nick Dedekind on 2017-01-06

merged with pre-req

605. By Nick Dedekind on 2017-01-27

merged with prereq

606. By Nick Dedekind on 2017-02-07

merged with prereq

607. By Nick Dedekind on 2017-02-07

review comments

608. By Nick Dedekind on 2017-02-08

removed mir 0.26 version check

609. By Nick Dedekind on 2017-02-08

merged parent

610. By Nick Dedekind on 2017-02-09

added check for ouput.connected to load/save config

611. By Nick Dedekind on 2017-02-09

print error on failed EDID parse

612. By Nick Dedekind on 2017-02-14

more info for identifying a display

613. By Nick Dedekind on 2017-02-22

merged with trunk

614. By Nick Dedekind on 2017-03-14

merged trunk

615. By Nick Dedekind on 2017-03-20

miral 1.3.1 compat

616. By Nick Dedekind on 2017-03-24

merged trunk

617. By Nick Dedekind on 2017-03-31

merged trunk

618. By Nick Dedekind on 2017-03-31

merged with trunk

Unmerged revisions

618. By Nick Dedekind on 2017-03-31

merged with trunk

617. By Nick Dedekind on 2017-03-31

merged trunk

616. By Nick Dedekind on 2017-03-24

merged trunk

615. By Nick Dedekind on 2017-03-20

miral 1.3.1 compat

614. By Nick Dedekind on 2017-03-14

merged trunk

613. By Nick Dedekind on 2017-02-22

merged with trunk

612. By Nick Dedekind on 2017-02-14

more info for identifying a display

611. By Nick Dedekind on 2017-02-09

print error on failed EDID parse

610. By Nick Dedekind on 2017-02-09

added check for ouput.connected to load/save config

609. By Nick Dedekind on 2017-02-08

merged parent

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2017-01-06 12:00:57 +0000
+++ CMakeLists.txt	2017-01-06 12:00:58 +0000
@@ -71,8 +71,8 @@
 
 find_package(Threads REQUIRED)
 
-pkg_check_modules(MIRSERVER mirserver>=0.25 REQUIRED)
-pkg_check_modules(MIRCLIENT mirclient>=0.25 REQUIRED)
+pkg_check_modules(MIRSERVER mirserver>=0.26 REQUIRED)
+pkg_check_modules(MIRCLIENT mirclient>=0.26 REQUIRED)
 pkg_check_modules(MIRRENDERERGLDEV mir-renderer-gl-dev>=0.24 REQUIRED)
 pkg_check_modules(MIRAL miral>=0.4 REQUIRED)
 

=== modified file 'debian/control'
--- debian/control	2016-12-13 15:51:14 +0000
+++ debian/control	2017-01-06 12:00:58 +0000
@@ -14,9 +14,9 @@
                libgsettings-qt-dev,
                liblttng-ust-dev,
                libmiral-dev (>= 0.4),
-               libmirclient-dev (>= 0.25.0),
-               libmircommon-dev (>= 0.25.0),
-               libmirserver-dev (>= 0.25.0),
+               libmirclient-dev (>= 0.26.0),
+               libmircommon-dev (>= 0.26.0),
+               libmirserver-dev (>= 0.26.0),
                libmtdev-dev,
                libprocess-cpp-dev,
                libqt5sensors5-dev,

=== modified file 'src/platforms/mirserver/miral/CMakeLists.txt'
--- src/platforms/mirserver/miral/CMakeLists.txt	2017-01-06 12:00:57 +0000
+++ src/platforms/mirserver/miral/CMakeLists.txt	2017-01-06 12:00:58 +0000
@@ -2,5 +2,6 @@
 
 add_library(miral-prototypes OBJECT
     edid.cpp edid.h
+    display_configuration_storage.h
     persist_display_config.cpp persist_display_config.h
 )

=== added file 'src/platforms/mirserver/miral/display_configuration_storage.h'
--- src/platforms/mirserver/miral/display_configuration_storage.h	1970-01-01 00:00:00 +0000
+++ src/platforms/mirserver/miral/display_configuration_storage.h	2017-01-06 12:00:58 +0000
@@ -0,0 +1,52 @@
+/*
+ * Copyright © 2016 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Nick Dedekind <[e-mail]>
+ */
+
+#ifndef MIRAL_DISPLAY_CONFIGURATION_STORAGE_H
+#define MIRAL_DISPLAY_CONFIGURATION_STORAGE_H
+
+#include "mir/geometry/rectangle.h"
+#include "mir/optional_value.h"
+#include "mir_toolkit/common.h"
+
+// Prototyping namespace for later incorporation in MirAL
+namespace miral
+{
+class Edid;
+
+struct DisplayOutputOptions
+{
+    mir::optional_value<bool> used;
+    mir::optional_value<uint> clone_output_index;
+    mir::optional_value<mir::geometry::Size> size;
+    mir::optional_value<MirOrientation> orientation;
+    mir::optional_value<MirFormFactor> form_factor;
+    mir::optional_value<float> scale;
+};
+
+class DisplayConfigurationStorage
+{
+public:
+    virtual ~DisplayConfigurationStorage() = default;
+
+    virtual void save(const Edid&, const DisplayOutputOptions&) = 0;
+    virtual bool load(const Edid&, DisplayOutputOptions&) const = 0;
+};
+
+}
+
+#endif // MIRAL_DISPLAY_CONFIGURATION_STORAGE_H

=== modified file 'src/platforms/mirserver/miral/persist_display_config.cpp'
--- src/platforms/mirserver/miral/persist_display_config.cpp	2017-01-06 12:00:57 +0000
+++ src/platforms/mirserver/miral/persist_display_config.cpp	2017-01-06 12:00:58 +0000
@@ -17,8 +17,11 @@
  */
 
 #include "persist_display_config.h"
+#include "display_configuration_storage.h"
+#include "edid.h"
 
 #include <mir/graphics/display_configuration_policy.h>
+#include <mir/graphics/display_configuration.h>
 #include <mir/server.h>
 #include <mir/version.h>
 
@@ -33,13 +36,16 @@
 {
 struct PersistDisplayConfigPolicy
 {
-    PersistDisplayConfigPolicy() = default;
+    PersistDisplayConfigPolicy(std::shared_ptr<miral::DisplayConfigurationStorage> const& storage) :
+        storage(storage) {}
     virtual ~PersistDisplayConfigPolicy() = default;
     PersistDisplayConfigPolicy(PersistDisplayConfigPolicy const&) = delete;
     auto operator=(PersistDisplayConfigPolicy const&) -> PersistDisplayConfigPolicy& = delete;
 
     void apply_to(mg::DisplayConfiguration& conf, mg::DisplayConfigurationPolicy& default_policy);
     void save_config(mg::DisplayConfiguration const& base_conf);
+
+    std::shared_ptr<miral::DisplayConfigurationStorage> storage;
 };
 
 struct DisplayConfigurationPolicyAdapter : mg::DisplayConfigurationPolicy
@@ -81,8 +87,11 @@
 
 struct miral::PersistDisplayConfig::Self : PersistDisplayConfigPolicy, DisplayConfigurationObserver
 {
-    Self() = default;
-    Self(DisplayConfigurationPolicyWrapper const& custom_wrapper) :
+    Self(std::shared_ptr<DisplayConfigurationStorage> const& storage) :
+        PersistDisplayConfigPolicy(storage) {}
+    Self(std::shared_ptr<DisplayConfigurationStorage> const& storage,
+         DisplayConfigurationPolicyWrapper const& custom_wrapper) :
+        PersistDisplayConfigPolicy(storage),
         custom_wrapper{custom_wrapper} {}
 
     DisplayConfigurationPolicyWrapper const custom_wrapper =
@@ -96,13 +105,14 @@
 #endif
 };
 
-miral::PersistDisplayConfig::PersistDisplayConfig() :
-    self{std::make_shared<Self>()}
+miral::PersistDisplayConfig::PersistDisplayConfig(std::shared_ptr<DisplayConfigurationStorage> const& storage) :
+    self{std::make_shared<Self>(storage)}
 {
 }
 
-miral::PersistDisplayConfig::PersistDisplayConfig(DisplayConfigurationPolicyWrapper const& custom_wrapper) :
-    self{std::make_shared<Self>(custom_wrapper)}
+miral::PersistDisplayConfig::PersistDisplayConfig(std::shared_ptr<DisplayConfigurationStorage> const& storage,
+                                                  DisplayConfigurationPolicyWrapper const& custom_wrapper) :
+    self{std::make_shared<Self>(storage, custom_wrapper)}
 {
 }
 
@@ -136,12 +146,82 @@
     mg::DisplayConfiguration& conf,
     mg::DisplayConfigurationPolicy& default_policy)
 {
-    // TODO if the h/w profile (by some definition) has changed, then apply corresponding saved config (if any).
-    // TODO Otherwise...
     default_policy.apply_to(conf);
+
+    if (!storage) return;
+
+    conf.for_each_output([this, &conf](mg::UserDisplayConfigurationOutput& output) {
+
+        try {
+            miral::Edid edid;
+            edid.parse_data(reinterpret_cast<std::vector<uint8_t> const&>(output.edid));
+
+            // TODO if the h/w profile (by some definition) has changed, then apply corresponding saved config (if any).
+            // TODO Otherwise...
+
+            miral::DisplayOutputOptions config;
+            if (storage->load(edid, config)) {
+
+                if (config.size.is_set()) {
+                    int mode_index = output.current_mode_index;
+                    int i = 0;
+                    // Find the mode index which supports the saved size.
+                    for(auto iter = output.modes.cbegin(); iter != output.modes.cend(); ++iter, i++) {
+                        if ((*iter).size == config.size.value()) {
+                            mode_index = i;
+                        }
+                    }
+                    output.current_mode_index = mode_index;
+                }
+
+                uint output_index = 0;
+                conf.for_each_output([this, &output, config, &output_index](mg::DisplayConfigurationOutput const& find_output) {
+                    if (output_index == config.clone_output_index.value()) {
+                        output.top_left = find_output.top_left;
+                    }
+                    output_index++;
+                });
+
+                if (config.orientation.is_set()) {output.orientation = config.orientation.value(); }
+                if (config.used.is_set()) {output.used = config.used.value(); }
+                if (config.form_factor.is_set()) {output.form_factor = config.form_factor.value(); }
+                if (config.scale.is_set()) {output.scale = config.scale.value(); }
+            }
+        } catch (std::runtime_error const&) {
+        }
+    });
 }
 
-void PersistDisplayConfigPolicy::save_config(mg::DisplayConfiguration const& /*base_conf*/)
+void PersistDisplayConfigPolicy::save_config(mg::DisplayConfiguration const& conf)
 {
-    // TODO save display config options against the h/w profile
+    if (!storage) return;
+
+    conf.for_each_output([this, &conf](mg::DisplayConfigurationOutput const& output) {
+
+        try {
+            miral::Edid edid;
+            // FIXME - output.edid should be std::vector<uint8_t>, not std::vector<uint8_t const>
+            edid.parse_data(reinterpret_cast<std::vector<uint8_t> const&>(output.edid));
+
+            miral::DisplayOutputOptions config;
+
+            uint output_index = 0;
+            conf.for_each_output([this, output, &config, &output_index](mg::DisplayConfigurationOutput const& find_output) {
+                if (!config.clone_output_index.is_set() && output.top_left == find_output.top_left) {
+                    config.clone_output_index = output_index;
+                }
+                output_index++;
+            });
+
+            config.size = output.extents().size;
+            config.form_factor = output.form_factor;
+            config.orientation = output.orientation;
+            config.scale = output.scale;
+            config.used = output.used;
+
+            storage->save(edid, config);
+        } catch (std::runtime_error const&) {
+
+        }
+    });
 }

=== modified file 'src/platforms/mirserver/miral/persist_display_config.h'
--- src/platforms/mirserver/miral/persist_display_config.h	2017-01-06 12:00:57 +0000
+++ src/platforms/mirserver/miral/persist_display_config.h	2017-01-06 12:00:58 +0000
@@ -27,11 +27,13 @@
 // Prototyping namespace for later incorporation in MirAL
 namespace miral
 {
+class DisplayConfigurationStorage;
+
 /// Restores the saved display configuration and saves changes to the base configuration
 class PersistDisplayConfig
 {
 public:
-    PersistDisplayConfig();
+    PersistDisplayConfig(std::shared_ptr<DisplayConfigurationStorage> const& storage);
     ~PersistDisplayConfig();
     PersistDisplayConfig(PersistDisplayConfig const&);
     auto operator=(PersistDisplayConfig const&) -> PersistDisplayConfig&;
@@ -39,7 +41,9 @@
     // TODO factor this out better
     using DisplayConfigurationPolicyWrapper =
         std::function<std::shared_ptr<mir::graphics::DisplayConfigurationPolicy>(const std::shared_ptr<mir::graphics::DisplayConfigurationPolicy> &wrapped)>;
-    PersistDisplayConfig(DisplayConfigurationPolicyWrapper const& custom_wrapper);
+
+    PersistDisplayConfig(std::shared_ptr<DisplayConfigurationStorage> const& storage,
+                         DisplayConfigurationPolicyWrapper const& custom_wrapper);
 
     void operator()(mir::Server& server);
 
@@ -47,6 +51,7 @@
     struct Self;
     std::shared_ptr<Self> self;
 };
+
 }
 
 #endif //MIRAL_PERSIST_DISPLAY_CONFIG_H

=== modified file 'src/platforms/mirserver/qmirserver_p.cpp'
--- src/platforms/mirserver/qmirserver_p.cpp	2017-01-06 12:00:57 +0000
+++ src/platforms/mirserver/qmirserver_p.cpp	2017-01-06 12:00:58 +0000
@@ -15,6 +15,7 @@
  */
 
 #include "qmirserver_p.h"
+#include "miral/display_configuration_storage.h"
 
 // local
 #include "logging.h"
@@ -37,6 +38,16 @@
 #include <QCoreApplication>
 #include <QOpenGLContext>
 
+namespace
+{
+struct DefaultDisplayConfigurationStorage : miral::DisplayConfigurationStorage
+{
+    void save(const miral::Edid&, const miral::DisplayOutputOptions&) override {}
+
+    bool load(const miral::Edid&, miral::DisplayOutputOptions&) const override { return false; }
+};
+}
+
 void MirServerThread::run()
 {
     auto start_callback = [this]
@@ -141,7 +152,8 @@
             addInitCallback,
             qtmir::SetQtCompositor{screensModel},
             setTerminator,
-            miral::PersistDisplayConfig{&qtmir::wrapDisplayConfigurationPolicy}
+            miral::PersistDisplayConfig{std::make_shared<DefaultDisplayConfigurationStorage>(),
+                                        &qtmir::wrapDisplayConfigurationPolicy}
         });
 }