fix: write normal-type from DurationData instead of inferring it from sibling notes

src/private/mx/impl/MeasureWriter.cpp

@@ -526,14 +526,13 @@ void MeasureWriter::writeVoices(const api::StaffData &inStaff)
         myHistory.setVoiceIndex(voice.first);
         auto noteIter = voice.second.notes.cbegin();
         auto noteEnd = voice.second.notes.cend();
-        int noteIndex = 0;
 
         if (directionIter != directionEnd)
         {
             writeDirections(directionIter, directionEnd, noteIter, std::cbegin(voice.second.notes), noteEnd);
         }
 
-        for (; noteIter != noteEnd; ++noteIter, ++noteIndex)
+        for (; noteIter != noteEnd; ++noteIter)
         {
             bool isStartOfChord = false;
 
@@ -596,14 +595,9 @@ void MeasureWriter::writeVoices(const api::StaffData &inStaff)
             myPropertiesWriter->flushBuffer();
             writeDirections(directionIter, directionEnd, noteIter, std::cbegin(voice.second.notes), noteEnd);
 
-            NoteWriter writer{apiNote,
-                              myHistory.getCursor(),
-                              myScoreWriter,
-                              myPreviousCursor.isChordActive,
-                              voice.second.notes,
-                              noteIndex,
-                              numVoices,
-                              voice.second.label};
+            NoteWriter writer{
+                apiNote,   myHistory.getCursor(), myScoreWriter, myPreviousCursor.isChordActive, voice.second.notes,
+                numVoices, voice.second.label};
             myOutMeasure.addMusicData(core::MusicDataChoice::note(writer.getNote(isStartOfChord)));
             myHistory.log("addNote cursorTime " + std::to_string(myHistory.getCursor().tickTimePosition) +
                           ", noteTime " + std::to_string(apiNote.tickTimePosition));

src/private/mx/impl/NoteFunctions.cpp

@@ -134,22 +134,14 @@ api::NoteData NoteFunctions::parseNote() const
 
     myOutNoteData.durationData.timeModificationActualNotes = reader.getTimeModificationActualNotes();
     myOutNoteData.durationData.timeModificationNormalNotes = reader.getTimeModificationNormalNotes();
-    myOutNoteData.durationData.timeModificationNormalType = converter.convert(reader.getTimeModificationNormalType());
-    myOutNoteData.durationData.timeModificationNormalTypeDots = reader.getTimeModificationNormalTypeDots();
-
-    const core::NoteTypeValue timeModType = reader.getTimeModificationNormalType();
-    const int timeModTypeDots = reader.getTimeModificationNormalTypeDots();
-    // TODO: should this be ||? Either conjunct alone proves <normal-type> was present (the
-    // reader defaults normalType to the note's own type with 0 dots when absent), so && drops
-    // an explicit undotted <normal-type>, and a dotted one equal to the note's own type, as
-    // "unspecified". Also, the two assignments above are dead stores -- both branches below
-    // overwrite them. Issue candidate.
-    bool isTimeModTypeSpecified = (timeModTypeDots > 0) && (timeModType != reader.getDurationType());
-
-    if (isTimeModTypeSpecified)
+
+    // An absent <normal-type> maps to unspecified rather than to the reader's derived default,
+    // so the writer can tell the difference and does not invent the element on output (#428).
+    if (reader.getIsTimeModificationNormalTypeSpecified())
     {
-        myOutNoteData.durationData.timeModificationNormalType = converter.convert(timeModType);
-        myOutNoteData.durationData.timeModificationNormalTypeDots = timeModTypeDots;
+        myOutNoteData.durationData.timeModificationNormalType =
+            converter.convert(reader.getTimeModificationNormalType());
+        myOutNoteData.durationData.timeModificationNormalTypeDots = reader.getTimeModificationNormalTypeDots();
     }
     else
     {

src/private/mx/impl/NoteReader.cpp

@@ -158,7 +158,8 @@ NoteReader::NoteReader(const core::Note &mxNote)
       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),
+      myIsTimeModificationNormalTypeSpecified(false), 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}
@@ -425,6 +426,7 @@ void NoteReader::setTimeModification()
         const auto &grp = *mxTimeMod.group();
         myTimeModificationNormalType = grp.normalType();
         myTimeModificationNormalTypeDots = static_cast<int>(grp.normalDot().size());
+        myIsTimeModificationNormalTypeSpecified = true;
     }
     else
     {

src/private/mx/impl/NoteReader.h

@@ -191,6 +191,13 @@ class NoteReader
         return myTimeModificationNormalTypeDots;
     }
 
+    // True when the <time-modification> carried a <normal-type> element. When false, the
+    // normal type getters return a default derived from the note's own type.
+    inline bool getIsTimeModificationNormalTypeSpecified() const
+    {
+        return myIsTimeModificationNormalTypeSpecified;
+    }
+
     inline bool getHasAccidental() const
     {
         return myHasAccidental;
@@ -289,6 +296,7 @@ class NoteReader
     int myTimeModificationNormalNotes;
     core::NoteTypeValue myTimeModificationNormalType;
     int myTimeModificationNormalTypeDots;
+    bool myIsTimeModificationNormalTypeSpecified;
     bool myHasAccidental;
     core::AccidentalValue myAccidental;
     bool myIsAccidentalParenthetical;

src/private/mx/impl/NoteWriter.cpp

@@ -39,10 +39,8 @@
 #include "mx/impl/PrintFunctions.h"
 #include "mx/impl/ScoreWriter.h"
 #include "mx/impl/WriteRefusal.h"
-#include "mx/utility/Throw.h"
 
 #include <algorithm>
-#include <cmath>
 
 namespace mx
 {
@@ -69,11 +67,11 @@ core::Syllabic convertLyricSyllabicForNoteWriter(api::LyricSyllabic value)
 
 NoteWriter::NoteWriter(const api::NoteData &inNoteData, const MeasureCursor &inCursor, const ScoreWriter &inScoreWriter,
                        bool isPreviousNoteAChordMember, const std::vector<mx::api::NoteData> &inSiblingNotes,
-                       int inNoteIndex, int inNumVoices, const std::string &inVoiceLabel)
+                       int inNumVoices, const std::string &inVoiceLabel)
     : myNoteData{inNoteData}, myCursor{inCursor}, myScoreWriter{inScoreWriter}, myConverter{},
       myIsPreviousNoteAChordMember{isPreviousNoteAChordMember}, mySiblingNotes{inSiblingNotes},
-      myNoteIndex{inNoteIndex}, myNumVoices{inNumVoices}, myVoiceLabel{inVoiceLabel}, myOutNote{}, myOutFullNoteGroup{},
-      myOutTies{}, myOutTieNotationsChoices{}
+      myNumVoices{inNumVoices}, myVoiceLabel{inVoiceLabel}, myOutNote{}, myOutFullNoteGroup{}, myOutTies{},
+      myOutTieNotationsChoices{}
 {
 }
 
@@ -179,109 +177,24 @@ core::Note NoteWriter::getNote(bool isStartOfChord) const
         timeMod.setActualNotes(myNoteData.durationData.timeModificationActualNotes);
         timeMod.setNormalNotes(myNoteData.durationData.timeModificationNormalNotes);
 
-        // find the tuplet start note and TupletStart object
-        bool isTupletStartFound = false;
-        int tupletStartIndex = myNoteIndex;
-        mx::api::NoteData tupletStartNote{};
-        mx::api::TupletStart tupletStart{};
-
-        for (; tupletStartIndex >= 0 && mySiblingNotes.size() > 0; --tupletStartIndex)
-        {
-            const auto &siblingNote = mySiblingNotes.at(static_cast<size_t>(tupletStartIndex));
-            if (siblingNote.noteAttachmentData.tupletStarts.size() == 1)
-            {
-                isTupletStartFound = true;
-                tupletStartNote = siblingNote;
-                tupletStart = siblingNote.noteAttachmentData.tupletStarts.at(0);
-                break;
-            }
-        }
-
-        // TODO: <normal-type> is recomputed from sibling TupletStart/TupletStop geometry;
-        // DurationData.timeModificationNormalType is never consulted as a fallback. Tuplets
-        // expressed only via <time-modification> (no <tuplet> notations -- legal MusicXML)
-        // hit this no-op throw in release builds and silently lose <normal-type>. Issue candidate.
-        if (!isTupletStartFound)
-        {
-            MX_DEBUG_THROW("tupletStart was not found");
-        }
-
-        // find the tuplet stop note and TupletStop object
-        bool isTupletStopFound = false;
-        int tupletStopIndex = tupletStartIndex;
-        mx::api::NoteData tupletStopNote{};
-        mx::api::TupletStop tupletStop{};
-
-        if (isTupletStartFound)
-        {
-            for (; tupletStopIndex < static_cast<int>(mySiblingNotes.size()); ++tupletStopIndex)
-            {
-                const auto &siblingNote = mySiblingNotes.at(static_cast<size_t>(tupletStopIndex));
-                if (siblingNote.noteAttachmentData.tupletStops.size() == 1)
-                {
-                    isTupletStopFound = true;
-                    tupletStopNote = siblingNote;
-                    tupletStop = siblingNote.noteAttachmentData.tupletStops.at(0);
-                    break;
-                }
-            }
-        }
-
-        if (!isTupletStopFound)
+        // <normal-type> is the author's statement, taken straight from DurationData. When it
+        // is unspecified it is omitted, and MusicXML reads the absence as the note's own
+        // <type> (#428).
+        if (myNoteData.durationData.timeModificationNormalType != api::DurationName::unspecified)
         {
-            MX_DEBUG_THROW("tupletStop was not found");
-        }
+            core::TimeModificationGroup group;
+            group.setNormalType(myConverter.convert(myNoteData.durationData.timeModificationNormalType));
 
-        // calculate the distance between the two
-        if (isTupletStartFound && isTupletStopFound)
-        {
-            const auto tickTimeDistance =
-                (tupletStopNote.tickTimePosition + tupletStopNote.durationData.durationTimeTicks) -
-                tupletStartNote.tickTimePosition;
-
-            if (tickTimeDistance > 0 && tupletStart.normalNumber != 0 && tupletStart.actualNumber != 0)
-            {
-                // calculate the tuplet normal type and dots based on the distance between start and stop and the ratio
-                const double normalLength =
-                    static_cast<double>(tickTimeDistance) / static_cast<double>(tupletStart.normalNumber);
-
-                mx::api::DurationName normalName = mx::api::DurationName::unspecified;
-                int normalDots = 0;
-                const bool isNormalNameAndDotsFound = this->findNormalNameAndDots(normalName, normalDots, normalLength);
-                if (isNormalNameAndDotsFound)
-                {
-                    core::TimeModificationGroup group;
-                    group.setNormalType(myConverter.convert(normalName));
-
-                    std::vector<core::Empty> normalDotsVec;
-                    for (int i = 0; i < normalDots; ++i)
-                    {
-                        normalDotsVec.emplace_back();
-                    }
-                    group.setNormalDot(std::move(normalDotsVec));
-                    timeMod.setGroup(std::move(group));
-                }
-                else
-                {
-                    MX_DEBUG_THROW("this->findNormalNameAndDots could not find what it was looking for. This probably "
-                                   "means that the file has a badly specified tuplet.");
-                }
-            }
-            else
+            std::vector<core::Empty> normalDots;
+            for (int i = 0; i < myNoteData.durationData.timeModificationNormalTypeDots; ++i)
             {
-                MX_DEBUG_THROW("one of these things was not true ( tickTimeDistance > 0 && tupletStart.normalNumber != "
-                               "0 && tupletStart.actualNumber != 0 )");
+                normalDots.emplace_back();
             }
-        }
-        else
-        {
-            MX_DEBUG_THROW("one of these things was not true ( isTupletStartFound && isTupletStopFound )");
+            group.setNormalDot(std::move(normalDots));
+            timeMod.setGroup(std::move(group));
         }
 
         myOutNote.setTimeModification(std::move(timeMod));
-
-        // TODO - decide what happens if the user entered specific tuplet type in the
-        // duration data, possibly remove those fields from duration data.
     }
 
     setLyrics();
@@ -730,85 +643,5 @@ void NoteWriter::setMiscData() const
     editorialVoice.setFootnote(std::move(footnote));
     myOutNote.setEditorialVoice(std::move(editorialVoice));
 }
-
-bool NoteWriter::findNormalNameAndDots(mx::api::DurationName &ioName, int &ioDots, double inTickLength) const
-{
-    const auto equals = [&](double a, double b) { return std::abs(a - b) < 0.0001; };
-
-    const auto isMatch = [&](double durQuarters, int numDots, mx::api::DurationName name) {
-        if (equals(mx::api::applyDots(durQuarters * static_cast<double>(myCursor.ticksPerQuarter), numDots),
-                   inTickLength))
-        {
-            ioName = name;
-            ioDots = numDots;
-            return true;
-        }
-
-        return false;
-    };
-
-    for (int dots = 0; dots < 4; ++dots)
-    {
-
-        if (isMatch(mx::api::DUR_QUARTERS_VALUE_QUARTER, dots, mx::api::DurationName::quarter))
-        {
-            return true;
-        }
-        else if (isMatch(mx::api::DUR_QUARTERS_VALUE_EIGHTH, dots, mx::api::DurationName::eighth))
-        {
-            return true;
-        }
-        else if (isMatch(mx::api::DUR_QUARTERS_VALUE_16TH, dots, mx::api::DurationName::dur16th))
-        {
-            return true;
-        }
-        else if (isMatch(mx::api::DUR_QUARTERS_VALUE_MAXIMA, dots, mx::api::DurationName::maxima))
-        {
-            return true;
-        }
-        else if (isMatch(mx::api::DUR_QUARTERS_VALUE_LONGA, dots, mx::api::DurationName::longa))
-        {
-            return true;
-        }
-        else if (isMatch(mx::api::DUR_QUARTERS_VALUE_BREVE, dots, mx::api::DurationName::breve))
-        {
-            return true;
-        }
-        else if (isMatch(mx::api::DUR_QUARTERS_VALUE_WHOLE, dots, mx::api::DurationName::whole))
-        {
-            return true;
-        }
-        else if (isMatch(mx::api::DUR_QUARTERS_VALUE_HALF, dots, mx::api::DurationName::half))
-        {
-            return true;
-        }
-        else if (isMatch(mx::api::DUR_QUARTERS_VALUE_32ND, dots, mx::api::DurationName::dur32nd))
-        {
-            return true;
-        }
-        else if (isMatch(mx::api::DUR_QUARTERS_VALUE_64TH, dots, mx::api::DurationName::dur64th))
-        {
-            return true;
-        }
-        else if (isMatch(mx::api::DUR_QUARTERS_VALUE_128TH, dots, mx::api::DurationName::dur128th))
-        {
-            return true;
-        }
-        else if (isMatch(mx::api::DUR_QUARTERS_VALUE_256TH, dots, mx::api::DurationName::dur256th))
-        {
-            return true;
-        }
-        else if (isMatch(mx::api::DUR_QUARTERS_VALUE_512TH, dots, mx::api::DurationName::dur512th))
-        {
-            return true;
-        }
-        else if (isMatch(mx::api::DUR_QUARTERS_VALUE_1024TH, dots, mx::api::DurationName::dur1024th))
-        {
-            return true;
-        }
-    }
-
-    return false;
-}
 } // namespace impl
 } // namespace mx

src/private/mx/impl/NoteWriter.h

@@ -25,8 +25,8 @@ class NoteWriter
 {
   public:
     NoteWriter(const api::NoteData &inNoteData, const MeasureCursor &inCursor, const ScoreWriter &inScoreWriter,
-               bool isPreviousNoteAChordMember, const std::vector<mx::api::NoteData> &inSiblingNotes, int inNoteIndex,
-               int inNumVoices, const std::string &inVoiceLabel);
+               bool isPreviousNoteAChordMember, const std::vector<mx::api::NoteData> &inSiblingNotes, int inNumVoices,
+               const std::string &inVoiceLabel);
 
     core::Note getNote(bool isStartOfChord) const;
 
@@ -37,7 +37,6 @@ class NoteWriter
     const Converter myConverter;
     const bool myIsPreviousNoteAChordMember;
     const std::vector<mx::api::NoteData> &mySiblingNotes;
-    const int myNoteIndex;
     const int myNumVoices;
     const std::string &myVoiceLabel;
     mutable core::Note myOutNote;
@@ -57,7 +56,6 @@ class NoteWriter
     void setStemDirection() const;
     void setLyrics() const;
     void setMiscData() const;
-    bool findNormalNameAndDots(mx::api::DurationName &ioName, int &ioDots, double inTickLength) const;
 };
 } // namespace impl
 } // namespace mx