Fix undo and redo matdef saving

Task-number: QT3DS-2706
Change-Id: I797d615df13029bf062b5437ac588dbc5c77324b
Reviewed-by: Miikka Heikkinen <miikka.heikkinen@qt.io>

4 files changed, 99 insertions, 79 deletions

diff --git a/src/Authoring/Client/Code/Core/Commands/CmdStack.cpp b/src/Authoring/Client/Code/Core/Commands/CmdStack.cpp
index c6aa0882..3530822f 100644
--- a/src/Authoring/Client/Code/Core/Commands/CmdStack.cpp
+++ b/src/Authoring/Client/Code/Core/Commands/CmdStack.cpp
@@ -141,6 +141,7 @@ void CCmdStack::Undo()
             return;
     }
     if (m_UndoList.size() > 0) {
+        m_undoingOrRedoing = true;
         CCmd *theLastCommand = m_UndoList.back();
         m_UndoList.pop_back();
 
@@ -162,6 +163,7 @@ void CCmdStack::Undo()
                 m_Listener->SetCommandModifiedFlag(TRUE);
             }
         }
+        m_undoingOrRedoing = false;
     }
 }
 
@@ -175,6 +177,7 @@ void CCmdStack::Undo()
 void CCmdStack::Redo()
 {
     if (m_RedoList.size() > 0) {
+        m_undoingOrRedoing = true;
         CCmd *theLastCommand = m_RedoList.back();
         m_RedoList.pop_back();
 
@@ -191,6 +194,7 @@ void CCmdStack::Redo()
                 m_Listener->SetCommandModifiedFlag(TRUE);
             }
         }
+        m_undoingOrRedoing = false;
     }
 }
 
@@ -216,6 +220,11 @@ bool CCmdStack::CanRedo()
     return m_RedoList.size() > 0;
 }
 
+bool CCmdStack::isUndoingOrRedoing() const
+{
+    return m_undoingOrRedoing;
+}
+
 //=============================================================================
 /**
  * Remove the last command from the undo list
diff --git a/src/Authoring/Client/Code/Core/Commands/CmdStack.h b/src/Authoring/Client/Code/Core/Commands/CmdStack.h
index 4aabd228..7388703d 100644
--- a/src/Authoring/Client/Code/Core/Commands/CmdStack.h
+++ b/src/Authoring/Client/Code/Core/Commands/CmdStack.h
@@ -80,6 +80,8 @@ public:
     bool CanUndo();
     bool CanRedo();
 
+    bool isUndoingOrRedoing() const;
+
     void RemoveLastUndo();
 
     QString GetUndoDescription();
@@ -105,6 +107,8 @@ protected:
     TCmdList m_UndoList;
     TCmdList m_RedoList;
 
+    bool m_undoingOrRedoing = false;
+
     unsigned long m_MaxUndoStackSize;
 
     CModificationListener *m_Listener;
diff --git a/src/Authoring/Client/Code/Core/Doc/Doc.cpp b/src/Authoring/Client/Code/Core/Doc/Doc.cpp
index b632513d..3457d50a 100644
--- a/src/Authoring/Client/Code/Core/Doc/Doc.cpp
+++ b/src/Authoring/Client/Code/Core/Doc/Doc.cpp
@@ -1275,7 +1275,11 @@ void CDoc::onPropertyChanged(qt3dsdm::Qt3DSDMInstanceHandle inInstance,
                              qt3dsdm::Qt3DSDMPropertyHandle inProperty)
 {
     using namespace qt3dsdm;
-
+    // Save the material definition upon undo and redo
+    if (m_Core->GetCmdStack()->isUndoingOrRedoing() &&
+            m_StudioSystem->GetClientDataModelBridge()->isInsideMaterialContainer(inInstance)) {
+        getSceneEditor()->saveIfMaterial(inInstance);
+    }
     // check if we changed datainput bindings
     if (inProperty == m_StudioSystem->GetPropertySystem()
             ->GetAggregateInstancePropertyByName(inInstance, L"controlledproperty")) {
diff --git a/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.cpp b/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.cpp
index b1703dab..a1183c72 100644
--- a/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.cpp
+++ b/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.cpp
@@ -513,98 +513,101 @@ void InspectorControlModel::setMatDatas(const std::vector<Q3DStudio::CFilePath>
     const auto doc = g_StudioApp.GetCore()->GetDoc();
 
     bool isDocModified = doc->IsModified();
-    { // Scope for the ScopedDocumentEditor
-        Q3DStudio::ScopedDocumentEditor sceneEditor(
-                    Q3DStudio::SCOPED_DOCUMENT_EDITOR(*doc, QString()));
-
-        bool newMaterialSelected = false;
-        for (const Q3DStudio::CFilePath &path : matDatas) {
-            bool isNewFile = true;
-            for (auto &oldPath : m_cachedMatDatas) {
-                if (path.toQString() == oldPath.toQString()) {
-                    isNewFile = false;
-                    break;
-                }
+    const auto sceneEditor = doc->getSceneEditor();
+    if (!sceneEditor)
+        return;
+
+    bool newMaterialSelected = false;
+    for (const Q3DStudio::CFilePath &path : matDatas) {
+        bool isNewFile = true;
+        for (auto &oldPath : m_cachedMatDatas) {
+            if (path.toQString() == oldPath.toQString()) {
+                isNewFile = false;
+                break;
             }
+        }
 
-            const QString relativePath = path.toQString();
-            const Q3DStudio::CFilePath absolutePath
-                = Q3DStudio::CFilePath::CombineBaseAndRelative(doc->GetDocumentDirectory(), path);
-
-            QString name;
-            QMap<QString, QString> values;
-            QMap<QString, QMap<QString, QString>> textureValues;
-            sceneEditor->getMaterialInfo(
-                        absolutePath.toQString(), name, values, textureValues);
-
-            m_matDatas.push_back({name, relativePath, values, textureValues});
-
-            bool needRewrite = false;
-            if (values.contains(QStringLiteral("path"))) {
-                const QString oldPath = values[QStringLiteral("path")];
-                needRewrite = oldPath != absolutePath.toQString();
-                if (!QFileInfo(oldPath).exists()) {
-                    const auto instance = sceneEditor->getMaterial(oldPath);
-                    if (instance.Valid()) {
-                        const QString oldName = sceneEditor->GetName(instance).toQString();
-                        const QString newName = sceneEditor
-                                ->getMaterialNameFromFilePath(relativePath);
-                        const QString actualPath = sceneEditor
-                                ->getFilePathFromMaterialName(oldName);
-                        if (actualPath == oldPath) {
-                            sceneEditor->setMaterialNameByPath(instance, relativePath);
-
-                            QVector<qt3dsdm::Qt3DSDMInstanceHandle> refMats;
-                            doc->getSceneReferencedMaterials(doc->GetSceneInstance(), refMats);
-                            for (auto &refMat : qAsConst(refMats)) {
-                                const auto origMat = bridge->getMaterialReference(refMat);
-                                if (origMat.Valid() && origMat == instance) {
-                                    sceneEditor->setMaterialSourcePath(
-                                                refMat,
-                                                Q3DStudio::CString::fromQString(relativePath));
-                                    sceneEditor->SetName(refMat, bridge->GetName(instance, true));
-                                    studio->GetFullSystemSignalSender()
-                                            ->SendInstancePropertyValue(refMat,
-                                                                        bridge->GetNameProperty());
-                                }
+        const QString relativePath = path.toQString();
+        const Q3DStudio::CFilePath absolutePath
+            = Q3DStudio::CFilePath::CombineBaseAndRelative(doc->GetDocumentDirectory(), path);
+
+        QString name;
+        QMap<QString, QString> values;
+        QMap<QString, QMap<QString, QString>> textureValues;
+        sceneEditor->getMaterialInfo(
+                    absolutePath.toQString(), name, values, textureValues);
+
+        m_matDatas.push_back({name, relativePath, values, textureValues});
+
+        bool needRewrite = false;
+        if (values.contains(QStringLiteral("path"))) {
+            const QString oldPath = values[QStringLiteral("path")];
+            needRewrite = oldPath != absolutePath.toQString();
+            if (!QFileInfo(oldPath).exists()) {
+                const auto instance = sceneEditor->getMaterial(oldPath);
+                if (instance.Valid()) {
+                    const QString oldName = sceneEditor->GetName(instance).toQString();
+                    const QString newName = sceneEditor
+                            ->getMaterialNameFromFilePath(relativePath);
+                    const QString actualPath = sceneEditor
+                            ->getFilePathFromMaterialName(oldName);
+                    if (actualPath == oldPath) {
+                        sceneEditor->setMaterialNameByPath(instance, relativePath);
+
+                        QVector<qt3dsdm::Qt3DSDMInstanceHandle> refMats;
+                        doc->getSceneReferencedMaterials(doc->GetSceneInstance(), refMats);
+                        for (auto &refMat : qAsConst(refMats)) {
+                            const auto origMat = bridge->getMaterialReference(refMat);
+                            if (origMat.Valid() && origMat == instance) {
+                                sceneEditor->setMaterialSourcePath(
+                                            refMat,
+                                            Q3DStudio::CString::fromQString(relativePath));
+                                sceneEditor->SetName(refMat, bridge->GetName(instance, true));
+                                studio->GetFullSystemSignalSender()
+                                        ->SendInstancePropertyValue(refMat,
+                                                                    bridge->GetNameProperty());
                             }
-                            g_StudioApp.GetCore()->getProjectFile().renameMaterial(
-                                        oldName, newName);
-                            isDocModified = true;
                         }
+                        g_StudioApp.GetCore()->getProjectFile().renameMaterial(
+                                    oldName, newName);
+                        isDocModified = true;
                     }
                 }
             }
+        }
 
-            auto material = sceneEditor->getMaterial(relativePath);
-            if (isNewFile && !newMaterialSelected && !material.Valid())
-                material = sceneEditor->getOrCreateMaterial(relativePath, false);
-
-            if (material.Valid())
-                sceneEditor->setMaterialValues(relativePath, values, textureValues);
-
-            if (isNewFile && !newMaterialSelected) {
-                doc->SelectDataModelObject(material);
-                newMaterialSelected = true;
+        auto material = sceneEditor->getMaterial(relativePath);
+        if (isNewFile && !newMaterialSelected && !material.Valid()) {
+            {
+                material = Q3DStudio::SCOPED_DOCUMENT_EDITOR(*doc, QString())
+                        ->getOrCreateMaterial(relativePath, false);
             }
-
-            if (needRewrite && material.Valid())
-                sceneEditor->writeMaterialFile(material, name, false, absolutePath.toQString());
+            // Several aspects of the editor are not updated correctly
+            // if the data core is changed without a transaction
+            // The above scope completes the transaction for creating a new material
+            // Next the added undo has to be popped from the stack
+            // TODO: Find a way to update the editor fully without a transaction
+            g_StudioApp.GetCore()->GetCmdStack()->RemoveLastUndo();
         }
 
-        if (isBasicMaterial())
-            updateMatDataValues();
+        if (material.Valid())
+            sceneEditor->setMaterialValues(relativePath, values, textureValues);
 
-        sceneEditor->removeDeletedFromMaterialContainer();
+        if (isNewFile && !newMaterialSelected) {
+            doc->SelectDataModelObject(material);
+            newMaterialSelected = true;
+        }
+
+        if (needRewrite && material.Valid())
+            sceneEditor->writeMaterialFile(material, name, false, absolutePath.toQString());
     }
-    // Several aspects of the editor are not updated correctly
-    // if the data core is changed without a transaction
-    // The above scope completes the transaction for creating a new material
-    // Next the added undo has to be popped from the stack
-    // and the modified flag has to be restored
-    // TODO: Find a way to update the editor fully without a transaction
+
+    if (isBasicMaterial())
+        updateMatDataValues();
+
+    sceneEditor->removeDeletedFromMaterialContainer();
+    // Modified flag has to be restored because of the hidden transaction
     doc->SetModifiedFlag(isDocModified);
-    g_StudioApp.GetCore()->GetCmdStack()->RemoveLastUndo();
 
     m_cachedMatDatas = matDatas;
 }