Adding DisplayConfigurationPolicy::confirm so that the display

configuration is only written once
canonical · Oct 17, 2023 · 63a2fd7 · 63a2fd7
1 parent 7bea001
commit 63a2fd7
Show file tree

Hide file tree

Showing 12 changed files with 56 additions and 7 deletions.
diff --git a/include/platform/mir/graphics/display_configuration_policy.h b/include/platform/mir/graphics/display_configuration_policy.h
@@ -30,6 +30,7 @@ class DisplayConfigurationPolicy
     virtual ~DisplayConfigurationPolicy() = default;
 
     virtual void apply_to(DisplayConfiguration& conf) = 0;
+    virtual void confirm(DisplayConfiguration const& conf) = 0;
 
 protected:
     DisplayConfigurationPolicy() = default;

diff --git a/src/include/server/mir/graphics/default_display_configuration_policy.h b/src/include/server/mir/graphics/default_display_configuration_policy.h
@@ -29,21 +29,24 @@ namespace graphics
 class CloneDisplayConfigurationPolicy : public DisplayConfigurationPolicy
 {
 public:
-    void apply_to(DisplayConfiguration& conf);
+    void apply_to(DisplayConfiguration& conf) override;
+    void confirm(DisplayConfiguration const& conf) override;
 };
 
 /// Each screen placed to the right of the previous one
 class SideBySideDisplayConfigurationPolicy : public DisplayConfigurationPolicy
 {
 public:
-    void apply_to(graphics::DisplayConfiguration& conf);
+    void apply_to(graphics::DisplayConfiguration& conf) override;
+    void confirm(DisplayConfiguration const& conf) override;
 };
 
 /// Just use the first screen
 class SingleDisplayConfigurationPolicy : public DisplayConfigurationPolicy
 {
 public:
-    void apply_to(graphics::DisplayConfiguration& conf);
+    void apply_to(graphics::DisplayConfiguration& conf) override;
+    void confirm(DisplayConfiguration const& conf) override;
 };
 /** @} */
 }

diff --git a/src/miral/display_configuration_option.cpp b/src/miral/display_configuration_option.cpp
@@ -51,6 +51,7 @@ class PixelFormatSelector : public mg::DisplayConfigurationPolicy
 public:
     PixelFormatSelector(std::shared_ptr<mg::DisplayConfigurationPolicy> const& base_policy, bool with_alpha);
     virtual void apply_to(mg::DisplayConfiguration& conf);
+    virtual void confirm(mg::DisplayConfiguration const& conf);
 private:
     std::shared_ptr<mg::DisplayConfigurationPolicy> const base_policy;
     bool const with_alpha;
@@ -94,6 +95,11 @@ void PixelFormatSelector::apply_to(mg::DisplayConfiguration& conf)
         });
 }
 
+void PixelFormatSelector::confirm(mg::DisplayConfiguration const& conf)
+{
+    base_policy->confirm(conf);
+}
+
 class ScaleSetter : public mg::DisplayConfigurationPolicy
 {
 public:
@@ -104,6 +110,7 @@ class ScaleSetter : public mg::DisplayConfigurationPolicy
     }
 
     void apply_to(mg::DisplayConfiguration& conf) override;
+    void confirm(mg::DisplayConfiguration const& conf) override;
 private:
     std::shared_ptr<mg::DisplayConfigurationPolicy> const base_policy;
     float const with_scale;
@@ -119,6 +126,11 @@ void ScaleSetter::apply_to(mg::DisplayConfiguration& conf)
         });
 }
 
+void ScaleSetter::confirm(mg::DisplayConfiguration const& conf)
+{
+    base_policy->confirm(conf);
+}
+
 void miral::display_configuration_options(mir::Server& server)
 {
     // Add choice of monitor configuration

diff --git a/src/miral/static_display_config.cpp b/src/miral/static_display_config.cpp
@@ -328,6 +328,10 @@ void miral::YamlFileDisplayConfig::apply_to(mg::DisplayConfiguration& conf)
     mir::log_info(out.str());
 }
 
+void miral::YamlFileDisplayConfig::confirm(mir::graphics::DisplayConfiguration const&)
+{
+}
+
 void miral::YamlFileDisplayConfig::apply_default_configuration(mg::DisplayConfiguration& conf)
 {
     conf.for_each_output([config=Config{}](mg::UserDisplayConfigurationOutput& conf_output)
@@ -549,7 +553,7 @@ void miral::ReloadingYamlFileDisplayConfig::config_path(std::string newpath)
     check_for_layout_override();
 }
 
-void miral::ReloadingYamlFileDisplayConfig::apply_to(mir::graphics::DisplayConfiguration& conf)
+void miral::ReloadingYamlFileDisplayConfig::confirm(mir::graphics::DisplayConfiguration const& conf)
 {
     std::lock_guard lock{mutex};
     if (!config_path_)
@@ -584,8 +588,6 @@ void miral::ReloadingYamlFileDisplayConfig::apply_to(mir::graphics::DisplayConfi
                 filename.c_str());
         }
     }
-
-    YamlFileDisplayConfig::apply_to(conf);
 }
 
 auto miral::ReloadingYamlFileDisplayConfig::the_main_loop() const -> std::shared_ptr<mir::MainLoop>

diff --git a/src/miral/static_display_config.h b/src/miral/static_display_config.h
@@ -45,6 +45,7 @@ class YamlFileDisplayConfig : public mir::graphics::DisplayConfigurationPolicy
     void load_config(std::istream& config_file, std::string const& filename);
 
     void apply_to(mir::graphics::DisplayConfiguration& conf) override;
+    virtual void confirm(mir::graphics::DisplayConfiguration const& conf) override;
 
     void select_layout(std::string const& layout);
 
@@ -94,7 +95,7 @@ class ReloadingYamlFileDisplayConfig : public YamlFileDisplayConfig
 
     void config_path(std::string newpath);
 
-    void apply_to(mir::graphics::DisplayConfiguration& conf) override;
+    void confirm(mir::graphics::DisplayConfiguration const& conf) override;
 
     void check_for_layout_override();
 

diff --git a/src/miroil/persist_display_config.cpp b/src/miroil/persist_display_config.cpp
@@ -77,6 +77,10 @@ struct DisplayConfigurationPolicyAdapter : mg::DisplayConfigurationPolicy
         self->apply_to(conf, *wrapped_policy, *custom_policy);
     }
 
+    void confirm(mg::DisplayConfiguration const&) override
+    {
+    }
+
     std::shared_ptr<PersistDisplayConfigPolicy> const self;
     std::shared_ptr<miroil::DisplayConfigurationPolicy> const wrapped_policy;
     std::shared_ptr<miroil::DisplayConfigurationPolicy> const custom_policy;

diff --git a/src/server/graphics/default_display_configuration_policy.cpp b/src/server/graphics/default_display_configuration_policy.cpp
@@ -88,6 +88,10 @@ void mg::CloneDisplayConfigurationPolicy::apply_to(DisplayConfiguration& conf)
         });
 }
 
+void mg::CloneDisplayConfigurationPolicy::confirm(mir::graphics::DisplayConfiguration const&)
+{
+}
+
 void mg::SideBySideDisplayConfigurationPolicy::apply_to(graphics::DisplayConfiguration& conf)
 {
     int max_x = 0;
@@ -113,6 +117,9 @@ void mg::SideBySideDisplayConfigurationPolicy::apply_to(graphics::DisplayConfigu
             });
 }
 
+void mg::SideBySideDisplayConfigurationPolicy::confirm(mir::graphics::DisplayConfiguration const&)
+{
+}
 
 void mg::SingleDisplayConfigurationPolicy::apply_to(graphics::DisplayConfiguration& conf)
 {
@@ -137,3 +144,7 @@ void mg::SingleDisplayConfigurationPolicy::apply_to(graphics::DisplayConfigurati
             }
             });
 }
+
+void mg::SingleDisplayConfigurationPolicy::confirm(mir::graphics::DisplayConfiguration const&)
+{
+}
diff --git a/src/server/graphics/multiplexing_display.cpp b/src/server/graphics/multiplexing_display.cpp
@@ -33,6 +33,7 @@ mg::MultiplexingDisplay::MultiplexingDisplay(
 {
     auto conf = configuration();
     initial_configuration_policy.apply_to(*conf);
+    initial_configuration_policy.confirm(*conf);
     configure(*conf);
 }
 

diff --git a/tests/include/mir/test/doubles/null_display_configuration_policy.h b/tests/include/mir/test/doubles/null_display_configuration_policy.h
@@ -29,6 +29,7 @@ class NullDisplayConfigurationPolicy : public graphics::DisplayConfigurationPoli
 {
 public:
     void apply_to(graphics::DisplayConfiguration&) override {}
+    void confirm(graphics::DisplayConfiguration const&) override {}
 };
 }
 }

diff --git a/tests/unit-tests/graphics/test_multiplexing_display.cpp b/tests/unit-tests/graphics/test_multiplexing_display.cpp
@@ -679,6 +679,10 @@ TEST(MultiplexingDisplay, applies_initial_display_configuration)
                 });
         }
 
+        void confirm(mg::DisplayConfiguration const&) override
+        {
+        }
+
         auto expected_location_for_display(mg::DisplayConfigurationOutput const& display) -> geom::Point
         {
             auto it = std::find_if(

diff --git a/tests/unit-tests/platforms/gbm-kms/kms/test_display_multi_monitor.cpp b/tests/unit-tests/platforms/gbm-kms/kms/test_display_multi_monitor.cpp
@@ -76,6 +76,10 @@ class ClonedDisplayConfigurationPolicy : public mg::DisplayConfigurationPolicy
                 }
             });
     }
+
+    void confirm(mg::DisplayConfiguration const&)
+    {
+    }
 };
 
 class SideBySideDisplayConfigurationPolicy : public mg::DisplayConfigurationPolicy
@@ -105,6 +109,10 @@ class SideBySideDisplayConfigurationPolicy : public mg::DisplayConfigurationPoli
                 }
             });
     }
+
+    void confirm(mg::DisplayConfiguration const&)
+    {
+    }
 };
 
 class MesaDisplayMultiMonitorTest : public ::testing::Test

diff --git a/tests/unit-tests/scene/test_mediating_display_changer.cpp b/tests/unit-tests/scene/test_mediating_display_changer.cpp
@@ -55,6 +55,7 @@ class MockDisplayConfigurationPolicy : public mg::DisplayConfigurationPolicy
 public:
     ~MockDisplayConfigurationPolicy() noexcept {}
     MOCK_METHOD(void, apply_to, (mg::DisplayConfiguration&));
+    MOCK_METHOD(void, confirm, (mg::DisplayConfiguration const&));
 };
-Original file line number
+Diff line change
@@ Expand Up @@
     {
     public:
         void apply_to(graphics::DisplayConfiguration&) override {}
+        void confirm(graphics::DisplayConfiguration const&) override {}
     };
     }
     }
@@ Expand Down @@