Skip to content

Commit

Permalink
Fix GH#9186: Prevent merging articulations on xml import
Browse files Browse the repository at this point in the history 
Backport of musescore#20248
  • Loading branch information
@miiizen @Jojo-Schmitz
miiizen authored and Jojo-Schmitz committed Nov 29, 2023
1 parent ba87541 commit 537b014
Show file tree
Hide file tree
Showing 7 changed files with 467 additions and 1,041 deletions.
107 changes: 11 additions & 96 deletions importexport/musicxml/importmxmlpass2.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6839,9 +6839,17 @@ void MusicXMLParserNotations::articulations()
while (_e.readNextStartElement()) {
SymId id { SymId::noSym };
if (convertArticulationToSymId(_e.name().toString(), id)) {
Notation artic = Notation::notationWithAttributes(_e.name().toString(),
_e.attributes(), "articulations", id);
_notations.push_back(artic);
if (_e.name() == "detached-legato") {
_notations.push_back(Notation::notationWithAttributes("tenuto",
_e.attributes(), "articulations", SymId::articTenutoAbove));
_notations.push_back(Notation::notationWithAttributes("staccato",
_e.attributes(), "articulations", SymId::articStaccatoAbove));
}
else {
Notation artic = Notation::notationWithAttributes(_e.name().toString(),
_e.attributes(), "articulations", id);
_notations.push_back(artic);
}
_e.skipCurrentElement(); // skip but don't log
}
else if (_e.name() == "breath-mark") {
Expand Down Expand Up @@ -7334,35 +7342,6 @@ Notation Notation::notationWithAttributes(const QString& name, const QXmlStreamA
return notation;
}

//---------------------------------------------------------
// mergeNotations
//---------------------------------------------------------

/**
Helper function to merge two Notations. Used to combine articulations in combineArticulations.
*/

Notation Notation::mergeNotations(const Notation& n1, const Notation& n2, const SymId& symId)
{
// Sort and combine the names
std::vector<QString> names{ n1.name(), n2.name() };
std::sort(names.begin(), names.end());
QString name = names[0] + " " + names[1];

// Parents should match (and will both be "articulation")
Q_ASSERT(n1.parent() == n2.parent());
QString parent = n1.parent();

Notation mergedNotation { name, parent, symId };
for (const auto& attr : n1.attributes()) {
mergedNotation.addAttribute(attr.first, attr.second);
}
for (const auto& attr : n2.attributes()) {
mergedNotation.addAttribute(attr.first, attr.second);
}
return mergedNotation;
}

//---------------------------------------------------------
// addAttribute
//---------------------------------------------------------
Expand Down Expand Up @@ -7454,69 +7433,6 @@ void MusicXMLParserNotations::skipLogCurrElem()
_e.skipCurrentElement();
}

//---------------------------------------------------------
// skipCombine
//---------------------------------------------------------

/**
Helper function to hold conditions under which a potential combine should be skipped.
*/

bool MusicXMLParserNotations::skipCombine(const Notation& n1, const Notation& n2)
{
bool placementsSpecifiedAndDifferent = n1.attribute("placement") != "" &&
n2.attribute("placement") != "" &&
n1.attribute("placement") != n2.attribute("placement");
bool upMarcatoDownOther = (n1.name() == "strong-accent" && n1.attribute("type") == "up" &&
n2.attribute("placement") == "below") ||
(n2.name() == "strong-accent" && n2.attribute("type") == "up" &&
n1.attribute("placement") == "below");
bool downMarcatoUpOther = (n1.name() == "strong-accent" && n1.attribute("type") == "down" &&
n2.attribute("placement") == "above") ||
(n2.name() == "strong-accent" && n2.attribute("type") == "down" &&
n1.attribute("placement") == "above");
bool slurEndpoint = _slurStart || _slurStop;
return placementsSpecifiedAndDifferent || upMarcatoDownOther || downMarcatoUpOther || slurEndpoint;
}

//---------------------------------------------------------
// combineArticulations
//---------------------------------------------------------
/**
Combine any eligible articulations.
i.e. accent + staccato = staccato accent
*/

void MusicXMLParserNotations::combineArticulations()
{
QMap<std::set<SymId>, SymId> map; // map set of symbols to combined symbol
map[{ SymId::articAccentAbove, SymId::articStaccatoAbove }] = SymId::articAccentStaccatoAbove;
map[{ SymId::articMarcatoAbove, SymId::articStaccatoAbove }] = SymId::articMarcatoStaccatoAbove;
map[{ SymId::articMarcatoAbove, SymId::articTenutoAbove }] = SymId::articMarcatoTenutoAbove;
map[{ SymId::articAccentAbove, SymId::articTenutoAbove }] = SymId::articTenutoAccentAbove;

// Iterate through each distinct pair (backwards, to allow for deletions)
for (std::vector<Notation>::reverse_iterator n1 = _notations.rbegin(), n1Next = n1; n1 != _notations.rend(); n1 = n1Next) {
n1Next = std::next(n1);
if (n1->parent() != "articulations")
continue;
for (std::vector<Notation>::reverse_iterator n2 = n1 + 1, n2Next = n1; n2 != _notations.rend(); n2 = n2Next) {
n2Next = std::next(n2);
if (n2->parent() != "articulations" || skipCombine(*n1, *n2))
continue;
// Combine and remove articulations if present in map
std::set<SymId> currentPair = { n1->symId(), n2->symId() };
if (map.contains(currentPair)) {
Notation mergedNotation = Notation::mergeNotations(*n1, *n2, map.value(currentPair));
n1Next = decltype(n1){ _notations.erase(std::next(n1).base()) };
n2Next = decltype(n2){ _notations.erase(std::next(n2).base()) };
_notations.push_back(mergedNotation);
}
}
}
}


//---------------------------------------------------------
// parse
//---------------------------------------------------------
Expand Down Expand Up @@ -7573,7 +7489,6 @@ void MusicXMLParserNotations::parse()
qDebug("%s", qPrintable(notation.print()));
}
*/
combineArticulations();

addError(checkAtEndElement(_e, "notations"));
}
Expand Down
3 changes: 0 additions & 3 deletions importexport/musicxml/importmxmlpass2.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ class Notation {
QString text() const { return _text; }
static Notation notationWithAttributes(const QString& name, const QXmlStreamAttributes attributes,
const QString& parent = "", const SymId& symId = SymId::noSym);
static Notation mergeNotations(const Notation& n1, const Notation& n2, const SymId& symId = SymId::noSym);
private:
QString _name;
QString _parent;
Expand Down Expand Up @@ -219,12 +218,10 @@ class MusicXMLParserNotations {
QString tremoloType() const { return _tremoloType; }
int tremoloNr() const { return _tremoloNr; }
bool mustStopGraceAFter() const { return _slurStop || _wavyLineStop; }
bool skipCombine(const Notation& n1, const Notation& n2);
private:
void addError(const QString& error); ///< Add an error to be shown in the GUI
void addNotation(const Notation& notation, ChordRest* const cr, Note* const note);
void addTechnical(const Notation& notation, Note* note);
void combineArticulations();
void harmonic();
void articulations();
void dynamics();
Expand Down

0 comments on commit 537b014

Please sign in to comment.