Browse Source

merge osmd-public: fix in-staff clefs missing or misplaced in 2nd voice or with backup/forward tags

sschmid 3 years ago
parent
commit
48bc9a6fd6

+ 12 - 13
src/MusicalScore/Graphical/VexFlow/VexFlowMeasure.ts

@@ -26,7 +26,6 @@ import {GraphicalVoiceEntry} from "../GraphicalVoiceEntry";
 import {VexFlowVoiceEntry} from "./VexFlowVoiceEntry";
 import {Fraction} from "../../../Common/DataObjects/Fraction";
 import {Voice} from "../../VoiceData/Voice";
-import {LinkedVoice} from "../../VoiceData/LinkedVoice";
 import {EngravingRules} from "../EngravingRules";
 import {OrnamentContainer} from "../../VoiceData/OrnamentContainer";
 import {TechnicalInstruction} from "../../VoiceData/Instructions/TechnicalInstruction";
@@ -1222,7 +1221,7 @@ export class VexFlowMeasure extends GraphicalMeasure {
             if (!voice) {
                 continue;
             }
-            const isMainVoice: boolean = !(voice instanceof LinkedVoice);
+            //const isMainVoice: boolean = !(voice instanceof LinkedVoice);
 
             // add a vexFlow voice for this voice:
             this.vfVoices[voice.VoiceId] = new Vex.Flow.Voice({
@@ -1253,17 +1252,17 @@ export class VexFlowMeasure extends GraphicalMeasure {
                 }
 
                 // check for in-measure clefs:
-                // only add clefs in main voice (to not add them twice)
-                if (isMainVoice) {
-                    const vfse: VexFlowStaffEntry = vexFlowVoiceEntry.parentStaffEntry as VexFlowStaffEntry;
-                    if (vfse && vfse.vfClefBefore) {
-                        // add clef as NoteSubGroup so that we get modifier layouting
-                        const clefModifier: NoteSubGroup = new NoteSubGroup( [vfse.vfClefBefore] );
-                        // The cast is necesary because...vexflow -> see types
-                        if (vexFlowVoiceEntry.vfStaveNote.getCategory && vexFlowVoiceEntry.vfStaveNote.getCategory() === "stavenotes") {
-                            // GhostNotes and other StemmableNotes don't have this function
-                            (vexFlowVoiceEntry.vfStaveNote as Vex.Flow.StaveNote).addModifier(0, clefModifier);
-                        }
+                // Note: we used to only add clefs in main voice to not add them twice,
+                //   but there are many legitimate clefs e.g. in 2nd voices, and this doesn't seem to cause issues.
+                //if (isMainVoice) {
+                const vfse: VexFlowStaffEntry = vexFlowVoiceEntry.parentStaffEntry as VexFlowStaffEntry;
+                if (vfse && vfse.vfClefBefore) {
+                    // add clef as NoteSubGroup so that we get modifier layouting
+                    const clefModifier: NoteSubGroup = new NoteSubGroup( [vfse.vfClefBefore] );
+                    // The cast is necesary because...vexflow -> see types
+                    if (vexFlowVoiceEntry.vfStaveNote.getCategory && vexFlowVoiceEntry.vfStaveNote.getCategory() === "stavenotes") {
+                        // GhostNotes and other StemmableNotes don't have this function
+                        (vexFlowVoiceEntry.vfStaveNote as Vex.Flow.StaveNote).addModifier(0, clefModifier);
                     }
                 }
 

+ 26 - 24
src/MusicalScore/ScoreIO/InstrumentReader.ts

@@ -92,7 +92,7 @@ export class InstrumentReader {
   private activeRhythm: RhythmInstruction;
   private activeClefsHaveBeenInitialized: boolean[];
   private activeKeyHasBeenInitialized: boolean = false;
-  private abstractInstructions: [number, AbstractNotationInstruction][] = [];
+  private abstractInstructions: [number, AbstractNotationInstruction, Fraction][] = [];
   private openChordSymbolContainers: ChordSymbolContainer[] = [];
   private expressionReaders: ExpressionReader[];
   private currentVoiceGenerator: VoiceGenerator;
@@ -460,7 +460,7 @@ export class InstrumentReader {
               throw new MusicSheetReadingException(errorMsg + this.instrument.Name);
             }
           }
-          this.addAbstractInstruction(xmlNode, octavePlusOne, previousNode);
+          this.addAbstractInstruction(xmlNode, octavePlusOne, previousNode, currentFraction.clone());
           if (currentFraction.Equals(new Fraction(0, 1)) &&
             this.isAttributesNodeAtBeginOfMeasure(this.xmlMeasureList[this.currentXmlMeasureIndex], xmlNode)) {
             this.saveAbstractInstructionList(this.instrument.Staves.length, true);
@@ -832,7 +832,7 @@ export class InstrumentReader {
    * @param attrNode
    * @param guitarPro
    */
-  private addAbstractInstruction(attrNode: IXmlElement, guitarPro: boolean, previousNode: IXmlElement): void {
+  private addAbstractInstruction(attrNode: IXmlElement, guitarPro: boolean, previousNode: IXmlElement, currentFraction: Fraction): void {
     if (attrNode.element("divisions")) {
       if (attrNode.elements().length === 1) {
         return;
@@ -929,16 +929,8 @@ export class InstrumentReader {
           }
         }
 
-        // TODO problem: in saveAbstractInstructionList, this is always saved in this.currentStaffEntry.
-        //   so when there's a <forward> or <backup> instruction in <attributes> (which is unfortunate encoding), this gets misplaced.
-        //   so for now we skip it.
-        const skipClefInstruction: boolean = previousNode?.name === "forward";
-          // || previousNode?.name === "backup") && // necessary for clef at beginning of measure/system,
-          //   see sample test_staverepetitions_coda_etc.musicxml, where the bass clef was placed over a previous treble clef
-        if (!skipClefInstruction) {
-          const clefInstruction: ClefInstruction = new ClefInstruction(clefEnum, clefOctaveOffset, line);
-          this.abstractInstructions.push([staffNumber, clefInstruction]);
-        }
+        const clefInstruction: ClefInstruction = new ClefInstruction(clefEnum, clefOctaveOffset, line);
+        this.abstractInstructions.push([staffNumber, clefInstruction, currentFraction]);
       }
     }
     if (attrNode.element("key") !== undefined && this.instrument.MidiInstrumentId !== MidiInstrument.Percussion) {
@@ -977,7 +969,7 @@ export class InstrumentReader {
         }
       }
       const keyInstruction: KeyInstruction = new KeyInstruction(undefined, key, keyEnum);
-      this.abstractInstructions.push([1, keyInstruction]);
+      this.abstractInstructions.push([1, keyInstruction, currentFraction]);
     }
     if (attrNode.element("time")) {
       const timeNode: IXmlElement = attrNode.element("time");
@@ -1058,9 +1050,9 @@ export class InstrumentReader {
           new Fraction(num, denom, 0, false), symbolEnum
         );
         newRhythmInstruction.PrintObject = timePrintObject;
-        this.abstractInstructions.push([1, newRhythmInstruction]);
+        this.abstractInstructions.push([1, newRhythmInstruction, currentFraction]);
       } else {
-        this.abstractInstructions.push([1, new RhythmInstruction(new Fraction(4, 4, 0, false), RhythmSymbolEnum.NONE)]);
+        this.abstractInstructions.push([1, new RhythmInstruction(new Fraction(4, 4, 0, false), RhythmSymbolEnum.NONE), currentFraction]);
       }
     }
   }
@@ -1072,21 +1064,31 @@ export class InstrumentReader {
    */
   private saveAbstractInstructionList(numberOfStaves: number, beginOfMeasure: boolean): void {
     for (let i: number = this.abstractInstructions.length - 1; i >= 0; i--) {
-      const pair: [number, AbstractNotationInstruction] = this.abstractInstructions[i];
-      const key: number = pair[0];
-      const value: AbstractNotationInstruction = pair[1];
+      const instruction: [number, AbstractNotationInstruction, Fraction] = this.abstractInstructions[i];
+      const key: number = instruction[0];
+      const value: AbstractNotationInstruction = instruction[1];
+      const instructionTimestamp: Fraction = instruction[2];
       if (value instanceof ClefInstruction) {
         const clefInstruction: ClefInstruction = <ClefInstruction>value;
         if (this.currentXmlMeasureIndex === 0 || (key <= this.activeClefs.length && clefInstruction !== this.activeClefs[key - 1])) {
-          if (!beginOfMeasure && this.currentStaffEntry !== undefined && !this.currentStaffEntry.hasNotes() && key - 1
-            === this.instrument.Staves.indexOf(this.currentStaffEntry.ParentStaff)) {
+          if (!beginOfMeasure && this.currentStaffEntry !== undefined && !this.currentStaffEntry.hasNotes() &&
+            key - 1 === this.instrument.Staves.indexOf(this.currentStaffEntry.ParentStaff)) {
             const newClefInstruction: ClefInstruction = clefInstruction;
-            newClefInstruction.Parent = this.currentStaffEntry;
-            this.currentStaffEntry.removeFirstInstructionOfTypeClefInstruction();
-            this.currentStaffEntry.Instructions.push(newClefInstruction);
+            const staffEntry: SourceStaffEntry = this.currentStaffEntry;
+            // the instructionTimestamp may differ from the current staffentry's when backup/forward tags are used in the XML.
+            //   in this case, we need to skip placing it at the current entry, and save it for later.
+            if (instructionTimestamp && Math.abs(instructionTimestamp.RealValue - staffEntry.Timestamp.RealValue) > 0.01) {
+              continue; // this instruction should be at a different staffentry/timestamp.
+            }
+            newClefInstruction.Parent = staffEntry;
+            staffEntry.removeFirstInstructionOfTypeClefInstruction();
+            staffEntry.Instructions.push(newClefInstruction);
             this.activeClefs[key - 1] = clefInstruction;
             this.abstractInstructions.splice(i, 1);
           } else if (beginOfMeasure) {
+            if (instructionTimestamp.RealValue !== 0) {
+              continue;
+            }
             let firstStaffEntry: SourceStaffEntry;
             if (this.currentMeasure) {
               const newClefInstruction: ClefInstruction = clefInstruction;

+ 159 - 0
test/data/test_clefs_instaff_secondvoice_backup_forward.musicxml

@@ -0,0 +1,159 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE score-partwise PUBLIC "-//Recordare//DTD MusicXML 3.1 Partwise//EN" "http://www.musicxml.org/dtds/partwise.dtd">
+<score-partwise version="3.1">
+  <identification>
+    <encoding>
+      <software>MuseScore 3.6.2</software>
+      <encoding-date>2021-11-26</encoding-date>
+      <supports element="accidental" type="yes"/>
+      <supports element="beam" type="yes"/>
+      <supports element="print" attribute="new-page" type="yes" value="yes"/>
+      <supports element="print" attribute="new-system" type="yes" value="yes"/>
+      <supports element="stem" type="yes"/>
+      </encoding>
+    </identification>
+  <defaults>
+    <scaling>
+      <millimeters>7</millimeters>
+      <tenths>40</tenths>
+      </scaling>
+    <page-layout>
+      <page-height>1697.14</page-height>
+      <page-width>1200</page-width>
+      <page-margins type="even">
+        <left-margin>85.7143</left-margin>
+        <right-margin>85.7143</right-margin>
+        <top-margin>85.7143</top-margin>
+        <bottom-margin>85.7143</bottom-margin>
+        </page-margins>
+      <page-margins type="odd">
+        <left-margin>85.7143</left-margin>
+        <right-margin>85.7143</right-margin>
+        <top-margin>85.7143</top-margin>
+        <bottom-margin>85.7143</bottom-margin>
+        </page-margins>
+      </page-layout>
+    <word-font font-family="Edwin" font-size="10"/>
+    <lyric-font font-family="Edwin" font-size="10"/>
+    </defaults>
+  <part-list>
+    <part-group type="start" number="1">
+      <group-symbol>brace</group-symbol>
+      </part-group>
+    <score-part id="P1">
+      <part-name>Piano</part-name>
+      <part-abbreviation>Pno.</part-abbreviation>
+      <score-instrument id="P1-I1">
+        <instrument-name>Piano</instrument-name>
+        </score-instrument>
+      <midi-device id="P1-I1" port="1"></midi-device>
+      <midi-instrument id="P1-I1">
+        <midi-channel>1</midi-channel>
+        <midi-program>1</midi-program>
+        <volume>78.7402</volume>
+        <pan>0</pan>
+        </midi-instrument>
+      </score-part>
+    </part-list>
+  <part id="P1">
+    <measure number="1" width="978.57">
+      <print>
+        <system-layout>
+          <system-margins>
+            <left-margin>50.00</left-margin>
+            <right-margin>0.00</right-margin>
+            </system-margins>
+          <top-system-distance>70.00</top-system-distance>
+          </system-layout>
+        </print>
+      <attributes>
+        <divisions>1</divisions>
+        <key>
+          <fifths>3</fifths>
+          </key>
+        <time>
+          <beats>4</beats>
+          <beat-type>4</beat-type>
+          </time>
+        <clef>
+          <sign>G</sign>
+          <line>2</line>
+          </clef>
+        </attributes>
+      <note default-x="120.48" default-y="10.00">
+        <pitch>
+          <step>A</step>
+          <octave>5</octave>
+          </pitch>
+        <duration>4</duration>
+        <voice>1</voice>
+        <type>whole</type>
+        </note>
+      <backup>
+        <duration>3</duration>
+        </backup>
+      <attributes>
+        <clef>
+          <sign>F</sign>
+          <line>4</line>
+          </clef>
+        </attributes>
+      <forward>
+        <duration>1</duration>
+        </forward>
+      <attributes>
+        <clef>
+          <sign>G</sign>
+          <line>2</line>
+          </clef>
+        </attributes>
+      <forward>
+        <duration>2</duration>
+        </forward>
+      <backup>
+        <duration>4</duration>
+        </backup>
+      <note>
+        <rest/>
+        <duration>1</duration>
+        <voice>2</voice>
+        <type>quarter</type>
+        </note>
+      <note default-x="338.06" default-y="-10.00">
+        <pitch>
+          <step>F</step>
+          <alter>1</alter>
+          <octave>3</octave>
+          </pitch>
+        <duration>1</duration>
+        <voice>2</voice>
+        <type>quarter</type>
+        <stem>down</stem>
+        </note>
+      <note default-x="554.68" default-y="-45.00">
+        <pitch>
+          <step>D</step>
+          <octave>4</octave>
+          </pitch>
+        <duration>1</duration>
+        <voice>2</voice>
+        <type>quarter</type>
+        <stem>down</stem>
+        </note>
+      <note default-x="761.30" default-y="5.00">
+        <pitch>
+          <step>G</step>
+          <alter>1</alter>
+          <octave>5</octave>
+          </pitch>
+        <duration>1</duration>
+        <voice>2</voice>
+        <type>quarter</type>
+        <stem>down</stem>
+        </note>
+      <barline location="right">
+        <bar-style>light-heavy</bar-style>
+        </barline>
+      </measure>
+    </part>
+  </score-partwise>