Merge pull request #26648 from RomanPudashkin/undo_add_instrument_crash

Fix #26598: Crash after adding instrument, Undo and Redo

src/engraving/dom/types.h

@@ -553,6 +553,23 @@ struct ScoreChangesRange {
     {
         return isValidBoundary() || !changedTypes.empty();
     }
+
+    void clear()
+    {
+        *this = ScoreChangesRange();
+    }
+
+    void combine(const ScoreChangesRange& r)
+    {
+        tickFrom = std::min(tickFrom, r.tickFrom);
+        tickTo = std::max(tickTo, r.tickTo);
+        staffIdxFrom = std::min(staffIdxFrom, r.staffIdxFrom);
+        staffIdxTo = std::max(staffIdxTo, r.staffIdxTo);
+        changedItems.insert(r.changedItems.begin(), r.changedItems.end());
+        changedTypes.insert(r.changedTypes.begin(), r.changedTypes.end());
+        changedPropertyIdSet.insert(r.changedPropertyIdSet.begin(), r.changedPropertyIdSet.end());
+        changedStyleIdSet.insert(r.changedStyleIdSet.begin(), r.changedStyleIdSet.end());
+    }
 };
 } // namespace mu::engraving
 

src/instrumentsscene/view/abstractlayoutpaneltreeitem.cpp

@@ -164,6 +164,10 @@ void AbstractLayoutPanelTreeItem::removeChildren(int row, int count, bool delete
     }
 }
 
+void AbstractLayoutPanelTreeItem::onScoreChanged(const mu::engraving::ScoreChangesRange&)
+{
+}
+
 AbstractLayoutPanelTreeItem* AbstractLayoutPanelTreeItem::parentItem() const
 {
     return m_parent;

src/instrumentsscene/view/abstractlayoutpaneltreeitem.h

@@ -91,6 +91,8 @@ class AbstractLayoutPanelTreeItem : public QObject
 
     virtual void removeChildren(int row, int count = 1, bool deleteChild = false);
 
+    virtual void onScoreChanged(const mu::engraving::ScoreChangesRange& changes);
+
     AbstractLayoutPanelTreeItem* parentItem() const;
     void setParentItem(AbstractLayoutPanelTreeItem* parent);
 

src/instrumentsscene/view/layoutpaneltreemodel.cpp

@@ -269,11 +269,20 @@ void LayoutPanelTreeModel::setupStavesConnections(const muse::ID& stavesPartId)
     });
 }
 
-void LayoutPanelTreeModel::listenNotationSelectionChanged()
+void LayoutPanelTreeModel::setupNotationConnections()
 {
     m_notation->interaction()->selectionChanged().onNotify(this, [this]() {
         updateSelectedRows();
     });
+
+    m_notation->undoStack()->changesChannel().onReceive(this, [this](const mu::engraving::ScoreChangesRange& changes) {
+        if (!m_layoutPanelVisible) {
+            m_scoreChangesCache.combine(changes);
+            return;
+        }
+
+        onScoreChanged(changes);
+    });
 }
 
 void LayoutPanelTreeModel::updateSelectedRows()
@@ -307,6 +316,17 @@ void LayoutPanelTreeModel::updateSelectedRows()
     }
 }
 
+void LayoutPanelTreeModel::onScoreChanged(const mu::engraving::ScoreChangesRange& changes)
+{
+    if (!m_rootItem) {
+        return;
+    }
+
+    for (AbstractLayoutPanelTreeItem* item : m_rootItem->childItems()) {
+        item->onScoreChanged(changes);
+    }
+}
+
 void LayoutPanelTreeModel::clear()
 {
     TRACEFUNC;
@@ -362,7 +382,7 @@ void LayoutPanelTreeModel::load()
     endResetModel();
 
     setupPartsConnections();
-    listenNotationSelectionChanged();
+    setupNotationConnections();
 
     emit isEmptyChanged();
     emit isAddingAvailableChanged(true);
@@ -405,6 +425,11 @@ void LayoutPanelTreeModel::setLayoutPanelVisible(bool visible)
 
     if (visible) {
         updateSelectedRows();
+
+        if (m_scoreChangesCache.isValid()) {
+            onScoreChanged(m_scoreChangesCache);
+            m_scoreChangesCache.clear();
+        }
     }
 }
 
@@ -964,7 +989,8 @@ void LayoutPanelTreeModel::updateSystemObjectLayers()
     m_shouldUpdateSystemObjectLayers = false;
 
     // Create copy, because we're going to modify them
-    std::vector<Staff*> newSystemObjectStaves = m_masterNotation->notation()->parts()->systemObjectStaves();
+    const INotationPartsPtr notationParts = m_masterNotation->notation()->parts();
+    std::vector<Staff*> newSystemObjectStaves = notationParts->systemObjectStaves();
     QList<AbstractLayoutPanelTreeItem*> children = m_rootItem->childItems();
 
     // Remove old system object layers
@@ -991,6 +1017,8 @@ void LayoutPanelTreeModel::updateSystemObjectLayers()
         endRemoveRows();
     }
 
+    SystemObjectGroupsByStaff systemObjects = collectSystemObjectGroups(notationParts->systemObjectStaves());
+
     // Update position of existing layers if changed
     for (SystemObjectsLayerTreeItem* layerItem : existingSystemObjectLayers) {
         const PartTreeItem* partItem = findPartItemByStaff(layerItem->staff());
@@ -1007,16 +1035,10 @@ void LayoutPanelTreeModel::updateSystemObjectLayers()
             endMoveRows();
         }
 
-        layerItem->updateSystemObjects();
-    }
-
-    if (newSystemObjectStaves.empty()) {
-        return;
+        layerItem->setSystemObjects(systemObjects[layerItem->staff()]);
     }
 
     // Create new system object layers
-    SystemObjectGroupsByStaff systemObjects = collectSystemObjectGroups(newSystemObjectStaves);
-
     for (const Staff* staff : newSystemObjectStaves) {
         for (const PartTreeItem* partItem : partItems) {
             if (staff->part() != partItem->part()) {

src/instrumentsscene/view/layoutpaneltreemodel.h

@@ -140,8 +140,10 @@ private slots:
 
     void setupPartsConnections();
     void setupStavesConnections(const muse::ID& stavesPartId);
-    void listenNotationSelectionChanged();
+    void setupNotationConnections();
+
     void updateSelectedRows();
+    void onScoreChanged(const mu::engraving::ScoreChangesRange& changes);
 
     void clear();
     void deleteItems();
@@ -183,8 +185,9 @@ private slots:
     using NotationKey = QString;
     QHash<NotationKey, QList<muse::ID> > m_sortedPartIdList;
 
-    bool m_layoutPanelVisible = true;
+    mu::engraving::ScoreChangesRange m_scoreChangesCache;
 
+    bool m_layoutPanelVisible = true;
     bool m_shouldUpdateSystemObjectLayers = false;
 
     bool m_dragInProgress = false;

src/instrumentsscene/view/systemobjectslayertreeitem.cpp

@@ -88,20 +88,13 @@ SystemObjectsLayerTreeItem::SystemObjectsLayerTreeItem(IMasterNotationPtr master
 
 void SystemObjectsLayerTreeItem::init(const Staff* staff, const SystemObjectGroups& systemObjects)
 {
-    m_systemObjectGroups = systemObjects;
-
     setStaff(staff);
+    setSystemObjects(systemObjects);
 
     bool isTopLayer = staff->score()->staff(0) == staff;
     setIsRemovable(!isTopLayer);
     setIsSelectable(!isTopLayer);
 
-    updateState();
-
-    notation()->undoStack()->changesChannel().onReceive(this, [this](const ChangesRange& changes) {
-        onUndoStackChanged(changes);
-    });
-
     connect(this, &AbstractLayoutPanelTreeItem::isVisibleChanged, this, [this](bool isVisible) {
         onVisibleChanged(isVisible);
     });
@@ -127,9 +120,9 @@ void SystemObjectsLayerTreeItem::setStaff(const Staff* staff)
     }
 }
 
-void SystemObjectsLayerTreeItem::updateSystemObjects()
+void SystemObjectsLayerTreeItem::setSystemObjects(const SystemObjectGroups& systemObjects)
 {
-    m_systemObjectGroups = collectSystemObjectGroups(m_staff);
+    m_systemObjectGroups = systemObjects;
     updateState();
 }
 
@@ -144,7 +137,7 @@ bool SystemObjectsLayerTreeItem::canAcceptDrop(const QVariant&) const
     return false;
 }
 
-void SystemObjectsLayerTreeItem::onUndoStackChanged(const mu::engraving::ScoreChangesRange& changes)
+void SystemObjectsLayerTreeItem::onScoreChanged(const mu::engraving::ScoreChangesRange& changes)
 {
     if (muse::contains(changes.changedPropertyIdSet, Pid::TRACK)) {
         updateStaff();

src/instrumentsscene/view/systemobjectslayertreeitem.h

@@ -23,14 +23,13 @@
 #pragma once
 
 #include "abstractlayoutpaneltreeitem.h"
-#include "async/asyncable.h"
 
 #include "notation/inotationparts.h"
 
 #include "layoutpanelutils.h"
 
 namespace mu::instrumentsscene {
-class SystemObjectsLayerTreeItem : public AbstractLayoutPanelTreeItem, public muse::async::Asyncable
+class SystemObjectsLayerTreeItem : public AbstractLayoutPanelTreeItem
 {
     Q_OBJECT
 
@@ -41,13 +40,13 @@ class SystemObjectsLayerTreeItem : public AbstractLayoutPanelTreeItem, public mu
 
     const mu::engraving::Staff* staff() const;
     void setStaff(const mu::engraving::Staff* staff);
-    void updateSystemObjects();
+    void setSystemObjects(const SystemObjectGroups& systemObjects);
 
     Q_INVOKABLE QString staffId() const;
     Q_INVOKABLE bool canAcceptDrop(const QVariant& item) const override;
 
 private:
-    void onUndoStackChanged(const mu::engraving::ScoreChangesRange& changes);
+    void onScoreChanged(const mu::engraving::ScoreChangesRange& changes) override;
     void onVisibleChanged(bool isVisible);
 
     bool addSystemObject(mu::engraving::EngravingItem* obj);

src/notation/internal/masternotationparts.cpp

@@ -94,8 +94,7 @@ void MasterNotationParts::setParts(const PartInstrumentList& partList, const Sco
         impl->setBracketsAndBarlines();
     }
 
-    updatePartList();
-    updateSystemObjectStaves();
+    updatePartsAndSystemObjectStaves();
     endGlobalEdit();
 }
 

src/notation/internal/notationparts.cpp

@@ -242,8 +242,7 @@ void NotationParts::setParts(const PartInstrumentList& parts, const ScoreOrder&
     updateSoloist(parts);
     sortParts(parts);
     setBracketsAndBarlines();
-    updatePartList();
-    updateSystemObjectStaves();
+    updatePartsAndSystemObjectStaves();
 
     apply();
 }
@@ -330,8 +329,7 @@ void NotationParts::listenUndoStackChanges()
         return;
     }
 
-    updatePartList();
-    updateSystemObjectStaves();
+    updatePartsAndSystemObjectStaves();
 
     m_undoStack->changesChannel().onReceive(this, [this](const ChangesRange& range) {
         if (range.changedTypes.empty()) {
@@ -346,23 +344,14 @@ void NotationParts::listenUndoStackChanges()
 
         for (ElementType type : TYPES_TO_CHECK) {
             if (muse::contains(range.changedTypes, type)) {
-                updatePartList();
-                updateSystemObjectStaves();
+                updatePartsAndSystemObjectStaves();
                 return;
             }
         }
     });
 }
 
-void NotationParts::updatePartList()
-{
-    if (m_parts != score()->parts()) {
-        m_parts = score()->parts();
-        m_partChangedNotifier.changed();
-    }
-}
-
-void NotationParts::updateSystemObjectStaves()
+void NotationParts::updatePartsAndSystemObjectStaves()
 {
     const auto systemObjectStavesWithTopStaff = [this]() {
         std::vector<Staff*> result;
@@ -375,10 +364,18 @@ void NotationParts::updateSystemObjectStaves()
         return result;
     };
 
+    const bool partsChanged = m_parts != score()->parts();
+    m_parts = score()->parts();
+
     std::vector<Staff*> newSystemObjectStaves = systemObjectStavesWithTopStaff();
+    const bool systemObjectStavesChanged = m_systemObjectStaves != newSystemObjectStaves;
+    m_systemObjectStaves = std::move(newSystemObjectStaves);
+
+    if (partsChanged) {
+        m_partChangedNotifier.changed();
+    }
 
-    if (m_systemObjectStaves != newSystemObjectStaves) {
-        m_systemObjectStaves = std::move(newSystemObjectStaves);
+    if (systemObjectStavesChanged) {
         m_systemObjectStavesChanged.notify();
     }
 }

src/notation/internal/notationparts.h

@@ -101,9 +101,7 @@ class NotationParts : public INotationParts, public muse::async::Asyncable
     friend class MasterNotationParts;
 
     void listenUndoStackChanges();
-
-    void updatePartList();
-    void updateSystemObjectStaves();
+    void updatePartsAndSystemObjectStaves();
 
     void doSetScoreOrder(const ScoreOrder& order);
     void doRemoveParts(const std::vector<Part*>& parts);