diff --git a/docs/ai/api-feature-audit.md b/docs/ai/api-feature-audit.md index e8bea525a..5b5feec26 100644 --- a/docs/ai/api-feature-audit.md +++ b/docs/ai/api-feature-audit.md @@ -175,7 +175,7 @@ so these boxes are status markers, not clickable; the live checklist is the pare | [x] | microtonal accidental arrow/double variants | 1b(6) | #182 | | [x] | notehead `circled` and `other` | 1b(7) | #183 | | [x] | instrument-sound 4.0 sound ids | 1b(8) | #184 | -| [ ] | technical marks with payloads (`fingering`, `pluck`, `bend`, ...) | 1c | #185 | +| [x] | technical marks with payloads (`fingering`, `pluck`, `bend`, ...) | 1c | #185 | | [x] | read `` per-measure layout | 2(1) | #186 | | [x] | `` gaps (credit-image, no-words credits, multiple credit-type) | 2(2) | #187 | | [ ] | read and write `` | 2(3) | #188 | diff --git a/src/include/mx/api/MarkData.h b/src/include/mx/api/MarkData.h index 51ecccfc2..8d95c3e88 100644 --- a/src/include/mx/api/MarkData.h +++ b/src/include/mx/api/MarkData.h @@ -137,8 +137,8 @@ enum class MarkType harmonic, openString, thumbPosition, - // fingering, - // pluck, + fingering, ///< carries text (e.g. "1", "2-3") in MarkData::name plus optional substitution/alternate flags + pluck, ///< carries text (e.g. "p", "i", "m", "a") in MarkData::name doubleTongue, tripleTongue, stopped, @@ -244,6 +244,11 @@ struct MarkData bool hasMordentApproach; Placement mordentDeparture; bool hasMordentDeparture; + // Fingering payload attributes (MusicXML 'substitution' and + // 'alternate'). Only meaningful when markType == MarkType::fingering. The + // fingering text itself (e.g. "1", "2-3") is carried in 'name'. + Bool fingeringSubstitution; + Bool fingeringAlternate; MarkData(); MarkData(MarkType inMarkType); @@ -262,6 +267,8 @@ MXAPI_EQUALS_MEMBER(mordentApproach) MXAPI_EQUALS_MEMBER(hasMordentApproach) MXAPI_EQUALS_MEMBER(mordentDeparture) MXAPI_EQUALS_MEMBER(hasMordentDeparture) +MXAPI_EQUALS_MEMBER(fingeringSubstitution) +MXAPI_EQUALS_MEMBER(fingeringAlternate) MXAPI_EQUALS_END; MXAPI_NOT_EQUALS_AND_VECTORS(MarkData); } // namespace api diff --git a/src/private/mx/api/MarkData.cpp b/src/private/mx/api/MarkData.cpp index a2fc9ca55..dee51df87 100644 --- a/src/private/mx/api/MarkData.cpp +++ b/src/private/mx/api/MarkData.cpp @@ -150,12 +150,12 @@ bool isMarkTechnical(MarkType markType) (markType == MarkType::openString) || (markType == MarkType::thumbPosition) || (markType == MarkType::doubleTongue) || (markType == MarkType::tripleTongue) || (markType == MarkType::stopped) || (markType == MarkType::snapPizzicato) || (markType == MarkType::fret) || - (markType == MarkType::string_) || (markType == MarkType::heel) || (markType == MarkType::toe) || - (markType == MarkType::fingernails) || (markType == MarkType::hole) || (markType == MarkType::arrow) || - (markType == MarkType::handbell) || (markType == MarkType::brassBend) || (markType == MarkType::flip) || - (markType == MarkType::smear) || (markType == MarkType::open) || (markType == MarkType::halfMuted) || - (markType == MarkType::harmonMute) || (markType == MarkType::golpe) || - (markType == MarkType::otherTechnical); + (markType == MarkType::string_) || (markType == MarkType::fingering) || (markType == MarkType::pluck) || + (markType == MarkType::heel) || (markType == MarkType::toe) || (markType == MarkType::fingernails) || + (markType == MarkType::hole) || (markType == MarkType::arrow) || (markType == MarkType::handbell) || + (markType == MarkType::brassBend) || (markType == MarkType::flip) || (markType == MarkType::smear) || + (markType == MarkType::open) || (markType == MarkType::halfMuted) || (markType == MarkType::harmonMute) || + (markType == MarkType::golpe) || (markType == MarkType::otherTechnical); } bool isMarkTremolo(MarkType markType) @@ -226,14 +226,16 @@ int numTremoloSlashes(MarkType markType) MarkData::MarkData() : markType(MarkType::unspecified), name{}, tickTimePosition{0}, printData{}, positionData{}, mordentLong{Bool::no}, hasMordentLong{false}, mordentApproach{Placement::unspecified}, hasMordentApproach{false}, - mordentDeparture{Placement::unspecified}, hasMordentDeparture{false} + mordentDeparture{Placement::unspecified}, hasMordentDeparture{false}, fingeringSubstitution{Bool::unspecified}, + fingeringAlternate{Bool::unspecified} { } MarkData::MarkData(MarkType inMarkType) : markType(inMarkType), name{}, tickTimePosition{0}, printData{}, positionData{}, mordentLong{Bool::no}, hasMordentLong{false}, mordentApproach{Placement::unspecified}, hasMordentApproach{false}, - mordentDeparture{Placement::unspecified}, hasMordentDeparture{false} + mordentDeparture{Placement::unspecified}, hasMordentDeparture{false}, fingeringSubstitution{Bool::unspecified}, + fingeringAlternate{Bool::unspecified} { impl::Converter converter; if (isMarkDynamic(markType)) @@ -253,7 +255,8 @@ MarkData::MarkData(MarkType inMarkType) MarkData::MarkData(Placement inPlacement, MarkType inMarkType) : markType(inMarkType), name{}, tickTimePosition{0}, printData{}, positionData{}, mordentLong{Bool::no}, hasMordentLong{false}, mordentApproach{Placement::unspecified}, hasMordentApproach{false}, - mordentDeparture{Placement::unspecified}, hasMordentDeparture{false} + mordentDeparture{Placement::unspecified}, hasMordentDeparture{false}, fingeringSubstitution{Bool::unspecified}, + fingeringAlternate{Bool::unspecified} { positionData.placement = inPlacement; impl::Converter converter; diff --git a/src/private/mx/impl/Converter.cpp b/src/private/mx/impl/Converter.cpp index 8fafda481..3a1fd4dc0 100644 --- a/src/private/mx/impl/Converter.cpp +++ b/src/private/mx/impl/Converter.cpp @@ -293,9 +293,8 @@ const Converter::EnumMap Converter:: {core::TechnicalChoice::Kind::harmonic, api::MarkType::harmonic}, {core::TechnicalChoice::Kind::openString, api::MarkType::openString}, {core::TechnicalChoice::Kind::thumbPosition, api::MarkType::thumbPosition}, - // { core::TechnicalChoice::Kind::fingering, - // api::MarkType::unspecified }, { - // core::TechnicalChoice::Kind::pluck, api::MarkType::unspecified }, + {core::TechnicalChoice::Kind::fingering, api::MarkType::fingering}, + {core::TechnicalChoice::Kind::pluck, api::MarkType::pluck}, {core::TechnicalChoice::Kind::doubleTongue, api::MarkType::doubleTongue}, {core::TechnicalChoice::Kind::tripleTongue, api::MarkType::tripleTongue}, {core::TechnicalChoice::Kind::stopped, api::MarkType::stopped}, diff --git a/src/private/mx/impl/NotationsWriter.cpp b/src/private/mx/impl/NotationsWriter.cpp index c96c0b40d..ac4cc5048 100644 --- a/src/private/mx/impl/NotationsWriter.cpp +++ b/src/private/mx/impl/NotationsWriter.cpp @@ -16,6 +16,7 @@ #include "mx/core/generated/EmptyTrillSound.h" #include "mx/core/generated/Fermata.h" #include "mx/core/generated/FermataShape.h" +#include "mx/core/generated/Fingering.h" #include "mx/core/generated/Fret.h" #include "mx/core/generated/Handbell.h" #include "mx/core/generated/HandbellValue.h" @@ -32,6 +33,7 @@ #include "mx/core/generated/OrnamentsGroup.h" #include "mx/core/generated/OrnamentsGroupChoice.h" #include "mx/core/generated/OtherPlacementText.h" +#include "mx/core/generated/PlacementText.h" #include "mx/core/generated/ShowTuplet.h" #include "mx/core/generated/Slur.h" #include "mx/core/generated/String.h" @@ -717,6 +719,28 @@ void NotationsWriter::addTechnical(const api::MarkData &mark, core::Technical &o outTechnical.addChoice(core::TechnicalChoice::string(s)); break; } + case core::TechnicalChoice::Kind::fingering: { + core::Fingering fingering; + setAttributesFromPositionData(mark.positionData, fingering); + fingering.setValue(mark.name); + if (mark.fingeringSubstitution != api::Bool::unspecified) + { + fingering.setSubstitution(myConverter.convert(mark.fingeringSubstitution)); + } + if (mark.fingeringAlternate != api::Bool::unspecified) + { + fingering.setAlternate(myConverter.convert(mark.fingeringAlternate)); + } + outTechnical.addChoice(core::TechnicalChoice::fingering(fingering)); + break; + } + case core::TechnicalChoice::Kind::pluck: { + core::PlacementText pt; + setAttributesFromPositionData(mark.positionData, pt); + pt.setValue(mark.name); + outTechnical.addChoice(core::TechnicalChoice::pluck(pt)); + break; + } case core::TechnicalChoice::Kind::heel: { core::HeelToe ht; setAttributesFromPositionData(mark.positionData, ht); diff --git a/src/private/mx/impl/TechnicalFunctions.cpp b/src/private/mx/impl/TechnicalFunctions.cpp index b7339bb70..e822ccb52 100644 --- a/src/private/mx/impl/TechnicalFunctions.cpp +++ b/src/private/mx/impl/TechnicalFunctions.cpp @@ -7,11 +7,13 @@ #include "mx/core/generated/ArrowChoice.h" #include "mx/core/generated/ArrowChoiceGroup.h" #include "mx/core/generated/ArrowDirection.h" +#include "mx/core/generated/Fingering.h" #include "mx/core/generated/Handbell.h" #include "mx/core/generated/HandbellValue.h" #include "mx/core/generated/Hole.h" #include "mx/core/generated/HoleClosed.h" #include "mx/core/generated/HoleClosedValue.h" +#include "mx/core/generated/PlacementText.h" #include "mx/core/generated/Technical.h" #include "mx/core/generated/TechnicalChoice.h" #include "mx/impl/Converter.h" @@ -162,11 +164,25 @@ bool TechnicalFunctions::parseTechicalMark(const core::TechnicalChoice &techical outMarkData.name = "thumb-position"; return true; } - case core::TechnicalChoice::Kind::fingering: - return false; + case core::TechnicalChoice::Kind::fingering: { + const auto &fingering = techicalChoice.asFingering(); + parseMarkDataAttributes(fingering, outMarkData); + outMarkData.name = fingering.value(); + Converter converter; + if (fingering.substitution().has_value()) + { + outMarkData.fingeringSubstitution = converter.convert(fingering.substitution().value()); + } + if (fingering.alternate().has_value()) + { + outMarkData.fingeringAlternate = converter.convert(fingering.alternate().value()); + } + return true; + } case core::TechnicalChoice::Kind::pluck: { - parseMarkDataAttributes(techicalChoice.asPluck(), outMarkData); - outMarkData.name = "pluck"; + const auto &pluck = techicalChoice.asPluck(); + parseMarkDataAttributes(pluck, outMarkData); + outMarkData.name = pluck.value(); return true; } case core::TechnicalChoice::Kind::doubleTongue: { diff --git a/src/private/mxtest/api/NoteDataTest.cpp b/src/private/mxtest/api/NoteDataTest.cpp index b732c7e70..c777c0aaa 100644 --- a/src/private/mxtest/api/NoteDataTest.cpp +++ b/src/private/mxtest/api/NoteDataTest.cpp @@ -312,6 +312,81 @@ TEST(technical, NoteData) T_END; +// Issue #185: technical marks with text payloads. carries text +// (e.g. "1", "2-3") plus substitution/alternate attributes; carries +// text (e.g. "p", "i", "m", "a"). Both must survive an XML round trip. +TEST(technical_fingering_pluck_roundtrip, NoteData) +{ + ScoreData score; + 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]; + voice.notes.emplace_back(); + auto ¬e = voice.notes.back(); + + // a plain fingering "1" + note.noteAttachmentData.marks.emplace_back(Placement::above, MarkType::fingering); + note.noteAttachmentData.marks.back().name = "1"; + + // a fingering "2-3" with substitution=yes and alternate=no + note.noteAttachmentData.marks.emplace_back(Placement::below, MarkType::fingering); + note.noteAttachmentData.marks.back().name = "2-3"; + note.noteAttachmentData.marks.back().fingeringSubstitution = Bool::yes; + note.noteAttachmentData.marks.back().fingeringAlternate = Bool::no; + + // a pluck "p" + note.noteAttachmentData.marks.emplace_back(Placement::above, MarkType::pluck); + note.noteAttachmentData.marks.back().name = "p"; + + auto &mgr = DocumentManager::getInstance(); + const auto r1 = mgr.createFromScore(score); + REQUIRE(r1.ok()); + auto docId = r1.value(); + std::stringstream ss; + mgr.writeToStream(docId, ss); + mgr.destroyDocument(docId); + std::istringstream iss{ss.str()}; + const auto r2 = mgr.createFromStream(iss); + REQUIRE(r2.ok()); + docId = r2.value(); + const auto rd = mgr.getData(docId); + REQUIRE(rd.ok()); + const auto oscore = rd.value(); + mgr.destroyDocument(docId); + + const auto &omarks = + oscore.parts.back().measures.back().staves.back().voices.at(0).notes.back().noteAttachmentData.marks; + REQUIRE(omarks.size() == 3); + + auto oIter = omarks.cbegin(); + auto md = *oIter; + CHECK(md.markType == MarkType::fingering); + CHECK_EQUAL("1", md.name); + CHECK(md.positionData.placement == Placement::above); + CHECK(md.fingeringSubstitution == Bool::unspecified); + CHECK(md.fingeringAlternate == Bool::unspecified); + + ++oIter; + md = *oIter; + CHECK(md.markType == MarkType::fingering); + CHECK_EQUAL("2-3", md.name); + CHECK(md.positionData.placement == Placement::below); + CHECK(md.fingeringSubstitution == Bool::yes); + CHECK(md.fingeringAlternate == Bool::no); + + ++oIter; + md = *oIter; + CHECK(md.markType == MarkType::pluck); + CHECK_EQUAL("p", md.name); + CHECK(md.positionData.placement == Placement::above); +} + +T_END; + TEST(technical_import_file, NoteData) { auto &mgr = DocumentManager::getInstance();