From 0f7369ed1b5fc9d844c24c5e4bfe0ea7f42437c3 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Thu, 19 Oct 2023 16:16:56 +0300 Subject: [PATCH] Fix scheduling repaints in Effect::prePaintScreen() If a repaint is scheduled in the prePaintScreen() function, we want it to be applied in the next frame, not the current one. Currently, it doesn't work like this because prePaintScreen() runs first then the Compositor gathers repaints and resets them. This is important to qtquick effects that use qtquick3d as some items in qtquick3d schedule repaints for the next frame after synchronizing, i.e. in OffscreenQuickView::update() which is called in prePaintScreen() by QuickSceneEffect. --- src/compositor.cpp | 39 ++++++++++++++++---------------- src/compositor.h | 3 +-- src/core/renderlayerdelegate.cpp | 8 ++----- src/core/renderlayerdelegate.h | 7 +----- src/scene/cursorscene.cpp | 3 ++- src/scene/cursorscene.h | 2 +- src/scene/scene.cpp | 9 ++------ src/scene/scene.h | 5 ++-- src/scene/workspacescene.cpp | 9 +++----- src/scene/workspacescene.h | 3 +-- 10 files changed, 34 insertions(+), 54 deletions(-) diff --git a/src/compositor.cpp b/src/compositor.cpp index 09e711c39b..efed37e760 100644 --- a/src/compositor.cpp +++ b/src/compositor.cpp @@ -157,7 +157,10 @@ void Compositor::composite(RenderLoop *renderLoop) if (superLayer->needsRepaint()) { renderLoop->beginPaint(); - prePaintPass(superLayer); + + QRegion surfaceDamage = primaryLayer->repaints(); + primaryLayer->resetRepaints(); + prePaintPass(superLayer, &surfaceDamage); SurfaceItem *scanoutCandidate = superLayer->delegate()->scanoutCandidate(); renderLoop->setFullscreenSurface(scanoutCandidate); @@ -175,10 +178,6 @@ void Compositor::composite(RenderLoop *renderLoop) } if (!directScanout) { - QRegion surfaceDamage = primaryLayer->repaints(); - primaryLayer->resetRepaints(); - preparePaintPass(superLayer, &surfaceDamage); - if (auto beginInfo = primaryLayer->beginFrame()) { auto &[renderTarget, repaint] = beginInfo.value(); @@ -219,33 +218,33 @@ void Compositor::framePass(RenderLayer *layer) } } -void Compositor::prePaintPass(RenderLayer *layer) +void Compositor::prePaintPass(RenderLayer *layer, QRegion *damage) { - layer->delegate()->prePaint(); - const auto sublayers = layer->sublayers(); - for (RenderLayer *sublayer : sublayers) { - prePaintPass(sublayer); + if (const QRegion repaints = layer->repaints(); !repaints.isEmpty()) { + *damage += layer->mapToGlobal(repaints); + layer->resetRepaints(); + } + + const QRegion repaints = layer->delegate()->prePaint(); + if (!repaints.isEmpty()) { + *damage += layer->mapToGlobal(repaints); } -} -void Compositor::postPaintPass(RenderLayer *layer) -{ - layer->delegate()->postPaint(); const auto sublayers = layer->sublayers(); for (RenderLayer *sublayer : sublayers) { - postPaintPass(sublayer); + if (sublayer->isVisible()) { + prePaintPass(sublayer, damage); + } } } -void Compositor::preparePaintPass(RenderLayer *layer, QRegion *repaint) +void Compositor::postPaintPass(RenderLayer *layer) { - // TODO: Cull opaque region. - *repaint += layer->mapToGlobal(layer->repaints() + layer->delegate()->repaints()); - layer->resetRepaints(); + layer->delegate()->postPaint(); const auto sublayers = layer->sublayers(); for (RenderLayer *sublayer : sublayers) { if (sublayer->isVisible()) { - preparePaintPass(sublayer, repaint); + postPaintPass(sublayer); } } } diff --git a/src/compositor.h b/src/compositor.h index 354d4ac214..223ee8720c 100644 --- a/src/compositor.h +++ b/src/compositor.h @@ -146,9 +146,8 @@ protected: void addSuperLayer(RenderLayer *layer); void removeSuperLayer(RenderLayer *layer); - void prePaintPass(RenderLayer *layer); + void prePaintPass(RenderLayer *layer, QRegion *damage); void postPaintPass(RenderLayer *layer); - void preparePaintPass(RenderLayer *layer, QRegion *repaint); void paintPass(RenderLayer *layer, const RenderTarget &renderTarget, const QRegion ®ion); void framePass(RenderLayer *layer); diff --git a/src/core/renderlayerdelegate.cpp b/src/core/renderlayerdelegate.cpp index 79258154e1..3533988a5a 100644 --- a/src/core/renderlayerdelegate.cpp +++ b/src/core/renderlayerdelegate.cpp @@ -19,17 +19,13 @@ void RenderLayerDelegate::setLayer(RenderLayer *layer) m_layer = layer; } -QRegion RenderLayerDelegate::repaints() const -{ - return QRegion(); -} - void RenderLayerDelegate::frame() { } -void RenderLayerDelegate::prePaint() +QRegion RenderLayerDelegate::prePaint() { + return QRegion(); } void RenderLayerDelegate::postPaint() diff --git a/src/core/renderlayerdelegate.h b/src/core/renderlayerdelegate.h index 16e552aba1..740c4dd68f 100644 --- a/src/core/renderlayerdelegate.h +++ b/src/core/renderlayerdelegate.h @@ -29,11 +29,6 @@ public: RenderLayer *layer() const; void setLayer(RenderLayer *layer); - /** - * Returns the repaints schduled for the next frame. - */ - virtual QRegion repaints() const; - /** * This function is called by the compositor after compositing the frame. */ @@ -43,7 +38,7 @@ public: * This function is called by the compositor before starting painting. Reimplement * this function to do frame initialization. */ - virtual void prePaint(); + virtual QRegion prePaint(); /** * This function is called by the compositor after finishing painting. Reimplement diff --git a/src/scene/cursorscene.cpp b/src/scene/cursorscene.cpp index f9dcb02ec6..d22898ba09 100644 --- a/src/scene/cursorscene.cpp +++ b/src/scene/cursorscene.cpp @@ -41,10 +41,11 @@ static void resetRepaintsHelper(Item *item, SceneDelegate *delegate) } } -void CursorScene::prePaint(SceneDelegate *delegate) +QRegion CursorScene::prePaint(SceneDelegate *delegate) { resetRepaintsHelper(m_rootItem.get(), delegate); m_paintedOutput = delegate->output(); + return QRegion(); } void CursorScene::postPaint() diff --git a/src/scene/cursorscene.h b/src/scene/cursorscene.h index b0970dc91d..c545c8c1cb 100644 --- a/src/scene/cursorscene.h +++ b/src/scene/cursorscene.h @@ -22,7 +22,7 @@ public: explicit CursorScene(std::unique_ptr &&renderer); ~CursorScene() override; - void prePaint(SceneDelegate *delegate) override; + QRegion prePaint(SceneDelegate *delegate) override; void postPaint() override; void paint(const RenderTarget &renderTarget, const QRegion ®ion) override; diff --git a/src/scene/scene.cpp b/src/scene/scene.cpp index ea731355cb..1494699ec6 100644 --- a/src/scene/scene.cpp +++ b/src/scene/scene.cpp @@ -24,19 +24,14 @@ SceneDelegate::~SceneDelegate() m_scene->removeDelegate(this); } -QRegion SceneDelegate::repaints() const -{ - return m_scene->damage().translated(-viewport().topLeft()); -} - SurfaceItem *SceneDelegate::scanoutCandidate() const { return m_scene->scanoutCandidate(); } -void SceneDelegate::prePaint() +QRegion SceneDelegate::prePaint() { - m_scene->prePaint(this); + return m_scene->prePaint(this); } void SceneDelegate::postPaint() diff --git a/src/scene/scene.h b/src/scene/scene.h index 83f356d726..1a6c749681 100644 --- a/src/scene/scene.h +++ b/src/scene/scene.h @@ -29,10 +29,9 @@ public: qreal scale() const; QRect viewport() const; - QRegion repaints() const override; SurfaceItem *scanoutCandidate() const override; void frame() override; - void prePaint() override; + QRegion prePaint() override; void postPaint() override; void paint(const RenderTarget &renderTarget, const QRegion ®ion) override; @@ -83,7 +82,7 @@ public: void removeDelegate(SceneDelegate *delegate); virtual SurfaceItem *scanoutCandidate() const; - virtual void prePaint(SceneDelegate *delegate) = 0; + virtual QRegion prePaint(SceneDelegate *delegate) = 0; virtual void postPaint() = 0; virtual void paint(const RenderTarget &renderTarget, const QRegion ®ion) = 0; virtual void frame(SceneDelegate *delegate); diff --git a/src/scene/workspacescene.cpp b/src/scene/workspacescene.cpp index 115efa1e17..e8929b2ffe 100644 --- a/src/scene/workspacescene.cpp +++ b/src/scene/workspacescene.cpp @@ -139,11 +139,6 @@ Item *WorkspaceScene::containerItem() const return m_containerItem.get(); } -QRegion WorkspaceScene::damage() const -{ - return m_paintContext.damage; -} - static SurfaceItem *findTopMostSurface(SurfaceItem *item) { const QList children = item->childItems(); @@ -215,7 +210,7 @@ void WorkspaceScene::frame(SceneDelegate *delegate) } } -void WorkspaceScene::prePaint(SceneDelegate *delegate) +QRegion WorkspaceScene::prePaint(SceneDelegate *delegate) { createStackingOrder(); @@ -260,6 +255,8 @@ void WorkspaceScene::prePaint(SceneDelegate *delegate) } else { preparePaintSimpleScreen(); } + + return m_paintContext.damage.translated(-delegate->viewport().topLeft()); } static void resetRepaintsHelper(Item *item, SceneDelegate *delegate) diff --git a/src/scene/workspacescene.h b/src/scene/workspacescene.h index acd2a3c167..efe1efb925 100644 --- a/src/scene/workspacescene.h +++ b/src/scene/workspacescene.h @@ -55,9 +55,8 @@ public: Item *containerItem() const; - QRegion damage() const override; SurfaceItem *scanoutCandidate() const override; - void prePaint(SceneDelegate *delegate) override; + QRegion prePaint(SceneDelegate *delegate) override; void postPaint() override; void paint(const RenderTarget &renderTarget, const QRegion ®ion) override; void frame(SceneDelegate *delegate) override;