ENG-34: Fix erroneous space created for melismas

Previously, melismas were treated the same as normal Lyrics in terms of
horizontal spacing—they would cause a spacer to be added to the
ChordRest to which they were attached, causing an unnecessary gap after
the first note of a melisma.

This commit prevents a melisma from adding a right spacer to
their first ChordRest, and rather creates a right
spacer on the last ChordRest of a lyric if necessary.

In pursuit of this, this commit refactors ChordRest::_melismaEnd(s) from
a bool to a std::set<*Lyrics>, giving a ChordRest a way to access the
melismatic Lyrics that end on it. This change also solves the problem of
said bool being untrustworthy (i.e. erroneously being changged to false
when one of multiple melismas was removed).

libmscore/chordrest.cpp

@@ -64,7 +64,6 @@ ChordRest::ChordRest(Score* s)
       _up          = true;
       _beamMode    = Beam::Mode::AUTO;
       _small       = false;
-      _melismaEnd  = false;
       _crossMeasure = CrossMeasure::UNKNOWN;
       }
 
@@ -80,7 +79,7 @@ ChordRest::ChordRest(const ChordRest& cr, bool link)
       _beamMode     = cr._beamMode;
       _up           = cr._up;
       _small        = cr._small;
-      _melismaEnd   = cr._melismaEnd;
+      _melismaEnds   = cr._melismaEnds;
       _crossMeasure = cr._crossMeasure;
 
       for (Lyrics* l : cr._lyrics) {        // make deep copy
@@ -1232,20 +1231,21 @@ QString ChordRest::accessibleExtraInfo() const
 
 bool ChordRest::isMelismaEnd() const
       {
-      return _melismaEnd;
+      return !_melismaEnds.empty();
       }
 
 //---------------------------------------------------------
-//   setMelismaEnd
+//   removeMelismaEnd()
+//    removes a Lyrics object from the _melismaEnds set
+//    if present by iterating through the set.
+//    Returns a boolean representing whether the Lyrics
+//    was found.
+//    No-op and return false if Lyrics not present in _melismaEnds.
 //---------------------------------------------------------
 
-void ChordRest::setMelismaEnd(bool v)
+void ChordRest::removeMelismaEnd(Lyrics* const l)
       {
-      _melismaEnd = v;
-      // TODO: don't take "false" at face value
-      // check to see if some other melisma ends here,
-      // in which case we can leave this set to true
-      // for now, rely on the fact that we'll generate the value correctly on layout
+      _melismaEnds.erase(l);
       }
 
 //---------------------------------------------------------
@@ -1256,6 +1256,8 @@ Shape ChordRest::shape() const
       {
       Shape shape;
       {
+      // Add horizontal spacing for lyrics
+
       qreal x1 = 1000000.0;
       qreal x2 = -1000000.0;
       bool adjustWidth = false;
@@ -1265,11 +1267,14 @@ Shape ChordRest::shape() const
             qreal lmargin = score()->styleS(Sid::lyricsMinDistance).val() * spatium() * 0.5;
             qreal rmargin = lmargin;
             Lyrics::Syllabic syl = l->syllabic();
-            if ((syl == Lyrics::Syllabic::BEGIN || syl == Lyrics::Syllabic::MIDDLE) && score()->styleB(Sid::lyricsDashForce))
+            bool hasHyphen = syl == Lyrics::Syllabic::BEGIN || syl == Lyrics::Syllabic::MIDDLE;
+            if (hasHyphen && score()->styleB(Sid::lyricsDashForce))
                   rmargin = qMax(rmargin, styleP(Sid::lyricsDashMinLength));
-            // for horizontal spacing we only need the lyrics width:
+            // for horizontal spacing we only need the lyrics' width:
             x1 = qMin(x1, l->bbox().x() - lmargin + l->pos().x());
-            x2 = qMax(x2, l->bbox().x() + l->bbox().width() + rmargin + l->pos().x());
+            // Melismas (without hyphens) only create right spacing on their last note
+            if (!l->isMelisma() || hasHyphen)
+                  x2 = qMax(x2, l->bbox().x() + l->bbox().width() + rmargin + l->pos().x());
             if (l->ticks() == Fraction::fromTicks(Lyrics::TEMP_MELISMA_TICKS))
                   x2 += spatium();
             adjustWidth = true;
@@ -1279,6 +1284,8 @@ Shape ChordRest::shape() const
       }
 
       {
+      // Add horizontal spacing for annotations
+
       qreal x1 = 1000000.0;
       qreal x2 = -1000000.0;
       bool adjustWidth = false;
@@ -1300,8 +1307,24 @@ Shape ChordRest::shape() const
       }
 
       if (isMelismaEnd()) {
+            // Add horizontal spacing (only on the right) for melismaEnds
+            // in case a melisma syllable extends beyond its last note.
+            // TODO: distribute this extra spacing rather than dumping
+            // it all on the last note (or otherwise overhaul horizontal spacing).
+            // TODO: this doesn't redraw on layout reset (like line breaks), 
+            // causing temporary incorrect rendering.
+
+            qreal rmargin = score()->styleS(Sid::lyricsMinDistance).val() * spatium() * 0.5;
             qreal right = rightEdge();
-            shape.addHorizontalSpacing(Shape::SPACING_LYRICS, right, right);
+
+            for (Lyrics* me : melismaEnds()) {
+                  if (me->measure()->system() != measure()->system()) // ignore cross-system melisma
+                        continue;
+                  // Lyric's right edge relative to Chord by way of page position
+                  qreal meRight = me->pageBoundingRect().right() + rmargin - pagePos().x();
+                  right = qMax(right, meRight);
+                  }
+            shape.addHorizontalSpacing(Shape::SPACING_LYRICS, 0, right);
             }
 
       return shape;

libmscore/chordrest.h

@@ -61,7 +61,7 @@ class ChordRest : public DurationElement {
       Beam::Mode _beamMode;
       bool _up;                           // actual stem direction
       bool _small;
-      bool _melismaEnd;
+      std::set<Lyrics*> _melismaEnds;     // lyrics ending on this ChordRest, used for spacing
 
       // CrossMeasure: combine 2 tied notes if across a bar line and can be combined in a single duration
       CrossMeasure _crossMeasure;         ///< 0: no cross-measure modification; 1: 1st note of a mod.; -1: 2nd note
@@ -138,8 +138,10 @@ class ChordRest : public DurationElement {
       std::vector<Lyrics*>& lyrics()             { return _lyrics; }
       Lyrics* lyrics(int verse, Placement) const;
       int lastVerse(Placement) const;
+      const std::set<Lyrics*>& melismaEnds() const { return _melismaEnds; }
+      std::set<Lyrics*>& melismaEnds()             { return _melismaEnds; }
+      void removeMelismaEnd(Lyrics* const l);
       bool isMelismaEnd() const;
-      void setMelismaEnd(bool v);
 
       virtual void add(Element*);
       virtual void remove(Element*);

libmscore/lyrics.cpp

@@ -188,7 +188,7 @@ void Lyrics::remove(Element* el)
                   if (!e)
                         e = score()->findCRinStaff(_separator->tick2(), track());
                   if (e && e->isChordRest())
-                        toChordRest(e)->setMelismaEnd(false);
+                        toChordRest(e)->removeMelismaEnd(this);
 #endif
                   // Lyrics::remove() and LyricsLine::removeUnmanaged() call each other;
                   // be sure each finds a clean context
@@ -368,7 +368,7 @@ void Lyrics::layout()
             // set melisma end
             ChordRest* ecr = score()->findCR(endTick(), track());
             if (ecr)
-                  ecr->setMelismaEnd(true);
+                  ecr->melismaEnds().insert(this);
             }
 
       }
@@ -525,7 +525,7 @@ void Lyrics::removeFromScore()
             // clear melismaEnd flag from end cr
             ChordRest* ecr = score()->findCR(endTick(), track());
             if (ecr)
-                  ecr->setMelismaEnd(false);
+                  ecr->removeMelismaEnd(this);
             }
       if (_separator) {
             _separator->removeUnmanaged();
@@ -567,16 +567,13 @@ bool Lyrics::setProperty(Pid propertyId, const QVariant& v)
                   break;
             case Pid::LYRIC_TICKS:
                   if (_ticks.isNotZero()) {
-                        // clear melismaEnd flag from previous end cr
-                        // this might be premature, as there may be other melismas ending there
-                        // but flag will be generated correctly on layout
                         // TODO: after inserting a measure,
                         // endTick info is wrong.
                         // Somehow we need to fix this.
                         // See https://musescore.org/en/node/285304 and https://musescore.org/en/node/311289
                         ChordRest* ecr = score()->findCR(endTick(), track());
                         if (ecr)
-                              ecr->setMelismaEnd(false);
+                              ecr->removeMelismaEnd(this);
                         }
                   _ticks = v.value<Fraction>();
                   break;

libmscore/lyrics.h

@@ -51,7 +51,6 @@ class Lyrics final : public TextBase {
       Syllabic _syllabic;
       LyricsLine* _separator;
 
-      bool isMelisma() const;
       void undoChangeProperty(Pid id, const QVariant&, PropertyFlags ps) override;
 
    protected:
@@ -93,6 +92,7 @@ class Lyrics final : public TextBase {
       Fraction ticks() const                          { return _ticks;    }
       void setTicks(const Fraction& tick)             { _ticks = tick;    }
       Fraction endTick() const;
+      bool isMelisma() const;
       void removeFromScore();
 
       using ScoreElement::undoChangeProperty;

libmscore/shape.cpp

@@ -28,7 +28,11 @@ void Shape::addHorizontalSpacing(HorizontalSpacingType type, qreal leftEdge, qre
       const qreal y = eps * int(type);
       if (leftEdge == rightEdge) // HACK zero-width shapes collide with everything currently.
             rightEdge += eps;
+#ifndef NDEBUG
+      add(QRectF(leftEdge, y, rightEdge - leftEdge, 0), "Horizontal Spacing");
+#else
       add(QRectF(leftEdge, y, rightEdge - leftEdge, 0));
+#endif
       }
 
 //---------------------------------------------------------