feat: artificial and natural harmonic detail in mx::api

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 <harmonic> 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<std::monostate, TremoloMarkData, ArpeggiateMarkData, NonArpeggiateMarkData, OtherMarkData,
-                 StandardDynamic, CompoundDynamicsData, OtherNotationMarkData>
+                 StandardDynamic, CompoundDynamicsData, OtherNotationMarkData, HarmonicMarkData>
         myValue;
 };
 

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<TremoloMarkData>(myValue))
@@ -84,6 +88,10 @@ MarkDataChoice::Kind MarkDataChoice::kind() const
     {
         return Kind::otherNotation;
     }
+    if (std::holds_alternative<HarmonicMarkData>(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<HarmonicMarkData>(myValue);
+}
+
+const HarmonicMarkData MarkDataChoice::harmonic() const
+{
+    if (const auto *value = std::get_if<HarmonicMarkData>(&myValue))
+    {
+        return *value;
+    }
+    return HarmonicMarkData{};
+}
+
 bool MarkDataChoice::operator==(const MarkDataChoice &other) const
 {
     return myValue == other.myValue;

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;
     }

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<const core::TechnicalChoice> 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: {

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 <harmonic/> 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 <caesura/> must not acquire a "normal" text value on write.

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
+# <harmonic> 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