From f2e3a91966dd2d0118a5dc7ef7b56d93dcad1809 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Mon, 7 Oct 2024 12:30:10 +0100 Subject: [PATCH 1/4] BasicWindowManager::place_new_surface() should not set a default size --- src/miral/basic_window_manager.cpp | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/miral/basic_window_manager.cpp b/src/miral/basic_window_manager.cpp index 89c3c3ccbef..0e678976e36 100644 --- a/src/miral/basic_window_manager.cpp +++ b/src/miral/basic_window_manager.cpp @@ -1855,10 +1855,7 @@ auto miral::BasicWindowManager::place_new_surface(WindowSpecification parameters if (!parameters.state().is_set()) parameters.state() = mir_window_state_restored; - if (!parameters.size().is_set()) - { - parameters.size() = {640, 480}; - } + auto const proposed_size = parameters.size().value_or({640, 480}); std::shared_ptr display_area; if (parameters.output_id().is_set()) @@ -1890,7 +1887,7 @@ auto miral::BasicWindowManager::place_new_surface(WindowSpecification parameters auto const position = place_relative( parent_content_area, parameters, - parameters.size().value()); + proposed_size); if (position.is_set()) { @@ -1911,8 +1908,8 @@ auto miral::BasicWindowManager::place_new_surface(WindowSpecification parameters auto const parent = info_for(parameters.parent().value()).window(); auto const parent_top_left = parent.top_left(); auto const centred = parent_top_left - + 0.5*(as_displacement(parent.size()) - as_displacement(parameters.size().value())) - - (as_delta(parent.size().height) - as_delta(parameters.size().value().height)) / 6; + + 0.5*(as_displacement(parent.size()) - as_displacement(proposed_size)) + - (as_delta(parent.size().height) - as_delta(proposed_size.height)) / 6; parameters.top_left() = centred; positioned = true; @@ -1932,8 +1929,8 @@ auto miral::BasicWindowManager::place_new_surface(WindowSpecification parameters if (!positioned) { auto centred = application_zone.top_left - + 0.5*(as_displacement(application_zone.size) - as_displacement(parameters.size().value())) - - (as_delta(application_zone.size.height) - as_delta(parameters.size().value().height)) / 6; + + 0.5*(as_displacement(application_zone.size) - as_displacement(proposed_size)) + - (as_delta(application_zone.size.height) - as_delta(proposed_size.height)) / 6; switch (parameters.state().value()) { @@ -1950,13 +1947,13 @@ auto miral::BasicWindowManager::place_new_surface(WindowSpecification parameters case mir_window_state_vertmaximized: centred.y = application_zone.top_left.y; parameters.top_left() = centred; - parameters.size() = Size{parameters.size().value().width, application_zone.size.height}; + parameters.size() = Size{proposed_size.width, application_zone.size.height}; break; case mir_window_state_horizmaximized: centred.x = application_zone.top_left.x; parameters.top_left() = centred; - parameters.size() = Size{application_zone.size.width, parameters.size().value().height}; + parameters.size() = Size{application_zone.size.width, proposed_size.height}; break; default: From 701b9d9d476b8acdcbdc2cadfb71be74c443af63 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Mon, 7 Oct 2024 12:31:13 +0100 Subject: [PATCH 2/4] WindowWlSurfaceRole::current_size() should return a sensible size, or {0, 0} --- .../window_wl_surface_role.cpp | 26 +++++++------------ .../frontend_wayland/window_wl_surface_role.h | 4 +-- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/server/frontend_wayland/window_wl_surface_role.cpp b/src/server/frontend_wayland/window_wl_surface_role.cpp index 9d29390ca5c..3b3608abdeb 100644 --- a/src/server/frontend_wayland/window_wl_surface_role.cpp +++ b/src/server/frontend_wayland/window_wl_surface_role.cpp @@ -294,22 +294,16 @@ auto mf::WindowWlSurfaceRole::pending_size() const -> geom::Size auto mf::WindowWlSurfaceRole::current_size() const -> geom::Size { - auto size = geom::Size{640, 480}; - if ((!committed_width_set_explicitly || !committed_height_set_explicitly) && surface) + if (surface) { - if (auto const buffer_size = surface.value().buffer_size()) - { - if (!committed_width_set_explicitly) - { - size.width = buffer_size->width; - } - if (!committed_height_set_explicitly) - { - size.height = buffer_size->height; - } - } + auto const buffer_size = surface.value().buffer_size(); + + return { + committed_width_set_explicitly.value_or(buffer_size->width), + committed_height_set_explicitly.value_or(buffer_size->height)}; } - return size; + + return geom::Size{0, 0}; } auto mf::WindowWlSurfaceRole::requested_window_size() const -> std::optional @@ -389,9 +383,9 @@ void mf::WindowWlSurfaceRole::commit(WlSurfaceState const& state) } if (pending_explicit_width) - committed_width_set_explicitly = true; + committed_width_set_explicitly = pending_explicit_width; if (pending_explicit_height) - committed_height_set_explicitly = true; + committed_height_set_explicitly = pending_explicit_height; pending_explicit_width = std::nullopt; pending_explicit_height = std::nullopt; diff --git a/src/server/frontend_wayland/window_wl_surface_role.h b/src/server/frontend_wayland/window_wl_surface_role.h index 04fc3c4c777..7a1f7dc1f07 100644 --- a/src/server/frontend_wayland/window_wl_surface_role.h +++ b/src/server/frontend_wayland/window_wl_surface_role.h @@ -155,8 +155,8 @@ class WindowWlSurfaceRole /// If the committed window size was set explicitly, rather than being taken from the buffer size /// @{ - bool committed_width_set_explicitly{false}; - bool committed_height_set_explicitly{false}; + std::optional committed_width_set_explicitly; + std::optional committed_height_set_explicitly; /// @} /// The min and max size of the window as of last commit From 935e08ca07ee04e82e92050347b9d7bbabdbe2bf Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Thu, 10 Oct 2024 11:34:49 +0100 Subject: [PATCH 3/4] Don't fake surface size --- src/server/scene/basic_surface.cpp | 4 ++-- src/server/scene/surface_allocator.cpp | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/server/scene/basic_surface.cpp b/src/server/scene/basic_surface.cpp index 3e82471bf0a..b851fe5ba83 100644 --- a/src/server/scene/basic_surface.cpp +++ b/src/server/scene/basic_surface.cpp @@ -982,8 +982,8 @@ void mir::scene::BasicSurface::update_frame_posted_callbacks(State& state) auto mir::scene::BasicSurface::content_size(State const& state) const -> geometry::Size { return geom::Size{ - std::max(state.surface_rect.size.width - state.margins.left - state.margins.right, geom::Width{1}), - std::max(state.surface_rect.size.height - state.margins.top - state.margins.bottom, geom::Height{1})}; + std::max(state.surface_rect.size.width - state.margins.left - state.margins.right, geom::Width{0}), + std::max(state.surface_rect.size.height - state.margins.top - state.margins.bottom, geom::Height{0})}; } auto mir::scene::BasicSurface::content_top_left(State const& state) const -> geometry::Point diff --git a/src/server/scene/surface_allocator.cpp b/src/server/scene/surface_allocator.cpp index 84274c3f8cf..9c020c73b33 100644 --- a/src/server/scene/surface_allocator.cpp +++ b/src/server/scene/surface_allocator.cpp @@ -44,9 +44,8 @@ std::shared_ptr ms::SurfaceAllocator::create_surface( auto confine = params.confine_pointer.is_set() ? params.confine_pointer.value() : mir_pointer_unconfined; auto const name = params.name.is_set() ? params.name.value() : ""; geom::Rectangle const rect{ - params.top_left.is_set() ? params.top_left.value() : geom::Point{}, { - params.width.is_set() ? params.width.value() : geom::Width{100}, - params.height.is_set() ? params.height.value() : geom::Height{100}}}; + params.top_left.value_or(geom::Point{}), + {params.width.value_or(geom::Width{0}), params.height.value_or(geom::Height{0})}}; auto const parent = params.parent.is_set() ? params.parent.value() : std::weak_ptr{}; auto const surface = std::make_shared( session, From d715f563774a09453a892d105c17a7354d1d1f13 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Thu, 10 Oct 2024 12:23:32 +0100 Subject: [PATCH 4/4] If no buffer has been submitted, use a zero size, not undefined behaviour --- src/server/frontend_wayland/window_wl_surface_role.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/server/frontend_wayland/window_wl_surface_role.cpp b/src/server/frontend_wayland/window_wl_surface_role.cpp index 3b3608abdeb..f76e29953ff 100644 --- a/src/server/frontend_wayland/window_wl_surface_role.cpp +++ b/src/server/frontend_wayland/window_wl_surface_role.cpp @@ -296,11 +296,11 @@ auto mf::WindowWlSurfaceRole::current_size() const -> geom::Size { if (surface) { - auto const buffer_size = surface.value().buffer_size(); + auto const buffer_size = surface.value().buffer_size().value_or(geom::Size{}); return { - committed_width_set_explicitly.value_or(buffer_size->width), - committed_height_set_explicitly.value_or(buffer_size->height)}; + committed_width_set_explicitly.value_or(buffer_size.width), + committed_height_set_explicitly.value_or(buffer_size.height)}; } return geom::Size{0, 0};