From d30ac6493178d7b9eb7f948d9acbee734a50008d Mon Sep 17 00:00:00 2001
From: Harlen Batagelo <hbatagelo@gmail.com>
Date: Mon, 27 Nov 2023 14:01:35 -0300
Subject: [PATCH 1/3] Make server-side decorations scale aware

---
 .../shell/decoration/basic_decoration.cpp     |  5 +-
 .../shell/decoration/basic_decoration.h       |  3 +-
 src/server/shell/decoration/renderer.cpp      | 76 +++++++++++--------
 src/server/shell/decoration/renderer.h        |  6 +-
 src/server/shell/default_configuration.cpp    | 17 ++++-
 .../test_decoration_basic_decoration.cpp      |  3 +-
 6 files changed, 72 insertions(+), 38 deletions(-)

diff --git a/src/server/shell/decoration/basic_decoration.cpp b/src/server/shell/decoration/basic_decoration.cpp
index 93e2d24ae18..846484e7c1a 100644
--- a/src/server/shell/decoration/basic_decoration.cpp
+++ b/src/server/shell/decoration/basic_decoration.cpp
@@ -158,7 +158,8 @@ msd::BasicDecoration::BasicDecoration(
     std::shared_ptr<mg::GraphicBufferAllocator> const& buffer_allocator,
     std::shared_ptr<Executor> const& executor,
     std::shared_ptr<input::CursorImages> const& cursor_images,
-    std::shared_ptr<ms::Surface> const& window_surface)
+    std::shared_ptr<ms::Surface> const& window_surface,
+    float scale)
     : threadsafe_self{std::make_shared<ThreadsafeAccess<BasicDecoration>>(executor)},
       static_geometry{std::make_shared<StaticGeometry>(default_geometry)},
       shell{shell},
@@ -166,7 +167,7 @@ msd::BasicDecoration::BasicDecoration(
       cursor_images{cursor_images},
       session{window_surface->session().lock()},
       buffer_streams{std::make_unique<BufferStreams>(session)},
-      renderer{std::make_unique<Renderer>(buffer_allocator, static_geometry)},
+      renderer{std::make_unique<Renderer>(buffer_allocator, static_geometry, scale)},
       window_surface{window_surface},
       decoration_surface{create_surface()},
       window_state{new_window_state()},
diff --git a/src/server/shell/decoration/basic_decoration.h b/src/server/shell/decoration/basic_decoration.h
index ba7e4bd8587..d6dd1aaf3fd 100644
--- a/src/server/shell/decoration/basic_decoration.h
+++ b/src/server/shell/decoration/basic_decoration.h
@@ -72,7 +72,8 @@ class BasicDecoration
         std::shared_ptr<graphics::GraphicBufferAllocator> const& buffer_allocator,
         std::shared_ptr<Executor> const& executor,
         std::shared_ptr<input::CursorImages> const& cursor_images,
-        std::shared_ptr<scene::Surface> const& window_surface);
+        std::shared_ptr<scene::Surface> const& window_surface,
+        float scale);
     ~BasicDecoration();
 
     void window_state_updated();
diff --git a/src/server/shell/decoration/renderer.cpp b/src/server/shell/decoration/renderer.cpp
index de614b674f0..4b04ca339c8 100644
--- a/src/server/shell/decoration/renderer.cpp
+++ b/src/server/shell/decoration/renderer.cpp
@@ -446,7 +446,8 @@ auto msd::Renderer::Text::Impl::utf8_to_utf32(std::string const& text) -> std::u
 
 msd::Renderer::Renderer(
     std::shared_ptr<graphics::GraphicBufferAllocator> const& buffer_allocator,
-    std::shared_ptr<StaticGeometry const> const& static_geometry)
+    std::shared_ptr<StaticGeometry const> const& static_geometry,
+    float scale)
     : buffer_allocator{buffer_allocator},
       focused_theme{
           default_focused_background,
@@ -473,7 +474,8 @@ msd::Renderer::Renderer(
               render_minimize_icon}},
       },
       static_geometry{static_geometry},
-      text{Text::instance()}
+      text{Text::instance()},
+      scale{scale}
 {
 }
 
@@ -491,6 +493,8 @@ void msd::Renderer::update_state(WindowState const& window_state, InputState con
     if (length != solid_color_pixels_length)
     {
         solid_color_pixels_length = length;
+        auto const squared_scale{scale * scale};
+        scaled_solid_color_pixels_length = length * squared_scale;
         solid_color_pixels.reset(); // force a reallocation next time it's needed
     }
 
@@ -540,31 +544,35 @@ void msd::Renderer::update_state(WindowState const& window_state, InputState con
 
 auto msd::Renderer::render_titlebar() -> std::optional<std::shared_ptr<mg::Buffer>>
 {
-    if (!area(titlebar_size))
+    auto const scaled_titlebar_size{titlebar_size * scale};
+
+    if (!area(scaled_titlebar_size))
         return std::nullopt;
 
     if (!titlebar_pixels)
     {
-        titlebar_pixels = alloc_pixels(titlebar_size);
+        titlebar_pixels = alloc_pixels(scaled_titlebar_size);
         needs_titlebar_redraw = true;
     }
 
     if (needs_titlebar_redraw)
     {
-        for (geom::Y y{0}; y < as_y(titlebar_size.height); y += geom::DeltaY{1})
+        for (geom::Y y{0}; y < as_y(scaled_titlebar_size.height); y += geom::DeltaY{1})
         {
             render_row(
-                titlebar_pixels.get(), titlebar_size,
-                {0, y}, titlebar_size.width,
+                titlebar_pixels.get(), scaled_titlebar_size,
+                {0, y}, scaled_titlebar_size.width,
                 current_theme->background_color);
         }
 
         text->render(
             titlebar_pixels.get(),
-            titlebar_size,
+            scaled_titlebar_size,
             name,
-            static_geometry->title_font_top_left,
-            static_geometry->title_font_height,
+            geom::Point{
+                static_geometry->title_font_top_left.x.as_value() * scale,
+                static_geometry->title_font_top_left.y.as_value() * scale},
+            static_geometry->title_font_height * scale,
             current_theme->text_color);
     }
 
@@ -572,30 +580,35 @@ auto msd::Renderer::render_titlebar() -> std::optional<std::shared_ptr<mg::Buffe
     {
         for (auto const& button : buttons)
         {
+            geom::Rectangle scaled_button_rect{
+                geom::Point{
+                    button.rect.left().as_value() * scale,
+                    button.rect.top().as_value() * scale},
+                button.rect.size * scale};
             auto const icon = button_icons.find(button.function);
             if (icon != button_icons.end())
             {
                 Pixel button_color = icon->second.normal_color;
                 if (button.state == ButtonState::Hovered)
                     button_color = icon->second.active_color;
-                for (geom::Y y{button.rect.top()}; y < button.rect.bottom(); y += geom::DeltaY{1})
+                for (geom::Y y{scaled_button_rect.top()}; y < scaled_button_rect.bottom(); y += geom::DeltaY{1})
                 {
                     render_row(
                         titlebar_pixels.get(),
-                        titlebar_size,
-                        {button.rect.left(), y},
-                        button.rect.size.width,
+                        scaled_titlebar_size,
+                        {scaled_button_rect.left(), y},
+                        scaled_button_rect.size.width,
                         button_color);
                 }
                 geom::Rectangle const icon_rect = {
-                button.rect.top_left + static_geometry->icon_padding, {
-                    button.rect.size.width - static_geometry->icon_padding.dx * 2,
-                    button.rect.size.height - static_geometry->icon_padding.dy * 2}};
+                scaled_button_rect.top_left + static_geometry->icon_padding * scale, {
+                    scaled_button_rect.size.width - static_geometry->icon_padding.dx * scale * 2,
+                    scaled_button_rect.size.height - static_geometry->icon_padding.dy * scale * 2}};
                 icon->second.render_icon(
                     titlebar_pixels.get(),
-                    titlebar_size,
+                    scaled_titlebar_size,
                     icon_rect,
-                    static_geometry->icon_line_width,
+                    static_geometry->icon_line_width * scale,
                     icon->second.icon_color);
             }
             else
@@ -608,46 +621,49 @@ auto msd::Renderer::render_titlebar() -> std::optional<std::shared_ptr<mg::Buffe
     needs_titlebar_redraw = false;
     needs_titlebar_buttons_redraw = false;
 
-    return make_buffer(titlebar_pixels.get(), titlebar_size);
+    return make_buffer(titlebar_pixels.get(), scaled_titlebar_size);
 }
 
 auto msd::Renderer::render_left_border() -> std::optional<std::shared_ptr<mg::Buffer>>
 {
-    if (!area(left_border_size))
+    auto const scaled_left_border_size{left_border_size * scale};
+    if (!area(scaled_left_border_size))
         return std::nullopt;
     update_solid_color_pixels();
-    return make_buffer(solid_color_pixels.get(), left_border_size);
+    return make_buffer(solid_color_pixels.get(), scaled_left_border_size);
 }
 
 auto msd::Renderer::render_right_border() -> std::optional<std::shared_ptr<mg::Buffer>>
 {
-    if (!area(right_border_size))
+    auto const scaled_right_border_size{right_border_size * scale};
+    if (!area(scaled_right_border_size))
         return std::nullopt;
     update_solid_color_pixels();
-    return make_buffer(solid_color_pixels.get(), right_border_size);
+    return make_buffer(solid_color_pixels.get(), scaled_right_border_size);
 }
 
 auto msd::Renderer::render_bottom_border() -> std::optional<std::shared_ptr<mg::Buffer>>
 {
-    if (!area(bottom_border_size))
+    auto const scaled_bottom_border_size{bottom_border_size * scale};
+    if (!area(scaled_bottom_border_size))
         return std::nullopt;
     update_solid_color_pixels();
-    return make_buffer(solid_color_pixels.get(), bottom_border_size);
+    return make_buffer(solid_color_pixels.get(), scaled_bottom_border_size);
 }
 
 void msd::Renderer::update_solid_color_pixels()
 {
     if (!solid_color_pixels)
     {
-        solid_color_pixels = alloc_pixels(geom::Size{solid_color_pixels_length, 1});
+        solid_color_pixels = alloc_pixels(geom::Size{scaled_solid_color_pixels_length, 1});
         needs_solid_color_redraw = true;
     }
 
     if (needs_solid_color_redraw)
     {
         render_row(
-            solid_color_pixels.get(), geom::Size{solid_color_pixels_length, 1},
-            geom::Point{}, geom::Width{solid_color_pixels_length},
+            solid_color_pixels.get(), geom::Size{scaled_solid_color_pixels_length, 1},
+            geom::Point{}, geom::Width{scaled_solid_color_pixels_length},
             current_theme->background_color);
     }
 
@@ -687,4 +703,4 @@ auto msd::Renderer::alloc_pixels(geometry::Size size) -> std::unique_ptr<uint32_
         return std::unique_ptr<uint32_t[]>{new uint32_t[buf_size]};
     else
         return nullptr;
-}
+}
\ No newline at end of file
diff --git a/src/server/shell/decoration/renderer.h b/src/server/shell/decoration/renderer.h
index 7329dac103f..0ed4d062276 100644
--- a/src/server/shell/decoration/renderer.h
+++ b/src/server/shell/decoration/renderer.h
@@ -47,7 +47,8 @@ class Renderer
 public:
     Renderer(
         std::shared_ptr<graphics::GraphicBufferAllocator> const& buffer_allocator,
-        std::shared_ptr<StaticGeometry const> const& static_geometry);
+        std::shared_ptr<StaticGeometry const> const& static_geometry,
+        float scale);
 
     void update_state(WindowState const& window_state, InputState const& input_state);
     auto render_titlebar() -> std::optional<std::shared_ptr<graphics::Buffer>>;
@@ -115,6 +116,7 @@ class Renderer
     geometry::Size right_border_size;
     geometry::Size bottom_border_size;
     size_t solid_color_pixels_length{0};
+    size_t scaled_solid_color_pixels_length{0};
     std::unique_ptr<Pixel[]> solid_color_pixels; // can be nullptr
 
     geometry::Size titlebar_size{};
@@ -127,6 +129,8 @@ class Renderer
 
     std::shared_ptr<Text> const text;
 
+    float scale{1.0f};
+
     void update_solid_color_pixels();
     auto make_buffer(
         Pixel const* pixels,
diff --git a/src/server/shell/default_configuration.cpp b/src/server/shell/default_configuration.cpp
index 282c953f304..45c1b108c21 100644
--- a/src/server/shell/default_configuration.cpp
+++ b/src/server/shell/default_configuration.cpp
@@ -19,6 +19,7 @@
 #include "mir/default_server_configuration.h"
 #include "mir/abnormal_exit.h"
 
+#include "mir/graphics/display.h"
 #include "mir/input/composite_event_filter.h"
 #include "mir/shell/abstract_shell.h"
 #include "mir/options/configuration.h"
@@ -72,16 +73,27 @@ auto mir::DefaultServerConfiguration::the_decoration_manager() -> std::shared_pt
             return std::make_shared<msd::BasicManager>(
                 [buffer_allocator = the_buffer_allocator(),
                  executor = the_main_loop(),
-                 cursor_images = the_cursor_images()](
+                 cursor_images = the_cursor_images(),
+                 display = the_display()](
                     std::shared_ptr<shell::Shell> const& shell,
                     std::shared_ptr<scene::Surface> const& surface) -> std::unique_ptr<msd::Decoration>
                 {
+                    // Use the maximum scale to ensure sharp-looking decorations on all outputs
+                    auto max_output_scale{0.0f};
+                    auto const display_config{display->configuration()};
+                    display_config->for_each_output(
+                        [&](graphics::DisplayConfigurationOutput const& output_config)
+                        {
+                            max_output_scale = std::max(max_output_scale, output_config.scale);
+                        });
+
                     return std::make_unique<msd::BasicDecoration>(
                         shell,
                         buffer_allocator,
                         executor,
                         cursor_images,
-                        surface);
+                        surface,
+                        max_output_scale);
                 });
         });
 }
@@ -149,4 +161,3 @@ mir::DefaultServerConfiguration::the_shell_display_layout()
             return std::make_shared<msh::GraphicsDisplayLayout>(the_display());
         });
 }
-
diff --git a/tests/unit-tests/shell/test_decoration_basic_decoration.cpp b/tests/unit-tests/shell/test_decoration_basic_decoration.cpp
index 5282c1c91c4..e7bf11d2877 100644
--- a/tests/unit-tests/shell/test_decoration_basic_decoration.cpp
+++ b/tests/unit-tests/shell/test_decoration_basic_decoration.cpp
@@ -185,7 +185,8 @@ struct DecorationBasicDecoration
             mt::fake_shared(buffer_allocator),
             mt::fake_shared(executor),
             mt::fake_shared(cursor_images),
-            mt::fake_shared(window_surface));
+            mt::fake_shared(window_surface),
+            1.0f);
         executor.execute();
     }
 

From a413607b024e1ecf8eee950f00339f4339c9c6d4 Mon Sep 17 00:00:00 2001
From: Harlen Batagelo <hbatagelo@gmail.com>
Date: Fri, 8 Dec 2023 11:17:07 -0300
Subject: [PATCH 2/3] Monitor dynamic scale changes

---
 .../shell/decoration/basic_decoration.cpp     | 25 ++++--
 .../shell/decoration/basic_decoration.h       |  7 +-
 src/server/shell/decoration/basic_manager.cpp | 85 ++++++++++++++++++-
 src/server/shell/decoration/basic_manager.h   | 33 ++++++-
 src/server/shell/decoration/decoration.h      |  2 +
 src/server/shell/decoration/renderer.cpp      | 26 ++++--
 src/server/shell/decoration/renderer.h        |  3 +-
 src/server/shell/decoration/window.cpp        | 11 ++-
 src/server/shell/decoration/window.h          |  6 +-
 src/server/shell/default_configuration.cpp    | 17 +---
 .../test_decoration_basic_decoration.cpp      |  3 +-
 .../shell/test_decoration_basic_manager.cpp   | 11 ++-
 12 files changed, 185 insertions(+), 44 deletions(-)

diff --git a/src/server/shell/decoration/basic_decoration.cpp b/src/server/shell/decoration/basic_decoration.cpp
index 846484e7c1a..35c12f8efd5 100644
--- a/src/server/shell/decoration/basic_decoration.cpp
+++ b/src/server/shell/decoration/basic_decoration.cpp
@@ -158,8 +158,7 @@ msd::BasicDecoration::BasicDecoration(
     std::shared_ptr<mg::GraphicBufferAllocator> const& buffer_allocator,
     std::shared_ptr<Executor> const& executor,
     std::shared_ptr<input::CursorImages> const& cursor_images,
-    std::shared_ptr<ms::Surface> const& window_surface,
-    float scale)
+    std::shared_ptr<ms::Surface> const& window_surface)
     : threadsafe_self{std::make_shared<ThreadsafeAccess<BasicDecoration>>(executor)},
       static_geometry{std::make_shared<StaticGeometry>(default_geometry)},
       shell{shell},
@@ -167,7 +166,7 @@ msd::BasicDecoration::BasicDecoration(
       cursor_images{cursor_images},
       session{window_surface->session().lock()},
       buffer_streams{std::make_unique<BufferStreams>(session)},
-      renderer{std::make_unique<Renderer>(buffer_allocator, static_geometry, scale)},
+      renderer{std::make_unique<Renderer>(buffer_allocator, static_geometry)},
       window_surface{window_surface},
       decoration_surface{create_surface()},
       window_state{new_window_state()},
@@ -267,9 +266,15 @@ void msd::BasicDecoration::set_cursor(std::string const& cursor_image_name)
     shell->modify_surface(session, decoration_surface, spec);
 }
 
+void msd::BasicDecoration::set_scale(float scale)
+{
+    scale_ = scale;
+    window_state_updated();
+}
+
 auto msd::BasicDecoration::new_window_state() const -> std::unique_ptr<WindowState>
 {
-    return std::make_unique<WindowState>(static_geometry, window_surface);
+    return std::make_unique<WindowState>(static_geometry, window_surface, scale_);
 }
 
 auto msd::BasicDecoration::create_surface() const -> std::shared_ptr<scene::Surface>
@@ -370,7 +375,8 @@ void msd::BasicDecoration::update(
             &WindowState::titlebar_rect,
             &WindowState::left_border_rect,
             &WindowState::right_border_rect,
-            &WindowState::bottom_border_rect}) ||
+            &WindowState::bottom_border_rect,
+            &WindowState::scale}) ||
         input_updated({
             &InputState::buttons}))
     {
@@ -384,7 +390,8 @@ void msd::BasicDecoration::update(
     if (window_updated({
             &WindowState::focused_state,
             &WindowState::side_border_width,
-            &WindowState::side_border_height}))
+            &WindowState::side_border_height,
+            &WindowState::scale}))
     {
         new_buffers.emplace_back(
             buffer_streams->left_border,
@@ -397,7 +404,8 @@ void msd::BasicDecoration::update(
     if (window_updated({
             &WindowState::focused_state,
             &WindowState::bottom_border_width,
-            &WindowState::bottom_border_height}))
+            &WindowState::bottom_border_height,
+            &WindowState::scale}))
     {
         new_buffers.emplace_back(
             buffer_streams->bottom_border,
@@ -407,7 +415,8 @@ void msd::BasicDecoration::update(
     if (window_updated({
             &WindowState::focused_state,
             &WindowState::window_name,
-            &WindowState::titlebar_rect}) ||
+            &WindowState::titlebar_rect,
+            &WindowState::scale}) ||
         input_updated({
             &InputState::buttons}))
     {
diff --git a/src/server/shell/decoration/basic_decoration.h b/src/server/shell/decoration/basic_decoration.h
index d6dd1aaf3fd..07f408d145d 100644
--- a/src/server/shell/decoration/basic_decoration.h
+++ b/src/server/shell/decoration/basic_decoration.h
@@ -72,8 +72,7 @@ class BasicDecoration
         std::shared_ptr<graphics::GraphicBufferAllocator> const& buffer_allocator,
         std::shared_ptr<Executor> const& executor,
         std::shared_ptr<input::CursorImages> const& cursor_images,
-        std::shared_ptr<scene::Surface> const& window_surface,
-        float scale);
+        std::shared_ptr<scene::Surface> const& window_surface);
     ~BasicDecoration();
 
     void window_state_updated();
@@ -86,6 +85,8 @@ class BasicDecoration
     void request_close();
     void set_cursor(std::string const& cursor_image_name);
 
+    void set_scale(float scale) override;
+
 protected:
     /// Creates an up-to-date WindowState object
     auto new_window_state() const -> std::unique_ptr<WindowState>;
@@ -109,6 +110,8 @@ class BasicDecoration
     std::shared_ptr<input::CursorImages> const cursor_images;
     std::shared_ptr<scene::Session> const session;
 
+    float scale_{1.0f};
+
     class BufferStreams;
     std::unique_ptr<BufferStreams> buffer_streams;
     std::unique_ptr<Renderer> renderer;
diff --git a/src/server/shell/decoration/basic_manager.cpp b/src/server/shell/decoration/basic_manager.cpp
index f710ad8ee35..aa6098a9e32 100644
--- a/src/server/shell/decoration/basic_manager.cpp
+++ b/src/server/shell/decoration/basic_manager.cpp
@@ -17,6 +17,9 @@
 #include "basic_manager.h"
 #include "decoration.h"
 
+#include <mir/graphics/display_configuration_observer.h>
+#include <mir/graphics/display_configuration.h>
+
 #include <boost/throw_exception.hpp>
 #include <vector>
 
@@ -25,9 +28,56 @@ namespace mg = mir::graphics;
 namespace msh = mir::shell;
 namespace msd = mir::shell::decoration;
 
-msd::BasicManager::BasicManager(DecorationBuilder&& decoration_builder)
-    : decoration_builder{decoration_builder}
+class msd::DisplayConfigurationListener :  public mg::DisplayConfigurationObserver
+{
+public:
+    void set_listener(OutputListener* listener)
+    {
+        output_listener = listener;
+    }
+
+private:
+    void initial_configuration(std::shared_ptr<mg::DisplayConfiguration const> const& config) override
+    {
+        output_listener->advise_output_update(*config);
+    }
+    void configuration_applied(std::shared_ptr<mg::DisplayConfiguration const> const& config) override
+    {
+        output_listener->advise_output_update(*config);
+    }
+
+    void configuration_failed(
+        std::shared_ptr<mg::DisplayConfiguration const /*attempted*/> const&,
+        std::exception const& /*error*/) override {}
+
+    void catastrophic_configuration_error(
+        std::shared_ptr<mg::DisplayConfiguration const /*failed_fallback*/> const&,
+        std::exception const& /*error*/) override {}
+
+    void base_configuration_updated(
+        std::shared_ptr<mg::DisplayConfiguration const> const& /*base_config*/) override {}
+
+    void session_configuration_applied(
+        std::shared_ptr<ms::Session> const& /*session*/,
+        std::shared_ptr<mg::DisplayConfiguration> const& /*config*/) override {}
+
+    void session_configuration_removed(std::shared_ptr<ms::Session> const& /*session*/) override {}
+
+    void configuration_updated_for_session(
+        std::shared_ptr<ms::Session> const& /*session*/,
+        std::shared_ptr<mg::DisplayConfiguration const> const& /*config*/) override {}
+
+    OutputListener* output_listener;
+};
+
+msd::BasicManager::BasicManager(
+    ObserverRegistrar<mg::DisplayConfigurationObserver>& display_configuration_observers,
+    DecorationBuilder&& decoration_builder) :
+    decoration_builder{decoration_builder},
+    display_config_monitor{std::make_shared<DisplayConfigurationListener>()}
 {
+    display_config_monitor->set_listener(this);
+    display_configuration_observers.register_interest(display_config_monitor);
 }
 
 msd::BasicManager::~BasicManager()
@@ -52,6 +102,10 @@ void msd::BasicManager::decorate(std::shared_ptr<ms::Surface> const& surface)
         decorations[surface.get()] = nullptr;
         lock.unlock();
         auto decoration = decoration_builder(locked_shell, surface);
+        if (decoration)
+        {
+            decoration->set_scale(scale);
+        }
         lock.lock();
         decorations[surface.get()] = std::move(decoration);
     }
@@ -85,3 +139,30 @@ void msd::BasicManager::undecorate_all()
     // Destroy the decorations outside the lock
     to_destroy.clear();
 }
+
+void msd::BasicManager::advise_output_update(mg::DisplayConfiguration const& config)
+{
+    // Use the maximum scale to ensure sharp-looking decorations on all outputs
+    auto max_output_scale{0.0f};
+    config.for_each_output(
+        [&](mg::DisplayConfigurationOutput const& output)
+        {
+            if (!output.used || !output.connected) return;
+            if (!output.valid() || (output.current_mode_index >= output.modes.size())) return;
+
+            max_output_scale = std::max(max_output_scale, output.scale);
+        });
+
+    if (max_output_scale == scale) return;
+
+    scale = max_output_scale;
+
+    std::lock_guard lock{mutex};
+    for (auto& it : decorations)
+    {
+        if (it.second)
+        {
+            it.second->set_scale(scale);
+        }
+    }
+}
\ No newline at end of file
diff --git a/src/server/shell/decoration/basic_manager.h b/src/server/shell/decoration/basic_manager.h
index 684461e88f2..c6b9c9c2930 100644
--- a/src/server/shell/decoration/basic_manager.h
+++ b/src/server/shell/decoration/basic_manager.h
@@ -18,16 +18,22 @@
 #define MIR_SHELL_DECORATION_BASIC_MANAGER_H_
 
 #include "manager.h"
+
+#include <mir/observer_registrar.h>
+
 #include <functional>
 #include <unordered_map>
 #include <mutex>
 
+namespace mir::graphics { class DisplayConfigurationObserver; }
+
 namespace mir
 {
 class Executor;
 namespace graphics
 {
 class GraphicBufferAllocator;
+class DisplayConfiguration;
 }
 namespace shell
 {
@@ -35,17 +41,33 @@ class Shell;
 namespace decoration
 {
 class Decoration;
+class DisplayConfigurationListener;
+
+class OutputListener
+{
+public:
+    virtual void advise_output_update(mir::graphics::DisplayConfiguration const& config) = 0;
+
+protected:
+    OutputListener() = default;
+    virtual ~OutputListener() = default;
+    OutputListener(OutputListener const&) = delete;
+    OutputListener operator=(OutputListener const&) = delete;
+};
 
 /// Facilitates decorating windows with Mir's built-in server size decorations
-class BasicManager
-    : public Manager
+class BasicManager :
+    public Manager,
+    private OutputListener
 {
 public:
     using DecorationBuilder = std::function<std::unique_ptr<Decoration>(
         std::shared_ptr<shell::Shell> const& shell,
         std::shared_ptr<scene::Surface> const& surface)>;
 
-    BasicManager(DecorationBuilder&& decoration_builder);
+    BasicManager(
+        mir::ObserverRegistrar<mir::graphics::DisplayConfigurationObserver>& display_configuration_observers,
+        DecorationBuilder&& decoration_builder);
     ~BasicManager();
 
     void init(std::weak_ptr<shell::Shell> const& shell) override;
@@ -55,10 +77,15 @@ class BasicManager
 
 private:
     DecorationBuilder const decoration_builder;
+    std::shared_ptr<DisplayConfigurationListener> const display_config_monitor;
     std::weak_ptr<shell::Shell> shell;
 
     std::mutex mutex;
     std::unordered_map<scene::Surface*, std::unique_ptr<Decoration>> decorations;
+
+    float scale{1.0f};
+
+    void advise_output_update(graphics::DisplayConfiguration const& config) override;
 };
 }
 }
diff --git a/src/server/shell/decoration/decoration.h b/src/server/shell/decoration/decoration.h
index 8823d177208..d7203b8998b 100644
--- a/src/server/shell/decoration/decoration.h
+++ b/src/server/shell/decoration/decoration.h
@@ -30,6 +30,8 @@ class Decoration
     Decoration() = default;
     virtual ~Decoration() = default;
 
+    virtual void set_scale(float scale) = 0;
+
 private:
     Decoration(Decoration const&) = delete;
     Decoration& operator=(Decoration const&) = delete;
diff --git a/src/server/shell/decoration/renderer.cpp b/src/server/shell/decoration/renderer.cpp
index 4b04ca339c8..d9013e505de 100644
--- a/src/server/shell/decoration/renderer.cpp
+++ b/src/server/shell/decoration/renderer.cpp
@@ -446,8 +446,7 @@ auto msd::Renderer::Text::Impl::utf8_to_utf32(std::string const& text) -> std::u
 
 msd::Renderer::Renderer(
     std::shared_ptr<graphics::GraphicBufferAllocator> const& buffer_allocator,
-    std::shared_ptr<StaticGeometry const> const& static_geometry,
-    float scale)
+    std::shared_ptr<StaticGeometry const> const& static_geometry)
     : buffer_allocator{buffer_allocator},
       focused_theme{
           default_focused_background,
@@ -474,13 +473,25 @@ msd::Renderer::Renderer(
               render_minimize_icon}},
       },
       static_geometry{static_geometry},
-      text{Text::instance()},
-      scale{scale}
+      text{Text::instance()}
 {
 }
 
 void msd::Renderer::update_state(WindowState const& window_state, InputState const& input_state)
 {
+    if (auto const new_scale{window_state.scale()}; new_scale != scale)
+    {
+        scale = new_scale;
+
+        needs_solid_color_redraw = true;
+        solid_color_pixels.reset(); // force a reallocation next time it's needed
+
+        needs_titlebar_redraw = true;
+        titlebar_pixels.reset(); // force a reallocation next time it's needed
+
+        needs_titlebar_buttons_redraw = true;
+    }
+
     left_border_size = window_state.left_border_rect().size;
     right_border_size = window_state.right_border_rect().size;
     bottom_border_size = window_state.bottom_border_rect().size;
@@ -490,11 +501,12 @@ void msd::Renderer::update_state(WindowState const& window_state, InputState con
     length = std::max(length, area(right_border_size));
     length = std::max(length, area(bottom_border_size));
 
-    if (length != solid_color_pixels_length)
+    if (auto const scaled_length{static_cast<size_t>(length * scale * scale)};
+        length != solid_color_pixels_length ||
+        scaled_length != scaled_solid_color_pixels_length)
     {
         solid_color_pixels_length = length;
-        auto const squared_scale{scale * scale};
-        scaled_solid_color_pixels_length = length * squared_scale;
+        scaled_solid_color_pixels_length = scaled_length;
         solid_color_pixels.reset(); // force a reallocation next time it's needed
     }
 
diff --git a/src/server/shell/decoration/renderer.h b/src/server/shell/decoration/renderer.h
index 0ed4d062276..09754a4c1da 100644
--- a/src/server/shell/decoration/renderer.h
+++ b/src/server/shell/decoration/renderer.h
@@ -47,8 +47,7 @@ class Renderer
 public:
     Renderer(
         std::shared_ptr<graphics::GraphicBufferAllocator> const& buffer_allocator,
-        std::shared_ptr<StaticGeometry const> const& static_geometry,
-        float scale);
+        std::shared_ptr<StaticGeometry const> const& static_geometry);
 
     void update_state(WindowState const& window_state, InputState const& input_state);
     auto render_titlebar() -> std::optional<std::shared_ptr<graphics::Buffer>>;
diff --git a/src/server/shell/decoration/window.cpp b/src/server/shell/decoration/window.cpp
index 12bae4c3c2f..77a61f364e8 100644
--- a/src/server/shell/decoration/window.cpp
+++ b/src/server/shell/decoration/window.cpp
@@ -76,12 +76,14 @@ auto border_type_for(std::shared_ptr<ms::Surface> const& surface) -> msd::Border
 
 msd::WindowState::WindowState(
     std::shared_ptr<StaticGeometry const> const& static_geometry,
-    std::shared_ptr<scene::Surface> const& surface)
+    std::shared_ptr<scene::Surface> const& surface,
+    float scale)
     : static_geometry{static_geometry},
       window_size_{surface->window_size()},
       border_type_{border_type_for(surface)},
       focus_state_{surface->focus_state()},
-      window_name_{surface->name()}
+      window_name_{surface->name()},
+      scale_{scale}
 {
 }
 
@@ -226,6 +228,11 @@ auto msd::WindowState::button_rect(unsigned n) const -> geom::Rectangle
         {static_geometry->button_width, titlebar.size.height}};
 }
 
+auto msd::WindowState::scale() const -> float
+{
+    return scale_;
+}
+
 class msd::WindowSurfaceObserverManager::Observer
     : public ms::NullSurfaceObserver
 {
diff --git a/src/server/shell/decoration/window.h b/src/server/shell/decoration/window.h
index 6c4b28ecef1..5448d61c5b4 100644
--- a/src/server/shell/decoration/window.h
+++ b/src/server/shell/decoration/window.h
@@ -70,7 +70,8 @@ class WindowState
 public:
     WindowState(
         std::shared_ptr<StaticGeometry const> const& static_geometry,
-        std::shared_ptr<scene::Surface> const& window_surface);
+        std::shared_ptr<scene::Surface> const& window_surface,
+        float scale);
 
     auto window_size() const -> geometry::Size;
     auto border_type() const -> BorderType;
@@ -94,6 +95,8 @@ class WindowState
     /// Returns the rectangle of the nth button
     auto button_rect(unsigned n) const -> geometry::Rectangle;
 
+    auto scale() const -> float;
+
 private:
     WindowState(WindowState const&) = delete;
     WindowState& operator=(WindowState const&) = delete;
@@ -103,6 +106,7 @@ class WindowState
     BorderType const border_type_;
     MirWindowFocusState const focus_state_;
     std::string window_name_;
+    float scale_;
 };
 
 /// Observes the decorated window and calls on_update when its state changes
diff --git a/src/server/shell/default_configuration.cpp b/src/server/shell/default_configuration.cpp
index 45c1b108c21..d9ebb69f7b5 100644
--- a/src/server/shell/default_configuration.cpp
+++ b/src/server/shell/default_configuration.cpp
@@ -19,7 +19,6 @@
 #include "mir/default_server_configuration.h"
 #include "mir/abnormal_exit.h"
 
-#include "mir/graphics/display.h"
 #include "mir/input/composite_event_filter.h"
 #include "mir/shell/abstract_shell.h"
 #include "mir/options/configuration.h"
@@ -71,29 +70,19 @@ auto mir::DefaultServerConfiguration::the_decoration_manager() -> std::shared_pt
         [this]()->std::shared_ptr<msd::Manager>
         {
             return std::make_shared<msd::BasicManager>(
+                *the_display_configuration_observer_registrar(),
                 [buffer_allocator = the_buffer_allocator(),
                  executor = the_main_loop(),
-                 cursor_images = the_cursor_images(),
-                 display = the_display()](
+                 cursor_images = the_cursor_images()](
                     std::shared_ptr<shell::Shell> const& shell,
                     std::shared_ptr<scene::Surface> const& surface) -> std::unique_ptr<msd::Decoration>
                 {
-                    // Use the maximum scale to ensure sharp-looking decorations on all outputs
-                    auto max_output_scale{0.0f};
-                    auto const display_config{display->configuration()};
-                    display_config->for_each_output(
-                        [&](graphics::DisplayConfigurationOutput const& output_config)
-                        {
-                            max_output_scale = std::max(max_output_scale, output_config.scale);
-                        });
-
                     return std::make_unique<msd::BasicDecoration>(
                         shell,
                         buffer_allocator,
                         executor,
                         cursor_images,
-                        surface,
-                        max_output_scale);
+                        surface);
                 });
         });
 }
diff --git a/tests/unit-tests/shell/test_decoration_basic_decoration.cpp b/tests/unit-tests/shell/test_decoration_basic_decoration.cpp
index e7bf11d2877..5282c1c91c4 100644
--- a/tests/unit-tests/shell/test_decoration_basic_decoration.cpp
+++ b/tests/unit-tests/shell/test_decoration_basic_decoration.cpp
@@ -185,8 +185,7 @@ struct DecorationBasicDecoration
             mt::fake_shared(buffer_allocator),
             mt::fake_shared(executor),
             mt::fake_shared(cursor_images),
-            mt::fake_shared(window_surface),
-            1.0f);
+            mt::fake_shared(window_surface));
         executor.execute();
     }
 
diff --git a/tests/unit-tests/shell/test_decoration_basic_manager.cpp b/tests/unit-tests/shell/test_decoration_basic_manager.cpp
index 8cbc439d60f..c44574160a1 100644
--- a/tests/unit-tests/shell/test_decoration_basic_manager.cpp
+++ b/tests/unit-tests/shell/test_decoration_basic_manager.cpp
@@ -17,6 +17,7 @@
 #include "src/server/shell/decoration/basic_manager.h"
 #include "src/server/shell/decoration/decoration.h"
 
+#include "mir/test/doubles/stub_observer_registrar.h"
 #include "mir/test/doubles/stub_shell.h"
 
 #include <gmock/gmock.h>
@@ -33,6 +34,7 @@ using namespace testing;
 class StubDecoration
     : public msd::Decoration
 {
+    void set_scale(float /*scale*/) override {};
 };
 
 struct DecorationBasicManager
@@ -53,7 +55,12 @@ struct DecorationBasicManager
 
     MOCK_METHOD1(decoration_destroyed, void(msd::Decoration*));
 
-    msd::BasicManager manager{[this](
+    std::shared_ptr<mir::ObserverRegistrar<mir::graphics::DisplayConfigurationObserver>>
+        registrar{std::make_shared<mtd::StubObserverRegistrar<mir::graphics::DisplayConfigurationObserver>>()};
+
+    msd::BasicManager manager{
+        *registrar,
+        [this](
             std::shared_ptr<msh::Shell> const&,
             std::shared_ptr<ms::Surface> const&) -> std::unique_ptr<msd::Decoration>
         {
@@ -76,6 +83,8 @@ class MockDecoration
         mock->decoration_destroyed(this);
     }
 
+    MOCK_METHOD1(set_scale, void(float));
+
     DecorationBasicManager* const mock;
 };
 

From 359a174b1ee4037f96f72583063f6eed9aaa0d73 Mon Sep 17 00:00:00 2001
From: Harlen Batagelo <hbatagelo@gmail.com>
Date: Wed, 13 Dec 2023 11:05:27 -0300
Subject: [PATCH 3/3] Refactoring based on code review feedback

---
 .../shell/decoration/basic_decoration.cpp     |  6 +-
 .../shell/decoration/basic_decoration.h       |  2 +-
 src/server/shell/decoration/basic_manager.cpp | 84 +++++++------------
 src/server/shell/decoration/basic_manager.h   | 19 +----
 src/server/shell/decoration/decoration.h      |  2 +-
 .../shell/test_decoration_basic_manager.cpp   |  8 +-
 6 files changed, 41 insertions(+), 80 deletions(-)

diff --git a/src/server/shell/decoration/basic_decoration.cpp b/src/server/shell/decoration/basic_decoration.cpp
index 35c12f8efd5..7d56d239bc7 100644
--- a/src/server/shell/decoration/basic_decoration.cpp
+++ b/src/server/shell/decoration/basic_decoration.cpp
@@ -266,15 +266,15 @@ void msd::BasicDecoration::set_cursor(std::string const& cursor_image_name)
     shell->modify_surface(session, decoration_surface, spec);
 }
 
-void msd::BasicDecoration::set_scale(float scale)
+void msd::BasicDecoration::set_scale(float new_scale)
 {
-    scale_ = scale;
+    scale = new_scale;
     window_state_updated();
 }
 
 auto msd::BasicDecoration::new_window_state() const -> std::unique_ptr<WindowState>
 {
-    return std::make_unique<WindowState>(static_geometry, window_surface, scale_);
+    return std::make_unique<WindowState>(static_geometry, window_surface, scale);
 }
 
 auto msd::BasicDecoration::create_surface() const -> std::shared_ptr<scene::Surface>
diff --git a/src/server/shell/decoration/basic_decoration.h b/src/server/shell/decoration/basic_decoration.h
index 07f408d145d..f2071902c6a 100644
--- a/src/server/shell/decoration/basic_decoration.h
+++ b/src/server/shell/decoration/basic_decoration.h
@@ -110,7 +110,7 @@ class BasicDecoration
     std::shared_ptr<input::CursorImages> const cursor_images;
     std::shared_ptr<scene::Session> const session;
 
-    float scale_{1.0f};
+    float scale{1.0f};
 
     class BufferStreams;
     std::unique_ptr<BufferStreams> buffer_streams;
diff --git a/src/server/shell/decoration/basic_manager.cpp b/src/server/shell/decoration/basic_manager.cpp
index aa6098a9e32..4056707df00 100644
--- a/src/server/shell/decoration/basic_manager.cpp
+++ b/src/server/shell/decoration/basic_manager.cpp
@@ -17,8 +17,8 @@
 #include "basic_manager.h"
 #include "decoration.h"
 
-#include <mir/graphics/display_configuration_observer.h>
 #include <mir/graphics/display_configuration.h>
+#include <mir/graphics/null_display_configuration_observer.h>
 
 #include <boost/throw_exception.hpp>
 #include <vector>
@@ -28,55 +28,46 @@ namespace mg = mir::graphics;
 namespace msh = mir::shell;
 namespace msd = mir::shell::decoration;
 
-class msd::DisplayConfigurationListener :  public mg::DisplayConfigurationObserver
+class msd::DisplayConfigurationListener :  public mg::NullDisplayConfigurationObserver
 {
 public:
-    void set_listener(OutputListener* listener)
-    {
-        output_listener = listener;
-    }
+    using Callback = std::function<void(mg::DisplayConfiguration const&)>;
+
+    explicit DisplayConfigurationListener(Callback callback) : callback{std::move(callback)} {}
 
 private:
     void initial_configuration(std::shared_ptr<mg::DisplayConfiguration const> const& config) override
     {
-        output_listener->advise_output_update(*config);
+        callback(*config);
     }
     void configuration_applied(std::shared_ptr<mg::DisplayConfiguration const> const& config) override
     {
-        output_listener->advise_output_update(*config);
+        callback(*config);
     }
 
-    void configuration_failed(
-        std::shared_ptr<mg::DisplayConfiguration const /*attempted*/> const&,
-        std::exception const& /*error*/) override {}
-
-    void catastrophic_configuration_error(
-        std::shared_ptr<mg::DisplayConfiguration const /*failed_fallback*/> const&,
-        std::exception const& /*error*/) override {}
-
-    void base_configuration_updated(
-        std::shared_ptr<mg::DisplayConfiguration const> const& /*base_config*/) override {}
-
-    void session_configuration_applied(
-        std::shared_ptr<ms::Session> const& /*session*/,
-        std::shared_ptr<mg::DisplayConfiguration> const& /*config*/) override {}
-
-    void session_configuration_removed(std::shared_ptr<ms::Session> const& /*session*/) override {}
-
-    void configuration_updated_for_session(
-        std::shared_ptr<ms::Session> const& /*session*/,
-        std::shared_ptr<mg::DisplayConfiguration const> const& /*config*/) override {}
-
-    OutputListener* output_listener;
+    Callback const callback;
 };
 
 msd::BasicManager::BasicManager(
     ObserverRegistrar<mg::DisplayConfigurationObserver>& display_configuration_observers,
     DecorationBuilder&& decoration_builder) :
-    decoration_builder{decoration_builder},
-    display_config_monitor{std::make_shared<DisplayConfigurationListener>()}
+    decoration_builder{std::move(decoration_builder)},
+    display_config_monitor{std::make_shared<DisplayConfigurationListener>(
+        [&](mg::DisplayConfiguration const& config)
+        {
+            // Use the maximum scale to ensure sharp-looking decorations on all outputs
+            auto max_output_scale{0.0f};
+            config.for_each_output(
+                [&](mg::DisplayConfigurationOutput const& output)
+                {
+                    if (!output.used || !output.connected) return;
+                    if (!output.valid() || (output.current_mode_index >= output.modes.size())) return;
+
+                    max_output_scale = std::max(max_output_scale, output.scale);
+                });
+            set_scale(max_output_scale);
+        })}
 {
-    display_config_monitor->set_listener(this);
     display_configuration_observers.register_interest(display_config_monitor);
 }
 
@@ -102,11 +93,8 @@ void msd::BasicManager::decorate(std::shared_ptr<ms::Surface> const& surface)
         decorations[surface.get()] = nullptr;
         lock.unlock();
         auto decoration = decoration_builder(locked_shell, surface);
-        if (decoration)
-        {
-            decoration->set_scale(scale);
-        }
         lock.lock();
+        decoration->set_scale(scale);
         decorations[surface.get()] = std::move(decoration);
     }
 }
@@ -140,27 +128,13 @@ void msd::BasicManager::undecorate_all()
     to_destroy.clear();
 }
 
-void msd::BasicManager::advise_output_update(mg::DisplayConfiguration const& config)
+void msd::BasicManager::set_scale(float new_scale)
 {
-    // Use the maximum scale to ensure sharp-looking decorations on all outputs
-    auto max_output_scale{0.0f};
-    config.for_each_output(
-        [&](mg::DisplayConfigurationOutput const& output)
-        {
-            if (!output.used || !output.connected) return;
-            if (!output.valid() || (output.current_mode_index >= output.modes.size())) return;
-
-            max_output_scale = std::max(max_output_scale, output.scale);
-        });
-
-    if (max_output_scale == scale) return;
-
-    scale = max_output_scale;
-
     std::lock_guard lock{mutex};
-    for (auto& it : decorations)
+    if (new_scale != scale)
     {
-        if (it.second)
+        scale = new_scale;
+        for (auto& it : decorations)
         {
             it.second->set_scale(scale);
         }
diff --git a/src/server/shell/decoration/basic_manager.h b/src/server/shell/decoration/basic_manager.h
index c6b9c9c2930..be4f2413924 100644
--- a/src/server/shell/decoration/basic_manager.h
+++ b/src/server/shell/decoration/basic_manager.h
@@ -33,7 +33,6 @@ class Executor;
 namespace graphics
 {
 class GraphicBufferAllocator;
-class DisplayConfiguration;
 }
 namespace shell
 {
@@ -43,22 +42,9 @@ namespace decoration
 class Decoration;
 class DisplayConfigurationListener;
 
-class OutputListener
-{
-public:
-    virtual void advise_output_update(mir::graphics::DisplayConfiguration const& config) = 0;
-
-protected:
-    OutputListener() = default;
-    virtual ~OutputListener() = default;
-    OutputListener(OutputListener const&) = delete;
-    OutputListener operator=(OutputListener const&) = delete;
-};
-
 /// Facilitates decorating windows with Mir's built-in server size decorations
 class BasicManager :
-    public Manager,
-    private OutputListener
+    public Manager
 {
 public:
     using DecorationBuilder = std::function<std::unique_ptr<Decoration>(
@@ -82,10 +68,9 @@ class BasicManager :
 
     std::mutex mutex;
     std::unordered_map<scene::Surface*, std::unique_ptr<Decoration>> decorations;
-
     float scale{1.0f};
 
-    void advise_output_update(graphics::DisplayConfiguration const& config) override;
+    void set_scale(float new_scale);
 };
 }
 }
diff --git a/src/server/shell/decoration/decoration.h b/src/server/shell/decoration/decoration.h
index d7203b8998b..2389ad2531c 100644
--- a/src/server/shell/decoration/decoration.h
+++ b/src/server/shell/decoration/decoration.h
@@ -30,7 +30,7 @@ class Decoration
     Decoration() = default;
     virtual ~Decoration() = default;
 
-    virtual void set_scale(float scale) = 0;
+    virtual void set_scale(float new_scale) = 0;
 
 private:
     Decoration(Decoration const&) = delete;
diff --git a/tests/unit-tests/shell/test_decoration_basic_manager.cpp b/tests/unit-tests/shell/test_decoration_basic_manager.cpp
index c44574160a1..5dcf7e6ba98 100644
--- a/tests/unit-tests/shell/test_decoration_basic_manager.cpp
+++ b/tests/unit-tests/shell/test_decoration_basic_manager.cpp
@@ -34,7 +34,7 @@ using namespace testing;
 class StubDecoration
     : public msd::Decoration
 {
-    void set_scale(float /*scale*/) override {};
+    void set_scale(float /*new_scale*/) override {};
 };
 
 struct DecorationBasicManager
@@ -92,7 +92,8 @@ TEST_F(DecorationBasicManager, calls_build_decoration)
 {
     auto const surface = std::make_shared<mtd::StubSurface>();
     EXPECT_CALL(*this, build_decoration())
-        .Times(1);
+        .Times(1)
+        .WillOnce(Invoke([](){ return new StubDecoration; }));
     manager.decorate(surface);
 }
 
@@ -108,7 +109,8 @@ TEST_F(DecorationBasicManager, decorating_a_surface_is_idempotent)
 {
     auto const surface = std::make_shared<mtd::StubSurface>();
     EXPECT_CALL(*this, build_decoration())
-        .Times(1);
+        .Times(1)
+        .WillOnce(Invoke([](){ return new StubDecoration; }));
     manager.decorate(surface);
     manager.decorate(surface);
     manager.decorate(surface);