Centralize WindowPixmap buffer updating code

Uses a setter and clear method pattern rather than having the code
repeated.
Instead of keeping a QPointer, now we are a QObject and we get notified
about destruction intention directly, so we can clear the pointer when
necessary.

platformsupport/scenes/opengl/abstract_egl_backend.cpp

@@ -345,8 +345,8 @@ bool AbstractEglTexture::loadTexture(WindowPixmap *pixmap)
 {
     // FIXME: Refactor this method.
 
-    const auto &buffer = pixmap->buffer();
+    const auto buffer = pixmap->buffer();
-    if (buffer.isNull()) {
+    if (!buffer) {
         if (updateFromFBO(pixmap->fbo())) {
             return true;
         }
@@ -371,8 +371,8 @@ void AbstractEglTexture::updateTexture(WindowPixmap *pixmap)
 {
     // FIXME: Refactor this method.
 
-    const auto &buffer = pixmap->buffer();
+    const auto buffer = pixmap->buffer();
-    if (buffer.isNull()) {
+    if (!buffer) {
         if (updateFromFBO(pixmap->fbo())) {
             return;
         }

plugins/platforms/drm/egl_stream_backend.cpp

@@ -627,7 +627,7 @@ bool EglStreamTexture::loadTexture(WindowPixmap *pixmap)
     using namespace KWaylandServer;
     SurfaceInterface *surface = pixmap->surface();
     const EglStreamBackend::StreamTexture *st = m_backend->lookupStreamTexture(surface);
-    if (!pixmap->buffer().isNull() && st != nullptr) {
+    if (pixmap->buffer() && st != nullptr) {
 
         glGenTextures(1, &m_texture);
         texture()->setWrapMode(GL_CLAMP_TO_EDGE);
@@ -655,7 +655,7 @@ void EglStreamTexture::updateTexture(WindowPixmap *pixmap)
     using namespace KWaylandServer;
     SurfaceInterface *surface = pixmap->surface();
     const EglStreamBackend::StreamTexture *st = m_backend->lookupStreamTexture(surface);
-    if (!pixmap->buffer().isNull() && st != nullptr) {
+    if (pixmap->buffer() && st != nullptr) {
 
         if (attachBuffer(surface->buffer())) {
             createFbo();

plugins/scenes/qpainter/scene_qpainter.cpp

@@ -417,7 +417,7 @@ void QPainterWindowPixmap::update()
         m_image = internalImage();
         return;
     }
-    if (b.isNull()) {
+    if (!b) {
         m_image = QImage();
         return;
     }

scene.cpp

@@ -1118,11 +1118,7 @@ WindowPixmap::~WindowPixmap()
     if (m_pixmap != XCB_WINDOW_NONE) {
         xcb_free_pixmap(connection(), m_pixmap);
     }
-    if (m_buffer) {
+    clear();
-        using namespace KWaylandServer;
-        QObject::disconnect(m_buffer.data(), &BufferInterface::aboutToBeDestroyed, m_buffer.data(), &BufferInterface::unref);
-        m_buffer->unref();
-    }
 }
 
 void WindowPixmap::create()
@@ -1170,13 +1166,33 @@ void WindowPixmap::create()
     m_window->discardQuads();
 }
 
+void WindowPixmap::clear()
+{
+    setBuffer(nullptr);
+}
+
+void WindowPixmap::setBuffer(KWaylandServer::BufferInterface *buffer)
+{
+    if (buffer == m_buffer) {
+        return;
+    }
+    if (m_buffer) {
+        disconnect(m_buffer, &KWaylandServer::BufferInterface::aboutToBeDestroyed, this, &WindowPixmap::clear);
+        m_buffer->unref();
+    }
+    m_buffer = buffer;
+    if (m_buffer) {
+        m_buffer->ref();
+        connect(m_buffer, &KWaylandServer::BufferInterface::aboutToBeDestroyed, this, &WindowPixmap::clear);
+    }
+}
+
 void WindowPixmap::update()
 {
     using namespace KWaylandServer;
     if (SurfaceInterface *s = surface()) {
         QVector<WindowPixmap*> oldTree = m_children;
         QVector<WindowPixmap*> children;
-        using namespace KWaylandServer;
         const auto subSurfaces = s->childSubSurfaces();
         for (const auto &subSurface : subSurfaces) {
             if (subSurface.isNull()) {
@@ -1198,34 +1214,16 @@ void WindowPixmap::update()
         setChildren(children);
         qDeleteAll(oldTree);
         if (auto b = s->buffer()) {
-            if (b == m_buffer) {
+            setBuffer(b);
-                // no change
-                return;
-            }
-            if (m_buffer) {
-                QObject::disconnect(m_buffer.data(), &BufferInterface::aboutToBeDestroyed, m_buffer.data(), &BufferInterface::unref);
-                m_buffer->unref();
-            }
-            m_buffer = b;
-            m_buffer->ref();
-            QObject::connect(m_buffer.data(), &BufferInterface::aboutToBeDestroyed, m_buffer.data(), &BufferInterface::unref);
         } else if (m_subSurface) {
-            if (m_buffer) {
+            clear();
-                QObject::disconnect(m_buffer.data(), &BufferInterface::aboutToBeDestroyed, m_buffer.data(), &BufferInterface::unref);
-                m_buffer->unref();
-                m_buffer.clear();
-            }
         }
     } else if (toplevel()->internalFramebufferObject()) {
         m_fbo = toplevel()->internalFramebufferObject();
     } else if (!toplevel()->internalImageObject().isNull()) {
         m_internalImage = toplevel()->internalImageObject();
     } else {
-        if (m_buffer) {
+        clear();
-            QObject::disconnect(m_buffer.data(), &BufferInterface::aboutToBeDestroyed, m_buffer.data(), &BufferInterface::unref);
-            m_buffer->unref();
-            m_buffer.clear();
-        }
     }
 }
 
@@ -1237,7 +1235,7 @@ WindowPixmap *WindowPixmap::createChild(const QPointer<KWaylandServer::SubSurfac
 
 bool WindowPixmap::isValid() const
 {
-    if (!m_buffer.isNull() || !m_fbo.isNull() || !m_internalImage.isNull()) {
+    if (m_buffer || !m_fbo.isNull() || !m_internalImage.isNull()) {
         return true;
     }
     return m_pixmap != XCB_PIXMAP_NONE;

scene.h

@@ -415,8 +415,9 @@ private:
  * This class is intended to be inherited for the needs of the compositor backends which need further mapping from
  * the native pixmap to the respective rendering format.
  */
-class KWIN_EXPORT WindowPixmap
+class KWIN_EXPORT WindowPixmap : public QObject
 {
+    Q_OBJECT
 public:
     virtual ~WindowPixmap();
     /**
@@ -448,7 +449,7 @@ public:
     /**
      * @return The Wayland BufferInterface for this WindowPixmap.
      */
-    QPointer<KWaylandServer::BufferInterface> buffer() const;
+    KWaylandServer::BufferInterface *buffer() const;
     const QSharedPointer<QOpenGLFramebufferObject> &fbo() const;
     QImage internalImage() const;
     /**
@@ -575,12 +576,15 @@ protected:
     }
 
 private:
+    void setBuffer(KWaylandServer::BufferInterface *buffer);
+    void clear();
+
     Scene::Window *m_window;
     xcb_pixmap_t m_pixmap;
     QSize m_pixmapSize;
     bool m_discarded;
     QRect m_contentsRect;
-    QPointer<KWaylandServer::BufferInterface> m_buffer;
+    KWaylandServer::BufferInterface *m_buffer = nullptr;
     QSharedPointer<QOpenGLFramebufferObject> m_fbo;
     QImage m_internalImage;
     WindowPixmap *m_parent = nullptr;
@@ -678,7 +682,7 @@ Shadow* Scene::Window::shadow()
 }
 
 inline
-QPointer<KWaylandServer::BufferInterface> WindowPixmap::buffer() const
+KWaylandServer::BufferInterface *WindowPixmap::buffer() const
 {
     return m_buffer;
 }