From 147b862d7fa980de00ad9f4b23bd9fcf536df9ad Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Tue, 20 Jun 2023 10:35:38 +0300 Subject: [PATCH] core: Add GraphicsBufferRef It's a helper to make GraphicsBuffer refcounting less error prone. --- src/backends/drm/drm_buffer.cpp | 13 +---- src/backends/drm/drm_buffer.h | 5 +- src/core/graphicsbuffer.h | 97 +++++++++++++++++++++++++++++++ src/wayland/surface_interface.cpp | 17 +----- src/wayland/surface_interface_p.h | 3 +- 5 files changed, 106 insertions(+), 29 deletions(-) diff --git a/src/backends/drm/drm_buffer.cpp b/src/backends/drm/drm_buffer.cpp index 5ce703801a..e52c4762b3 100644 --- a/src/backends/drm/drm_buffer.cpp +++ b/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; } } diff --git a/src/backends/drm/drm_buffer.h b/src/backends/drm/drm_buffer.h index 94ae0a0f14..182fe3e5b6 100644 --- a/src/backends/drm/drm_buffer.h +++ b/src/backends/drm/drm_buffer.h @@ -9,14 +9,13 @@ */ #pragma once -#include +#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; }; } diff --git a/src/core/graphicsbuffer.h b/src/core/graphicsbuffer.h index 073bcd5663..ce98a523da 100644 --- a/src/core/graphicsbuffer.h +++ b/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) diff --git a/src/wayland/surface_interface.cpp b/src/wayland/surface_interface.cpp index 8f0cedf364..16d819bb0f 100644 --- a/src/wayland/surface_interface.cpp +++ b/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(¤t); + 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 diff --git a/src/wayland/surface_interface_p.h b/src/wayland/surface_interface_p.h index 04ecc66e20..962af13a21 100644 --- a/src/wayland/surface_interface_p.h +++ b/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;