From ef8d3b17d231a1619087dec860145f18095060df Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Tue, 19 Nov 2024 09:56:54 +0000 Subject: [PATCH 1/4] New attached windows need to be placed --- src/miral/basic_window_manager.cpp | 85 +++++++++++++++++++----------- src/miral/basic_window_manager.h | 1 + 2 files changed, 54 insertions(+), 32 deletions(-) diff --git a/src/miral/basic_window_manager.cpp b/src/miral/basic_window_manager.cpp index 0e678976e36..03248a34a10 100644 --- a/src/miral/basic_window_manager.cpp +++ b/src/miral/basic_window_manager.cpp @@ -1283,38 +1283,7 @@ void miral::BasicWindowManager::place_attached_to_zone( case mir_window_state_attached: { - MirPlacementGravity edges = info.attached_edges(); - Rectangle placement_zone = application_zone; - - if ((edges & mir_placement_gravity_west) && - (edges & mir_placement_gravity_east)) - { - placement_zone.top_left.x = application_zone.top_left.x; - placement_zone.size.width = application_zone.size.width; - size.width = placement_zone.size.width; - } - - if ((edges & mir_placement_gravity_north) && - (edges & mir_placement_gravity_south)) - { - placement_zone.top_left.y = application_zone.top_left.y; - placement_zone.size.height = application_zone.size.height; - size.height = placement_zone.size.height; - } - - if (edges & mir_placement_gravity_west) - top_left.x = placement_zone.left(); - else if (edges & mir_placement_gravity_east) - top_left.x = placement_zone.right() - (size.width - Width{0}); - else - top_left.x = placement_zone.top_left.x + (placement_zone.size.width - size.width) * 0.5; - - if (edges & mir_placement_gravity_north) - top_left.y = placement_zone.top(); - else if (edges & mir_placement_gravity_south) - top_left.y = placement_zone.bottom() - (size.height - Height{0}); - else - top_left.y = placement_zone.top_left.y + (placement_zone.size.height - size.height) * 0.5; + place_attached(top_left, size, info.attached_edges(), application_zone); update_window = true; break; } @@ -1847,6 +1816,47 @@ auto miral::BasicWindowManager::can_activate_window_for_session_in_workspace( return new_focus; } +void miral::BasicWindowManager::place_attached(Point& top_left, Size& size, MirPlacementGravity edges, Rectangle const application_zone) +{ + if ((edges & mir_placement_gravity_west) && + (edges & mir_placement_gravity_east)) + { + size.width = application_zone.size.width; + } + + if ((edges & mir_placement_gravity_north) && + (edges & mir_placement_gravity_south)) + { + size.height = application_zone.size.height; + } + + if (edges & mir_placement_gravity_west) + { + top_left.x = application_zone.left(); + } + else if (edges & mir_placement_gravity_east) + { + top_left.x = application_zone.right() - (size.width - Width{0}); + } + else + { + top_left.x = application_zone.top_left.x + (application_zone.size.width - size.width) * 0.5; + } + + if (edges & mir_placement_gravity_north) + { + top_left.y = application_zone.top(); + } + else if (edges & mir_placement_gravity_south) + { + top_left.y = application_zone.bottom() - (size.height - Height{0}); + } + else + { + top_left.y = application_zone.top_left.y + (application_zone.size.height - size.height) * 0.5; + } +} + auto miral::BasicWindowManager::place_new_surface(WindowSpecification parameters) -> WindowSpecification { if (!parameters.type().is_set()) @@ -1956,6 +1966,17 @@ auto miral::BasicWindowManager::place_new_surface(WindowSpecification parameters parameters.size() = Size{application_zone.size.width, proposed_size.height}; break; + case mir_window_state_attached: + { + auto size = proposed_size; + auto top_left = display_area->area.top_left; + + place_attached(top_left, size, parameters.attached_edges().value(), application_zone); + + parameters.top_left() = top_left; + parameters.size() = size; + break; + } default: parameters.top_left() = centred; } diff --git a/src/miral/basic_window_manager.h b/src/miral/basic_window_manager.h index faeac782cf1..f2ac8ed19ea 100644 --- a/src/miral/basic_window_manager.h +++ b/src/miral/basic_window_manager.h @@ -286,6 +286,7 @@ class BasicWindowManager : public virtual mir::shell::WindowManager, auto can_activate_window_for_session_in_workspace( miral::Application const& session, std::vector> const& workspaces) -> bool; + void place_attached(Point& top_left, Size& size, MirPlacementGravity edges, Rectangle application_zone); auto place_new_surface(WindowSpecification parameters) -> WindowSpecification; auto place_relative(mir::geometry::Rectangle const& parent, miral::WindowSpecification const& parameters, Size size) From a9586363e92adc8869cc2dc8d09eaf94f52ba506 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Thu, 21 Nov 2024 12:55:21 +0000 Subject: [PATCH 2/4] Don't send configure before WM places the window --- src/server/frontend_wayland/layer_shell_v1.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/frontend_wayland/layer_shell_v1.cpp b/src/server/frontend_wayland/layer_shell_v1.cpp index 2b6ca3b0a02..59451171462 100644 --- a/src/server/frontend_wayland/layer_shell_v1.cpp +++ b/src/server/frontend_wayland/layer_shell_v1.cpp @@ -205,7 +205,7 @@ class LayerSurfaceV1 : public wayland::LayerSurfaceV1, public WindowWlSurfaceRol /// the client acks a configure. DoubleBuffered client_size; DoubleBuffered offset; - bool configure_on_next_commit{true}; ///< If to send a .configure event at the end of the next or current commit + bool configure_on_next_commit{false}; ///< If to send a .configure event at the end of the next or current commit MirFocusMode current_focus_mode{mir_focus_mode_disabled}; std::deque> inflight_configures; std::vector> popups; ///< We have to keep track of popups to adjust their offset From 9247cc95c8c4a981dc1a43ac2b3ccb34653511d8 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Fri, 22 Nov 2024 16:16:22 -0500 Subject: [PATCH 3/4] Making the client_size an OptionalSize so that we do not accidentally use 0 widths and height --- .../frontend_wayland/layer_shell_v1.cpp | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/src/server/frontend_wayland/layer_shell_v1.cpp b/src/server/frontend_wayland/layer_shell_v1.cpp index 59451171462..01fdc96a6c8 100644 --- a/src/server/frontend_wayland/layer_shell_v1.cpp +++ b/src/server/frontend_wayland/layer_shell_v1.cpp @@ -199,11 +199,9 @@ class LayerSurfaceV1 : public wayland::LayerSurfaceV1, public WindowWlSurfaceRol DoubleBuffered exclusive_zone{0}; DoubleBuffered anchors; DoubleBuffered margin; - bool width_set_by_client{false}; ///< If the client gave a width on the last .set_size() request - bool height_set_by_client{false}; ///< If the client gave a width on the last .set_size() request /// This is the size known to the client. It's pending value is set either by the .set_size() request(), or when /// the client acks a configure. - DoubleBuffered client_size; + DoubleBuffered client_size; DoubleBuffered offset; bool configure_on_next_commit{false}; ///< If to send a .configure event at the end of the next or current commit MirFocusMode current_focus_mode{mir_focus_mode_disabled}; @@ -417,8 +415,12 @@ auto mf::LayerSurfaceV1::unpadded_requested_size() const -> geom::Size auto mf::LayerSurfaceV1::inform_window_role_of_pending_placement() { - set_pending_width(client_size.pending().width + horiz_padding(anchors.pending(), margin.pending())); - set_pending_height(client_size.pending().height + vert_padding(anchors.pending(), margin.pending())); + set_pending_width(client_size.pending().width + ? client_size.pending().width.value() + horiz_padding(anchors.pending(), margin.pending()) + : client_size.pending().width); + set_pending_height(client_size.pending().height + ? client_size.pending().height.value() + vert_padding(anchors.pending(), margin.pending()) + : client_size.pending().height); offset.set_pending({ anchors.pending().left ? margin.pending().left : geom::DeltaX{}, @@ -462,12 +464,12 @@ void mf::LayerSurfaceV1::configure() configure_size.height = requested.height; } - if (width_set_by_client) + if (client_size.committed().width) { configure_size.width = client_size.committed().width; } - if (height_set_by_client) + if (client_size.committed().height) { configure_size.height = client_size.committed().height; } @@ -485,17 +487,25 @@ void mf::LayerSurfaceV1::configure() void mf::LayerSurfaceV1::set_size(uint32_t width, uint32_t height) { - width_set_by_client = width > 0; - height_set_by_client = height > 0; auto pending = client_size.pending(); if (width > 0) { pending.width = geom::Width{width}; } + else + { + pending.width = std::nullopt; + } + if (height > 0) { pending.height = geom::Height{height}; } + else + { + pending.height = std::nullopt; + } + client_size.set_pending(pending); inform_window_role_of_pending_placement(); configure_on_next_commit = true; @@ -606,9 +616,13 @@ void mf::LayerSurfaceV1::ack_configure(uint32_t serial) return; } - client_size.set_pending(geom::Size{ - acked_configure_size.width.value_or(client_size.pending().width), - acked_configure_size.height.value_or(client_size.pending().height)}); + auto const& width = acked_configure_size.width + ? acked_configure_size.width + : client_size.pending().width; + auto const& height = acked_configure_size.height + ? acked_configure_size.height + : client_size.pending().height; + client_size.set_pending(OptionalSize{width, height}); // Do NOT set configure_on_next_commit (as we do in the other case when opt_size is set) // We don't want to make the client acking one configure result in us sending another @@ -648,14 +662,14 @@ void mf::LayerSurfaceV1::handle_commit() // "You must set your anchor to opposite edges in the dimensions you omit; not doing so is a protocol error." bool const horiz_stretched = anchors.committed().left && anchors.committed().right; bool const vert_stretched = anchors.committed().top && anchors.committed().bottom; - if (!horiz_stretched && !width_set_by_client) + if (!horiz_stretched && !client_size.committed().width) { BOOST_THROW_EXCEPTION(mw::ProtocolError( resource, Error::invalid_size, "Width may be unspecified only when surface is anchored to left and right edges")); } - if (!vert_stretched && !height_set_by_client) + if (!vert_stretched && !client_size.committed().height) { BOOST_THROW_EXCEPTION(mw::ProtocolError( resource, From bb8b9d458e23570c955968a8da8975fdf2c8e246 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Mon, 25 Nov 2024 16:36:41 +0000 Subject: [PATCH 4/4] Don't conflate the client explicitly setting the size with there being a committed size --- src/server/frontend_wayland/layer_shell_v1.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/server/frontend_wayland/layer_shell_v1.cpp b/src/server/frontend_wayland/layer_shell_v1.cpp index 01fdc96a6c8..7b901c560ea 100644 --- a/src/server/frontend_wayland/layer_shell_v1.cpp +++ b/src/server/frontend_wayland/layer_shell_v1.cpp @@ -199,6 +199,8 @@ class LayerSurfaceV1 : public wayland::LayerSurfaceV1, public WindowWlSurfaceRol DoubleBuffered exclusive_zone{0}; DoubleBuffered anchors; DoubleBuffered margin; + bool width_set_by_client{false}; ///< If the client gave a width on the last .set_size() request + bool height_set_by_client{false}; ///< If the client gave a width on the last .set_size() request /// This is the size known to the client. It's pending value is set either by the .set_size() request(), or when /// the client acks a configure. DoubleBuffered client_size; @@ -464,12 +466,12 @@ void mf::LayerSurfaceV1::configure() configure_size.height = requested.height; } - if (client_size.committed().width) + if (width_set_by_client) { configure_size.width = client_size.committed().width; } - if (client_size.committed().height) + if (height_set_by_client) { configure_size.height = client_size.committed().height; } @@ -487,6 +489,8 @@ void mf::LayerSurfaceV1::configure() void mf::LayerSurfaceV1::set_size(uint32_t width, uint32_t height) { + width_set_by_client = width > 0; + height_set_by_client = height > 0; auto pending = client_size.pending(); if (width > 0) { @@ -662,14 +666,14 @@ void mf::LayerSurfaceV1::handle_commit() // "You must set your anchor to opposite edges in the dimensions you omit; not doing so is a protocol error." bool const horiz_stretched = anchors.committed().left && anchors.committed().right; bool const vert_stretched = anchors.committed().top && anchors.committed().bottom; - if (!horiz_stretched && !client_size.committed().width) + if (!horiz_stretched && !width_set_by_client) { BOOST_THROW_EXCEPTION(mw::ProtocolError( resource, Error::invalid_size, "Width may be unspecified only when surface is anchored to left and right edges")); } - if (!vert_stretched && !client_size.committed().height) + if (!vert_stretched && !height_set_by_client) { BOOST_THROW_EXCEPTION(mw::ProtocolError( resource,