Merge pull request #18556 from RomanPudashkin/musicxml_fixes

musicxml_fixes

src/importexport/musicxml/internal/musicxml/exportxml.cpp

@@ -3134,8 +3134,11 @@ void ExportMusicXml::chordAttributes(Chord* chord, Notations& notations, Technic
                 _xml.startElementRaw(mxmlTechn + attr);
                 _xml.tag("natural");
                 _xml.endElement();
-            } else {   // TODO: check additional modifier (attr) for other symbols
-                _xml.tagRaw(mxmlTechn);
+            } else {
+                if (placement != "") {
+                    attr += QString(" placement=\"%1\"").arg(placement);
+                }
+                _xml.tagRaw(mxmlTechn + attr);
             }
         }
     }

src/importexport/musicxml/internal/musicxml/importmxml.cpp

@@ -1,4 +1,4 @@
-/*
+/*
  * SPDX-License-Identifier: GPL-3.0-only
  * MuseScore-CLA-applies
  *
@@ -53,6 +53,34 @@ static int musicXMLImportErrorDialog(QString text, QString detailedText)
     return errorDialog.exec();
 }
 
+static void updateNamesForAccidentals(Instrument* inst)
+{
+    auto replace = [](String name) {
+        name = name.replace(std::regex(
+                                R"(((?:^|\s)([A-Ga-g]|[Uu][Tt]|[Dd][Oo]|[Rr][EeÉé]|[MmSsTt][Ii]|[FfLl][Aa]|[Ss][Oo][Ll]))b(?=\s|$))"),
+                            String::fromStdString(R"($1♭)"));
+
+        name = name.replace(std::regex(
+                                R"(((?:^|\s)([A-Ga-g]|[Uu][Tt]|[Dd][Oo]|[Rr][EeÉé]|[MmSsTt][Ii]|[FfLl][Aa]|[Ss][Oo][Ll]))#(?=\s|$))"),
+                            String::fromStdString(R"($1♯)"));
+
+        return name;
+    };
+    // change staff names from simple text (eg 'Eb') to text using accidental symbols (eg 'E♭')
+
+    // Instrument::longNames() is const af so we need to make a deep copy, update it, and then set it again
+    StaffNameList longNamesCopy = inst->longNames();
+    for (StaffName& sn : longNamesCopy) {
+        sn.setName(replace(sn.name()));
+    }
+    StaffNameList shortNamesCopy = inst->shortNames();
+    for (StaffName& sn : shortNamesCopy) {
+        sn.setName(replace(sn.name()));
+    }
+    inst->setLongNames(longNamesCopy);
+    inst->setShortNames(shortNamesCopy);
+}
+
 //---------------------------------------------------------
 //   importMusicXMLfromBuffer
 //---------------------------------------------------------
@@ -83,6 +111,7 @@ Err importMusicXMLfromBuffer(Score* score, const QString& /*name*/, QIODevice* d
     for (const Part* part : score->parts()) {
         for (const auto& pair : part->instruments()) {
             pair.second->updateInstrumentId();
+            updateNamesForAccidentals(pair.second);
         }
     }
 

src/importexport/musicxml/internal/musicxml/importmxmlpass2.cpp

@@ -2161,6 +2161,21 @@ void MusicXMLParserPass2::measure(const QString& partId, const Fraction time)
     measure->setRepeatStart(false);
     measure->setRepeatEnd(false);
 
+    /* TODO: for cutaway measures, i believe we can expect the staff to continue to be cutaway until another
+    * print-object="yes" attribute is found. Here is the code that does that, though I don't want to actually commit this until
+    * we have the exporter dealing with this sort of stuff as well.
+    *
+    * When print-object="yes" is encountered, the measure will explicitly be set to visible (see MusicXMLParserPass2::staffDetails)
+
+    MeasureBase* prevBase = measure->prev();
+    if (prevBase) {
+        Part* part = _pass1.getPart(partId);
+        staff_idx_t staffIdx = _score->staffIdx(part);
+        if (!toMeasure(prevBase)->visible(staffIdx)) {
+            measure->setStaffVisible(staffIdx, false);
+        }
+    } */
+
     Fraction mTime;   // current time stamp within measure
     Fraction prevTime;   // time stamp within measure previous chord
     Chord* prevChord = 0;         // previous chord
@@ -2394,7 +2409,7 @@ void MusicXMLParserPass2::attributes(const QString& partId, Measure* measure, co
         } else if (_e.name() == "measure-style") {
             measureStyle(measure);
         } else if (_e.name() == "staff-details") {
-            staffDetails(partId);
+            staffDetails(partId, measure);
         } else if (_e.name() == "time") {
             time(partId, measure, tick);
         } else if (_e.name() == "transpose") {
@@ -2427,7 +2442,7 @@ static void setStaffLines(Score* score, staff_idx_t staffIdx, int stafflines)
  Parse the /score-partwise/part/measure/attributes/staff-details node.
  */
 
-void MusicXMLParserPass2::staffDetails(const QString& partId)
+void MusicXMLParserPass2::staffDetails(const QString& partId, Measure* measure)
 {
     //logDebugTrace("MusicXMLParserPass2::staffDetails");
 
@@ -2452,9 +2467,29 @@ void MusicXMLParserPass2::staffDetails(const QString& partId)
 
     StringData* t = new StringData;
     QString visible = _e.attributes().value("print-object").toString();
+    QString spacing = _e.attributes().value("print-spacing").toString();
     if (visible == "no") {
-        _score->staff(staffIdx)->setVisible(false);
-    } else if (!visible.isEmpty() && visible != "yes") {
+        // EITHER:
+        //  1) this indicates an empty staff that is hidden
+        //  2) this indicates a cutaway measure. if it is a cutaway measure then print-spacing will be yes
+        if (spacing == "yes") {
+            measure->setStaffVisible(staffIdx, false);
+        } else if (measure && !measure->hasVoices(staffIdx) && measure->isOnlyRests(staffIdx * VOICES)) {
+            // measures with print-object="no" are generally exported by exporters such as dolet when empty staves are hidden.
+            // for this reason, if we see print-object="no" (and no print-spacing), we can assume that this indicates we should set
+            // the hide empty staves style.
+            _score->style().set(Sid::hideEmptyStaves, true);
+            _score->style().set(Sid::dontHideStavesInFirstSystem, false);
+        } else {
+            // this doesn't apply to a measure, so we'll assume the entire staff has to be hidden.
+            _score->staff(staffIdx)->setVisible(false);
+        }
+    } else if (visible == "yes") {
+        if (measure) {
+            _score->staff(staffIdx)->setVisible(true);
+            measure->setStaffVisible(staffIdx, true);
+        }
+    } else {
         _logger->logError(QString("print-object should be \"yes\" or \"no\""));
     }
 

src/importexport/musicxml/internal/musicxml/importmxmlpass2.h

@@ -305,7 +305,7 @@ class MusicXMLParserPass2
     void stem(DirectionV& sd, bool& nost);
     void doEnding(const QString& partId, Measure* measure, const QString& number, const QString& type, const QString& text,
                   const bool print);
-    void staffDetails(const QString& partId);
+    void staffDetails(const QString& partId, Measure* measure = nullptr);
     void staffTuning(StringData* t);
     void skipLogCurrElem();
 

src/importexport/musicxml/tests/musicxml_tests.cpp

@@ -592,9 +592,6 @@ TEST_F(Musicxml_Tests, helloReadCompr) {
 TEST_F(Musicxml_Tests, helloReadWriteCompr) {
     mxmlReadWriteTestCompr("testHello");
 }
-TEST_F(Musicxml_Tests, hiddenStaves) {
-    mxmlIoTest("testHiddenStaves");
-}
 TEST_F(Musicxml_Tests, implicitMeasure1) {
     mxmlIoTest("testImplicitMeasure1");
 }
@@ -973,3 +970,10 @@ TEST_F(Musicxml_Tests, words1) {
 TEST_F(Musicxml_Tests, words2) {
     mxmlIoTest("testWords2");
 }
+TEST_F(Musicxml_Tests, hiddenStaves)
+{
+    String fileName = String::fromUtf8("testHiddenStaves.xml");
+    MasterScore* score = readScore(XML_IO_DATA_DIR + fileName);
+
+    EXPECT_EQ(score->style().value(Sid::hideEmptyStaves).toBool(), true);
+}