From e80daf5ffa960b79b1adc8884015aee8bd5904c8 Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Fri, 14 Jun 2024 12:08:48 +0300 Subject: [PATCH 01/81] Add xdg-decoration-unstable-v1 and modify CMakeLists.txt to generate its code --- src/wayland/CMakeLists.txt | 1 + .../xdg-decoration-unstable-v1.xml | 156 ++++++++++++++++++ 2 files changed, 157 insertions(+) create mode 100644 wayland-protocols/xdg-decoration-unstable-v1.xml diff --git a/src/wayland/CMakeLists.txt b/src/wayland/CMakeLists.txt index 0b41617a27d..99d4f9a2dc1 100644 --- a/src/wayland/CMakeLists.txt +++ b/src/wayland/CMakeLists.txt @@ -36,6 +36,7 @@ mir_generate_protocol_wrapper(mirwayland "z" wlr-screencopy-unstable-v1.xml) mir_generate_protocol_wrapper(mirwayland "zwlr_" wlr-virtual-pointer-unstable-v1.xml) mir_generate_protocol_wrapper(mirwayland "ext_" ext-session-lock-v1.xml) mir_generate_protocol_wrapper(mirwayland "zmir_" mir-shell-unstable-v1.xml) +mir_generate_protocol_wrapper(mirwayland "z" xdg-decoration-unstable-v1.xml) target_link_libraries(mirwayland PUBLIC diff --git a/wayland-protocols/xdg-decoration-unstable-v1.xml b/wayland-protocols/xdg-decoration-unstable-v1.xml new file mode 100644 index 00000000000..c175cadf7d7 --- /dev/null +++ b/wayland-protocols/xdg-decoration-unstable-v1.xml @@ -0,0 +1,156 @@ + + + + Copyright © 2018 Simon Ser + + Permission is hereby granted, free of charge, to any person obtaining a + copy of this software and associated documentation files (the "Software"), + to deal in the Software without restriction, including without limitation + the rights to use, copy, modify, merge, publish, distribute, sublicense, + and/or sell copies of the Software, and to permit persons to whom the + Software is furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice (including the next + paragraph) shall be included in all copies or substantial portions of the + Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + DEALINGS IN THE SOFTWARE. + + + + + This interface allows a compositor to announce support for server-side + decorations. + + A window decoration is a set of window controls as deemed appropriate by + the party managing them, such as user interface components used to move, + resize and change a window's state. + + A client can use this protocol to request being decorated by a supporting + compositor. + + If compositor and client do not negotiate the use of a server-side + decoration using this protocol, clients continue to self-decorate as they + see fit. + + Warning! The protocol described in this file is experimental and + backward incompatible changes may be made. Backward compatible changes + may be added together with the corresponding interface version bump. + Backward incompatible changes are done by bumping the version number in + the protocol and interface names and resetting the interface version. + Once the protocol is to be declared stable, the 'z' prefix and the + version number in the protocol and interface names are removed and the + interface version number is reset. + + + + + Destroy the decoration manager. This doesn't destroy objects created + with the manager. + + + + + + Create a new decoration object associated with the given toplevel. + + Creating an xdg_toplevel_decoration from an xdg_toplevel which has a + buffer attached or committed is a client error, and any attempts by a + client to attach or manipulate a buffer prior to the first + xdg_toplevel_decoration.configure event must also be treated as + errors. + + + + + + + + + The decoration object allows the compositor to toggle server-side window + decorations for a toplevel surface. The client can request to switch to + another mode. + + The xdg_toplevel_decoration object must be destroyed before its + xdg_toplevel. + + + + + + + + + + + Switch back to a mode without any server-side decorations at the next + commit. + + + + + + These values describe window decoration modes. + + + + + + + + Set the toplevel surface decoration mode. This informs the compositor + that the client prefers the provided decoration mode. + + After requesting a decoration mode, the compositor will respond by + emitting an xdg_surface.configure event. The client should then update + its content, drawing it without decorations if the received mode is + server-side decorations. The client must also acknowledge the configure + when committing the new content (see xdg_surface.ack_configure). + + The compositor can decide not to use the client's mode and enforce a + different mode instead. + + Clients whose decoration mode depend on the xdg_toplevel state may send + a set_mode request in response to an xdg_surface.configure event and wait + for the next xdg_surface.configure event to prevent unwanted state. + Such clients are responsible for preventing configure loops and must + make sure not to send multiple successive set_mode requests with the + same decoration mode. + + + + + + + Unset the toplevel surface decoration mode. This informs the compositor + that the client doesn't prefer a particular decoration mode. + + This request has the same semantics as set_mode. + + + + + + The configure event configures the effective decoration mode. The + configured state should not be applied immediately. Clients must send an + ack_configure in response to this event. See xdg_surface.configure and + xdg_surface.ack_configure for details. + + A configure event can be sent at any time. The specified mode must be + obeyed by the client. + + + + + From feca272fd08d3801c92fa75b1f28b8602436d01b Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Fri, 14 Jun 2024 12:09:46 +0300 Subject: [PATCH 02/81] Add first prototype of xdg-decoration-unstable-v1. Had to move `XdgTopLevelStable` to its header file so I can include it, and moved `from()` to be public. --- src/server/frontend_wayland/CMakeLists.txt | 1 + .../wayland_default_configuration.cpp | 6 + .../xdg_decoration_unstable_v1.cpp | 131 ++++++++++++++++++ .../xdg_decoration_unstable_v1.h | 32 +++++ .../frontend_wayland/xdg_shell_stable.cpp | 36 ----- .../frontend_wayland/xdg_shell_stable.h | 38 +++++ 6 files changed, 208 insertions(+), 36 deletions(-) create mode 100644 src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp create mode 100644 src/server/frontend_wayland/xdg_decoration_unstable_v1.h diff --git a/src/server/frontend_wayland/CMakeLists.txt b/src/server/frontend_wayland/CMakeLists.txt index 9cb115a438f..a56b23a75ff 100644 --- a/src/server/frontend_wayland/CMakeLists.txt +++ b/src/server/frontend_wayland/CMakeLists.txt @@ -54,6 +54,7 @@ set( text_input_v1.cpp text_input_v1.h primary_selection_v1.cpp primary_selection_v1.h session_lock_v1.cpp session_lock_v1.h + xdg_decoration_unstable_v1.cpp xdg_decoration_unstable_v1.h ${PROJECT_SOURCE_DIR}/src/include/server/mir/frontend/wayland.h ${CMAKE_CURRENT_BINARY_DIR}/wayland_frontend.tp.c ${CMAKE_CURRENT_BINARY_DIR}/wayland_frontend.tp.h diff --git a/src/server/frontend_wayland/wayland_default_configuration.cpp b/src/server/frontend_wayland/wayland_default_configuration.cpp index 553f8f3d253..8fb90cc6df6 100644 --- a/src/server/frontend_wayland/wayland_default_configuration.cpp +++ b/src/server/frontend_wayland/wayland_default_configuration.cpp @@ -41,6 +41,8 @@ #include "wl_seat.h" #include "wl_shell.h" #include "wlr_screencopy_v1.h" +#include "xdg-decoration-unstable-v1_wrapper.h" +#include "xdg_decoration_unstable_v1.h" #include "xdg_output_v1.h" #include "xdg_shell_stable.h" #include "xdg_shell_v6.h" @@ -214,6 +216,10 @@ std::vector const internal_extension_builders = { { return mf::create_mir_shell_v1(ctx.display); }), + make_extension_builder([](auto const& ctx) + { + return mf::create_xdg_decoration_unstable_v1(ctx.display); + }) }; ExtensionBuilder const xwayland_builder { diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp new file mode 100644 index 00000000000..1e1daaea9b4 --- /dev/null +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -0,0 +1,131 @@ +#include "xdg_decoration_unstable_v1.h" + +#include "mir/shell/surface_specification.h" +#include "mir/log.h" + +#include "xdg-decoration-unstable-v1_wrapper.h" +#include "xdg-shell_wrapper.h" +#include "xdg_output_v1.h" +#include "xdg_shell_stable.h" + +#include + +namespace mir { +namespace frontend { +class XdgDecorationManagerV1 : public wayland::XdgDecorationManagerV1 +{ +public: + XdgDecorationManagerV1(wl_resource *resource); + + class Global : public wayland::XdgDecorationManagerV1::Global + { + public: + Global(wl_display* display); + + private: + void bind(wl_resource* new_zxdg_decoration_manager_v1) override; + }; + +private: + void get_toplevel_decoration(wl_resource* id, wl_resource* toplevel) override; +}; + +class XdgToplevelDecorationV1 : public wayland::XdgToplevelDecorationV1 +{ + public: + XdgToplevelDecorationV1(wl_resource* id, mir::frontend::XdgToplevelStable* toplevel); + + void set_mode(uint32_t mode) override; + void unset_mode() override; + void configure(); + + private: + void update_mode(std::optional new_mode); + + mir::frontend::XdgToplevelStable* toplevel; + std::optional mode; +}; +} // namespace frontend +} // namespace mir + +auto mir::frontend::create_xdg_decoration_unstable_v1(wl_display* display) +-> std::shared_ptr +{ + log_info("Creating XdgDecorationManagerV1::Global"); + return std::make_shared(display); +} + +mir::frontend::XdgDecorationManagerV1::Global::Global(wl_display *display) + : wayland::XdgDecorationManagerV1::Global::Global{display, Version<1>{}} +{ +} + +void mir::frontend::XdgDecorationManagerV1::Global::bind(wl_resource* new_zxdg_decoration_manager_v1) +{ + log_info(__PRETTY_FUNCTION__); + new XdgDecorationManagerV1{new_zxdg_decoration_manager_v1}; +} + +mir::frontend::XdgDecorationManagerV1::XdgDecorationManagerV1(wl_resource *resource) + : mir::wayland::XdgDecorationManagerV1{resource, Version<1>{}} +{ +} + +void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* id, wl_resource* toplevel) +{ + log_info(__PRETTY_FUNCTION__); + mir::frontend::XdgToplevelStable * tl = mir::frontend::XdgToplevelStable::from(toplevel); + if(tl) { + new XdgToplevelDecorationV1{id, tl}; + } +} + +mir::frontend::XdgToplevelDecorationV1::XdgToplevelDecorationV1(wl_resource* id, mir::frontend::XdgToplevelStable* toplevel) + : wayland::XdgToplevelDecorationV1{id, Version<1>{}}, toplevel{toplevel} +{ +} + +void mir::frontend::XdgToplevelDecorationV1::update_mode(std::optional new_mode) +{ + this->mode = new_mode; + auto spec = shell::SurfaceSpecification{}; + + if(this->mode) { + switch(*this->mode) { + case Mode::client_side: + spec.server_side_decorated = false; + break; + case Mode::server_side: + spec.server_side_decorated = true; + break; + } + } else { + // Client side by default + spec.server_side_decorated = false; + } + + this->toplevel->apply_spec(spec); +} + +void mir::frontend::XdgToplevelDecorationV1::set_mode(uint32_t mode) +{ + log_info(__PRETTY_FUNCTION__); + update_mode(mode); +} + +void mir::frontend::XdgToplevelDecorationV1::unset_mode() +{ + log_info(__PRETTY_FUNCTION__); + update_mode(Mode::server_side); +} + +void mir::frontend::XdgToplevelDecorationV1::configure() +{ + log_info(__PRETTY_FUNCTION__); + if(!mode) { + send_configure_event(Mode::client_side); + } + + send_configure_event(*mode); +} + diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.h b/src/server/frontend_wayland/xdg_decoration_unstable_v1.h new file mode 100644 index 00000000000..ed250d09683 --- /dev/null +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.h @@ -0,0 +1,32 @@ +/* + * Copyright © Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 or 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef MIR_FRONTEND_XDG_DECORATION_UNSTABLE_V1_H +#define MIR_FRONTEND_XDG_DECORATION_UNSTABLE_V1_H + +#include "xdg-decoration-unstable-v1_wrapper.h" + +namespace mir +{ +namespace frontend +{ +auto create_xdg_decoration_unstable_v1( + wl_display* display) +-> std::shared_ptr; +} +} + +#endif // MIR_FRONTEND_RELATIVE_POINTER_UNSTABLE_V1_H diff --git a/src/server/frontend_wayland/xdg_shell_stable.cpp b/src/server/frontend_wayland/xdg_shell_stable.cpp index db33552b161..11960cc2306 100644 --- a/src/server/frontend_wayland/xdg_shell_stable.cpp +++ b/src/server/frontend_wayland/xdg_shell_stable.cpp @@ -73,42 +73,6 @@ class XdgSurfaceStable : public mw::XdgSurface XdgShellStable const& xdg_shell; }; -class XdgToplevelStable : mw::XdgToplevel, public WindowWlSurfaceRole -{ -public: - XdgToplevelStable( - wl_resource* new_resource, - XdgSurfaceStable* xdg_surface, - WlSurface* surface); - - void set_parent(std::optional const& parent) override; - void set_title(std::string const& title) override; - void set_app_id(std::string const& app_id) override; - void show_window_menu(struct wl_resource* seat, uint32_t serial, int32_t x, int32_t y) override; - void move(struct wl_resource* seat, uint32_t serial) override; - void resize(struct wl_resource* seat, uint32_t serial, uint32_t edges) override; - void set_max_size(int32_t width, int32_t height) override; - void set_min_size(int32_t width, int32_t height) override; - void set_maximized() override; - void unset_maximized() override; - void set_fullscreen(std::optional const& output) override; - void unset_fullscreen() override; - void set_minimized() override; - - void handle_commit() override {}; - void handle_state_change(MirWindowState /*new_state*/) override; - void handle_active_change(bool /*is_now_active*/) override; - void handle_resize(std::optional const& new_top_left, geometry::Size const& new_size) override; - void handle_close_request() override; - -private: - static XdgToplevelStable* from(wl_resource* surface); - void send_toplevel_configure(); - void destroy_role() const override; - - mw::Weak const xdg_surface; -}; - class XdgPositionerStable : public mw::XdgPositioner, public shell::SurfaceSpecification { public: diff --git a/src/server/frontend_wayland/xdg_shell_stable.h b/src/server/frontend_wayland/xdg_shell_stable.h index a2f606d2a2d..be8da39ee92 100644 --- a/src/server/frontend_wayland/xdg_shell_stable.h +++ b/src/server/frontend_wayland/xdg_shell_stable.h @@ -20,6 +20,8 @@ #include "xdg-shell_wrapper.h" #include "window_wl_surface_role.h" +namespace mw = mir::wayland; + namespace mir { namespace scene @@ -101,6 +103,42 @@ class XdgPopupStable : public wayland::XdgPopup, public WindowWlSurfaceRole auto popup_rect() const -> std::optional; }; +class XdgToplevelStable : mw::XdgToplevel, public WindowWlSurfaceRole +{ +public: + XdgToplevelStable( + wl_resource* new_resource, + XdgSurfaceStable* xdg_surface, + WlSurface* surface); + + void set_parent(std::optional const& parent) override; + void set_title(std::string const& title) override; + void set_app_id(std::string const& app_id) override; + void show_window_menu(struct wl_resource* seat, uint32_t serial, int32_t x, int32_t y) override; + void move(struct wl_resource* seat, uint32_t serial) override; + void resize(struct wl_resource* seat, uint32_t serial, uint32_t edges) override; + void set_max_size(int32_t width, int32_t height) override; + void set_min_size(int32_t width, int32_t height) override; + void set_maximized() override; + void unset_maximized() override; + void set_fullscreen(std::optional const& output) override; + void unset_fullscreen() override; + void set_minimized() override; + + void handle_commit() override {}; + void handle_state_change(MirWindowState /*new_state*/) override; + void handle_active_change(bool /*is_now_active*/) override; + void handle_resize(std::optional const& new_top_left, geometry::Size const& new_size) override; + void handle_close_request() override; + + static XdgToplevelStable* from(wl_resource* surface); + +private: + void send_toplevel_configure(); + void destroy_role() const override; + + mw::Weak const xdg_surface; +}; } } From cc04d24c25d3415d0cad9eb5f379ff4ec32ac84d Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Fri, 14 Jun 2024 14:46:32 +0300 Subject: [PATCH 03/81] Apply formatting --- .../xdg_decoration_unstable_v1.cpp | 109 ++++++++++-------- 1 file changed, 58 insertions(+), 51 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 1e1daaea9b4..a61cceda752 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -1,68 +1,69 @@ #include "xdg_decoration_unstable_v1.h" -#include "mir/shell/surface_specification.h" #include "mir/log.h" +#include "mir/shell/surface_specification.h" #include "xdg-decoration-unstable-v1_wrapper.h" -#include "xdg-shell_wrapper.h" #include "xdg_output_v1.h" #include "xdg_shell_stable.h" #include -namespace mir { -namespace frontend { -class XdgDecorationManagerV1 : public wayland::XdgDecorationManagerV1 +namespace mir +{ +namespace frontend +{ +class XdgDecorationManagerV1 : public wayland::XdgDecorationManagerV1 { public: XdgDecorationManagerV1(wl_resource *resource); - class Global : public wayland::XdgDecorationManagerV1::Global + class Global : public wayland::XdgDecorationManagerV1::Global { - public: - Global(wl_display* display); + public: + Global(wl_display *display); - private: - void bind(wl_resource* new_zxdg_decoration_manager_v1) override; + private: + void bind(wl_resource *new_zxdg_decoration_manager_v1) override; }; private: - void get_toplevel_decoration(wl_resource* id, wl_resource* toplevel) override; + void get_toplevel_decoration(wl_resource *id, wl_resource *toplevel) override; }; class XdgToplevelDecorationV1 : public wayland::XdgToplevelDecorationV1 { - public: - XdgToplevelDecorationV1(wl_resource* id, mir::frontend::XdgToplevelStable* toplevel); +public: + XdgToplevelDecorationV1(wl_resource *id, mir::frontend::XdgToplevelStable *toplevel); - void set_mode(uint32_t mode) override; - void unset_mode() override; - void configure(); + void set_mode(uint32_t mode) override; + void unset_mode() override; + void configure(); - private: - void update_mode(std::optional new_mode); +private: + void update_mode(std::optional new_mode); - mir::frontend::XdgToplevelStable* toplevel; - std::optional mode; + mir::frontend::XdgToplevelStable *toplevel; + std::optional mode; }; } // namespace frontend } // namespace mir - -auto mir::frontend::create_xdg_decoration_unstable_v1(wl_display* display) --> std::shared_ptr + +auto mir::frontend::create_xdg_decoration_unstable_v1(wl_display *display) + -> std::shared_ptr { - log_info("Creating XdgDecorationManagerV1::Global"); + log_info("Creating XdgDecorationManagerV1::Global"); return std::make_shared(display); } -mir::frontend::XdgDecorationManagerV1::Global::Global(wl_display *display) +mir::frontend::XdgDecorationManagerV1::Global::Global(wl_display *display) : wayland::XdgDecorationManagerV1::Global::Global{display, Version<1>{}} { } -void mir::frontend::XdgDecorationManagerV1::Global::bind(wl_resource* new_zxdg_decoration_manager_v1) +void mir::frontend::XdgDecorationManagerV1::Global::bind(wl_resource *new_zxdg_decoration_manager_v1) { - log_info(__PRETTY_FUNCTION__); + log_info(__PRETTY_FUNCTION__); new XdgDecorationManagerV1{new_zxdg_decoration_manager_v1}; } @@ -71,61 +72,67 @@ mir::frontend::XdgDecorationManagerV1::XdgDecorationManagerV1(wl_resource *resou { } -void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* id, wl_resource* toplevel) +void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource *id, wl_resource *toplevel) { - log_info(__PRETTY_FUNCTION__); - mir::frontend::XdgToplevelStable * tl = mir::frontend::XdgToplevelStable::from(toplevel); - if(tl) { + log_info(__PRETTY_FUNCTION__); + mir::frontend::XdgToplevelStable *tl = mir::frontend::XdgToplevelStable::from(toplevel); + if (tl) + { new XdgToplevelDecorationV1{id, tl}; } } -mir::frontend::XdgToplevelDecorationV1::XdgToplevelDecorationV1(wl_resource* id, mir::frontend::XdgToplevelStable* toplevel) +mir::frontend::XdgToplevelDecorationV1::XdgToplevelDecorationV1(wl_resource *id, + mir::frontend::XdgToplevelStable *toplevel) : wayland::XdgToplevelDecorationV1{id, Version<1>{}}, toplevel{toplevel} { } -void mir::frontend::XdgToplevelDecorationV1::update_mode(std::optional new_mode) +void mir::frontend::XdgToplevelDecorationV1::update_mode(std::optional new_mode) { this->mode = new_mode; auto spec = shell::SurfaceSpecification{}; - if(this->mode) { - switch(*this->mode) { - case Mode::client_side: - spec.server_side_decorated = false; - break; - case Mode::server_side: - spec.server_side_decorated = true; - break; + if (this->mode) + { + switch (*this->mode) + { + case Mode::client_side: + spec.server_side_decorated = false; + break; + case Mode::server_side: + spec.server_side_decorated = true; + break; } - } else { + } + else + { // Client side by default - spec.server_side_decorated = false; + spec.server_side_decorated = false; } this->toplevel->apply_spec(spec); } -void mir::frontend::XdgToplevelDecorationV1::set_mode(uint32_t mode) +void mir::frontend::XdgToplevelDecorationV1::set_mode(uint32_t mode) { - log_info(__PRETTY_FUNCTION__); + log_info(__PRETTY_FUNCTION__); update_mode(mode); } -void mir::frontend::XdgToplevelDecorationV1::unset_mode() +void mir::frontend::XdgToplevelDecorationV1::unset_mode() { - log_info(__PRETTY_FUNCTION__); + log_info(__PRETTY_FUNCTION__); update_mode(Mode::server_side); } -void mir::frontend::XdgToplevelDecorationV1::configure() +void mir::frontend::XdgToplevelDecorationV1::configure() { - log_info(__PRETTY_FUNCTION__); - if(!mode) { + log_info(__PRETTY_FUNCTION__); + if (!mode) + { send_configure_event(Mode::client_side); } send_configure_event(*mode); } - From 5b8027c5177113acb63b6213b17fb7371a674db4 Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Fri, 14 Jun 2024 19:43:29 +0300 Subject: [PATCH 04/81] Add missing configure event call. Removed superfluous `configure` method, as well as unwrapping `mode` to a normal uint32_t. --- .../xdg_decoration_unstable_v1.cpp | 51 +++++++------------ 1 file changed, 17 insertions(+), 34 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index a61cceda752..475c23aafee 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -7,8 +7,6 @@ #include "xdg_output_v1.h" #include "xdg_shell_stable.h" -#include - namespace mir { namespace frontend @@ -38,13 +36,14 @@ class XdgToplevelDecorationV1 : public wayland::XdgToplevelDecorationV1 void set_mode(uint32_t mode) override; void unset_mode() override; - void configure(); private: - void update_mode(std::optional new_mode); + void update_mode(uint32_t new_mode); + + static const uint32_t default_mode = Mode::client_side; - mir::frontend::XdgToplevelStable *toplevel; - std::optional mode; + mir::frontend::XdgToplevelStable* toplevel; + uint32_t mode; }; } // namespace frontend } // namespace mir @@ -88,30 +87,25 @@ mir::frontend::XdgToplevelDecorationV1::XdgToplevelDecorationV1(wl_resource *id, { } -void mir::frontend::XdgToplevelDecorationV1::update_mode(std::optional new_mode) +void mir::frontend::XdgToplevelDecorationV1::update_mode(uint32_t new_mode) { - this->mode = new_mode; + log_info("Updating mode to: %s", new_mode == Mode::client_side ? "client-side" : "server-side"); + auto spec = shell::SurfaceSpecification{}; - if (this->mode) + mode = new_mode; + switch (mode) { - switch (*this->mode) - { - case Mode::client_side: - spec.server_side_decorated = false; - break; - case Mode::server_side: - spec.server_side_decorated = true; - break; - } - } - else - { - // Client side by default + case Mode::client_side: spec.server_side_decorated = false; + break; + case Mode::server_side: + spec.server_side_decorated = true; + break; } this->toplevel->apply_spec(spec); + send_configure_event(mode); } void mir::frontend::XdgToplevelDecorationV1::set_mode(uint32_t mode) @@ -123,16 +117,5 @@ void mir::frontend::XdgToplevelDecorationV1::set_mode(uint32_t mode) void mir::frontend::XdgToplevelDecorationV1::unset_mode() { log_info(__PRETTY_FUNCTION__); - update_mode(Mode::server_side); -} - -void mir::frontend::XdgToplevelDecorationV1::configure() -{ - log_info(__PRETTY_FUNCTION__); - if (!mode) - { - send_configure_event(Mode::client_side); - } - - send_configure_event(*mode); + update_mode(default_mode); } From 68aad57971843976f427b8db1c5dcd530f18621e Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Fri, 14 Jun 2024 19:48:04 +0300 Subject: [PATCH 05/81] Apply formatting. --- .../xdg_decoration_unstable_v1.cpp | 26 +++++++++---------- .../xdg_decoration_unstable_v1.h | 8 +++--- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 475c23aafee..2a84e6d0b9d 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -14,25 +14,25 @@ namespace frontend class XdgDecorationManagerV1 : public wayland::XdgDecorationManagerV1 { public: - XdgDecorationManagerV1(wl_resource *resource); + XdgDecorationManagerV1(wl_resource* resource); class Global : public wayland::XdgDecorationManagerV1::Global { public: - Global(wl_display *display); + Global(wl_display* display); private: - void bind(wl_resource *new_zxdg_decoration_manager_v1) override; + void bind(wl_resource* new_zxdg_decoration_manager_v1) override; }; private: - void get_toplevel_decoration(wl_resource *id, wl_resource *toplevel) override; + void get_toplevel_decoration(wl_resource* id, wl_resource* toplevel) override; }; class XdgToplevelDecorationV1 : public wayland::XdgToplevelDecorationV1 { public: - XdgToplevelDecorationV1(wl_resource *id, mir::frontend::XdgToplevelStable *toplevel); + XdgToplevelDecorationV1(wl_resource* id, mir::frontend::XdgToplevelStable* toplevel); void set_mode(uint32_t mode) override; void unset_mode() override; @@ -48,41 +48,41 @@ class XdgToplevelDecorationV1 : public wayland::XdgToplevelDecorationV1 } // namespace frontend } // namespace mir -auto mir::frontend::create_xdg_decoration_unstable_v1(wl_display *display) +auto mir::frontend::create_xdg_decoration_unstable_v1(wl_display* display) -> std::shared_ptr { log_info("Creating XdgDecorationManagerV1::Global"); return std::make_shared(display); } -mir::frontend::XdgDecorationManagerV1::Global::Global(wl_display *display) +mir::frontend::XdgDecorationManagerV1::Global::Global(wl_display* display) : wayland::XdgDecorationManagerV1::Global::Global{display, Version<1>{}} { } -void mir::frontend::XdgDecorationManagerV1::Global::bind(wl_resource *new_zxdg_decoration_manager_v1) +void mir::frontend::XdgDecorationManagerV1::Global::bind(wl_resource* new_zxdg_decoration_manager_v1) { log_info(__PRETTY_FUNCTION__); new XdgDecorationManagerV1{new_zxdg_decoration_manager_v1}; } -mir::frontend::XdgDecorationManagerV1::XdgDecorationManagerV1(wl_resource *resource) +mir::frontend::XdgDecorationManagerV1::XdgDecorationManagerV1(wl_resource* resource) : mir::wayland::XdgDecorationManagerV1{resource, Version<1>{}} { } -void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource *id, wl_resource *toplevel) +void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* id, wl_resource* toplevel) { log_info(__PRETTY_FUNCTION__); - mir::frontend::XdgToplevelStable *tl = mir::frontend::XdgToplevelStable::from(toplevel); + auto* tl = mir::frontend::XdgToplevelStable::from(toplevel); if (tl) { new XdgToplevelDecorationV1{id, tl}; } } -mir::frontend::XdgToplevelDecorationV1::XdgToplevelDecorationV1(wl_resource *id, - mir::frontend::XdgToplevelStable *toplevel) +mir::frontend::XdgToplevelDecorationV1::XdgToplevelDecorationV1(wl_resource* id, + mir::frontend::XdgToplevelStable* toplevel) : wayland::XdgToplevelDecorationV1{id, Version<1>{}}, toplevel{toplevel} { } diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.h b/src/server/frontend_wayland/xdg_decoration_unstable_v1.h index ed250d09683..b368cbe2372 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.h +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.h @@ -23,10 +23,8 @@ namespace mir { namespace frontend { -auto create_xdg_decoration_unstable_v1( - wl_display* display) --> std::shared_ptr; -} +auto create_xdg_decoration_unstable_v1(wl_display* display) -> std::shared_ptr; } +} // namespace mir -#endif // MIR_FRONTEND_RELATIVE_POINTER_UNSTABLE_V1_H +#endif // MIR_FRONTEND_RELATIVE_POINTER_UNSTABLE_V1_H From 59c8c08bf8c1bb928a416531dca8cce37e597e7b Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Fri, 14 Jun 2024 19:49:49 +0300 Subject: [PATCH 06/81] Remove `mir::wayland` alias in header. --- src/server/frontend_wayland/xdg_shell_stable.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/server/frontend_wayland/xdg_shell_stable.h b/src/server/frontend_wayland/xdg_shell_stable.h index be8da39ee92..c45a9ea3850 100644 --- a/src/server/frontend_wayland/xdg_shell_stable.h +++ b/src/server/frontend_wayland/xdg_shell_stable.h @@ -20,8 +20,6 @@ #include "xdg-shell_wrapper.h" #include "window_wl_surface_role.h" -namespace mw = mir::wayland; - namespace mir { namespace scene @@ -103,7 +101,7 @@ class XdgPopupStable : public wayland::XdgPopup, public WindowWlSurfaceRole auto popup_rect() const -> std::optional; }; -class XdgToplevelStable : mw::XdgToplevel, public WindowWlSurfaceRole +class XdgToplevelStable : mir::wayland::XdgToplevel, public WindowWlSurfaceRole { public: XdgToplevelStable( @@ -137,7 +135,7 @@ class XdgToplevelStable : mw::XdgToplevel, public WindowWlSurfaceRole void send_toplevel_configure(); void destroy_role() const override; - mw::Weak const xdg_surface; + mir::wayland::Weak const xdg_surface; }; } } From 826096a8628a13ce8aa4504caca2dae79da8e0d2 Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Fri, 14 Jun 2024 19:54:33 +0300 Subject: [PATCH 07/81] Add missing copyright notice to xdg_decoration_unstable_v1.cpp --- .../xdg_decoration_unstable_v1.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 2a84e6d0b9d..15f1c4395a2 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -1,3 +1,19 @@ +/* + * Copyright © Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 or 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + #include "xdg_decoration_unstable_v1.h" #include "mir/log.h" From 7e759766253c32739c3502dadc88947744f45ca7 Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Fri, 14 Jun 2024 20:02:16 +0300 Subject: [PATCH 08/81] Strip logs from xdg_decoration_unstable_v1.cpp --- .../frontend_wayland/xdg_decoration_unstable_v1.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 15f1c4395a2..19b0947d9f6 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -16,7 +16,6 @@ #include "xdg_decoration_unstable_v1.h" -#include "mir/log.h" #include "mir/shell/surface_specification.h" #include "xdg-decoration-unstable-v1_wrapper.h" @@ -67,7 +66,6 @@ class XdgToplevelDecorationV1 : public wayland::XdgToplevelDecorationV1 auto mir::frontend::create_xdg_decoration_unstable_v1(wl_display* display) -> std::shared_ptr { - log_info("Creating XdgDecorationManagerV1::Global"); return std::make_shared(display); } @@ -78,7 +76,6 @@ mir::frontend::XdgDecorationManagerV1::Global::Global(wl_display* display) void mir::frontend::XdgDecorationManagerV1::Global::bind(wl_resource* new_zxdg_decoration_manager_v1) { - log_info(__PRETTY_FUNCTION__); new XdgDecorationManagerV1{new_zxdg_decoration_manager_v1}; } @@ -89,7 +86,6 @@ mir::frontend::XdgDecorationManagerV1::XdgDecorationManagerV1(wl_resource* resou void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* id, wl_resource* toplevel) { - log_info(__PRETTY_FUNCTION__); auto* tl = mir::frontend::XdgToplevelStable::from(toplevel); if (tl) { @@ -105,8 +101,6 @@ mir::frontend::XdgToplevelDecorationV1::XdgToplevelDecorationV1(wl_resource* id, void mir::frontend::XdgToplevelDecorationV1::update_mode(uint32_t new_mode) { - log_info("Updating mode to: %s", new_mode == Mode::client_side ? "client-side" : "server-side"); - auto spec = shell::SurfaceSpecification{}; mode = new_mode; @@ -126,12 +120,10 @@ void mir::frontend::XdgToplevelDecorationV1::update_mode(uint32_t new_mode) void mir::frontend::XdgToplevelDecorationV1::set_mode(uint32_t mode) { - log_info(__PRETTY_FUNCTION__); update_mode(mode); } void mir::frontend::XdgToplevelDecorationV1::unset_mode() { - log_info(__PRETTY_FUNCTION__); update_mode(default_mode); } From b29803bc78d8a740cd3249aa803c07412b56421d Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Thu, 20 Jun 2024 11:39:31 +0100 Subject: [PATCH 09/81] Notify display configuration changes when an output is added --- src/platforms/wayland/displayclient.cpp | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/platforms/wayland/displayclient.cpp b/src/platforms/wayland/displayclient.cpp index 9f4f13bce48..108a481ba20 100644 --- a/src/platforms/wayland/displayclient.cpp +++ b/src/platforms/wayland/displayclient.cpp @@ -305,20 +305,14 @@ void mgw::DisplayClient::Output::surface_configure(uint32_t serial) dcout.custom_logical_size = pending_toplevel_size.value(); pending_toplevel_size.reset(); output_size = dcout.extents().size; - if (!has_initialized) + if (!has_initialized || size_is_changed) { has_initialized = true; provider = std::make_shared( owner_->provider->get_egl_display(), surface, output_size); - } - else if (size_is_changed) - { - /* TODO: We should, again, handle this by storing the pending size, raising a hardware-changed - * notification, and then letting the `configure()` system tear down everything and bring it back - * up at the new size. - */ + owner_->on_display_config_changed(); } } } From 0470c538ed46983fa6849367bcb44288837dbb53 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Thu, 20 Jun 2024 17:31:25 +0100 Subject: [PATCH 10/81] Drop dead code --- src/platforms/wayland/wl_egl_display_provider.h | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/platforms/wayland/wl_egl_display_provider.h b/src/platforms/wayland/wl_egl_display_provider.h index a3c92aaddd6..45c6a72da49 100644 --- a/src/platforms/wayland/wl_egl_display_provider.h +++ b/src/platforms/wayland/wl_egl_display_provider.h @@ -31,11 +31,6 @@ class WlDisplayProvider : public GenericEGLDisplayProvider public: WlDisplayProvider(EGLDisplay dpy); - WlDisplayProvider( - WlDisplayProvider const& from, - struct wl_surface* surface, - geometry::Size size); - auto get_egl_display() -> EGLDisplay override; private: EGLDisplay const dpy; From ba4db7c665e88c605786959edcc2bcf3cce86679 Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Mon, 24 Jun 2024 15:36:37 +0300 Subject: [PATCH 11/81] Add basic error handling for xdg decoration --- .../xdg_decoration_unstable_v1.cpp | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 19b0947d9f6..7069135f158 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -17,11 +17,14 @@ #include "xdg_decoration_unstable_v1.h" #include "mir/shell/surface_specification.h" +#include "mir/wayland/protocol_error.h" #include "xdg-decoration-unstable-v1_wrapper.h" #include "xdg_output_v1.h" #include "xdg_shell_stable.h" +#include + namespace mir { namespace frontend @@ -42,6 +45,7 @@ class XdgDecorationManagerV1 : public wayland::XdgDecorationManagerV1 private: void get_toplevel_decoration(wl_resource* id, wl_resource* toplevel) override; + std::unordered_set toplevels_with_decorations; }; class XdgToplevelDecorationV1 : public wayland::XdgToplevelDecorationV1 @@ -86,11 +90,24 @@ mir::frontend::XdgDecorationManagerV1::XdgDecorationManagerV1(wl_resource* resou void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* id, wl_resource* toplevel) { + using Error = mir::frontend::XdgToplevelDecorationV1::Error; + + if (toplevels_with_decorations.contains(toplevel)) + { + BOOST_THROW_EXCEPTION(mir::wayland::ProtocolError(resource, Error::already_constructed, + "Decoration already constructed for this toplevel")); + } + auto* tl = mir::frontend::XdgToplevelStable::from(toplevel); - if (tl) + if (!tl) { - new XdgToplevelDecorationV1{id, tl}; + BOOST_THROW_EXCEPTION( + mir::wayland::ProtocolError(resource, Error::orphaned, "Toplevel destroyed before attached decoration")); } + + + new XdgToplevelDecorationV1{id, tl}; + toplevels_with_decorations.insert(toplevel); } mir::frontend::XdgToplevelDecorationV1::XdgToplevelDecorationV1(wl_resource* id, From 9d8905267136fe41861dbf267614ffa735077d30 Mon Sep 17 00:00:00 2001 From: Tarek Yasser Date: Mon, 24 Jun 2024 19:04:07 +0300 Subject: [PATCH 12/81] Apply formatting to xdg_decoration_unstable_v1.cpp --- .../xdg_decoration_unstable_v1.cpp | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 7069135f158..93b480e4803 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -59,7 +59,7 @@ class XdgToplevelDecorationV1 : public wayland::XdgToplevelDecorationV1 private: void update_mode(uint32_t new_mode); - static const uint32_t default_mode = Mode::client_side; + static uint32_t const default_mode = Mode::client_side; mir::frontend::XdgToplevelStable* toplevel; uint32_t mode; @@ -73,8 +73,8 @@ auto mir::frontend::create_xdg_decoration_unstable_v1(wl_display* display) return std::make_shared(display); } -mir::frontend::XdgDecorationManagerV1::Global::Global(wl_display* display) - : wayland::XdgDecorationManagerV1::Global::Global{display, Version<1>{}} +mir::frontend::XdgDecorationManagerV1::Global::Global(wl_display* display) : + wayland::XdgDecorationManagerV1::Global::Global{display, Version<1>{}} { } @@ -83,8 +83,8 @@ void mir::frontend::XdgDecorationManagerV1::Global::bind(wl_resource* new_zxdg_d new XdgDecorationManagerV1{new_zxdg_decoration_manager_v1}; } -mir::frontend::XdgDecorationManagerV1::XdgDecorationManagerV1(wl_resource* resource) - : mir::wayland::XdgDecorationManagerV1{resource, Version<1>{}} +mir::frontend::XdgDecorationManagerV1::XdgDecorationManagerV1(wl_resource* resource) : + mir::wayland::XdgDecorationManagerV1{resource, Version<1>{}} { } @@ -94,8 +94,8 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* if (toplevels_with_decorations.contains(toplevel)) { - BOOST_THROW_EXCEPTION(mir::wayland::ProtocolError(resource, Error::already_constructed, - "Decoration already constructed for this toplevel")); + BOOST_THROW_EXCEPTION(mir::wayland::ProtocolError( + resource, Error::already_constructed, "Decoration already constructed for this toplevel")); } auto* tl = mir::frontend::XdgToplevelStable::from(toplevel); @@ -110,9 +110,10 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* toplevels_with_decorations.insert(toplevel); } -mir::frontend::XdgToplevelDecorationV1::XdgToplevelDecorationV1(wl_resource* id, - mir::frontend::XdgToplevelStable* toplevel) - : wayland::XdgToplevelDecorationV1{id, Version<1>{}}, toplevel{toplevel} +mir::frontend::XdgToplevelDecorationV1::XdgToplevelDecorationV1( + wl_resource* id, mir::frontend::XdgToplevelStable* toplevel) : + wayland::XdgToplevelDecorationV1{id, Version<1>{}}, + toplevel{toplevel} { } @@ -135,12 +136,6 @@ void mir::frontend::XdgToplevelDecorationV1::update_mode(uint32_t new_mode) send_configure_event(mode); } -void mir::frontend::XdgToplevelDecorationV1::set_mode(uint32_t mode) -{ - update_mode(mode); -} +void mir::frontend::XdgToplevelDecorationV1::set_mode(uint32_t mode) { update_mode(mode); } -void mir::frontend::XdgToplevelDecorationV1::unset_mode() -{ - update_mode(default_mode); -} +void mir::frontend::XdgToplevelDecorationV1::unset_mode() { update_mode(default_mode); } From b8f0b2c7232b5a17ef311dbb377e8e630358ebde Mon Sep 17 00:00:00 2001 From: Tarek Yasser Date: Mon, 24 Jun 2024 19:04:42 +0300 Subject: [PATCH 13/81] Convert the wayland error check to a sanity check in get_toplevel_decoration --- src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 93b480e4803..d2838f99060 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -101,8 +101,7 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* auto* tl = mir::frontend::XdgToplevelStable::from(toplevel); if (!tl) { - BOOST_THROW_EXCEPTION( - mir::wayland::ProtocolError(resource, Error::orphaned, "Toplevel destroyed before attached decoration")); + BOOST_THROW_EXCEPTION(std::runtime_error("Invalid toplevel pointer")); } From f6317468e7d7b2f2de657a968e948df1ca45eb06 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Mon, 24 Jun 2024 16:36:58 +0100 Subject: [PATCH 14/81] Drop dead code --- src/platforms/wayland/displayclient.cpp | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/platforms/wayland/displayclient.cpp b/src/platforms/wayland/displayclient.cpp index 108a481ba20..9fe441b5f28 100644 --- a/src/platforms/wayland/displayclient.cpp +++ b/src/platforms/wayland/displayclient.cpp @@ -66,10 +66,6 @@ class mgw::DisplayClient::Output : xdg_surface* shell_surface{nullptr}; xdg_toplevel* shell_toplevel{nullptr}; - EGLContext eglctx{EGL_NO_CONTEXT}; - wl_egl_window* egl_window{nullptr}; - EGLSurface eglsurface{EGL_NO_SURFACE}; - std::optional pending_toplevel_size; bool has_initialized{false}; @@ -159,11 +155,6 @@ mgw::DisplayClient::Output::~Output() } wl_surface_destroy(surface); - - if (egl_window != nullptr) - { - wl_egl_window_destroy(egl_window); - } } void mgw::DisplayClient::Output::geometry( From d8adb77bbf86e9dc213bde51b68b6caf9bffd531 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Mon, 24 Jun 2024 15:37:45 +0100 Subject: [PATCH 15/81] Mutex to guard DisplayClient::Output state --- src/platforms/wayland/displayclient.cpp | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/platforms/wayland/displayclient.cpp b/src/platforms/wayland/displayclient.cpp index 9fe441b5f28..d61617ba2f7 100644 --- a/src/platforms/wayland/displayclient.cpp +++ b/src/platforms/wayland/displayclient.cpp @@ -100,6 +100,7 @@ class mgw::DisplayClient::Output : void set_next_image(std::unique_ptr content) override; private: + std::mutex mutex; std::unique_ptr next_frame; std::shared_ptr provider; }; @@ -299,10 +300,15 @@ void mgw::DisplayClient::Output::surface_configure(uint32_t serial) if (!has_initialized || size_is_changed) { has_initialized = true; - provider = std::make_shared( - owner_->provider->get_egl_display(), - surface, - output_size); + { + std::lock_guard lock{mutex}; + next_frame.reset(); + provider.reset(); + provider = std::make_shared( + owner_->provider->get_egl_display(), + surface, + output_size); + } owner_->on_display_config_changed(); } } @@ -368,8 +374,10 @@ void mgw::DisplayClient::Output::post() }); // The Framebuffer ensures that this swap_buffers call doesn't block... - next_frame->swap_buffers(); - + { + std::lock_guard lock{mutex}; + next_frame->swap_buffers(); + } // ...so we need external synchronisation to throttle rendering. // Wait for the host compositor to tell us to render. frame_sync->wait_for_done(); @@ -403,6 +411,8 @@ auto mgw::DisplayClient::Output::maybe_create_allocator(DisplayAllocator::Tag co { BOOST_THROW_EXCEPTION((std::runtime_error{"Attempted to create allocator before Output is fully initialised"})); } + + std::lock_guard lock{mutex}; return provider.get(); } return nullptr; @@ -428,6 +438,7 @@ void mgw::DisplayClient::Output::set_next_image(std::unique_ptr con { if (auto wl_content = unique_ptr_cast(std::move(content))) { + std::lock_guard lock{mutex}; next_frame = std::move(wl_content); } else From 5aa5fee10955892a4b1c565ead3f860bf9692729 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Mon, 24 Jun 2024 17:26:23 +0100 Subject: [PATCH 16/81] Do not call eglCreatePlatformWindowSurface twice for the same window --- src/platforms/wayland/wl_egl_display_provider.cpp | 12 +++++++++--- src/platforms/wayland/wl_egl_display_provider.h | 2 ++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/platforms/wayland/wl_egl_display_provider.cpp b/src/platforms/wayland/wl_egl_display_provider.cpp index a4803c29e70..99f78812700 100644 --- a/src/platforms/wayland/wl_egl_display_provider.cpp +++ b/src/platforms/wayland/wl_egl_display_provider.cpp @@ -138,10 +138,16 @@ auto mgw::WlDisplayAllocator::alloc_framebuffer( auto egl_context = eglCreateContext(dpy, egl_config, share_context, context_attr); if (egl_context == EGL_NO_CONTEXT) + { BOOST_THROW_EXCEPTION(egl_error("Failed to create EGL context")); + } + + if (surface == EGL_NO_SURFACE) + { + surface = eglCreatePlatformWindowSurface(dpy, egl_config, wl_window, nullptr); + } - auto surf = eglCreatePlatformWindowSurface(dpy, egl_config, wl_window, nullptr); - if (surf == EGL_NO_SURFACE) + if (surface == EGL_NO_SURFACE) { BOOST_THROW_EXCEPTION(egl_error("Failed to create EGL surface")); } @@ -149,7 +155,7 @@ auto mgw::WlDisplayAllocator::alloc_framebuffer( return std::make_unique( dpy, egl_context, - surf, + surface, size); } diff --git a/src/platforms/wayland/wl_egl_display_provider.h b/src/platforms/wayland/wl_egl_display_provider.h index 45c6a72da49..18e09854af1 100644 --- a/src/platforms/wayland/wl_egl_display_provider.h +++ b/src/platforms/wayland/wl_egl_display_provider.h @@ -20,6 +20,7 @@ #include "mir/graphics/platform.h" #include +#include struct wl_egl_window; @@ -73,6 +74,7 @@ class WlDisplayAllocator : public GenericEGLDisplayAllocator }; private: EGLDisplay const dpy; + std::atomic surface{EGL_NO_SURFACE}; struct ::wl_egl_window* const wl_window; geometry::Size const size; }; From 07418682f40656fb1ee10410c407c3b6555ceb1c Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Wed, 26 Jun 2024 12:42:07 +0300 Subject: [PATCH 17/81] Add listeners for decoration and toplevel destruction --- .../xdg_decoration_unstable_v1.cpp | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index d2838f99060..4fbb4da3846 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -23,6 +23,7 @@ #include "xdg_output_v1.h" #include "xdg_shell_stable.h" +#include #include namespace mir @@ -45,7 +46,7 @@ class XdgDecorationManagerV1 : public wayland::XdgDecorationManagerV1 private: void get_toplevel_decoration(wl_resource* id, wl_resource* toplevel) override; - std::unordered_set toplevels_with_decorations; + std::shared_ptr> toplevels_with_decorations; }; class XdgToplevelDecorationV1 : public wayland::XdgToplevelDecorationV1 @@ -84,7 +85,8 @@ void mir::frontend::XdgDecorationManagerV1::Global::bind(wl_resource* new_zxdg_d } mir::frontend::XdgDecorationManagerV1::XdgDecorationManagerV1(wl_resource* resource) : - mir::wayland::XdgDecorationManagerV1{resource, Version<1>{}} + mir::wayland::XdgDecorationManagerV1{resource, Version<1>{}}, + toplevels_with_decorations(std::make_shared>()) { } @@ -92,7 +94,7 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* { using Error = mir::frontend::XdgToplevelDecorationV1::Error; - if (toplevels_with_decorations.contains(toplevel)) + if (toplevels_with_decorations->contains(toplevel)) { BOOST_THROW_EXCEPTION(mir::wayland::ProtocolError( resource, Error::already_constructed, "Decoration already constructed for this toplevel")); @@ -104,9 +106,22 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* BOOST_THROW_EXCEPTION(std::runtime_error("Invalid toplevel pointer")); } - - new XdgToplevelDecorationV1{id, tl}; - toplevels_with_decorations.insert(toplevel); + auto decoration = new XdgToplevelDecorationV1{id, tl}; + toplevels_with_decorations->insert(toplevel); + + decoration->add_destroy_listener([toplevels_with_decorations = this->toplevels_with_decorations, toplevel]() + { toplevels_with_decorations->erase(toplevel); }); + + tl->add_destroy_listener( + [toplevels_with_decorations = this->toplevels_with_decorations, toplevel]() + { + if (toplevels_with_decorations->contains(toplevel)) + { + toplevels_with_decorations->erase(toplevel); + BOOST_THROW_EXCEPTION(mir::wayland::ProtocolError( + toplevel, Error::orphaned, "Toplevel destroyed before its attached decoration")); + } + }); } mir::frontend::XdgToplevelDecorationV1::XdgToplevelDecorationV1( From e2f5e93671ddd1280a9188483489225854c9ca78 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Tue, 25 Jun 2024 12:29:48 +0100 Subject: [PATCH 18/81] Move logic into EGLState --- .../wayland/wl_egl_display_provider.cpp | 77 ++++++++++++------- .../wayland/wl_egl_display_provider.h | 3 +- 2 files changed, 50 insertions(+), 30 deletions(-) diff --git a/src/platforms/wayland/wl_egl_display_provider.cpp b/src/platforms/wayland/wl_egl_display_provider.cpp index 99f78812700..49c06eb818f 100644 --- a/src/platforms/wayland/wl_egl_display_provider.cpp +++ b/src/platforms/wayland/wl_egl_display_provider.cpp @@ -20,6 +20,19 @@ class mgw::WlDisplayAllocator::Framebuffer::EGLState ctx{ctx}, surf{surf} { + auto current_ctx = eglGetCurrentContext(); + auto current_draw_surf = eglGetCurrentSurface(EGL_DRAW); + auto current_read_surf = eglGetCurrentSurface(EGL_READ); + + make_current(); + // Don't block in eglSwapBuffers; we rely on external synchronisation to throttle rendering + eglSwapInterval(dpy, 0); + release_current(); + + // Paranoia: Restore the previous EGL context state, just in case + eglMakeCurrent(dpy, current_draw_surf, current_read_surf, current_ctx); + + printf("{arg} == %d ==>> [%p] EGLState+\n", gettid(), (void*)this); fflush(stdout); } ~EGLState() @@ -28,32 +41,47 @@ class mgw::WlDisplayAllocator::Framebuffer::EGLState eglDestroyContext(dpy, ctx); eglDestroySurface(dpy, surf); } - + + void make_current() const + { + if (eglMakeCurrent(dpy, surf, surf, ctx) != EGL_TRUE) + { + BOOST_THROW_EXCEPTION((mg::egl_error("eglMakeCurrent failed"))); + } + } + + void release_current() const + { + if (eglMakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT) != EGL_TRUE) + { + BOOST_THROW_EXCEPTION(mg::egl_error("Failed to release EGL context")); + } + } + + void swap_buffers() const + { + if (eglSwapBuffers(dpy, surf) != EGL_TRUE) + { + printf("{arg} == %d ==>> [%p] EGLState::swap_buffers() !!!\n", gettid(), (void*)this); fflush(stdout); + BOOST_THROW_EXCEPTION((mg::egl_error("eglSwapBuffers failed"))); + } + } + EGLDisplay const dpy; EGLContext const ctx; EGLSurface const surf; }; mgw::WlDisplayAllocator::Framebuffer::Framebuffer(EGLDisplay dpy, EGLContext ctx, EGLSurface surf, geom::Size size) - : Framebuffer(std::make_shared(dpy, ctx, surf), size) + : state{std::make_shared(dpy, ctx, surf)}, size_{size} { - auto current_ctx = eglGetCurrentContext(); - auto current_draw_surf = eglGetCurrentSurface(EGL_DRAW); - auto current_read_surf = eglGetCurrentSurface(EGL_READ); - - make_current(); - // Don't block in eglSwapBuffers; we rely on external synchronisation to throttle rendering - eglSwapInterval(dpy, 0); - release_current(); - - // Paranoia: Restore the previous EGL context state, just in case - eglMakeCurrent(dpy, current_draw_surf, current_read_surf, current_ctx); } -mgw::WlDisplayAllocator::Framebuffer::Framebuffer(std::shared_ptr state, geom::Size size) - : state{std::move(state)}, - size_{size} +mgw::WlDisplayAllocator::Framebuffer::Framebuffer(Framebuffer const& that) + : state{that.state}, + size_{that.size_} { + printf("{arg} == %d ==>> [%p] Framebuffer=\n", gettid(), (void*)this); fflush(stdout); } auto mgw::WlDisplayAllocator::Framebuffer::size() const -> geom::Size @@ -63,31 +91,22 @@ auto mgw::WlDisplayAllocator::Framebuffer::size() const -> geom::Size void mgw::WlDisplayAllocator::Framebuffer::make_current() { - if (eglMakeCurrent(state->dpy, state->surf, state->surf, state->ctx) != EGL_TRUE) - { - BOOST_THROW_EXCEPTION((mg::egl_error("eglMakeCurrent failed"))); - } + state->make_current(); } void mgw::WlDisplayAllocator::Framebuffer::release_current() { - if (eglMakeCurrent(state->dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT) != EGL_TRUE) - { - BOOST_THROW_EXCEPTION(mg::egl_error("Failed to release EGL context")); - } + state->release_current(); } void mgw::WlDisplayAllocator::Framebuffer::swap_buffers() { - if (eglSwapBuffers(state->dpy, state->surf) != EGL_TRUE) - { - BOOST_THROW_EXCEPTION((mg::egl_error("eglSwapBuffers failed"))); - } + state->swap_buffers(); } auto mgw::WlDisplayAllocator::Framebuffer::clone_handle() -> std::unique_ptr { - return std::unique_ptr{new Framebuffer(state, size_)}; + return std::unique_ptr{new Framebuffer(*this)}; } mgw::WlDisplayAllocator::WlDisplayAllocator( diff --git a/src/platforms/wayland/wl_egl_display_provider.h b/src/platforms/wayland/wl_egl_display_provider.h index 18e09854af1..4f24ae33734 100644 --- a/src/platforms/wayland/wl_egl_display_provider.h +++ b/src/platforms/wayland/wl_egl_display_provider.h @@ -67,7 +67,8 @@ class WlDisplayAllocator : public GenericEGLDisplayAllocator void swap_buffers(); private: class EGLState; - Framebuffer(std::shared_ptr surf, geometry::Size size); + Framebuffer(std::shared_ptr state, geometry::Size size); + Framebuffer(Framebuffer const& that); std::shared_ptr const state; geometry::Size const size_; From 9de853e4c0fa8819e0df1be48266c9c68a448d81 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Tue, 25 Jun 2024 17:49:55 +0100 Subject: [PATCH 19/81] Ensure that the EGLSurface outlasts Framebuffers --- src/platforms/wayland/displayclient.cpp | 2 - .../wayland/wl_egl_display_provider.cpp | 80 +++++++++++++------ .../wayland/wl_egl_display_provider.h | 6 +- 3 files changed, 58 insertions(+), 30 deletions(-) diff --git a/src/platforms/wayland/displayclient.cpp b/src/platforms/wayland/displayclient.cpp index d61617ba2f7..c8089af2a4f 100644 --- a/src/platforms/wayland/displayclient.cpp +++ b/src/platforms/wayland/displayclient.cpp @@ -302,8 +302,6 @@ void mgw::DisplayClient::Output::surface_configure(uint32_t serial) has_initialized = true; { std::lock_guard lock{mutex}; - next_frame.reset(); - provider.reset(); provider = std::make_shared( owner_->provider->get_egl_display(), surface, diff --git a/src/platforms/wayland/wl_egl_display_provider.cpp b/src/platforms/wayland/wl_egl_display_provider.cpp index 49c06eb818f..ee18ad88dc9 100644 --- a/src/platforms/wayland/wl_egl_display_provider.cpp +++ b/src/platforms/wayland/wl_egl_display_provider.cpp @@ -12,13 +12,47 @@ namespace mg = mir::graphics; namespace mgw = mir::graphics::wayland; namespace geom = mir::geometry; +class mgw::WlDisplayAllocator::SurfaceState +{ +public: + SurfaceState(EGLDisplay dpy, struct ::wl_egl_window* wl_window) : + dpy{dpy}, wl_window{wl_window} {} + + ~SurfaceState() + { + if (egl_surface != EGL_NO_SURFACE) eglDestroySurface(dpy, egl_surface); + wl_egl_window_destroy(wl_window); + } + + auto surface(EGLConfig egl_config) -> EGLSurface + { + if (egl_surface == EGL_NO_SURFACE) + { + egl_surface = eglCreatePlatformWindowSurface(dpy, egl_config, wl_window, nullptr); + } + + if (egl_surface == EGL_NO_SURFACE) + { + BOOST_THROW_EXCEPTION(egl_error("Failed to create EGL surface")); + } + + return egl_surface; + } + +private: + EGLDisplay const dpy; + struct ::wl_egl_window* const wl_window; + std::atomic egl_surface{EGL_NO_SURFACE}; +}; + class mgw::WlDisplayAllocator::Framebuffer::EGLState { public: - EGLState(EGLDisplay dpy, EGLContext ctx, EGLSurface surf) + EGLState(EGLDisplay dpy, EGLContext ctx, EGLSurface surf, std::shared_ptr ss) : dpy{dpy}, ctx{ctx}, - surf{surf} + surf{surf}, + ss{std::move(ss)} { auto current_ctx = eglGetCurrentContext(); auto current_draw_surf = eglGetCurrentSurface(EGL_DRAW); @@ -31,15 +65,19 @@ class mgw::WlDisplayAllocator::Framebuffer::EGLState // Paranoia: Restore the previous EGL context state, just in case eglMakeCurrent(dpy, current_draw_surf, current_read_surf, current_ctx); - - printf("{arg} == %d ==>> [%p] EGLState+\n", gettid(), (void*)this); fflush(stdout); } ~EGLState() { + auto current_ctx = eglGetCurrentContext(); + auto current_draw_surf = eglGetCurrentSurface(EGL_DRAW); + auto current_read_surf = eglGetCurrentSurface(EGL_READ); + eglMakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT); eglDestroyContext(dpy, ctx); - eglDestroySurface(dpy, surf); + + // Paranoia: Restore the previous EGL context state, just in case + eglMakeCurrent(dpy, current_draw_surf, current_read_surf, current_ctx); } void make_current() const @@ -62,7 +100,6 @@ class mgw::WlDisplayAllocator::Framebuffer::EGLState { if (eglSwapBuffers(dpy, surf) != EGL_TRUE) { - printf("{arg} == %d ==>> [%p] EGLState::swap_buffers() !!!\n", gettid(), (void*)this); fflush(stdout); BOOST_THROW_EXCEPTION((mg::egl_error("eglSwapBuffers failed"))); } } @@ -70,10 +107,17 @@ class mgw::WlDisplayAllocator::Framebuffer::EGLState EGLDisplay const dpy; EGLContext const ctx; EGLSurface const surf; + std::shared_ptr const ss; }; -mgw::WlDisplayAllocator::Framebuffer::Framebuffer(EGLDisplay dpy, EGLContext ctx, EGLSurface surf, geom::Size size) - : state{std::make_shared(dpy, ctx, surf)}, size_{size} +mgw::WlDisplayAllocator::Framebuffer::Framebuffer( + EGLDisplay dpy, EGLContext ctx, EGLSurface surf, std::shared_ptr ss, mir::geometry::Size size) : + Framebuffer{std::make_shared(dpy, ctx, surf, ss), size} +{ +} + +mgw::WlDisplayAllocator::Framebuffer::Framebuffer(std::shared_ptr state, geom::Size size) + : state{std::move(state)}, size_{size} { } @@ -81,7 +125,6 @@ mgw::WlDisplayAllocator::Framebuffer::Framebuffer(Framebuffer const& that) : state{that.state}, size_{that.size_} { - printf("{arg} == %d ==>> [%p] Framebuffer=\n", gettid(), (void*)this); fflush(stdout); } auto mgw::WlDisplayAllocator::Framebuffer::size() const -> geom::Size @@ -114,14 +157,13 @@ mgw::WlDisplayAllocator::WlDisplayAllocator( struct wl_surface* surface, geometry::Size size) : dpy{dpy}, - wl_window{wl_egl_window_create(surface, size.width.as_int(), size.height.as_int())}, + surface_state{std::make_shared(dpy, wl_egl_window_create(surface, size.width.as_int(), size.height.as_int()))}, size{size} { } mgw::WlDisplayAllocator::~WlDisplayAllocator() { - wl_egl_window_destroy(wl_window); } auto mgw::WlDisplayAllocator::alloc_framebuffer( @@ -161,21 +203,9 @@ auto mgw::WlDisplayAllocator::alloc_framebuffer( BOOST_THROW_EXCEPTION(egl_error("Failed to create EGL context")); } - if (surface == EGL_NO_SURFACE) - { - surface = eglCreatePlatformWindowSurface(dpy, egl_config, wl_window, nullptr); - } - - if (surface == EGL_NO_SURFACE) - { - BOOST_THROW_EXCEPTION(egl_error("Failed to create EGL surface")); - } + auto surface = surface_state->surface(egl_config); - return std::make_unique( - dpy, - egl_context, - surface, - size); + return std::make_unique(dpy, egl_context, surface, surface_state, size); } mgw::WlDisplayProvider::WlDisplayProvider(EGLDisplay dpy) diff --git a/src/platforms/wayland/wl_egl_display_provider.h b/src/platforms/wayland/wl_egl_display_provider.h index 4f24ae33734..68bdac3b547 100644 --- a/src/platforms/wayland/wl_egl_display_provider.h +++ b/src/platforms/wayland/wl_egl_display_provider.h @@ -46,6 +46,7 @@ class WlDisplayAllocator : public GenericEGLDisplayAllocator auto alloc_framebuffer(GLConfig const& config, EGLContext share_context) -> std::unique_ptr override; + struct SurfaceState; class Framebuffer : public GenericEGLDisplayAllocator::EGLFramebuffer { public: @@ -56,7 +57,7 @@ class WlDisplayAllocator : public GenericEGLDisplayAllocator * final handle generated from this Framebuffer is released, * the EGL resources \param ctx and \param surff will be freed. */ - Framebuffer(EGLDisplay dpy, EGLContext ctx, EGLSurface surf, geometry::Size size); + Framebuffer(EGLDisplay dpy, EGLContext ctx, EGLSurface surf, std::shared_ptr ss, geometry::Size size); auto size() const -> geometry::Size override; @@ -75,8 +76,7 @@ class WlDisplayAllocator : public GenericEGLDisplayAllocator }; private: EGLDisplay const dpy; - std::atomic surface{EGL_NO_SURFACE}; - struct ::wl_egl_window* const wl_window; + std::shared_ptr surface_state; geometry::Size const size; }; } From dce938cc47f0067da7d472319e424d4e8ff0ff61 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Wed, 26 Jun 2024 16:38:22 +0100 Subject: [PATCH 20/81] Cleaner threading --- src/platforms/wayland/wl_egl_display_provider.cpp | 5 ++++- src/platforms/wayland/wl_egl_display_provider.h | 3 +-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/platforms/wayland/wl_egl_display_provider.cpp b/src/platforms/wayland/wl_egl_display_provider.cpp index ee18ad88dc9..9f5d0eb5e94 100644 --- a/src/platforms/wayland/wl_egl_display_provider.cpp +++ b/src/platforms/wayland/wl_egl_display_provider.cpp @@ -26,6 +26,7 @@ class mgw::WlDisplayAllocator::SurfaceState auto surface(EGLConfig egl_config) -> EGLSurface { + std::lock_guard lock{mutex}; if (egl_surface == EGL_NO_SURFACE) { egl_surface = eglCreatePlatformWindowSurface(dpy, egl_config, wl_window, nullptr); @@ -42,7 +43,9 @@ class mgw::WlDisplayAllocator::SurfaceState private: EGLDisplay const dpy; struct ::wl_egl_window* const wl_window; - std::atomic egl_surface{EGL_NO_SURFACE}; + + std::mutex mutex; + EGLSurface egl_surface{EGL_NO_SURFACE}; }; class mgw::WlDisplayAllocator::Framebuffer::EGLState diff --git a/src/platforms/wayland/wl_egl_display_provider.h b/src/platforms/wayland/wl_egl_display_provider.h index 68bdac3b547..86349b17979 100644 --- a/src/platforms/wayland/wl_egl_display_provider.h +++ b/src/platforms/wayland/wl_egl_display_provider.h @@ -20,7 +20,6 @@ #include "mir/graphics/platform.h" #include -#include struct wl_egl_window; @@ -76,7 +75,7 @@ class WlDisplayAllocator : public GenericEGLDisplayAllocator }; private: EGLDisplay const dpy; - std::shared_ptr surface_state; + std::shared_ptr const surface_state; geometry::Size const size; }; } From 6564219aa9c217dfc0bc595f9fd67340c242429c Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Wed, 26 Jun 2024 16:49:24 +0100 Subject: [PATCH 21/81] Cleaner EGL state stashing --- .../wayland/wl_egl_display_provider.cpp | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/src/platforms/wayland/wl_egl_display_provider.cpp b/src/platforms/wayland/wl_egl_display_provider.cpp index 9f5d0eb5e94..312d4a851f0 100644 --- a/src/platforms/wayland/wl_egl_display_provider.cpp +++ b/src/platforms/wayland/wl_egl_display_provider.cpp @@ -12,6 +12,27 @@ namespace mg = mir::graphics; namespace mgw = mir::graphics::wayland; namespace geom = mir::geometry; +namespace +{ +// Utility to restore EGL state on scope exit +class CacheEglState +{ +public: + CacheEglState() = default; + + ~CacheEglState() + { + eglMakeCurrent(dpy, draw_surf, read_surf, ctx); + } +private: + CacheEglState(CacheEglState const&) = delete; + EGLDisplay const dpy = eglGetCurrentDisplay(); + EGLContext const ctx = eglGetCurrentContext(); + EGLSurface const draw_surf = eglGetCurrentSurface(EGL_DRAW); + EGLSurface const read_surf = eglGetCurrentSurface(EGL_READ); +}; +} + class mgw::WlDisplayAllocator::SurfaceState { public: @@ -57,30 +78,20 @@ class mgw::WlDisplayAllocator::Framebuffer::EGLState surf{surf}, ss{std::move(ss)} { - auto current_ctx = eglGetCurrentContext(); - auto current_draw_surf = eglGetCurrentSurface(EGL_DRAW); - auto current_read_surf = eglGetCurrentSurface(EGL_READ); + CacheEglState stash; make_current(); // Don't block in eglSwapBuffers; we rely on external synchronisation to throttle rendering eglSwapInterval(dpy, 0); release_current(); - - // Paranoia: Restore the previous EGL context state, just in case - eglMakeCurrent(dpy, current_draw_surf, current_read_surf, current_ctx); } ~EGLState() { - auto current_ctx = eglGetCurrentContext(); - auto current_draw_surf = eglGetCurrentSurface(EGL_DRAW); - auto current_read_surf = eglGetCurrentSurface(EGL_READ); + CacheEglState stash; eglMakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT); eglDestroyContext(dpy, ctx); - - // Paranoia: Restore the previous EGL context state, just in case - eglMakeCurrent(dpy, current_draw_surf, current_read_surf, current_ctx); } void make_current() const From 2a42cf4777ae602fb11e17909fc54815a366a884 Mon Sep 17 00:00:00 2001 From: Tarek Yasser Date: Fri, 28 Jun 2024 12:12:18 +0300 Subject: [PATCH 22/81] Log instead of throwing `orphaned` when toplevels are destroyed. --- .../frontend_wayland/xdg_decoration_unstable_v1.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 4fbb4da3846..b17f36c53c9 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -17,7 +17,9 @@ #include "xdg_decoration_unstable_v1.h" #include "mir/shell/surface_specification.h" +#include "mir/wayland/client.h" #include "mir/wayland/protocol_error.h" +#include "mir/log.h" #include "xdg-decoration-unstable-v1_wrapper.h" #include "xdg_output_v1.h" @@ -113,13 +115,14 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* { toplevels_with_decorations->erase(toplevel); }); tl->add_destroy_listener( - [toplevels_with_decorations = this->toplevels_with_decorations, toplevel]() + [toplevels_with_decorations = this->toplevels_with_decorations, client = this->client, toplevel]() { - if (toplevels_with_decorations->contains(toplevel)) + if (!client->is_being_destroyed() && toplevels_with_decorations->erase(toplevel) > 0) { - toplevels_with_decorations->erase(toplevel); - BOOST_THROW_EXCEPTION(mir::wayland::ProtocolError( - toplevel, Error::orphaned, "Toplevel destroyed before its attached decoration")); + mir::log_warning("Toplevel destroyed before attached decoration!"); + // https://github.com/canonical/mir/issues/3452 + /* BOOST_THROW_EXCEPTION(mir::wayland::ProtocolError( */ + /* resource, Error::orphaned, "Toplevel destroyed before its attached decoration")); */ } }); } From f52391eabe8675b5c9989c96f061bc5eeb6e022d Mon Sep 17 00:00:00 2001 From: Tarek Yasser Date: Fri, 28 Jun 2024 14:12:44 +0300 Subject: [PATCH 23/81] Add `destroy_toplevel_before_decoration` to expected failure list. --- tests/acceptance-tests/wayland/CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/acceptance-tests/wayland/CMakeLists.txt b/tests/acceptance-tests/wayland/CMakeLists.txt index b468f6e4e6c..d545a0323d9 100644 --- a/tests/acceptance-tests/wayland/CMakeLists.txt +++ b/tests/acceptance-tests/wayland/CMakeLists.txt @@ -113,6 +113,10 @@ set(EXPECTED_FAILURES # Not yet implemented, see https://github.com/MirServer/mir/issues/2861 XdgPopupTest.when_parent_surface_is_moved_a_reactive_popup_is_moved + + # Should throw an exception, but we log instead because of + # https://github.com/canonical/mir/issues/3452 + XdgDecorationV1Test.destroy_toplevel_before_decoration ) if (MIR_SIGBUS_HANDLER_ENVIRONMENT_BROKEN) From 66326012f0649389527dbca49de84fa8e3ee7496 Mon Sep 17 00:00:00 2001 From: Tarek Yasser Date: Mon, 1 Jul 2024 12:41:10 +0300 Subject: [PATCH 24/81] Wrap `toplevels_with_decorations` in a class. --- .../xdg_decoration_unstable_v1.cpp | 64 ++++++++++++++----- 1 file changed, 48 insertions(+), 16 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index b17f36c53c9..37283220c05 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -16,10 +16,10 @@ #include "xdg_decoration_unstable_v1.h" +#include "mir/log.h" #include "mir/shell/surface_specification.h" #include "mir/wayland/client.h" #include "mir/wayland/protocol_error.h" -#include "mir/log.h" #include "xdg-decoration-unstable-v1_wrapper.h" #include "xdg_output_v1.h" @@ -32,6 +32,31 @@ namespace mir { namespace frontend { +class ToplevelsWithDecorations +{ +public: + ToplevelsWithDecorations() : + toplevels_with_decorations{std::make_shared>()} + { + } + + /// \return true if no duplicates existed before insertion, false otherwise. + bool registerToplevel(wl_resource* toplevel) + { + auto [_, inserted] = toplevels_with_decorations->insert(toplevel); + return inserted; + } + + /// \return true if only one element was erase, false otherwise. + bool unregisterToplevel(wl_resource* toplevel) + { + return toplevels_with_decorations->erase(toplevel) == 1; + } + +private: + std::shared_ptr> toplevels_with_decorations; +}; + class XdgDecorationManagerV1 : public wayland::XdgDecorationManagerV1 { public: @@ -48,7 +73,7 @@ class XdgDecorationManagerV1 : public wayland::XdgDecorationManagerV1 private: void get_toplevel_decoration(wl_resource* id, wl_resource* toplevel) override; - std::shared_ptr> toplevels_with_decorations; + ToplevelsWithDecorations toplevels_with_decorations; }; class XdgToplevelDecorationV1 : public wayland::XdgToplevelDecorationV1 @@ -88,7 +113,7 @@ void mir::frontend::XdgDecorationManagerV1::Global::bind(wl_resource* new_zxdg_d mir::frontend::XdgDecorationManagerV1::XdgDecorationManagerV1(wl_resource* resource) : mir::wayland::XdgDecorationManagerV1{resource, Version<1>{}}, - toplevels_with_decorations(std::make_shared>()) + toplevels_with_decorations{} { } @@ -96,12 +121,6 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* { using Error = mir::frontend::XdgToplevelDecorationV1::Error; - if (toplevels_with_decorations->contains(toplevel)) - { - BOOST_THROW_EXCEPTION(mir::wayland::ProtocolError( - resource, Error::already_constructed, "Decoration already constructed for this toplevel")); - } - auto* tl = mir::frontend::XdgToplevelStable::from(toplevel); if (!tl) { @@ -109,15 +128,22 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* } auto decoration = new XdgToplevelDecorationV1{id, tl}; - toplevels_with_decorations->insert(toplevel); + if (!toplevels_with_decorations.registerToplevel(toplevel)) + { + BOOST_THROW_EXCEPTION(mir::wayland::ProtocolError( + resource, Error::already_constructed, "Decoration already constructed for this toplevel")); + } - decoration->add_destroy_listener([toplevels_with_decorations = this->toplevels_with_decorations, toplevel]() - { toplevels_with_decorations->erase(toplevel); }); + decoration->add_destroy_listener( + [&toplevels_with_decorations = this->toplevels_with_decorations, toplevel]() + { + toplevels_with_decorations.unregisterToplevel(toplevel); + }); tl->add_destroy_listener( - [toplevels_with_decorations = this->toplevels_with_decorations, client = this->client, toplevel]() + [&toplevels_with_decorations = this->toplevels_with_decorations, client = this->client, toplevel]() { - if (!client->is_being_destroyed() && toplevels_with_decorations->erase(toplevel) > 0) + if (!client->is_being_destroyed() && !toplevels_with_decorations.unregisterToplevel(toplevel)) { mir::log_warning("Toplevel destroyed before attached decoration!"); // https://github.com/canonical/mir/issues/3452 @@ -153,6 +179,12 @@ void mir::frontend::XdgToplevelDecorationV1::update_mode(uint32_t new_mode) send_configure_event(mode); } -void mir::frontend::XdgToplevelDecorationV1::set_mode(uint32_t mode) { update_mode(mode); } +void mir::frontend::XdgToplevelDecorationV1::set_mode(uint32_t mode) +{ + update_mode(mode); +} -void mir::frontend::XdgToplevelDecorationV1::unset_mode() { update_mode(default_mode); } +void mir::frontend::XdgToplevelDecorationV1::unset_mode() +{ + update_mode(default_mode); +} From f240d13da242d3a3e23659e896c8ea74fd926372 Mon Sep 17 00:00:00 2001 From: Tarek Yasser Date: Mon, 1 Jul 2024 12:44:58 +0300 Subject: [PATCH 25/81] Fix comment typo in `mir::frontend::ToplevelsWithDecorations::unregisterToplevel` --- src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 37283220c05..2fcfa88a087 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -47,7 +47,7 @@ class ToplevelsWithDecorations return inserted; } - /// \return true if only one element was erase, false otherwise. + /// \return true if only one element was erased, false otherwise. bool unregisterToplevel(wl_resource* toplevel) { return toplevels_with_decorations->erase(toplevel) == 1; From 27d1207280701be6adf56fd1a0835da14b869d79 Mon Sep 17 00:00:00 2001 From: Tarek Yasser Date: Mon, 1 Jul 2024 12:47:25 +0300 Subject: [PATCH 26/81] Make comment in `acceptance-tests/wayland/CMakeLists.txt` clearer. --- tests/acceptance-tests/wayland/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/acceptance-tests/wayland/CMakeLists.txt b/tests/acceptance-tests/wayland/CMakeLists.txt index d545a0323d9..493b3abb4f5 100644 --- a/tests/acceptance-tests/wayland/CMakeLists.txt +++ b/tests/acceptance-tests/wayland/CMakeLists.txt @@ -114,8 +114,8 @@ set(EXPECTED_FAILURES # Not yet implemented, see https://github.com/MirServer/mir/issues/2861 XdgPopupTest.when_parent_surface_is_moved_a_reactive_popup_is_moved - # Should throw an exception, but we log instead because of - # https://github.com/canonical/mir/issues/3452 + # Should report an `orphaned` protocol error/violation , but we log instead + # because of https://github.com/canonical/mir/issues/3452 XdgDecorationV1Test.destroy_toplevel_before_decoration ) From 7d848b0d4fb845b540d23bae87322899b1a6d660 Mon Sep 17 00:00:00 2001 From: Tarek Yasser Date: Mon, 1 Jul 2024 15:58:33 +0300 Subject: [PATCH 27/81] Add mir server side decoration strategy --- src/core/symbols.map | 3 +- src/include/server/mir/decoration_strategy.h | 33 +++++++ .../server/mir/default_server_configuration.h | 5 + src/include/server/mir/server.h | 3 + src/include/server/mir/server_configuration.h | 3 + src/server/default_server_configuration.cpp | 21 +++++ .../frontend_wayland/wayland_connector.cpp | 6 +- .../frontend_wayland/wayland_connector.h | 5 +- .../wayland_default_configuration.cpp | 5 +- .../xdg_decoration_unstable_v1.cpp | 91 ++++++++++++++----- .../xdg_decoration_unstable_v1.h | 4 +- src/server/server.cpp | 3 +- src/server/symbols.map | 12 +++ 13 files changed, 163 insertions(+), 31 deletions(-) create mode 100644 src/include/server/mir/decoration_strategy.h diff --git a/src/core/symbols.map b/src/core/symbols.map index b1e30db825e..d3675afa067 100644 --- a/src/core/symbols.map +++ b/src/core/symbols.map @@ -1,5 +1,5 @@ MIR_CORE_2.9 { - global: +global: extern "C++" { mir::AnonymousShmFile::?AnonymousShmFile*; mir::AnonymousShmFile::AnonymousShmFile*; @@ -29,3 +29,4 @@ MIR_CORE_2.9 { }; local: *; }; + diff --git a/src/include/server/mir/decoration_strategy.h b/src/include/server/mir/decoration_strategy.h new file mode 100644 index 00000000000..b63a628eff2 --- /dev/null +++ b/src/include/server/mir/decoration_strategy.h @@ -0,0 +1,33 @@ +/* + * Copyright © Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 or 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include +namespace mir +{ +class DecorationStrategy +{ +public: + enum class DecorationsType : uint32_t + { + csd, + ssd + }; + + virtual ~DecorationStrategy() = default; + virtual auto default_style() const -> DecorationsType = 0; + virtual auto request_style(DecorationsType type) const -> DecorationsType = 0; +}; +} diff --git a/src/include/server/mir/default_server_configuration.h b/src/include/server/mir/default_server_configuration.h index a5f77388b16..fefcaf19cad 100644 --- a/src/include/server/mir/default_server_configuration.h +++ b/src/include/server/mir/default_server_configuration.h @@ -36,6 +36,7 @@ template class ObserverMultiplexer; class ConsoleServices; +class DecorationStrategy; namespace dispatch { @@ -334,6 +335,9 @@ class DefaultServerConfiguration : public virtual ServerConfiguration auto default_reports() -> std::shared_ptr; + auto the_decoration_strategy() -> std::shared_ptr override; + void set_the_decoration_strategy(std::shared_ptr strategy) override; + protected: std::shared_ptr the_options() const; std::shared_ptr the_default_input_device_hub(); @@ -420,6 +424,7 @@ class DefaultServerConfiguration : public virtual ServerConfiguration CachedPtr cookie_authority; CachedPtr key_mapper; std::shared_ptr console_services; + std::shared_ptr decoration_strategy; private: std::shared_ptr const configuration_options; diff --git a/src/include/server/mir/server.h b/src/include/server/mir/server.h index 789919094b7..28ef5ff9493 100644 --- a/src/include/server/mir/server.h +++ b/src/include/server/mir/server.h @@ -74,6 +74,7 @@ class RendererFactory; class Fd; class MainLoop; class ServerStatusListener; +class DecorationStrategy; enum class OptionType { @@ -469,6 +470,8 @@ class Server void set_enabled_wayland_extensions(std::vector const& extensions); /** @} */ + auto the_decoration_strategy() const -> std::shared_ptr; + void set_the_decoration_strategy(std::shared_ptr strategy); private: struct ServerConfiguration; struct Self; diff --git a/src/include/server/mir/server_configuration.h b/src/include/server/mir/server_configuration.h index e5f2881b507..ba515527d7c 100644 --- a/src/include/server/mir/server_configuration.h +++ b/src/include/server/mir/server_configuration.h @@ -72,6 +72,7 @@ class ServerStatusListener; class DisplayChanger; class EmergencyCleanup; class ConsoleServices; +class DecorationStrategy; class ServerConfiguration { @@ -111,6 +112,8 @@ class ServerConfiguration virtual void set_wayland_extension_filter(WaylandProtocolExtensionFilter const& extension_filter) = 0; virtual void set_enabled_wayland_extensions(std::vector const& extensions) = 0; + virtual auto the_decoration_strategy() -> std::shared_ptr = 0; + virtual void set_the_decoration_strategy(std::shared_ptr strategy) = 0; protected: ServerConfiguration() = default; virtual ~ServerConfiguration() = default; diff --git a/src/server/default_server_configuration.cpp b/src/server/default_server_configuration.cpp index 94216b82c82..09d4a5fc138 100644 --- a/src/server/default_server_configuration.cpp +++ b/src/server/default_server_configuration.cpp @@ -38,6 +38,7 @@ #include "default_emergency_cleanup.h" #include "mir/graphics/platform.h" #include "mir/console_services.h" +#include "mir/decoration_strategy.h" namespace mc = mir::compositor; namespace geom = mir::geometry; @@ -198,3 +199,23 @@ auto mir::DefaultServerConfiguration::the_logger() return std::make_shared(); }); } + +auto mir::DefaultServerConfiguration::the_decoration_strategy() -> std::shared_ptr +{ + if(!decoration_strategy) { + class DefaultDecorationStrategy: public mir::DecorationStrategy + { + DecorationsType default_style() const override { return DecorationsType::csd; } + DecorationsType request_style(DecorationsType type) const override { return type; } + }; + + decoration_strategy = std::make_shared(); + } + + return decoration_strategy; +} + +void mir::DefaultServerConfiguration::set_the_decoration_strategy(std::shared_ptr strategy) +{ + decoration_strategy = strategy; +} diff --git a/src/server/frontend_wayland/wayland_connector.cpp b/src/server/frontend_wayland/wayland_connector.cpp index ed20fa716cc..e7158fb5f04 100644 --- a/src/server/frontend_wayland/wayland_connector.cpp +++ b/src/server/frontend_wayland/wayland_connector.cpp @@ -239,7 +239,8 @@ mf::WaylandConnector::WaylandConnector( std::unique_ptr extensions_, WaylandProtocolExtensionFilter const& extension_filter, bool enable_key_repeat, - std::shared_ptr const& session_lock) + std::shared_ptr const& session_lock, + std::shared_ptr const& decoration_strategy) : extension_filter{extension_filter}, display{wl_display_create(), &cleanup_display}, pause_signal{eventfd(0, EFD_CLOEXEC | EFD_SEMAPHORE)}, @@ -325,7 +326,8 @@ mf::WaylandConnector::WaylandConnector( screen_shooter, main_loop, desktop_file_manager, - session_lock_}); + session_lock_, + decoration_strategy}); shm_global = std::make_unique(display.get(), executor); diff --git a/src/server/frontend_wayland/wayland_connector.h b/src/server/frontend_wayland/wayland_connector.h index bf599cf547e..7f742bd17d3 100644 --- a/src/server/frontend_wayland/wayland_connector.h +++ b/src/server/frontend_wayland/wayland_connector.h @@ -36,6 +36,7 @@ class Executor; class MainLoop; template class ObserverRegistrar; +class DecorationStrategy; namespace compositor { @@ -110,6 +111,7 @@ class WaylandExtensions std::shared_ptr main_loop; std::shared_ptr desktop_file_manager; std::shared_ptr session_lock; + std::shared_ptr decoration_strategy; }; WaylandExtensions() = default; @@ -161,7 +163,8 @@ class WaylandConnector : public Connector std::unique_ptr extensions, WaylandProtocolExtensionFilter const& extension_filter, bool enable_key_repeat, - std::shared_ptr const& session_lock); + std::shared_ptr const& session_lock, + std::shared_ptr const& decoration_strategy); ~WaylandConnector() override; diff --git a/src/server/frontend_wayland/wayland_default_configuration.cpp b/src/server/frontend_wayland/wayland_default_configuration.cpp index 8fb90cc6df6..8d4b9c28304 100644 --- a/src/server/frontend_wayland/wayland_default_configuration.cpp +++ b/src/server/frontend_wayland/wayland_default_configuration.cpp @@ -218,7 +218,7 @@ std::vector const internal_extension_builders = { }), make_extension_builder([](auto const& ctx) { - return mf::create_xdg_decoration_unstable_v1(ctx.display); + return mf::create_xdg_decoration_unstable_v1(ctx.display, ctx.decoration_strategy); }) }; @@ -375,7 +375,8 @@ std::shared_ptr wayland_extension_hooks), wayland_extension_filter, enable_repeat, - the_session_lock()); + the_session_lock(), + the_decoration_strategy()); }); } diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 2fcfa88a087..b3534e1e320 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -16,6 +16,7 @@ #include "xdg_decoration_unstable_v1.h" +#include "mir/decoration_strategy.h" #include "mir/log.h" #include "mir/shell/surface_specification.h" #include "mir/wayland/client.h" @@ -25,6 +26,7 @@ #include "xdg_output_v1.h" #include "xdg_shell_stable.h" +#include #include #include @@ -60,60 +62,68 @@ class ToplevelsWithDecorations class XdgDecorationManagerV1 : public wayland::XdgDecorationManagerV1 { public: - XdgDecorationManagerV1(wl_resource* resource); + XdgDecorationManagerV1(wl_resource* resource, std::shared_ptr strategy); class Global : public wayland::XdgDecorationManagerV1::Global { public: - Global(wl_display* display); + Global(wl_display* display, std::shared_ptr strategy); private: + std::shared_ptr decoration_strategy; void bind(wl_resource* new_zxdg_decoration_manager_v1) override; }; private: void get_toplevel_decoration(wl_resource* id, wl_resource* toplevel) override; + ToplevelsWithDecorations toplevels_with_decorations; + std::shared_ptr decoration_strategy; }; class XdgToplevelDecorationV1 : public wayland::XdgToplevelDecorationV1 { public: - XdgToplevelDecorationV1(wl_resource* id, mir::frontend::XdgToplevelStable* toplevel); + XdgToplevelDecorationV1( + wl_resource* id, mir::frontend::XdgToplevelStable* toplevel, std::shared_ptr strategy); void set_mode(uint32_t mode) override; void unset_mode() override; private: + auto decorations_type_to_protocol_mode(DecorationStrategy::DecorationsType) -> uint32_t; + auto protocol_mode_to_decorations_type(uint32_t) -> DecorationStrategy::DecorationsType; void update_mode(uint32_t new_mode); - static uint32_t const default_mode = Mode::client_side; - mir::frontend::XdgToplevelStable* toplevel; - uint32_t mode; + std::shared_ptr decoration_strategy; }; } // namespace frontend } // namespace mir -auto mir::frontend::create_xdg_decoration_unstable_v1(wl_display* display) +auto mir::frontend::create_xdg_decoration_unstable_v1(wl_display* display, std::shared_ptr strategy) -> std::shared_ptr { - return std::make_shared(display); + return std::make_shared(display, strategy); } -mir::frontend::XdgDecorationManagerV1::Global::Global(wl_display* display) : - wayland::XdgDecorationManagerV1::Global::Global{display, Version<1>{}} +mir::frontend::XdgDecorationManagerV1::Global::Global( + wl_display* display, std::shared_ptr strategy) : + wayland::XdgDecorationManagerV1::Global::Global{display, Version<1>{}}, + decoration_strategy{strategy} { } void mir::frontend::XdgDecorationManagerV1::Global::bind(wl_resource* new_zxdg_decoration_manager_v1) { - new XdgDecorationManagerV1{new_zxdg_decoration_manager_v1}; + new XdgDecorationManagerV1{new_zxdg_decoration_manager_v1, decoration_strategy}; } -mir::frontend::XdgDecorationManagerV1::XdgDecorationManagerV1(wl_resource* resource) : +mir::frontend::XdgDecorationManagerV1::XdgDecorationManagerV1( + wl_resource* resource, std::shared_ptr strategy) : mir::wayland::XdgDecorationManagerV1{resource, Version<1>{}}, - toplevels_with_decorations{} + toplevels_with_decorations{}, + decoration_strategy{strategy} { } @@ -127,7 +137,7 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* BOOST_THROW_EXCEPTION(std::runtime_error("Invalid toplevel pointer")); } - auto decoration = new XdgToplevelDecorationV1{id, tl}; + auto decoration = new XdgToplevelDecorationV1{id, tl, decoration_strategy}; if (!toplevels_with_decorations.registerToplevel(toplevel)) { BOOST_THROW_EXCEPTION(mir::wayland::ProtocolError( @@ -154,29 +164,63 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* } mir::frontend::XdgToplevelDecorationV1::XdgToplevelDecorationV1( - wl_resource* id, mir::frontend::XdgToplevelStable* toplevel) : + wl_resource* id, mir::frontend::XdgToplevelStable* toplevel, std::shared_ptr strategy) : wayland::XdgToplevelDecorationV1{id, Version<1>{}}, - toplevel{toplevel} + toplevel{toplevel}, + decoration_strategy{strategy} { } -void mir::frontend::XdgToplevelDecorationV1::update_mode(uint32_t new_mode) +auto mir::frontend::XdgToplevelDecorationV1::decorations_type_to_protocol_mode(DecorationStrategy::DecorationsType type) + -> uint32_t { - auto spec = shell::SurfaceSpecification{}; + switch (type) + { + case DecorationStrategy::DecorationsType::ssd: + return Mode::server_side; + case DecorationStrategy::DecorationsType::csd: + return Mode::client_side; + default: + mir::log_warning("%s: Got invalid protocol decorations type, defaulting to client side", __FUNCTION__); + return Mode::client_side; + } +} - mode = new_mode; +auto mir::frontend::XdgToplevelDecorationV1::protocol_mode_to_decorations_type(uint32_t mode) + -> DecorationStrategy::DecorationsType +{ switch (mode) { case Mode::client_side: - spec.server_side_decorated = false; - break; + return DecorationStrategy::DecorationsType::csd; case Mode::server_side: + return DecorationStrategy::DecorationsType::ssd; + default: + mir::log_warning("%s: Got invalid protocol mode (%d), defaulting to client side", __FUNCTION__, mode); + return DecorationStrategy::DecorationsType::csd; + } +} + +void mir::frontend::XdgToplevelDecorationV1::update_mode(uint32_t new_mode) +{ + auto spec = shell::SurfaceSpecification{}; + + auto const new_type = decoration_strategy->request_style(protocol_mode_to_decorations_type(new_mode)); + + switch (new_type) + { + case DecorationStrategy::DecorationsType::ssd: spec.server_side_decorated = true; break; + case DecorationStrategy::DecorationsType::csd: + spec.server_side_decorated = false; + break; } this->toplevel->apply_spec(spec); - send_configure_event(mode); + + auto const strategy_new_mode = decorations_type_to_protocol_mode(new_type); + send_configure_event(strategy_new_mode); } void mir::frontend::XdgToplevelDecorationV1::set_mode(uint32_t mode) @@ -186,5 +230,6 @@ void mir::frontend::XdgToplevelDecorationV1::set_mode(uint32_t mode) void mir::frontend::XdgToplevelDecorationV1::unset_mode() { - update_mode(default_mode); + auto const protocol_mode = decorations_type_to_protocol_mode(decoration_strategy->default_style()); + update_mode(protocol_mode); } diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.h b/src/server/frontend_wayland/xdg_decoration_unstable_v1.h index b368cbe2372..bf9c28c7aa6 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.h +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.h @@ -21,9 +21,11 @@ namespace mir { +class DecorationStrategy; + namespace frontend { -auto create_xdg_decoration_unstable_v1(wl_display* display) -> std::shared_ptr; +auto create_xdg_decoration_unstable_v1(wl_display* display, std::shared_ptr strategy) -> std::shared_ptr; } } // namespace mir diff --git a/src/server/server.cpp b/src/server/server.cpp index 8732d988d77..5131573e69c 100644 --- a/src/server/server.cpp +++ b/src/server/server.cpp @@ -132,7 +132,8 @@ struct TemporaryCompositeEventFilter : public mi::CompositeEventFilter MACRO(the_display_configuration_observer_registrar)\ MACRO(the_seat_observer_registrar)\ MACRO(the_session_lock)\ - MACRO(the_renderer_factory) + MACRO(the_renderer_factory)\ + MACRO(the_decoration_strategy) #define MIR_SERVER_BUILDER(name)\ std::function()> name##_builder; diff --git a/src/server/symbols.map b/src/server/symbols.map index c199faee3c9..01bec218222 100644 --- a/src/server/symbols.map +++ b/src/server/symbols.map @@ -10,10 +10,12 @@ global: mir::BasicCallback::lock*; mir::BasicCallback::operator*; mir::BasicCallback::unlock*; + mir::DecorationStrategy::?DecorationStrategy*; mir::DefaultServerConfiguration::DefaultServerConfiguration*; mir::DefaultServerConfiguration::add_wayland_extension*; mir::DefaultServerConfiguration::default_reports*; mir::DefaultServerConfiguration::set_enabled_wayland_extensions*; + mir::DefaultServerConfiguration::set_the_decoration_strategy*; mir::DefaultServerConfiguration::set_wayland_extension_filter*; mir::DefaultServerConfiguration::the_application_not_responding_detector*; mir::DefaultServerConfiguration::the_buffer_allocator*; @@ -26,6 +28,7 @@ global: mir::DefaultServerConfiguration::the_cursor_images*; mir::DefaultServerConfiguration::the_cursor_listener*; mir::DefaultServerConfiguration::the_decoration_manager*; + mir::DefaultServerConfiguration::the_decoration_strategy*; mir::DefaultServerConfiguration::the_default_cursor_image*; mir::DefaultServerConfiguration::the_default_input_device_hub*; mir::DefaultServerConfiguration::the_display*; @@ -179,6 +182,7 @@ global: mir::Server::set_enabled_wayland_extensions*; mir::Server::set_exception_handler*; mir::Server::set_terminator*; + mir::Server::set_the_decoration_strategy*; mir::Server::set_wayland_extension_filter*; mir::Server::stop*; mir::Server::supported_pixel_formats*; @@ -188,6 +192,7 @@ global: mir::Server::the_compositor_report*; mir::Server::the_cursor*; mir::Server::the_cursor_listener*; + mir::Server::the_decoration_strategy*; mir::Server::the_display*; mir::Server::the_display_configuration_controller*; mir::Server::the_display_configuration_observer_registrar*; @@ -669,8 +674,10 @@ global: non-virtual?thunk?to?mir::BasicCallback::lock*; non-virtual?thunk?to?mir::BasicCallback::operator*; non-virtual?thunk?to?mir::BasicCallback::unlock*; + non-virtual?thunk?to?mir::DecorationStrategy::?DecorationStrategy*; non-virtual?thunk?to?mir::DefaultServerConfiguration::add_wayland_extension*; non-virtual?thunk?to?mir::DefaultServerConfiguration::set_enabled_wayland_extensions*; + non-virtual?thunk?to?mir::DefaultServerConfiguration::set_the_decoration_strategy*; non-virtual?thunk?to?mir::DefaultServerConfiguration::set_wayland_extension_filter*; non-virtual?thunk?to?mir::DefaultServerConfiguration::the_application_not_responding_detector*; non-virtual?thunk?to?mir::DefaultServerConfiguration::the_buffer_allocator*; @@ -683,6 +690,7 @@ global: non-virtual?thunk?to?mir::DefaultServerConfiguration::the_cursor_images*; non-virtual?thunk?to?mir::DefaultServerConfiguration::the_cursor_listener*; non-virtual?thunk?to?mir::DefaultServerConfiguration::the_decoration_manager*; + non-virtual?thunk?to?mir::DefaultServerConfiguration::the_decoration_strategy*; non-virtual?thunk?to?mir::DefaultServerConfiguration::the_default_cursor_image*; non-virtual?thunk?to?mir::DefaultServerConfiguration::the_display*; non-virtual?thunk?to?mir::DefaultServerConfiguration::the_display_buffer_compositor_factory*; @@ -1026,6 +1034,7 @@ global: non-virtual?thunk?to?mir::time::AlarmFactory::?AlarmFactory*; std::hash::operator*; typeinfo?for?mir::BasicCallback; + typeinfo?for?mir::DecorationStrategy; typeinfo?for?mir::DefaultServerConfiguration; typeinfo?for?mir::DefaultServerStatusListener; typeinfo?for?mir::DisplayChanger; @@ -1160,10 +1169,12 @@ global: typeinfo?for?std::hash; virtual?thunk?to?mir::DefaultServerConfiguration::add_wayland_extension*; virtual?thunk?to?mir::DefaultServerConfiguration::set_enabled_wayland_extensions*; + virtual?thunk?to?mir::DefaultServerConfiguration::set_the_decoration_strategy*; virtual?thunk?to?mir::DefaultServerConfiguration::set_wayland_extension_filter*; virtual?thunk?to?mir::DefaultServerConfiguration::the_application_not_responding_detector*; virtual?thunk?to?mir::DefaultServerConfiguration::the_compositor*; virtual?thunk?to?mir::DefaultServerConfiguration::the_console_services*; + virtual?thunk?to?mir::DefaultServerConfiguration::the_decoration_strategy*; virtual?thunk?to?mir::DefaultServerConfiguration::the_display*; virtual?thunk?to?mir::DefaultServerConfiguration::the_display_changer*; virtual?thunk?to?mir::DefaultServerConfiguration::the_display_platforms*; @@ -1229,6 +1240,7 @@ global: virtual?thunk?to?mir::shell::ShellWrapper::surface_at*; virtual?thunk?to?mir::shell::ShellWrapper::swap_z_order*; vtable?for?mir::BasicCallback; + vtable?for?mir::DecorationStrategy; vtable?for?mir::DefaultServerConfiguration; vtable?for?mir::DefaultServerStatusListener; vtable?for?mir::DisplayChanger; From e0ca81e4820bae00e520cffadd5da079577fd9ce Mon Sep 17 00:00:00 2001 From: Tarek Yasser Date: Mon, 1 Jul 2024 16:34:44 +0300 Subject: [PATCH 28/81] Fix lifetime issues related to `toplevels_with_decorations`. --- .../xdg_decoration_unstable_v1.cpp | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 2fcfa88a087..6456f7782dc 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -36,25 +36,25 @@ class ToplevelsWithDecorations { public: ToplevelsWithDecorations() : - toplevels_with_decorations{std::make_shared>()} + toplevels_with_decorations{std::unordered_set()} { } /// \return true if no duplicates existed before insertion, false otherwise. bool registerToplevel(wl_resource* toplevel) { - auto [_, inserted] = toplevels_with_decorations->insert(toplevel); + auto [_, inserted] = toplevels_with_decorations.insert(toplevel); return inserted; } /// \return true if only one element was erased, false otherwise. bool unregisterToplevel(wl_resource* toplevel) { - return toplevels_with_decorations->erase(toplevel) == 1; + return toplevels_with_decorations.erase(toplevel) == 1; } private: - std::shared_ptr> toplevels_with_decorations; + std::unordered_set toplevels_with_decorations; }; class XdgDecorationManagerV1 : public wayland::XdgDecorationManagerV1 @@ -73,7 +73,7 @@ class XdgDecorationManagerV1 : public wayland::XdgDecorationManagerV1 private: void get_toplevel_decoration(wl_resource* id, wl_resource* toplevel) override; - ToplevelsWithDecorations toplevels_with_decorations; + std::shared_ptr const toplevels_with_decorations; }; class XdgToplevelDecorationV1 : public wayland::XdgToplevelDecorationV1 @@ -113,7 +113,7 @@ void mir::frontend::XdgDecorationManagerV1::Global::bind(wl_resource* new_zxdg_d mir::frontend::XdgDecorationManagerV1::XdgDecorationManagerV1(wl_resource* resource) : mir::wayland::XdgDecorationManagerV1{resource, Version<1>{}}, - toplevels_with_decorations{} + toplevels_with_decorations{std::make_shared()} { } @@ -128,22 +128,22 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* } auto decoration = new XdgToplevelDecorationV1{id, tl}; - if (!toplevels_with_decorations.registerToplevel(toplevel)) + if (!toplevels_with_decorations->registerToplevel(toplevel)) { BOOST_THROW_EXCEPTION(mir::wayland::ProtocolError( resource, Error::already_constructed, "Decoration already constructed for this toplevel")); } decoration->add_destroy_listener( - [&toplevels_with_decorations = this->toplevels_with_decorations, toplevel]() + [toplevels_with_decorations = this->toplevels_with_decorations, toplevel]() { - toplevels_with_decorations.unregisterToplevel(toplevel); + toplevels_with_decorations->unregisterToplevel(toplevel); }); tl->add_destroy_listener( - [&toplevels_with_decorations = this->toplevels_with_decorations, client = this->client, toplevel]() + [toplevels_with_decorations = this->toplevels_with_decorations, client = this->client, toplevel]() { - if (!client->is_being_destroyed() && !toplevels_with_decorations.unregisterToplevel(toplevel)) + if (!client->is_being_destroyed() && !toplevels_with_decorations->unregisterToplevel(toplevel)) { mir::log_warning("Toplevel destroyed before attached decoration!"); // https://github.com/canonical/mir/issues/3452 From 2012b20c4a1ce7e2ecb08a37bad10c14eb01c2f2 Mon Sep 17 00:00:00 2001 From: Tarek Yasser Date: Mon, 1 Jul 2024 16:35:58 +0300 Subject: [PATCH 29/81] Fix naming convention of `{register,unregister}Toplevel`. --- .../frontend_wayland/xdg_decoration_unstable_v1.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 6456f7782dc..c1e142d6f33 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -41,14 +41,14 @@ class ToplevelsWithDecorations } /// \return true if no duplicates existed before insertion, false otherwise. - bool registerToplevel(wl_resource* toplevel) + bool register_toplevel(wl_resource* toplevel) { auto [_, inserted] = toplevels_with_decorations.insert(toplevel); return inserted; } /// \return true if only one element was erased, false otherwise. - bool unregisterToplevel(wl_resource* toplevel) + bool unregister_toplevel(wl_resource* toplevel) { return toplevels_with_decorations.erase(toplevel) == 1; } @@ -128,7 +128,7 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* } auto decoration = new XdgToplevelDecorationV1{id, tl}; - if (!toplevels_with_decorations->registerToplevel(toplevel)) + if (!toplevels_with_decorations->register_toplevel(toplevel)) { BOOST_THROW_EXCEPTION(mir::wayland::ProtocolError( resource, Error::already_constructed, "Decoration already constructed for this toplevel")); @@ -137,13 +137,13 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* decoration->add_destroy_listener( [toplevels_with_decorations = this->toplevels_with_decorations, toplevel]() { - toplevels_with_decorations->unregisterToplevel(toplevel); + toplevels_with_decorations->unregister_toplevel(toplevel); }); tl->add_destroy_listener( [toplevels_with_decorations = this->toplevels_with_decorations, client = this->client, toplevel]() { - if (!client->is_being_destroyed() && !toplevels_with_decorations->unregisterToplevel(toplevel)) + if (!client->is_being_destroyed() && !toplevels_with_decorations->unregister_toplevel(toplevel)) { mir::log_warning("Toplevel destroyed before attached decoration!"); // https://github.com/canonical/mir/issues/3452 From 6faabb42dd58e5842d20434084cc9405da6e8469 Mon Sep 17 00:00:00 2001 From: tarek-y-ismail Date: Mon, 1 Jul 2024 17:33:20 +0300 Subject: [PATCH 30/81] Remove redundant `ToplevelsWithDecorations` construction code and disable copying. Co-authored-by: Alan Griffiths --- src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index c1e142d6f33..9b470521282 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -35,10 +35,9 @@ namespace frontend class ToplevelsWithDecorations { public: - ToplevelsWithDecorations() : - toplevels_with_decorations{std::unordered_set()} - { - } + ToplevelsWithDecorations() = default; + ToplevelsWithDecorations(ToplevelsWithDecorations const&) = delete; + ToplevelsWithDecorations& operator=(ToplevelsWithDecorations const&) = delete; /// \return true if no duplicates existed before insertion, false otherwise. bool register_toplevel(wl_resource* toplevel) From e69e676db97ab8d7521e456dc4b8bea91035a991 Mon Sep 17 00:00:00 2001 From: Tarek Yasser Date: Mon, 1 Jul 2024 17:38:41 +0300 Subject: [PATCH 31/81] Always unregister toplevel on destruction. Also fixed a sneaky bug caused by checking if one toplevel was registered instead of zero. toplevels should be already unregistered by their decorations by the time they're destroyed, otherwise it's a protocol error. --- .../frontend_wayland/xdg_decoration_unstable_v1.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 9b470521282..fec77bd74a1 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -46,10 +46,13 @@ class ToplevelsWithDecorations return inserted; } - /// \return true if only one element was erased, false otherwise. + /// \return true if the toplevel didn't exist in the set, false otherwise. + /// The return value is used in the destroy callback of the toplevel to + /// check if the toplevel is destroyed before the decoration (orphaned + /// decoration). bool unregister_toplevel(wl_resource* toplevel) { - return toplevels_with_decorations.erase(toplevel) == 1; + return toplevels_with_decorations.erase(toplevel) == 0; } private: @@ -142,7 +145,8 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* tl->add_destroy_listener( [toplevels_with_decorations = this->toplevels_with_decorations, client = this->client, toplevel]() { - if (!client->is_being_destroyed() && !toplevels_with_decorations->unregister_toplevel(toplevel)) + const auto orphaned_decoration = !toplevels_with_decorations->unregister_toplevel(toplevel); + if (!client->is_being_destroyed() && orphaned_decoration) { mir::log_warning("Toplevel destroyed before attached decoration!"); // https://github.com/canonical/mir/issues/3452 From bb4bca604fca9f2806a1a9ca18241148381b531f Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Mon, 1 Jul 2024 17:12:45 +0100 Subject: [PATCH 32/81] Avoid restoring the context being destroyed --- src/platforms/wayland/wl_egl_display_provider.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/platforms/wayland/wl_egl_display_provider.cpp b/src/platforms/wayland/wl_egl_display_provider.cpp index 312d4a851f0..8078eca714e 100644 --- a/src/platforms/wayland/wl_egl_display_provider.cpp +++ b/src/platforms/wayland/wl_egl_display_provider.cpp @@ -88,9 +88,10 @@ class mgw::WlDisplayAllocator::Framebuffer::EGLState ~EGLState() { - CacheEglState stash; - - eglMakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT); + if (ctx == eglGetCurrentContext()) + { + eglMakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT); + } eglDestroyContext(dpy, ctx); } From 51f5fb331439c382989082115dbd407d333fe274 Mon Sep 17 00:00:00 2001 From: tarek-y-ismail Date: Tue, 2 Jul 2024 09:36:29 +0300 Subject: [PATCH 33/81] Update naming of protocol mode and decorations type conversion functions Co-authored-by: Alan Griffiths --- src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index b3534e1e320..2210368226d 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -91,8 +91,8 @@ class XdgToplevelDecorationV1 : public wayland::XdgToplevelDecorationV1 void unset_mode() override; private: - auto decorations_type_to_protocol_mode(DecorationStrategy::DecorationsType) -> uint32_t; - auto protocol_mode_to_decorations_type(uint32_t) -> DecorationStrategy::DecorationsType; + static auto to_mode(DecorationStrategy::DecorationsType) -> uint32_t; + static auto to_decorations_type(uint32_t) -> DecorationStrategy::DecorationsType; void update_mode(uint32_t new_mode); mir::frontend::XdgToplevelStable* toplevel; From 6025dfc36370520763f421835df994982cc46134 Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Tue, 2 Jul 2024 09:41:29 +0300 Subject: [PATCH 34/81] Fix naming for function definitions --- .../frontend_wayland/xdg_decoration_unstable_v1.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 2210368226d..26041529eb0 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -171,7 +171,7 @@ mir::frontend::XdgToplevelDecorationV1::XdgToplevelDecorationV1( { } -auto mir::frontend::XdgToplevelDecorationV1::decorations_type_to_protocol_mode(DecorationStrategy::DecorationsType type) +auto mir::frontend::XdgToplevelDecorationV1::to_mode(DecorationStrategy::DecorationsType type) -> uint32_t { switch (type) @@ -186,7 +186,7 @@ auto mir::frontend::XdgToplevelDecorationV1::decorations_type_to_protocol_mode(D } } -auto mir::frontend::XdgToplevelDecorationV1::protocol_mode_to_decorations_type(uint32_t mode) +auto mir::frontend::XdgToplevelDecorationV1::to_decorations_type(uint32_t mode) -> DecorationStrategy::DecorationsType { switch (mode) @@ -205,7 +205,7 @@ void mir::frontend::XdgToplevelDecorationV1::update_mode(uint32_t new_mode) { auto spec = shell::SurfaceSpecification{}; - auto const new_type = decoration_strategy->request_style(protocol_mode_to_decorations_type(new_mode)); + auto const new_type = decoration_strategy->request_style(to_decorations_type(new_mode)); switch (new_type) { @@ -219,7 +219,7 @@ void mir::frontend::XdgToplevelDecorationV1::update_mode(uint32_t new_mode) this->toplevel->apply_spec(spec); - auto const strategy_new_mode = decorations_type_to_protocol_mode(new_type); + auto const strategy_new_mode = to_mode(new_type); send_configure_event(strategy_new_mode); } @@ -230,6 +230,6 @@ void mir::frontend::XdgToplevelDecorationV1::set_mode(uint32_t mode) void mir::frontend::XdgToplevelDecorationV1::unset_mode() { - auto const protocol_mode = decorations_type_to_protocol_mode(decoration_strategy->default_style()); + auto const protocol_mode = to_mode(decoration_strategy->default_style()); update_mode(protocol_mode); } From fe33489b63a8af5fbf34bd571396e50a24e5c843 Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Tue, 2 Jul 2024 10:02:47 +0300 Subject: [PATCH 35/81] Make `decoration_strategy` const and move more instead of copying. --- .../xdg_decoration_unstable_v1.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 26041529eb0..ca9b3d908e7 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -70,7 +70,7 @@ class XdgDecorationManagerV1 : public wayland::XdgDecorationManagerV1 Global(wl_display* display, std::shared_ptr strategy); private: - std::shared_ptr decoration_strategy; + std::shared_ptr const decoration_strategy; void bind(wl_resource* new_zxdg_decoration_manager_v1) override; }; @@ -78,7 +78,7 @@ class XdgDecorationManagerV1 : public wayland::XdgDecorationManagerV1 void get_toplevel_decoration(wl_resource* id, wl_resource* toplevel) override; ToplevelsWithDecorations toplevels_with_decorations; - std::shared_ptr decoration_strategy; + std::shared_ptr const decoration_strategy; }; class XdgToplevelDecorationV1 : public wayland::XdgToplevelDecorationV1 @@ -96,7 +96,7 @@ class XdgToplevelDecorationV1 : public wayland::XdgToplevelDecorationV1 void update_mode(uint32_t new_mode); mir::frontend::XdgToplevelStable* toplevel; - std::shared_ptr decoration_strategy; + std::shared_ptr const decoration_strategy; }; } // namespace frontend } // namespace mir @@ -104,26 +104,26 @@ class XdgToplevelDecorationV1 : public wayland::XdgToplevelDecorationV1 auto mir::frontend::create_xdg_decoration_unstable_v1(wl_display* display, std::shared_ptr strategy) -> std::shared_ptr { - return std::make_shared(display, strategy); + return std::make_shared(display, std::move(strategy)); } mir::frontend::XdgDecorationManagerV1::Global::Global( wl_display* display, std::shared_ptr strategy) : wayland::XdgDecorationManagerV1::Global::Global{display, Version<1>{}}, - decoration_strategy{strategy} + decoration_strategy{std::move(strategy)} { } void mir::frontend::XdgDecorationManagerV1::Global::bind(wl_resource* new_zxdg_decoration_manager_v1) { - new XdgDecorationManagerV1{new_zxdg_decoration_manager_v1, decoration_strategy}; + new XdgDecorationManagerV1{new_zxdg_decoration_manager_v1, std::move(decoration_strategy)}; } mir::frontend::XdgDecorationManagerV1::XdgDecorationManagerV1( wl_resource* resource, std::shared_ptr strategy) : mir::wayland::XdgDecorationManagerV1{resource, Version<1>{}}, toplevels_with_decorations{}, - decoration_strategy{strategy} + decoration_strategy{std::move(strategy)} { } @@ -137,7 +137,7 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* BOOST_THROW_EXCEPTION(std::runtime_error("Invalid toplevel pointer")); } - auto decoration = new XdgToplevelDecorationV1{id, tl, decoration_strategy}; + auto decoration = new XdgToplevelDecorationV1{id, tl, std::move(decoration_strategy)}; if (!toplevels_with_decorations.registerToplevel(toplevel)) { BOOST_THROW_EXCEPTION(mir::wayland::ProtocolError( @@ -167,7 +167,7 @@ mir::frontend::XdgToplevelDecorationV1::XdgToplevelDecorationV1( wl_resource* id, mir::frontend::XdgToplevelStable* toplevel, std::shared_ptr strategy) : wayland::XdgToplevelDecorationV1{id, Version<1>{}}, toplevel{toplevel}, - decoration_strategy{strategy} + decoration_strategy{std::move(strategy)} { } From bd287f80d344a38f500bdad8d391b880ba102dc1 Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Tue, 2 Jul 2024 10:12:42 +0300 Subject: [PATCH 36/81] Remove unnecessary default case. --- src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index ca9b3d908e7..87f955253ce 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -29,6 +29,7 @@ #include #include #include +#include namespace mir { @@ -180,10 +181,9 @@ auto mir::frontend::XdgToplevelDecorationV1::to_mode(DecorationStrategy::Decorat return Mode::server_side; case DecorationStrategy::DecorationsType::csd: return Mode::client_side; - default: - mir::log_warning("%s: Got invalid protocol decorations type, defaulting to client side", __FUNCTION__); - return Mode::client_side; } + + std::unreachable(); } auto mir::frontend::XdgToplevelDecorationV1::to_decorations_type(uint32_t mode) From f30374f663d536e9abeefdfefdef6dfce4901dab Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Tue, 2 Jul 2024 10:51:04 +0300 Subject: [PATCH 37/81] Make `to_decorations_type` a method and improve logging on invalid values --- .../frontend_wayland/xdg_decoration_unstable_v1.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 87f955253ce..200d151b52e 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -93,7 +93,7 @@ class XdgToplevelDecorationV1 : public wayland::XdgToplevelDecorationV1 private: static auto to_mode(DecorationStrategy::DecorationsType) -> uint32_t; - static auto to_decorations_type(uint32_t) -> DecorationStrategy::DecorationsType; + auto to_decorations_type(uint32_t) -> DecorationStrategy::DecorationsType; void update_mode(uint32_t new_mode); mir::frontend::XdgToplevelStable* toplevel; @@ -196,7 +196,11 @@ auto mir::frontend::XdgToplevelDecorationV1::to_decorations_type(uint32_t mode) case Mode::server_side: return DecorationStrategy::DecorationsType::ssd; default: - mir::log_warning("%s: Got invalid protocol mode (%d), defaulting to client side", __FUNCTION__, mode); + pid_t pid; + wl_client_get_credentials(client->raw_client(), &pid, nullptr, nullptr); // null pointers are allowed + + mir::log_warning("Client PID: %d, got invalid protocol mode (%d), defaulting to client side.", pid, mode); + return DecorationStrategy::DecorationsType::csd; } } From 6469f70cec29abbd0652cbd53725379d2d76c1c7 Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Tue, 2 Jul 2024 10:52:52 +0300 Subject: [PATCH 38/81] Drop the base for `mir::DecorationStrategy::DecorationsType` --- src/include/server/mir/decoration_strategy.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/include/server/mir/decoration_strategy.h b/src/include/server/mir/decoration_strategy.h index b63a628eff2..42843e61a52 100644 --- a/src/include/server/mir/decoration_strategy.h +++ b/src/include/server/mir/decoration_strategy.h @@ -14,13 +14,12 @@ * along with this program. If not, see . */ -#include namespace mir { class DecorationStrategy { public: - enum class DecorationsType : uint32_t + enum class DecorationsType { csd, ssd From 9225da04f2f824f8ea3b7d69c5fd8009d442dc52 Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Tue, 2 Jul 2024 11:04:13 +0300 Subject: [PATCH 39/81] Specify default constructor, disable copy constructor/operator in `DecorationStrategy` --- src/include/server/mir/decoration_strategy.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/include/server/mir/decoration_strategy.h b/src/include/server/mir/decoration_strategy.h index 42843e61a52..e43df57536a 100644 --- a/src/include/server/mir/decoration_strategy.h +++ b/src/include/server/mir/decoration_strategy.h @@ -25,7 +25,11 @@ class DecorationStrategy ssd }; + DecorationStrategy() = default; + DecorationStrategy(DecorationStrategy const&) = delete; + DecorationStrategy& operator=(DecorationStrategy const&) = delete; virtual ~DecorationStrategy() = default; + virtual auto default_style() const -> DecorationsType = 0; virtual auto request_style(DecorationsType type) const -> DecorationsType = 0; }; From c6952223202a5ac485d4d399609d36d0db997c0d Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Tue, 2 Jul 2024 11:10:15 +0300 Subject: [PATCH 40/81] Revert unnecessary changes done to `src/core/symbols.map` --- src/core/symbols.map | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/symbols.map b/src/core/symbols.map index d3675afa067..b1e30db825e 100644 --- a/src/core/symbols.map +++ b/src/core/symbols.map @@ -1,5 +1,5 @@ MIR_CORE_2.9 { -global: + global: extern "C++" { mir::AnonymousShmFile::?AnonymousShmFile*; mir::AnonymousShmFile::AnonymousShmFile*; @@ -29,4 +29,3 @@ global: }; local: *; }; - From 1af2a26fb197f6095f86a6a8ec0a852e79a039cc Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Tue, 2 Jul 2024 12:20:32 +0300 Subject: [PATCH 41/81] Improve semantics of `unregister_toplevel` and explain decoration orphaning --- .../xdg_decoration_unstable_v1.cpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index fec77bd74a1..d59ee422217 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -46,13 +46,10 @@ class ToplevelsWithDecorations return inserted; } - /// \return true if the toplevel didn't exist in the set, false otherwise. - /// The return value is used in the destroy callback of the toplevel to - /// check if the toplevel is destroyed before the decoration (orphaned - /// decoration). + /// \return true if the toplevel was still registered, false otherwise. bool unregister_toplevel(wl_resource* toplevel) { - return toplevels_with_decorations.erase(toplevel) == 0; + return toplevels_with_decorations.erase(toplevel) > 0; } private: @@ -145,7 +142,15 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* tl->add_destroy_listener( [toplevels_with_decorations = this->toplevels_with_decorations, client = this->client, toplevel]() { - const auto orphaned_decoration = !toplevels_with_decorations->unregister_toplevel(toplevel); + // Under normal conditions, decorations should be destroyed before + // toplevels. Causing `unregister_toplevel` to return false. + // + // If the attached decoration is not destroyed before its toplevel, + // then its a protocol error. This can happen in two cases: A + // protocol violation caused by the client, or another error + // triggering wayland cleanup code which destroys wayland objects + // with no guaranteed order. + const auto orphaned_decoration = toplevels_with_decorations->unregister_toplevel(toplevel); if (!client->is_being_destroyed() && orphaned_decoration) { mir::log_warning("Toplevel destroyed before attached decoration!"); From 3a75b0f4b725b55a837ffa4b41a48b5d91fdc672 Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Tue, 2 Jul 2024 13:16:38 +0300 Subject: [PATCH 42/81] Remove incorrect use of `std::move` in `get_toplevel_decoration` --- src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 200d151b52e..88720166800 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -138,7 +138,7 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* BOOST_THROW_EXCEPTION(std::runtime_error("Invalid toplevel pointer")); } - auto decoration = new XdgToplevelDecorationV1{id, tl, std::move(decoration_strategy)}; + auto decoration = new XdgToplevelDecorationV1{id, tl, decoration_strategy}; if (!toplevels_with_decorations.registerToplevel(toplevel)) { BOOST_THROW_EXCEPTION(mir::wayland::ProtocolError( From faa1e1619fca25fa8e79fb45a9c3ddbb477ea867 Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Tue, 2 Jul 2024 14:54:40 +0300 Subject: [PATCH 43/81] Rename `destroy_toplevel_before_decoration` to `destroy_toplevel_before_decoration_throws_orphaned` --- tests/acceptance-tests/wayland/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/acceptance-tests/wayland/CMakeLists.txt b/tests/acceptance-tests/wayland/CMakeLists.txt index 493b3abb4f5..c12b7a5e291 100644 --- a/tests/acceptance-tests/wayland/CMakeLists.txt +++ b/tests/acceptance-tests/wayland/CMakeLists.txt @@ -116,7 +116,7 @@ set(EXPECTED_FAILURES # Should report an `orphaned` protocol error/violation , but we log instead # because of https://github.com/canonical/mir/issues/3452 - XdgDecorationV1Test.destroy_toplevel_before_decoration + XdgDecorationV1Test.destroying_toplevel_before_decoration_throws_orphaned ) if (MIR_SIGBUS_HANDLER_ENVIRONMENT_BROKEN) From 10a2773df313dd4d4a54d48e31d8225692a1b19c Mon Sep 17 00:00:00 2001 From: tarek-y-ismail Date: Tue, 2 Jul 2024 15:01:12 +0300 Subject: [PATCH 44/81] Fix incorrect curly brace placement in `the_decoration_strategy` Co-authored-by: Alan Griffiths --- src/server/default_server_configuration.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/server/default_server_configuration.cpp b/src/server/default_server_configuration.cpp index 09d4a5fc138..6f6ce12ed03 100644 --- a/src/server/default_server_configuration.cpp +++ b/src/server/default_server_configuration.cpp @@ -202,7 +202,8 @@ auto mir::DefaultServerConfiguration::the_logger() auto mir::DefaultServerConfiguration::the_decoration_strategy() -> std::shared_ptr { - if(!decoration_strategy) { + if (!decoration_strategy) + { class DefaultDecorationStrategy: public mir::DecorationStrategy { DecorationsType default_style() const override { return DecorationsType::csd; } From dcc38ab81173b626071edcbfa28b67b7082dbcb0 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Tue, 2 Jul 2024 08:25:34 -0400 Subject: [PATCH 45/81] 11;rgb:1717/1414/2121bugfix: BasicScreenShooter now selects a GLRenderingProvider based off of its compatibility with the BufferAllocator --- .../compositor/basic_screen_shooter.cpp | 24 ++++++++++++++----- src/server/compositor/basic_screen_shooter.h | 5 +++- .../compositor/default_configuration.cpp | 1 + .../compositor/test_basic_screen_shooter.cpp | 6 +++++ 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/server/compositor/basic_screen_shooter.cpp b/src/server/compositor/basic_screen_shooter.cpp index 4353648b9a6..570c75d69c7 100644 --- a/src/server/compositor/basic_screen_shooter.cpp +++ b/src/server/compositor/basic_screen_shooter.cpp @@ -217,13 +217,16 @@ auto mc::BasicScreenShooter::Self::renderer_for_buffer(std::shared_ptr> const& providers) + std::span> const& providers, + std::shared_ptr const& buffer_allocator) -> std::shared_ptr { auto display_provider = std::make_shared(); OffscreenDisplaySink temp_db{display_provider, geom::Size{640, 480}}; - for (auto const& render_provider : providers) + std::pair> best_provider = std::make_pair( + mg::probe::unsupported, nullptr); + for (auto const& provider: providers) { /* TODO: There might be more sensible ways to select a provider * (one in use by at least one DisplaySink, the only one in use, the lowest-powered one,...) @@ -231,12 +234,20 @@ auto mc::BasicScreenShooter::select_provider( * * For now, just use the first that claims to work. */ - if (render_provider->suitability_for_display(temp_db) >= mg::probe::supported) + auto suitability = provider->suitability_for_display(temp_db); + // We also need to make sure that the GLRenderingProvider can access client buffers... + if (provider->suitability_for_allocator(buffer_allocator) > mg::probe::unsupported && + suitability > best_provider.first) { - return render_provider; + best_provider = std::make_pair(suitability, provider); } } - BOOST_THROW_EXCEPTION((std::runtime_error{"No rendering provider claims to support a CPU addressable target"})); + if (best_provider.first == mg::probe::unsupported) + { + BOOST_THROW_EXCEPTION((std::runtime_error{"No rendering provider claims to support a CPU addressable target"})); + } + + return best_provider.second; } mc::BasicScreenShooter::BasicScreenShooter( @@ -245,8 +256,9 @@ mc::BasicScreenShooter::BasicScreenShooter( Executor& executor, std::span> const& providers, std::shared_ptr render_factory, + std::shared_ptr const& buffer_allocator, std::shared_ptr const& config) - : self{std::make_shared(scene, clock, select_provider(providers), std::move(render_factory), config)}, + : self{std::make_shared(scene, clock, select_provider(providers, buffer_allocator), std::move(render_factory), config)}, executor{executor} { } diff --git a/src/server/compositor/basic_screen_shooter.h b/src/server/compositor/basic_screen_shooter.h index 77392338fb3..48ae530525f 100644 --- a/src/server/compositor/basic_screen_shooter.h +++ b/src/server/compositor/basic_screen_shooter.h @@ -46,6 +46,7 @@ class BasicScreenShooter: public ScreenShooter Executor& executor, std::span> const& providers, std::shared_ptr render_factory, + std::shared_ptr const& buffer_allocator, std::shared_ptr const& config); void capture( @@ -92,7 +93,9 @@ class BasicScreenShooter: public ScreenShooter std::shared_ptr const self; Executor& executor; - static auto select_provider(std::span> const& providers) + static auto select_provider( + std::span> const& providers, + std::shared_ptr const& buffer_allocator) -> std::shared_ptr; }; } diff --git a/src/server/compositor/default_configuration.cpp b/src/server/compositor/default_configuration.cpp index 5d00a8f75a1..09ec094f863 100644 --- a/src/server/compositor/default_configuration.cpp +++ b/src/server/compositor/default_configuration.cpp @@ -120,6 +120,7 @@ auto mir::DefaultServerConfiguration::the_screen_shooter() -> std::shared_ptr @@ -98,6 +99,8 @@ struct BasicScreenShooter : Test } BOOST_THROW_EXCEPTION((std::runtime_error{"CPU output support not available?!"})); }); + ON_CALL(*gl_provider, suitability_for_allocator(_)) + .WillByDefault(Return(mg::probe::supported)); ON_CALL(*gl_provider, suitability_for_display(_)) .WillByDefault(Return(mg::probe::supported)); @@ -107,6 +110,7 @@ struct BasicScreenShooter : Test executor, gl_providers, renderer_factory, + buffer_allocator, std::make_shared()); } @@ -135,6 +139,7 @@ struct BasicScreenShooter : Test std::shared_ptr gl_provider{std::make_shared>()}; std::vector> gl_providers{gl_provider}; std::shared_ptr renderer_factory{std::make_shared>()}; + std::shared_ptr buffer_allocator; std::shared_ptr clock{std::make_shared()}; mtd::ExplicitExecutor executor; std::unique_ptr shooter; @@ -249,6 +254,7 @@ TEST_F(BasicScreenShooter, ensures_renderer_is_current_on_only_one_thread) mir::thread_pool_executor, gl_providers, renderer_factory, + buffer_allocator, std::make_shared()); ON_CALL(*next_renderer, render(_)) From d1a8a680fabdf2b507aa373539efde78ba22310d Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Tue, 2 Jul 2024 16:56:08 +0300 Subject: [PATCH 46/81] Implement some decoration strategies on the miral side. --- .../core}/mir/decoration_strategy.h | 0 include/miral/miral/decorations.h | 62 ++++++++++++++ src/miral/CMakeLists.txt | 1 + src/miral/decorations.cpp | 85 +++++++++++++++++++ src/miral/symbols.map | 16 ++++ src/server/server.cpp | 6 ++ src/server/symbols.map | 2 + 7 files changed, 172 insertions(+) rename {src/include/server => include/core}/mir/decoration_strategy.h (100%) create mode 100644 include/miral/miral/decorations.h create mode 100644 src/miral/decorations.cpp diff --git a/src/include/server/mir/decoration_strategy.h b/include/core/mir/decoration_strategy.h similarity index 100% rename from src/include/server/mir/decoration_strategy.h rename to include/core/mir/decoration_strategy.h diff --git a/include/miral/miral/decorations.h b/include/miral/miral/decorations.h new file mode 100644 index 00000000000..e31d6eda0e3 --- /dev/null +++ b/include/miral/miral/decorations.h @@ -0,0 +1,62 @@ +/* + * Copyright © Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 or 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef MIRAL_DECORATIONS_H +#define MIRAL_DECORATIONS_H + + +#include "mir/decoration_strategy.h" + +#include + +namespace mir { class Server; } + +namespace miral +{ +class Decorations +{ +public: + Decorations() = delete; + Decorations(Decorations const&) = default; + auto operator=(Decorations const&) -> Decorations& = default; + + void operator()(mir::Server&) const; + + /// Always use server side decorations regardless of the client's choice + static auto always_ssd() -> Decorations; + + /// Always use client side decorations regardless of the client's choice + static auto always_csd() -> Decorations; + + /// Prefer server side decorations if the client does not set a specific + /// mode. Otherwise use the mode specified by the client. + static auto prefer_ssd() -> Decorations; + + /// Prefer client side decorations if the client does not set a specific + /// mode. Otherwise use the mode specified by the client. + static auto prefer_csd() -> Decorations; + +private: + struct Self; + + Decorations(std::shared_ptr strategy); + + std::shared_ptr self; +}; +} + +#endif // MIRAL_DECORATIONS_H + diff --git a/src/miral/CMakeLists.txt b/src/miral/CMakeLists.txt index b984ce51ef8..ac937e39142 100644 --- a/src/miral/CMakeLists.txt +++ b/src/miral/CMakeLists.txt @@ -82,6 +82,7 @@ add_library(miral SHARED ${miral_include}/miral/lambda_as_function.h x11_support.cpp ${miral_include}/miral/x11_support.h zone.cpp ${miral_include}/miral/zone.h + decorations.cpp ${miral_include}/miral/decorations.h ) #include diff --git a/src/miral/decorations.cpp b/src/miral/decorations.cpp new file mode 100644 index 00000000000..423e0b2d7c1 --- /dev/null +++ b/src/miral/decorations.cpp @@ -0,0 +1,85 @@ +/* + * Copyright © Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 or 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "miral/decorations.h" + +#include +#include +#include + + +struct miral::Decorations::Self +{ + std::shared_ptr strategy; +}; + +void miral::Decorations::operator()(mir::Server& server) const +{ + server.add_pre_init_callback( + [this, &server]() + { + server.set_the_decoration_strategy(self->strategy); + }); +} + +miral::Decorations::Decorations(std::shared_ptr strategy) : + self{std::make_shared(std::move(strategy))} +{ +} + +auto miral::Decorations::always_ssd() -> Decorations +{ + struct AlwaysServerSide : public mir::DecorationStrategy + { + DecorationsType default_style() const override { return DecorationsType::ssd; } + DecorationsType request_style(DecorationsType) const override { return DecorationsType::ssd; } + }; + + return Decorations(std::make_shared()); +} + +auto miral::Decorations::always_csd() -> Decorations +{ + struct AlwaysClientSide : public mir::DecorationStrategy + { + DecorationsType default_style() const override { return DecorationsType::csd; } + DecorationsType request_style(DecorationsType) const override { return DecorationsType::csd; } + }; + + return Decorations(std::make_shared()); +} + +auto miral::Decorations::prefer_ssd() -> Decorations +{ + struct PreferServerSide : public mir::DecorationStrategy + { + DecorationsType default_style() const override { return DecorationsType::ssd; } + DecorationsType request_style(DecorationsType type) const override { return type; } + }; + + return Decorations(std::make_shared()); +} + +auto miral::Decorations::prefer_csd() -> Decorations +{ + struct PreferClientSide : public mir::DecorationStrategy + { + DecorationsType default_style() const override { return DecorationsType::csd; } + DecorationsType request_style(DecorationsType type) const override { return type; } + }; + + return Decorations(std::make_shared()); +} diff --git a/src/miral/symbols.map b/src/miral/symbols.map index 8b3aa2b4877..22bcc1bbd3d 100644 --- a/src/miral/symbols.map +++ b/src/miral/symbols.map @@ -41,6 +41,12 @@ global: miral::CustomRenderer::?CustomRenderer*; miral::CustomRenderer::CustomRenderer*; miral::CustomRenderer::operator*; + miral::Decorations::Decorations*; + miral::Decorations::always_csd*; + miral::Decorations::always_ssd*; + miral::Decorations::operator*; + miral::Decorations::prefer_csd*; + miral::Decorations::prefer_ssd*; miral::DisplayConfiguration::?DisplayConfiguration*; miral::DisplayConfiguration::DisplayConfiguration*; miral::DisplayConfiguration::add_output_attribute*; @@ -312,6 +318,13 @@ global: miral::X11Support::X11Support*; miral::X11Support::default_to_enabled*; miral::X11Support::operator*; + miral::XdgDecorations::?XdgDecorations*; + miral::XdgDecorations::XdgDecorations*; + miral::XdgDecorations::always_csd*; + miral::XdgDecorations::always_ssd*; + miral::XdgDecorations::operator*; + miral::XdgDecorations::prefer_csd*; + miral::XdgDecorations::prefer_ssd*; miral::Zone::?Zone*; miral::Zone::Zone*; miral::Zone::extents*; @@ -415,6 +428,7 @@ global: typeinfo?for?miral::ConfigurationOption; typeinfo?for?miral::CursorTheme; typeinfo?for?miral::CustomRenderer; + typeinfo?for?miral::Decorations; typeinfo?for?miral::DisplayConfiguration; typeinfo?for?miral::ExternalClientLauncher; typeinfo?for?miral::FdHandle; @@ -443,6 +457,7 @@ global: typeinfo?for?miral::WindowSpecification::AspectRatio; typeinfo?for?miral::WindowSpecification; typeinfo?for?miral::X11Support; + typeinfo?for?miral::XdgDecorations; typeinfo?for?miral::Zone; vtable?for?miral::ApplicationAuthorizer; vtable?for?miral::CanonicalWindowManagerPolicy; @@ -452,3 +467,4 @@ global: }; local: *; }; + diff --git a/src/server/server.cpp b/src/server/server.cpp index 5131573e69c..41c8c982200 100644 --- a/src/server/server.cpp +++ b/src/server/server.cpp @@ -759,3 +759,9 @@ void mir::Server::add_configuration_option( break; } } + + +void mir::Server::set_the_decoration_strategy(std::shared_ptr strategy) +{ + self->server_config->set_the_decoration_strategy(strategy); +} diff --git a/src/server/symbols.map b/src/server/symbols.map index 01bec218222..2caa562c2d5 100644 --- a/src/server/symbols.map +++ b/src/server/symbols.map @@ -11,6 +11,8 @@ global: mir::BasicCallback::operator*; mir::BasicCallback::unlock*; mir::DecorationStrategy::?DecorationStrategy*; + mir::DecorationStrategy::DecorationStrategy*; + mir::DecorationStrategy::operator*; mir::DefaultServerConfiguration::DefaultServerConfiguration*; mir::DefaultServerConfiguration::add_wayland_extension*; mir::DefaultServerConfiguration::default_reports*; From f5e92a19cc2bbbe6953c19a3e67b371a37651286 Mon Sep 17 00:00:00 2001 From: Tarek Yasser Date: Mon, 1 Jul 2024 16:34:44 +0300 Subject: [PATCH 47/81] Fix lifetime issues related to `toplevels_with_decorations`. --- .../xdg_decoration_unstable_v1.cpp | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 88720166800..c4472a66de1 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -39,25 +39,25 @@ class ToplevelsWithDecorations { public: ToplevelsWithDecorations() : - toplevels_with_decorations{std::make_shared>()} + toplevels_with_decorations{std::unordered_set()} { } /// \return true if no duplicates existed before insertion, false otherwise. bool registerToplevel(wl_resource* toplevel) { - auto [_, inserted] = toplevels_with_decorations->insert(toplevel); + auto [_, inserted] = toplevels_with_decorations.insert(toplevel); return inserted; } /// \return true if only one element was erased, false otherwise. bool unregisterToplevel(wl_resource* toplevel) { - return toplevels_with_decorations->erase(toplevel) == 1; + return toplevels_with_decorations.erase(toplevel) == 1; } private: - std::shared_ptr> toplevels_with_decorations; + std::unordered_set toplevels_with_decorations; }; class XdgDecorationManagerV1 : public wayland::XdgDecorationManagerV1 @@ -78,7 +78,7 @@ class XdgDecorationManagerV1 : public wayland::XdgDecorationManagerV1 private: void get_toplevel_decoration(wl_resource* id, wl_resource* toplevel) override; - ToplevelsWithDecorations toplevels_with_decorations; + std::shared_ptr const toplevels_with_decorations; std::shared_ptr const decoration_strategy; }; @@ -123,7 +123,7 @@ void mir::frontend::XdgDecorationManagerV1::Global::bind(wl_resource* new_zxdg_d mir::frontend::XdgDecorationManagerV1::XdgDecorationManagerV1( wl_resource* resource, std::shared_ptr strategy) : mir::wayland::XdgDecorationManagerV1{resource, Version<1>{}}, - toplevels_with_decorations{}, + toplevels_with_decorations{std::make_shared()}, decoration_strategy{std::move(strategy)} { } @@ -139,22 +139,22 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* } auto decoration = new XdgToplevelDecorationV1{id, tl, decoration_strategy}; - if (!toplevels_with_decorations.registerToplevel(toplevel)) + if (!toplevels_with_decorations->registerToplevel(toplevel)) { BOOST_THROW_EXCEPTION(mir::wayland::ProtocolError( resource, Error::already_constructed, "Decoration already constructed for this toplevel")); } decoration->add_destroy_listener( - [&toplevels_with_decorations = this->toplevels_with_decorations, toplevel]() + [toplevels_with_decorations = this->toplevels_with_decorations, toplevel]() { - toplevels_with_decorations.unregisterToplevel(toplevel); + toplevels_with_decorations->unregisterToplevel(toplevel); }); tl->add_destroy_listener( - [&toplevels_with_decorations = this->toplevels_with_decorations, client = this->client, toplevel]() + [toplevels_with_decorations = this->toplevels_with_decorations, client = this->client, toplevel]() { - if (!client->is_being_destroyed() && !toplevels_with_decorations.unregisterToplevel(toplevel)) + if (!client->is_being_destroyed() && !toplevels_with_decorations->unregisterToplevel(toplevel)) { mir::log_warning("Toplevel destroyed before attached decoration!"); // https://github.com/canonical/mir/issues/3452 From aa970674d351a90530f6e53cd038b41543dca1b2 Mon Sep 17 00:00:00 2001 From: Tarek Yasser Date: Mon, 1 Jul 2024 16:35:58 +0300 Subject: [PATCH 48/81] Fix naming convention of `{register,unregister}Toplevel`. --- .../frontend_wayland/xdg_decoration_unstable_v1.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index c4472a66de1..595dcd9be01 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -44,14 +44,14 @@ class ToplevelsWithDecorations } /// \return true if no duplicates existed before insertion, false otherwise. - bool registerToplevel(wl_resource* toplevel) + bool register_toplevel(wl_resource* toplevel) { auto [_, inserted] = toplevels_with_decorations.insert(toplevel); return inserted; } /// \return true if only one element was erased, false otherwise. - bool unregisterToplevel(wl_resource* toplevel) + bool unregister_toplevel(wl_resource* toplevel) { return toplevels_with_decorations.erase(toplevel) == 1; } @@ -139,7 +139,7 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* } auto decoration = new XdgToplevelDecorationV1{id, tl, decoration_strategy}; - if (!toplevels_with_decorations->registerToplevel(toplevel)) + if (!toplevels_with_decorations->register_toplevel(toplevel)) { BOOST_THROW_EXCEPTION(mir::wayland::ProtocolError( resource, Error::already_constructed, "Decoration already constructed for this toplevel")); @@ -148,13 +148,13 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* decoration->add_destroy_listener( [toplevels_with_decorations = this->toplevels_with_decorations, toplevel]() { - toplevels_with_decorations->unregisterToplevel(toplevel); + toplevels_with_decorations->unregister_toplevel(toplevel); }); tl->add_destroy_listener( [toplevels_with_decorations = this->toplevels_with_decorations, client = this->client, toplevel]() { - if (!client->is_being_destroyed() && !toplevels_with_decorations->unregisterToplevel(toplevel)) + if (!client->is_being_destroyed() && !toplevels_with_decorations->unregister_toplevel(toplevel)) { mir::log_warning("Toplevel destroyed before attached decoration!"); // https://github.com/canonical/mir/issues/3452 From 2e60ad6a9b5325396b952fa2e9867b499df60be8 Mon Sep 17 00:00:00 2001 From: tarek-y-ismail Date: Mon, 1 Jul 2024 17:33:20 +0300 Subject: [PATCH 49/81] Remove redundant `ToplevelsWithDecorations` construction code and disable copying. Co-authored-by: Alan Griffiths --- src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 595dcd9be01..0e3e7938657 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -38,10 +38,9 @@ namespace frontend class ToplevelsWithDecorations { public: - ToplevelsWithDecorations() : - toplevels_with_decorations{std::unordered_set()} - { - } + ToplevelsWithDecorations() = default; + ToplevelsWithDecorations(ToplevelsWithDecorations const&) = delete; + ToplevelsWithDecorations& operator=(ToplevelsWithDecorations const&) = delete; /// \return true if no duplicates existed before insertion, false otherwise. bool register_toplevel(wl_resource* toplevel) From 2bcf71176654b7c3757f6be78cb4a25da01b01f2 Mon Sep 17 00:00:00 2001 From: Tarek Yasser Date: Mon, 1 Jul 2024 17:38:41 +0300 Subject: [PATCH 50/81] Always unregister toplevel on destruction. Also fixed a sneaky bug caused by checking if one toplevel was registered instead of zero. toplevels should be already unregistered by their decorations by the time they're destroyed, otherwise it's a protocol error. --- .../frontend_wayland/xdg_decoration_unstable_v1.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 0e3e7938657..da1cbd3ca9a 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -49,10 +49,13 @@ class ToplevelsWithDecorations return inserted; } - /// \return true if only one element was erased, false otherwise. + /// \return true if the toplevel didn't exist in the set, false otherwise. + /// The return value is used in the destroy callback of the toplevel to + /// check if the toplevel is destroyed before the decoration (orphaned + /// decoration). bool unregister_toplevel(wl_resource* toplevel) { - return toplevels_with_decorations.erase(toplevel) == 1; + return toplevels_with_decorations.erase(toplevel) == 0; } private: @@ -153,7 +156,8 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* tl->add_destroy_listener( [toplevels_with_decorations = this->toplevels_with_decorations, client = this->client, toplevel]() { - if (!client->is_being_destroyed() && !toplevels_with_decorations->unregister_toplevel(toplevel)) + const auto orphaned_decoration = !toplevels_with_decorations->unregister_toplevel(toplevel); + if (!client->is_being_destroyed() && orphaned_decoration) { mir::log_warning("Toplevel destroyed before attached decoration!"); // https://github.com/canonical/mir/issues/3452 From 148715d8892c508e3ed2d9580e4c5a5f90f8f817 Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Tue, 2 Jul 2024 12:20:32 +0300 Subject: [PATCH 51/81] Improve semantics of `unregister_toplevel` and explain decoration orphaning --- .../xdg_decoration_unstable_v1.cpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index da1cbd3ca9a..8c02e49f8ac 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -49,13 +49,10 @@ class ToplevelsWithDecorations return inserted; } - /// \return true if the toplevel didn't exist in the set, false otherwise. - /// The return value is used in the destroy callback of the toplevel to - /// check if the toplevel is destroyed before the decoration (orphaned - /// decoration). + /// \return true if the toplevel was still registered, false otherwise. bool unregister_toplevel(wl_resource* toplevel) { - return toplevels_with_decorations.erase(toplevel) == 0; + return toplevels_with_decorations.erase(toplevel) > 0; } private: @@ -156,7 +153,15 @@ void mir::frontend::XdgDecorationManagerV1::get_toplevel_decoration(wl_resource* tl->add_destroy_listener( [toplevels_with_decorations = this->toplevels_with_decorations, client = this->client, toplevel]() { - const auto orphaned_decoration = !toplevels_with_decorations->unregister_toplevel(toplevel); + // Under normal conditions, decorations should be destroyed before + // toplevels. Causing `unregister_toplevel` to return false. + // + // If the attached decoration is not destroyed before its toplevel, + // then its a protocol error. This can happen in two cases: A + // protocol violation caused by the client, or another error + // triggering wayland cleanup code which destroys wayland objects + // with no guaranteed order. + const auto orphaned_decoration = toplevels_with_decorations->unregister_toplevel(toplevel); if (!client->is_being_destroyed() && orphaned_decoration) { mir::log_warning("Toplevel destroyed before attached decoration!"); From f9b62e667a132206e5d72b49d6986be38683360f Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Tue, 2 Jul 2024 14:54:40 +0300 Subject: [PATCH 52/81] Rename `destroy_toplevel_before_decoration` to `destroy_toplevel_before_decoration_throws_orphaned` --- tests/acceptance-tests/wayland/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/acceptance-tests/wayland/CMakeLists.txt b/tests/acceptance-tests/wayland/CMakeLists.txt index 493b3abb4f5..c12b7a5e291 100644 --- a/tests/acceptance-tests/wayland/CMakeLists.txt +++ b/tests/acceptance-tests/wayland/CMakeLists.txt @@ -116,7 +116,7 @@ set(EXPECTED_FAILURES # Should report an `orphaned` protocol error/violation , but we log instead # because of https://github.com/canonical/mir/issues/3452 - XdgDecorationV1Test.destroy_toplevel_before_decoration + XdgDecorationV1Test.destroying_toplevel_before_decoration_throws_orphaned ) if (MIR_SIGBUS_HANDLER_ENVIRONMENT_BROKEN) From 4b65e2b0ef0047c9cdacdc2b1d3b75b052fb7abc Mon Sep 17 00:00:00 2001 From: Tarek Yasser Date: Mon, 1 Jul 2024 15:58:33 +0300 Subject: [PATCH 53/81] Add mir server side decoration strategy --- src/core/symbols.map | 3 +- src/include/server/mir/decoration_strategy.h | 33 +++++++++++++++++++ .../xdg_decoration_unstable_v1.cpp | 6 ++-- 3 files changed, 37 insertions(+), 5 deletions(-) create mode 100644 src/include/server/mir/decoration_strategy.h diff --git a/src/core/symbols.map b/src/core/symbols.map index b1e30db825e..d3675afa067 100644 --- a/src/core/symbols.map +++ b/src/core/symbols.map @@ -1,5 +1,5 @@ MIR_CORE_2.9 { - global: +global: extern "C++" { mir::AnonymousShmFile::?AnonymousShmFile*; mir::AnonymousShmFile::AnonymousShmFile*; @@ -29,3 +29,4 @@ MIR_CORE_2.9 { }; local: *; }; + diff --git a/src/include/server/mir/decoration_strategy.h b/src/include/server/mir/decoration_strategy.h new file mode 100644 index 00000000000..b63a628eff2 --- /dev/null +++ b/src/include/server/mir/decoration_strategy.h @@ -0,0 +1,33 @@ +/* + * Copyright © Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 or 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include +namespace mir +{ +class DecorationStrategy +{ +public: + enum class DecorationsType : uint32_t + { + csd, + ssd + }; + + virtual ~DecorationStrategy() = default; + virtual auto default_style() const -> DecorationsType = 0; + virtual auto request_style(DecorationsType type) const -> DecorationsType = 0; +}; +} diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 8c02e49f8ac..ae96e4202de 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -180,8 +180,7 @@ mir::frontend::XdgToplevelDecorationV1::XdgToplevelDecorationV1( { } -auto mir::frontend::XdgToplevelDecorationV1::to_mode(DecorationStrategy::DecorationsType type) - -> uint32_t +auto mir::frontend::XdgToplevelDecorationV1::to_mode(DecorationStrategy::DecorationsType type) -> uint32_t { switch (type) { @@ -194,8 +193,7 @@ auto mir::frontend::XdgToplevelDecorationV1::to_mode(DecorationStrategy::Decorat std::unreachable(); } -auto mir::frontend::XdgToplevelDecorationV1::to_decorations_type(uint32_t mode) - -> DecorationStrategy::DecorationsType +auto mir::frontend::XdgToplevelDecorationV1::to_decorations_type(uint32_t mode) -> DecorationStrategy::DecorationsType { switch (mode) { From a1555bc642766bdc9c8ea1e1b2d6b0ae4944cf48 Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Wed, 3 Jul 2024 12:41:20 +0300 Subject: [PATCH 54/81] Dont expose `DecorationStrategy` in `miral::Decorations`'s interface. Also, changed `DecorationStrategy::Self` to inherit from `DecorationStrategy` and cleaned up the code a bit. --- include/core/mir/decoration_strategy.h | 36 -------------------------- include/miral/miral/decorations.h | 6 ++--- src/miral/decorations.cpp | 19 +++++++------- 3 files changed, 12 insertions(+), 49 deletions(-) delete mode 100644 include/core/mir/decoration_strategy.h diff --git a/include/core/mir/decoration_strategy.h b/include/core/mir/decoration_strategy.h deleted file mode 100644 index e43df57536a..00000000000 --- a/include/core/mir/decoration_strategy.h +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright © Canonical Ltd. - * - * This program is free software: you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 or 3 as - * published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ - -namespace mir -{ -class DecorationStrategy -{ -public: - enum class DecorationsType - { - csd, - ssd - }; - - DecorationStrategy() = default; - DecorationStrategy(DecorationStrategy const&) = delete; - DecorationStrategy& operator=(DecorationStrategy const&) = delete; - virtual ~DecorationStrategy() = default; - - virtual auto default_style() const -> DecorationsType = 0; - virtual auto request_style(DecorationsType type) const -> DecorationsType = 0; -}; -} diff --git a/include/miral/miral/decorations.h b/include/miral/miral/decorations.h index e31d6eda0e3..7a3b8cd3a4b 100644 --- a/include/miral/miral/decorations.h +++ b/include/miral/miral/decorations.h @@ -18,11 +18,9 @@ #define MIRAL_DECORATIONS_H -#include "mir/decoration_strategy.h" - #include -namespace mir { class Server; } +namespace mir { class Server; class DecorationStrategy; } namespace miral { @@ -52,7 +50,7 @@ class Decorations private: struct Self; - Decorations(std::shared_ptr strategy); + Decorations(std::shared_ptr strategy); std::shared_ptr self; }; diff --git a/src/miral/decorations.cpp b/src/miral/decorations.cpp index 423e0b2d7c1..fa60455594f 100644 --- a/src/miral/decorations.cpp +++ b/src/miral/decorations.cpp @@ -17,13 +17,14 @@ #include "miral/decorations.h" #include + #include #include +#include -struct miral::Decorations::Self +struct miral::Decorations::Self : mir::DecorationStrategy { - std::shared_ptr strategy; }; void miral::Decorations::operator()(mir::Server& server) const @@ -31,18 +32,18 @@ void miral::Decorations::operator()(mir::Server& server) const server.add_pre_init_callback( [this, &server]() { - server.set_the_decoration_strategy(self->strategy); + server.set_the_decoration_strategy(self); }); } -miral::Decorations::Decorations(std::shared_ptr strategy) : - self{std::make_shared(std::move(strategy))} +miral::Decorations::Decorations(std::shared_ptr strategy) : + self{std::move(strategy)} { } auto miral::Decorations::always_ssd() -> Decorations { - struct AlwaysServerSide : public mir::DecorationStrategy + struct AlwaysServerSide : Self { DecorationsType default_style() const override { return DecorationsType::ssd; } DecorationsType request_style(DecorationsType) const override { return DecorationsType::ssd; } @@ -53,7 +54,7 @@ auto miral::Decorations::always_ssd() -> Decorations auto miral::Decorations::always_csd() -> Decorations { - struct AlwaysClientSide : public mir::DecorationStrategy + struct AlwaysClientSide : Self { DecorationsType default_style() const override { return DecorationsType::csd; } DecorationsType request_style(DecorationsType) const override { return DecorationsType::csd; } @@ -64,7 +65,7 @@ auto miral::Decorations::always_csd() -> Decorations auto miral::Decorations::prefer_ssd() -> Decorations { - struct PreferServerSide : public mir::DecorationStrategy + struct PreferServerSide : Self { DecorationsType default_style() const override { return DecorationsType::ssd; } DecorationsType request_style(DecorationsType type) const override { return type; } @@ -75,7 +76,7 @@ auto miral::Decorations::prefer_ssd() -> Decorations auto miral::Decorations::prefer_csd() -> Decorations { - struct PreferClientSide : public mir::DecorationStrategy + struct PreferClientSide : Self { DecorationsType default_style() const override { return DecorationsType::csd; } DecorationsType request_style(DecorationsType type) const override { return type; } From 9c189ee891c2601720a776b9c4906240e5139f4a Mon Sep 17 00:00:00 2001 From: tarek-y-ismail Date: Wed, 3 Jul 2024 12:49:50 +0300 Subject: [PATCH 55/81] Make log message in `XdgToplevelDecorationV1::to_decorations_type` clearer Co-authored-by: Alan Griffiths --- src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index ae96e4202de..14b5eb0eaab 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -205,7 +205,7 @@ auto mir::frontend::XdgToplevelDecorationV1::to_decorations_type(uint32_t mode) pid_t pid; wl_client_get_credentials(client->raw_client(), &pid, nullptr, nullptr); // null pointers are allowed - mir::log_warning("Client PID: %d, got invalid protocol mode (%d), defaulting to client side.", pid, mode); + mir::log_warning("Client PID: %d, attempted to set invalid zxdg_toplevel_decoration_v1 mode (%d), defaulting to client side.", pid, mode); return DecorationStrategy::DecorationsType::csd; } From 75526c487b0aa0c4847259a9486ce4c506698c22 Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Wed, 3 Jul 2024 12:47:52 +0300 Subject: [PATCH 56/81] Undo changes done to `core/symbols.map` erroneously --- src/core/symbols.map | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/symbols.map b/src/core/symbols.map index d3675afa067..b1e30db825e 100644 --- a/src/core/symbols.map +++ b/src/core/symbols.map @@ -1,5 +1,5 @@ MIR_CORE_2.9 { -global: + global: extern "C++" { mir::AnonymousShmFile::?AnonymousShmFile*; mir::AnonymousShmFile::AnonymousShmFile*; @@ -29,4 +29,3 @@ global: }; local: *; }; - From 63c728784d8e88437f0c4797021347fc2ec01add Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Wed, 3 Jul 2024 13:21:29 +0300 Subject: [PATCH 57/81] Remove unnecessary `DecorationStrategy` forward declaration. --- include/miral/miral/decorations.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/miral/miral/decorations.h b/include/miral/miral/decorations.h index 7a3b8cd3a4b..f7d54a672ee 100644 --- a/include/miral/miral/decorations.h +++ b/include/miral/miral/decorations.h @@ -20,7 +20,7 @@ #include -namespace mir { class Server; class DecorationStrategy; } +namespace mir { class Server; } namespace miral { From 2dc3ec61ef669bff57389a4ee5da4772159e7b34 Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Wed, 3 Jul 2024 13:21:36 +0300 Subject: [PATCH 58/81] Remove the base of `DecorationStrategy::DecorationsType` --- src/include/server/mir/decoration_strategy.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/include/server/mir/decoration_strategy.h b/src/include/server/mir/decoration_strategy.h index b63a628eff2..42843e61a52 100644 --- a/src/include/server/mir/decoration_strategy.h +++ b/src/include/server/mir/decoration_strategy.h @@ -14,13 +14,12 @@ * along with this program. If not, see . */ -#include namespace mir { class DecorationStrategy { public: - enum class DecorationsType : uint32_t + enum class DecorationsType { csd, ssd From cdfc15c2402626d6a7cf24bcadf73512e922abd2 Mon Sep 17 00:00:00 2001 From: Tarek Ismail Date: Wed, 3 Jul 2024 14:51:11 +0300 Subject: [PATCH 59/81] Add debian symbols --- debian/libmiral7.symbols | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/debian/libmiral7.symbols b/debian/libmiral7.symbols index ea9af947ccb..192c2df8320 100644 --- a/debian/libmiral7.symbols +++ b/debian/libmiral7.symbols @@ -434,6 +434,12 @@ libmiral.so.7 libmiral7 #MINVER# (c++)"vtable for miral::MinimalWindowManager@MIRAL_5.0" 5.0.0 (c++)"vtable for miral::WindowManagementPolicy@MIRAL_5.0" 5.0.0 MIRAL_5.1@MIRAL_5.1 5.1.0 + (c++)"miral::Decorations::Decorations(std::shared_ptr)@MIRAL_5.1" 5.1.0 + (c++)"miral::Decorations::always_csd()@MIRAL_5.1" 5.1.0 + (c++)"miral::Decorations::always_ssd()@MIRAL_5.1" 5.1.0 + (c++)"miral::Decorations::operator()(mir::Server&) const@MIRAL_5.1" 5.1.0 + (c++)"miral::Decorations::prefer_csd()@MIRAL_5.1" 5.1.0 + (c++)"miral::Decorations::prefer_ssd()@MIRAL_5.1" 5.1.0 (c++)"miral::IdleListener::IdleListener()@MIRAL_5.1" 5.1.0 (c++)"miral::IdleListener::on_dim(std::function const&)@MIRAL_5.1" 5.1.0 (c++)"miral::IdleListener::on_off(std::function const&)@MIRAL_5.1" 5.1.0 From a5578107e2a269ea0197d1b8a44341bf1ee4dfba Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Tue, 2 Jul 2024 14:57:49 -0400 Subject: [PATCH 60/81] docs: document describing how to update symbols map files + bugfix in the symbols generator for major versioning --- .../how-to/how-to-update-symbols-map.md | 197 ++++++++++++++++++ doc/sphinx/how-to/index.md | 1 + tools/symbols_map_generator/main.py | 23 +- 3 files changed, 216 insertions(+), 5 deletions(-) create mode 100644 doc/sphinx/how-to/how-to-update-symbols-map.md diff --git a/doc/sphinx/how-to/how-to-update-symbols-map.md b/doc/sphinx/how-to/how-to-update-symbols-map.md new file mode 100644 index 00000000000..a3dcd1a84eb --- /dev/null +++ b/doc/sphinx/how-to/how-to-update-symbols-map.md @@ -0,0 +1,197 @@ +# How to Update Symbols Map Files +The Mir project is a collection of C++ libraries that a consumer +can use to create a Wayland compositor. In order for consumers +of Mir's libraries to be confident in what they're building against, the +symbols of each library are versioned. + +To describe these versions, each user-facing library defines a `symbols.map` +file. These files are comprised of version stanzas which contain the symbols +associated with that version. There are `symbols.map` files for the following +libraries, although only the first three are typically used in practice: + +- [miral](#how-to-update-miral-symbols) +- [miroil](#how-to-update-miroil-symbols) +- [mirserver](#how-to-update-mirserver-symbols-map) +- mircore +- mircommon +- mirplatform +- mirwayland + +It is tedious to edit these files. Luckily for us, this versioning business +is (mostly) automated so that we have to think very little about it. + +## When are symbols.map files updated +Two different circumstances will prompt a developer to update +these files: + +1. **A new symbols is added to a library** + + This means that we have extended the existing interface. + The new symbols can simply be put in a new stanza with a "bumped" + minor version. Note that the minor version must only be bumped once + per release cycle. This means that if I add a new symbol two months + before a release and you add a symbol 1 day before the release, then + our new symbols will go in the same stanza. In that situation, I + would have been the one to create the new stanza. + +2. **A symbol is changed or removed** (e.g. a new parameter is + added to a method call, a class is removed, etc.) + + This is an API break. This means that a consumer of the library + can no longer safely compile against the new version of that library + without maybe breaking their build. In this scenario, we are forced + to create a single new stanza at a brand new version in the `symbols.map` + file. This new stanza will contain all of the symbols in that library. + +Sometimes we break a symbols but don't even realize it until someone +points it out in a pull request. Nowadays, the symbols check is automated +by CI. You can find results of this check +[here](https://github.com/canonical/mir/actions/workflows/symbols-check.yml). +This check is run on each pull request. It will inform you whether or not +a `symbols.map` file needs to be updated due to your change. + +**Warning**: This check is not sophisticated to know if you change the +definition of a symbol (e.g. you changed the parameters of a method). This +check will still have to be done with your ever-wary eyes! + +## Setup +Before we can update the symbols of Mir's libraries, we'll want to set +up our environment so that the our tools can work correctly. + +To do this, you will first want to install **clang-19**. Note that it is +very important that you have the most recent version of clang, as that +works the best. + +```sh +sudo apt install clang-19 +``` + +Afterwards, you will want to set the following environment variables to point +at the path of `libclang.so` and `libclang.so`'s `lib` directory. Note that +these paths are likely to be the same on most Ubuntu 24.04 setups, but may +vary on other distros: + +```sh +export MIR_SYMBOLS_MAP_GENERATOR_CLANG_SO_PATH=/usr/lib/llvm-19/lib/libclang.so.1 +export MIR_SYMBOLS_MAP_GENERATOR_CLANG_LIBRARY_PATH=/usr/lib/llvm-19/lib +``` + +It is recommended that you put those `export ...` commands in your `.bashrc` +so that you don't have to think about it in the future. + +And that's it! Now we are ready to update our symbols automatically. + + +## How to update miral symbols + +### Scenario 1: Adding a new symbol +1. Make the additive change to the interface (e.g. by adding a new method + to an existing class) +2. If not already bumped in this release cycle, bump `MIRAL_VERSION_MINOR` + in `src/CMakeLists.txt` +3. If not already bumped in this release cycle, bump `MIRAL_ABI` in + `src/miral/CMakeLists.txt` +4. From the root of the project, run: + ```sh + cmake --build build --target generate-miral-symbols-map + ``` +5. Check that your new symbols is in the `symbols.map` file: + ```sh + git diff src/miral/symbols.map + ``` +6. Regenerate the debian symbols: + ```sh + cmake --build build --target regenerate-miral-debian-symbols + ``` + +### Scenario 2: Removing or changing a symbol +1. Make a destructive change to the interface (e.g. remove a parameter from + a method). +2. If not already bumped in this release cycle, bump `MIRAL_VERSION_MAJOR` + in `src/CMakeLists.txt`. Set `MIRAL_VERSION_MINOR` and `MIRAL_VERSION_PATCH` + to `0` +3. If not already bumped in this release cycle, bump `MIRAL_ABI` in + `src/miral/CMakeLists.txt` +4. From the root of the project, run: + ```sh + cmake --build build --target generate-miral-symbols-map + ``` +5. Check that your symbols are reflected properly in the `symbols.map` file: + ```sh + git diff src/miral/symbols.map + ``` +6. Regenerate the debian symbols: + ```sh + cmake --build build --target regenerate-miral-debian-symbols + ``` + +## How to update miroil symbols + +### Scenario 1: Adding a new symbol +1. Make the additive change to the interface (e.g. by adding a new method + to an existing class) +2. If not already bumped in this release cycle, bump `MIROIL_VERSION_MINOR` + in `src/miroil/CMakeLists.txt` +3. If not already bumped in this release cycle, bump `MIROIL_ABI` in + `src/miroil/CMakeLists.txt` +4. From the root of the project, run: + ```sh + cmake --build build --target generate-miroil-symbols-map + ``` +5. Check that your new symbols is in the `symbols.map` file: + ```sh + git diff src/miroil/symbols.map + ``` + +### Scenario 2: Removing or changing a symbol +1. Make a destructive change to the interface (e.g. remove a parameter from + a method). +2. If not already bumped in this release cycle, bump `MIROIL_ABI`` + in `src/miroil/CMakeLists.txt`. Set `MIROIL_VERSION_MINOR` and `MIROIL_VERSION_PATCH` + to `0` +3. From the root of the project, run: + ```sh + cmake --build build --target generate-miroil-symbols-map + ``` +4. Check that your symbols are reflected properly in the `symbols.map` file: + ```sh + git diff src/miroil/symbols.map + ``` +5. Regenerate the debian symbols: + ```sh + cmake --build build --target regenerate-miroil-debian-symbols + ``` + +## How to update mirserver symbols map +### Scenario 1: Adding a new symbol +1. Make the additive change to the interface (e.g. by adding a new method + to an existing class) +2. If not already bumped in this release cycle, bump `MIR_VERSION_MINOR` + in `CMakeLists.txt` +3. If not already bumped in this release cycle, bump `MIRSERVER_ABI` in + `src/server/CMakeLists.txt` +4. From the root of the project, run: + ```sh + cmake --build build --target generate-mirserver-symbols-map + ``` +5. Check that your new symbols is in the `symbols.map` file: + ```sh + git diff src/server/symbols.map + ``` + +### Scenario 2: Removing or changing a symbol +1. Make a destructive change to the interface (e.g. remove a parameter from + a method). +2. If not already bumped in this release cycle, bump `MIR_VERSION_MAJOR` + in `CMakeLists.txt`. Set `MIR_VERSION_MINOR` and `MIR_VERSION_PATCH` + to `0` +3. If not already bumped in this release cycle, bump `MIRSERVER_ABI` in + `src/server/CMakeLists.txt` +4. From the root of the project, run: + ```sh + cmake --build build --target generate-mirserver-symbols-map + ``` +5. Check that your symbols are reflected properly in the `symbols.map` file: + ```sh + git diff src/server/symbols.map + ``` \ No newline at end of file diff --git a/doc/sphinx/how-to/index.md b/doc/sphinx/how-to/index.md index 5ad49116851..9a7bcd73787 100644 --- a/doc/sphinx/how-to/index.md +++ b/doc/sphinx/how-to/index.md @@ -18,4 +18,5 @@ how-to-use-checkbox-mir how-to-calibrate-a-touchscreen-device how-to-enable-graphics-core22-on-a-device how-to-test-mir-for-a-release +how-to-update-symbols-map ``` diff --git a/tools/symbols_map_generator/main.py b/tools/symbols_map_generator/main.py index cdb0d871d06..d0b5376df91 100755 --- a/tools/symbols_map_generator/main.py +++ b/tools/symbols_map_generator/main.py @@ -42,6 +42,10 @@ class LibraryInfo(TypedDict): map_file: str +def get_major_version_from_str(version: str) -> int: + return int(version.split('.')[0]) + + class Symbol: name: str version: str @@ -471,22 +475,31 @@ def main(): new_external_symbols = get_added_symbols(previous_symbols, external_symbols, False) new_internal_symbols = get_added_symbols(previous_symbols, internal_symbols, True) + next_major = get_major_version_from_str(version) next_version = f"{library.upper()}_{version}" next_internal_version = f"{library.upper()}_INTERNAL_{version}" data_to_output: OrderedDict[str, dict[str, list[str]]] = OrderedDict() # Remake the stanzas for the previous symbols for symbol in previous_symbols: - version = f"{library.upper()}_{symbol.version}" - if not version in data_to_output: - data_to_output[version] = { + major = get_major_version_from_str(symbol.version) + + # If we are going up by a major version, then we should add + # all existing symbols to the new stanza + if major == next_major: + symbol_version = f"{library.upper()}_{symbol.version}" + else: + symbol_version = f"{library.upper()}_{version}" + + if not symbol_version in data_to_output: + data_to_output[symbol_version] = { "c": [], "c++": [] } if symbol.is_c_symbol: - bisect.insort(data_to_output[version]["c"], symbol.name) + bisect.insort(data_to_output[symbol_version]["c"], symbol.name) else: - bisect.insort(data_to_output[version]["c++"], symbol.name) + bisect.insort(data_to_output[symbol_version]["c++"], symbol.name) # Add the external symbols for symbol in new_external_symbols: From c25188c77c4c51213226f7b9e7b8e8c7409a9de0 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Wed, 3 Jul 2024 08:21:09 -0400 Subject: [PATCH 61/81] docs: update to reflect when the situations where we bump ABI versions --- doc/sphinx/how-to/how-to-update-symbols-map.md | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/doc/sphinx/how-to/how-to-update-symbols-map.md b/doc/sphinx/how-to/how-to-update-symbols-map.md index a3dcd1a84eb..a33826dee7d 100644 --- a/doc/sphinx/how-to/how-to-update-symbols-map.md +++ b/doc/sphinx/how-to/how-to-update-symbols-map.md @@ -89,17 +89,15 @@ And that's it! Now we are ready to update our symbols automatically. to an existing class) 2. If not already bumped in this release cycle, bump `MIRAL_VERSION_MINOR` in `src/CMakeLists.txt` -3. If not already bumped in this release cycle, bump `MIRAL_ABI` in - `src/miral/CMakeLists.txt` -4. From the root of the project, run: +3. From the root of the project, run: ```sh cmake --build build --target generate-miral-symbols-map ``` -5. Check that your new symbols is in the `symbols.map` file: +4. Check that your new symbols is in the `symbols.map` file: ```sh git diff src/miral/symbols.map ``` -6. Regenerate the debian symbols: +5. Regenerate the debian symbols: ```sh cmake --build build --target regenerate-miral-debian-symbols ``` @@ -132,13 +130,11 @@ And that's it! Now we are ready to update our symbols automatically. to an existing class) 2. If not already bumped in this release cycle, bump `MIROIL_VERSION_MINOR` in `src/miroil/CMakeLists.txt` -3. If not already bumped in this release cycle, bump `MIROIL_ABI` in - `src/miroil/CMakeLists.txt` -4. From the root of the project, run: +3. From the root of the project, run: ```sh cmake --build build --target generate-miroil-symbols-map ``` -5. Check that your new symbols is in the `symbols.map` file: +4. Check that your new symbols is in the `symbols.map` file: ```sh git diff src/miroil/symbols.map ``` @@ -146,7 +142,7 @@ And that's it! Now we are ready to update our symbols automatically. ### Scenario 2: Removing or changing a symbol 1. Make a destructive change to the interface (e.g. remove a parameter from a method). -2. If not already bumped in this release cycle, bump `MIROIL_ABI`` +2. If not already bumped in this release cycle, bump `MIROIL_ABI` in `src/miroil/CMakeLists.txt`. Set `MIROIL_VERSION_MINOR` and `MIROIL_VERSION_PATCH` to `0` 3. From the root of the project, run: From 05746c19abcbc723af3d8eff2afb95f913e250e9 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Wed, 3 Jul 2024 08:23:19 -0400 Subject: [PATCH 62/81] doc: including command for ./tools/update_package_abis.sh --- .../how-to/how-to-update-symbols-map.md | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/doc/sphinx/how-to/how-to-update-symbols-map.md b/doc/sphinx/how-to/how-to-update-symbols-map.md index a33826dee7d..869a6a6acc7 100644 --- a/doc/sphinx/how-to/how-to-update-symbols-map.md +++ b/doc/sphinx/how-to/how-to-update-symbols-map.md @@ -123,6 +123,12 @@ And that's it! Now we are ready to update our symbols automatically. cmake --build build --target regenerate-miral-debian-symbols ``` +If `MIRAL_ABI` needed to be updated, you should also run: + +```sh +./tools/update_package_abis.sh +``` + ## How to update miroil symbols ### Scenario 1: Adding a new symbol @@ -158,6 +164,12 @@ And that's it! Now we are ready to update our symbols automatically. cmake --build build --target regenerate-miroil-debian-symbols ``` +If `MIROIL_ABI` needed to be updated, you should also run: + +```sh +./tools/update_package_abis.sh +``` + ## How to update mirserver symbols map ### Scenario 1: Adding a new symbol 1. Make the additive change to the interface (e.g. by adding a new method @@ -175,6 +187,12 @@ And that's it! Now we are ready to update our symbols automatically. git diff src/server/symbols.map ``` +If `MIRSERVER_ABI` needed to be updated, you should also run: + +```sh +./tools/update_package_abis.sh +``` + ### Scenario 2: Removing or changing a symbol 1. Make a destructive change to the interface (e.g. remove a parameter from a method). @@ -190,4 +208,10 @@ And that's it! Now we are ready to update our symbols automatically. 5. Check that your symbols are reflected properly in the `symbols.map` file: ```sh git diff src/server/symbols.map - ``` \ No newline at end of file + ``` + +If `MIRSERVER_ABI` needed to be updated, you should also run: + +```sh +./tools/update_package_abis.sh +``` \ No newline at end of file From 976b5b4540cdbc63fbbe577308966d0938694c7a Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Wed, 3 Jul 2024 10:07:06 -0400 Subject: [PATCH 63/81] doc: slightly better wording around Github actions for symbols.map --- .../how-to/how-to-update-symbols-map.md | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/doc/sphinx/how-to/how-to-update-symbols-map.md b/doc/sphinx/how-to/how-to-update-symbols-map.md index 869a6a6acc7..c07ab577bd6 100644 --- a/doc/sphinx/how-to/how-to-update-symbols-map.md +++ b/doc/sphinx/how-to/how-to-update-symbols-map.md @@ -24,7 +24,7 @@ is (mostly) automated so that we have to think very little about it. Two different circumstances will prompt a developer to update these files: -1. **A new symbols is added to a library** +1. **A new symbol is added to a library** This means that we have extended the existing interface. The new symbols can simply be put in a new stanza with a "bumped" @@ -43,16 +43,18 @@ these files: to create a single new stanza at a brand new version in the `symbols.map` file. This new stanza will contain all of the symbols in that library. -Sometimes we break a symbols but don't even realize it until someone -points it out in a pull request. Nowadays, the symbols check is automated -by CI. You can find results of this check +If you forget to update the `symbols.map` file, Github CI will complain and +prevent the merge until the issue is addressed. You can find results of this action [here](https://github.com/canonical/mir/actions/workflows/symbols-check.yml). -This check is run on each pull request. It will inform you whether or not -a `symbols.map` file needs to be updated due to your change. - -**Warning**: This check is not sophisticated to know if you change the -definition of a symbol (e.g. you changed the parameters of a method). This -check will still have to be done with your ever-wary eyes! +Each time a pull request is open or updated on Github, this action will be +run. If you check the log of this action, you will see which symbols have been +changed so that you can address the issue. + +**Warning**: Please be aware that this action is not sophisticated enough to +know if you changed the definition of a symbol. If you add or remove a symbol, +the action will inform you. However, if you modify a symbol (e.g. by changing +the parameters that a method takes), you will *not* be informed that the symbol +has changes. Such changes will need to be moved to a new stanza manually. ## Setup Before we can update the symbols of Mir's libraries, we'll want to set From 2aeb44076a55f9da4b466e2b11ee2a220f41c3fa Mon Sep 17 00:00:00 2001 From: tarek-y-ismail Date: Wed, 3 Jul 2024 17:34:56 +0300 Subject: [PATCH 64/81] Add class documentation comments to `mir::Decorations` Co-authored-by: Alan Griffiths --- include/miral/miral/decorations.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/miral/miral/decorations.h b/include/miral/miral/decorations.h index f7d54a672ee..abdec1cc7fd 100644 --- a/include/miral/miral/decorations.h +++ b/include/miral/miral/decorations.h @@ -24,6 +24,9 @@ namespace mir { class Server; } namespace miral { +/// Configures the window decoration strategy. +/// \note The strategy can only be applied to clients that are able to negotiate the decoration style with the server. +/// \remark Since MirAL 5.1 class Decorations { public: From fc2d4357d5553c61f1dbcc0becc6b39031090876 Mon Sep 17 00:00:00 2001 From: tarek-y-ismail Date: Wed, 3 Jul 2024 18:08:24 +0300 Subject: [PATCH 65/81] Properly scope `pid` inside `to_decorations_type` Co-authored-by: Matthew Kosarek --- src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp index 14b5eb0eaab..ba17af5e3f3 100644 --- a/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp +++ b/src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp @@ -202,6 +202,7 @@ auto mir::frontend::XdgToplevelDecorationV1::to_decorations_type(uint32_t mode) case Mode::server_side: return DecorationStrategy::DecorationsType::ssd; default: + { pid_t pid; wl_client_get_credentials(client->raw_client(), &pid, nullptr, nullptr); // null pointers are allowed @@ -209,6 +210,7 @@ auto mir::frontend::XdgToplevelDecorationV1::to_decorations_type(uint32_t mode) return DecorationStrategy::DecorationsType::csd; } + } } void mir::frontend::XdgToplevelDecorationV1::update_mode(uint32_t new_mode) From 3d590e57269436b61b0ff59cb3ce8461a186094a Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Wed, 3 Jul 2024 11:14:21 -0400 Subject: [PATCH 66/81] docs: more feedback --- .../how-to/how-to-update-symbols-map.md | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/doc/sphinx/how-to/how-to-update-symbols-map.md b/doc/sphinx/how-to/how-to-update-symbols-map.md index c07ab577bd6..efd49bc6771 100644 --- a/doc/sphinx/how-to/how-to-update-symbols-map.md +++ b/doc/sphinx/how-to/how-to-update-symbols-map.md @@ -93,15 +93,15 @@ And that's it! Now we are ready to update our symbols automatically. in `src/CMakeLists.txt` 3. From the root of the project, run: ```sh - cmake --build build --target generate-miral-symbols-map + cmake --build --target generate-miral-symbols-map ``` 4. Check that your new symbols is in the `symbols.map` file: ```sh git diff src/miral/symbols.map ``` -5. Regenerate the debian symbols: +5. Regenerate Debian symbols: ```sh - cmake --build build --target regenerate-miral-debian-symbols + cmake --build --target regenerate-miral-debian-symbols ``` ### Scenario 2: Removing or changing a symbol @@ -114,7 +114,7 @@ And that's it! Now we are ready to update our symbols automatically. `src/miral/CMakeLists.txt` 4. From the root of the project, run: ```sh - cmake --build build --target generate-miral-symbols-map + cmake --build --target generate-miral-symbols-map ``` 5. Check that your symbols are reflected properly in the `symbols.map` file: ```sh @@ -122,7 +122,7 @@ And that's it! Now we are ready to update our symbols automatically. ``` 6. Regenerate the debian symbols: ```sh - cmake --build build --target regenerate-miral-debian-symbols + cmake --build --target regenerate-miral-debian-symbols ``` If `MIRAL_ABI` needed to be updated, you should also run: @@ -140,7 +140,7 @@ If `MIRAL_ABI` needed to be updated, you should also run: in `src/miroil/CMakeLists.txt` 3. From the root of the project, run: ```sh - cmake --build build --target generate-miroil-symbols-map + cmake --build --target generate-miroil-symbols-map ``` 4. Check that your new symbols is in the `symbols.map` file: ```sh @@ -155,7 +155,7 @@ If `MIRAL_ABI` needed to be updated, you should also run: to `0` 3. From the root of the project, run: ```sh - cmake --build build --target generate-miroil-symbols-map + cmake --build --target generate-miroil-symbols-map ``` 4. Check that your symbols are reflected properly in the `symbols.map` file: ```sh @@ -163,7 +163,7 @@ If `MIRAL_ABI` needed to be updated, you should also run: ``` 5. Regenerate the debian symbols: ```sh - cmake --build build --target regenerate-miroil-debian-symbols + cmake --build --target regenerate-miroil-debian-symbols ``` If `MIROIL_ABI` needed to be updated, you should also run: @@ -182,7 +182,7 @@ If `MIROIL_ABI` needed to be updated, you should also run: `src/server/CMakeLists.txt` 4. From the root of the project, run: ```sh - cmake --build build --target generate-mirserver-symbols-map + cmake --build --target generate-mirserver-symbols-map ``` 5. Check that your new symbols is in the `symbols.map` file: ```sh @@ -205,7 +205,7 @@ If `MIRSERVER_ABI` needed to be updated, you should also run: `src/server/CMakeLists.txt` 4. From the root of the project, run: ```sh - cmake --build build --target generate-mirserver-symbols-map + cmake --build --target generate-mirserver-symbols-map ``` 5. Check that your symbols are reflected properly in the `symbols.map` file: ```sh From 5becb953cc9a9aa230a5223660f91c1bef241088 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Fri, 5 Jul 2024 14:35:57 -0400 Subject: [PATCH 67/81] docs: update the tutorial to include less unnecessary cruft for newcomers --- doc/sphinx/tutorial.md | 143 ++++++++++++++++++----------------------- 1 file changed, 64 insertions(+), 79 deletions(-) diff --git a/doc/sphinx/tutorial.md b/doc/sphinx/tutorial.md index 71aeb7853ca..4dd9f3707b5 100644 --- a/doc/sphinx/tutorial.md +++ b/doc/sphinx/tutorial.md @@ -3,31 +3,15 @@ discourse: 4911,5164,5603,6756,8037 --- # Tutorial +This tutorial will instruct you on how to get started building your first +Wayland compositor with Mir. Additionally, we will provide information on how +to install some demo compositors that will give you a sense of what you can +build with Mir. -Mir is a library for building things, not an end-user product, but it does come -with some demos to illustrate the possibilities. - -## Getting Mir demos - -The Mir libraries and demos are available on Ubuntu, Fedora and Arch. It has -also been built and tested on Debian but, at the time or writing, is not in the -archive. - -For Linux distributions that don't currently package Mir you need to build it -yourself. (See [Getting Involved in Mir](how-to/getting_involved_in_mir)). - -## Getting Mir on Ubuntu - -You can install the Mir examples along with the Mir graphics drivers as follows: - - sudo apt install mir-demos mir-graphics-drivers-desktop - -It is also useful to install Wayland support for Qt and qterminal: - - sudo apt install qterminal qtwayland5 - -### Getting the latest Mir release on Ubuntu +## Installing Mir Libraries +The Mir libraries and demos are available on Ubuntu, Fedora, Debian and Arch. +### Installing Mir on Ubuntu As a matter of policy Ubuntu does not routinely update packages after a series is released. This means the Mir team cannot reasonably guarantee that all series will have the latest Mir release available at all times. @@ -35,54 +19,84 @@ will have the latest Mir release available at all times. The latest Mir release is available for all supported Ubuntu series from the Mir team's "release PPA". To add the PPA to your system: - sudo add-apt-repository --update ppa:mir-team/release +```sh +sudo add-apt-repository --update ppa:mir-team/release +``` + +Afterwards, you will be able to install any of the libraries that are published +by the project. The library that one might be interested in as a compositor author +is `libmiral`. To install it, you may run: + + +```sh +sudo apt install libmiral7 libmiral-dev mir-graphics-drivers-desktop +``` To remove the PPA from your system: - sudo ppa-purge mir-team/release +```sh +sudo ppa-purge mir-team/release +``` -## Getting Mir on Fedora +### Installing Mir on Fedora -On Fedora Mir is available from the archive: +On Fedora, Mir is available from the archive: - sudo dnf install mir-demos +```sh +sudo dnf install mir-devel mir-server-libs +``` -It is also useful to install qterminal: +### Installing Mir on Debian +On Debian, Mir is available as `mir` (https://tracker.debian.org/pkg/mir): - sudo dnf install qterminal +``` +sudo apt install mir +``` -## Getting Mir on Arch -On Arch Linux, you can install the [mir](https://aur.archlinux.org/packages/mir/) package from the AUR. +### Installing Mir on Arch -## Using Mir demos +On Arch Linux, you can install the [mir](https://aur.archlinux.org/packages/mir/) package from the AUR. -For convenient testing under X11 there's a "miral-app" script that wraps the -commands used to start a server and then launches a terminal (as the current -user): +## Installing and Running Demo Compositors +Mir provides a number of few example compositors in the +[examples folder](https://github.com/canonical/mir/tree/main/examples) of the Mir project. +Each folder in the "examples" folder is an independent Wayland compositor. - miral-app +### Installing demos on Ubuntu -If you're using a desktop that supports X11 then you can run this in a terminal -window. In that case Mir will automatically select a "Mir-on-X11" backend and -run in a Window. +```sh +sudo apt install mir-demos mir-graphics-drivers-desktop +``` -Alternatively, to run Mir "natively" you can run the same command in a Virtual -Terminal. +### Installing Mir demos manually +For Linux distributions that don't currently package Mir you need to build it +yourself. (See [Getting Involved in Mir](how-to/getting_involved_in_mir)). -### Running applications on Mir +### Running Mir demos +The main mir demo that you might be interested in is `miral-app`, which provides +access to a number of different window manager demos depending on which arguments +are provided to it. By default, it runs `miral-shell`, which is a floating window manager +with pleasant startup screen. To run the demos, you can simply run `miral-app` +from a terminal in your current session (e.g. a gnome-shell session): -If you use the terminal launched by `miral-app` Wayland applications can be -started as usual: +```sh +miral-app +``` - kate - neverputt - gedit +This will open up a new window in your compositor that contains a nested +"floating window manager" compositor. +Likewise, you can switch to a different VT (using `Ctrl + Alt + F`). From +there, you may log in and run: -### Options when running the Mir example shell +```sh +miral-app +``` -#### Script Options +In both cases, you should see a startup screen with information on how to use the +compositor. The compositor should behave like a typical floating window manager. +#### Options to `miral-app` The `miral-app` script provides options for using an alternative shell (e.g. `miral-kiosk` as used by the mir-kiosk snap) and an alternative terminal. @@ -98,32 +112,3 @@ For example: There are some additional options (listed with "-h") but those are the important ones. - -#### `miral-shell` Options - -The script will pass everything on the command-line following the first thing it -doesn't understand to `miral-shell`. The options can be listed by -`miral-shell --help`. The following are likely to be of interest: - - --window-management-trace log trace message - -Probably the main use for `miral-shell` is to test window-management (either of -a client toolkit or of a server) and this logs all calls to and from the window -management policy. This option is supported directly in the MirAL library and -works for any MirAL based shell - even one you write yourself. - - --window-manager arg (=floating) window management strategy - [{floating|tiling|system-compositor}] - -This allows an alternative "tiling" window manager to be selected. *Note: -`--window-manager` is only supported by `miral-shell` (not `miral-kiosk`).* - -For example: - - miral-app --window-manager tiling - -These options can also be specified in a configuration file. For example: - - $ cat ~/.config/miral-shell.config - keymap=gb - window-manager=tiling From 1b60c40ed392750111b03f1dfc9b069d4cfe8f93 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Wed, 3 Jul 2024 10:53:45 +0100 Subject: [PATCH 68/81] Default to enabling xdg_decorations --- src/server/frontend_wayland/wayland_default_configuration.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/server/frontend_wayland/wayland_default_configuration.cpp b/src/server/frontend_wayland/wayland_default_configuration.cpp index 8d4b9c28304..0e0dd5f923b 100644 --- a/src/server/frontend_wayland/wayland_default_configuration.cpp +++ b/src/server/frontend_wayland/wayland_default_configuration.cpp @@ -319,7 +319,8 @@ auto mf::get_standard_extensions() -> std::vector mw::TextInputManagerV1::interface_name, mw::TextInputManagerV2::interface_name, mw::TextInputManagerV3::interface_name, - mw::MirShellV1::interface_name}; + mw::MirShellV1::interface_name, + mw::XdgDecorationManagerV1::interface_name}; } auto mf::get_supported_extensions() -> std::vector From 965736da25119f24cb9a10a441b97347c3c28b72 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Wed, 3 Jul 2024 11:32:42 +0100 Subject: [PATCH 69/81] Demo configurable window decorations in miral-shell --- examples/miral-shell/shell_main.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/examples/miral-shell/shell_main.cpp b/examples/miral-shell/shell_main.cpp index 1bccf852c00..84ebc4ae718 100644 --- a/examples/miral-shell/shell_main.cpp +++ b/examples/miral-shell/shell_main.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -34,6 +35,30 @@ #include +#include + +namespace +{ +struct ConfigureDecorations +{ + miral::Decorations const decorations{[] + { + if (auto const strategy = getenv("MIRAL_SHELL_DECORATIONS")) + { + if (strcmp(strategy, "always-ssd") == 0) return miral::Decorations::always_ssd(); + if (strcmp(strategy, "prefer-ssd") == 0) return miral::Decorations::prefer_ssd(); + if (strcmp(strategy, "always-csd") == 0) return miral::Decorations::always_csd(); + } + return miral::Decorations::prefer_csd(); + }()}; + + void operator()(mir::Server& s) const + { + decorations(s); + } +}; +} + int main(int argc, char const* argv[]) { using namespace miral; @@ -113,6 +138,7 @@ int main(int argc, char const* argv[]) CursorTheme{"default:DMZ-White"}, WaylandExtensions{}, X11Support{}, + ConfigureDecorations{}, window_managers, display_configuration_options, external_client_launcher, From d6bf592dafd0a7f2bd97459a003a4954167c2088 Mon Sep 17 00:00:00 2001 From: Christopher James Halse Rogers Date: Tue, 11 Jun 2024 16:34:38 +1000 Subject: [PATCH 70/81] wayland: Add wp_viewporter protocol definition & wrapper --- src/wayland/CMakeLists.txt | 1 + wayland-protocols/viewporter.xml | 177 +++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+) create mode 100644 wayland-protocols/viewporter.xml diff --git a/src/wayland/CMakeLists.txt b/src/wayland/CMakeLists.txt index 99d4f9a2dc1..baa50acdcab 100644 --- a/src/wayland/CMakeLists.txt +++ b/src/wayland/CMakeLists.txt @@ -37,6 +37,7 @@ mir_generate_protocol_wrapper(mirwayland "zwlr_" wlr-virtual-pointer-unstable-v1 mir_generate_protocol_wrapper(mirwayland "ext_" ext-session-lock-v1.xml) mir_generate_protocol_wrapper(mirwayland "zmir_" mir-shell-unstable-v1.xml) mir_generate_protocol_wrapper(mirwayland "z" xdg-decoration-unstable-v1.xml) +mir_generate_protocol_wrapper(mirwayland "wp_" viewporter.xml) target_link_libraries(mirwayland PUBLIC diff --git a/wayland-protocols/viewporter.xml b/wayland-protocols/viewporter.xml new file mode 100644 index 00000000000..1374aeca065 --- /dev/null +++ b/wayland-protocols/viewporter.xml @@ -0,0 +1,177 @@ + + + + + Copyright © 2013-2016 Collabora, Ltd. + + Permission is hereby granted, free of charge, to any person obtaining a + copy of this software and associated documentation files (the "Software"), + to deal in the Software without restriction, including without limitation + the rights to use, copy, modify, merge, publish, distribute, sublicense, + and/or sell copies of the Software, and to permit persons to whom the + Software is furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice (including the next + paragraph) shall be included in all copies or substantial portions of the + Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + DEALINGS IN THE SOFTWARE. + + + + + The global interface exposing surface cropping and scaling + capabilities is used to instantiate an interface extension for a + wl_surface object. This extended interface will then allow + cropping and scaling the surface contents, effectively + disconnecting the direct relationship between the buffer and the + surface size. + + + + + Informs the server that the client will not be using this + protocol object anymore. This does not affect any other objects, + wp_viewport objects included. + + + + + + + + + + Instantiate an interface extension for the given wl_surface to + crop and scale its content. If the given wl_surface already has + a wp_viewport object associated, the viewport_exists + protocol error is raised. + + + + + + + + + An additional interface to a wl_surface object, which allows the + client to specify the cropping and scaling of the surface + contents. + + This interface works with two concepts: the source rectangle (src_x, + src_y, src_width, src_height), and the destination size (dst_width, + dst_height). The contents of the source rectangle are scaled to the + destination size, and content outside the source rectangle is ignored. + This state is double-buffered, see wl_surface.commit. + + The two parts of crop and scale state are independent: the source + rectangle, and the destination size. Initially both are unset, that + is, no scaling is applied. The whole of the current wl_buffer is + used as the source, and the surface size is as defined in + wl_surface.attach. + + If the destination size is set, it causes the surface size to become + dst_width, dst_height. The source (rectangle) is scaled to exactly + this size. This overrides whatever the attached wl_buffer size is, + unless the wl_buffer is NULL. If the wl_buffer is NULL, the surface + has no content and therefore no size. Otherwise, the size is always + at least 1x1 in surface local coordinates. + + If the source rectangle is set, it defines what area of the wl_buffer is + taken as the source. If the source rectangle is set and the destination + size is not set, then src_width and src_height must be integers, and the + surface size becomes the source rectangle size. This results in cropping + without scaling. If src_width or src_height are not integers and + destination size is not set, the bad_size protocol error is raised when + the surface state is applied. + + The coordinate transformations from buffer pixel coordinates up to + the surface-local coordinates happen in the following order: + 1. buffer_transform (wl_surface.set_buffer_transform) + 2. buffer_scale (wl_surface.set_buffer_scale) + 3. crop and scale (wp_viewport.set*) + This means, that the source rectangle coordinates of crop and scale + are given in the coordinates after the buffer transform and scale, + i.e. in the coordinates that would be the surface-local coordinates + if the crop and scale was not applied. + + If src_x or src_y are negative, the bad_value protocol error is raised. + Otherwise, if the source rectangle is partially or completely outside of + the non-NULL wl_buffer, then the out_of_buffer protocol error is raised + when the surface state is applied. A NULL wl_buffer does not raise the + out_of_buffer error. + + If the wl_surface associated with the wp_viewport is destroyed, + all wp_viewport requests except 'destroy' raise the protocol error + no_surface. + + If the wp_viewport object is destroyed, the crop and scale + state is removed from the wl_surface. The change will be applied + on the next wl_surface.commit. + + + + + The associated wl_surface's crop and scale state is removed. + The change is applied on the next wl_surface.commit. + + + + + + + + + + + + + Set the source rectangle of the associated wl_surface. See + wp_viewport for the description, and relation to the wl_buffer + size. + + If all of x, y, width and height are -1.0, the source rectangle is + unset instead. Any other set of values where width or height are zero + or negative, or x or y are negative, raise the bad_value protocol + error. + + The crop and scale state is double-buffered, see wl_surface.commit. + + + + + + + + + + Set the destination size of the associated wl_surface. See + wp_viewport for the description, and relation to the wl_buffer + size. + + If width is -1 and height is -1, the destination size is unset + instead. Any other pair of values for width and height that + contains zero or negative values raises the bad_value protocol + error. + + The crop and scale state is double-buffered, see wl_surface.commit. + + + + + + + From 40b2ecc2b371b3b3546c3179d73daad6c85c9013 Mon Sep 17 00:00:00 2001 From: Christopher James Halse Rogers Date: Wed, 26 Jun 2024 18:08:11 +1000 Subject: [PATCH 71/81] frontend_wayland: Add WpViewporter support --- include/platform/mir/graphics/renderable.h | 7 + src/gl/tessellation_helpers.cpp | 44 +++-- src/include/gl/mir/gl/tessellation_helpers.h | 1 + src/server/frontend_wayland/CMakeLists.txt | 1 + .../frontend_wayland/wayland_connector.cpp | 3 + .../frontend_wayland/wayland_connector.h | 2 + src/server/frontend_wayland/wl_surface.cpp | 139 ++++++++++++++-- src/server/frontend_wayland/wl_surface.h | 20 +++ src/server/frontend_wayland/wp_viewporter.cpp | 150 ++++++++++++++++++ src/server/frontend_wayland/wp_viewporter.h | 65 ++++++++ src/server/graphics/software_cursor.cpp | 5 + src/server/input/touchspot_controller.cpp | 5 + src/server/scene/basic_surface.cpp | 3 + src/server/shell/basic_idle_handler.cpp | 5 + .../wayland/miral_integration.cpp | 3 +- .../mir/test/doubles/fake_renderable.h | 5 + .../mir/test/doubles/mock_renderable.h | 1 + .../mir/test/doubles/stub_renderable.h | 4 + .../graphics_platform_test_harness.cpp | 5 + 19 files changed, 443 insertions(+), 25 deletions(-) create mode 100644 src/server/frontend_wayland/wp_viewporter.cpp create mode 100644 src/server/frontend_wayland/wp_viewporter.h diff --git a/include/platform/mir/graphics/renderable.h b/include/platform/mir/graphics/renderable.h index 7cd9fb06bd7..5e3103151fb 100644 --- a/include/platform/mir/graphics/renderable.h +++ b/include/platform/mir/graphics/renderable.h @@ -51,6 +51,13 @@ class Renderable virtual std::shared_ptr buffer() const = 0; virtual geometry::Rectangle screen_position() const = 0; + /** + * The region of \ref buffer that should be sampled from. + * + * This is in buffer coordinates, so {{0, 0}, {buffer->size().width, buffer->size().height} means + * “use the whole buffer”. + */ + virtual geometry::RectangleD src_bounds() const = 0; virtual std::optional clip_area() const = 0; // These are from the old CompositingCriteria. There is a little bit diff --git a/src/gl/tessellation_helpers.cpp b/src/gl/tessellation_helpers.cpp index 3598b35cea5..60649c554c1 100644 --- a/src/gl/tessellation_helpers.cpp +++ b/src/gl/tessellation_helpers.cpp @@ -14,32 +14,58 @@ * along with this program. If not, see . */ #include "mir/gl/tessellation_helpers.h" +#include "mir/geometry/forward.h" #include "mir/graphics/renderable.h" #include "mir/graphics/buffer.h" namespace mg = mir::graphics; namespace mgl = mir::gl; namespace geom = mir::geometry; + +namespace +{ +struct SrcTexCoords +{ + GLfloat top; + GLfloat bottom; + GLfloat left; + GLfloat right; +}; + +auto tex_coords_from_rect(geom::Size buffer_size, geom::RectangleD sample_rect) -> SrcTexCoords +{ + /* GL Texture coordinates are normalised to the size of the buffer, so (0.0, 0.0) is the top-left + * and (1.0, 1.0) is the bottom-right + */ + SrcTexCoords coords; + coords.top = sample_rect.top().as_value() / buffer_size.height.as_uint32_t(); + coords.bottom = sample_rect.bottom().as_value() / buffer_size.height.as_uint32_t(); + coords.left = sample_rect.left().as_value() / buffer_size.width.as_uint32_t(); + coords.right = sample_rect.right().as_value() / buffer_size.width.as_uint32_t(); + return coords; +} +} + mgl::Primitive mgl::tessellate_renderable_into_rectangle( mg::Renderable const& renderable, geom::Displacement const& offset) { auto rect = renderable.screen_position(); rect.top_left = rect.top_left - offset; - GLfloat left = rect.top_left.x.as_int(); - GLfloat right = left + rect.size.width.as_int(); - GLfloat top = rect.top_left.y.as_int(); - GLfloat bottom = top + rect.size.height.as_int(); + GLfloat const left = rect.top_left.x.as_int(); + GLfloat const right = left + rect.size.width.as_int(); + GLfloat const top = rect.top_left.y.as_int(); + GLfloat const bottom = top + rect.size.height.as_int(); mgl::Primitive rectangle; rectangle.type = GL_TRIANGLE_STRIP; - GLfloat const tex_right = 1.0f; - GLfloat const tex_bottom = 1.0f; + auto const [tex_top, tex_bottom, tex_left, tex_right] = + tex_coords_from_rect(renderable.buffer()->size(), renderable.src_bounds()); auto& vertices = rectangle.vertices; - vertices[0] = {{left, top, 0.0f}, {0.0f, 0.0f}}; - vertices[1] = {{left, bottom, 0.0f}, {0.0f, tex_bottom}}; - vertices[2] = {{right, top, 0.0f}, {tex_right, 0.0f}}; + vertices[0] = {{left, top, 0.0f}, {tex_left, tex_top}}; + vertices[1] = {{left, bottom, 0.0f}, {tex_left, tex_bottom}}; + vertices[2] = {{right, top, 0.0f}, {tex_right, tex_top}}; vertices[3] = {{right, bottom, 0.0f}, {tex_right, tex_bottom}}; return rectangle; } diff --git a/src/include/gl/mir/gl/tessellation_helpers.h b/src/include/gl/mir/gl/tessellation_helpers.h index 0c8b0bbd320..57569778302 100644 --- a/src/include/gl/mir/gl/tessellation_helpers.h +++ b/src/include/gl/mir/gl/tessellation_helpers.h @@ -18,6 +18,7 @@ #define MIR_GL_TESSELLATION_HELPERS_H_ #include "mir/gl/primitive.h" #include "mir/geometry/displacement.h" +#include "mir/geometry/rectangle.h" namespace mir { diff --git a/src/server/frontend_wayland/CMakeLists.txt b/src/server/frontend_wayland/CMakeLists.txt index a56b23a75ff..e0e1ce015ed 100644 --- a/src/server/frontend_wayland/CMakeLists.txt +++ b/src/server/frontend_wayland/CMakeLists.txt @@ -63,6 +63,7 @@ set( ${PROJECT_SOURCE_DIR}/src/include/server/mir/frontend/pointer_input_dispatcher.h session_credentials.cpp ${PROJECT_SOURCE_DIR}/src/include/server/mir/frontend/buffer_stream.h + wp_viewporter.cpp wp_viewporter.h ) add_custom_command( diff --git a/src/server/frontend_wayland/wayland_connector.cpp b/src/server/frontend_wayland/wayland_connector.cpp index e7158fb5f04..f6486d70b0f 100644 --- a/src/server/frontend_wayland/wayland_connector.cpp +++ b/src/server/frontend_wayland/wayland_connector.cpp @@ -28,6 +28,7 @@ #include "wayland_executor.h" #include "desktop_file_manager.h" #include "foreign_toplevel_manager_v1.h" +#include "wp_viewporter.h" #include "mir/main_loop.h" #include "mir/thread_name.h" @@ -331,6 +332,8 @@ mf::WaylandConnector::WaylandConnector( shm_global = std::make_unique(display.get(), executor); + viewporter = std::make_unique(display.get()); + char const* wayland_display = nullptr; if (auto const display_name = getenv("WAYLAND_DISPLAY")) diff --git a/src/server/frontend_wayland/wayland_connector.h b/src/server/frontend_wayland/wayland_connector.h index 7f742bd17d3..096cc52eb28 100644 --- a/src/server/frontend_wayland/wayland_connector.h +++ b/src/server/frontend_wayland/wayland_connector.h @@ -85,6 +85,7 @@ class WlSeat; class WlShm; class WlSubcompositor; class WlSurface; +class WpViewporter; class DesktopFileManager; class WaylandExtensions @@ -210,6 +211,7 @@ class WaylandConnector : public Connector std::shared_ptr desktop_file_manager; std::unique_ptr data_device_manager_global; std::unique_ptr shm_global; + std::unique_ptr viewporter; std::shared_ptr const executor; std::shared_ptr const allocator; std::shared_ptr const shell; diff --git a/src/server/frontend_wayland/wl_surface.cpp b/src/server/frontend_wayland/wl_surface.cpp index 6345489f7b6..5a1a24a889d 100644 --- a/src/server/frontend_wayland/wl_surface.cpp +++ b/src/server/frontend_wayland/wl_surface.cpp @@ -16,7 +16,7 @@ #include "wl_surface.h" -#include "mir/geometry/forward.h" +#include "viewporter_wrapper.h" #include "wayland_utils.h" #include "wl_surface_role.h" #include "wl_subcompositor.h" @@ -39,9 +39,12 @@ #include "mir/scene/surface.h" #include "mir/shell/surface_specification.h" #include "mir/log.h" +#include "wp_viewporter.h" #include #include +#include +#include #include namespace mf = mir::frontend; @@ -72,6 +75,9 @@ void mf::WlSurfaceState::update_from(WlSurfaceState const& source) begin(source.frame_callbacks), end(source.frame_callbacks)); + if (source.viewport) + viewport = source.viewport; + if (source.surface_data_invalidated) surface_data_invalidated = true; } @@ -317,6 +323,30 @@ void mf::WlSurface::set_input_region(std::optional const& region) } } +namespace +{ +auto as_int_if_exact(double d) -> std::optional +{ + if (std::trunc(d) == d && d < std::numeric_limits::max()) + { + return static_cast(d); + } + return {}; +} + +auto as_int_size_if_exact(geom::SizeD size) -> std::optional +{ + auto width = as_int_if_exact(size.width.as_value()); + auto height = as_int_if_exact(size.height.as_value()); + + if (width && height) + { + return geom::Size{*width, *height}; + } + return {}; +} +} + void mf::WlSurface::commit(WlSurfaceState const& state) { // We're going to lose the value of state, so copy the frame_callbacks first. We have to maintain a list of @@ -333,6 +363,18 @@ void mf::WlSurface::commit(WlSurfaceState const& state) if (state.scale) inv_scale = 1.0f / state.scale.value(); + if (state.viewport) + { + viewport = std::move(state.viewport); + } + + bool needs_buffer_submission = + state.scale || // If the scale has changed, or... + state.viewport || // ...we've added a viewport, or... + (viewport && viewport.value().dirty()); // ...the viewport has changed... + // ...then we'll need to submit a new frame, even if the client hasn't + // attached a new buffer. + auto const executor_send_frame_callbacks = [executor = wayland_executor, weak_self = mw::make_weak(this)]() { executor->spawn([weak_self]() @@ -366,11 +408,10 @@ void mf::WlSurface::commit(WlSurfaceState const& state) } }); }; - std::shared_ptr mir_buffer; if (auto const shm_buffer = ShmBuffer::from(weak_buffer.value())) { - mir_buffer = allocator->buffer_from_shm( + current_buffer = allocator->buffer_from_shm( shm_buffer->data(), std::move(executor_send_frame_callbacks), std::move(release_buffer)); @@ -378,11 +419,11 @@ void mf::WlSurface::commit(WlSurfaceState const& state) mir_server_wayland, sw_buffer_committed, wl_resource_get_client(resource), - mir_buffer->id().as_value()); + current_buffer->id().as_value()); } else { - mir_buffer = allocator->buffer_from_resource( + current_buffer = allocator->buffer_from_resource( weak_buffer.value(), std::move(executor_send_frame_callbacks), std::move(release_buffer)); @@ -390,18 +431,10 @@ void mf::WlSurface::commit(WlSurfaceState const& state) mir_server_wayland, hw_buffer_committed, wl_resource_get_client(resource), - mir_buffer->id().as_value()); + current_buffer->id().as_value()); } - stream->submit_buffer(mir_buffer, mir_buffer->size() * inv_scale, {{0, 0}, geom::SizeD{mir_buffer->size()}}); - auto const new_buffer_size = mir_buffer->size() * inv_scale; - - if (std::make_optional(new_buffer_size) != buffer_size_) - { - state.invalidate_surface_data(); // input shape needs to be recalculated for the new size - } - - buffer_size_ = new_buffer_size; + needs_buffer_submission = true; } } else @@ -409,6 +442,73 @@ void mf::WlSurface::commit(WlSurfaceState const& state) frame_callback_executor->spawn(std::move(executor_send_frame_callbacks)); } + if (needs_buffer_submission) + { + auto const src_sample = + [&]() -> geom::RectangleD + { + auto const entire_buffer = geom::RectangleD{{0, 0}, geom::SizeD{current_buffer->size()}}; + if (viewport && viewport.value().source) + { + auto raw_source = viewport.value().source.value(); + /* Viewport coordinates are in buffer coordinates *after* applying transformation and scale. + * That means this rectangle needs to be scaled. (And have buffer transform applied, when we support that) + */ + geom::RectangleD source_in_buffer_coords{ + {raw_source.left().as_value() / inv_scale, raw_source.top().as_value() / inv_scale}, + raw_source.size / inv_scale + }; + if (!entire_buffer.contains(source_in_buffer_coords)) + { + throw wayland::ProtocolError{ + viewport.value().resource, + wayland::Viewport::Error::out_of_buffer, + "Source viewport (%f, %f), (%f × %f) is not entirely within buffer (%i × %i)", + source_in_buffer_coords.left().as_value(), + source_in_buffer_coords.top().as_value(), + source_in_buffer_coords.size.width.as_value(), + source_in_buffer_coords.size.height.as_value(), + current_buffer->size().width.as_uint32_t(), + current_buffer->size().height.as_uint32_t() + }; + } + return source_in_buffer_coords; + } + return entire_buffer; + }(); + auto const logical_size = + [&]() + { + if (viewport) + { + if (viewport.value().destination) + { + return viewport.value().destination.value(); + } + } + if (as_int_size_if_exact(src_sample.size)) + { + return as_int_size_if_exact(src_sample.size).value() * inv_scale; + } + else + { + throw wayland::ProtocolError{ + viewport.value().resource, + wayland::Viewport::Error::bad_size, + "No wp_viewport destination set, and source size (%f × %f) is not integral", src_sample.size.width.as_value(), src_sample.size.height.as_value()}; + } + }(); + + stream->submit_buffer(current_buffer, logical_size, src_sample); + + if (std::make_optional(logical_size) != buffer_size_) + { + state.invalidate_surface_data(); // input shape needs to be recalculated for the new size + } + + buffer_size_ = logical_size; + } + for (WlSubsurface* child: children) { child->parent_has_committed(); @@ -468,6 +568,15 @@ auto mf::WlSurface::confine_pointer_state() const -> MirPointerConfinementState return mir_pointer_unconfined; } +void mf::WlSurface::associate_viewport(wayland::Weak viewport) +{ + if (this->viewport || pending.viewport) + { + BOOST_THROW_EXCEPTION((std::logic_error{"Cannot associate a viewport to a window with an existing viewport"})); + } + pending.viewport = viewport; +} + void mir::frontend::WlSurface::update_surface_spec(shell::SurfaceSpecification const& spec) { pending.surface_spec.update_from(spec); diff --git a/src/server/frontend_wayland/wl_surface.h b/src/server/frontend_wayland/wl_surface.h index cefbd79e675..3caffec057e 100644 --- a/src/server/frontend_wayland/wl_surface.h +++ b/src/server/frontend_wayland/wl_surface.h @@ -39,6 +39,7 @@ class Executor; namespace graphics { class GraphicBufferAllocator; +class Buffer; } namespace scene { @@ -57,6 +58,7 @@ namespace frontend class WlSurface; class WlSubsurface; class ResourceLifetimeTracker; +class Viewport; struct WlSurfaceState { @@ -83,6 +85,7 @@ struct WlSurfaceState std::optional offset; std::optional>> input_shape; std::vector> frame_callbacks; + wayland::Weak viewport; private: // only set to true if invalidate_surface_data() is called @@ -142,6 +145,13 @@ class WlSurface : public wayland::Surface void commit(WlSurfaceState const& state); auto confine_pointer_state() const -> MirPointerConfinementState; + /** + * Associate a viewport (buffer scale & crop metadata) with this surface + * + * \throws A std::logic_error if the surface already has a viewport associated + */ + void associate_viewport(wayland::Weak viewport); + std::shared_ptr const session; std::shared_ptr const stream; @@ -155,6 +165,15 @@ class WlSurface : public wayland::Surface NullWlSurfaceRole null_role; WlSurfaceRole* role; std::vector children; // ordering is from bottom to top + /* We might need to resubmit the current buffer, but with different metadata + * For example: if a client commits a wl_surface.buffer_scale() without attaching + * a new buffer, we need to generate a new frame with the same content, but at the + * new scale. + * + * To simplify other interfaces, keep a reference to the current content so we can + * hide this, here, in the frontend. + */ + std::shared_ptr current_buffer; WlSurfaceState pending; geometry::Displacement offset_; @@ -163,6 +182,7 @@ class WlSurface : public wayland::Surface std::vector> frame_callbacks; std::optional> input_shape; std::vector scene_surface_created_callbacks; + wayland::Weak viewport; void send_frame_callbacks(); diff --git a/src/server/frontend_wayland/wp_viewporter.cpp b/src/server/frontend_wayland/wp_viewporter.cpp new file mode 100644 index 00000000000..38db1365a85 --- /dev/null +++ b/src/server/frontend_wayland/wp_viewporter.cpp @@ -0,0 +1,150 @@ + +/* + * Copyright © Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "wp_viewporter.h" +#include "mir/geometry/forward.h" +#include "mir/wayland/protocol_error.h" +#include "mir/wayland/weak.h" +#include "wl_surface.h" + +#include + +namespace mf = mir::frontend; + +namespace +{ +class ViewporterInstance : public mir::wayland::Viewporter +{ +public: + ViewporterInstance(wl_resource* resource) + : Viewporter(resource, Version<1>{}) + { + } + +private: + void get_viewport(wl_resource* id, wl_resource* surface) override + { + auto surf = mf::WlSurface::from(surface); + try + { + new mf::Viewport(id, surf); + } + catch (std::logic_error const&) + { + // We get a std::logic_error if the surface already had a viewport; translate to protocol exception here + throw mir::wayland::ProtocolError{ + resource, + Viewporter::Error::viewport_exists, + "Surface already has a viewport associated"}; + } + } +}; +} + +mf::WpViewporter::WpViewporter(wl_display* display) + : Global(display, Version<1>{}) +{ +} + +void mf::WpViewporter::bind(wl_resource* new_resource) +{ + new ViewporterInstance(new_resource); +} + +mf::Viewport::Viewport(wl_resource* new_viewport, WlSurface* surface) + : wayland::Viewport(new_viewport, Version<1>{}) +{ + surface->associate_viewport(wayland::make_weak(this)); + this->surface = wayland::make_weak(surface); +} + +mf::Viewport::~Viewport() +{ +} + +auto mf::Viewport::dirty() -> bool +{ + return std::exchange(dirty_, false); +} + +void mf::Viewport::set_source(double x, double y, double width, double height) +{ + if (!surface) + { + throw wayland::ProtocolError{ + resource, + Error::no_surface, + "Surface associated with viewport has been destroyed"}; + } + // We know the parameters are coming from a 16.16 wl_fixed value, so doubles give exact representation + if (x < 0 || y < 0 || width <= 0 || height <= 0) + { + // All values == -1 means “unset the source viewport” + if ((x == y) && (y == width) && (width == height) && (height == -1.0f)) + { + source = std::nullopt; + } + else + { + throw wayland::ProtocolError{ + resource, + Error::bad_value, + "Source viewport (%f, %f), (%f × %f) invalid: (x,y) must be non-negative, (width, height) must be positive", + x, y, + width, height + }; + } + } + else + { + source = geometry::RectangleD{{x, y}, {width, height}}; + } + dirty_ = true; +} + +void mf::Viewport::set_destination(int32_t width, int32_t height) +{ + if (!surface) + { + throw wayland::ProtocolError{ + resource, + Error::no_surface, + "Surface associated with viewport has been destroyed"}; + } + if (width <= 0 || height <= 0) + { + if (width == -1 && height == -1) + { + destination = std::nullopt; + } + else + { + throw wayland::ProtocolError{ + resource, + Error::bad_value, + "Destination viewport (%i × %i) invalid: (width, height) must both be positive", + width, height + }; + } + } + else + { + destination = geometry::Size{width, height}; + } + + dirty_ = true; +} diff --git a/src/server/frontend_wayland/wp_viewporter.h b/src/server/frontend_wayland/wp_viewporter.h new file mode 100644 index 00000000000..694d4868ee9 --- /dev/null +++ b/src/server/frontend_wayland/wp_viewporter.h @@ -0,0 +1,65 @@ + +/* + * Copyright © Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef MIR_FRONTEND_WP_VIEWPORTER_H +#define MIR_FRONTEND_WP_VIEWPORTER_H + +#include "mir/wayland/weak.h" +#include "viewporter_wrapper.h" +#include "wayland_wrapper.h" + +#include "mir/geometry/rectangle.h" + +#include + +namespace mir::frontend +{ +class WlSurface; + +class WpViewporter : public wayland::Viewporter::Global +{ +public: + WpViewporter(wl_display* display); + +private: + void bind(wl_resource* new_wp_viewporter) override; +}; + +class Viewport : public wayland::Viewport +{ +public: + Viewport(wl_resource* new_viewport, WlSurface* surface); + ~Viewport() override; + + // Threadsafety: This is a Wayland object, these should only be referenced on the Wayland mainloop + std::optional source; + std::optional destination; + + /** + * Have the source or destination values changed since the last call to dirty()? + */ + auto dirty() -> bool; +private: + void set_source(double x, double y, double width, double height) override; + void set_destination(int32_t width, int32_t height) override; + + bool dirty_; + wayland::Weak surface; +}; +} + +#endif // MIR_FRONTEND_WP_VIEWPORTER_H diff --git a/src/server/graphics/software_cursor.cpp b/src/server/graphics/software_cursor.cpp index a901d5d1e42..e99822a1640 100644 --- a/src/server/graphics/software_cursor.cpp +++ b/src/server/graphics/software_cursor.cpp @@ -80,6 +80,11 @@ class mg::detail::CursorRenderable : public mg::Renderable return {position, buffer_->size()}; } + geom::RectangleD src_bounds() const override + { + return {geom::PointD{0, 0}, geom::SizeD{buffer_->size()}}; + } + std::optional clip_area() const override { return std::optional(); diff --git a/src/server/input/touchspot_controller.cpp b/src/server/input/touchspot_controller.cpp index 68eade869a9..50a19bf5d6c 100644 --- a/src/server/input/touchspot_controller.cpp +++ b/src/server/input/touchspot_controller.cpp @@ -63,6 +63,11 @@ class mi::TouchspotRenderable : public mg::Renderable return {position, buffer_->size()}; } + geom::RectangleD src_bounds() const override + { + return {{0, 0}, geom::SizeD{buffer_->size()}}; + } + std::optional clip_area() const override { return std::optional(); diff --git a/src/server/scene/basic_surface.cpp b/src/server/scene/basic_surface.cpp index a58e54e40aa..bc8987bf604 100644 --- a/src/server/scene/basic_surface.cpp +++ b/src/server/scene/basic_surface.cpp @@ -719,6 +719,9 @@ class SurfaceSnapshot : public mg::Renderable geom::Rectangle screen_position() const override { return screen_position_; } + geom::RectangleD src_bounds() const override + { return entry->source_rect(); } + std::optional clip_area() const override { return clip_area_; } diff --git a/src/server/shell/basic_idle_handler.cpp b/src/server/shell/basic_idle_handler.cpp index e5f901d95fd..af4985703f1 100644 --- a/src/server/shell/basic_idle_handler.cpp +++ b/src/server/shell/basic_idle_handler.cpp @@ -65,6 +65,11 @@ struct DimmingRenderable : public mg::Renderable return {{-coverage_size / 2, -coverage_size / 2}, {coverage_size, coverage_size}}; } + auto src_bounds() const -> geom::RectangleD override + { + return {{0, 0}, geom::SizeD{buffer_->size()}}; + } + auto clip_area() const -> std::optional override { return {}; diff --git a/tests/acceptance-tests/wayland/miral_integration.cpp b/tests/acceptance-tests/wayland/miral_integration.cpp index 1cc042790bc..bcfa7d26394 100644 --- a/tests/acceptance-tests/wayland/miral_integration.cpp +++ b/tests/acceptance-tests/wayland/miral_integration.cpp @@ -31,7 +31,8 @@ WlcsExtensionDescriptor const extensions[] = { {"zxdg_shell_unstable_v6", 1}, {"wlr_layer_shell_unstable_v1", 1}, {"zwp_pointer_constraints_v1", 1}, - {"zwp_relative_pointer_manager_v1", 1} + {"zwp_relative_pointer_manager_v1", 1}, + {"wp_viewporter", 1}, }; WlcsIntegrationDescriptor const descriptor{ diff --git a/tests/include/mir/test/doubles/fake_renderable.h b/tests/include/mir/test/doubles/fake_renderable.h index 232e40d884e..c8f50fe1ef1 100644 --- a/tests/include/mir/test/doubles/fake_renderable.h +++ b/tests/include/mir/test/doubles/fake_renderable.h @@ -92,6 +92,11 @@ class FakeRenderable : public graphics::Renderable { return rect; } + + geometry::RectangleD src_bounds() const override + { + return {{0, 0}, geometry::SizeD{buf->size()}}; + } std::optional clip_area() const override { diff --git a/tests/include/mir/test/doubles/mock_renderable.h b/tests/include/mir/test/doubles/mock_renderable.h index 0e3b667c29e..7e0160cbed3 100644 --- a/tests/include/mir/test/doubles/mock_renderable.h +++ b/tests/include/mir/test/doubles/mock_renderable.h @@ -48,6 +48,7 @@ struct MockRenderable : public graphics::Renderable MOCK_CONST_METHOD0(id, ID()); MOCK_CONST_METHOD0(buffer, std::shared_ptr()); MOCK_CONST_METHOD0(screen_position, geometry::Rectangle()); + MOCK_CONST_METHOD0(src_bounds, geometry::RectangleD()); MOCK_CONST_METHOD0(clip_area, std::optional()); MOCK_CONST_METHOD0(alpha, float()); MOCK_CONST_METHOD0(transformation, glm::mat4()); diff --git a/tests/include/mir/test/doubles/stub_renderable.h b/tests/include/mir/test/doubles/stub_renderable.h index f8ead8e46aa..943e6445e1c 100644 --- a/tests/include/mir/test/doubles/stub_renderable.h +++ b/tests/include/mir/test/doubles/stub_renderable.h @@ -72,6 +72,10 @@ class StubRenderable : public graphics::Renderable { return rect; } + geometry::RectangleD src_bounds() const override + { + return {{0, 0}, geometry::SizeD{stub_buffer->size()}}; + } std::optional clip_area() const override { return std::optional(); diff --git a/tests/platform_test_harness/graphics_platform_test_harness.cpp b/tests/platform_test_harness/graphics_platform_test_harness.cpp index ffb23f9d17a..6f8cb691d2f 100644 --- a/tests/platform_test_harness/graphics_platform_test_harness.cpp +++ b/tests/platform_test_harness/graphics_platform_test_harness.cpp @@ -426,6 +426,11 @@ void basic_software_buffer_drawing( return mir::geometry::Rectangle{top_left, buffer()->size()}; } + auto src_bounds() const -> mir::geometry::RectangleD override + { + return {{0, 0}, mir::geometry::SizeD{buffer()->size()}}; + } + auto alpha() const -> float override { return 1.0f; From 791cdb1417302c96539a96659212856eb0519e03 Mon Sep 17 00:00:00 2001 From: Christopher James Halse Rogers Date: Thu, 27 Jun 2024 17:03:30 +1000 Subject: [PATCH 72/81] tests/Tessellation: Make the `MockRenderable` fit the new expectations of the code --- tests/unit-tests/gl/test_tessellation_helpers.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/unit-tests/gl/test_tessellation_helpers.cpp b/tests/unit-tests/gl/test_tessellation_helpers.cpp index fe99461903e..1e1067c9d98 100644 --- a/tests/unit-tests/gl/test_tessellation_helpers.cpp +++ b/tests/unit-tests/gl/test_tessellation_helpers.cpp @@ -14,6 +14,7 @@ * along with this program. If not, see . */ +#include "mir/test/doubles/mock_buffer.h" #include #include #include @@ -24,6 +25,7 @@ namespace geom = mir::geometry; namespace mt = mir::test; namespace mtd = mir::test::doubles; namespace mgl = mir::gl; +namespace mg = mir::graphics; struct BoundingBox { @@ -60,6 +62,10 @@ class Tessellation : public testing::Test { ON_CALL(renderable, screen_position()) .WillByDefault(Return(rect)); + ON_CALL(renderable, src_bounds()) + .WillByDefault(Return(geom::RectangleD{{0, 0}, {geom::SizeD{rect.size}}})); + ON_CALL(renderable, buffer()) + .WillByDefault(Return(buffer_)); } static auto bounding_box(mgl::Primitive const& primitive) -> BoundingBox @@ -95,6 +101,7 @@ class Tessellation : public testing::Test geom::Rectangle const rect{{4, 6}, {10, 20}}; NiceMock const renderable; + std::shared_ptr const buffer_ = std::make_shared>(rect.size, geom::Stride{}, mir_pixel_format_argb_8888); }; TEST_F(Tessellation, has_4_vertices) From 71f77df74c40a95b88fa5b66e8b666c0e8e82ddc Mon Sep 17 00:00:00 2001 From: Christopher James Halse Rogers Date: Mon, 1 Jul 2024 16:59:59 +1000 Subject: [PATCH 73/81] frontend_wayland/WpViewporter: Encapsulate `Viewport` better. We can provide an interface to the rest of the frontend to calculate the details `WlSurface` needs and keep all the protocol error generation code in with the `wp_viewporter` rather than spread across multiple files. --- src/server/frontend_wayland/wl_surface.cpp | 90 +++---------------- src/server/frontend_wayland/wp_viewporter.cpp | 87 ++++++++++++++++++ src/server/frontend_wayland/wp_viewporter.h | 21 ++++- 3 files changed, 116 insertions(+), 82 deletions(-) diff --git a/src/server/frontend_wayland/wl_surface.cpp b/src/server/frontend_wayland/wl_surface.cpp index 5a1a24a889d..c20199f2634 100644 --- a/src/server/frontend_wayland/wl_surface.cpp +++ b/src/server/frontend_wayland/wl_surface.cpp @@ -323,30 +323,6 @@ void mf::WlSurface::set_input_region(std::optional const& region) } } -namespace -{ -auto as_int_if_exact(double d) -> std::optional -{ - if (std::trunc(d) == d && d < std::numeric_limits::max()) - { - return static_cast(d); - } - return {}; -} - -auto as_int_size_if_exact(geom::SizeD size) -> std::optional -{ - auto width = as_int_if_exact(size.width.as_value()); - auto height = as_int_if_exact(size.height.as_value()); - - if (width && height) - { - return geom::Size{*width, *height}; - } - return {}; -} -} - void mf::WlSurface::commit(WlSurfaceState const& state) { // We're going to lose the value of state, so copy the frame_callbacks first. We have to maintain a list of @@ -444,60 +420,18 @@ void mf::WlSurface::commit(WlSurfaceState const& state) if (needs_buffer_submission) { - auto const src_sample = - [&]() -> geom::RectangleD - { - auto const entire_buffer = geom::RectangleD{{0, 0}, geom::SizeD{current_buffer->size()}}; - if (viewport && viewport.value().source) - { - auto raw_source = viewport.value().source.value(); - /* Viewport coordinates are in buffer coordinates *after* applying transformation and scale. - * That means this rectangle needs to be scaled. (And have buffer transform applied, when we support that) - */ - geom::RectangleD source_in_buffer_coords{ - {raw_source.left().as_value() / inv_scale, raw_source.top().as_value() / inv_scale}, - raw_source.size / inv_scale - }; - if (!entire_buffer.contains(source_in_buffer_coords)) - { - throw wayland::ProtocolError{ - viewport.value().resource, - wayland::Viewport::Error::out_of_buffer, - "Source viewport (%f, %f), (%f × %f) is not entirely within buffer (%i × %i)", - source_in_buffer_coords.left().as_value(), - source_in_buffer_coords.top().as_value(), - source_in_buffer_coords.size.width.as_value(), - source_in_buffer_coords.size.height.as_value(), - current_buffer->size().width.as_uint32_t(), - current_buffer->size().height.as_uint32_t() - }; - } - return source_in_buffer_coords; - } - return entire_buffer; - }(); - auto const logical_size = - [&]() - { - if (viewport) - { - if (viewport.value().destination) - { - return viewport.value().destination.value(); - } - } - if (as_int_size_if_exact(src_sample.size)) - { - return as_int_size_if_exact(src_sample.size).value() * inv_scale; - } - else - { - throw wayland::ProtocolError{ - viewport.value().resource, - wayland::Viewport::Error::bad_size, - "No wp_viewport destination set, and source size (%f × %f) is not integral", src_sample.size.width.as_value(), src_sample.size.height.as_value()}; - } - }(); + geom::Size logical_size; + geom::RectangleD src_sample; + + if (viewport) + { + std::tie(src_sample, logical_size) = viewport.value().resolve_viewport(1.0f / inv_scale, current_buffer->size()); + } + else + { + src_sample = geom::RectangleD{{0, 0}, geom::SizeD{current_buffer->size()}}; + logical_size = current_buffer->size() * inv_scale; + } stream->submit_buffer(current_buffer, logical_size, src_sample); diff --git a/src/server/frontend_wayland/wp_viewporter.cpp b/src/server/frontend_wayland/wp_viewporter.cpp index 38db1365a85..ea0bc8caafa 100644 --- a/src/server/frontend_wayland/wp_viewporter.cpp +++ b/src/server/frontend_wayland/wp_viewporter.cpp @@ -24,6 +24,7 @@ #include namespace mf = mir::frontend; +namespace geom = mir::geometry; namespace { @@ -81,6 +82,92 @@ auto mf::Viewport::dirty() -> bool return std::exchange(dirty_, false); } +namespace +{ +auto as_int_if_exact(double d) -> std::optional +{ + if (std::trunc(d) == d && d < std::numeric_limits::max()) + { + return static_cast(d); + } + return {}; +} + +auto as_int_size_if_exact(geom::SizeD size) -> std::optional +{ + auto width = as_int_if_exact(size.width.as_value()); + auto height = as_int_if_exact(size.height.as_value()); + + if (width && height) + { + return geom::Size{*width, *height}; + } + return {}; +} +} + +auto mf::Viewport::resolve_viewport(int32_t scale, geom::Size buffer_size) const + -> std::pair +{ + auto const src_bounds = + [&]() + { + auto const entire_buffer = geom::RectangleD{{0, 0}, geom::SizeD{buffer_size}}; + if (source) + { + auto raw_source = source.value(); + /* Viewport coordinates are in buffer coordinates *after* applying transformation and scale. + * That means this rectangle needs to be scaled. (And have buffer transform applied, when we support that) + */ + geom::RectangleD source_in_buffer_coords{ + {raw_source.left().as_value() * scale, raw_source.top().as_value() * scale}, + raw_source.size * scale + }; + if (!entire_buffer.contains(source_in_buffer_coords)) + { + throw wayland::ProtocolError{ + resource, + wayland::Viewport::Error::out_of_buffer, + "Source viewport (%f, %f), (%f × %f) is not entirely within buffer (%i × %i)", + source_in_buffer_coords.left().as_value(), + source_in_buffer_coords.top().as_value(), + source_in_buffer_coords.size.width.as_value(), + source_in_buffer_coords.size.height.as_value(), + buffer_size.width.as_uint32_t(), + buffer_size.height.as_uint32_t() + }; + } + return source_in_buffer_coords; + } + else + { + return entire_buffer; + } + }(); + + auto const logical_size = + [&]() + { + if (destination) + { + return destination.value(); + } + if (as_int_size_if_exact(src_bounds.size)) + { + return as_int_size_if_exact(src_bounds.size).value() / scale; + } + else + { + throw wayland::ProtocolError{ + resource, + wayland::Viewport::Error::bad_size, + "No wp_viewport destination set, and source size (%f × %f) is not integral", src_bounds.size.width.as_value(), src_bounds.size.height.as_value()}; + } + }(); + + return std::make_pair(src_bounds, logical_size); +} + void mf::Viewport::set_source(double x, double y, double width, double height) { if (!surface) diff --git a/src/server/frontend_wayland/wp_viewporter.h b/src/server/frontend_wayland/wp_viewporter.h index 694d4868ee9..c40299e852d 100644 --- a/src/server/frontend_wayland/wp_viewporter.h +++ b/src/server/frontend_wayland/wp_viewporter.h @@ -39,26 +39,39 @@ class WpViewporter : public wayland::Viewporter::Global void bind(wl_resource* new_wp_viewporter) override; }; +/** + * Manages the wp_viewport state + * + * Threadsafety: This is a Wayland object, and should only be accessed from the Wayland thread + */ class Viewport : public wayland::Viewport { public: Viewport(wl_resource* new_viewport, WlSurface* surface); ~Viewport() override; - // Threadsafety: This is a Wayland object, these should only be referenced on the Wayland mainloop - std::optional source; - std::optional destination; - /** * Have the source or destination values changed since the last call to dirty()? */ auto dirty() -> bool; + + /** + * Combine the viewport parameters with submitted buffer and scale to produce final parameters + * + * \returns The source viewport into the buffer, in buffer coordinates, and + * The logical (post-scaling) size of this wl_surface + * \throws A wayland::ProtocolError if any of the wp_viewporter preconditions are not met. + */ + auto resolve_viewport(int32_t scale, geometry::Size buffer_size) const + -> std::pair; private: void set_source(double x, double y, double width, double height) override; void set_destination(int32_t width, int32_t height) override; bool dirty_; wayland::Weak surface; + std::optional source; + std::optional destination; }; } From 15f7a54625e911d023eb381d8c346eaf00115d64 Mon Sep 17 00:00:00 2001 From: Christopher James Halse Rogers Date: Mon, 1 Jul 2024 17:05:55 +1000 Subject: [PATCH 74/81] frontend_wayland/WlSurface: Store `scale` directly. We now have a requirement for both the actual value of `scale` and for its reciprocal. Rather than storing the reciprocal and calculating the original value when needed, store the original value and calculate the reciprocal when needed. --- src/server/frontend_wayland/wl_surface.cpp | 6 +++--- src/server/frontend_wayland/wl_surface.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/server/frontend_wayland/wl_surface.cpp b/src/server/frontend_wayland/wl_surface.cpp index c20199f2634..8323f8c511b 100644 --- a/src/server/frontend_wayland/wl_surface.cpp +++ b/src/server/frontend_wayland/wl_surface.cpp @@ -337,7 +337,7 @@ void mf::WlSurface::commit(WlSurfaceState const& state) input_shape = state.input_shape.value(); if (state.scale) - inv_scale = 1.0f / state.scale.value(); + scale = state.scale.value(); if (state.viewport) { @@ -425,12 +425,12 @@ void mf::WlSurface::commit(WlSurfaceState const& state) if (viewport) { - std::tie(src_sample, logical_size) = viewport.value().resolve_viewport(1.0f / inv_scale, current_buffer->size()); + std::tie(src_sample, logical_size) = viewport.value().resolve_viewport(scale, current_buffer->size()); } else { src_sample = geom::RectangleD{{0, 0}, geom::SizeD{current_buffer->size()}}; - logical_size = current_buffer->size() * inv_scale; + logical_size = current_buffer->size() / scale; } stream->submit_buffer(current_buffer, logical_size, src_sample); diff --git a/src/server/frontend_wayland/wl_surface.h b/src/server/frontend_wayland/wl_surface.h index 3caffec057e..d658f5f8483 100644 --- a/src/server/frontend_wayland/wl_surface.h +++ b/src/server/frontend_wayland/wl_surface.h @@ -177,7 +177,7 @@ class WlSurface : public wayland::Surface WlSurfaceState pending; geometry::Displacement offset_; - float inv_scale{1.0f}; + int32_t scale{1}; std::optional buffer_size_; std::vector> frame_callbacks; std::optional> input_shape; From 7457c7e228f6db4a285db3a0b9e04f95dd0665f2 Mon Sep 17 00:00:00 2001 From: Christopher James Halse Rogers Date: Tue, 2 Jul 2024 12:46:07 +1000 Subject: [PATCH 75/81] WpViewporter: Single argument constructors should be `explicit` Co-authored-by: Alan Griffiths --- src/server/frontend_wayland/wp_viewporter.cpp | 2 +- src/server/frontend_wayland/wp_viewporter.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/frontend_wayland/wp_viewporter.cpp b/src/server/frontend_wayland/wp_viewporter.cpp index ea0bc8caafa..812e3bf5779 100644 --- a/src/server/frontend_wayland/wp_viewporter.cpp +++ b/src/server/frontend_wayland/wp_viewporter.cpp @@ -31,7 +31,7 @@ namespace class ViewporterInstance : public mir::wayland::Viewporter { public: - ViewporterInstance(wl_resource* resource) + explicit ViewporterInstance(wl_resource* resource) : Viewporter(resource, Version<1>{}) { } diff --git a/src/server/frontend_wayland/wp_viewporter.h b/src/server/frontend_wayland/wp_viewporter.h index c40299e852d..7e88bf03376 100644 --- a/src/server/frontend_wayland/wp_viewporter.h +++ b/src/server/frontend_wayland/wp_viewporter.h @@ -33,7 +33,7 @@ class WlSurface; class WpViewporter : public wayland::Viewporter::Global { public: - WpViewporter(wl_display* display); + explicit WpViewporter(wl_display* display); private: void bind(wl_resource* new_wp_viewporter) override; From fbe9df2284f08bc7b14822d65cfe74a4c9fa92f3 Mon Sep 17 00:00:00 2001 From: Christopher James Halse Rogers Date: Tue, 2 Jul 2024 13:10:31 +1000 Subject: [PATCH 76/81] Drop unnecessary and accidentally-added `geometry/forward.h` `#include` --- src/gl/tessellation_helpers.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/gl/tessellation_helpers.cpp b/src/gl/tessellation_helpers.cpp index 60649c554c1..f7e9addddb6 100644 --- a/src/gl/tessellation_helpers.cpp +++ b/src/gl/tessellation_helpers.cpp @@ -14,7 +14,6 @@ * along with this program. If not, see . */ #include "mir/gl/tessellation_helpers.h" -#include "mir/geometry/forward.h" #include "mir/graphics/renderable.h" #include "mir/graphics/buffer.h" From 629f063d8b092f487775fe45bd21cb62baa4101f Mon Sep 17 00:00:00 2001 From: Christopher James Halse Rogers Date: Tue, 9 Jul 2024 12:28:26 +1000 Subject: [PATCH 77/81] geometry/Size: Add non-lossy implicit conversions between underlying types. It's fine to implicitly convert `Size` -> `SizeD` or `SizeF` -> `SizeD`; these conversions lose no information. Use this to simplify some `RectangleD` constructions in the code --- include/core/mir/geometry/size.h | 18 ++++++++++++++++++ src/server/graphics/software_cursor.cpp | 2 +- src/server/input/touchspot_controller.cpp | 2 +- src/server/shell/basic_idle_handler.cpp | 2 +- .../include/mir/test/doubles/fake_renderable.h | 2 +- .../include/mir/test/doubles/stub_renderable.h | 2 +- .../graphics_platform_test_harness.cpp | 2 +- tests/unit-tests/geometry/test-size.cpp | 7 +++++++ 8 files changed, 31 insertions(+), 6 deletions(-) diff --git a/include/core/mir/geometry/size.h b/include/core/mir/geometry/size.h index cd49b723df2..14885a0952e 100644 --- a/include/core/mir/geometry/size.h +++ b/include/core/mir/geometry/size.h @@ -20,6 +20,9 @@ #include "forward.h" #include "dimensions.h" #include +#include +#include +#include namespace mir { @@ -32,6 +35,13 @@ struct Point; template struct Displacement; +template +concept is_exactly_representable = requires +{ + typename std::enable_if::digits >= std::numeric_limits::digits, U>::type; + requires std::floating_point; +}; + template struct Size { @@ -41,6 +51,14 @@ struct Size constexpr Size(Size const&) noexcept = default; Size& operator=(Size const&) noexcept = default; + template + requires is_exactly_representable + constexpr Size(Size const& other) noexcept + : width{Width{other.width}}, + height{Height{other.height}} + { + } + template explicit constexpr Size(Size const& other) noexcept : width{Width{other.width}}, diff --git a/src/server/graphics/software_cursor.cpp b/src/server/graphics/software_cursor.cpp index e99822a1640..2ab0bd4df10 100644 --- a/src/server/graphics/software_cursor.cpp +++ b/src/server/graphics/software_cursor.cpp @@ -82,7 +82,7 @@ class mg::detail::CursorRenderable : public mg::Renderable geom::RectangleD src_bounds() const override { - return {geom::PointD{0, 0}, geom::SizeD{buffer_->size()}}; + return {{0, 0}, buffer_->size()}; } std::optional clip_area() const override diff --git a/src/server/input/touchspot_controller.cpp b/src/server/input/touchspot_controller.cpp index 50a19bf5d6c..a4ac78ada4f 100644 --- a/src/server/input/touchspot_controller.cpp +++ b/src/server/input/touchspot_controller.cpp @@ -65,7 +65,7 @@ class mi::TouchspotRenderable : public mg::Renderable geom::RectangleD src_bounds() const override { - return {{0, 0}, geom::SizeD{buffer_->size()}}; + return {{0, 0}, buffer_->size()}; } std::optional clip_area() const override diff --git a/src/server/shell/basic_idle_handler.cpp b/src/server/shell/basic_idle_handler.cpp index af4985703f1..51057549a79 100644 --- a/src/server/shell/basic_idle_handler.cpp +++ b/src/server/shell/basic_idle_handler.cpp @@ -67,7 +67,7 @@ struct DimmingRenderable : public mg::Renderable auto src_bounds() const -> geom::RectangleD override { - return {{0, 0}, geom::SizeD{buffer_->size()}}; + return {{0, 0}, buffer_->size()}; } auto clip_area() const -> std::optional override diff --git a/tests/include/mir/test/doubles/fake_renderable.h b/tests/include/mir/test/doubles/fake_renderable.h index c8f50fe1ef1..5b10f861a64 100644 --- a/tests/include/mir/test/doubles/fake_renderable.h +++ b/tests/include/mir/test/doubles/fake_renderable.h @@ -95,7 +95,7 @@ class FakeRenderable : public graphics::Renderable geometry::RectangleD src_bounds() const override { - return {{0, 0}, geometry::SizeD{buf->size()}}; + return {{0, 0}, buf->size()}; } std::optional clip_area() const override diff --git a/tests/include/mir/test/doubles/stub_renderable.h b/tests/include/mir/test/doubles/stub_renderable.h index 943e6445e1c..d7554b53b91 100644 --- a/tests/include/mir/test/doubles/stub_renderable.h +++ b/tests/include/mir/test/doubles/stub_renderable.h @@ -74,7 +74,7 @@ class StubRenderable : public graphics::Renderable } geometry::RectangleD src_bounds() const override { - return {{0, 0}, geometry::SizeD{stub_buffer->size()}}; + return {{0, 0}, stub_buffer->size()}; } std::optional clip_area() const override { diff --git a/tests/platform_test_harness/graphics_platform_test_harness.cpp b/tests/platform_test_harness/graphics_platform_test_harness.cpp index 6f8cb691d2f..58321760d28 100644 --- a/tests/platform_test_harness/graphics_platform_test_harness.cpp +++ b/tests/platform_test_harness/graphics_platform_test_harness.cpp @@ -428,7 +428,7 @@ void basic_software_buffer_drawing( auto src_bounds() const -> mir::geometry::RectangleD override { - return {{0, 0}, mir::geometry::SizeD{buffer()->size()}}; + return {{0, 0}, buffer()->size()}; } auto alpha() const -> float override diff --git a/tests/unit-tests/geometry/test-size.cpp b/tests/unit-tests/geometry/test-size.cpp index cb0fc684730..8c0fba7b7a8 100644 --- a/tests/unit-tests/geometry/test-size.cpp +++ b/tests/unit-tests/geometry/test-size.cpp @@ -61,3 +61,10 @@ TEST(geometry, size_can_be_converted_from_float_to_int) EXPECT_EQ(Width(1), i.width); EXPECT_EQ(Height(3), i.height); } + +// Test that non-lossy implicit conversions exist +static_assert(std::convertible_to, "Implicit conversion int->double preserves precision"); +static_assert(std::convertible_to, "Implicit conversion float->double preserves precision"); +// Test that lossy implicit conversions fail +static_assert(!std::convertible_to, "Conversion float->int is lossy"); +static_assert(!std::convertible_to, "Conversion double->float is lossy"); From 279c854ab0b3613f8081a64452120b004ff7b2cc Mon Sep 17 00:00:00 2001 From: Christopher James Halse Rogers Date: Tue, 9 Jul 2024 13:54:12 +1000 Subject: [PATCH 78/81] geometry/Dimensions: Add `X/Width` and `Y/Height` operators. --- include/core/mir/geometry/dimensions.h | 6 ++++++ src/gl/tessellation_helpers.cpp | 8 ++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/include/core/mir/geometry/dimensions.h b/include/core/mir/geometry/dimensions.h index 812b051af4d..f8518e6f2ad 100644 --- a/include/core/mir/geometry/dimensions.h +++ b/include/core/mir/geometry/dimensions.h @@ -200,6 +200,12 @@ template inline constexpr DeltaX operator/(DeltaX const& dx, Scalar scale) { return DeltaX{dx.as_value() / scale}; } template inline constexpr DeltaY operator/(DeltaY const& dy, Scalar scale) { return DeltaY{dy.as_value() / scale}; } + +// Can divide a coördinate by a distance to get a scalar +template +inline constexpr auto operator/(X const& x, Width const& width) { return x.as_value() / width.as_value(); } +template +inline constexpr auto operator/(Y const& y, Height const& height) { return y.as_value() / height.as_value(); } } // namespace // Converting between types is fine, as long as they are along the same axis diff --git a/src/gl/tessellation_helpers.cpp b/src/gl/tessellation_helpers.cpp index f7e9addddb6..26bfca6d405 100644 --- a/src/gl/tessellation_helpers.cpp +++ b/src/gl/tessellation_helpers.cpp @@ -37,10 +37,10 @@ auto tex_coords_from_rect(geom::Size buffer_size, geom::RectangleD sample_rect) * and (1.0, 1.0) is the bottom-right */ SrcTexCoords coords; - coords.top = sample_rect.top().as_value() / buffer_size.height.as_uint32_t(); - coords.bottom = sample_rect.bottom().as_value() / buffer_size.height.as_uint32_t(); - coords.left = sample_rect.left().as_value() / buffer_size.width.as_uint32_t(); - coords.right = sample_rect.right().as_value() / buffer_size.width.as_uint32_t(); + coords.top = sample_rect.top() / buffer_size.height; + coords.bottom = sample_rect.bottom() / buffer_size.height; + coords.left = sample_rect.left() / buffer_size.width; + coords.right = sample_rect.right() / buffer_size.width; return coords; } } From 2e8880d3ccefb22674c8122afa5a4ce7496fa4ad Mon Sep 17 00:00:00 2001 From: Christopher James Halse Rogers Date: Tue, 9 Jul 2024 14:07:44 +1000 Subject: [PATCH 79/81] frontend_wayland/WpViewporter: Better name for `dirty()` We change this to `changed_since_last_resolve()`. --- src/server/frontend_wayland/wl_surface.cpp | 10 +++++----- src/server/frontend_wayland/wp_viewporter.cpp | 9 +++++---- src/server/frontend_wayland/wp_viewporter.h | 6 +++--- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/server/frontend_wayland/wl_surface.cpp b/src/server/frontend_wayland/wl_surface.cpp index 8323f8c511b..2ac9b1869d9 100644 --- a/src/server/frontend_wayland/wl_surface.cpp +++ b/src/server/frontend_wayland/wl_surface.cpp @@ -345,11 +345,11 @@ void mf::WlSurface::commit(WlSurfaceState const& state) } bool needs_buffer_submission = - state.scale || // If the scale has changed, or... - state.viewport || // ...we've added a viewport, or... - (viewport && viewport.value().dirty()); // ...the viewport has changed... - // ...then we'll need to submit a new frame, even if the client hasn't - // attached a new buffer. + state.scale || // If the scale has changed, or... + state.viewport || // ...we've added a viewport, or... + (viewport && viewport.value().changed_since_last_resolve()); // ...the viewport has changed... + // ...then we'll need to submit a new frame, even if the client hasn't + // attached a new buffer. auto const executor_send_frame_callbacks = [executor = wayland_executor, weak_self = mw::make_weak(this)]() { diff --git a/src/server/frontend_wayland/wp_viewporter.cpp b/src/server/frontend_wayland/wp_viewporter.cpp index 812e3bf5779..3a65e05a064 100644 --- a/src/server/frontend_wayland/wp_viewporter.cpp +++ b/src/server/frontend_wayland/wp_viewporter.cpp @@ -77,9 +77,9 @@ mf::Viewport::~Viewport() { } -auto mf::Viewport::dirty() -> bool +auto mf::Viewport::changed_since_last_resolve() -> bool { - return std::exchange(dirty_, false); + return viewport_changed; } namespace @@ -165,6 +165,7 @@ auto mf::Viewport::resolve_viewport(int32_t scale, geom::Size buffer_size) const } }(); + viewport_changed = false; return std::make_pair(src_bounds, logical_size); } @@ -200,7 +201,7 @@ void mf::Viewport::set_source(double x, double y, double width, double height) { source = geometry::RectangleD{{x, y}, {width, height}}; } - dirty_ = true; + viewport_changed = true; } void mf::Viewport::set_destination(int32_t width, int32_t height) @@ -233,5 +234,5 @@ void mf::Viewport::set_destination(int32_t width, int32_t height) destination = geometry::Size{width, height}; } - dirty_ = true; + viewport_changed = true; } diff --git a/src/server/frontend_wayland/wp_viewporter.h b/src/server/frontend_wayland/wp_viewporter.h index 7e88bf03376..139bd80995b 100644 --- a/src/server/frontend_wayland/wp_viewporter.h +++ b/src/server/frontend_wayland/wp_viewporter.h @@ -51,9 +51,9 @@ class Viewport : public wayland::Viewport ~Viewport() override; /** - * Have the source or destination values changed since the last call to dirty()? + * Have the source or destination values changed since the last `resolve_viewport`? */ - auto dirty() -> bool; + auto changed_since_last_resolve() -> bool; /** * Combine the viewport parameters with submitted buffer and scale to produce final parameters @@ -68,7 +68,7 @@ class Viewport : public wayland::Viewport void set_source(double x, double y, double width, double height) override; void set_destination(int32_t width, int32_t height) override; - bool dirty_; + bool mutable viewport_changed; wayland::Weak surface; std::optional source; std::optional destination; From 3c948ffaaa8dcf6a1eadffabab65b19c5609cb12 Mon Sep 17 00:00:00 2001 From: Christopher James Halse Rogers Date: Tue, 9 Jul 2024 14:08:08 +1000 Subject: [PATCH 80/81] Use the implicit `Size` -> `SizeD` conversion a bit more --- src/server/frontend_wayland/wl_surface.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/frontend_wayland/wl_surface.cpp b/src/server/frontend_wayland/wl_surface.cpp index 2ac9b1869d9..3d80c10c604 100644 --- a/src/server/frontend_wayland/wl_surface.cpp +++ b/src/server/frontend_wayland/wl_surface.cpp @@ -429,7 +429,7 @@ void mf::WlSurface::commit(WlSurfaceState const& state) } else { - src_sample = geom::RectangleD{{0, 0}, geom::SizeD{current_buffer->size()}}; + src_sample = geom::RectangleD{{0, 0}, current_buffer->size()}; logical_size = current_buffer->size() / scale; } From 45d7c8d24bb0100a730b634b2e19bac93bdcf878 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Thu, 11 Jul 2024 10:24:03 +0100 Subject: [PATCH 81/81] Revert spurious change --- src/include/gl/mir/gl/tessellation_helpers.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/include/gl/mir/gl/tessellation_helpers.h b/src/include/gl/mir/gl/tessellation_helpers.h index 57569778302..0c8b0bbd320 100644 --- a/src/include/gl/mir/gl/tessellation_helpers.h +++ b/src/include/gl/mir/gl/tessellation_helpers.h @@ -18,7 +18,6 @@ #define MIR_GL_TESSELLATION_HELPERS_H_ #include "mir/gl/primitive.h" #include "mir/geometry/displacement.h" -#include "mir/geometry/rectangle.h" namespace mir {