Fix various mmRest issues and some refactoring of anchor-related functions

src/engraving/dom/anchors.cpp

@@ -241,13 +241,58 @@ void MoveElementAnchors::moveElementAnchorsOnDrag(EngravingItem* element, EditDa
     Segment* newSeg = nullptr;     // don't prefer any segment while dragging, just snap to the closest
     static constexpr double spacingFactor = 0.5;
     element->score()->dragPosition(element->canvasPos(), &si, &newSeg, spacingFactor, element->allowTimeAnchor());
-    if (newSeg && (newSeg != segment || element->staffIdx() != si)) {
+    if (newSeg && ((newSeg != segment && !newSeg->measure()->isMMRest()) || element->staffIdx() != si)) {
         PointF curOffset = element->offset();
         moveSegment(element, newSeg, newSeg->tick() - segment->tick());
         rebaseOffsetOnMoveSegment(element, curOffset, newSeg, segment);
     }
 }
 
+Segment* MoveElementAnchors::findNewAnchorSegmentForLine(LineSegment* lineSegment, const EditData& ed, const Segment* curSeg)
+{
+    SLine* line = lineSegment->line();
+    Score* score = lineSegment->score();
+    if (!line->allowTimeAnchor()) {
+        if (ed.key == Key_Left) {
+            return curSeg->prev1WithElemsOnStaff(lineSegment->staffIdx());
+        }
+        if (ed.key == Key_Right) {
+            Segment* lastCRSegInScore = score->lastSegment();
+            while (lastCRSegInScore && !lastCRSegInScore->isChordRestType()) {
+                lastCRSegInScore = lastCRSegInScore->prev1(SegmentType::ChordRest);
+            }
+            if (curSeg == lastCRSegInScore && curSeg == line->endSegment()) {
+                // If we reach this point, it means that the line in question does not accept time anchors
+                // and that the line's end segment is the last CR segment in the score. Trying to use
+                // next1WithElemsOnStaff won't do anything from here, but the last segment in the score is
+                // still a valid new anchor segment for this line (see also LineSegment::edit)...
+                return score->lastSegment();
+            }
+            return curSeg->next1WithElemsOnStaff(lineSegment->staffIdx());
+        }
+    }
+
+    if (ed.modifiers & ControlModifier) {
+        if (ed.key == Key_Left) {
+            Measure* measure = curSeg->rtick().isZero() ? curSeg->measure()->prevMeasure() : curSeg->measure();
+            return measure ? measure->findFirstR(SegmentType::ChordRest, Fraction(0, 1)) : nullptr;
+        }
+        if (ed.key == Key_Right) {
+            Measure* measure = curSeg->measure()->nextMeasure();
+            return measure ? measure->findFirstR(SegmentType::ChordRest, Fraction(0, 1)) : nullptr;
+        }
+    }
+
+    if (ed.key == Key_Left) {
+        return findNewAnchorableSegment(curSeg, /*forward*/ false);
+    }
+    if (ed.key == Key_Right) {
+        return findNewAnchorableSegment(curSeg, /*forward*/ true);
+    }
+
+    return nullptr;
+}
+
 void MoveElementAnchors::moveSegment(EngravingItem* element, bool forward)
 {
     Segment* curSeg = toSegment(element->parentItem());
@@ -268,8 +313,31 @@ Segment* MoveElementAnchors::getNewSegment(EngravingItem* element, Segment* curS
         return newMeasure ? newMeasure->first(SegmentType::ChordRest) : nullptr;
     }
     default:
-        return forward ? curSeg->next1ChordRestOrTimeTick() : curSeg->prev1ChordRestOrTimeTick();
+        return findNewAnchorableSegment(curSeg, forward);
+    }
+}
+
+Segment* MoveElementAnchors::findNewAnchorableSegment(const Segment* curSeg, bool forward)
+{
+    Segment* newSeg = forward ? curSeg->next1(Segment::CHORD_REST_OR_TIME_TICK_TYPE) : curSeg->prev1(Segment::CHORD_REST_OR_TIME_TICK_TYPE);
+
+    // Continue until we get to a different tick than where we are
+    while (newSeg && newSeg->tick() == curSeg->tick()) {
+        newSeg = forward ? newSeg->next1(Segment::CHORD_REST_OR_TIME_TICK_TYPE) : newSeg->prev1(Segment::CHORD_REST_OR_TIME_TICK_TYPE);
+    }
+    if (!newSeg) {
+        return nullptr;
+    }
+
+    // Always prefer ChordRest segment if one exists at the same tick
+    if (newSeg->segmentType() != SegmentType::ChordRest) {
+        Segment* newChordRestSeg = forward ? newSeg->next1(SegmentType::ChordRest) : newSeg->prev1(SegmentType::ChordRest);
+        if (newChordRestSeg && newChordRestSeg->tick() == newSeg->tick()) {
+            newSeg = newChordRestSeg;
+        }
     }
+
+    return newSeg;
 }
 
 void MoveElementAnchors::moveSegment(EngravingItem* element, Segment* newSeg, Fraction tickDiff)
@@ -301,14 +369,22 @@ void MoveElementAnchors::doMoveSegment(EngravingItem* element, Segment* newSeg,
     // change approach and *move* elements onto the mmRests, not clone them. [M.S.]
 
     Score* score = element->score();
+    Measure* parentMeasure = toMeasure(element->findAncestor(ElementType::MEASURE));
+    bool parentMeasureIsMMRest = parentMeasure && parentMeasure->isMMRest();
+
     std::list<EngravingObject*> linkedElements = element->linkList();
     for (EngravingObject* linkedElement : linkedElements) {
         if (linkedElement == element) {
             continue;
         }
-        Segment* curParent = toSegment(linkedElement->parent());
-        bool isOnMMRest = curParent->parent() && toMeasure(curParent->parent())->isMMRest();
-        if (isOnMMRest) {
+        Measure* linkedParentMeasure = toMeasure(toEngravingItem(linkedElement)->findAncestor(ElementType::MEASURE));
+        bool linkedParentMeasureIsMMrest = linkedParentMeasure && linkedParentMeasure->isMMRest();
+        if (linkedParentMeasureIsMMrest != parentMeasureIsMMRest) {
+            for (EngravingObject* child : linkedElement->children()) {
+                // e.g. fret diagram should also remove the harmony
+                child->undoUnlink();
+                score->undoRemoveElement(toEngravingItem(child));
+            }
             linkedElement->undoUnlink();
             score->undoRemoveElement(static_cast<EngravingItem*>(linkedElement));
         }

src/engraving/dom/anchors.h

@@ -46,10 +46,13 @@ class MoveElementAnchors
 
     static void moveElementAnchorsOnDrag(EngravingItem* element, EditData& ed);
 
+    static Segment* findNewAnchorSegmentForLine(LineSegment* lineSegment, const EditData& ed, const Segment* curSeg);
+
 private:
     static bool canAnchorToEndOfPrevious(const EngravingItem* element);
     static void moveSegment(EngravingItem* element, bool forward);
     static Segment* getNewSegment(EngravingItem* element, Segment* curSeg, bool forward);
+    static Segment* findNewAnchorableSegment(const Segment* curSeg, bool forward);
 
     static void doMoveSegment(EngravingItem* element, Segment* newSeg, Fraction tickDiff);
     static void doMoveSegment(FiguredBass* element, Segment* newSeg, Fraction tickDiff);

src/engraving/dom/engravingitem.cpp

@@ -1326,7 +1326,7 @@ void EngravingItem::setPlacementBasedOnVoiceAssignment(DirectionV styledDirectio
                 if (segment && segment->isTimeTickType() && segment->measure() != measure) {
                     // Edge case: this is a TimeTick segment at the end of previous measure. Happens only
                     // when dynamic is anchorToEndOfPrevious. In this case look for preceding segment.
-                    segment = segment->prev1ChordRestOrTimeTick();
+                    segment = segment->prev1(Segment::CHORD_REST_OR_TIME_TICK_TYPE);
                     assert(segment);
                     measure = segment->measure();
                 }

src/engraving/dom/line.cpp

@@ -256,9 +256,9 @@ bool LineSegment::edit(EditData& ed)
             return true;
         }
         if (moveStart) {
-            s1 = findNewAnchorSegment(ed, s1);
+            s1 = MoveElementAnchors::findNewAnchorSegmentForLine(this, ed, s1);
         } else {
-            s2 = findNewAnchorSegment(ed, s2);
+            s2 = MoveElementAnchors::findNewAnchorSegmentForLine(this, ed, s2);
         }
         if (s1 == 0 || s2 == 0 || s1->tick() >= s2->tick()) {
             return true;
@@ -399,49 +399,6 @@ bool LineSegment::edit(EditData& ed)
     return true;
 }
 
-Segment* LineSegment::findNewAnchorSegment(const EditData& ed, const Segment* curSeg)
-{
-    if (!line()->allowTimeAnchor()) {
-        if (ed.key == Key_Left) {
-            return curSeg->prev1WithElemsOnStaff(staffIdx());
-        }
-        if (ed.key == Key_Right) {
-            Segment* lastCRSegInScore = score()->lastSegment();
-            while (lastCRSegInScore && !lastCRSegInScore->isChordRestType()) {
-                lastCRSegInScore = lastCRSegInScore->prev1(SegmentType::ChordRest);
-            }
-            if (curSeg == lastCRSegInScore && curSeg == line()->endSegment()) {
-                // If we reach this point, it means that the line in question does not accept time anchors
-                // and that the line's end segment is the last CR segment in the score. Trying to use
-                // next1WithElemsOnStaff won't do anything from here, but the last segment in the score is
-                // still a valid new anchor segment for this line (see also LineSegment::edit)...
-                return score()->lastSegment();
-            }
-            return curSeg->next1WithElemsOnStaff(staffIdx());
-        }
-    }
-
-    if (ed.modifiers & ControlModifier) {
-        if (ed.key == Key_Left) {
-            Measure* measure = curSeg->rtick().isZero() ? curSeg->measure()->prevMeasure() : curSeg->measure();
-            return measure ? measure->findFirstR(SegmentType::ChordRest, Fraction(0, 1)) : nullptr;
-        }
-        if (ed.key == Key_Right) {
-            Measure* measure = curSeg->measure()->nextMeasure();
-            return measure ? measure->findFirstR(SegmentType::ChordRest, Fraction(0, 1)) : nullptr;
-        }
-    }
-
-    if (ed.key == Key_Left) {
-        return curSeg->prev1ChordRestOrTimeTick();
-    }
-    if (ed.key == Key_Right) {
-        return curSeg->next1ChordRestOrTimeTick();
-    }
-
-    return nullptr;
-}
-
 //---------------------------------------------------------
 //   findSegmentForGrip
 //---------------------------------------------------------

src/engraving/dom/line.h

@@ -81,7 +81,6 @@ class LineSegment : public SpannerSegment
     virtual void rebaseOffsetsOnAnchorChanged(Grip grip, const PointF& oldPos, System* sys);
 
 private:
-    Segment* findNewAnchorSegment(const EditData& ed, const Segment* curSeg);
     void undoMoveStartEndAndSnappedItems(bool moveStart, bool moveEnd, Segment* s1, Segment* s2);
     PointF leftAnchorPosition(const double& systemPositionY) const;
     PointF rightAnchorPosition(const double& systemPositionY) const;

src/engraving/dom/linkedobjects.cpp

@@ -92,7 +92,15 @@ EngravingObject* LinkedObjects::mainElement()
 
                     // MM rests may be generated but not written (e.g. if
                     // saving a file right after disabling MM rests)
-                    const bool mmRestsWritten = e1->style().styleB(Sid::createMultiMeasureRests);
+                    bool mmRestsWritten = e1->style().styleB(Sid::createMultiMeasureRests);
+                    if (m1->tick() == m2->tick()) {
+                        // MM rests may be still enabled but the mmRest may have been removed by editing
+                        // NOTE: the fact that we have linked clones between the original measure and the MMRest
+                        // and we have to decide which one is the "main" one is really bad tech debt. We should not
+                        // use linked clones for that. Needs fix in future. [M.S.]
+                        bool oneIsMMRestCoveringOther = m1->mmRest() == m2 || m2->mmRest() == m1;
+                        mmRestsWritten &= oneIsMMRestCoveringOther;
+                    }
 
                     if (m1->isMMRest()) {
                         // m1 is earlier if m2 is *not* the first MM rest measure

src/engraving/dom/score.cpp

@@ -1132,7 +1132,7 @@ Measure* Score::searchMeasure(const PointF& p, const System* preferredSystem, do
     for (System* system : systems) {
         double x = p.x() - system->canvasPos().x();
         for (MeasureBase* mb : system->measures()) {
-            if (mb->isMeasure() && !toMeasure(mb)->isMMRest() && (x < (mb->x() + mb->ldata()->bbox().width()))) {
+            if (mb->isMeasure() && (x < (mb->x() + mb->ldata()->bbox().width()))) {
                 return toMeasure(mb);
             }
         }

src/engraving/dom/segment.cpp

@@ -324,28 +324,6 @@ Segment* Segment::next1(SegmentType types) const
     return 0;
 }
 
-Segment* Segment::next1ChordRestOrTimeTick() const
-{
-    Segment* nextSeg = next1(CHORD_REST_OR_TIME_TICK_TYPE);
-    while (nextSeg && nextSeg->tick() == tick()) {
-        nextSeg = nextSeg->next1(CHORD_REST_OR_TIME_TICK_TYPE);
-    }
-    if (!nextSeg) {
-        return nullptr;
-    }
-
-    Segment* nextNextSeg = nextSeg->next1(CHORD_REST_OR_TIME_TICK_TYPE);
-    if (!nextNextSeg) {
-        return nextSeg;
-    }
-
-    if (nextSeg->tick() == nextNextSeg->tick()) {
-        return nextSeg->isChordRestType() ? nextSeg : nextNextSeg;
-    }
-
-    return nextSeg;
-}
-
 Segment* Segment::next1WithElemsOnStaff(staff_idx_t staffIdx, SegmentType segType) const
 {
     Segment* next = next1(segType);
@@ -461,28 +439,6 @@ Segment* Segment::prev1() const
     return m ? m->last() : 0;
 }
 
-Segment* Segment::prev1ChordRestOrTimeTick() const
-{
-    Segment* prevSeg = prev1(CHORD_REST_OR_TIME_TICK_TYPE);
-    while (prevSeg && prevSeg->tick() == tick()) {
-        prevSeg = prevSeg->prev1(CHORD_REST_OR_TIME_TICK_TYPE);
-    }
-    if (!prevSeg) {
-        return nullptr;
-    }
-
-    Segment* prevPrevSeg = prevSeg->prev1(CHORD_REST_OR_TIME_TICK_TYPE);
-    if (!prevPrevSeg) {
-        return prevSeg;
-    }
-
-    if (prevSeg->tick() == prevPrevSeg->tick()) {
-        return prevSeg->isChordRestType() ? prevSeg : prevPrevSeg;
-    }
-
-    return prevSeg;
-}
-
 Segment* Segment::prev1WithElemsOnStaff(staff_idx_t staffIdx, SegmentType segType) const
 {
     Segment* prev = prev1(segType);

src/engraving/dom/segment.h

@@ -158,13 +158,11 @@ class Segment final : public EngravingItem
     Segment* next1MM() const;
     Segment* next1MMenabled() const;
     Segment* next1(SegmentType) const;
-    Segment* next1ChordRestOrTimeTick() const;
     Segment* next1WithElemsOnStaff(staff_idx_t staffIdx, SegmentType segType = SegmentType::ChordRest) const;
     Segment* next1WithElemsOnTrack(track_idx_t trackIdx, SegmentType segType = SegmentType::ChordRest) const;
     Segment* next1MM(SegmentType) const;
 
     Segment* prev1() const;
-    Segment* prev1ChordRestOrTimeTick() const;
     Segment* prev1WithElemsOnStaff(staff_idx_t staffIdx, SegmentType segType = SegmentType::ChordRest) const;
     Segment* prev1enabled() const;
     Segment* prev1MM() const;

src/engraving/rendering/score/measurelayout.cpp

@@ -887,14 +887,6 @@ void MeasureLayout::removeMMRestElements(Measure* mmRestMeasure)
     }
 
     for (Segment* seg = mmRestMeasure->first(); seg && seg->rtick().isZero(); seg = seg->next()) {
-        if (!seg->isChordRestType()) {
-            for (EngravingItem* item : seg->elist()) {
-                if (item) {
-                    item->undoUnlink();
-                    mmRestMeasure->score()->doUndoRemoveElement(item);
-                }
-            }
-        }
         for (EngravingItem* item : seg->annotations()) {
             item->undoUnlink();
             mmRestMeasure->score()->doUndoRemoveElement(item);