From e93739641acb3edf64289f15cfd7e0642d5d700b Mon Sep 17 00:00:00 2001 From: James Mizen Date: Wed, 29 Nov 2023 14:18:17 +0000 Subject: [PATCH] Remove code to merge articulations on xml import --- .../internal/musicxml/importmxmlpass2.cpp | 112 +---- .../internal/musicxml/importmxmlpass2.h | 3 - .../data/testArticulationCombination_ref.xml | 454 ++++++++++++++++++ .../musicxml/tests/data/testColors.xml | 2 +- .../tests/data/testNoteAttributes2_ref.xml | 3 +- .../musicxml/tests/musicxml_tests.cpp | 2 +- 6 files changed, 468 insertions(+), 108 deletions(-) create mode 100644 src/importexport/musicxml/tests/data/testArticulationCombination_ref.xml diff --git a/src/importexport/musicxml/internal/musicxml/importmxmlpass2.cpp b/src/importexport/musicxml/internal/musicxml/importmxmlpass2.cpp index bc66c4e10b4f..906db621860f 100644 --- a/src/importexport/musicxml/internal/musicxml/importmxmlpass2.cpp +++ b/src/importexport/musicxml/internal/musicxml/importmxmlpass2.cpp @@ -6259,9 +6259,16 @@ 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") { auto value = _e.readElementText(); @@ -6764,35 +6771,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 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 //--------------------------------------------------------- @@ -6884,75 +6862,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) -{ - // at this point, if only one placement is specified, don't combine. - // we may revisit this in the future once we have a better idea of how we want to combine - // things by default. - bool placementsSpecifiedAndDifferent = 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, 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; - map[{ SymId::articSoftAccentAbove, SymId::articStaccatoAbove }] = SymId::articSoftAccentStaccatoAbove; - map[{ SymId::articSoftAccentAbove, SymId::articTenutoAbove }] = SymId::articSoftAccentTenutoAbove; - map[{ SymId::articSoftAccentAbove, SymId::articTenutoStaccatoAbove }] = SymId::articSoftAccentTenutoStaccatoAbove; - - // Iterate through each distinct pair (backwards, to allow for deletions) - for (std::vector::reverse_iterator n1 = _notations.rbegin(), n1Next = n1; n1 != _notations.rend(); n1 = n1Next) { - n1Next = std::next(n1); - if (n1->parent() != "articulations") { - continue; - } - for (std::vector::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 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 //--------------------------------------------------------- @@ -6999,7 +6908,6 @@ void MusicXMLParserNotations::parse() LOGD("%s", qPrintable(notation.print())); } */ - combineArticulations(); addError(checkAtEndElement(_e, "notations")); } diff --git a/src/importexport/musicxml/internal/musicxml/importmxmlpass2.h b/src/importexport/musicxml/internal/musicxml/importmxmlpass2.h index c5e9eb436bea..8323e9c2c17e 100644 --- a/src/importexport/musicxml/internal/musicxml/importmxmlpass2.h +++ b/src/importexport/musicxml/internal/musicxml/importmxmlpass2.h @@ -170,7 +170,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; @@ -217,12 +216,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(); diff --git a/src/importexport/musicxml/tests/data/testArticulationCombination_ref.xml b/src/importexport/musicxml/tests/data/testArticulationCombination_ref.xml new file mode 100644 index 000000000000..2ef77d032a46 --- /dev/null +++ b/src/importexport/musicxml/tests/data/testArticulationCombination_ref.xml @@ -0,0 +1,454 @@ + + + + + Title + + + Composer + + MuseScore 0.7.0 + 2007-09-10 + + + + + + + + + + Piano + Pno. + + Piano + + + + 1 + 1 + 78.7402 + 0 + + + + + + + 1 + + 0 + + + + G + 2 + + + + + C + 5 + + 1 + 1 + quarter + down + + + + + + + + + + D + 5 + + 1 + 1 + quarter + down + + + + + + + + + + E + 5 + + 1 + 1 + quarter + down + + + + + + + + + + F + 5 + + 1 + 1 + quarter + down + + + + + + + + + + + + G + 5 + + 1 + 1 + quarter + down + + + + + + + + + F + 5 + + 1 + 1 + quarter + down + + + + + + + + + + E + 5 + + 1 + 1 + quarter + down + + + + + + + + + + D + 5 + + 1 + 1 + quarter + down + + + + + + + + + + + + + + A + 4 + + 1 + 1 + quarter + up + + + + + + + + + + G + 4 + + 1 + 1 + quarter + up + + + + + + + + + + F + 4 + + 1 + 1 + quarter + up + + + + + + + + + + E + 4 + + 1 + 1 + quarter + up + + + + + + + + + + + + D + 4 + + 1 + 1 + quarter + up + + + + + + + + + E + 4 + + 1 + 1 + quarter + up + + + + + + + + + + F + 4 + + 1 + 1 + quarter + up + + + + + + + + + + G + 4 + + 1 + 1 + quarter + up + + + + + + + + + + + + + + C + 5 + + 1 + 1 + quarter + down + + + + + + + + + + C + 5 + + 1 + 1 + quarter + down + + + + + + + + + + C + 5 + + 1 + 1 + quarter + down + + + + + + + + + + C + 5 + + 1 + 1 + quarter + down + + + + + + + + + + + + C + 5 + + 1 + 1 + quarter + down + + + + + + + + + + C + 5 + + 1 + 1 + quarter + down + + + + + + + + + + C + 5 + + 1 + 1 + quarter + down + + + + + + + + + + + C + 5 + + 1 + 1 + quarter + down + + + + + + + + + + light-heavy + + + + diff --git a/src/importexport/musicxml/tests/data/testColors.xml b/src/importexport/musicxml/tests/data/testColors.xml index 90cc1a5ee761..da020898bdb6 100644 --- a/src/importexport/musicxml/tests/data/testColors.xml +++ b/src/importexport/musicxml/tests/data/testColors.xml @@ -72,8 +72,8 @@ down - + diff --git a/src/importexport/musicxml/tests/data/testNoteAttributes2_ref.xml b/src/importexport/musicxml/tests/data/testNoteAttributes2_ref.xml index b2ec46a6823a..3c04cc65c868 100644 --- a/src/importexport/musicxml/tests/data/testNoteAttributes2_ref.xml +++ b/src/importexport/musicxml/tests/data/testNoteAttributes2_ref.xml @@ -448,7 +448,8 @@ up - + + diff --git a/src/importexport/musicxml/tests/musicxml_tests.cpp b/src/importexport/musicxml/tests/musicxml_tests.cpp index cf85348dd080..1d7b33e93def 100644 --- a/src/importexport/musicxml/tests/musicxml_tests.cpp +++ b/src/importexport/musicxml/tests/musicxml_tests.cpp @@ -378,7 +378,7 @@ TEST_F(Musicxml_Tests, arpGliss3) { mxmlIoTest("testArpGliss3"); } TEST_F(Musicxml_Tests, articulationCombination) { - mxmlIoTest("testArticulationCombination"); + mxmlIoTestRef("testArticulationCombination"); } TEST_F(Musicxml_Tests, backupRoundingError) { mxmlImportTestRef("testBackupRoundingError");