Skip to content

Commit

Permalink
ENG-63: Properly handle cue grace notes
Browse files Browse the repository at this point in the history
Previously, our importer and exporter didn't account for notes that
were both grace notes and cue notes (therefore extra-small in size).
This commit amends that. Additionally, since Dolet also seems to fail
in this area, this commit adds a function to coerce the `small` and
`play` attributes of a grace chord to match that of its main chord.
Finally, it adds support for the "grace-cue" size to the exporter and
importer. In the importer, this is treated the same as the "cue" size, as
exporters tend to misinterpret the distinction between these.

Duplicate of musescore#8581, plus fixing a compiler warning, see musescore#8554
  • Loading branch information
iveshenry18 authored and Jojo-Schmitz committed Sep 2, 2021
1 parent 52cf12b commit 1be25fb
Show file tree
Hide file tree
Showing 7 changed files with 2,801 additions and 13 deletions.
4 changes: 3 additions & 1 deletion importexport/musicxml/exportxml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3184,7 +3184,7 @@ static bool isSmallNote(const Note* const note)

static bool isCueNote(const Note* const note)
{
return (!note->chord()->isGrace()) && isSmallNote(note) && !note->play();
return isSmallNote(note) && !note->play();
}

//---------------------------------------------------------
Expand Down Expand Up @@ -3227,6 +3227,8 @@ static void writeTypeAndDots(XmlWriter& xml, const Note* const note)
// small notes are indicated by size=cue, but for grace and cue notes this is implicit
if (isSmallNote(note) && !isCueNote(note) && !note->chord()->isGrace())
xml.tag("type size=\"cue\"", s);
else if (isSmallNote(note) && !isCueNote(note) && note->chord()->isGrace())
xml.tag("type size=\"grace-cue\"", s);
else
xml.tag("type", s);
for (int ni = dots; ni > 0; ni--)
Expand Down
51 changes: 40 additions & 11 deletions importexport/musicxml/importmxmlpass2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2009,6 +2009,30 @@ static void markUserAccidentals(const int firstStaff,
}
}

//---------------------------------------------------------
// coerceGraceCue
//---------------------------------------------------------

/**
If the mainChord is small and/or silent, the grace note should likely
match this. Exporters tend to incorrectly omit <cue> or <type size="cue">
from grace notes.
*/

static void coerceGraceCue(Chord* mainChord, Chord* graceChord)
{
if (mainChord->small())
graceChord->setSmall(true);
bool anyPlays = false;
for (auto n : mainChord->notes()) {
anyPlays |= n->play();
}
if (!anyPlays) {
for (auto gn : graceChord->notes())
gn->setPlay(false);
}
}

//---------------------------------------------------------
// addGraceChordsAfter
//---------------------------------------------------------
Expand All @@ -2029,6 +2053,7 @@ static void addGraceChordsAfter(Chord* c, GraceChordList& gcl, int& gac)
gcl.removeFirst();
graceChord->toGraceAfter();
c->add(graceChord); // TODO check if same voice ?
coerceGraceCue(c, graceChord);
qDebug("addGraceChordsAfter chord %p grace after chord %p", c, graceChord);
}
gac--;
Expand Down Expand Up @@ -2056,6 +2081,7 @@ static void addGraceChordsBefore(Chord* c, GraceChordList& gcl)
}
}
c->add(gc); // TODO check if same voice ?
coerceGraceCue(c, gc);
}
gcl.clear();
}
Expand Down Expand Up @@ -4503,11 +4529,14 @@ NoteType graceNoteType(const TDuration duration, const bool slash)
*/

static Chord* createGraceChord(Score* score, const int track,
const TDuration duration, const bool slash)
const TDuration duration, const bool slash, const bool small)
{
Chord* c = new Chord(score);
c->setNoteType(graceNoteType(duration, slash));
c->setTrack(track);
// Chord is initialized with the smallness of its first note.
// If a non-small note is added later, this is handled in handleSmallness.
c->setSmall(small);
// note grace notes have no durations, use default fraction 0/1
setChordRestDuration(c, duration, Fraction());
return c;
Expand Down Expand Up @@ -4907,7 +4936,7 @@ Note* MusicXMLParserPass2::note(const QString& partId,
else if (_e.name() == "stem")
stem(stemDir, noStem);
else if (_e.name() == "type") {
small = _e.attributes().value("size") == "cue";
small = (_e.attributes().value("size") == "cue" || _e.attributes().value("size") == "grace-cue");
type = _e.readElementText();
}
else if (_e.name() == "voice")
Expand Down Expand Up @@ -5019,7 +5048,7 @@ Note* MusicXMLParserPass2::note(const QString& partId,
// TODO: check if explicit stem direction should also be set for grace notes
// (the DOM parser does that, but seems to have no effect on the autotester)
if (!chord || gcl.isEmpty()) {
c = createGraceChord(_score, msTrack + msVoice, duration, graceSlash);
c = createGraceChord(_score, msTrack + msVoice, duration, graceSlash, small || cue);
// TODO FIX
// the setStaffMove() below results in identical behaviour as 2.0:
// grace note will be at the wrong staff with the wrong pitch,
Expand Down Expand Up @@ -5069,6 +5098,14 @@ Note* MusicXMLParserPass2::note(const QString& partId,
}
}
else {
handleSmallness(cue || small, note, c);
note->setPlay(!cue); // cue notes don't play
note->setHeadGroup(headGroup);
if (noteColor != QColor::Invalid)
note->setColor(noteColor);
setNoteHead(note, noteheadColor, noteheadParentheses, noteheadFilled);
note->setVisible(printObject); // TODO also set the stem to invisible

if (!grace) {
// regular note
// handle beam
Expand All @@ -5084,14 +5121,6 @@ Note* MusicXMLParserPass2::note(const QString& partId,
addGraceChordsBefore(c, gcl);
}

handleSmallness(cue || small, note, c);
note->setPlay(!cue); // cue notes don't play
note->setHeadGroup(headGroup);
if (noteColor != QColor::Invalid)
note->setColor(noteColor);
setNoteHead(note, noteheadColor, noteheadParentheses, noteheadFilled);
note->setVisible(printObject); // TODO also set the stem to invisible

if (velocity > 0) {
note->setVeloType(Note::ValueType::USER_VAL);
note->setVeloOffset(velocity);
Expand Down

0 comments on commit 1be25fb

Please sign in to comment.