Skip to content

Commit

Permalink
ENG-70: Tweak default-y handling
Browse files Browse the repository at this point in the history
Previously, default-y handling failed for directions with multiple
words in some cases. This commit fixes that by prefering the first
non-zero default-y and relative-y values and preventing later words
from overwriting that. It also simplifies the use of _hasDefaultY and
_hasRelativeY.

Duplicate of musescore#8649
  • Loading branch information
iveshenry18 authored and Jojo-Schmitz committed Aug 13, 2021
1 parent dc6ec00 commit cd77762
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 21 deletions.
32 changes: 19 additions & 13 deletions importexport/musicxml/importmxmlpass2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2767,7 +2767,7 @@ void MusicXMLParserDirection::direction(const QString& partId,
else {
// Add element to score later, after collecting all the others and sorting by default-y
// This allows default-y to be at least respected by the order of elements
MusicXMLDelayedDirectionElement* delayedDirection = new MusicXMLDelayedDirectionElement(hasTotalY() ? totalY() : 100, t, track, wordsPlacement, measure, tick + _offset);
MusicXMLDelayedDirectionElement* delayedDirection = new MusicXMLDelayedDirectionElement(totalY(), t, track, wordsPlacement, measure, tick + _offset);
delayedDirections.push_back(delayedDirection);
}
}
Expand Down Expand Up @@ -2814,15 +2814,15 @@ void MusicXMLParserDirection::direction(const QString& partId,

// Add element to score later, after collecting all the others and sorting by default-y
// This allows default-y to be at least respected by the order of elements
MusicXMLDelayedDirectionElement* delayedDirection = new MusicXMLDelayedDirectionElement(hasTotalY() ? totalY() : 100, dyn, track, dynamicsPlacement, measure, tick + _offset);
MusicXMLDelayedDirectionElement* delayedDirection = new MusicXMLDelayedDirectionElement(totalY(), dyn, track, dynamicsPlacement, measure, tick + _offset);
delayedDirections.push_back(delayedDirection);
}

// handle the elems
foreach( auto elem, _elems) {
// Add element to score later, after collecting all the others and sorting by default-y
// This allows default-y to be at least respected by the order of elements
MusicXMLDelayedDirectionElement* delayedDirection = new MusicXMLDelayedDirectionElement(hasTotalY() ? totalY() : 100, elem, track, placement(), measure, tick + _offset);
MusicXMLDelayedDirectionElement* delayedDirection = new MusicXMLDelayedDirectionElement(totalY(), elem, track, placement(), measure, tick + _offset);
delayedDirections.push_back(delayedDirection);
}

Expand Down Expand Up @@ -2889,8 +2889,17 @@ void MusicXMLParserDirection::directionType(QList<MusicXmlSpannerDesc>& starts,
Q_ASSERT(_e.isStartElement() && _e.name() == "direction-type");

while (_e.readNextStartElement()) {
_defaultY = _e.attributes().value("default-y").toDouble(&_hasDefaultY) * -0.1;
_relativeY = _e.attributes().value("relative-y").toDouble(&_hasRelativeY) * -0.1;
// Prevent multi-word directions from overwriting y-values.
bool hasDefaultYCandidate = false;
bool hasRelativeYCandidate = false;
qreal defaultYCandidate = _e.attributes().value("default-y").toDouble(&hasDefaultYCandidate) * -0.1;
qreal relativeYCandidate =_e.attributes().value("relative-y").toDouble(&hasRelativeYCandidate) * -0.1;
if (hasDefaultYCandidate && !_hasDefaultY)
_defaultY = defaultYCandidate;
if (hasRelativeYCandidate && !_hasRelativeY)
_relativeY = relativeYCandidate;
_hasDefaultY |= hasDefaultYCandidate;
_hasRelativeY |= hasRelativeYCandidate;
QString number = _e.attributes().value("number").toString();
int n = 0;
if (number != "") {
Expand Down Expand Up @@ -5535,18 +5544,15 @@ FiguredBass* MusicXMLParserPass2::figuredBass()
which affects both single strings and barres
*/

FretDiagram* MusicXMLParserPass2::frame(qreal& defaultY, qreal& relativeY, bool& hasTotalY)
FretDiagram* MusicXMLParserPass2::frame(qreal& defaultY, qreal& relativeY)
{
Q_ASSERT(_e.isStartElement() && _e.name() == "frame");

FretDiagram* fd = new FretDiagram(_score);

int fretOffset = 0;
bool tempHasY = false;
defaultY += _e.attributes().value("default-y").toDouble(&tempHasY) * -0.1;
hasTotalY |= tempHasY;
relativeY += _e.attributes().value("relative-y").toDouble(&tempHasY) * -0.1;
hasTotalY |= tempHasY;
defaultY += _e.attributes().value("default-y").toDouble() * -0.1;
relativeY += _e.attributes().value("relative-y").toDouble() * -0.1;

// Format: fret: string
std::map<int, int> bStarts;
Expand Down Expand Up @@ -5819,7 +5825,7 @@ void MusicXMLParserPass2::harmony(const QString& partId, Measure* measure, const
}
}
else if (_e.name() == "frame")
fd = frame(defaultY, relativeY, hasTotalY);
fd = frame(defaultY, relativeY);
else if (_e.name() == "level")
skipLogCurrElem();
else if (_e.name() == "offset") {
Expand Down Expand Up @@ -5876,7 +5882,7 @@ void MusicXMLParserPass2::harmony(const QString& partId, Measure* measure, const
}
// Add element to score later, after collecting all the others and sorting by default-y
// This allows default-y to be at least respected by the order of elements
MusicXMLDelayedDirectionElement* delayedDirection = new MusicXMLDelayedDirectionElement(hasTotalY ? totalY : 100, se, track, placement, measure, sTime + offset);
MusicXMLDelayedDirectionElement* delayedDirection = new MusicXMLDelayedDirectionElement(totalY, se, track, placement, measure, sTime + offset);
delayedDirections.push_back(delayedDirection);
}

Expand Down
2 changes: 1 addition & 1 deletion importexport/musicxml/importmxmlpass2.h
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ class MusicXMLParserPass2 {
void notePrintSpacingNo(Fraction& dura);
FiguredBassItem* figure(const int idx, const bool paren);
FiguredBass* figuredBass();
FretDiagram* frame(qreal& defaultY, qreal& relativeY, bool& hasTotalY);
FretDiagram* frame(qreal& defaultY, qreal& relativeY);
void harmony(const QString& partId, Measure* measure, const Fraction sTime, DelayedDirectionsList& delayedDirections);
Accidental* accidental();
void beam(QMap<int, QString>& beamTypes);
Expand Down
10 changes: 5 additions & 5 deletions mtest/musicxml/io/testFretDiagramLayoutOrder_ref.mscx
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,6 @@
<sigN>4</sigN>
<sigD>4</sigD>
</TimeSig>
<Tempo>
<tempo>4</tempo>
<followText>1</followText>
<text><sym>metNoteHalfUp</sym> <sym>metAugmentationDot</sym> = 80</text>
</Tempo>
<FretDiagram>
<fretOffset>2</fretOffset>
<frets>4</frets>
Expand Down Expand Up @@ -186,6 +181,11 @@
<dot>2</dot>
</string>
</FretDiagram>
<Tempo>
<tempo>4</tempo>
<followText>1</followText>
<text><sym>metNoteHalfUp</sym> <sym>metAugmentationDot</sym> = 80</text>
</Tempo>
<Chord>
<durationType>quarter</durationType>
<StemDirection>down</StemDirection>
Expand Down
Binary file modified mtest/musicxml/io/testTextOrder.pdf
Binary file not shown.
3 changes: 2 additions & 1 deletion mtest/musicxml/io/testTextOrder.xml
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@
</direction>
<direction>
<direction-type>
<words relative-y="11.15">3rd time senza Invisalign</words>
<words relative-y="11.15">3rd time senza </words>
<words font-style="italic">Invisalign</words>
</direction-type>
</direction>
<direction placement="below">
Expand Down
2 changes: 1 addition & 1 deletion mtest/musicxml/io/testTextOrder_ref.mscx
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@
<sigD>4</sigD>
</TimeSig>
<StaffText>
<text>3rd time senza Invisalign</text>
<text>3rd time senza <i>Invisalign</i></text>
</StaffText>
<StaffText>
<text>2nd time ad lib.</text>
Expand Down

0 comments on commit cd77762

Please sign in to comment.