core/surface/buffer: Buffer lock/release fixes (#7110)

6 files changed, 140 insertions, 67 deletions

diff --git a/src/protocols/core/Compositor.cpp b/src/protocols/core/Compositor.cpp
index 43d3059b..81be8e46 100644
--- a/src/protocols/core/Compositor.cpp
+++ b/src/protocols/core/Compositor.cpp
@@ -1,5 +1,6 @@
 #include "Compositor.hpp"
 #include "Output.hpp"
+#include "Seat.hpp"
 #include "../types/WLBuffer.hpp"
 #include <algorithm>
 #include <ranges>
@@ -9,6 +10,7 @@
 #include "../PresentationTime.hpp"
 #include "../DRMSyncobj.hpp"
 #include "../../render/Renderer.hpp"
+#include <cstring>
 
 #define LOGM PROTO::compositor->protoLog
 
@@ -19,8 +21,6 @@ class CDefaultSurfaceRole : public ISurfaceRole {
     }
 };
 
-SP<CDefaultSurfaceRole> defaultRole = makeShared<CDefaultSurfaceRole>();
-
 CWLCallbackResource::CWLCallbackResource(SP<CWlCallback> resource_) : resource(resource_) {
     ;
 }
@@ -63,7 +63,7 @@ CWLSurfaceResource::CWLSurfaceResource(SP<CWlSurface> resource_) : resource(reso
 
     resource->setData(this);
 
-    role = defaultRole;
+    role = makeShared<CDefaultSurfaceRole>();
 
     resource->setDestroy([this](CWlSurface* r) { destroy(); });
     resource->setOnDestroy([this](CWlSurface* r) { destroy(); });
@@ -75,41 +75,42 @@ CWLSurfaceResource::CWLSurfaceResource(SP<CWlSurface> resource_) : resource(reso
             pending.buffer.reset();
             pending.texture.reset();
         } else {
-            auto res        = CWLBufferResource::fromResource(buffer);
-            pending.buffer  = res && res->buffer ? res->buffer.lock() : nullptr;
-            pending.size    = res && res->buffer ? res->buffer->size : Vector2D{};
-            pending.texture = res && res->buffer ? res->buffer->texture : nullptr;
-            if (res)
-                res->released = false;
+            auto res           = CWLBufferResource::fromResource(buffer);
+            pending.buffer     = res && res->buffer ? makeShared<CHLBufferReference>(res->buffer.lock(), self.lock()) : nullptr;
+            pending.size       = res && res->buffer ? res->buffer->size : Vector2D{};
+            pending.texture    = res && res->buffer ? res->buffer->texture : nullptr;
+            pending.bufferSize = res && res->buffer ? res->buffer->size : Vector2D{};
         }
 
-        Vector2D oldBufSize = current.buffer ? current.buffer->size : Vector2D{};
-        Vector2D newBufSize = pending.buffer ? pending.buffer->size : Vector2D{};
+        Vector2D oldBufSize = current.buffer ? current.bufferSize : Vector2D{};
+        Vector2D newBufSize = pending.buffer ? pending.bufferSize : Vector2D{};
 
         if (oldBufSize != newBufSize || current.buffer != pending.buffer)
             pending.bufferDamage = CBox{{}, {INT32_MAX, INT32_MAX}};
     });
 
     resource->setCommit([this](CWlSurface* r) {
-        if (pending.buffer)
-            pending.bufferDamage.intersect(CBox{{}, pending.buffer->size});
+        if (pending.texture)
+            pending.bufferDamage.intersect(CBox{{}, pending.bufferSize});
 
-        if (!pending.buffer)
+        if (!pending.texture)
             pending.size = {};
         else if (pending.viewport.hasDestination)
             pending.size = pending.viewport.destination;
         else if (pending.viewport.hasSource)
             pending.size = pending.viewport.source.size();
         else {
-            Vector2D tfs = pending.transform % 2 == 1 ? Vector2D{pending.buffer->size.y, pending.buffer->size.x} : pending.buffer->size;
+            Vector2D tfs = pending.transform % 2 == 1 ? Vector2D{pending.bufferSize.y, pending.bufferSize.x} : pending.bufferSize;
             pending.size = tfs / pending.scale;
         }
 
         pending.damage.intersect(CBox{{}, pending.size});
 
         events.precommit.emit();
-        if (pending.rejected)
+        if (pending.rejected) {
+            dropPendingBuffer();
             return;
+        }
 
         if (stateLocks <= 0)
             commitPendingState();
@@ -160,6 +161,14 @@ void CWLSurfaceResource::destroy() {
     PROTO::compositor->destroyResource(this);
 }
 
+void CWLSurfaceResource::dropPendingBuffer() {
+    pending.buffer.reset();
+}
+
+void CWLSurfaceResource::dropCurrentBuffer() {
+    current.buffer.reset();
+}
+
 SP<CWLSurfaceResource> CWLSurfaceResource::fromResource(wl_resource* res) {
     auto data = (CWLSurfaceResource*)(((CWlSurface*)wl_resource_get_user_data(res))->data());
     return data ? data->self.lock() : nullptr;
@@ -240,7 +249,7 @@ void CWLSurfaceResource::frame(timespec* now) {
 }
 
 void CWLSurfaceResource::resetRole() {
-    role = defaultRole;
+    role = makeShared<CDefaultSurfaceRole>();
 }
 
 void CWLSurfaceResource::bfHelper(std::vector<SP<CWLSurfaceResource>> nodes, std::function<void(SP<CWLSurfaceResource>, const Vector2D&, void*)> fn, void* data) {
@@ -254,6 +263,8 @@ void CWLSurfaceResource::bfHelper(std::vector<SP<CWLSurfaceResource>> nodes, std
         for (auto& c : n->subsurfaces) {
             if (c->zIndex >= 0)
                 break;
+            if (c->surface.expired())
+                continue;
             nodes2.push_back(c->surface.lock());
         }
     }
@@ -277,6 +288,8 @@ void CWLSurfaceResource::bfHelper(std::vector<SP<CWLSurfaceResource>> nodes, std
         for (auto& c : n->subsurfaces) {
             if (c->zIndex < 0)
                 continue;
+            if (c->surface.expired())
+                continue;
             nodes2.push_back(c->surface.lock());
         }
     }
@@ -343,14 +356,9 @@ void CWLSurfaceResource::unmap() {
 }
 
 void CWLSurfaceResource::releaseBuffers(bool onlyCurrent) {
-    if (current.buffer && !current.buffer->resource->released)
-        current.buffer->sendRelease();
-    if (pending.buffer && !pending.buffer->resource->released && !onlyCurrent)
-        pending.buffer->sendRelease();
-
-    pending.buffer.reset();
     if (!onlyCurrent)
-        current.buffer.reset();
+        dropPendingBuffer();
+    dropCurrentBuffer();
 }
 
 void CWLSurfaceResource::error(int code, const std::string& str) {
@@ -375,18 +383,18 @@ CBox CWLSurfaceResource::extends() {
 }
 
 Vector2D CWLSurfaceResource::sourceSize() {
-    if (!current.buffer)
+    if (!current.texture)
         return {};
 
     if (current.viewport.hasSource)
         return current.viewport.source.size();
 
-    Vector2D trc = current.transform % 2 == 1 ? Vector2D{current.buffer->size.y, current.buffer->size.x} : current.buffer->size;
+    Vector2D trc = current.transform % 2 == 1 ? Vector2D{current.bufferSize.y, current.bufferSize.x} : current.bufferSize;
     return trc / current.scale;
 }
 
 CRegion CWLSurfaceResource::accumulateCurrentBufferDamage() {
-    if (!current.buffer)
+    if (!current.texture)
         return {};
 
     CRegion surfaceDamage = current.damage;
@@ -398,7 +406,7 @@ CRegion CWLSurfaceResource::accumulateCurrentBufferDamage() {
     if (current.viewport.hasSource)
         surfaceDamage.translate(current.viewport.source.pos());
 
-    Vector2D trc = current.transform % 2 == 1 ? Vector2D{current.buffer->size.y, current.buffer->size.x} : current.buffer->size;
+    Vector2D trc = current.transform % 2 == 1 ? Vector2D{current.bufferSize.y, current.bufferSize.x} : current.bufferSize;
 
     return surfaceDamage.scale(current.scale).transform(wlTransformToHyprutils(invertTransform(current.transform)), trc.x, trc.y).add(current.bufferDamage);
 }
@@ -421,16 +429,21 @@ void CWLSurfaceResource::commitPendingState() {
     pending.damage.clear();
     pending.bufferDamage.clear();
 
-    if (current.buffer && current.buffer->texture)
-        current.buffer->texture->m_eTransform = wlTransformToHyprutils(current.transform);
+    if (current.texture)
+        current.texture->m_eTransform = wlTransformToHyprutils(current.transform);
+
+    if (current.buffer && current.buffer->buffer) {
+        current.buffer->buffer->update(accumulateCurrentBufferDamage());
 
-    if (current.buffer && !current.buffer->resource->released) {
-        current.buffer->update(accumulateCurrentBufferDamage());
+        // if the surface is a cursor, update the shm buffer
+        // TODO: don't update the entire texture
+        if (role->role() == SURFACE_ROLE_CURSOR)
+            updateCursorShm();
 
         // release the buffer if it's synchronous as update() has done everything thats needed
         // so we can let the app know we're done.
-        if (current.buffer->isSynchronous())
-            current.buffer->sendReleaseWithSurface(self.lock());
+        if (current.buffer->buffer->isSynchronous())
+            dropCurrentBuffer();
     }
 
     // TODO: we should _accumulate_ and not replace above if sync
@@ -455,20 +468,37 @@ void CWLSurfaceResource::commitPendingState() {
     }
 
     // for async buffers, we can only release the buffer once we are unrefing it from current.
-    if (previousBuffer && !previousBuffer->isSynchronous() && !previousBuffer->resource->released) {
-        if (previousBuffer->lockedByBackend) {
-            previousBuffer->hlEvents.backendRelease = previousBuffer->events.backendRelease.registerListener([this, previousBuffer](std::any data) {
-                if (!self.expired()) // could be dead in the dtor
-                    previousBuffer->sendReleaseWithSurface(self.lock());
-                else
-                    previousBuffer->sendRelease();
-                previousBuffer->hlEvents.backendRelease.reset();
-            });
-        } else
-            previousBuffer->sendReleaseWithSurface(self.lock());
-
-        previousBuffer->resource->released = true; // set it here regardless so we dont set more listeners for backendRelease
+    // if the backend took it, ref it with the lambda. Otherwise, the end of this scope will release it.
+    if (previousBuffer && previousBuffer->buffer && !previousBuffer->buffer->isSynchronous()) {
+        if (previousBuffer->buffer->lockedByBackend && !previousBuffer->buffer->hlEvents.backendRelease) {
+            previousBuffer->buffer->lock();
+            previousBuffer->buffer->unlockOnBufferRelease(self);
+        }
+    }
+
+    lastBuffer = current.buffer ? current.buffer->buffer : WP<IHLBuffer>{};
+}
+
+void CWLSurfaceResource::updateCursorShm() {
+    auto buf = current.buffer ? current.buffer : lastBuffer;
+
+    if (!buf)
+        return;
+
+    // TODO: actually use damage
+    auto& shmData  = CCursorSurfaceRole::cursorPixelData(self.lock());
+    auto  shmAttrs = current.buffer->buffer->shm();
+
+    if (!shmAttrs.success) {
+        LOGM(TRACE, "updateCursorShm: ignoring, not a shm buffer");
+        return;
     }
+
+    // no need to end, shm.
+    auto [pixelData, fmt, bufLen] = current.buffer->buffer->beginDataPtr(0);
+
+    shmData.resize(bufLen);
+    memcpy(shmData.data(), pixelData, bufLen);
 }
 
 void CWLSurfaceResource::presentFeedback(timespec* when, CMonitor* pMonitor, bool needsExplicitSync) {
diff --git a/src/protocols/core/Compositor.hpp b/src/protocols/core/Compositor.hpp
index 79cd1de6..460ec755 100644
--- a/src/protocols/core/Compositor.hpp
+++ b/src/protocols/core/Compositor.hpp
@@ -84,13 +84,13 @@ class CWLSurfaceResource {
     } events;
 
     struct SState {
-        CRegion             opaque, input = CBox{{}, {INT32_MAX, INT32_MAX}}, damage, bufferDamage = CBox{{}, {INT32_MAX, INT32_MAX}} /* initial damage */;
-        wl_output_transform transform = WL_OUTPUT_TRANSFORM_NORMAL;
-        int                 scale     = 1;
-        SP<IHLBuffer>       buffer;
-        SP<CTexture>        texture;
-        Vector2D            offset;
-        Vector2D            size;
+        CRegion                opaque, input = CBox{{}, {INT32_MAX, INT32_MAX}}, damage, bufferDamage = CBox{{}, {INT32_MAX, INT32_MAX}} /* initial damage */;
+        wl_output_transform    transform = WL_OUTPUT_TRANSFORM_NORMAL;
+        int                    scale     = 1;
+        SP<CHLBufferReference> buffer; // buffer ref will be released once the buffer is no longer locked. For checking if a buffer is attached to this state, check texture.
+        SP<CTexture>           texture;
+        Vector2D               offset;
+        Vector2D               size, bufferSize;
         struct {
             bool     hasDestination = false;
             bool     hasSource      = false;
@@ -116,7 +116,7 @@ class CWLSurfaceResource {
     std::vector<WP<CMonitor>>              enteredOutputs;
     bool                                   mapped = false;
     std::vector<WP<CWLSubsurfaceResource>> subsurfaces;
-    WP<ISurfaceRole>                       role;
+    SP<ISurfaceRole>                       role;
     WP<CViewportResource>                  viewportResource;
     WP<CDRMSyncobjSurfaceResource>         syncobj; // may not be present
 
@@ -134,12 +134,21 @@ class CWLSurfaceResource {
     SP<CWlSurface> resource;
     wl_client*     pClient = nullptr;
 
-    int            stateLocks = 0;
+    // this is for cursor dumb copy. Due to our (and wayland's...) architecture,
+    // this stupid-ass hack is used
+    WP<IHLBuffer> lastBuffer;
 
-    void           destroy();
-    void           releaseBuffers(bool onlyCurrent = true);
-    void           commitPendingState();
-    void           bfHelper(std::vector<SP<CWLSurfaceResource>> nodes, std::function<void(SP<CWLSurfaceResource>, const Vector2D&, void*)> fn, void* data);
+    int           stateLocks = 0;
+
+    void          destroy();
+    void          releaseBuffers(bool onlyCurrent = true);
+    void          dropPendingBuffer();
+    void          dropCurrentBuffer();
+    void          commitPendingState();
+    void          bfHelper(std::vector<SP<CWLSurfaceResource>> nodes, std::function<void(SP<CWLSurfaceResource>, const Vector2D&, void*)> fn, void* data);
+    void          updateCursorShm();
+
+    friend class CWLPointerResource;
 };
 
 class CWLCompositorResource {
diff --git a/src/protocols/core/DataDevice.cpp b/src/protocols/core/DataDevice.cpp
index 9634d569..fe3905d0 100644
--- a/src/protocols/core/DataDevice.cpp
+++ b/src/protocols/core/DataDevice.cpp
@@ -469,12 +469,12 @@ void CWLDataDeviceProtocol::initiateDrag(WP<CWLDataSourceResource> currentSource
     if (dragSurface) {
         dnd.dndSurfaceDestroy = dragSurface->events.destroy.registerListener([this](std::any d) { abortDrag(); });
         dnd.dndSurfaceCommit  = dragSurface->events.commit.registerListener([this](std::any d) {
-            if (dnd.dndSurface->current.buffer && !dnd.dndSurface->mapped) {
+            if (dnd.dndSurface->current.texture && !dnd.dndSurface->mapped) {
                 dnd.dndSurface->map();
                 return;
             }
 
-            if (dnd.dndSurface->current.buffer <= 0 && dnd.dndSurface->mapped) {
+            if (dnd.dndSurface->current.texture <= 0 && dnd.dndSurface->mapped) {
                 dnd.dndSurface->unmap();
                 return;
             }
@@ -660,13 +660,13 @@ void CWLDataDeviceProtocol::abortDrag() {
 }
 
 void CWLDataDeviceProtocol::renderDND(CMonitor* pMonitor, timespec* when) {
-    if (!dnd.dndSurface || !dnd.dndSurface->current.buffer || !dnd.dndSurface->current.buffer->texture)
+    if (!dnd.dndSurface || !dnd.dndSurface->current.texture)
         return;
 
     const auto POS = g_pInputManager->getMouseCoordsInternal();
 
     CBox       box = CBox{POS, dnd.dndSurface->current.size}.translate(-pMonitor->vecPosition + g_pPointerManager->cursorSizeLogical() / 2.F).scale(pMonitor->scale);
-    g_pHyprOpenGL->renderTexture(dnd.dndSurface->current.buffer->texture, &box, 1.F);
+    g_pHyprOpenGL->renderTexture(dnd.dndSurface->current.texture, &box, 1.F);
 
     box = CBox{POS, dnd.dndSurface->current.size}.translate(g_pPointerManager->cursorSizeLogical() / 2.F);
     g_pHyprRenderer->damageBox(&box);
diff --git a/src/protocols/core/Seat.cpp b/src/protocols/core/Seat.cpp
index 7a295372..bb6a9d4d 100644
--- a/src/protocols/core/Seat.cpp
+++ b/src/protocols/core/Seat.cpp
@@ -119,7 +119,19 @@ CWLPointerResource::CWLPointerResource(SP<CWlPointer> resource_, SP<CWLSeatResou
             return;
         }
 
-        g_pSeatManager->onSetCursor(owner.lock(), serial, surf ? CWLSurfaceResource::fromResource(surf) : nullptr, {hotX, hotY});
+        auto surfResource = surf ? CWLSurfaceResource::fromResource(surf) : nullptr;
+
+        if (surfResource && surfResource->role->role() != SURFACE_ROLE_CURSOR && surfResource->role->role() != SURFACE_ROLE_UNASSIGNED) {
+            r->error(-1, "Cursor surface already has a different role");
+            return;
+        }
+
+        if (surfResource) {
+            surfResource->role = makeShared<CCursorSurfaceRole>();
+            surfResource->updateCursorShm();
+        }
+
+        g_pSeatManager->onSetCursor(owner.lock(), serial, surfResource, {hotX, hotY});
     });
 
     if (g_pSeatManager->state.pointerFocus && g_pSeatManager->state.pointerFocus->client() == resource->client())
@@ -546,3 +558,10 @@ SP<CWLSeatResource> CWLSeatProtocol::seatResourceForClient(wl_client* client) {
 
     return nullptr;
 }
+
+std::vector<uint8_t>& CCursorSurfaceRole::cursorPixelData(SP<CWLSurfaceResource> surface) {
+    RASSERT(surface->role->role() == SURFACE_ROLE_CURSOR, "cursorPixelData called on a non-cursor surface");
+
+    auto role = (CCursorSurfaceRole*)surface->role.get();
+    return role->cursorShmPixelData;
+}
diff --git a/src/protocols/core/Seat.hpp b/src/protocols/core/Seat.hpp
index 09b36056..755a9c2f 100644
--- a/src/protocols/core/Seat.hpp
+++ b/src/protocols/core/Seat.hpp
@@ -16,6 +16,7 @@
 #include "wayland.hpp"
 #include "../../helpers/signal/Signal.hpp"
 #include "../../helpers/math/Math.hpp"
+#include "../types/SurfaceRole.hpp"
 
 constexpr const char* HL_SEAT_NAME = "Hyprland";
 
@@ -27,6 +28,20 @@ class CWLKeyboardResource;
 class CWLTouchResource;
 class CWLSeatResource;
 
+class CCursorSurfaceRole : public ISurfaceRole {
+  public:
+    virtual eSurfaceRole role() {
+        return SURFACE_ROLE_CURSOR;
+    }
+
+    // gets the current pixel data from a shm surface
+    // will assert if the surface is not a cursor
+    static std::vector<uint8_t>& cursorPixelData(SP<CWLSurfaceResource> surface);
+
+  private:
+    std::vector<uint8_t> cursorShmPixelData;
+};
+
 class CWLTouchResource {
   public:
     CWLTouchResource(SP<CWlTouch> resource_, SP<CWLSeatResource> owner_);
diff --git a/src/protocols/core/Subcompositor.cpp b/src/protocols/core/Subcompositor.cpp
index c0c1f258..90f89d91 100644
--- a/src/protocols/core/Subcompositor.cpp
+++ b/src/protocols/core/Subcompositor.cpp
@@ -80,13 +80,13 @@ CWLSubsurfaceResource::CWLSubsurfaceResource(SP<CWlSubsurface> resource_, SP<CWL
     });
 
     listeners.commitSurface = surface->events.commit.registerListener([this](std::any d) {
-        if (surface->current.buffer && !surface->mapped) {
+        if (surface->current.texture && !surface->mapped) {
             surface->map();
             surface->events.map.emit();
             return;
         }
 
-        if (!surface->current.buffer && surface->mapped) {
+        if (!surface->current.texture && surface->mapped) {
             surface->events.unmap.emit();
             surface->unmap();
             return;