feat: add notehead filled and smufl to mx::api

src/include/mx/api/NoteData.h

@@ -13,6 +13,7 @@
 #include "mx/api/PrintData.h"
 
 #include <optional>
+#include <string>
 #include <vector>
 
 namespace mx
@@ -132,6 +133,22 @@ class NoteData
     bool isCue;
 
     Notehead notehead;
+
+    // The <notehead> element's filled attribute: whether the notehead shape is drawn solid or
+    // hollow. Leave it unspecified to get MusicXML's default, which follows the note's duration
+    // (hollow for a half note and longer, solid for shorter). Set yes or no to override that,
+    // for example a solid whole note in a rhythm-notation part.
+    Bool noteheadFilled;
+
+    // The <notehead> element's smufl attribute: the canonical SMuFL glyph name to draw for this
+    // notehead, for example "noteheadSlashHorizontalEnds". It names a glyph directly, so a
+    // notation program can draw a notehead that MusicXML has no value for -- pair it with
+    // Notehead::other -- or narrow one that MusicXML names only broadly, such as
+    // Notehead::cluster. Note-name noteheads (the SMuFL ranges U+E150-U+E1AF and U+EEE0-U+EEFF)
+    // are not written this way; MusicXML spells those out in <notehead-text>. An empty string
+    // names no glyph and is written the same as leaving this empty.
+    std::optional<std::string> noteheadSmufl;
+
     PitchData pitchData; // step, alter, octave, accidental, etc
     int userRequestedVoiceNumber;
 
@@ -196,6 +213,9 @@ MXAPI_EQUALS_MEMBER(tieLetRing)
 MXAPI_EQUALS_MEMBER(isGrace)
 MXAPI_EQUALS_MEMBER(isCue)
 MXAPI_EQUALS_MEMBER(graceSlash)
+MXAPI_EQUALS_MEMBER(notehead)
+MXAPI_EQUALS_MEMBER(noteheadFilled)
+MXAPI_EQUALS_MEMBER(noteheadSmufl)
 MXAPI_EQUALS_MEMBER(pitchData)
 MXAPI_EQUALS_MEMBER(userRequestedVoiceNumber)
 MXAPI_EQUALS_MEMBER(writeStaffNumber)

src/private/mx/api/NoteData.cpp

@@ -11,9 +11,9 @@ namespace api
 NoteData::NoteData()
     : isRest{false}, isMeasureRest{false}, isUnpitched{false}, isDisplayStepOctaveSpecified{false}, isChord{false},
       isTieStart{false}, isTieStop{false}, tieLetRing{}, isGrace{false}, graceSlash{Bool::unspecified}, isCue{false},
-      notehead{Notehead::normal}, pitchData{}, userRequestedVoiceNumber{VALUE_UNSPECIFIED},
-      writeStaffNumber{Bool::unspecified}, stem{Stem::unspecified}, tickTimePosition{0}, durationData{}, beams{},
-      positionData{}, printData{}, noteAttachmentData{}, lyrics{}
+      notehead{Notehead::normal}, noteheadFilled{Bool::unspecified}, noteheadSmufl{}, pitchData{},
+      userRequestedVoiceNumber{VALUE_UNSPECIFIED}, writeStaffNumber{Bool::unspecified}, stem{Stem::unspecified},
+      tickTimePosition{0}, durationData{}, beams{}, positionData{}, printData{}, noteAttachmentData{}, lyrics{}
 {
 }
 } // namespace api

src/private/mx/impl/NoteFunctions.cpp

@@ -93,6 +93,13 @@ api::NoteData NoteFunctions::parseNote() const
 
     myOutNoteData.notehead = converter.convert(reader.getNoteheadValue());
 
+    if (reader.getNoteheadFilled().has_value())
+    {
+        myOutNoteData.noteheadFilled = converter.convert(*reader.getNoteheadFilled());
+    }
+
+    myOutNoteData.noteheadSmufl = reader.getNoteheadSmufl();
+
     if (reader.getIsDurationTypeSpecified())
     {
         myOutNoteData.durationData.durationName = converter.convert(reader.getDurationType());

src/private/mx/impl/NoteReader.cpp

@@ -145,20 +145,21 @@ NoteReader::NoteReader(const core::Note &mxNote)
       myIsNormal(false), myIsGrace(false), myIsCue(false), myIsRest(false), myIsChord(false), myIsMeasureRest(false),
       myIsUnpitched(false), myIsPitch(false), myIsDisplayStepOctaveSpecified(false), myDurationValue(0.0),
       myStep(core::Step::c()), myAlter(0), myCents(0.0), myOctave(4), myStaffNumber(0), myIsStaffSpecified(false),
-      myVoiceNumber(0), myNoteheadValue(core::NoteheadValue::normal()), myDurationType(core::NoteTypeValue::maxima()),
-      myIsDurationTypeSpecified(false), myNumDots(0), myBeams(), myTimeModificationActualNotes(-1),
-      myTimeModificationNormalNotes(-1), myTimeModificationNormalType(core::NoteTypeValue::maxima()),
-      myTimeModificationNormalTypeDots(0), myHasAccidental(false), myAccidental(core::AccidentalValue::natural()),
-      myIsAccidentalParenthetical(false), myIsAccidentalCautionary{false}, myIsAccidentalEditorial{false},
-      myIsAccidentalBracketed{false}, myIsStemSpecified{false}, myStem{}, myIsGraceSlashSpecified{false},
-      myGraceSlash{}, myIsTieStart{false}, myIsTieStop{false}, myHasLyric{false}
+      myVoiceNumber(0), myNoteheadValue(core::NoteheadValue::normal()), myNoteheadFilled{}, myNoteheadSmufl{},
+      myDurationType(core::NoteTypeValue::maxima()), myIsDurationTypeSpecified(false), myNumDots(0), myBeams(),
+      myTimeModificationActualNotes(-1), myTimeModificationNormalNotes(-1),
+      myTimeModificationNormalType(core::NoteTypeValue::maxima()), myTimeModificationNormalTypeDots(0),
+      myHasAccidental(false), myAccidental(core::AccidentalValue::natural()), myIsAccidentalParenthetical(false),
+      myIsAccidentalCautionary{false}, myIsAccidentalEditorial{false}, myIsAccidentalBracketed{false},
+      myIsStemSpecified{false}, myStem{}, myIsGraceSlashSpecified{false}, myGraceSlash{}, myIsTieStart{false},
+      myIsTieStop{false}, myHasLyric{false}
 {
     setNormalGraceCueItems();
     setRestPitchUnpitchedItems();
     setChord();
     setStaffNumber();
     setVoiceNumber();
-    setNoteheadValue();
+    setNoteheadItems();
     setDurationType();
     setNumDots();
     setBeams();
@@ -323,11 +324,20 @@ void NoteReader::setVoiceNumber()
     utility::stringToInt(myNote.editorialVoice().voice()->c_str(), myVoiceNumber);
 }
 
-void NoteReader::setNoteheadValue()
+void NoteReader::setNoteheadItems()
 {
-    if (myNote.notehead().has_value())
+    if (!myNote.notehead().has_value())
     {
-        myNoteheadValue = myNote.notehead()->value();
+        return;
+    }
+
+    const auto &notehead = *myNote.notehead();
+    myNoteheadValue = notehead.value();
+    myNoteheadFilled = notehead.filled();
+
+    if (notehead.smufl().has_value())
+    {
+        myNoteheadSmufl = notehead.smufl()->toString();
     }
 }
 

src/private/mx/impl/NoteReader.h

@@ -7,7 +7,9 @@
 #include "mx/api/LyricData.h"
 #include "mx/core/generated/Note.h"
 
+#include <optional>
 #include <span>
+#include <string>
 #include <vector>
 
 namespace mx
@@ -132,6 +134,16 @@ class NoteReader
         return myNoteheadValue;
     }
 
+    inline const std::optional<core::YesNo> &getNoteheadFilled() const
+    {
+        return myNoteheadFilled;
+    }
+
+    inline const std::optional<std::string> &getNoteheadSmufl() const
+    {
+        return myNoteheadSmufl;
+    }
+
     inline core::NoteTypeValue getDurationType() const
     {
         return myDurationType;
@@ -259,6 +271,8 @@ class NoteReader
     bool myIsStaffSpecified;
     int myVoiceNumber;
     core::NoteheadValue myNoteheadValue;
+    std::optional<core::YesNo> myNoteheadFilled;
+    std::optional<std::string> myNoteheadSmufl;
     core::NoteTypeValue myDurationType;
     bool myIsDurationTypeSpecified;
     int myNumDots;
@@ -289,7 +303,7 @@ class NoteReader
     void setChord();
     void setStaffNumber();
     void setVoiceNumber();
-    void setNoteheadValue();
+    void setNoteheadItems();
     void setDurationType();
     void setNumDots();
     void setBeams();

src/private/mx/impl/NoteWriter.cpp

@@ -20,6 +20,7 @@
 #include "mx/core/generated/NormalNoteGroup.h"
 #include "mx/core/generated/Pitch.h"
 #include "mx/core/generated/Rest.h"
+#include "mx/core/generated/SmuflGlyphName.h"
 #include "mx/core/generated/Syllabic.h"
 #include "mx/core/generated/TextElementData.h"
 #include "mx/core/generated/Tied.h"
@@ -477,12 +478,31 @@ void NoteWriter::setDurationNameAndDots() const
 
 void NoteWriter::setNotehead() const
 {
-    if (myNoteData.notehead != mx::api::Notehead::normal)
+    const bool isFilledSpecified = myNoteData.noteheadFilled != api::Bool::unspecified;
+    // An empty glyph name names no glyph, so it is treated the same as no smufl at all.
+    const bool isSmuflSpecified = myNoteData.noteheadSmufl.has_value() && !myNoteData.noteheadSmufl->empty();
+
+    // <notehead> stays out of the file for a plain notehead, but filled or smufl still needs
+    // the element even when the value itself is 'normal'.
+    if (myNoteData.notehead == api::Notehead::normal && !isFilledSpecified && !isSmuflSpecified)
+    {
+        return;
+    }
+
+    core::Notehead notehead;
+    notehead.setValue(myConverter.convert(myNoteData.notehead));
+
+    if (isFilledSpecified)
     {
-        core::Notehead notehead;
-        notehead.setValue(myConverter.convert(myNoteData.notehead));
-        myOutNote.setNotehead(std::move(notehead));
+        notehead.setFilled(myConverter.convert(myNoteData.noteheadFilled));
     }
+
+    if (isSmuflSpecified)
+    {
+        notehead.setSmufl(core::SmuflGlyphName{*myNoteData.noteheadSmufl});
+    }
+
+    myOutNote.setNotehead(std::move(notehead));
 }
 
 void NoteWriter::setStemDirection() const

src/private/mxtest/api/NoteDataTest.cpp

@@ -1471,6 +1471,150 @@ TEST(noteheadOtherRoundtrip, NoteData)
     CHECK(outNote.notehead == Notehead::other);
 }
 
+T_END;
+
+// A one-note score whose single quarter note is ready for notehead fields to be set on it.
+ScoreData makeNoteheadScore()
+{
+    ScoreData score;
+    score.ticksPerQuarter = 96;
+    score.parts.emplace_back();
+    auto &part = score.parts.back();
+    part.measures.emplace_back();
+    auto &measure = part.measures.back();
+    measure.staves.emplace_back();
+    auto &staff = measure.staves.back();
+    auto &voice = staff.voices[0];
+
+    NoteData note;
+    note.durationData.durationName = DurationName::quarter;
+    note.durationData.durationTimeTicks = 96;
+    voice.notes.push_back(note);
+
+    return score;
+}
+
+const NoteData &firstNoteheadNote(const ScoreData &score)
+{
+    return score.parts.front().measures.front().staves.front().voices.begin()->second.notes.front();
+}
+
+// filled="no" on an otherwise normal notehead: the <notehead> element has to be written even
+// though the notehead value itself is the default.
+TEST(noteheadFilledRoundtrip, NoteData)
+{
+    auto score = makeNoteheadScore();
+    score.parts.back().measures.back().staves.back().voices.at(0).notes.back().noteheadFilled = Bool::no;
+
+    const std::string xml = mxtest::toXml(score);
+    CHECK(xml.find(R"(<notehead filled="no">normal</notehead>)") != std::string::npos);
+
+    const auto outScore = mxtest::fromXml(xml);
+    CHECK(Bool::no == firstNoteheadNote(outScore).noteheadFilled);
+    CHECK(Notehead::normal == firstNoteheadNote(outScore).notehead);
+}
+
+T_END;
+
+// filled="yes" alongside a non-default notehead value.
+TEST(noteheadFilledYesWithShapeRoundtrip, NoteData)
+{
+    auto score = makeNoteheadScore();
+    auto &note = score.parts.back().measures.back().staves.back().voices.at(0).notes.back();
+    note.notehead = Notehead::diamond;
+    note.noteheadFilled = Bool::yes;
+
+    const std::string xml = mxtest::toXml(score);
+    CHECK(xml.find(R"(<notehead filled="yes">diamond</notehead>)") != std::string::npos);
+
+    const auto outScore = mxtest::fromXml(xml);
+    CHECK(Bool::yes == firstNoteheadNote(outScore).noteheadFilled);
+    CHECK(Notehead::diamond == firstNoteheadNote(outScore).notehead);
+}
+
+T_END;
+
+// The smufl attribute names a glyph that MusicXML has no notehead value for; it pairs with the
+// 'other' notehead value.
+TEST(noteheadSmuflRoundtrip, NoteData)
+{
+    auto score = makeNoteheadScore();
+    auto &note = score.parts.back().measures.back().staves.back().voices.at(0).notes.back();
+    note.notehead = Notehead::other;
+    note.noteheadSmufl = "noteheadSlashHorizontalEnds";
+
+    const std::string xml = mxtest::toXml(score);
+    CHECK(xml.find(R"(smufl="noteheadSlashHorizontalEnds")") != std::string::npos);
+
+    const auto outScore = mxtest::fromXml(xml);
+    const auto &outNote = firstNoteheadNote(outScore);
+    CHECK(Notehead::other == outNote.notehead);
+    REQUIRE(outNote.noteheadSmufl.has_value());
+    CHECK_EQUAL("noteheadSlashHorizontalEnds", *outNote.noteheadSmufl);
+}
+
+T_END;
+
+// Both attributes at once, refining a notehead value that MusicXML names only broadly.
+TEST(noteheadFilledAndSmuflRoundtrip, NoteData)
+{
+    auto score = makeNoteheadScore();
+    auto &note = score.parts.back().measures.back().staves.back().voices.at(0).notes.back();
+    note.notehead = Notehead::cluster;
+    note.noteheadFilled = Bool::yes;
+    note.noteheadSmufl = "noteheadClusterSquareBlack";
+
+    const auto outScore = mxtest::fromXml(mxtest::toXml(score));
+    const auto &outNote = firstNoteheadNote(outScore);
+    CHECK(Notehead::cluster == outNote.notehead);
+    CHECK(Bool::yes == outNote.noteheadFilled);
+    REQUIRE(outNote.noteheadSmufl.has_value());
+    CHECK_EQUAL("noteheadClusterSquareBlack", *outNote.noteheadSmufl);
+}
+
+T_END;
+
+// A note that sets neither field writes no <notehead> element at all.
+TEST(noteheadAttributesDefaultToAbsent, NoteData)
+{
+    const auto score = makeNoteheadScore();
+
+    const std::string xml = mxtest::toXml(score);
+    CHECK(xml.find("<notehead") == std::string::npos);
+
+    const auto outScore = mxtest::fromXml(xml);
+    const auto &outNote = firstNoteheadNote(outScore);
+    CHECK(Bool::unspecified == outNote.noteheadFilled);
+    CHECK(!outNote.noteheadSmufl.has_value());
+}
+
+T_END;
+
+// Parse the synthetic notehead file and confirm mx::api surfaces both attributes. This pins the
+// core -> api read path independently of what the writer emits.
+TEST(noteheadSyntheticFileRead, NoteData)
+{
+    const std::string path = mxtest::getResourcesDirectoryPath() + "synthetic/notehead.3.1.xml";
+    auto &docMgr = DocumentManager::getInstance();
+    const auto docIdResult = docMgr.createFromFile(path);
+    REQUIRE(docIdResult.ok());
+    const int docId = docIdResult.value();
+    const auto scoreResult = docMgr.getData(docId);
+    docMgr.destroyDocument(docId);
+    REQUIRE(scoreResult.ok());
+    const auto &score = scoreResult.value();
+    REQUIRE(score.parts.size() == 1);
+    REQUIRE(score.parts.front().measures.size() == 1);
+
+    const auto &outNote = firstNoteheadNote(score);
+    CHECK(Notehead::slash == outNote.notehead);
+    CHECK(Bool::yes == outNote.noteheadFilled);
+    REQUIRE(outNote.noteheadSmufl.has_value());
+    CHECK_EQUAL("noteheadBlack", *outNote.noteheadSmufl);
+}
+
+T_END;
+
 TEST(printObjectNo, NoteData)
 {
     ScoreData score;