Merge pull request #24221 from cbjeukendrup/crash_reorder_delete_stav…

…es_caused_by_brackets Fix crash when reordering and deleting staves, caused by brackets
musescore · Aug 27, 2024 · 1a96357 · 1a96357
2 parents 807c9b0 + 2fee030
commit 1a96357
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 32 deletions.
diff --git a/src/engraving/dom/score.cpp b/src/engraving/dom/score.cpp
@@ -2702,15 +2702,34 @@ void Score::removeStaff(Staff* staff)
 
 void Score::adjustBracketsDel(size_t sidx, size_t eidx)
 {
-    for (size_t staffIdx = 0; staffIdx < m_staves.size(); ++staffIdx) {
+    IF_ASSERT_FAILED(sidx < eidx && eidx <= m_staves.size()) {
+        return;
+    }
+
+    for (size_t staffIdx = 0; staffIdx < eidx; ++staffIdx) {
         Staff* staff = m_staves[staffIdx];
+        std::vector<BracketItem*> brackets = staff->brackets(); // create copy because it's modified during loop
         for (BracketItem* bi : staff->brackets()) {
             size_t span = bi->bracketSpan();
-            if ((span == 0) || ((staffIdx + span) < sidx) || (staffIdx > eidx)) {
+            if ((span == 0) || ((staffIdx + span) <= sidx)) {
                 continue;
             }
-            if ((sidx >= staffIdx) && (eidx <= (staffIdx + span))) {
+            const bool startsOutsideDeletedRange = (staffIdx < sidx);
+            const bool endsOutsideDeletedRange = ((staffIdx + span) > eidx);
+            if (startsOutsideDeletedRange && endsOutsideDeletedRange) {
+                // Shorten the bracket by the number of staves deleted
                 bi->undoChangeProperty(Pid::BRACKET_SPAN, int(span - (eidx - sidx)));
+            } else if (startsOutsideDeletedRange) {
+                // Shorten the bracket by the number of staves deleted that were spanned by it
+                bi->undoChangeProperty(Pid::BRACKET_SPAN, int(staffIdx - sidx));
+            } else if (endsOutsideDeletedRange) {
+                if (eidx < m_staves.size()) {
+                    // Move the bracket past the end of the deleted range,
+                    // and shorten it by the number of staves deleted that were spanned by it.
+                    // That is, add a new bracket; the old one will be removed when removing the staves.
+
+                    undoAddBracket(m_staves.at(eidx), bi->column(), bi->bracketType(), int(span - (eidx - staffIdx)));
+                }
             }
         }
     }

diff --git a/src/notation/internal/notationparts.cpp b/src/notation/internal/notationparts.cpp
@@ -200,14 +200,32 @@ std::vector<Staff*> NotationParts::staves(const IDList& stavesIds) const
     }
 
     for (Staff* staff : score()->staves()) {
-        if (std::find(stavesIds.cbegin(), stavesIds.cend(), staff->id()) != stavesIds.cend()) {
+        if (muse::contains(stavesIds, staff->id())) {
             staves.push_back(staff);
         }
     }
 
     return staves;
 }
 
+std::vector<staff_idx_t> NotationParts::staffIndices(const muse::IDList& stavesIds) const
+{
+    std::vector<staff_idx_t> staffIndices;
+
+    if (stavesIds.empty()) {
+        return staffIndices;
+    }
+
+    const auto& staves = score()->staves();
+    for (staff_idx_t staffIdx = 0; staffIdx < staves.size(); ++staffIdx) {
+        if (muse::contains(stavesIds, staves.at(staffIdx)->id())) {
+            staffIndices.push_back(staffIdx);
+        }
+    }
+
+    return staffIndices;
+}
+
 std::vector<Part*> NotationParts::parts(const IDList& partsIds) const
 {
     std::vector<Part*> parts;
@@ -797,44 +815,21 @@ void NotationParts::removeStaves(const IDList& stavesIds)
 {
     TRACEFUNC;
 
-    std::vector<Staff*> stavesToRemove = staves(stavesIds);
-    if (stavesToRemove.empty()) {
-        return;
-    }
+    std::vector<staff_idx_t> staffIndicesToRemove = staffIndices(stavesIds);
 
     endInteractionWithScore();
     startEdit();
 
-    for (Staff* staff: stavesToRemove) {
-        class BracketData
-        {
-        public:
-            size_t column;
-            size_t span;
-
-            BracketData(size_t c, size_t s)
-                : column(c), span(s) {}
-        };
-
-        std::vector<BracketData> newBrackets;
-        staff_idx_t staffIdx = score()->staffIdx(staff);
-        for (BracketItem* bi : staff->brackets()) {
-            if ((bi->bracketType() == BracketType::BRACE) && (bi->bracketSpan() > 1)) {
-                newBrackets.push_back(BracketData(bi->column(), bi->bracketSpan() - 1));
-            }
-        }
-        score()->cmdRemoveStaff(staff->idx());
-        for (BracketData bd : newBrackets) {
-            score()->undoAddBracket(score()->staff(staffIdx), bd.column, BracketType::BRACE, bd.span);
-        }
+    for (staff_idx_t staffIdx: staffIndicesToRemove) {
+        score()->cmdRemoveStaff(staffIdx);
     }
 
     setBracketsAndBarlines();
 
     apply();
 
-    for (const Staff* staff : stavesToRemove) {
-        notifyAboutStaffRemoved(staff);
+    for (staff_idx_t staffIdx: staffIndicesToRemove) {
+        notifyAboutStaffRemoved(score()->staff(staffIdx));
     }
 }
 

diff --git a/src/notation/internal/notationparts.h b/src/notation/internal/notationparts.h
@@ -101,6 +101,7 @@ class NotationParts : public INotationParts, public muse::async::Asyncable
     Staff* staffModifiable(const muse::ID& staffId) const;
 
     std::vector<Staff*> staves(const muse::IDList& stavesIds) const;
+    std::vector<engraving::staff_idx_t> staffIndices(const muse::IDList& stavesIds) const;
     std::vector<Part*> parts(const muse::IDList& partsIds) const;
 
     mu::engraving::InstrumentChange* findInstrumentChange(const Part* part, const Fraction& tick) const;