From ab38a60e217f97c24152fe5f42667008b84e9dd3 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Fri, 17 Jan 2025 09:34:36 -0500 Subject: [PATCH] Adding an is_above method to the SurfaceStack so that compositor authors can write tests to assert the relative z-order of surfaces --- CMakeLists.txt | 2 +- debian/control | 6 +-- debian/libmirserver62.install | 1 - debian/libmirserver63.install | 1 + src/include/server/mir/shell/abstract_shell.h | 1 + .../server/mir/shell/focus_controller.h | 4 ++ src/include/server/mir/shell/shell_wrapper.h | 2 + src/include/server/mir/shell/surface_stack.h | 3 ++ .../server/mir/shell/surface_stack_wrapper.h | 2 + src/server/CMakeLists.txt | 2 +- src/server/scene/surface_stack.cpp | 27 +++++++++++- src/server/scene/surface_stack.h | 2 + src/server/shell/abstract_shell.cpp | 6 +++ src/server/shell/shell_wrapper.cpp | 8 +++- src/server/shell/surface_stack_wrapper.cpp | 7 ++++ src/server/symbols.map | 14 +++++++ .../mir/test/doubles/mock_surface_stack.h | 1 + tests/include/mir/test/doubles/stub_shell.h | 5 +++ .../window_management_test_harness.h | 3 ++ .../window_management_test_harness.cpp | 11 +++++ tests/miral/test_window_manager_tools.cpp | 5 +++ .../scene/test_application_session.cpp | 5 +++ .../test_minimal_window_manager.cpp | 41 ++++++++++++++++++- 23 files changed, 150 insertions(+), 9 deletions(-) delete mode 100644 debian/libmirserver62.install create mode 100644 debian/libmirserver63.install diff --git a/CMakeLists.txt b/CMakeLists.txt index 4f64ab56a3d..0f90dc80719 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -25,7 +25,7 @@ set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin) set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib) set(MIR_VERSION_MAJOR 2) -set(MIR_VERSION_MINOR 19) +set(MIR_VERSION_MINOR 20) set(MIR_VERSION_PATCH 2) add_compile_definitions(MIR_VERSION_MAJOR=${MIR_VERSION_MAJOR}) diff --git a/debian/control b/debian/control index bfa0834ede4..5f7ef304ecb 100644 --- a/debian/control +++ b/debian/control @@ -64,7 +64,7 @@ Vcs-Git: https://github.com/MirServer/mir #TODO: Packaging infrastructure for better dependency generation, # ala pkg-xorg's xviddriver:Provides and ABI detection. -Package: libmirserver62 +Package: libmirserver63 Section: libs Architecture: linux-any Multi-Arch: same @@ -158,7 +158,7 @@ Section: libdevel Architecture: linux-any Multi-Arch: same Pre-Depends: ${misc:Pre-Depends} -Depends: libmirserver62 (= ${binary:Version}), +Depends: libmirserver63 (= ${binary:Version}), libmirplatform-dev (= ${binary:Version}), libmircommon-dev (= ${binary:Version}), libglm-dev, @@ -175,7 +175,7 @@ Section: libdevel Architecture: linux-any Multi-Arch: same Pre-Depends: ${misc:Pre-Depends} -Depends: libmirserver62 (= ${binary:Version}), +Depends: libmirserver63 (= ${binary:Version}), libmirplatform-dev (= ${binary:Version}), libmircommon-dev (= ${binary:Version}), libmircore-dev (= ${binary:Version}), diff --git a/debian/libmirserver62.install b/debian/libmirserver62.install deleted file mode 100644 index 7b6662070a7..00000000000 --- a/debian/libmirserver62.install +++ /dev/null @@ -1 +0,0 @@ -usr/lib/*/libmirserver.so.62 diff --git a/debian/libmirserver63.install b/debian/libmirserver63.install new file mode 100644 index 00000000000..cbc1333e60d --- /dev/null +++ b/debian/libmirserver63.install @@ -0,0 +1 @@ +usr/lib/*/libmirserver.so.63 diff --git a/src/include/server/mir/shell/abstract_shell.h b/src/include/server/mir/shell/abstract_shell.h index ba01ffa3794..a3ae367fb91 100644 --- a/src/include/server/mir/shell/abstract_shell.h +++ b/src/include/server/mir/shell/abstract_shell.h @@ -132,6 +132,7 @@ class AbstractShell : public virtual Shell, public virtual FocusController void raise(SurfaceSet const& surfaces) override; void swap_z_order(SurfaceSet const& first, SurfaceSet const& second) override; void send_to_back(SurfaceSet const& surfaces) override; + auto is_above(std::weak_ptr const& a, std::weak_ptr const& b) const -> bool override; /** @} */ void add_display(geometry::Rectangle const& area) override; diff --git a/src/include/server/mir/shell/focus_controller.h b/src/include/server/mir/shell/focus_controller.h index 93010102512..349d621c03e 100644 --- a/src/include/server/mir/shell/focus_controller.h +++ b/src/include/server/mir/shell/focus_controller.h @@ -59,6 +59,10 @@ class FocusController virtual void send_to_back(SurfaceSet const& surfaces) = 0; + /// Returns true if surface [a] is above surface [b]. + virtual auto is_above(std::weak_ptr const& a, std::weak_ptr const& b) const + -> bool = 0; + protected: FocusController() = default; FocusController(FocusController const&) = delete; diff --git a/src/include/server/mir/shell/shell_wrapper.h b/src/include/server/mir/shell/shell_wrapper.h index 70c1b004b73..3b4b86f87b5 100644 --- a/src/include/server/mir/shell/shell_wrapper.h +++ b/src/include/server/mir/shell/shell_wrapper.h @@ -49,6 +49,8 @@ class ShellWrapper : public Shell void send_to_back(SurfaceSet const& surfaces) override; + auto is_above(std::weak_ptr const& a, std::weak_ptr const& b) const -> bool override; + auto open_session( pid_t client_pid, Fd socket_fd, diff --git a/src/include/server/mir/shell/surface_stack.h b/src/include/server/mir/shell/surface_stack.h index cb586c6d2c5..000ed0ec4fe 100644 --- a/src/include/server/mir/shell/surface_stack.h +++ b/src/include/server/mir/shell/surface_stack.h @@ -55,6 +55,9 @@ class SurfaceStack virtual void send_to_back(scene::SurfaceSet const& surfaces) = 0; + virtual auto is_above(std::weak_ptr const& a, std::weak_ptr const& b) const + -> bool = 0; + protected: SurfaceStack() = default; virtual ~SurfaceStack() = default; diff --git a/src/include/server/mir/shell/surface_stack_wrapper.h b/src/include/server/mir/shell/surface_stack_wrapper.h index de0376c026d..52b5a611653 100644 --- a/src/include/server/mir/shell/surface_stack_wrapper.h +++ b/src/include/server/mir/shell/surface_stack_wrapper.h @@ -44,6 +44,8 @@ class SurfaceStackWrapper : public SurfaceStack void send_to_back(scene::SurfaceSet const& surfaces) override; + auto is_above(std::weak_ptr const& a, std::weak_ptr const& b) const -> bool override; + protected: std::shared_ptr const wrapped; }; diff --git a/src/server/CMakeLists.txt b/src/server/CMakeLists.txt index aa8d2e25a0d..5253fe06b2e 100644 --- a/src/server/CMakeLists.txt +++ b/src/server/CMakeLists.txt @@ -168,7 +168,7 @@ install(DIRECTORY ${CMAKE_SOURCE_DIR}/src/include/server/mir DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/mirserver-internal" ) -set(MIRSERVER_ABI 62) # Be sure to increment MIR_VERSION_MINOR at the same time +set(MIRSERVER_ABI 63) # Be sure to increment MIR_VERSION_MINOR at the same time set(symbol_map ${CMAKE_CURRENT_SOURCE_DIR}/symbols.map) set_target_properties( diff --git a/src/server/scene/surface_stack.cpp b/src/server/scene/surface_stack.cpp index ee6b4c627b1..c656b9ee6f7 100644 --- a/src/server/scene/surface_stack.cpp +++ b/src/server/scene/surface_stack.cpp @@ -126,7 +126,6 @@ struct SurfaceDepthLayerObserver : ms::NullSurfaceObserver private: ms::SurfaceStack* stack; }; - } ms::SurfaceStack::SurfaceStack(std::shared_ptr const& report) : @@ -493,6 +492,32 @@ void ms::SurfaceStack::send_to_back(const mir::scene::SurfaceSet &ss) } } +auto ms::SurfaceStack::is_above(std::weak_ptr const& a, std::weak_ptr const& b) const -> bool +{ + RecursiveReadLock lg(guard); + if (a.expired()) + return false; + else if (b.expired()) + return true; + + auto const shared_a = a.lock(); + auto const shared_b = b.lock(); + if (shared_a->depth_layer() < shared_b->depth_layer()) + return false; + else if (shared_a->depth_layer() > shared_b->depth_layer()) + return true; + + auto const& layer = surface_layers[shared_a->depth_layer()]; + bool found_b = false; + for (auto const& entry : layer) + { + if (entry == shared_b) found_b = true; + if (entry == shared_a) return found_b; + } + + return false; +} + void ms::SurfaceStack::create_rendering_tracker_for(std::shared_ptr const& surface) { auto const tracker = std::make_shared(surface); diff --git a/src/server/scene/surface_stack.h b/src/server/scene/surface_stack.h index 92eb0c48489..8c63f157abe 100644 --- a/src/server/scene/surface_stack.h +++ b/src/server/scene/surface_stack.h @@ -101,6 +101,8 @@ class SurfaceStack : void raise(SurfaceSet const& surfaces) override; void swap_z_order(SurfaceSet const& first, SurfaceSet const& second) override; void send_to_back(SurfaceSet const& surfaces) override; + auto is_above(std::weak_ptr const& a, std::weak_ptr const& b) const + -> bool override; void add_surface( std::shared_ptr const& surface, diff --git a/src/server/shell/abstract_shell.cpp b/src/server/shell/abstract_shell.cpp index f6972e454b7..c4be09d5ece 100644 --- a/src/server/shell/abstract_shell.cpp +++ b/src/server/shell/abstract_shell.cpp @@ -734,3 +734,9 @@ void msh::AbstractShell::send_to_back(const mir::shell::SurfaceSet &surfaces) { surface_stack->send_to_back(surfaces); } + +auto msh::AbstractShell::is_above(std::weak_ptr const& a, std::weak_ptr const& b) const + -> bool +{ + return surface_stack->is_above(a, b); +} diff --git a/src/server/shell/shell_wrapper.cpp b/src/server/shell/shell_wrapper.cpp index d89b9784865..760a0692e7d 100644 --- a/src/server/shell/shell_wrapper.cpp +++ b/src/server/shell/shell_wrapper.cpp @@ -180,4 +180,10 @@ void msh::ShellWrapper::swap_z_order(SurfaceSet const& first, SurfaceSet const& void msh::ShellWrapper::send_to_back(const mir::shell::SurfaceSet &surfaces) { return wrapped->send_to_back(surfaces); -} \ No newline at end of file +} + +auto msh::ShellWrapper::is_above(std::weak_ptr const& a, std::weak_ptr const& b) const + -> bool +{ + return wrapped->is_above(a, b); +} diff --git a/src/server/shell/surface_stack_wrapper.cpp b/src/server/shell/surface_stack_wrapper.cpp index e161c8cc350..e49fb53f66d 100644 --- a/src/server/shell/surface_stack_wrapper.cpp +++ b/src/server/shell/surface_stack_wrapper.cpp @@ -62,4 +62,11 @@ void msh::SurfaceStackWrapper::swap_z_order(scene::SurfaceSet const& first, scen void msh::SurfaceStackWrapper::send_to_back(const scene::SurfaceSet &surfaces) { wrapped->send_to_back(surfaces); +} + +auto msh::SurfaceStackWrapper::is_above( + std::weak_ptr const& a, + std::weak_ptr const& b) const -> bool +{ + return wrapped->is_above(a, b); } \ No newline at end of file diff --git a/src/server/symbols.map b/src/server/symbols.map index b80c8321250..470eec78b1e 100644 --- a/src/server/symbols.map +++ b/src/server/symbols.map @@ -1404,3 +1404,17 @@ global: local: *; }; +MIR_SERVER_INTERNAL_2.20 { +global: + extern "C++" { + mir::shell::AbstractShell::is_above*; + mir::shell::ShellWrapper::is_above*; + mir::shell::SurfaceStackWrapper::is_above*; + non-virtual?thunk?to?mir::shell::AbstractShell::is_above*; + non-virtual?thunk?to?mir::shell::ShellWrapper::is_above*; + non-virtual?thunk?to?mir::shell::SurfaceStackWrapper::is_above*; + virtual?thunk?to?mir::shell::AbstractShell::is_above*; + virtual?thunk?to?mir::shell::ShellWrapper::is_above*; + }; +} MIR_SERVER_INTERNAL_2.19; + diff --git a/tests/include/mir/test/doubles/mock_surface_stack.h b/tests/include/mir/test/doubles/mock_surface_stack.h index 938a1112b8c..6e3504433ac 100644 --- a/tests/include/mir/test/doubles/mock_surface_stack.h +++ b/tests/include/mir/test/doubles/mock_surface_stack.h @@ -40,6 +40,7 @@ struct MockSurfaceStack : public shell::SurfaceStack MOCK_CONST_METHOD1(surface_at, std::shared_ptr(geometry::Point)); MOCK_METHOD2(swap_z_order, void(scene::SurfaceSet const&, scene::SurfaceSet const&)); MOCK_METHOD1(send_to_back, void(scene::SurfaceSet const&)); + MOCK_CONST_METHOD2(is_above, bool(std::weak_ptr const& a, std::weak_ptr const& b)); }; } diff --git a/tests/include/mir/test/doubles/stub_shell.h b/tests/include/mir/test/doubles/stub_shell.h index 9ee1f18a9e3..8b8a3105bae 100644 --- a/tests/include/mir/test/doubles/stub_shell.h +++ b/tests/include/mir/test/doubles/stub_shell.h @@ -95,6 +95,11 @@ struct StubShell : public shell::Shell void send_to_back(shell::SurfaceSet const& /*surfaces*/) override { } + + auto is_above(std::weak_ptr const& /*a*/, std::weak_ptr const& /*b*/) const -> bool override + { + return false; + } /// @} /// Overrides from shell::Shell diff --git a/tests/include/mir_test_framework/window_management_test_harness.h b/tests/include/mir_test_framework/window_management_test_harness.h index 88a7825565f..1af2a0e0d40 100644 --- a/tests/include/mir_test_framework/window_management_test_harness.h +++ b/tests/include/mir_test_framework/window_management_test_harness.h @@ -56,9 +56,12 @@ class WindowManagementTestHarness : public mir_test_framework::HeadlessInProcess void publish_event(MirEvent const& event); void request_resize(miral::Window const&, MirInputEvent const*, MirResizeEdge); void request_move(miral::Window const&, MirInputEvent const*); + void request_focus(miral::Window const&); auto focused_surface() -> std::shared_ptr; auto tools() -> miral::WindowManagerTools const&; + auto is_above(miral::Window const& a, miral::Window const& b) const -> bool; + virtual auto get_builder() -> WindowManagementPolicyBuilder = 0; virtual auto get_output_rectangles() -> std::vector = 0; diff --git a/tests/mir_test_framework/window_management_test_harness.cpp b/tests/mir_test_framework/window_management_test_harness.cpp index 2360d239f00..5aba30e0315 100644 --- a/tests/mir_test_framework/window_management_test_harness.cpp +++ b/tests/mir_test_framework/window_management_test_harness.cpp @@ -190,6 +190,11 @@ void mir_test_framework::WindowManagementTestHarness::request_move( server.the_shell()->request_move(surface->session().lock(), surface, event); } +void mir_test_framework::WindowManagementTestHarness::request_focus(miral::Window const& window) +{ + self->tools.select_active_window(window); +} + auto mir_test_framework::WindowManagementTestHarness::focused_surface() -> std::shared_ptr { return server.the_shell()->focused_surface(); @@ -199,3 +204,9 @@ auto mir_test_framework::WindowManagementTestHarness::tools() -> miral::WindowMa { return self->tools; } + +auto mir_test_framework::WindowManagementTestHarness::is_above( + miral::Window const& a, miral::Window const& b) const -> bool +{ + return server.the_shell()->is_above(a.operator std::shared_ptr(), b.operator std::shared_ptr()); +} diff --git a/tests/miral/test_window_manager_tools.cpp b/tests/miral/test_window_manager_tools.cpp index 3292ee6de4b..f0b97af39cc 100644 --- a/tests/miral/test_window_manager_tools.cpp +++ b/tests/miral/test_window_manager_tools.cpp @@ -64,6 +64,11 @@ struct StubFocusController : mir::shell::FocusController void swap_z_order(mir::shell::SurfaceSet const& /*first*/, mir::shell::SurfaceSet const& /*second*/) override {} void send_to_back(mir::shell::SurfaceSet const& /*windows*/) override {} + + auto is_above(std::weak_ptr const& /*a*/, std::weak_ptr const& /*b*/) const -> bool override + { + return false; + } }; struct StubDisplayLayout : mir::shell::DisplayLayout diff --git a/tests/unit-tests/scene/test_application_session.cpp b/tests/unit-tests/scene/test_application_session.cpp index 05ca20032b8..abcd96834d6 100644 --- a/tests/unit-tests/scene/test_application_session.cpp +++ b/tests/unit-tests/scene/test_application_session.cpp @@ -93,6 +93,11 @@ struct StubSurfaceStack : public msh::SurfaceStack void send_to_back(ms::SurfaceSet const&) override { } + + auto is_above(std::weak_ptr const& /*a*/, std::weak_ptr const& /*b*/) const -> bool override + { + return false; + } }; struct ApplicationSession : public testing::Test diff --git a/tests/window_management_tests/test_minimal_window_manager.cpp b/tests/window_management_tests/test_minimal_window_manager.cpp index e2e92fefca8..da636a1572d 100644 --- a/tests/window_management_tests/test_minimal_window_manager.cpp +++ b/tests/window_management_tests/test_minimal_window_manager.cpp @@ -500,6 +500,45 @@ TEST_F(MinimalWindowManagerTest, can_resize_south_east_with_touch) EXPECT_EQ(window.size(), geom::Size(50, 50)); } +TEST_F(MinimalWindowManagerTest, new_windows_are_inserted_above_older_windows) +{ + auto const app1 = open_application("app1"); + auto const app2 = open_application("app2"); + auto const app3 = open_application("app3"); + msh::SurfaceSpecification spec; + spec.width = geom::Width {100}; + spec.height = geom::Height{100}; + spec.depth_layer = mir_depth_layer_application; + auto window1 = create_window(app1, spec); + auto window2 = create_window(app2, spec); + auto window3 = create_window(app3, spec); + + EXPECT_TRUE(is_above(window3, window2)); + EXPECT_TRUE(is_above(window2, window1)); +} + +TEST_F(MinimalWindowManagerTest, when_a_window_is_focused_then_it_appears_above_other_windows) +{ + auto const app1 = open_application("app1"); + auto const app2 = open_application("app2"); + auto const app3 = open_application("app3"); + msh::SurfaceSpecification spec; + spec.width = geom::Width {100}; + spec.height = geom::Height{100}; + spec.depth_layer = mir_depth_layer_application; + auto window1 = create_window(app1, spec); + auto window2 = create_window(app2, spec); + auto window3 = create_window(app3, spec); + + request_focus(window1); + + auto surface1 = window1.operator std::shared_ptr(); + EXPECT_EQ(focused_surface(), surface1); + + EXPECT_TRUE(is_above(window1, window3)); + EXPECT_TRUE(is_above(window3, window2)); +} + struct SurfacePlacementCase { struct SurfacePlacement @@ -601,4 +640,4 @@ INSTANTIATE_TEST_SUITE_P(MinimalWindowManagerAttachedTestPlacement, MinimalWindo } } } -)); \ No newline at end of file +));