QmlDesigner: Fix dynamic properties on material library init

Nodes with dynamic properties cannot be reparented under a newly
created node in the same transaction without breaking said properties.

To work around this issue, ensureMaterialLibraryNode() was split into
two transactions. This also meant removing ensureMaterialLibraryNode()
call from materialLibraryNode(), so now material library will only be
created in response to model attach or qtquick3d import addition.
This should still cover all cases where user doesn't manually remove
the material library node.

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

2 files changed, 31 insertions, 32 deletions

diff --git a/src/plugins/qmldesigner/components/materialeditor/materialeditorview.cpp b/src/plugins/qmldesigner/components/materialeditor/materialeditorview.cpp
index c8c42b2fa9..f6ed9acff9 100644
--- a/src/plugins/qmldesigner/components/materialeditor/materialeditorview.cpp
+++ b/src/plugins/qmldesigner/components/materialeditor/materialeditorview.cpp
@@ -80,9 +80,7 @@ MaterialEditorView::MaterialEditorView(QWidget *parent)
     m_ensureMatLibTimer.callOnTimeout([this] {
         if (model() && model()->rewriterView() && !model()->rewriterView()->hasIncompleteTypeInformation()
             && model()->rewriterView()->errors().isEmpty()) {
-            executeInTransaction("MaterialEditorView::MaterialEditorView", [this] {
-                ensureMaterialLibraryNode();
-            });
+            ensureMaterialLibraryNode();
             m_ensureMatLibTimer.stop();
         }
     });
diff --git a/src/plugins/qmldesigner/designercore/model/abstractview.cpp b/src/plugins/qmldesigner/designercore/model/abstractview.cpp
index 5e0e07fde1..8012bc2236 100644
--- a/src/plugins/qmldesigner/designercore/model/abstractview.cpp
+++ b/src/plugins/qmldesigner/designercore/model/abstractview.cpp
@@ -829,47 +829,48 @@ void AbstractView::changeRootNodeType(const TypeName &type, int majorVersion, in
     m_model.data()->d->changeRootNodeType(type, majorVersion, minorVersion);
 }
 
-// Creates material library if it doesn't exist and moves any existing materials into it
-// This function should be called only from inside a transaction, as it potentially does many
-// changes to model, or undo stack should be cleared after the call.
+// Creates material library if it doesn't exist and moves any existing materials into it.
 void AbstractView::ensureMaterialLibraryNode()
 {
     ModelNode matLib = modelNodeForId(Constants::MATERIAL_LIB_ID);
     if (matLib.isValid() || rootModelNode().isSubclassOf("QtQuick3D.Material"))
         return;
 
-    // Create material library node
-    TypeName nodeType = rootModelNode().isSubclassOf("QtQuick3D.Node") ? "QtQuick3D.Node"
-                                                                       : "QtQuick.Item";
-    NodeMetaInfo metaInfo = model()->metaInfo(nodeType);
-    matLib = createModelNode(nodeType, metaInfo.majorVersion(), metaInfo.minorVersion());
-
-    matLib.setIdWithoutRefactoring(Constants::MATERIAL_LIB_ID);
-    rootModelNode().defaultNodeListProperty().reparentHere(matLib);
-
-    const QList<ModelNode> materials = rootModelNode().subModelNodesOfType("QtQuick3D.Material");
-    if (!materials.isEmpty()) {
-        // Move all materials to under material library node
-        for (const ModelNode &node : materials) {
-            // If material has no name, set name to id
-            QString matName = node.variantProperty("objectName").value().toString();
-            if (matName.isEmpty()) {
-                VariantProperty objNameProp = node.variantProperty("objectName");
-                objNameProp.setValue(node.id());
-            }
+    executeInTransaction(__FUNCTION__, [&] {
+        // Create material library node
+        TypeName nodeType = rootModelNode().isSubclassOf("QtQuick3D.Node") ? "QtQuick3D.Node"
+                                                                           : "QtQuick.Item";
+        NodeMetaInfo metaInfo = model()->metaInfo(nodeType);
+
+        matLib = createModelNode(nodeType, metaInfo.majorVersion(), metaInfo.minorVersion());
 
-            matLib.defaultNodeListProperty().reparentHere(node);
+        matLib.setIdWithoutRefactoring(Constants::MATERIAL_LIB_ID);
+        rootModelNode().defaultNodeListProperty().reparentHere(matLib);
+    });
+
+    // Do the material reparentings in different transaction to work around issue QDS-8094
+    executeInTransaction(__FUNCTION__, [&] {
+        const QList<ModelNode> materials = rootModelNode().subModelNodesOfType("QtQuick3D.Material");
+        if (!materials.isEmpty()) {
+            // Move all materials to under material library node
+            for (const ModelNode &node : materials) {
+                // If material has no name, set name to id
+                QString matName = node.variantProperty("objectName").value().toString();
+                if (matName.isEmpty()) {
+                    VariantProperty objNameProp = node.variantProperty("objectName");
+                    objNameProp.setValue(node.id());
+                }
+
+                matLib.defaultNodeListProperty().reparentHere(node);
+            }
         }
-    }
+    });
 }
 
-// Returns ModelNode for project's material library.
-// Since this calls ensureMaterialLibraryNode(), it should only be called within a transaction.
+// Returns ModelNode for project's material library if it exists.
 ModelNode AbstractView::materialLibraryNode()
 {
-    ensureMaterialLibraryNode();
-    ModelNode matLib = modelNodeForId(Constants::MATERIAL_LIB_ID);
-    return matLib;
+    return modelNodeForId(Constants::MATERIAL_LIB_ID);
 }
 
 // Assigns given material to a 3D model.