core: Add GraphicsBufferRef

It's a helper to make GraphicsBuffer refcounting less error prone.

src/backends/drm/drm_buffer.cpp

@@ -29,17 +29,13 @@ namespace KWin
 DrmFramebuffer::DrmFramebuffer(DrmGpu *gpu, uint32_t fbId, GraphicsBuffer *buffer)
     : m_framebufferId(fbId)
     , m_gpu(gpu)
-    , m_buffer(buffer)
+    , m_bufferRef(buffer)
 {
-    m_buffer->ref();
 }
 
 DrmFramebuffer::~DrmFramebuffer()
 {
     drmModeRmFB(m_gpu->fd(), m_framebufferId);
-    if (m_buffer) {
-        m_buffer->unref();
-    }
 }
 
 uint32_t DrmFramebuffer::framebufferId() const
@@ -49,14 +45,11 @@ uint32_t DrmFramebuffer::framebufferId() const
 
 GraphicsBuffer *DrmFramebuffer::buffer() const
 {
-    return m_buffer;
+    return *m_bufferRef;
 }
 
 void DrmFramebuffer::releaseBuffer()
 {
-    if (m_buffer) {
-        m_buffer->unref();
-        m_buffer = nullptr;
-    }
+    m_bufferRef = nullptr;
 }
 }

src/backends/drm/drm_buffer.h

@@ -9,14 +9,13 @@
 */
 #pragma once
 
-#include <stdint.h>
+#include "core/graphicsbuffer.h"
 
 namespace KWin
 {
 
 class DrmGpu;
 class DrmFramebuffer;
-class GraphicsBuffer;
 
 class DrmFramebuffer
 {
@@ -35,7 +34,7 @@ public:
 protected:
     const uint32_t m_framebufferId;
     DrmGpu *const m_gpu;
-    GraphicsBuffer *m_buffer = nullptr;
+    GraphicsBufferRef m_bufferRef;
 };
 
 }

src/core/graphicsbuffer.h

@@ -89,6 +89,103 @@ protected:
     bool m_dropped = false;
 };
 
+/**
+ * The GraphicsBufferRef type holds a reference to a GraphicsBuffer. While the reference
+ * exists, the graphics buffer cannot be destroyed and the client cannnot modify it.
+ */
+class GraphicsBufferRef
+{
+public:
+    GraphicsBufferRef()
+        : m_buffer(nullptr)
+    {
+    }
+
+    GraphicsBufferRef(GraphicsBuffer *buffer)
+        : m_buffer(buffer)
+    {
+        m_buffer->ref();
+    }
+
+    GraphicsBufferRef(const GraphicsBufferRef &other)
+        : m_buffer(other.m_buffer)
+    {
+        if (m_buffer) {
+            m_buffer->unref();
+        }
+    }
+
+    GraphicsBufferRef(GraphicsBufferRef &&other)
+        : m_buffer(std::exchange(other.m_buffer, nullptr))
+    {
+    }
+
+    ~GraphicsBufferRef()
+    {
+        if (m_buffer) {
+            m_buffer->unref();
+        }
+    }
+
+    GraphicsBufferRef &operator=(const GraphicsBufferRef &other)
+    {
+        if (other.m_buffer) {
+            other.m_buffer->ref();
+        }
+        if (m_buffer) {
+            m_buffer->unref();
+        }
+        m_buffer = other.m_buffer;
+        return *this;
+    }
+
+    GraphicsBufferRef &operator=(GraphicsBufferRef &&other)
+    {
+        if (m_buffer) {
+            m_buffer->unref();
+        }
+        m_buffer = std::exchange(other.m_buffer, nullptr);
+        return *this;
+    }
+
+    GraphicsBufferRef &operator=(GraphicsBuffer *buffer)
+    {
+        if (m_buffer != buffer) {
+            if (m_buffer) {
+                m_buffer->unref();
+            }
+            if (buffer) {
+                buffer->ref();
+            }
+            m_buffer = buffer;
+        }
+        return *this;
+    }
+
+    inline GraphicsBuffer *buffer() const
+    {
+        return m_buffer;
+    }
+
+    inline GraphicsBuffer *operator*() const
+    {
+        return m_buffer;
+    }
+
+    inline GraphicsBuffer *operator->() const
+    {
+        return m_buffer;
+    }
+
+    inline operator bool() const
+    {
+        return m_buffer;
+    }
+
+private:
+    GraphicsBuffer *m_buffer;
+};
+
 } // namespace KWin
 
 Q_DECLARE_OPERATORS_FOR_FLAGS(KWin::GraphicsBuffer::MapFlags)

src/wayland/surface_interface.cpp

@@ -60,10 +60,6 @@ SurfaceInterfacePrivate::~SurfaceInterfacePrivate()
     wl_resource_for_each_safe (resource, tmp, &cached.frameCallbacks) {
         wl_resource_destroy(resource);
     }
-
-    if (current.buffer) {
-        current.buffer->unref();
-    }
 }
 
 void SurfaceInterfacePrivate::addChild(SubSurfaceInterface *child)
@@ -569,18 +565,9 @@ void SurfaceInterfacePrivate::applyState(SurfaceState *next)
     const QRegion oldInputRegion = inputRegion;
 
     next->mergeInto(&current);
+    bufferRef = current.buffer;
     scaleOverride = pendingScaleOverride;
 
-    if (bufferRef != current.buffer) {
-        if (bufferRef) {
-            bufferRef->unref();
-        }
-        bufferRef = current.buffer;
-        if (bufferRef) {
-            bufferRef->ref();
-        }
-    }
-
     // TODO: Refactor the state management code because it gets more clumsy.
     if (current.buffer) {
         bufferSize = current.buffer->size();
@@ -805,7 +792,7 @@ KWin::Output::Transform SurfaceInterface::bufferTransform() const
 
 KWin::GraphicsBuffer *SurfaceInterface::buffer() const
 {
-    return d->bufferRef;
+    return d->bufferRef.buffer();
 }
 
 QPoint SurfaceInterface::offset() const

src/wayland/surface_interface_p.h

@@ -6,6 +6,7 @@
 */
 #pragma once
 
+#include "core/graphicsbuffer.h"
 #include "surface_interface.h"
 #include "utils.h"
 // Qt
@@ -124,7 +125,7 @@ public:
 
     QRegion inputRegion;
     QRegion opaqueRegion;
-    KWin::GraphicsBuffer *bufferRef = nullptr;
+    KWin::GraphicsBufferRef bufferRef;
     QRegion bufferDamage;
     bool mapped = false;
     bool hasCacheState = false;