Honour an explicit dynamics="0" on MusicXML note import

The dynamics attribute of the <note> element expresses the note velocity
as a percentage of the MIDI 1.0 default forte value of 90. Positive
values were imported correctly, but an explicit dynamics="0" (a silent
note, e.g. a ghost note or an unpitched gesture with an x notehead) was
indistinguishable from an absent attribute and silently dropped, so the
note played at full default velocity.

Detect the attribute's presence instead of testing the computed value,
and clamp the velocity to [1, 127]: the score model reserves velocity 0
for "unset", so 1 is the lowest representable (effectively silent)
value, and the upper bound matches the clamping already applied to
direction-level <sound dynamics> values.

Resolves: #33803

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: jonashogstrom

src/importexport/musicxml/internal/import/importmusicxmlpass2.cpp

@@ -6929,7 +6929,11 @@ Note* MusicXmlParserPass2::note(const String& partId,
     Color beamColor;
     bool noteheadParentheses = false;
     String noteheadFilled;
-    int velocity = round(m_e.doubleAttribute("dynamics") * 0.9);
+    // velocity as a percentage of the MIDI 1.0 default forte value of 90;
+    // an explicit dynamics="0" means a silent note, which the score model can
+    // only represent as velocity 1 (velocity 0 means "unset")
+    const bool hasDynamics = m_e.hasAttribute("dynamics");
+    const int velocity = std::clamp(int(round(m_e.doubleAttribute("dynamics") * 0.9)), 1, 127);
     bool graceSlash = false;
     bool printObject = m_e.asciiAttribute("print-object") != "no";
     bool printLyric = (printObject && m_e.asciiAttribute("print-lyric") != "no") || m_e.asciiAttribute("print-lyric") == "yes";
@@ -7260,7 +7264,7 @@ Note* MusicXmlParserPass2::note(const String& partId,
             }
         }
 
-        if (velocity > 0) {
+        if (hasDynamics) {
             note->setUserVelocity(velocity);
         }
 

src/importexport/musicxml/tests/data/testNoteDynamics.xml

@@ -0,0 +1,73 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<score-partwise version="4.0">
+  <movement-title>Note dynamics attribute</movement-title>
+  <part-list>
+    <score-part id="P1">
+      <part-name>Voice</part-name>
+    </score-part>
+  </part-list>
+  <part id="P1">
+    <measure number="1">
+      <attributes>
+        <divisions>480</divisions>
+        <key>
+          <fifths>0</fifths>
+        </key>
+        <time>
+          <beats>5</beats>
+          <beat-type>4</beat-type>
+        </time>
+        <clef>
+          <sign>G</sign>
+          <line>2</line>
+        </clef>
+      </attributes>
+      <note>
+        <pitch>
+          <step>C</step>
+          <octave>4</octave>
+        </pitch>
+        <duration>480</duration>
+        <voice>1</voice>
+        <type>quarter</type>
+      </note>
+      <note dynamics="0">
+        <pitch>
+          <step>D</step>
+          <octave>4</octave>
+        </pitch>
+        <duration>480</duration>
+        <voice>1</voice>
+        <type>quarter</type>
+        <notehead>x</notehead>
+      </note>
+      <note dynamics="50">
+        <pitch>
+          <step>E</step>
+          <octave>4</octave>
+        </pitch>
+        <duration>480</duration>
+        <voice>1</voice>
+        <type>quarter</type>
+      </note>
+      <note dynamics="200">
+        <pitch>
+          <step>F</step>
+          <octave>4</octave>
+        </pitch>
+        <duration>480</duration>
+        <voice>1</voice>
+        <type>quarter</type>
+      </note>
+      <note>
+        <pitch>
+          <step>G</step>
+          <octave>4</octave>
+        </pitch>
+        <duration>480</duration>
+        <voice>1</voice>
+        <type>quarter</type>
+      </note>
+    </measure>
+  </part>
+</score-partwise>

src/importexport/musicxml/tests/musicxml_tests.cpp

@@ -23,7 +23,11 @@
 #include <gtest/gtest.h>
 
 #include "engraving/engravingerrors.h"
+#include "engraving/dom/chord.h"
 #include "engraving/dom/masterscore.h"
+#include "engraving/dom/measure.h"
+#include "engraving/dom/note.h"
+#include "engraving/dom/segment.h"
 
 #include "settings.h"
 #include "importexport/musicxml/imusicxmlconfiguration.h"
@@ -1024,6 +1028,34 @@ TEST_F(MusicXml_Tests, noteAttributes3) {
 TEST_F(MusicXml_Tests, noteColor) {
     musicXmlIoTest("testNoteColor");
 }
+TEST_F(MusicXml_Tests, noteDynamics) {
+    // The dynamics attribute of <note> maps to the note's velocity as a
+    // percentage of the MIDI 1.0 default forte value of 90. An explicit
+    // dynamics="0" (a silent note) must not be confused with an absent
+    // attribute; since velocity 0 means "unset" in the score model, the
+    // imported value is clamped to [1, 127].
+    MScore::debugMode = true;
+
+    MasterScore* score = readScore(XML_IO_DATA_DIR + u"testNoteDynamics.xml");
+    ASSERT_TRUE(score);
+
+    std::vector<int> velocities;
+    for (const Segment* s = score->firstMeasure()->first(SegmentType::ChordRest); s; s = s->next1(SegmentType::ChordRest)) {
+        const EngravingItem* el = s->element(0);
+        if (el && el->isChord()) {
+            velocities.push_back(toChord(el)->notes().front()->userVelocity());
+        }
+    }
+
+    ASSERT_EQ(velocities.size(), 5u);
+    EXPECT_EQ(velocities.at(0), 0);   // no dynamics attribute: unset
+    EXPECT_EQ(velocities.at(1), 1);   // dynamics="0": silent, clamped to the lowest representable value
+    EXPECT_EQ(velocities.at(2), 45);  // dynamics="50": 50% of forte (90)
+    EXPECT_EQ(velocities.at(3), 127); // dynamics="200": clamped to the MIDI maximum
+    EXPECT_EQ(velocities.at(4), 0);   // no dynamics attribute: unset
+
+    delete score;
+}
 TEST_F(MusicXml_Tests, noteheadNames) {
     musicXmlIoTest("testNoteheadNames");
 }