fix(Note X-Positions): Fix Cross Stave Note Position X-Shift and Ghost Notes for complex fractions (osmd-public:1062)

merge osmd-public (future 1.2.0)

* fix cross stave note x-shift for simple sample in osmd-public:1062
https://github.com/opensheetmusicdisplay/opensheetmusicdisplay/issues/1062

This fixes a lot of note x-shifts in piano scores (grand staff).

* fix(VexFlowConverter.durations): Calculate multiple durations for fractions like 5/16, which can't be represented by a single note (1062). Fixes Elite Syncopations M2

fixes measure 2 in Elite Syncopations

* VexFlowConverter: fix note length 0 used from typeLength (Actor Prelude, 1062)

Fixes osmd-public:1062

package.json

@@ -1,6 +1,6 @@
 {
   "name": "opensheetmusicdisplay-private",
-  "version": "1.1.1",
+  "version": "1.2.0",
   "description": "Private OSMD mirror/audio player.",
   "main": "build/opensheetmusicdisplay.min.js",
   "typings": "build/dist/src/",

src/Common/DataObjects/Fraction.ts

@@ -153,6 +153,17 @@ export class Fraction {
     return this.wholeValue * this.denominator + this.numerator;
   }
 
+  public calculateNumberOfNeededDots(): number {
+    let num: number = 1;
+    let product: number = 2;
+    const expandedNumerator: number = this.GetExpandedNumerator();
+    while (product < expandedNumerator) {
+      num++;
+      product = Math.pow(2, num);
+    }
+    return Math.min(3, num - 1);
+  }
+
   public IsNegative(): boolean {
     return this.realValue < 0;
   }

src/MusicalScore/Graphical/GraphicalNote.ts

@@ -55,16 +55,10 @@ export class GraphicalNote extends GraphicalObject {
      * @returns {number}
      */
     private calculateNumberOfNeededDots(fraction: Fraction): number {
-      let num: number = 1;
-      let product: number = 2;
-      const expandedNumerator: number = fraction.GetExpandedNumerator();
       if (!this.sourceNote || !this.sourceNote.NoteTuplet) {
-        while (product < expandedNumerator) {
-          num++;
-          product = Math.pow(2, num);
-        }
+        return fraction.calculateNumberOfNeededDots();
       }
-      return Math.min(3, num - 1);
+      return 0;
     }
 
     public get ParentMusicPage(): GraphicalMusicPage {

src/MusicalScore/Graphical/VexFlow/VexFlowConverter.ts

@@ -54,61 +54,93 @@ export class VexFlowConverter {
     };
 
     /**
-     * Convert a fraction to a string which represents a duration in VexFlow
+     * Convert a fraction to Vexflow string durations.
+     * A duration like 5/16 (5 16th notes) can't be represented by a single (dotted) note,
+     *   so we need to return multiple durations (e.g. for 5/16th ghost notes).
+     * Currently, for a dotted quarter ghost note, we return a quarter and an eighth ghost note.
+     *   We could return a dotted quarter instead, but then the code would need to distinguish between
+     *   notes that can be represented as dotted notes and notes that can't, which would complicate things.
+     *   We could e.g. add a parameter "allowSingleDottedNote" which makes it possible to return single dotted notes instead.
+     * But currently, this is only really used for Ghost notes, so it doesn't make a difference visually.
+     *   (for other uses like StaveNotes, we calculate the dots separately)
      * @param fraction a fraction representing the duration of a note
-     * @returns {string}
+     * @returns {string[]} Vexflow note type strings (e.g. "h" = half note)
      */
-    public static duration(fraction: Fraction, isTuplet: boolean): string {
-      const dur: number = fraction.RealValue;
-
-      if (dur === 2) { // Breve
-        return "1/2";
-      }
-      // TODO consider long (dur=4) and maxima (dur=8), though Vexflow doesn't seem to support them
-      if (dur >= 1) {
-          return "w";
-      } else if (dur < 1 && dur >= 0.5) {
-        // change to the next higher straight note to get the correct note display type
-        if (isTuplet && dur > 0.5) {
-          return "w";
-        }
-        return "h";
-      } else if (dur < 0.5 && dur >= 0.25) {
-        // change to the next higher straight note to get the correct note display type
-        if (isTuplet && dur > 0.25) {
-          return "h";
-        }
-        return "q";
-      } else if (dur < 0.25 && dur >= 0.125) {
-        // change to the next higher straight note to get the correct note display type
-        if (isTuplet && dur > 0.125) {
-          return "q";
-        }
-        return "8";
-      } else if (dur < 0.125 && dur >= 0.0625) {
-        // change to the next higher straight note to get the correct note display type
-        if (isTuplet && dur > 0.0625) {
-          return "8";
-        }
-        return "16";
-      } else if (dur < 0.0625 && dur >= 0.03125) {
-        // change to the next higher straight note to get the correct note display type
-        if (isTuplet && dur > 0.03125) {
-          return "16";
-        }
-        return "32";
-      } else if (dur < 0.03125 && dur >= 0.015625) {
-        // change to the next higher straight note to get the correct note display type
-        if (isTuplet && dur > 0.015625) {
-          return "32";
-        }
-        return "64";
-      }
-
-      if (isTuplet) {
-        return "64";
-      }
-      return "128";
+    public static durations(fraction: Fraction, isTuplet: boolean): string[] {
+        const durations: string[] = [];
+        const remainingFraction: Fraction = fraction.clone();
+        while (remainingFraction.RealValue > 0) {
+            const dur: number = remainingFraction.RealValue;
+            // TODO consider long (dur=4) and maxima (dur=8), though Vexflow doesn't seem to support them
+            if (dur >= 2) { // Breve
+                durations.push("1/2");
+                remainingFraction.Sub(new Fraction(2, 1));
+            } else if (dur >= 1) {
+                durations.push("w");
+                remainingFraction.Sub(new Fraction(1, 1));
+            } else if (dur < 1 && dur >= 0.5) {
+                // change to the next higher straight note to get the correct note display type
+                if (isTuplet && dur > 0.5) {
+                    return ["w"];
+                } else {
+                    durations.push("h");
+                    remainingFraction.Sub(new Fraction(1, 2));
+                }
+            } else if (dur < 0.5 && dur >= 0.25) {
+                // change to the next higher straight note to get the correct note display type
+                if (isTuplet && dur > 0.25) {
+                    return ["h"];
+                } else {
+                    durations.push("q");
+                    remainingFraction.Sub(new Fraction(1, 4));
+                }
+            } else if (dur < 0.25 && dur >= 0.125) {
+                // change to the next higher straight note to get the correct note display type
+                if (isTuplet && dur > 0.125) {
+                    return ["q"];
+                } else {
+                    durations.push("8");
+                    remainingFraction.Sub(new Fraction(1, 8));
+                }
+            } else if (dur < 0.125 && dur >= 0.0625) {
+                // change to the next higher straight note to get the correct note display type
+                if (isTuplet && dur > 0.0625) {
+                    return ["8"];
+                } else {
+                    durations.push("16");
+                    remainingFraction.Sub(new Fraction(1, 16));
+                }
+            } else if (dur < 0.0625 && dur >= 0.03125) {
+                // change to the next higher straight note to get the correct note display type
+                if (isTuplet && dur > 0.03125) {
+                    return ["16"];
+                } else {
+                    durations.push("32");
+                    remainingFraction.Sub(new Fraction(1, 32));
+                }
+            } else if (dur < 0.03125 && dur >= 0.015625) {
+                // change to the next higher straight note to get the correct note display type
+                if (isTuplet && dur > 0.015625) {
+                    return ["32"];
+                } else {
+                    durations.push("64");
+                    remainingFraction.Sub(new Fraction(1, 64));
+                }
+            } else {
+                if (isTuplet) {
+                    return ["64"];
+                } else {
+                    durations.push("128");
+                    remainingFraction.Sub(new Fraction(1, 128));
+                }
+            }
+        }
+        //   if (isTuplet) {
+        //     dots = 0; // TODO (different) calculation?
+        //   } else {
+        //     dots = fraction.calculateNumberOfNeededDots();
+        //   }
+        return durations;
     }
 
     /**
@@ -185,10 +217,16 @@ export class VexFlowConverter {
         }
     }
 
-    public static GhostNote(frac: Fraction): Vex.Flow.GhostNote {
-        return new Vex.Flow.GhostNote({
-            duration: VexFlowConverter.duration(frac, false),
-        });
+    public static GhostNotes(frac: Fraction): Vex.Flow.GhostNote[] {
+        const ghostNotes: Vex.Flow.GhostNote[] = [];
+        const durations: string[] = VexFlowConverter.durations(frac, false);
+        for (const duration of durations) {
+            ghostNotes.push(new Vex.Flow.GhostNote({
+                duration: duration,
+                //dots: dots
+            }));
+        }
+        return ghostNotes;
     }
 
     /**
@@ -215,9 +253,11 @@ export class VexFlowConverter {
         const accidentals: string[] = [];
         const baseNoteLength: Fraction = baseNote.graphicalNoteLength;
         const isTuplet: boolean = baseNote.sourceNote.NoteTuplet !== undefined;
-        let duration: string = VexFlowConverter.duration(baseNoteLength, isTuplet);
-        if (baseNote.sourceNote.TypeLength !== undefined && baseNote.sourceNote.TypeLength !== baseNoteLength) {
-            duration = VexFlowConverter.duration(baseNote.sourceNote.TypeLength, isTuplet);
+        let duration: string = VexFlowConverter.durations(baseNoteLength, isTuplet)[0];
+        if (baseNote.sourceNote.TypeLength !== undefined &&
+            baseNote.sourceNote.TypeLength !== baseNoteLength &&
+            baseNote.sourceNote.TypeLength.RealValue !== 0) {
+            duration = VexFlowConverter.durations(baseNote.sourceNote.TypeLength, isTuplet)[0];
         }
         let vfClefType: string = undefined;
         let numDots: number = baseNote.numberOfDots;
@@ -648,7 +688,7 @@ export class VexFlowConverter {
         const tabPhrases: { type: number, text: string, width: number }[] = [];
         const frac: Fraction = gve.notes[0].graphicalNoteLength;
         const isTuplet: boolean = gve.notes[0].sourceNote.NoteTuplet !== undefined;
-        let duration: string = VexFlowConverter.duration(frac, isTuplet);
+        let duration: string = VexFlowConverter.durations(frac, isTuplet)[0];
         let numDots: number = 0;
         for (const note of gve.notes) {
             const tabNote: TabNote = note.sourceNote as TabNote;

src/MusicalScore/Graphical/VexFlow/VexFlowMeasure.ts

@@ -735,7 +735,7 @@ export class VexFlowMeasure extends GraphicalMeasure {
      */
     protected getRestFilledVexFlowStaveNotesPerVoice(voice: Voice): GraphicalVoiceEntry[] {
         let latestVoiceTimestamp: Fraction = undefined;
-        const gvEntries: GraphicalVoiceEntry[] = this.getGraphicalVoiceEntriesPerVoice(voice);
+        let gvEntries: GraphicalVoiceEntry[] = this.getGraphicalVoiceEntriesPerVoice(voice);
         for (let idx: number = 0, len: number = gvEntries.length; idx < len; ++idx) {
             const gve: GraphicalVoiceEntry = gvEntries[idx];
             const gNotesStartTimestamp: Fraction = gve.notes[0].sourceNote.getAbsoluteTimestamp();
@@ -754,11 +754,9 @@ export class VexFlowMeasure extends GraphicalMeasure {
                 const gapFromMeasureStart: Fraction = Fraction.minus(gNotesStartTimestamp, this.parentSourceMeasure.AbsoluteTimestamp);
                 if (gapFromMeasureStart.RealValue > 0) {
                     log.trace(`Ghost Found at start (measure ${this.MeasureNumber})`); // happens too often for valid measures to be logged to debug
-                    const vfghost: Vex.Flow.GhostNote = VexFlowConverter.GhostNote(gapFromMeasureStart);
-                    const ghostGve: VexFlowVoiceEntry = new VexFlowVoiceEntry(undefined, undefined);
-                    ghostGve.vfStaveNote = vfghost;
-                    gvEntries.splice(0, 0, ghostGve);
-                    idx++;
+                    const ghostGves: GraphicalVoiceEntry[] = this.createGhostGves(gapFromMeasureStart);
+                    gvEntries.splice(0, 0, ...ghostGves);
+                    idx += ghostGves.length;
                 }
             } else {
                 // get the length of the empty space between notes:
@@ -766,13 +764,11 @@ export class VexFlowMeasure extends GraphicalMeasure {
 
                 if (inBetweenLength.RealValue > 0) {
                     log.trace(`Ghost Found in between (measure ${this.MeasureNumber})`); // happens too often for valid measures to be logged to debug
-                    const vfghost: Vex.Flow.GhostNote = VexFlowConverter.GhostNote(inBetweenLength);
-                    const ghostGve: VexFlowVoiceEntry = new VexFlowVoiceEntry(undefined, undefined);
-                    ghostGve.vfStaveNote = vfghost;
-                    // add element before current element:
-                    gvEntries.splice(idx, 0, ghostGve);
-                    // and increase index, as we added an element:
-                    idx++;
+                    const ghostGves: VexFlowVoiceEntry[] = this.createGhostGves(inBetweenLength);
+                    // add elements before current element:
+                    gvEntries.splice(idx, 0, ...ghostGves);
+                    // and increase index, as we added elements:
+                    idx += ghostGves.length;
                 }
             }
 
@@ -787,12 +783,21 @@ export class VexFlowMeasure extends GraphicalMeasure {
             // starting from lastFraction
             // with length restLength:
             log.trace(`Ghost Found at end (measure ${this.MeasureNumber})`); // happens too often for valid measures to be logged to debug
-            const vfghost: Vex.Flow.GhostNote = VexFlowConverter.GhostNote(restLength);
+            const ghostGves: VexFlowVoiceEntry[] = this.createGhostGves(restLength);
+            gvEntries = gvEntries.concat(ghostGves);
+        }
+        return gvEntries;
+    }
+
+    private createGhostGves(duration: Fraction): VexFlowVoiceEntry[] {
+        const vfghosts: Vex.Flow.GhostNote[] = VexFlowConverter.GhostNotes(duration);
+        const ghostGves: VexFlowVoiceEntry[] = [];
+        for (const vfghost of vfghosts) {
             const ghostGve: VexFlowVoiceEntry = new VexFlowVoiceEntry(undefined, undefined);
             ghostGve.vfStaveNote = vfghost;
-            gvEntries.push(ghostGve);
+            ghostGves.push(ghostGve);
         }
-        return gvEntries;
+        return ghostGves;
     }
 
     /**

src/MusicalScore/Graphical/VexFlow/VexFlowMusicSheetCalculator.ts

@@ -761,7 +761,7 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
     let vexflowDuration: string = "q";
     if (metronomeExpression.beatUnit) {
       const duration: Fraction = NoteTypeHandler.getNoteDurationFromType(metronomeExpression.beatUnit);
-      vexflowDuration = VexFlowConverter.duration(duration, false);
+      vexflowDuration = VexFlowConverter.durations(duration, false)[0];
     }
 
     let yShift: number = this.rules.MetronomeMarkYShift;

src/MusicalScore/Graphical/VexFlow/VexFlowTabMeasure.ts

@@ -43,7 +43,7 @@ export class VexFlowTabMeasure extends VexFlowMeasure {
             // create vex flow Notes:
             for (const gve of graphicalStaffEntry.graphicalVoiceEntries) {
                 if (gve.notes[0].sourceNote.isRest()) {
-                    (gve as VexFlowVoiceEntry).vfStaveNote = VexFlowConverter.GhostNote(gve.notes[0].sourceNote.Length);
+                    (gve as VexFlowVoiceEntry).vfStaveNote = VexFlowConverter.GhostNotes(gve.notes[0].sourceNote.Length)[0];
                 } else {
                     (gve as VexFlowVoiceEntry).vfStaveNote = VexFlowConverter.CreateTabNote(gve);
                 }