QmlDesigner: Fix composed effect reparenting and removal issues

Reparenting or removing composed effect nodes requires recursive update
of the old parent to ensure all children of the old parent are rendered
correctly without the effect.

If the new parent of composed effect node has Image children, they can
in some situations randomly lose their textures in 2D view.
It seems that the only way to fix this issue is to toggle the
visibility of the affected node or its ancestor off and on, so we do
that to the new parent item when reparenting composed effects.

Fixes: QDS-11688
Change-Id: I003d3976d619f24164938846d9b4a15201bf7b59
Reviewed-by: Mahmoud Badri <mahmoud.badri@qt.io>

src/plugins/effectcomposer/effectcomposermodel.cpp

@@ -846,7 +846,8 @@ R"(
 
     QString parentChanged{
 R"(
-    onParentChanged: {
+    function setupParentLayer()
+    {
         if (_oldParent && _oldParent !== parent) {
             _oldParent.layer.enabled = false
             _oldParent.layer.effect = null
@@ -860,26 +861,21 @@ R"(
             if (visible) {
                 parent.layer.enabled = true
                 parent.layer.effect = effectComponent
+                %6
+                %4%1%5%3
+            } else {
+                parent.layer.enabled = false
+                parent.layer.effect = null
+                %8
+                %4%2
             }
-            %6
+            parent.update()
-            %4%1%5%3
         }
     }
 
-    onVisibleChanged: {
+    onParentChanged: setupParentLayer()
-        if (visible) {
+
-            parent.layer.enabled = true
+    onVisibleChanged: setupParentLayer()
-            parent.layer.effect = effectComponent
-            %6
-            %4%1%5%3
-        } else {
-            parent.layer.enabled = false
-            parent.layer.effect = null
-            %8
-            %4%2
-        }
-        parent.update()
-    }
 
 )"
     };

src/tools/qml2puppet/qml2puppet/instances/objectnodeinstance.cpp

@@ -173,6 +173,11 @@ bool ObjectNodeInstance::isPropertyChange() const
     return false;
 }
 
+bool ObjectNodeInstance::isComposedEffect() const
+{
+    return false;
+}
+
 bool ObjectNodeInstance::equalGraphicsItem(QGraphicsItem * /*item*/) const
 {
     return false;

src/tools/qml2puppet/qml2puppet/instances/objectnodeinstance.h

@@ -83,6 +83,7 @@ public:
     virtual bool isLayoutable() const;
     virtual bool isRenderable() const;
     virtual bool isPropertyChange() const;
+    virtual bool isComposedEffect() const;
 
     virtual bool equalGraphicsItem(QGraphicsItem *item) const;
 

src/tools/qml2puppet/qml2puppet/instances/qt5rendernodeinstanceserver.cpp

@@ -239,7 +239,7 @@ void Qt5RenderNodeInstanceServer::changePropertyValues(const ChangeValuesCommand
                     instance = instanceForObject(targetObject);
             }
 
-            if (instance.hasParent() && instance.propertyNames().contains("_isEffectItem"))
+            if (instance.hasParent() && instance.isComposedEffect())
                 makeDirtyRecursive(instance.parent());
         } else if (container.isDynamic() && hasInstanceForId(container.instanceId())) {
             // Changes to dynamic properties are not always noticed by normal signal spy mechanism
@@ -263,13 +263,68 @@ void Qt5RenderNodeInstanceServer::changePropertyBindings(const ChangeBindingsCom
     }
 }
 
-void Qt5RenderNodeInstanceServer::makeDirtyRecursive(const ServerNodeInstance &instance)
+void Qt5RenderNodeInstanceServer::reparentInstances(const ReparentInstancesCommand &command)
 {
-    const QList<ServerNodeInstance> children = instance.childItems();
+    ServerNodeInstance effectNode;
-    for (const auto &child : children) {
+    ServerNodeInstance oldParent;
-        m_dirtyInstanceSet.insert(child);
+    const QVector<ReparentContainer> containers = command.reparentInstances();
-        makeDirtyRecursive(child);
+    for (const ReparentContainer &container : containers) {
+        if (hasInstanceForId(container.instanceId())) {
+            ServerNodeInstance instance = instanceForId(container.instanceId());
+            if (instance.isComposedEffect()) {
+                oldParent = instance.parent();
+                effectNode = instance;
+                break;
+            }
+        }
+    }
+
+    Qt5NodeInstanceServer::reparentInstances(command);
+
+    if (oldParent.isValid())
+        makeDirtyRecursive(oldParent);
+    if (effectNode.isValid()) {
+        ServerNodeInstance newParent = effectNode.parent();
+        if (newParent.isValid()) {
+            // This is a hack to work around Image elements sometimes losing their textures when
+            // used as children of an effect. Toggling the visibility of the affected node seems
+            // to be the only way to fix this issue.
+            // Note that just marking the children's visibility dirty doesn't fix this issue.
+            QQuickItem *parentItem = newParent.rootQuickItem();
+            if (parentItem && parentItem->isVisible()) {
+                parentItem->setVisible(false);
+                parentItem->setVisible(true);
+            }
+        }
     }
 }
 
+void Qt5RenderNodeInstanceServer::removeInstances(const RemoveInstancesCommand &command)
+{
+    ServerNodeInstance oldParent;
+    const QVector<qint32> ids = command.instanceIds();
+    for (qint32 id : ids) {
+        if (hasInstanceForId(id)) {
+            ServerNodeInstance instance = instanceForId(id);
+            if (instance.isComposedEffect()) {
+                oldParent = instance.parent();
+                break;
+            }
+        }
+    }
+
+    Qt5NodeInstanceServer::removeInstances(command);
+
+    if (oldParent.isValid())
+        makeDirtyRecursive(oldParent);
+}
+
+void Qt5RenderNodeInstanceServer::makeDirtyRecursive(const ServerNodeInstance &instance)
+{
+    m_dirtyInstanceSet.insert(instance);
+    const QList<ServerNodeInstance> children = instance.childItems();
+    for (const auto &child : children)
+        makeDirtyRecursive(child);
+}
+
 } // namespace QmlDesigner

src/tools/qml2puppet/qml2puppet/instances/qt5rendernodeinstanceserver.h

@@ -19,6 +19,8 @@ public:
     void removeSharedMemory(const RemoveSharedMemoryCommand &command) override;
     void changePropertyValues(const ChangeValuesCommand &command) override;
     void changePropertyBindings(const ChangeBindingsCommand &command) override;
+    void reparentInstances(const ReparentInstancesCommand &command) override;
+    void removeInstances(const RemoveInstancesCommand &command) override;
 
 protected:
     void collectItemChangesAndSendChangeCommands() override;

src/tools/qml2puppet/qml2puppet/instances/quickitemnodeinstance.cpp

@@ -179,6 +179,10 @@ void QuickItemNodeInstance::doComponentComplete()
     if (contentItemProperty.isValid())
         m_contentItem = contentItemProperty.read().value<QQuickItem*>();
 
+    QQmlProperty composedEffectProperty(quickItem(), "_isEffectItem", engine());
+    if (composedEffectProperty.isValid())
+        m_isComposedEffect = true;
+
     quickItem()->update();
 }
 
@@ -488,6 +492,11 @@ bool QuickItemNodeInstance::isRenderable() const
     return quickItem() && (!s_unifiedRenderPath || isRootNodeInstance());
 }
 
+bool QuickItemNodeInstance::isComposedEffect() const
+{
+    return m_isComposedEffect;
+}
+
 QList<ServerNodeInstance> QuickItemNodeInstance::stateInstances() const
 {
     QList<ServerNodeInstance> instanceList;

src/tools/qml2puppet/qml2puppet/instances/quickitemnodeinstance.h

@@ -75,6 +75,7 @@ public:
     bool isMovable() const override;
     bool isQuickItem() const override;
     bool isRenderable() const override;
+    bool isComposedEffect() const override;
 
     QList<ServerNodeInstance> stateInstances() const override;
 
@@ -123,6 +124,7 @@ private: //variables
     double m_width;
     double m_height;
     bool m_hidden = false;
+    bool m_isComposedEffect = false;
     static bool s_createEffectItem;
     static bool s_unifiedRenderPath;
 };

src/tools/qml2puppet/qml2puppet/instances/servernodeinstance.cpp

@@ -158,6 +158,11 @@ bool ServerNodeInstance::isComponentWrap() const
     return m_nodeInstance->isComponentWrap();
 }
 
+bool ServerNodeInstance::isComposedEffect() const
+{
+    return m_nodeInstance->isComposedEffect();
+}
+
 QQuickItem *ServerNodeInstance::contentItem() const
 {
     return m_nodeInstance->contentItem();

src/tools/qml2puppet/qml2puppet/instances/servernodeinstance.h

@@ -54,6 +54,7 @@ public:
     friend class Qt5BakeLightsNodeInstanceServer;
     friend class Qt5PreviewNodeInstanceServer;
     friend class Qt5CapturePreviewNodeInstanceServer;
+    friend class Qt5RenderNodeInstanceServer;
     friend class Qt5TestNodeInstanceServer;
     friend class QHash<qint32, ServerNodeInstance>;
     friend QHashValueType qHash(const ServerNodeInstance &instance);
@@ -160,6 +161,7 @@ public:
     bool holdsGraphical() const;
 
     bool isComponentWrap() const;
+    bool isComposedEffect() const;
 
     QQuickItem *contentItem() const;