[scene] Build window pixmap trees before starting rendering

In order to generate window quads for sub-surfaces, we need a valid
window pixmap tree. The problem is that the window pixmap tree is
created too late in the rendering process. This change adjusts the
scene so it creates window pixmap trees before buildQuads().

Differential Revision: https://phabricator.kde.org/D29131

plugins/scenes/opengl/scene_opengl.cpp

@@ -1569,10 +1569,6 @@ static bool needsPixmapUpdate(const OpenGLWindowPixmap *pixmap)
 bool OpenGLWindowPixmap::bind()
 {
     if (!m_texture->isNull()) {
-        // always call updateBuffer to get the sub-surface tree updated
-        if (subSurface().isNull() && !toplevel()->damage().isEmpty()) {
-            updateBuffer();
-        }
         if (needsPixmapUpdate(this)) {
             m_texture->updateFromPixmap(this);
             // mipmaps need to be updated
@@ -1587,11 +1583,6 @@ bool OpenGLWindowPixmap::bind()
         }
         return true;
     }
-    // also bind all children, needs to be done before checking isValid
-    // as there might be valid children to render, see https://bugreports.qt.io/browse/QTBUG-52192
-    if (subSurface().isNull()) {
-        updateBuffer();
-    }
     for (auto it = children().constBegin(); it != children().constEnd(); ++it) {
         static_cast<OpenGLWindowPixmap*>(*it)->bind();
     }

plugins/scenes/qpainter/scene_qpainter.cpp

@@ -271,10 +271,7 @@ void SceneQPainter::Window::performPaint(int mask, const QRegion &_region, const
     if (!pixmap || !pixmap->isValid()) {
         return;
     }
-    if (!toplevel->damage().isEmpty()) {
-        pixmap->updateBuffer();
-        toplevel->resetDamage();
-    }
+    toplevel->resetDamage();
 
     QPainter *scenePainter = m_scene->scenePainter();
     QPainter *painter = scenePainter;
@@ -451,10 +448,10 @@ WindowPixmap *QPainterWindowPixmap::createChild(const QPointer<KWaylandServer::S
     return new QPainterWindowPixmap(subSurface, this);
 }
 
-void QPainterWindowPixmap::updateBuffer()
+void QPainterWindowPixmap::update()
 {
     const auto oldBuffer = buffer();
-    WindowPixmap::updateBuffer();
+    WindowPixmap::update();
     const auto &b = buffer();
     if (!surface()) {
         // That's an internal client.

plugins/scenes/qpainter/scene_qpainter.h

@@ -91,9 +91,9 @@ public:
     explicit QPainterWindowPixmap(Scene::Window *window);
     ~QPainterWindowPixmap() override;
     void create() override;
+    void update() override;
     bool isValid() const override;
 
-    void updateBuffer() override;
     const QImage &image();
 
 protected:

scene.cpp

@@ -211,6 +211,9 @@ void Scene::paintGenericScreen(int orig_mask, const ScreenPaintData &)
     foreach (Window * w, stacking_order) { // bottom to top
         Toplevel* topw = w->window();
 
+        // Let the scene window update the window pixmap tree.
+        w->preprocess();
+
         // Reset the repaint_region.
         // This has to be done here because many effects schedule a repaint for
         // the next frame within Effects::prePaintWindow.
@@ -266,6 +269,9 @@ void Scene::paintSimpleScreen(int orig_mask, const QRegion &region)
         data.paint = region;
         data.paint |= toplevel->repaints();
 
+        // Let the scene window update the window pixmap tree.
+        window->preprocess();
+
         // Reset the repaint_region.
         // This has to be done here because many effects schedule a repaint for
         // the next frame within Effects::prePaintWindow.
@@ -732,7 +738,9 @@ void Scene::Window::updatePixmap()
     if (m_currentPixmap.isNull()) {
         m_currentPixmap.reset(createWindowPixmap());
     }
-    if (!m_currentPixmap->isValid()) {
+    if (m_currentPixmap->isValid()) {
+        m_currentPixmap->update();
+    } else {
         m_currentPixmap->create();
     }
 }
@@ -1018,6 +1026,16 @@ void Scene::Window::updateShadow(Shadow* shadow)
     m_shadow = shadow;
 }
 
+void Scene::Window::preprocess()
+{
+    // The tracked damage will be reset after the scene is done with copying buffer's data.
+    // Note that we have to be prepared for the case where no damage has occurred since kwin
+    // core may discard the current window pixmap at any moment.
+    if (!m_currentPixmap || !window()->damage().isEmpty()) {
+        updatePixmap();
+    }
+}
+
 //****************************************
 // WindowPixmap
 //****************************************
@@ -1059,7 +1077,7 @@ void WindowPixmap::create()
     // always update from Buffer on Wayland, don't try using XPixmap
     if (kwinApp()->shouldUseWaylandForCompositing()) {
         // use Buffer
-        updateBuffer();
+        update();
         if ((m_buffer || !m_fbo.isNull()) && m_subSurface.isNull()) {
             m_window->unreferencePreviousPixmap();
         }
@@ -1094,21 +1112,7 @@ void WindowPixmap::create()
     m_window->unreferencePreviousPixmap();
 }
 
-WindowPixmap *WindowPixmap::createChild(const QPointer<KWaylandServer::SubSurfaceInterface> &subSurface)
-{
-    Q_UNUSED(subSurface)
-    return nullptr;
-}
-
-bool WindowPixmap::isValid() const
-{
-    if (!m_buffer.isNull() || !m_fbo.isNull() || !m_internalImage.isNull()) {
-        return true;
-    }
-    return m_pixmap != XCB_PIXMAP_NONE;
-}
-
-void WindowPixmap::updateBuffer()
+void WindowPixmap::update()
 {
     using namespace KWaylandServer;
     if (SurfaceInterface *s = surface()) {
@@ -1123,7 +1127,7 @@ void WindowPixmap::updateBuffer()
             auto it = std::find_if(oldTree.begin(), oldTree.end(), [subSurface] (WindowPixmap *p) { return p->m_subSurface == subSurface; });
             if (it != oldTree.end()) {
                 children << *it;
-                (*it)->updateBuffer();
+                (*it)->update();
                 oldTree.erase(it);
             } else {
                 WindowPixmap *p = createChild(subSurface);
@@ -1167,6 +1171,20 @@ void WindowPixmap::updateBuffer()
     }
 }
 
+WindowPixmap *WindowPixmap::createChild(const QPointer<KWaylandServer::SubSurfaceInterface> &subSurface)
+{
+    Q_UNUSED(subSurface)
+    return nullptr;
+}
+
+bool WindowPixmap::isValid() const
+{
+    if (!m_buffer.isNull() || !m_fbo.isNull() || !m_internalImage.isNull()) {
+        return true;
+    }
+    return m_pixmap != XCB_PIXMAP_NONE;
+}
+
 KWaylandServer::SurfaceInterface *WindowPixmap::surface() const
 {
     if (!m_subSurface.isNull()) {

scene.h

@@ -344,6 +344,7 @@ public:
     void referencePreviousPixmap();
     void unreferencePreviousPixmap();
     void invalidateQuadsCache();
+    void preprocess();
 protected:
     WindowQuadList makeDecorationQuads(const QRect *rects, const QRegion &region, qreal textureScale = 1.0) const;
     WindowQuadList makeContentsQuads() const;
@@ -362,8 +363,8 @@ protected:
      *
      * @return The WindowPixmap casted to T* or @c NULL if there is no valid window pixmap.
      */
-    template<typename T> T *windowPixmap();
-    template<typename T> T *previousWindowPixmap();
+    template<typename T> T *windowPixmap() const;
+    template<typename T> T *previousWindowPixmap() const;
     /**
      * @brief Factory method to create a WindowPixmap.
      *
@@ -415,6 +416,10 @@ public:
      * native pixmap to the rendering format.
      */
     virtual void create();
+    /**
+     * @brief Recursively updates the mapping between the WindowPixmap and the buffer.
+     */
+    virtual void update();
     /**
      * @return @c true if the pixmap has been created and is valid, @c false otherwise
      */
@@ -495,12 +500,6 @@ protected:
      */
     Scene::Window *window();
 
-    /**
-     * Should be called by the implementing subclasses when the Wayland Buffer changed and needs
-     * updating.
-     */
-    virtual void updateBuffer();
-
     /**
      * Sets the sub-surface tree to @p children.
      */
@@ -631,15 +630,8 @@ QImage WindowPixmap::internalImage() const
 
 template <typename T>
 inline
-T* Scene::Window::windowPixmap()
+T *Scene::Window::windowPixmap() const
 {
-    if (m_currentPixmap.isNull()) {
-        m_currentPixmap.reset(createWindowPixmap());
-    }
-    if (m_currentPixmap->isValid()) {
-        return static_cast<T*>(m_currentPixmap.data());
-    }
-    m_currentPixmap->create();
     if (m_currentPixmap->isValid()) {
         return static_cast<T*>(m_currentPixmap.data());
     } else {
@@ -649,7 +641,7 @@ T* Scene::Window::windowPixmap()
 
 template <typename T>
 inline
-T* Scene::Window::previousWindowPixmap()
+T *Scene::Window::previousWindowPixmap() const
 {
     return static_cast<T*>(m_previousPixmap.data());
 }