feature: 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

@@ -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})

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}),

debian/libmirserver63.install

@@ -0,0 +1 @@
+usr/lib/*/libmirserver.so.63

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<scene::Surface> const& a, std::weak_ptr<scene::Surface> const& b) const -> bool override;
 /** @} */
 
     void add_display(geometry::Rectangle const& area) override;

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<scene::Surface> const& a, std::weak_ptr<scene::Surface> const& b) const
+        -> bool = 0;
+
 protected:
     FocusController() = default;
     FocusController(FocusController const&) = delete;

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<scene::Surface> const& a, std::weak_ptr<scene::Surface> const& b) const -> bool override;
+
     auto open_session(
         pid_t client_pid,
         Fd socket_fd,

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<scene::Surface> const& a, std::weak_ptr<scene::Surface> const& b) const
+        -> bool = 0;
+
 protected:
     SurfaceStack() = default;
     virtual ~SurfaceStack() = default;

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<scene::Surface> const& a, std::weak_ptr<scene::Surface> const& b) const -> bool override;
+
 protected:
     std::shared_ptr<SurfaceStack> const wrapped;
 };

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(

src/server/scene/surface_stack.cpp

@@ -126,7 +126,6 @@ struct SurfaceDepthLayerObserver : ms::NullSurfaceObserver
 private:
     ms::SurfaceStack* stack;
 };
-
 }
 
 ms::SurfaceStack::SurfaceStack(std::shared_ptr<SceneReport> 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<scene::Surface> const& a, std::weak_ptr<scene::Surface> 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<Surface> const& surface)
 {
     auto const tracker = std::make_shared<RenderingTracker>(surface);

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<scene::Surface> const& a, std::weak_ptr<scene::Surface> const& b) const
+        -> bool override;
 
     void add_surface(
         std::shared_ptr<Surface> const& surface,

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<scene::Surface> const& a, std::weak_ptr<scene::Surface> const& b) const
+    -> bool
+{
+   return surface_stack->is_above(a, 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);
-}
+}
+
+auto msh::ShellWrapper::is_above(std::weak_ptr<scene::Surface> const& a, std::weak_ptr<scene::Surface> const& b) const
+    -> bool
+{
+    return wrapped->is_above(a, 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<scene::Surface> const& a,
+    std::weak_ptr<scene::Surface> const& b) const -> bool
+{
+    return wrapped->is_above(a, 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;
+

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<scene::Surface>(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<scene::Surface> const& a, std::weak_ptr<scene::Surface> const& 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<scene::Surface> const& /*a*/, std::weak_ptr<scene::Surface> const& /*b*/) const -> bool override
+    {
+        return false;
+    }
     /// @}
 
     /// Overrides from shell::Shell

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<mir::scene::Surface>;
     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<mir::geometry::Rectangle> = 0;
 

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<ms::Surface>
 {
     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<ms::Surface>(), b.operator std::shared_ptr<ms::Surface>());
+}

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<mir::scene::Surface> const& /*a*/, std::weak_ptr<mir::scene::Surface> const& /*b*/) const -> bool override
+    {
+        return false;
+    }
 };
 
 struct StubDisplayLayout : mir::shell::DisplayLayout

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<ms::Surface> const& /*a*/, std::weak_ptr<ms::Surface> const& /*b*/) const -> bool override
+    {
+        return false;
+    }
 };
 
 struct ApplicationSession : public testing::Test

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<ms::Surface>();
+    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
             }
         }
     }
-));
+));