Skip to content

Commit

Permalink
ENG-59: Fix missing secondary beam breaks
Browse files Browse the repository at this point in the history
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
  • Loading branch information
iveshenry18 authored and Jojo-Schmitz committed Jul 27, 2021
1 parent 5e7c022 commit c118cb2
Show file tree
Hide file tree
Showing 7 changed files with 1,225 additions and 30 deletions.
21 changes: 17 additions & 4 deletions importexport/musicxml/exportxml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2928,30 +2928,43 @@ static void writeBeam(XmlWriter& xml, ChordRest* const cr, Beam* const b)
int blp = -1; // beam level previous chord
int blc = -1; // beam level current chord
int bln = -1; // beam level next chord
Beam::Mode bmc = Beam::Mode::AUTO; // beam mode current chord
Beam::Mode bmn = Beam::Mode::AUTO; // beam mode next chord
// find beam level previous chord
for (int i = idx - 1; blp == -1 && i >= 0; --i) {
const auto crst = elements[i];
if (crst->isChord())
blp = toChord(crst)->beams();
}
// find beam level current chord
if (cr->isChord())
if (cr->isChord()) {
blc = toChord(cr)->beams();
bmc = toChord(cr)->beamMode();
}
// find beam level next chord
for (int i = idx + 1; bln == -1 && i < elements.size(); ++i) {
const auto crst = elements[i];
if (crst->isChord())
if (crst->isChord()) {
bln = toChord(crst)->beams();
bmn = toChord(crst)->beamMode();
}
}
// find beam type and write
for (int i = 1; i <= blc; ++i) {
QString text;
if (blp < i && bln >= i) text = "begin";
// TODO: correctly handle Beam::Mode::AUTO
// when equivalent to BEGIN32 or BEGIN64
if ((blp < i && bln >= i)
|| (bmc == Beam::Mode::BEGIN32 && i > 1)
|| (bmc == Beam::Mode::BEGIN64 && i > 2))
text = "begin";
else if (blp < i && bln < i) {
if (bln > 0) text = "forward hook";
else if (blp > 0) text = "backward hook";
}
else if (blp >= i && bln < i)
else if ((blp >= i && bln < i)
|| (bmn == Beam::Mode::BEGIN32 && i > 1)
|| (bmn == Beam::Mode::BEGIN64 && i > 2))
text = "end";
else if (blp >= i && bln >= i)
text = "continue";
Expand Down
68 changes: 43 additions & 25 deletions importexport/musicxml/importmxmlpass2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1836,7 +1836,7 @@ static void handleBeamAndStemDir(ChordRest* cr, const Beam::Mode bm, const Direc
// reset beam mode for all elements and remove the beam
removeBeam(beam);
}
else if (!(bm == Beam::Mode::BEGIN || bm == Beam::Mode::MID || bm == Beam::Mode::END)) {
else if (!(bm == Beam::Mode::BEGIN || bm == Beam::Mode::MID || bm == Beam::Mode::END || bm == Beam::Mode::BEGIN32 || bm == Beam::Mode::BEGIN64)) {
qDebug("handleBeamAndStemDir() in beam, bm %d -> abort beam", static_cast<int>(bm));
// reset beam mode for all elements and remove the beam
removeBeam(beam);
Expand Down Expand Up @@ -4497,6 +4497,36 @@ static void setNoteHead(Note* note, const QColor noteheadColor, const bool noteh
note->setHeadType(NoteHead::Type::HEAD_QUARTER);
}

//---------------------------------------------------------
// computeBeamMode
//---------------------------------------------------------

/**
Calculate the beam mode based on the collected beamTypes.
*/

static Beam::Mode computeBeamMode(const QMap<int, QString>& beamTypes)
{
// Start with uniquely-handled beam modes
if (beamTypes.value(1) == "continue"
&& beamTypes.value(2) == "begin")
return Beam::Mode::BEGIN32;
else if (beamTypes.value(1) == "continue"
&& beamTypes.value(2) == "continue"
&& beamTypes.value(3) == "begin")
return Beam::Mode::BEGIN64;
// Generic beam modes are naive to all except the first beam
else if (beamTypes.value(1) == "begin")
return Beam::Mode::BEGIN;
else if (beamTypes.value(1) == "continue")
return Beam::Mode::MID;
else if (beamTypes.value(1) == "end")
return Beam::Mode::END;
else
// backward-hook, forward-hook, and other unknown combinations
return Beam::Mode::AUTO;
}

//---------------------------------------------------------
// addFiguredBassElemens
//---------------------------------------------------------
Expand Down Expand Up @@ -4707,7 +4737,8 @@ Note* MusicXMLParserPass2::note(const QString& partId,
int velocity = round(_e.attributes().value("dynamics").toDouble() * 0.9);
bool graceSlash = false;
bool printObject = _e.attributes().value("print-object") != "no";
Beam::Mode bm = Beam::Mode::AUTO;
Beam::Mode bm;
QMap<int, QString> beamTypes;
QString instrumentId;
MusicXMLParserLyric lyric { _pass1.getMusicXmlPart(partId).lyricNumberHandler(), _e, _score, _logger };
MusicXMLParserNotations notations { _e, _score, _logger };
Expand All @@ -4723,7 +4754,7 @@ Note* MusicXMLParserPass2::note(const QString& partId,
// element handled
}
else if (_e.name() == "beam")
beam(bm);
beam(beamTypes);
else if (_e.name() == "chord") {
chord = true;
_e.readNext();
Expand Down Expand Up @@ -4805,6 +4836,8 @@ Note* MusicXMLParserPass2::note(const QString& partId,
currBeams.insert(currentVoice, (Beam *)nullptr);
Beam* &currBeam = currBeams[currentVoice];

bm = computeBeamMode(beamTypes);

// check for timing error(s) and set dura
// keep in this order as checkTiming() might change dura
auto errorStr = mnd.checkTiming(type, rest, grace);
Expand Down Expand Up @@ -5630,32 +5663,17 @@ void MusicXMLParserPass2::harmony(const QString& partId, Measure* measure, const

/**
Parse the /score-partwise/part/measure/note/beam node.
Sets beamMode in case of begin, continue or end beam number 1.
Collects beamTypes, used in computeBeamMode.
*/

void MusicXMLParserPass2::beam(Beam::Mode& beamMode)
{
void MusicXMLParserPass2::beam(QMap<int, QString>& beamTypes) {
Q_ASSERT(_e.isStartElement() && _e.name() == "beam");

int beamNo = _e.attributes().value("number").toInt();

if (beamNo == 1) {
QString s = _e.readElementText();
if (s == "begin")
beamMode = Beam::Mode::BEGIN;
else if (s == "end")
beamMode = Beam::Mode::END;
else if (s == "continue")
beamMode = Beam::Mode::MID;
else if (s == "backward hook")
;
else if (s == "forward hook")
;
else
_logger->logError(QString("unknown beam keyword '%1'").arg(s), &_e);
}
else
_e.skipCurrentElement();
bool hasBeamNo;
int beamNo = _e.attributes().value("number").toInt(&hasBeamNo);
QString s = _e.readElementText();

beamTypes.insert(hasBeamNo ? beamNo : 1, s);
}

//---------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion importexport/musicxml/importmxmlpass2.h
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ class MusicXMLParserPass2 {
FretDiagram* frame(qreal& defaultY, qreal& relativeY, bool& hasTotalY);
void harmony(const QString& partId, Measure* measure, const Fraction sTime, DelayedDirectionsList& delayedDirections);
Accidental* accidental();
void beam(Beam::Mode& beamMode);
void beam(QMap<int, QString>& beamTypes);
void duration(Fraction& dura);
void forward(Fraction& dura);
void backup(Fraction& dura);
Expand Down
Binary file added mtest/musicxml/io/testBeamModes.pdf
Binary file not shown.

0 comments on commit c118cb2

Please sign in to comment.