From 3700eac9f34bbafe337e4fe7cd6f31b838d17744 Mon Sep 17 00:00:00 2001 From: Miikka Heikkinen Date: Fri, 12 Apr 2024 15:38:50 +0300 Subject: [PATCH] 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 Reviewed-by: Qt CI Patch Build Bot Reviewed-by: --- .../effectcomposer/effectcomposermodel.cpp | 41 +++++++++++- .../effectcomposer/effectcomposermodel.h | 3 + .../effectcomposer/effectcomposerview.cpp | 65 ++++++++++++++++++- 3 files changed, 106 insertions(+), 3 deletions(-) diff --git a/src/plugins/effectcomposer/effectcomposermodel.cpp b/src/plugins/effectcomposer/effectcomposermodel.cpp index b3df849e059..a983072334b 100644 --- a/src/plugins/effectcomposer/effectcomposermodel.cpp +++ b/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 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 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 EffectComposerModel::getExposedProperties(const QByteArray &qmlContent) +{ + QSet 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; diff --git a/src/plugins/effectcomposer/effectcomposermodel.h b/src/plugins/effectcomposer/effectcomposermodel.h index 2381b09ec1c..14ef09e8a97 100644 --- a/src/plugins/effectcomposer/effectcomposermodel.h +++ b/src/plugins/effectcomposer/effectcomposermodel.h @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -124,6 +125,7 @@ signals: void resourcesSaved(const QByteArray &type, const Utils::FilePath &path); void hasUnsavedChangesChanged(); void assignToSelectedTriggered(const QString &effectPath); + void removePropertiesFromScene(QSet props, const QString &typeName); private: enum Roles { @@ -185,6 +187,7 @@ private: void connectCompositionNode(CompositionNode *node); void updateExtraMargin(); + QSet getExposedProperties(const QByteArray &qmlContent); QList m_nodes; diff --git a/src/plugins/effectcomposer/effectcomposerview.cpp b/src/plugins/effectcomposer/effectcomposerview.cpp index c7967db4ae9..48c6a33c4b6 100644 --- a/src/plugins/effectcomposer/effectcomposerview.cpp +++ b/src/plugins/effectcomposer/effectcomposerview.cpp @@ -10,7 +10,9 @@ #include #include #include +#include #include +#include #include @@ -49,14 +51,73 @@ QmlDesigner::WidgetInfo EffectComposerView::widgetInfo() m_widget = new EffectComposerWidget{this}; connect(m_widget->effectComposerModel(), &EffectComposerModel::assignToSelectedTriggered, this, - [&] (const QString &effectPath) { - executeInTransaction("EffectComposerView::widgetInfo", [&] { + [this] (const QString &effectPath) { + executeInTransaction("EffectComposerView assignToSelectedTriggered", [&] { const QList selectedNodes = selectedModelNodes(); for (const QmlDesigner::ModelNode &node : selectedNodes) QmlDesigner::ModelNodeOperations::handleItemLibraryEffectDrop(effectPath, node); }); }); + connect(m_widget->effectComposerModel(), &EffectComposerModel::removePropertiesFromScene, this, + [this] (QSet 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 allNodes = allModelNodes(); + QList typeNodes; + QList 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 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); }