From 85c4c90c636af8dc7ae35b0ce0e0476d091a6f54 Mon Sep 17 00:00:00 2001 From: Miikka Heikkinen Date: Tue, 27 Jun 2023 12:49:36 +0300 Subject: [PATCH] QmlDesigner: Delete materials and textures inside a transaction Removal of the node and references to the removed node are separate operations done during node deletion, so the deletion must be done inside a transaction to avoid getting multiple undo stack entries. Fixes: QDS-10169 Change-Id: I2ef142b98cfaa60b1130ac729dd89347bb8cac13 Reviewed-by: Mahmoud Badri Reviewed-by: Qt CI Bot --- .../materialbrowser/materialbrowsermodel.cpp | 13 +++++++++---- .../materialbrowser/materialbrowsermodel.h | 6 +++++- .../materialbrowsertexturesmodel.cpp | 13 +++++++++---- .../materialbrowser/materialbrowsertexturesmodel.h | 6 +++++- .../materialbrowser/materialbrowserwidget.cpp | 4 ++-- .../materialeditor/materialeditorview.cpp | 7 +++++-- .../components/textureeditor/textureeditorview.cpp | 7 +++++-- 7 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/plugins/qmldesigner/components/materialbrowser/materialbrowsermodel.cpp b/src/plugins/qmldesigner/components/materialbrowser/materialbrowsermodel.cpp index d6a098168c7..81ec4cbb3a5 100644 --- a/src/plugins/qmldesigner/components/materialbrowser/materialbrowsermodel.cpp +++ b/src/plugins/qmldesigner/components/materialbrowser/materialbrowsermodel.cpp @@ -4,6 +4,7 @@ #include "materialbrowsermodel.h" #include "designmodewidget.h" +#include "materialbrowserview.h" #include "qmldesignerplugin.h" #include "qmlobjectnode.h" #include "variantproperty.h" @@ -13,8 +14,9 @@ namespace QmlDesigner { -MaterialBrowserModel::MaterialBrowserModel(QObject *parent) +MaterialBrowserModel::MaterialBrowserModel(MaterialBrowserView *view, QObject *parent) : QAbstractListModel(parent) + , m_view(view) { } @@ -459,10 +461,13 @@ void MaterialBrowserModel::pasteMaterialProperties(int idx) void MaterialBrowserModel::deleteMaterial(int idx) { - if (isValidIndex(idx)) { + if (m_view && isValidIndex(idx)) { ModelNode node = m_materialList[idx]; - if (node.isValid()) - QmlObjectNode(node).destroy(); + if (node.isValid()) { + m_view->executeInTransaction(__FUNCTION__, [&] { + node.destroy(); + }); + } } } diff --git a/src/plugins/qmldesigner/components/materialbrowser/materialbrowsermodel.h b/src/plugins/qmldesigner/components/materialbrowser/materialbrowsermodel.h index 23e6a68973b..24c34394386 100644 --- a/src/plugins/qmldesigner/components/materialbrowser/materialbrowsermodel.h +++ b/src/plugins/qmldesigner/components/materialbrowser/materialbrowsermodel.h @@ -12,6 +12,8 @@ namespace QmlDesigner { +class MaterialBrowserView; + class MaterialBrowserModel : public QAbstractListModel { Q_OBJECT @@ -28,7 +30,7 @@ class MaterialBrowserModel : public QAbstractListModel Q_PROPERTY(QStringList customMaterialSections MEMBER m_customMaterialSections NOTIFY materialSectionsChanged) public: - MaterialBrowserModel(QObject *parent = nullptr); + MaterialBrowserModel(MaterialBrowserView *view, QObject *parent = nullptr); ~MaterialBrowserModel() override; int rowCount(const QModelIndex &parent = QModelIndex()) const override; @@ -125,6 +127,8 @@ private: bool m_hasMaterialLibrary = false; bool m_allPropsCopied = true; QString m_copiedMaterialType; + + QPointer m_view; }; } // namespace QmlDesigner diff --git a/src/plugins/qmldesigner/components/materialbrowser/materialbrowsertexturesmodel.cpp b/src/plugins/qmldesigner/components/materialbrowser/materialbrowsertexturesmodel.cpp index ba4536132a2..ec95f3e5d3d 100644 --- a/src/plugins/qmldesigner/components/materialbrowser/materialbrowsertexturesmodel.cpp +++ b/src/plugins/qmldesigner/components/materialbrowser/materialbrowsertexturesmodel.cpp @@ -5,6 +5,7 @@ #include "designmodewidget.h" #include "imageutils.h" +#include "materialbrowserview.h" #include "qmldesignerplugin.h" #include "qmlobjectnode.h" #include "variantproperty.h" @@ -13,8 +14,9 @@ namespace QmlDesigner { -MaterialBrowserTexturesModel::MaterialBrowserTexturesModel(QObject *parent) +MaterialBrowserTexturesModel::MaterialBrowserTexturesModel(MaterialBrowserView *view, QObject *parent) : QAbstractListModel(parent) + , m_view(view) { } @@ -292,10 +294,13 @@ void MaterialBrowserTexturesModel::duplicateTexture(int idx) void MaterialBrowserTexturesModel::deleteTexture(int idx) { - if (isValidIndex(idx)) { + if (m_view && isValidIndex(idx)) { ModelNode node = m_textureList[idx]; - if (node.isValid()) - QmlObjectNode(node).destroy(); + if (node.isValid()) { + m_view->executeInTransaction(__FUNCTION__, [&] { + node.destroy(); + }); + } } } diff --git a/src/plugins/qmldesigner/components/materialbrowser/materialbrowsertexturesmodel.h b/src/plugins/qmldesigner/components/materialbrowser/materialbrowsertexturesmodel.h index 9cb7c5ac18f..8836d3b5db8 100644 --- a/src/plugins/qmldesigner/components/materialbrowser/materialbrowsertexturesmodel.h +++ b/src/plugins/qmldesigner/components/materialbrowser/materialbrowsertexturesmodel.h @@ -10,6 +10,8 @@ namespace QmlDesigner { +class MaterialBrowserView; + class MaterialBrowserTexturesModel : public QAbstractListModel { Q_OBJECT @@ -20,7 +22,7 @@ class MaterialBrowserTexturesModel : public QAbstractListModel Q_PROPERTY(bool hasSceneEnv READ hasSceneEnv NOTIFY hasSceneEnvChanged) public: - MaterialBrowserTexturesModel(QObject *parent = nullptr); + MaterialBrowserTexturesModel(MaterialBrowserView *view, QObject *parent = nullptr); ~MaterialBrowserTexturesModel() override; int rowCount(const QModelIndex &parent = QModelIndex()) const override; @@ -89,6 +91,8 @@ private: bool m_hasSingleModelSelection = false; bool m_hasSceneEnv = false; + QPointer m_view; + enum { RoleTexHasDynamicProps = Qt::UserRole + 1, RoleTexInternalId, diff --git a/src/plugins/qmldesigner/components/materialbrowser/materialbrowserwidget.cpp b/src/plugins/qmldesigner/components/materialbrowser/materialbrowserwidget.cpp index e2bb82216bb..bde22fdccad 100644 --- a/src/plugins/qmldesigner/components/materialbrowser/materialbrowserwidget.cpp +++ b/src/plugins/qmldesigner/components/materialbrowser/materialbrowserwidget.cpp @@ -141,8 +141,8 @@ bool MaterialBrowserWidget::eventFilter(QObject *obj, QEvent *event) MaterialBrowserWidget::MaterialBrowserWidget(AsynchronousImageCache &imageCache, MaterialBrowserView *view) : m_materialBrowserView(view) - , m_materialBrowserModel(new MaterialBrowserModel(this)) - , m_materialBrowserTexturesModel(new MaterialBrowserTexturesModel(this)) + , m_materialBrowserModel(new MaterialBrowserModel(view, this)) + , m_materialBrowserTexturesModel(new MaterialBrowserTexturesModel(view, this)) , m_quickWidget(new StudioQuickWidget(this)) , m_previewImageProvider(new PreviewImageProvider()) { diff --git a/src/plugins/qmldesigner/components/materialeditor/materialeditorview.cpp b/src/plugins/qmldesigner/components/materialeditor/materialeditorview.cpp index 55c587f3cb4..933686febda 100644 --- a/src/plugins/qmldesigner/components/materialeditor/materialeditorview.cpp +++ b/src/plugins/qmldesigner/components/materialeditor/materialeditorview.cpp @@ -442,8 +442,11 @@ void MaterialEditorView::handleToolBarAction(int action) } case MaterialEditorContextObject::DeleteCurrentMaterial: { - if (m_selectedMaterial.isValid()) - m_selectedMaterial.destroy(); + if (m_selectedMaterial.isValid()) { + executeInTransaction(__FUNCTION__, [&] { + m_selectedMaterial.destroy(); + }); + } break; } diff --git a/src/plugins/qmldesigner/components/textureeditor/textureeditorview.cpp b/src/plugins/qmldesigner/components/textureeditor/textureeditorview.cpp index 0c2756db238..98601ce45ed 100644 --- a/src/plugins/qmldesigner/components/textureeditor/textureeditorview.cpp +++ b/src/plugins/qmldesigner/components/textureeditor/textureeditorview.cpp @@ -389,8 +389,11 @@ void TextureEditorView::handleToolBarAction(int action) } case TextureEditorContextObject::DeleteCurrentTexture: { - if (m_selectedTexture.isValid()) - m_selectedTexture.destroy(); + if (m_selectedTexture.isValid()) { + executeInTransaction(__FUNCTION__, [&] { + m_selectedTexture.destroy(); + }); + } break; }