EffectComposer: Remove invalid properties from effects in use

When effect is saved, the properties it exposed can change if nodes
are deleted from the effect. If any of those removed exposed properties
was assigned a value in the scene, those property assignments are
removed from the scene.

Fixes: QDS-12359
Change-Id: Ia3840584c6361b9e140c6840ff8fa3036c1b7d93
Reviewed-by: Mahmoud Badri <mahmoud.badri@qt.io>
Reviewed-by: Qt CI Patch Build Bot <ci_patchbuild_bot@qt.io>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>

src/plugins/effectcomposer/effectcomposermodel.cpp

@@ -1147,7 +1147,27 @@ void EffectComposerModel::saveResources(const QString &name)
 
     const QString qmlString = qmlStringList.join('\n');
     QString qmlFilePath = effectsResPath + qmlFilename;
-    writeToFile(qmlString.toUtf8(), qmlFilePath, FileType::Text);
+
+    // Get exposed properties from the old qml file if it exists
+    QSet<QByteArray> oldExposedProps;
+    Utils::FilePath oldQmlFile = Utils::FilePath::fromString(qmlFilePath);
+    if (oldQmlFile.exists()) {
+        const QByteArray oldQmlContent = oldQmlFile.fileContents().value();
+        oldExposedProps = getExposedProperties(oldQmlContent);
+    }
+
+    const QByteArray qmlUtf8 = qmlString.toUtf8();
+    if (!oldExposedProps.isEmpty()) {
+        const QSet<QByteArray> newExposedProps = getExposedProperties(qmlUtf8);
+        oldExposedProps.subtract(newExposedProps);
+        if (!oldExposedProps.isEmpty()) {
+            // If there were exposed properties that are no longer exposed, those
+            // need to be removed from any instances of the effect in the scene
+            emit removePropertiesFromScene(oldExposedProps, name);
+        }
+    }
+
+    writeToFile(qmlUtf8, qmlFilePath, FileType::Text);
     newFileNames.append(qmlFilename);
 
     // Save shaders and images
@@ -1998,6 +2018,25 @@ void EffectComposerModel::updateExtraMargin()
         m_extraMargin = qMax(node->extraMargin(), m_extraMargin);
 }
 
+QSet<QByteArray> EffectComposerModel::getExposedProperties(const QByteArray &qmlContent)
+{
+    QSet<QByteArray> returnSet;
+    const QByteArrayList lines = qmlContent.split('\n');
+    const QByteArray propertyTag {"    property"}; // Match only toplevel exposed properties
+    for (const QByteArray &line : lines) {
+        if (line.startsWith(propertyTag)) {
+            QByteArrayList words = line.trimmed().split(' ');
+            if (words.size() >= 3) {
+                QByteArray propName = words[2];
+                if (propName.endsWith(':'))
+                    propName.chop(1);
+                returnSet.insert(propName);
+            }
+        }
+    }
+    return returnSet;
+}
+
 QString EffectComposerModel::currentComposition() const
 {
     return m_currentComposition;

src/plugins/effectcomposer/effectcomposermodel.h

@@ -11,6 +11,7 @@
 #include <QFileSystemWatcher>
 #include <QMap>
 #include <QRegularExpression>
+#include <QSet>
 #include <QTemporaryFile>
 #include <QTimer>
 
@@ -124,6 +125,7 @@ signals:
     void resourcesSaved(const QByteArray &type, const Utils::FilePath &path);
     void hasUnsavedChangesChanged();
     void assignToSelectedTriggered(const QString &effectPath);
+    void removePropertiesFromScene(QSet<QByteArray> props, const QString &typeName);
 
 private:
     enum Roles {
@@ -185,6 +187,7 @@ private:
 
     void connectCompositionNode(CompositionNode *node);
     void updateExtraMargin();
+    QSet<QByteArray> getExposedProperties(const QByteArray &qmlContent);
 
     QList<CompositionNode *> m_nodes;
 

src/plugins/effectcomposer/effectcomposerview.cpp

@@ -10,7 +10,9 @@
 #include <designermcumanager.h>
 #include <documentmanager.h>
 #include <modelnodeoperations.h>
+#include <qmlchangeset.h>
 #include <qmldesignerconstants.h>
+#include <qmldesignerplugin.h>
 
 #include <coreplugin/icore.h>
 
@@ -49,14 +51,73 @@ QmlDesigner::WidgetInfo EffectComposerView::widgetInfo()
         m_widget = new EffectComposerWidget{this};
 
         connect(m_widget->effectComposerModel(), &EffectComposerModel::assignToSelectedTriggered, this,
-                [&] (const QString &effectPath) {
+                [this] (const QString &effectPath) {
-            executeInTransaction("EffectComposerView::widgetInfo", [&] {
+            executeInTransaction("EffectComposerView assignToSelectedTriggered", [&] {
                 const QList<QmlDesigner::ModelNode> selectedNodes = selectedModelNodes();
                 for (const QmlDesigner::ModelNode &node : selectedNodes)
                     QmlDesigner::ModelNodeOperations::handleItemLibraryEffectDrop(effectPath, node);
             });
         });
 
+        connect(m_widget->effectComposerModel(), &EffectComposerModel::removePropertiesFromScene, this,
+                [this] (QSet<QByteArray> props, const QString &typeName) {
+            // Remove specified properties from all instances of specified type
+
+            QmlDesigner::DesignDocument *document
+                = QmlDesigner::QmlDesignerPlugin::instance()->currentDesignDocument();
+            if (!document)
+                return;
+
+            const QByteArray fullType = QString("%1.%2.%2").arg(m_componentUtils.composedEffectsTypePrefix(),
+                                                             typeName).toUtf8();
+            const QList<QmlDesigner::ModelNode> allNodes = allModelNodes();
+            QList<QmlDesigner::ModelNode> typeNodes;
+            QList<QmlDesigner::ModelNode> propertyChangeNodes;
+            for (const QmlDesigner::ModelNode &node : allNodes) {
+                if (QmlDesigner::QmlPropertyChanges::isValidQmlPropertyChanges(node))
+                    propertyChangeNodes.append(node);
+#ifdef QDS_USE_PROJECTSTORAGE
+// TODO: typeName() shouldn't be used with projectstorage. Needs alternative solution (using modules?)
+#else
+                else if (node.metaInfo().typeName() == fullType)
+                    typeNodes.append(node);
+#endif
+            }
+            if (!typeNodes.isEmpty()) {
+                bool clearStacks = false;
+
+                executeInTransaction("EffectComposerView removePropertiesFromScene", [&] {
+                    for (QmlDesigner::ModelNode node : std::as_const(propertyChangeNodes)) {
+                        QmlDesigner::ModelNode targetNode = QmlDesigner::QmlPropertyChanges(node).target();
+                        if (typeNodes.contains(targetNode)) {
+                            for (const QByteArray &prop : props) {
+                                if (node.hasProperty(prop)) {
+                                    node.removeProperty(prop);
+                                    clearStacks = true;
+                                }
+                            }
+                            QList<QmlDesigner::AbstractProperty> remainingProps = node.properties();
+                            if (remainingProps.size() == 1 && remainingProps[0].name() == "target")
+                                node.destroy(); // Remove empty changes node
+                        }
+                    }
+                    for (const QmlDesigner::ModelNode &node : std::as_const(typeNodes)) {
+                        for (const QByteArray &prop : props) {
+                            if (node.hasProperty(prop)) {
+                                node.removeProperty(prop);
+                                clearStacks = true;
+                            }
+                        }
+                    }
+                });
+
+                // Reset undo stack as changing of the actual effect cannot be undone, and thus the
+                // stack will contain only unworkable states
+                if (clearStacks)
+                    document->clearUndoRedoStacks();
+            }
+        });
+
         auto context = new EffectComposerContext(m_widget.data());
         Core::ICore::addContextObject(context);
     }