From 63a2fd70b77a5f41b1d1a8bd3174024c26b81054 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Tue, 17 Oct 2023 13:31:35 -0400 Subject: [PATCH] Adding DisplayConfigurationPolicy::confirm so that the display configuration is only written once --- .../mir/graphics/display_configuration_policy.h | 1 + .../graphics/default_display_configuration_policy.h | 9 ++++++--- src/miral/display_configuration_option.cpp | 12 ++++++++++++ src/miral/static_display_config.cpp | 8 +++++--- src/miral/static_display_config.h | 3 ++- src/miroil/persist_display_config.cpp | 4 ++++ .../default_display_configuration_policy.cpp | 11 +++++++++++ src/server/graphics/multiplexing_display.cpp | 1 + .../test/doubles/null_display_configuration_policy.h | 1 + .../graphics/test_multiplexing_display.cpp | 4 ++++ .../gbm-kms/kms/test_display_multi_monitor.cpp | 8 ++++++++ .../scene/test_mediating_display_changer.cpp | 1 + 12 files changed, 56 insertions(+), 7 deletions(-) diff --git a/include/platform/mir/graphics/display_configuration_policy.h b/include/platform/mir/graphics/display_configuration_policy.h index 3caa300e5d7..261894f6f64 100644 --- 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 index fca7812ff09..c81ed612523 100644 --- 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 index 69d390cbc6e..e89c054ed9c 100644 --- 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 const& base_policy, bool with_alpha); virtual void apply_to(mg::DisplayConfiguration& conf); + virtual void confirm(mg::DisplayConfiguration const& conf); private: std::shared_ptr 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 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 index 73be9049f50..0e2ebdc2f45 100644 --- 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 diff --git a/src/miral/static_display_config.h b/src/miral/static_display_config.h index b01527b7f4a..1df3a1cd984 100644 --- 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 index 4426163f4f1..404e008d976 100644 --- 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 const self; std::shared_ptr const wrapped_policy; std::shared_ptr const custom_policy; diff --git a/src/server/graphics/default_display_configuration_policy.cpp b/src/server/graphics/default_display_configuration_policy.cpp index 19ca41c12a4..7f333a1f9db 100644 --- 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&) +{ +} \ No newline at end of file diff --git a/src/server/graphics/multiplexing_display.cpp b/src/server/graphics/multiplexing_display.cpp index 7966392edba..0981f5ccf83 100644 --- 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 index 54907ffd1b8..d62f905791d 100644 --- 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 index dfa83582d96..34321b7b32c 100644 --- 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 index 8f34ccd195e..111a1dcc7fa 100644 --- 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 index e726846f030..671c45a5441 100644 --- 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&)); };