Allow melismas across rests and fix lyrics crashes

src/engraving/dom/lyrics.cpp

@@ -299,7 +299,7 @@ bool Lyrics::isEditAllowed(EditData& ed) const
 void Lyrics::adjustPrevious()
 {
     Lyrics* prev = Navigation::prevLyrics(toLyrics(this));
-    if (prev) {
+    if (prev && prev->ticks().isNotZero()) {
         // search for lyric spanners to split at this point if necessary
         if (prev->tick() + prev->ticks() >= tick()) {
             // the previous lyric has a spanner attached that goes through this one
@@ -312,10 +312,10 @@ void Lyrics::adjustPrevious()
                 } else {
                     prev->undoChangeProperty(Pid::LYRIC_TICKS, Fraction::eps());
                 }
-                prev->setNeedRemoveInvalidSegments();
-                prev->triggerLayout();
             }
         }
+        prev->setNeedRemoveInvalidSegments();
+        prev->triggerLayout();
     }
 }
 

src/engraving/dom/repeatlist.cpp

@@ -526,9 +526,10 @@ void RepeatList::collectRepeatListElements()
         if (mb->isMeasure()) {
             Measure* m = toMeasure(mb);
             sectionEndMeasureBase = mb; // ending measure of section is the most recently encountered actual Measure
+            Measure* underlyingMeasure = m->isMMRest() ? m->mmRestFirst() : m;
 
             // Volta ?
-            if ((!preProcessedVoltas.empty()) && (preProcessedVoltas.front()->startMeasure() == m)) {
+            if ((!preProcessedVoltas.empty()) && (preProcessedVoltas.front()->startMeasure() == underlyingMeasure)) {
                 if (volta != nullptr) {
                     //if (volta->endMeasure()->tick() < m->tick()) {
                     // The previous volta was supposed to end before us (open volta case) -> insert the end
@@ -546,7 +547,7 @@ void RepeatList::collectRepeatListElements()
             // Start
             if (m->repeatStart()) {
                 if (volta != nullptr) {
-                    if (volta->startMeasure() != m) {
+                    if (volta->startMeasure() != underlyingMeasure) {
                         // Volta and Start repeat are not on the same measure
                         // assume the previous volta was supposed to end before us (open volta case) -> insert the end
                         // Warning: This might "break" a volta prematurely if its explicit notated end is later than this point

src/engraving/dom/score.h

@@ -462,7 +462,7 @@ class Score : public EngravingObject, public muse::Contextable
     void cmdFullMeasureRest();
 
     void startCmd(const TranslatableString& actionName);             // start undoable command
-    void endCmd(bool rollback = false, bool layoutAllParts = false); // end undoable command
+    void endCmd(bool rollback = false, bool layoutAllParts = false, bool keepRolledBackElements = false); // end undoable command
     void update();
     void undoRedo(bool undo, EditData*);
 

src/engraving/dom/utils.cpp

@@ -1506,7 +1506,7 @@ std::vector<Measure*> findPreviousRepeatMeasures(const Measure* measure)
     const MasterScore* master = measure->masterScore();
     const Score* score = measure->score();
 
-    const Measure* masterMeasure = master->tick2measure(measure->tick());
+    const Measure* masterMeasure = master->tick2measureMM(measure->tick());
 
     const RepeatList& repeatList = master->repeatList(true, false);
 
@@ -1526,7 +1526,7 @@ std::vector<Measure*> findPreviousRepeatMeasures(const Measure* measure)
         // Get next segment
         const RepeatSegment* prevSeg = *prevSegIt;
         const Measure* lastMasterMeasure = prevSeg->lastMeasure();
-        Measure* lastMeasure = lastMasterMeasure ? score->tick2measure(lastMasterMeasure->tick()) : nullptr;
+        Measure* lastMeasure = lastMasterMeasure ? score->tick2measureMM(lastMasterMeasure->tick()) : nullptr;
         if (!lastMeasure) {
             continue;
         }
@@ -1572,6 +1572,13 @@ bool segmentsAreAdjacent(const Segment* firstSeg, const Segment* secondSeg)
     const Measure* firstMasterMeasure = master->tick2measure(firstMeasure->tick());
     const Measure* secondMasterMeasure = master->tick2measure(secondMeasure->tick());
 
+    if (firstMasterMeasure) {
+        firstMasterMeasure = firstMasterMeasure->coveringMMRestOrThis();
+    }
+    if (secondMasterMeasure) {
+        secondMasterMeasure = secondMasterMeasure->coveringMMRestOrThis();
+    }
+
     Score* score = firstSeg->score();
 
     const RepeatList& repeatList = score->repeatList(true, false);

src/engraving/editing/cmd.cpp

@@ -316,9 +316,9 @@ void Score::undoRedo(bool undo, EditData* ed)
 ///   and (always) updating the redraw area.
 //---------------------------------------------------------
 
-void Score::endCmd(bool rollback, bool layoutAllParts)
+void Score::endCmd(bool rollback, bool layoutAllParts, bool keepRolledBackElements)
 {
-    masterScore()->transactionManager()->endTransaction(rollback, layoutAllParts);
+    masterScore()->transactionManager()->endTransaction(rollback, layoutAllParts, keepRolledBackElements);
 }
 
 //---------------------------------------------------------

src/engraving/editing/textedit.cpp

@@ -160,14 +160,15 @@ void TextBase::endEdit(EditData& ed)
         }
     }
 
-    const bool newlyAdded = ted->oldXmlText.isEmpty();
+    const bool textStartedEmpty = ted->oldXmlText.isEmpty();
+    const UndoableTransaction* addTransaction = !textStartedEmpty ? nullptr
+                                                : (textWasEdited ? undo->prev() : undo->last());
+    const bool newlyAdded = addTransaction
+                            && addTransaction->hasCommandsMatchingFilter(UndoableCommandFilter::AddElement, this);
     if (newlyAdded) {
-        const UndoableTransaction* transaction = textWasEdited ? undo->prev() : undo->last();
-        if (transaction && transaction->hasCommandsMatchingFilter(UndoableCommandFilter::AddElement, this)) {
-            // We have just added this element to a score.
-            // Combine undo records of text creation with text editing.
-            undo->mergeTransactions(ted->startUndoIdx - 1);
-        }
+        // We have just added this element to a score.
+        // Combine undo records of text creation with text editing.
+        undo->mergeTransactions(ted->startUndoIdx - 1);
     }
 
     // TBox'es manage their Text themselves and are not removed if text is empty
@@ -176,16 +177,23 @@ void TextBase::endEdit(EditData& ed)
     if (actualPlainText.isEmpty() && removeTextIfEmpty) {
         LOGD("actual text is empty");
 
-        // If this assertion fails, no undo command relevant to this text
-        // resides on undo stack and reopen() would corrupt the previous
-        // command. Text shouldn't happen to be empty in other cases though.
-        assert(newlyAdded || textWasEdited);
-
-        undo->reopen();
         if (newlyAdded) {
-            score()->endCmd(true); // rollback the "add element" command
+            undo->reopen();
+            // Rollback the "AddElement" command, but don't delete the rolled-back element(s):
+            score()->endCmd(true, false, /*keepRolledBackElements*/ true);
+            // Defer deletion (to ~TextEditData):
             ted->deleteText = true;
         } else {
+            if (textWasEdited) {
+                // The text was just edited down to empty: the top transaction is this element's own edit:
+                undo->reopen();
+            } else {
+                /* The text was already empty before editing began (e.g. an empty text from a
+                 * legacy/corrupt file). No undo command relevant to this text resides on undo
+                 * stack and reopen() would corrupt the previous command: */
+                score()->startCmd(TranslatableString("undoableAction", "Remove empty text"));
+            }
+
             score()->undoRemoveElement(this);
 
             if (isLyrics()) {

src/engraving/editing/transaction/transaction.cpp

@@ -148,7 +148,7 @@ void TransactionManager::beginTransaction(const muse::TranslatableString& descri
     m_currentTransaction = std::unique_ptr<Transaction>(new Transaction(stack->activeTransaction()));
 }
 
-void TransactionManager::endTransaction(bool rollback, bool layoutAllParts)
+void TransactionManager::endTransaction(bool rollback, bool layoutAllParts, bool keepRolledBackElements)
 {
     UndoStack* stack = undoStack();
 
@@ -167,7 +167,10 @@ void TransactionManager::endTransaction(bool rollback, bool layoutAllParts)
     }
 
     if (rollback) {
-        stack->activeTransaction()->unwind();
+        /* When keepRolledBackElements is true, unwind removes the rolled-back elements from
+         * the score but does not free them: an external owner is responsible for the deferred
+         * deletion, since the elements are still referenced after this returns. */
+        stack->activeTransaction()->unwind(!keepRolledBackElements);
     }
 
     // NOTE: perform update and re-layout within the transaction, so that

src/engraving/editing/transaction/transaction.h

@@ -133,7 +133,7 @@ class TransactionManager
     void transaction(const muse::TranslatableString& description, std::function<void(Transaction&)> func);
 
     void beginTransaction(const muse::TranslatableString& description);
-    void endTransaction(bool rollback = false, bool layoutAllParts = false);
+    void endTransaction(bool rollback = false, bool layoutAllParts = false, bool keepRolledBackElements = false);
 
     /// Returns the current transaction, if any is active.
     /// Where possible, prefer using `transaction()` instead of this method.

src/engraving/editing/transaction/undostack.cpp

@@ -260,13 +260,15 @@ void UndoableTransaction::cleanup(bool wasDone)
     }
 }
 
-void UndoableTransaction::unwind()
+void UndoableTransaction::unwind(bool cleanUp)
 {
     while (!m_commands.empty()) {
         UndoableCommand* command = muse::takeLast(m_commands);
         LOG_UNDO() << "unwind: " << command->name();
         command->undo();
-        command->cleanup(false);
+        if (cleanUp) {
+            command->cleanup(false);
+        }
         delete command;
     }
 }

src/engraving/editing/transaction/undostack.h

@@ -52,7 +52,7 @@ class UndoableTransaction
     void appendCommand(UndoableCommand* cmd) { m_commands.push_back(cmd); }
     void append(UndoableTransaction&& other);
 
-    void unwind();
+    void unwind(bool cleanUp = true);
     void cleanup(bool wasDone);
 
     const std::vector<UndoableCommand*>& commands() const { return m_commands; }

src/engraving/rendering/score/lyricslayout.cpp

@@ -120,8 +120,14 @@ void LyricsLayout::layout(Lyrics* item, LayoutContext& ctx)
     ChordRest* cr = item->chordRest();
     double x = -cr->x();
 
+    /* Preserve the vertical position: a lyric's Y is set in computeVerticalPositions(). Necessary
+     * because layoutBaseTextBase1() ends with ldata->move(defaultPos), which accumulates a downward
+     * shift of the lyric if the current function is called in a partially-relaid system without a
+     * following computeVerticalPositions() call. */
+    const double posY = ldata->pos().y();
     TextLayout::layoutBaseTextBase1(item, ctx);
     TextLayout::computeTextHighResShape(item, ldata);
+    ldata->setPosY(posY);
 
     double centerAdjust = 0.0;
     double leftAdjust   = 0.0;

src/notation/internal/notationinteraction.cpp

@@ -7977,11 +7977,6 @@ void NotationInteraction::addMelisma()
             }
         }
         segment = segment->prev1(SegmentType::ChordRest);
-        // if the segment has a rest in this track, stop going back
-        EngravingItem* e = segment ? segment->element(track) : 0;
-        if (e && !e->isChord()) {
-            break;
-        }
     }
 
     // one-chord melisma?