Skip to content

Commit

Permalink
Revert "ENG-34: Fix erroneous space created for melismas"
Browse files Browse the repository at this point in the history 
This reverts the 1st part of PR musescore#8296, which is causing a regression.
  • Loading branch information
@Jojo-Schmitz
Jojo-Schmitz committed Jan 17, 2022
1 parent b8c5638 commit c06b240
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 731 deletions.
51 changes: 14 additions & 37 deletions libmscore/chordrest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ ChordRest::ChordRest(Score* s)
_up = true;
_beamMode = Beam::Mode::AUTO;
m_isSmall = false;
_melismaEnd = false;
_crossMeasure = CrossMeasure::UNKNOWN;
}

Expand All @@ -79,7 +80,7 @@ ChordRest::ChordRest(const ChordRest& cr, bool link)
_beamMode = cr._beamMode;
_up = cr._up;
m_isSmall = cr.m_isSmall;
_melismaEnds = cr._melismaEnds;
_melismaEnd = cr._melismaEnd;
_crossMeasure = cr._crossMeasure;

for (Lyrics* l : cr._lyrics) { // make deep copy
Expand Down Expand Up @@ -1231,21 +1232,20 @@ QString ChordRest::accessibleExtraInfo() const

bool ChordRest::isMelismaEnd() const
{
return !_melismaEnds.empty();
return _melismaEnd;
}

//---------------------------------------------------------
// 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.
// setMelismaEnd
//---------------------------------------------------------

void ChordRest::removeMelismaEnd(Lyrics* const l)
void ChordRest::setMelismaEnd(bool v)
{
_melismaEnds.erase(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
}

//---------------------------------------------------------
Expand All @@ -1256,8 +1256,6 @@ Shape ChordRest::shape() const
{
Shape shape;
{
// Add horizontal spacing for lyrics

qreal x1 = 1000000.0;
qreal x2 = -1000000.0;
bool adjustWidth = false;
Expand All @@ -1267,14 +1265,11 @@ Shape ChordRest::shape() const
qreal lmargin = score()->styleS(Sid::lyricsMinDistance).val() * spatium() * 0.5;
qreal rmargin = lmargin;
Lyrics::Syllabic syl = l->syllabic();
bool hasHyphen = syl == Lyrics::Syllabic::BEGIN || syl == Lyrics::Syllabic::MIDDLE;
if (hasHyphen && score()->styleB(Sid::lyricsDashForce))
if ((syl == Lyrics::Syllabic::BEGIN || syl == Lyrics::Syllabic::MIDDLE) && 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());
// 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());
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;
Expand All @@ -1284,8 +1279,6 @@ Shape ChordRest::shape() const
}

{
// Add horizontal spacing for annotations

qreal x1 = 1000000.0;
qreal x2 = -1000000.0;
bool adjustWidth = false;
Expand Down Expand Up @@ -1332,24 +1325,8 @@ 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();

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);
shape.addHorizontalSpacing(Shape::SPACING_LYRICS, right, right);
}

return shape;
Expand Down
6 changes: 2 additions & 4 deletions libmscore/chordrest.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class ChordRest : public DurationElement {
Beam::Mode _beamMode;
bool _up; // actual stem direction
bool m_isSmall;
std::set<Lyrics*> _melismaEnds; // lyrics ending on this ChordRest, used for spacing
bool _melismaEnd;

// 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
Expand Down Expand Up @@ -138,10 +138,8 @@ 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*);
Expand Down
11 changes: 7 additions & 4 deletions libmscore/lyrics.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ void Lyrics::remove(Element* el)
if (!e)
e = score()->findCRinStaff(_separator->tick2(), track());
if (e && e->isChordRest())
toChordRest(e)->removeMelismaEnd(this);
toChordRest(e)->setMelismaEnd(false);
#endif
// Lyrics::remove() and LyricsLine::removeUnmanaged() call each other;
// be sure each finds a clean context
Expand Down Expand Up @@ -368,7 +368,7 @@ void Lyrics::layout()
// set melisma end
ChordRest* ecr = score()->findCR(endTick(), track());
if (ecr)
ecr->melismaEnds().insert(this);
ecr->setMelismaEnd(true);
}

}
Expand Down Expand Up @@ -525,7 +525,7 @@ void Lyrics::removeFromScore()
// clear melismaEnd flag from end cr
ChordRest* ecr = score()->findCR(endTick(), track());
if (ecr)
ecr->removeMelismaEnd(this);
ecr->setMelismaEnd(false);
}
if (_separator) {
_separator->removeUnmanaged();
Expand Down Expand Up @@ -567,13 +567,16 @@ 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->removeMelismaEnd(this);
ecr->setMelismaEnd(false);
}
_ticks = v.value<Fraction>();
break;
Expand Down
2 changes: 1 addition & 1 deletion libmscore/lyrics.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class Lyrics final : public TextBase {
Syllabic _syllabic;
LyricsLine* _separator;

bool isMelisma() const;
void undoChangeProperty(Pid id, const QVariant&, PropertyFlags ps) override;

protected:
Expand Down Expand Up @@ -92,7 +93,6 @@ 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;
Expand Down
4 changes: 0 additions & 4 deletions libmscore/shape.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,7 @@ 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
}

//---------------------------------------------------------
Expand Down

0 comments on commit c06b240

Please sign in to comment.