diff --git a/src/include/mx/api/MarkDataChoice.h b/src/include/mx/api/MarkDataChoice.h index 800c6de77..4eff58120 100644 --- a/src/include/mx/api/MarkDataChoice.h +++ b/src/include/mx/api/MarkDataChoice.h @@ -137,6 +137,63 @@ MXAPI_EQUALS_MEMBER(id) MXAPI_EQUALS_END; MXAPI_NOT_EQUALS_AND_VECTORS(OtherNotationMarkData); +// How a harmonic is produced on a string instrument. +enum class HarmonicKind +{ + // Leave the production method unstated, drawing only the circular harmonic symbol. + unspecified, + + // The string sounds open and is touched lightly at a node, so the harmonic's pitch is fixed by + // where the node falls on the open string. + natural, + + // The string is stopped at one pitch and touched lightly a fixed interval above it -- usually a + // fourth, sometimes a major third or a fifth. The stopped note is written as an ordinary + // notehead and the touched note as a diamond (see NoteData::notehead). + artificial, +}; + +// Which of a harmonic's three pitches the note's written notehead states. +enum class HarmonicPitch +{ + // Leave it unstated. Notation that writes only one note for the harmonic, or that relies on + // notehead shape alone to say which pitch is meant, does not need this. + unspecified, + + // The stopped pitch: where the finger presses the string. The lower note of an artificial + // harmonic pair. + basePitch, + + // The lightly touched pitch. The upper, diamond-notehead note of an artificial harmonic pair, + // and the node touched on an open string for a natural harmonic. + touchingPitch, + + // The pitch that actually sounds. Written when the notation states the sounding result + // explicitly, often as a small or parenthesized note above the pair. + soundingPitch, +}; + +// Payload for MarkType::harmonic: the symbol placed on a string-instrument note. +// +// A note in an artificial harmonic pair carries HarmonicKind::artificial along with the +// HarmonicPitch its notehead states -- basePitch on the stopped note, touchingPitch on the diamond +// note -- so each note says what it is on its own. Leaving both fields unspecified draws the plain +// circular harmonic symbol, which is what a natural harmonic most often needs. +struct HarmonicMarkData +{ + // Whether the harmonic is natural or artificial. + HarmonicKind kind = HarmonicKind::unspecified; + + // Which pitch this note's notehead states. + HarmonicPitch pitch = HarmonicPitch::unspecified; +}; + +MXAPI_EQUALS_BEGIN(HarmonicMarkData) +MXAPI_EQUALS_MEMBER(kind) +MXAPI_EQUALS_MEMBER(pitch) +MXAPI_EQUALS_END; +MXAPI_NOT_EQUALS_AND_VECTORS(HarmonicMarkData); + // A variant class that carries data for MarkType values whose payload does not fit MarkData's // common fields. // @@ -163,7 +220,8 @@ class MarkDataChoice otherMark, dynamic, compoundDynamics, - otherNotation + otherNotation, + harmonic }; MarkDataChoice(); @@ -185,6 +243,8 @@ class MarkDataChoice MarkDataChoice(OtherNotationMarkData value); + MarkDataChoice(HarmonicMarkData value); + Kind kind() const; bool isNone() const; bool isTremolo() const; @@ -194,6 +254,7 @@ class MarkDataChoice bool isDynamic() const; bool isCompoundDynamics() const; bool isOtherNotation() const; + bool isHarmonic() const; // Returns a copy of the internally held TremoloMarkData. // @@ -225,11 +286,17 @@ class MarkDataChoice // Returns a copy of the internally held OtherNotationMarkData, or a default value for another kind. const OtherNotationMarkData otherNotation() const; + // Returns a copy of the internally held HarmonicMarkData. + // + // Check isHarmonic() first. If this is not a harmonic payload, a default constructed + // HarmonicMarkData is returned. + const HarmonicMarkData harmonic() const; + bool operator==(const MarkDataChoice &other) const; private: std::variant + StandardDynamic, CompoundDynamicsData, OtherNotationMarkData, HarmonicMarkData> myValue; }; diff --git a/src/private/mx/api/MarkDataChoice.cpp b/src/private/mx/api/MarkDataChoice.cpp index 3303459a8..cadb16412 100644 --- a/src/private/mx/api/MarkDataChoice.cpp +++ b/src/private/mx/api/MarkDataChoice.cpp @@ -54,6 +54,10 @@ MarkDataChoice::MarkDataChoice(OtherNotationMarkData value) : myValue{std::move( { } +MarkDataChoice::MarkDataChoice(HarmonicMarkData value) : myValue{std::move(value)} +{ +} + MarkDataChoice::Kind MarkDataChoice::kind() const { if (std::holds_alternative(myValue)) @@ -84,6 +88,10 @@ MarkDataChoice::Kind MarkDataChoice::kind() const { return Kind::otherNotation; } + if (std::holds_alternative(myValue)) + { + return Kind::harmonic; + } return Kind::none; } @@ -190,6 +198,20 @@ const OtherNotationMarkData MarkDataChoice::otherNotation() const return OtherNotationMarkData{}; } +bool MarkDataChoice::isHarmonic() const +{ + return std::holds_alternative(myValue); +} + +const HarmonicMarkData MarkDataChoice::harmonic() const +{ + if (const auto *value = std::get_if(&myValue)) + { + return *value; + } + return HarmonicMarkData{}; +} + bool MarkDataChoice::operator==(const MarkDataChoice &other) const { return myValue == other.myValue; diff --git a/src/private/mx/impl/NotationsWriter.cpp b/src/private/mx/impl/NotationsWriter.cpp index 9e80f3ff3..6973c3b25 100644 --- a/src/private/mx/impl/NotationsWriter.cpp +++ b/src/private/mx/impl/NotationsWriter.cpp @@ -24,6 +24,8 @@ #include "mx/core/generated/HandbellValue.h" #include "mx/core/generated/HarmonMute.h" #include "mx/core/generated/Harmonic.h" +#include "mx/core/generated/HarmonicChoice.h" +#include "mx/core/generated/HarmonicChoice2.h" #include "mx/core/generated/HeelToe.h" #include "mx/core/generated/Hole.h" #include "mx/core/generated/HoleClosed.h" @@ -802,7 +804,36 @@ void NotationsWriter::addTechnical(const api::MarkData &mark, core::Technical &o } case core::TechnicalChoice::Kind::harmonic: { core::Harmonic h; - setAttributesFromPositionData(mark.positionData, h); + impl::setAttributesFromMarkData(mark, h); + const auto harmonicData = mark.choice.isHarmonic() ? mark.choice.harmonic() : api::HarmonicMarkData{}; + + switch (harmonicData.kind) + { + case api::HarmonicKind::natural: + h.setChoice(core::HarmonicChoice::natural(core::Empty{})); + break; + case api::HarmonicKind::artificial: + h.setChoice(core::HarmonicChoice::artificial(core::Empty{})); + break; + case api::HarmonicKind::unspecified: + break; + } + + switch (harmonicData.pitch) + { + case api::HarmonicPitch::basePitch: + h.setChoice2(core::HarmonicChoice2::basePitch(core::Empty{})); + break; + case api::HarmonicPitch::touchingPitch: + h.setChoice2(core::HarmonicChoice2::touchingPitch(core::Empty{})); + break; + case api::HarmonicPitch::soundingPitch: + h.setChoice2(core::HarmonicChoice2::soundingPitch(core::Empty{})); + break; + case api::HarmonicPitch::unspecified: + break; + } + outTechnical.addChoice(core::TechnicalChoice::harmonic(h)); break; } diff --git a/src/private/mx/impl/TechnicalFunctions.cpp b/src/private/mx/impl/TechnicalFunctions.cpp index fa6abfc8d..b557fe524 100644 --- a/src/private/mx/impl/TechnicalFunctions.cpp +++ b/src/private/mx/impl/TechnicalFunctions.cpp @@ -10,6 +10,9 @@ #include "mx/core/generated/Fingering.h" #include "mx/core/generated/Handbell.h" #include "mx/core/generated/HandbellValue.h" +#include "mx/core/generated/Harmonic.h" +#include "mx/core/generated/HarmonicChoice.h" +#include "mx/core/generated/HarmonicChoice2.h" #include "mx/core/generated/Hole.h" #include "mx/core/generated/HoleClosed.h" #include "mx/core/generated/HoleClosedValue.h" @@ -103,6 +106,42 @@ std::string technicalFunctionsHandbellToSmuflName(const mx::core::HandbellValue } } +api::HarmonicMarkData harmonicMarkDataFromCore(const core::Harmonic &inHarmonic) +{ + api::HarmonicMarkData outData{}; + + if (inHarmonic.choice().has_value()) + { + switch (inHarmonic.choice()->kind()) + { + case core::HarmonicChoice::Kind::natural: + outData.kind = api::HarmonicKind::natural; + break; + case core::HarmonicChoice::Kind::artificial: + outData.kind = api::HarmonicKind::artificial; + break; + } + } + + if (inHarmonic.choice2().has_value()) + { + switch (inHarmonic.choice2()->kind()) + { + case core::HarmonicChoice2::Kind::basePitch: + outData.pitch = api::HarmonicPitch::basePitch; + break; + case core::HarmonicChoice2::Kind::touchingPitch: + outData.pitch = api::HarmonicPitch::touchingPitch; + break; + case core::HarmonicChoice2::Kind::soundingPitch: + outData.pitch = api::HarmonicPitch::soundingPitch; + break; + } + } + + return outData; +} + TechnicalFunctions::TechnicalFunctions(std::span inTechincalChoiceSet, Cursor inCursor) : myTechincalChoiceSet{inTechincalChoiceSet}, myCursor{inCursor} { @@ -148,8 +187,15 @@ bool TechnicalFunctions::parseTechicalMark(const core::TechnicalChoice &techical return true; } case core::TechnicalChoice::Kind::harmonic: { - parseMarkDataAttributes(techicalChoice.asHarmonic(), outMarkData); + const auto &harmonic = techicalChoice.asHarmonic(); + parseMarkDataAttributes(harmonic, outMarkData); outMarkData.name = "harmonic"; + const auto harmonicData = harmonicMarkDataFromCore(harmonic); + if (harmonicData.kind != api::HarmonicKind::unspecified || + harmonicData.pitch != api::HarmonicPitch::unspecified) + { + outMarkData.choice = api::MarkDataChoice{harmonicData}; + } return true; } case core::TechnicalChoice::Kind::openString: { diff --git a/src/private/mxtest/api/MarkRoundTripTest.cpp b/src/private/mxtest/api/MarkRoundTripTest.cpp index 8f43f465e..361a83358 100644 --- a/src/private/mxtest/api/MarkRoundTripTest.cpp +++ b/src/private/mxtest/api/MarkRoundTripTest.cpp @@ -285,6 +285,79 @@ TEST(NonArpeggiateAttributes, MarkRoundTrip) T_END; +TEST(HarmonicArtificialPair, MarkRoundTrip) +{ + // The two notes of an artificial harmonic each state what they are: the stopped note carries + // the base pitch, the diamond-notehead note the touching pitch. + MarkData stopped{Placement::unspecified, MarkType::harmonic}; + HarmonicMarkData stoppedPayload{}; + stoppedPayload.kind = HarmonicKind::artificial; + stoppedPayload.pitch = HarmonicPitch::basePitch; + stopped.choice = stoppedPayload; + + const auto stoppedMarks = roundTripMarkData(stopped); + REQUIRE(stoppedMarks.size() == 1); + CHECK(stoppedMarks.front().markType == MarkType::harmonic); + REQUIRE(stoppedMarks.front().choice.isHarmonic()); + CHECK(stoppedPayload == stoppedMarks.front().choice.harmonic()); + + MarkData touched{Placement::unspecified, MarkType::harmonic}; + HarmonicMarkData touchedPayload{}; + touchedPayload.kind = HarmonicKind::artificial; + touchedPayload.pitch = HarmonicPitch::touchingPitch; + touched.choice = touchedPayload; + + const auto touchedMarks = roundTripMarkData(touched); + REQUIRE(touchedMarks.size() == 1); + REQUIRE(touchedMarks.front().choice.isHarmonic()); + CHECK(touchedPayload == touchedMarks.front().choice.harmonic()); +} + +T_END; + +TEST(HarmonicKindAndPitchCombinations, MarkRoundTrip) +{ + const HarmonicKind kinds[] = {HarmonicKind::unspecified, HarmonicKind::natural, HarmonicKind::artificial}; + const HarmonicPitch pitches[] = {HarmonicPitch::unspecified, HarmonicPitch::basePitch, HarmonicPitch::touchingPitch, + HarmonicPitch::soundingPitch}; + + for (const auto kind : kinds) + { + for (const auto pitch : pitches) + { + if (kind == HarmonicKind::unspecified && pitch == HarmonicPitch::unspecified) + { + continue; // covered by HarmonicDefaultsToNoPayload below + } + + MarkData mark{Placement::unspecified, MarkType::harmonic}; + HarmonicMarkData payload{}; + payload.kind = kind; + payload.pitch = pitch; + mark.choice = payload; + + const auto marks = roundTripMarkData(mark); + REQUIRE(marks.size() == 1); + REQUIRE(marks.front().choice.isHarmonic()); + CHECK(payload == marks.front().choice.harmonic()); + } + } +} + +T_END; + +TEST(HarmonicDefaultsToNoPayload, MarkRoundTrip) +{ + // An undecorated must not acquire a natural/artificial or pitch value, and must + // still round-trip for callers written before the payload existed. + const auto marks = roundTripMark(MarkType::harmonic); + REQUIRE(marks.size() == 1); + CHECK(marks.front().markType == MarkType::harmonic); + CHECK(marks.front().choice.isNone()); +} + +T_END; + TEST(CaesuraEmpty, MarkRoundTrip) { // The common empty form must not acquire a "normal" text value on write. diff --git a/src/private/mxtest/api/roundtrip-baseline.txt b/src/private/mxtest/api/roundtrip-baseline.txt index f4268889c..7f811545b 100644 --- a/src/private/mxtest/api/roundtrip-baseline.txt +++ b/src/private/mxtest/api/roundtrip-baseline.txt @@ -627,3 +627,9 @@ lysuite/ly33i_Ties_NotEnded.xml lysuite/ly61d_Lyrics_Melisma.xml lysuite/ly61f_Lyrics_GracedNotes.xml synthetic/tied.cue.4.0.xml + +# Artificial/natural harmonic detail: HarmonicMarkData in MarkDataChoice carries the +# element's (natural|artificial) and (base-pitch|touching-pitch|sounding-pitch) +# children, which both directions previously dropped. The writer also now emits the +# harmonic's print-object attribute. +synthetic/harmonic.3.0.xml