diff --git a/src/include/mx/api/KeyData.h b/src/include/mx/api/KeyData.h index f0c745ec5..16cb446eb 100644 --- a/src/include/mx/api/KeyData.h +++ b/src/include/mx/api/KeyData.h @@ -18,6 +18,19 @@ enum class KeyMode minor }; +// CancelLocation represents the cancel element's optional location attribute. From MusicXML +// Specification: The cancel-location type is used to indicate where a key signature +// cancellation appears relative to a new key signature: to the left, to the right, or before +// the barline and to the left. It is left by default. For mid-measure key elements, a +// cancel-location of before-barline should be treated like a cancel-location of left. +enum class CancelLocation +{ + unspecified, // a location value was not provided + left, + right, + beforeBarline +}; + // KeyData represents a key signature. It can be in one of two configurations. Either you specify // 'fifths' and 'mode', or you can create a custom key signature by adding items to the customKey // vector. If anything is found in the customKey vector, then fifths and mode will be ignored. @@ -61,6 +74,10 @@ struct KeyData // appears relative to the new key signature. int cancel; + // The cancel element's optional location attribute. It is ignored unless cancel is non-zero + // (i.e. unless a cancel element is present). + CancelLocation cancelLocation; + // Mode specifies whether the key is major or minor. It is optional. KeyMode mode; @@ -79,8 +96,8 @@ struct KeyData std::vector nonTraditional; KeyData() - : fifths{0}, cancel{0}, mode{KeyMode::unspecified}, tickTimePosition{0}, staffIndex{INDEX_UNSPECIFIED}, - nonTraditional{} + : fifths{0}, cancel{0}, cancelLocation{CancelLocation::unspecified}, mode{KeyMode::unspecified}, + tickTimePosition{0}, staffIndex{INDEX_UNSPECIFIED}, nonTraditional{} { } }; @@ -88,6 +105,7 @@ struct KeyData MXAPI_EQUALS_BEGIN(KeyData) MXAPI_EQUALS_MEMBER(fifths) MXAPI_EQUALS_MEMBER(cancel) +MXAPI_EQUALS_MEMBER(cancelLocation) MXAPI_EQUALS_MEMBER(mode) MXAPI_EQUALS_MEMBER(tickTimePosition) MXAPI_EQUALS_MEMBER(staffIndex) diff --git a/src/private/mx/impl/Converter.cpp b/src/private/mx/impl/Converter.cpp index b9268ace0..93b2abfcb 100644 --- a/src/private/mx/impl/Converter.cpp +++ b/src/private/mx/impl/Converter.cpp @@ -1332,6 +1332,12 @@ const Converter::EnumMap Converter::kindMap = { {core::KindValue::none(), api::ChordKind::none}, }; +const Converter::EnumMap Converter::cancelLocationMap = { + {core::CancelLocation::left(), api::CancelLocation::left}, + {core::CancelLocation::right(), api::CancelLocation::right}, + {core::CancelLocation::beforeBarline(), api::CancelLocation::beforeBarline}, +}; + api::Step Converter::convert(core::Step inStep) const { return findApiItem(stepMap, api::Step::c, inStep); @@ -1654,6 +1660,16 @@ api::ChordKind Converter::convert(core::KindValue value) const return findApiItem(kindMap, api::ChordKind::unspecified, value); } +core::CancelLocation Converter::convert(api::CancelLocation value) const +{ + return findCoreItem(cancelLocationMap, core::CancelLocation::left(), value); +} + +api::CancelLocation Converter::convert(core::CancelLocation value) const +{ + return findApiItem(cancelLocationMap, api::CancelLocation::unspecified, value); +} + double Converter::convertToAlter(int semitones, double cents) { double alter = 0.0; diff --git a/src/private/mx/impl/Converter.h b/src/private/mx/impl/Converter.h index 37052855f..549d92b0c 100644 --- a/src/private/mx/impl/Converter.h +++ b/src/private/mx/impl/Converter.h @@ -6,6 +6,7 @@ #include "mx/api/BarlineData.h" #include "mx/api/ClefData.h" +#include "mx/api/KeyData.h" #include "mx/api/MarkData.h" #include "mx/api/NoteData.h" #include "mx/api/PositionData.h" @@ -17,6 +18,7 @@ #include "mx/core/generated/BarStyle.h" #include "mx/core/generated/BeamValue.h" #include "mx/core/generated/CSSFontSize.h" +#include "mx/core/generated/CancelLocation.h" #include "mx/core/generated/ClefSign.h" #include "mx/core/generated/DynamicsChoice.h" #include "mx/core/generated/FermataShape.h" @@ -155,6 +157,9 @@ class Converter core::KindValue convert(api::ChordKind value) const; api::ChordKind convert(core::KindValue value) const; + core::CancelLocation convert(api::CancelLocation value) const; + api::CancelLocation convert(core::CancelLocation value) const; + static double convertToAlter(int semitones, double cents); static std::pair convertToSemitonesAndCents(double alter); @@ -192,6 +197,7 @@ class Converter const static EnumMap fermataMap; const static EnumMap instrumentMap; const static EnumMap kindMap; + const static EnumMap cancelLocationMap; private: template diff --git a/src/private/mx/impl/MeasureReader.cpp b/src/private/mx/impl/MeasureReader.cpp index 8a4c706e7..0b3d4d6b5 100644 --- a/src/private/mx/impl/MeasureReader.cpp +++ b/src/private/mx/impl/MeasureReader.cpp @@ -484,6 +484,10 @@ std::optional MeasureReader::parseAttributes(const core::Att if (traditionalKey.cancel().has_value()) { keyData.cancel = traditionalKey.cancel()->value().value(); + if (traditionalKey.cancel()->location().has_value()) + { + keyData.cancelLocation = myConverter.convert(*traditionalKey.cancel()->location()); + } } if (traditionalKey.mode().has_value()) diff --git a/src/private/mx/impl/PropertiesWriter.cpp b/src/private/mx/impl/PropertiesWriter.cpp index a580d19c3..09e44b10d 100644 --- a/src/private/mx/impl/PropertiesWriter.cpp +++ b/src/private/mx/impl/PropertiesWriter.cpp @@ -123,6 +123,11 @@ void PropertiesWriter::writeTraditionalKey(const api::KeyData &inKeyData, core:: { core::Cancel cancel{}; cancel.setValue(core::Fifths{inKeyData.cancel}); + if (inKeyData.cancelLocation != api::CancelLocation::unspecified) + { + Converter converter; + cancel.setLocation(converter.convert(inKeyData.cancelLocation)); + } tkg.setCancel(cancel); } diff --git a/src/private/mxtest/api/KeyDataTest.cpp b/src/private/mxtest/api/KeyDataTest.cpp index 4f64c77b8..b1f43f23b 100644 --- a/src/private/mxtest/api/KeyDataTest.cpp +++ b/src/private/mxtest/api/KeyDataTest.cpp @@ -10,6 +10,7 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/DocumentManager.h" #include "mx/core/generated/Attributes.h" +#include "mx/core/generated/Cancel.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/Key.h" #include "mx/core/generated/KeyChoice.h" @@ -19,6 +20,7 @@ #include "mx/core/generated/PartwisePart.h" #include "mx/core/generated/ScorePartwise.h" #include "mx/core/generated/TraditionalKeyGroup.h" +#include "mxtest/api/TestHelpers.h" using namespace std; using namespace mx::api; @@ -425,4 +427,157 @@ TEST(IsInitialized, KeyComponent) CHECK(k.accidental == Accidental{}); } +TEST(CancelLocationBeforeBarline, KeyData) +{ + // https://github.com/webern/mx/issues/272 + KeyData key; + key.fifths = 0; + key.cancel = 2; + key.cancelLocation = CancelLocation::beforeBarline; + + const auto original = putKeyInScore(key); + auto &docMgr = DocumentManager::getInstance(); + const auto originalIdResult = docMgr.createFromScore(original); + REQUIRE(originalIdResult.ok()); + const int originalId = originalIdResult.value(); + const mx::core::DocumentPtr corePtr = docMgr.getDocument(originalId); + + const auto &coreKey = getFirstCoreKey(corePtr); + const auto &coreKeyChoice = coreKey.choice(); + CHECK(coreKeyChoice.isTraditionalKey()); + const auto &coreTraditionalKey = coreKeyChoice.asTraditionalKey(); + + // check that the cancel and its location were written + REQUIRE(coreTraditionalKey.cancel().has_value()); + CHECK_EQUAL(2, coreTraditionalKey.cancel()->value().value()); + CHECK(coreTraditionalKey.cancel()->location().has_value()) + if (coreTraditionalKey.cancel()->location().has_value()) + { + CHECK(core::CancelLocation::Tag::beforeBarline == coreTraditionalKey.cancel()->location()->tag()); + } + + // serialize and deserialize + std::stringstream xml; + docMgr.writeToStream(originalId, xml); + docMgr.destroyDocument(originalId); + CHECK(xml.str().find("location=\"before-barline\"") != std::string::npos); + std::istringstream iss{xml.str()}; + const auto deserializedIdResult = docMgr.createFromStream(iss); + REQUIRE(deserializedIdResult.ok()); + const int deserializedId = deserializedIdResult.value(); + const auto deserializedScoreResult = docMgr.getData(deserializedId); + docMgr.destroyDocument(deserializedId); + REQUIRE(deserializedScoreResult.ok()); + const auto &deserializedScore = deserializedScoreResult.value(); + const auto &deserializedKeys = deserializedScore.parts.at(0).measures.at(0).keys; + CHECK_EQUAL(1, deserializedKeys.size()) + const auto deserializedKey = deserializedKeys.at(0); + + CHECK_EQUAL(key.cancel, deserializedKey.cancel); + CHECK_EQUAL(CancelLocation::beforeBarline, deserializedKey.cancelLocation); +} + +TEST(CancelLocationUnspecified, KeyData) +{ + // when cancelLocation is unspecified, no location attribute is written + KeyData key; + key.fifths = 0; + key.cancel = -3; + + const auto original = putKeyInScore(key); + auto &docMgr = DocumentManager::getInstance(); + const auto originalIdResult = docMgr.createFromScore(original); + REQUIRE(originalIdResult.ok()); + const int originalId = originalIdResult.value(); + const mx::core::DocumentPtr corePtr = docMgr.getDocument(originalId); + + const auto &coreKey = getFirstCoreKey(corePtr); + const auto &coreKeyChoice = coreKey.choice(); + CHECK(coreKeyChoice.isTraditionalKey()); + const auto &coreTraditionalKey = coreKeyChoice.asTraditionalKey(); + + REQUIRE(coreTraditionalKey.cancel().has_value()); + CHECK(!coreTraditionalKey.cancel()->location().has_value()) + + // serialize and deserialize + std::stringstream xml; + docMgr.writeToStream(originalId, xml); + docMgr.destroyDocument(originalId); + CHECK(xml.str().find("location=") == std::string::npos); + std::istringstream iss{xml.str()}; + const auto deserializedIdResult = docMgr.createFromStream(iss); + REQUIRE(deserializedIdResult.ok()); + const int deserializedId = deserializedIdResult.value(); + const auto deserializedScoreResult = docMgr.getData(deserializedId); + docMgr.destroyDocument(deserializedId); + REQUIRE(deserializedScoreResult.ok()); + const auto &deserializedScore = deserializedScoreResult.value(); + const auto &deserializedKeys = deserializedScore.parts.at(0).measures.at(0).keys; + CHECK_EQUAL(1, deserializedKeys.size()) + const auto deserializedKey = deserializedKeys.at(0); + + CHECK_EQUAL(key.cancel, deserializedKey.cancel); + CHECK_EQUAL(CancelLocation::unspecified, deserializedKey.cancelLocation); +} + +TEST(CancelLocationFromXml, KeyData) +{ + // mirrors data/synthetic/cancel.location.3.0.xml, but with location="right" + const std::string xml = R"( + + + + P + + + + + + 1 + + 2 + 0 + major + + + + + C + 4 + + 1 + quarter + + + + +)"; + const auto score = mxtest::fromXml(xml); + REQUIRE(!score.parts.empty()); + REQUIRE(!score.parts.at(0).measures.empty()); + const auto &keys = score.parts.at(0).measures.at(0).keys; + REQUIRE(!keys.empty()); + CHECK_EQUAL(2, keys.at(0).cancel); + CHECK_EQUAL(CancelLocation::right, keys.at(0).cancelLocation); +} + +TEST(KeyDataEquality_change_cancelLocation, KeyData) +{ + KeyData key1; + key1.fifths = 4; + key1.cancel = -1; + key1.cancelLocation = CancelLocation::left; + key1.mode = KeyMode::major; + key1.staffIndex = 0; + key1.tickTimePosition = 13; + auto key2 = key1; + CHECK(key1 == key2); + CHECK(!(key1 != key2)); + + // change one thing + key1.cancelLocation = CancelLocation::right; + CHECK(key1 != key2); + CHECK(!(key1 == key2)); +} + #endif diff --git a/src/private/mxtest/api/roundtrip-baseline.txt b/src/private/mxtest/api/roundtrip-baseline.txt index 675f6147a..e210c8a41 100644 --- a/src/private/mxtest/api/roundtrip-baseline.txt +++ b/src/private/mxtest/api/roundtrip-baseline.txt @@ -191,3 +191,9 @@ lysuite/ly46c_Midmeasure_Clef.xml # repeat's times attribute survives the round-trip. lysuite/ly45a_SimpleRepeat.xml lysuite/ly45c_RepeatMultipleTimes.xml + +# Unblocked by #272 (key cancel location round-trip). The reader dropped the +# cancel element's location attribute and the writer never emitted it. Added +# KeyData::cancelLocation (CancelLocation::unspecified when absent) so the +# attribute survives the round-trip. +synthetic/cancel.location.3.0.xml