protocols/subcompositor: fix subsurface sorting (#11136)

3 files changed, 41 insertions, 12 deletions

diff --git a/src/protocols/core/Compositor.cpp b/src/protocols/core/Compositor.cpp
index 5efe020c..10b554f6 100644
--- a/src/protocols/core/Compositor.cpp
+++ b/src/protocols/core/Compositor.cpp
@@ -555,6 +555,30 @@ SImageDescription CWLSurfaceResource::getPreferredImageDescription() {
     return monitor ? monitor->m_imageDescription : g_pCompositor->getPreferredImageDescription();
 }
 
+void CWLSurfaceResource::sortSubsurfaces() {
+    std::ranges::sort(m_subsurfaces, [](const auto& a, const auto& b) { return a->m_zIndex < b->m_zIndex; });
+
+    // find the first non-negative index. We will preserve negativity: e.g. -2, -1, 1, 2
+    int firstNonNegative = -1;
+    for (size_t i = 0; i < m_subsurfaces.size(); ++i) {
+        if (m_subsurfaces.at(i)->m_zIndex >= 0) {
+            firstNonNegative = i;
+            break;
+        }
+    }
+
+    if (firstNonNegative == -1)
+        firstNonNegative = m_subsurfaces.size();
+
+    for (size_t i = firstNonNegative; i < m_subsurfaces.size(); ++i) {
+        m_subsurfaces.at(i)->m_zIndex = i - firstNonNegative;
+    }
+
+    for (int i = 0; i < firstNonNegative; ++i) {
+        m_subsurfaces.at(i)->m_zIndex = -firstNonNegative + i;
+    }
+}
+
 void CWLSurfaceResource::updateCursorShm(CRegion damage) {
     if (damage.empty())
         return;
diff --git a/src/protocols/core/Compositor.hpp b/src/protocols/core/Compositor.hpp
index cfbc3ea8..fd91d8d9 100644
--- a/src/protocols/core/Compositor.hpp
+++ b/src/protocols/core/Compositor.hpp
@@ -111,6 +111,7 @@ class CWLSurfaceResource {
     void                                   presentFeedback(const Time::steady_tp& when, PHLMONITOR pMonitor, bool discarded = false);
     void                                   commitState(SSurfaceState& state);
     NColorManagement::SImageDescription    getPreferredImageDescription();
+    void                                   sortSubsurfaces();
 
     // returns a pair: found surface (null if not found) and surface local coords.
     // localCoords param is relative to 0,0 of this surface
diff --git a/src/protocols/core/Subcompositor.cpp b/src/protocols/core/Subcompositor.cpp
index d18d9fc6..a8873390 100644
--- a/src/protocols/core/Subcompositor.cpp
+++ b/src/protocols/core/Subcompositor.cpp
@@ -27,8 +27,14 @@ CWLSubsurfaceResource::CWLSubsurfaceResource(SP<CWlSubsurface> resource_, SP<CWL
 
         auto it = std::ranges::find_if(m_parent->m_subsurfaces, [SURF](const auto& s) { return s->m_surface == SURF; });
 
+        if ((it == m_parent->m_subsurfaces.end() && m_parent != SURF) || SURF == m_surface) {
+            // protocol error, this is not a valid surface
+            r->error(-1, "Invalid surface in placeAbove");
+            return;
+        }
+
         if (it == m_parent->m_subsurfaces.end()) {
-            LOGM(ERR, "Invalid surface reference in placeAbove, likely parent");
+            // parent surface
             m_parent->m_subsurfaces.emplace_back(m_self);
             m_zIndex = 1;
         } else {
@@ -36,11 +42,7 @@ CWLSubsurfaceResource::CWLSubsurfaceResource(SP<CWlSubsurface> resource_, SP<CWL
             m_parent->m_subsurfaces.emplace_back(m_self);
         }
 
-        std::ranges::sort(m_parent->m_subsurfaces, [](const auto& a, const auto& b) { return a->m_zIndex < b->m_zIndex; });
-
-        for (size_t i = 0; i < m_parent->m_subsurfaces.size(); ++i) {
-            m_parent->m_subsurfaces.at(i)->m_zIndex = i;
-        }
+        m_parent->sortSubsurfaces();
     });
 
     m_resource->setPlaceBelow([this](CWlSubsurface* r, wl_resource* surf) {
@@ -55,8 +57,14 @@ CWLSubsurfaceResource::CWLSubsurfaceResource(SP<CWlSubsurface> resource_, SP<CWL
 
         auto it = std::ranges::find_if(m_parent->m_subsurfaces, [SURF](const auto& s) { return s->m_surface == SURF; });
 
+        if ((it == m_parent->m_subsurfaces.end() && m_parent != SURF) || SURF == m_surface) {
+            // protocol error, this is not a valid surface
+            r->error(-1, "Invalid surface in placeBelow");
+            return;
+        }
+
         if (it == m_parent->m_subsurfaces.end()) {
-            LOGM(ERR, "Invalid surface reference in placeBelow, likely parent");
+            // parent
             m_parent->m_subsurfaces.emplace_back(m_self);
             m_zIndex = -1;
         } else {
@@ -64,11 +72,7 @@ CWLSubsurfaceResource::CWLSubsurfaceResource(SP<CWlSubsurface> resource_, SP<CWL
             m_parent->m_subsurfaces.emplace_back(m_self);
         }
 
-        std::ranges::sort(m_parent->m_subsurfaces, [](const auto& a, const auto& b) { return a->m_zIndex < b->m_zIndex; });
-
-        for (size_t i = 0; i < m_parent->m_subsurfaces.size(); ++i) {
-            m_parent->m_subsurfaces.at(i)->m_zIndex = i;
-        }
+        m_parent->sortSubsurfaces();
     });
 
     m_listeners.commitSurface = m_surface->m_events.commit.listen([this] {