Fix Shapes not playing well with QtGraphicalEffects

This patch allow shapes to skip the sync optimization in situations
where the shape is added to a shader effect either directly or
as nested inside another item, and is set to be invisible.

Task-number: QTBUG-63105
Change-Id: I595fbc052032f420982be1267d22a24bcffb7212
Reviewed-by: Robin Burchell <robin.burchell@crimson.no>

4 files changed, 37 insertions, 6 deletions

diff --git a/src/imports/shapes/qquickshape.cpp b/src/imports/shapes/qquickshape.cpp
index c749357cc5..de49baed48 100644
--- a/src/imports/shapes/qquickshape.cpp
+++ b/src/imports/shapes/qquickshape.cpp
@@ -632,6 +632,7 @@ void QQuickShapePath::resetFillGradient()
 
 QQuickShapePrivate::QQuickShapePrivate()
     : spChanged(false),
+      effectRefCount(0),
       rendererType(QQuickShape::UnknownRenderer),
       async(false),
       status(QQuickShape::Null),
@@ -857,10 +858,12 @@ void QQuickShape::updatePolish()
 {
     Q_D(QQuickShape);
 
-    if (!d->spChanged)
+    const int currentEffectRefCount = d->extra.isAllocated() ? d->extra->recursiveEffectRefCount : 0;
+    if (!d->spChanged && currentEffectRefCount <= d->effectRefCount)
         return;
 
     d->spChanged = false;
+    d->effectRefCount = currentEffectRefCount;
 
     if (!d->renderer) {
         d->createRenderer();
@@ -872,7 +875,7 @@ void QQuickShape::updatePolish()
     // endSync() is where expensive calculations may happen (or get kicked off
     // on worker threads), depending on the backend. Therefore do this only
     // when the item is visible.
-    if (isVisible())
+    if (isVisible() || d->effectRefCount > 0)
         d->sync();
 
     update();
diff --git a/src/imports/shapes/qquickshape_p_p.h b/src/imports/shapes/qquickshape_p_p.h
index f43831516a..350c2756bb 100644
--- a/src/imports/shapes/qquickshape_p_p.h
+++ b/src/imports/shapes/qquickshape_p_p.h
@@ -171,6 +171,7 @@ public:
     static void asyncShapeReady(void *data);
 
     bool spChanged;
+    int effectRefCount;
     QQuickShape::RendererType rendererType;
     bool async;
     QQuickShape::Status status;
diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp
index dac37e284b..07bba46c25 100644
--- a/src/quick/items/qquickitem.cpp
+++ b/src/quick/items/qquickitem.cpp
@@ -2943,6 +2943,7 @@ void QQuickItemPrivate::addChild(QQuickItem *child)
     if (childPrivate->subtreeHoverEnabled && !subtreeHoverEnabled)
         setHasHoverInChild(true);
 
+    childPrivate->recursiveRefFromEffectItem(extra.value().recursiveEffectRefCount);
     markSortedChildrenDirty(child);
     dirty(QQuickItemPrivate::ChildrenChanged);
 
@@ -2971,6 +2972,7 @@ void QQuickItemPrivate::removeChild(QQuickItem *child)
     if (childPrivate->subtreeHoverEnabled && subtreeHoverEnabled)
         setHasHoverInChild(false);
 
+    childPrivate->recursiveRefFromEffectItem(-extra.value().recursiveEffectRefCount);
     markSortedChildrenDirty(child);
     dirty(QQuickItemPrivate::ChildrenChanged);
 
@@ -6071,28 +6073,48 @@ void QQuickItemPrivate::removeFromDirtyList()
 void QQuickItemPrivate::refFromEffectItem(bool hide)
 {
     ++extra.value().effectRefCount;
-    if (1 == extra->effectRefCount) {
+    if (extra->effectRefCount == 1) {
         dirty(EffectReference);
-        if (parentItem) QQuickItemPrivate::get(parentItem)->dirty(ChildrenStackingChanged);
+        if (parentItem)
+            QQuickItemPrivate::get(parentItem)->dirty(ChildrenStackingChanged);
     }
     if (hide) {
         if (++extra->hideRefCount == 1)
             dirty(HideReference);
     }
+    recursiveRefFromEffectItem(1);
+}
+
+void QQuickItemPrivate::recursiveRefFromEffectItem(int refs)
+{
+    Q_Q(QQuickItem);
+    if (!refs)
+        return;
+    extra.value().recursiveEffectRefCount += refs;
+    for (int ii = 0; ii < childItems.count(); ++ii) {
+        QQuickItem *child = childItems.at(ii);
+        QQuickItemPrivate::get(child)->recursiveRefFromEffectItem(refs);
+    }
+    // Polish may rely on the effect ref count so trigger one, if item is not visible
+    // (if visible, it will be triggered automatically).
+    if (!effectiveVisible && refs > 0 && extra.value().recursiveEffectRefCount == 1) // it wasn't referenced, now it's referenced
+        q->polish();
 }
 
 void QQuickItemPrivate::derefFromEffectItem(bool unhide)
 {
     Q_ASSERT(extra->effectRefCount);
     --extra->effectRefCount;
-    if (0 == extra->effectRefCount) {
+    if (extra->effectRefCount == 0) {
         dirty(EffectReference);
-        if (parentItem) QQuickItemPrivate::get(parentItem)->dirty(ChildrenStackingChanged);
+        if (parentItem)
+            QQuickItemPrivate::get(parentItem)->dirty(ChildrenStackingChanged);
     }
     if (unhide) {
         if (--extra->hideRefCount == 0)
             dirty(HideReference);
     }
+    recursiveRefFromEffectItem(-1);
 }
 
 void QQuickItemPrivate::setCulled(bool cull)
@@ -8534,6 +8556,7 @@ QQuickItemPrivate::ExtraData::ExtraData()
   layer(0),
 #endif
   effectRefCount(0), hideRefCount(0),
+  recursiveEffectRefCount(0),
   opacityNode(0), clipNode(0), rootNode(0),
   acceptedMouseButtons(0), origin(QQuickItem::Center),
   transparentForPositioner(false)
diff --git a/src/quick/items/qquickitem_p.h b/src/quick/items/qquickitem_p.h
index 285e86048c..adc9d22610 100644
--- a/src/quick/items/qquickitem_p.h
+++ b/src/quick/items/qquickitem_p.h
@@ -360,8 +360,11 @@ public:
 #endif
         QPointF userTransformOriginPoint;
 
+        // these do not include child items
         int effectRefCount;
         int hideRefCount;
+        // updated recursively for child items as well
+        int recursiveEffectRefCount;
 
         QSGOpacityNode *opacityNode;
         QQuickDefaultClipNode *clipNode;
@@ -607,6 +610,7 @@ public:
     // A reference from an effect item means that this item is used by the effect, so
     // it should insert a root node.
     void refFromEffectItem(bool hide);
+    void recursiveRefFromEffectItem(int refs);
     void derefFromEffectItem(bool unhide);
 
     void itemChange(QQuickItem::ItemChange, const QQuickItem::ItemChangeData &);