fix(Beams): Fix nested beams, erroneous xml beam numbers (#909). Save IsGrace in Note

fix #909

Whether the note is a grace note was not saved in Note for some reason,
only IsCueNote was saved before.

VoiceGenerator.openBeam was refactored into openBeams, a stack (array),
which pops when the beam has ended.

Beam: save BeamNumber, BeamNumberOffsetToXML

src/MusicalScore/Graphical/VexFlow/VexFlowMusicSheetCalculator.ts

@@ -49,6 +49,7 @@ import { AlignRestOption } from "../../../OpenSheetMusicDisplay/OSMDOptions";
 import { VexFlowStaffLine } from "./VexFlowStaffLine";
 import { EngravingRules } from "../EngravingRules";
 import { VexflowStafflineNoteCalculator } from "./VexflowStafflineNoteCalculator";
+import { MusicSystem } from "../MusicSystem";
 import { NoteTypeHandler } from "../../VoiceData/NoteType";
 import { VexFlowConverter } from "./VexFlowConverter";
 import { TabNote } from "../../VoiceData/TabNote";
@@ -733,8 +734,6 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
           return;
         }
       }
-      startStaffLine.OctaveShifts.push(graphicalOctaveShift);
-
       // calculate RelativePosition and Dashes
       let startStaffEntry: GraphicalStaffEntry = startMeasure.findGraphicalStaffEntryFromTimestamp(startTimeStamp);
       if (!startStaffEntry) { // fix for rendering range set
@@ -744,34 +743,55 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
       if (!endStaffEntry) { // fix for rendering range set
         endStaffEntry = endMeasure.staffEntries[endMeasure.staffEntries.length - 1];
       }
-
       graphicalOctaveShift.setStartNote(startStaffEntry);
 
       if (endStaffLine !== startStaffLine) {
         graphicalOctaveShift.endsOnDifferentStaffLine = true;
-        let lastMeasure: GraphicalMeasure = startStaffLine.Measures[startStaffLine.Measures.length - 1];
-        if (!lastMeasure) { // TODO handle this case correctly (when drawUpToMeasureNumber etc set)
-          lastMeasure = endMeasure;
+        let lastMeasureOfFirstShift: GraphicalMeasure = startStaffLine.Measures[startStaffLine.Measures.length - 1];
+        if (lastMeasureOfFirstShift === undefined) { // TODO handle this case correctly (when drawUpToMeasureNumber etc set)
+          lastMeasureOfFirstShift = endMeasure;
         }
-        const lastNote: GraphicalStaffEntry = lastMeasure.staffEntries[lastMeasure.staffEntries.length - 1];
-        graphicalOctaveShift.setEndNote(lastNote);
-
-        // Now finish the shift on the next line
-        const remainingOctaveShift: VexFlowOctaveShift = new VexFlowOctaveShift(octaveShift, endMeasure.PositionAndShape);
-        endStaffLine.OctaveShifts.push(remainingOctaveShift);
-        let firstMeasure: GraphicalMeasure = endStaffLine.Measures[0];
-        if (!firstMeasure) { // TODO handle this case correctly (when drawUpToMeasureNumber etc set)
-          firstMeasure = startMeasure;
+        const lastNoteOfFirstShift: GraphicalStaffEntry = lastMeasureOfFirstShift.staffEntries[lastMeasureOfFirstShift.staffEntries.length - 1];
+        graphicalOctaveShift.setEndNote(lastNoteOfFirstShift);
+
+        const systemsInBetweenCount: number = endStaffLine.ParentMusicSystem.Id - startStaffLine.ParentMusicSystem.Id;
+        if (systemsInBetweenCount > 0) {
+          //Loop through the stafflines in between to the end
+          for (let i: number = startStaffLine.ParentMusicSystem.Id; i < endStaffLine.ParentMusicSystem.Id; i++) {
+            const idx: number = i + 1;
+            const nextShiftMusicSystem: MusicSystem = this.musicSystems[idx];
+            const nextShiftStaffline: StaffLine = nextShiftMusicSystem.StaffLines[staffIndex];
+            const nextShiftFirstMeasure: GraphicalMeasure = nextShiftStaffline.Measures[0];
+            // Shift starts on the first measure
+            const nextOctaveShift: VexFlowOctaveShift = new VexFlowOctaveShift(octaveShift, nextShiftFirstMeasure.PositionAndShape);
+
+            if (i < systemsInBetweenCount) {
+              nextOctaveShift.endsOnDifferentStaffLine = true;
+            }
+
+            let nextShiftLastMeasure: GraphicalMeasure = nextShiftStaffline.Measures[nextShiftStaffline.Measures.length - 1];
+            const firstNote: GraphicalStaffEntry = nextShiftFirstMeasure.staffEntries[0];
+            let lastNote: GraphicalStaffEntry = nextShiftLastMeasure.staffEntries[nextShiftLastMeasure.staffEntries.length - 1];
+
+            //If the is the ending staffline, this endMeasure is the end of the shift
+            if (endMeasure.ParentStaffLine === nextShiftStaffline) {
+              nextShiftLastMeasure = endMeasure;
+              lastNote = endStaffEntry;
+            }
+
+            nextOctaveShift.setStartNote(firstNote);
+            nextOctaveShift.setEndNote(lastNote);
+            nextShiftStaffline.OctaveShifts.push(nextOctaveShift);
+            this.calculateOctaveShiftSkyBottomLine(firstNote, lastNote, nextOctaveShift, nextShiftStaffline);
+          }
         }
-        const firstNote: GraphicalStaffEntry = firstMeasure.staffEntries[0];
-        remainingOctaveShift.setStartNote(firstNote);
-        remainingOctaveShift.setEndNote(endStaffEntry);
-        this.calculateOctaveShiftSkyBottomLine(startStaffEntry, lastNote, graphicalOctaveShift, startStaffLine);
-        this.calculateOctaveShiftSkyBottomLine(firstNote, endStaffEntry, remainingOctaveShift, endStaffLine);
+
+        this.calculateOctaveShiftSkyBottomLine(startStaffEntry, lastNoteOfFirstShift, graphicalOctaveShift, startStaffLine);
       } else {
         graphicalOctaveShift.setEndNote(endStaffEntry);
         this.calculateOctaveShiftSkyBottomLine(startStaffEntry, endStaffEntry, graphicalOctaveShift, startStaffLine);
       }
+      startStaffLine.OctaveShifts.push(graphicalOctaveShift);
     } else {
       log.warn("End measure or staffLines for octave shift are undefined! This should not happen!");
     }
@@ -780,8 +800,27 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
   private calculateOctaveShiftSkyBottomLine(startStaffEntry: GraphicalStaffEntry, endStaffEntry: GraphicalStaffEntry,
                                             vfOctaveShift: VexFlowOctaveShift, parentStaffline: StaffLine): void {
 
-    const startX: number = startStaffEntry.PositionAndShape.AbsolutePosition.x - startStaffEntry.PositionAndShape.Size.width / 2;
-    const stopX: number = endStaffEntry.PositionAndShape.AbsolutePosition.x + endStaffEntry.PositionAndShape.Size.width / 2;
+    let startXOffset: number = startStaffEntry.PositionAndShape.Size.width;
+    let endXOffset: number = endStaffEntry.PositionAndShape.Size.width;
+
+    //Vexflow renders differently with rests
+    if (startStaffEntry.hasOnlyRests()) {
+      startXOffset = -startXOffset;
+    } else {
+      startXOffset /= 2;
+    }
+
+    if (!endStaffEntry.hasOnlyRests()) {
+      endXOffset /= 2;
+    } else {
+      endXOffset *= 2;
+    }
+
+    if (startStaffEntry === endStaffEntry) {
+      endXOffset *= 2;
+    }
+    const startX: number = startStaffEntry.PositionAndShape.AbsolutePosition.x - startXOffset;
+    const stopX: number = endStaffEntry.PositionAndShape.AbsolutePosition.x + endXOffset;
     vfOctaveShift.PositionAndShape.Size.width = startX - stopX;
     const textBracket: Vex.Flow.TextBracket = vfOctaveShift.getTextBracket();
     const fontSize: number = (textBracket as any).font.size / 10;
@@ -789,18 +828,18 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
     if ((<any>textBracket).position === Vex.Flow.TextBracket.Positions.TOP) {
       const headroom: number = Math.ceil(parentStaffline.SkyBottomLineCalculator.getSkyLineMinInRange(startX, stopX));
       if (headroom === Infinity) { // will cause Vexflow error
-          return;
+        return;
       }
       (textBracket.start.getStave().options as any).top_text_position = Math.abs(headroom);
       parentStaffline.SkyBottomLineCalculator.updateSkyLineInRange(startX, stopX, headroom - fontSize * 2);
     } else {
-        const footroom: number = parentStaffline.SkyBottomLineCalculator.getBottomLineMaxInRange(startX, stopX);
-        if (footroom === Infinity) { // will cause Vexflow error
-            return;
-        }
-        (textBracket.start.getStave().options as any).bottom_text_position = footroom;
-        //Vexflow positions top vs. bottom text in a slightly inconsistent way it seems
-        parentStaffline.SkyBottomLineCalculator.updateBottomLineInRange(startX, stopX, footroom + fontSize * 1.5);
+      const footroom: number = parentStaffline.SkyBottomLineCalculator.getBottomLineMaxInRange(startX, stopX);
+      if (footroom === Infinity) { // will cause Vexflow error
+        return;
+      }
+      (textBracket.start.getStave().options as any).bottom_text_position = footroom;
+      //Vexflow positions top vs. bottom text in a slightly inconsistent way it seems
+      parentStaffline.SkyBottomLineCalculator.updateBottomLineInRange(startX, stopX, footroom + fontSize * 1.5);
     }
   }
 

src/MusicalScore/ScoreIO/InstrumentReader.ts

@@ -395,7 +395,7 @@ export class InstrumentReader {
             this.currentStaffEntry, this.currentMeasure,
             measureStartAbsoluteTimestamp,
             this.maxTieNoteFraction, isChord, guitarPro,
-            printObject, isCueNote, stemDirectionXml, tremoloStrokes, stemColorXml, noteheadColorXml,
+            printObject, isCueNote, isGraceNote, stemDirectionXml, tremoloStrokes, stemColorXml, noteheadColorXml,
             vibratoStrokes
           );
 

src/MusicalScore/ScoreIO/VoiceGenerator.ts

@@ -55,8 +55,9 @@ export class VoiceGenerator {
   private currentNote: Note;
   private currentMeasure: SourceMeasure;
   private currentStaffEntry: SourceStaffEntry;
-  private lastBeamTag: string = "";
-  private openBeam: Beam;
+  // private lastBeamTag: string = "";
+  private openBeams: Beam[] = []; // works like a stack, with push and pop
+  private beamNumberOffset: number = 0;
   private openTieDict: { [_: number]: Tie; } = {};
   private currentOctaveShift: number = 0;
   private tupletDict: { [_: number]: Tuplet; } = {};
@@ -107,7 +108,7 @@ export class VoiceGenerator {
   public read(noteNode: IXmlElement, noteDuration: Fraction, typeDuration: Fraction, noteTypeXml: NoteType, normalNotes: number, restNote: boolean,
               parentStaffEntry: SourceStaffEntry, parentMeasure: SourceMeasure,
               measureStartAbsoluteTimestamp: Fraction, maxTieNoteFraction: Fraction, chord: boolean, guitarPro: boolean,
-              printObject: boolean, isCueNote: boolean, stemDirectionXml: StemDirectionType, tremoloStrokes: number,
+              printObject: boolean, isCueNote: boolean, isGraceNote: boolean, stemDirectionXml: StemDirectionType, tremoloStrokes: number,
               stemColorXml: string, noteheadColorXml: string, vibratoStrokes: boolean): Note {
     this.currentStaffEntry = parentStaffEntry;
     this.currentMeasure = parentMeasure;
@@ -117,7 +118,7 @@ export class VoiceGenerator {
       this.currentNote = restNote
         ? this.addRestNote(noteNode.element("rest"), noteDuration, noteTypeXml, printObject, isCueNote, noteheadColorXml)
         : this.addSingleNote(noteNode, noteDuration, noteTypeXml, typeDuration, normalNotes, chord, guitarPro,
-                             printObject, isCueNote, stemDirectionXml, tremoloStrokes, stemColorXml, noteheadColorXml, vibratoStrokes);
+                             printObject, isCueNote, isGraceNote, stemDirectionXml, tremoloStrokes, stemColorXml, noteheadColorXml, vibratoStrokes);
       // read lyrics
       const lyricElements: IXmlElement[] = noteNode.elements("lyric");
       if (this.lyricsReader !== undefined && lyricElements) {
@@ -259,7 +260,7 @@ export class VoiceGenerator {
   }
 
   public checkForOpenBeam(): void {
-    if (this.openBeam !== undefined && this.currentNote) {
+    if (this.openBeams.length > 0 && this.currentNote) {
       this.handleOpenBeam();
     }
   }
@@ -314,7 +315,7 @@ export class VoiceGenerator {
    */
   private addSingleNote(node: IXmlElement, noteDuration: Fraction, noteTypeXml: NoteType, typeDuration: Fraction,
                         normalNotes: number, chord: boolean, guitarPro: boolean,
-                        printObject: boolean, isCueNote: boolean, stemDirectionXml: StemDirectionType, tremoloStrokes: number,
+                        printObject: boolean, isCueNote: boolean, isGraceNote: boolean, stemDirectionXml: StemDirectionType, tremoloStrokes: number,
                         stemColorXml: string, noteheadColorXml: string, vibratoStrokes: boolean): Note {
     //log.debug("addSingleNote called");
     let noteAlter: number = 0;
@@ -450,6 +451,7 @@ export class VoiceGenerator {
     note.NormalNotes = normalNotes;
     note.PrintObject = printObject;
     note.IsCueNote = isCueNote;
+    note.IsGraceNote = isGraceNote;
     note.StemDirectionXml = stemDirectionXml; // maybe unnecessary, also in VoiceEntry
     note.TremoloStrokes = tremoloStrokes; // could be a Tremolo object in future if we have more data to manage like two-note tremolo
     if ((noteheadShapeXml !== undefined && noteheadShapeXml !== "normal") || noteheadFilledXml !== undefined) {
@@ -494,8 +496,8 @@ export class VoiceGenerator {
     restNote.NoteheadColorXml = noteheadColorXml;
     restNote.NoteheadColor = noteheadColorXml;
     this.currentVoiceEntry.Notes.push(restNote);
-    if (this.openBeam) {
-      this.openBeam.ExtendedNoteList.push(restNote);
+    if (this.openBeams.length > 0) {
+      this.openBeams.last().ExtendedNoteList.push(restNote);
     }
     return restNote;
   }
@@ -513,35 +515,44 @@ export class VoiceGenerator {
         beamAttr = beamNode.attribute("number");
       }
       if (beamAttr) {
-        const beamNumber: number = parseInt(beamAttr.value, 10);
+        let beamNumber: number = parseInt(beamAttr.value, 10);
         const mainBeamNode: IXmlElement[] = node.elements("beam");
         const currentBeamTag: string = mainBeamNode[0].value;
-        if (beamNumber === 1 && mainBeamNode) {
-          if (currentBeamTag === "begin" && this.lastBeamTag !== currentBeamTag) {
-              if (this.openBeam) {
+        if (mainBeamNode) {
+          if (currentBeamTag === "begin") {
+            if (beamNumber === this.openBeams.last()?.BeamNumber) {
+              // beam with same number already existed (error in XML), bump beam number
+              this.beamNumberOffset++;
+              beamNumber += this.beamNumberOffset;
+            } else if (this.openBeams.last()) {
                 this.handleOpenBeam();
-              }
-              this.openBeam = new Beam();
             }
-          this.lastBeamTag = currentBeamTag;
+            this.openBeams.push(new Beam(beamNumber, this.beamNumberOffset));
+          } else {
+            beamNumber += this.beamNumberOffset;
+          }
         }
         let sameVoiceEntry: boolean = false;
-        if (!this.openBeam) {
-            return;
-          }
-        for (let idx: number = 0, len: number = this.openBeam.Notes.length; idx < len; ++idx) {
-            const beamNote: Note = this.openBeam.Notes[idx];
-            if (this.currentVoiceEntry === beamNote.ParentVoiceEntry) {
-              sameVoiceEntry = true;
-            }
+        if (!(beamNumber > 0 && beamNumber <= this.openBeams.length) || !this.openBeams[beamNumber - 1]) {
+          console.log("invalid beamnumber"); // this shouldn't happen, probably error in this method
+          return;
+        }
+        for (let idx: number = 0, len: number = this.openBeams[beamNumber - 1].Notes.length; idx < len; ++idx) {
+          const beamNote: Note = this.openBeams[beamNumber - 1].Notes[idx];
+          if (this.currentVoiceEntry === beamNote.ParentVoiceEntry) {
+            sameVoiceEntry = true;
           }
+        }
         if (!sameVoiceEntry) {
-            this.openBeam.addNoteToBeam(note);
-            if (currentBeamTag === "end" && beamNumber === 1) {
-              this.openBeam = undefined;
-            }
+          const openBeam: Beam = this.openBeams[beamNumber - 1];
+          openBeam.addNoteToBeam(note);
+          // const lastBeamNote: Note = openBeam.Notes.last();
+          // const graceStatusChanged: boolean = (lastBeamNote?.IsCueNote || lastBeamNote?.IsGraceNote) !== (note.IsCueNote) || (note.IsGraceNote);
+          if (currentBeamTag === "end") {
+            this.endBeam();
           }
         }
+      }
     } catch (e) {
       const errorMsg: string = ITextTranslation.translateText(
         "ReaderErrorMessages/BeamError", "Error while reading beam."
@@ -549,23 +560,28 @@ export class VoiceGenerator {
       this.musicSheet.SheetErrors.pushMeasureError(errorMsg);
       throw new MusicSheetReadingException("", e);
     }
+  }
 
+  private endBeam(): void {
+    this.openBeams.pop(); // pop the last open beam from the stack. the latest openBeam will be the one before that now
+    this.beamNumberOffset = Math.max(0, this.beamNumberOffset - 1);
   }
 
   /**
    * Check for open [[Beam]]s at end of [[SourceMeasure]] and closes them explicity.
    */
   private handleOpenBeam(): void {
-    if (this.openBeam.Notes.length === 1) {
-      const beamNote: Note = this.openBeam.Notes[0];
+    const openBeam: Beam = this.openBeams.last();
+    if (openBeam.Notes.length === 1) {
+      const beamNote: Note = openBeam.Notes[0];
       beamNote.NoteBeam = undefined;
-      this.openBeam = undefined;
+      this.endBeam();
       return;
     }
-    if (this.currentNote === CollectionUtil.last(this.openBeam.Notes)) {
-      this.openBeam = undefined;
+    if (this.currentNote === CollectionUtil.last(openBeam.Notes)) {
+      this.endBeam();
     } else {
-      const beamLastNote: Note = CollectionUtil.last(this.openBeam.Notes);
+      const beamLastNote: Note = CollectionUtil.last(openBeam.Notes);
       const beamLastNoteStaffEntry: SourceStaffEntry = beamLastNote.ParentStaffEntry;
       const horizontalIndex: number = this.currentMeasure.getVerticalContainerIndexByTimestamp(beamLastNoteStaffEntry.Timestamp);
       const verticalIndex: number = beamLastNoteStaffEntry.VerticalContainerParent.StaffEntries.indexOf(beamLastNoteStaffEntry);
@@ -579,16 +595,16 @@ export class VoiceGenerator {
             if (voiceEntry.ParentVoice === this.voice) {
               const candidateNote: Note = voiceEntry.Notes[0];
               if (candidateNote.Length.lte(new Fraction(1, 8))) {
-                this.openBeam.addNoteToBeam(candidateNote);
-                this.openBeam = undefined;
+                this.openBeams.last().addNoteToBeam(candidateNote);
+                this.endBeam();
               } else {
-                this.openBeam = undefined;
+                this.endBeam();
               }
             }
           }
         }
       } else {
-        this.openBeam = undefined;
+        this.endBeam();
       }
     }
   }

src/MusicalScore/VoiceData/Beam.ts

@@ -7,6 +7,13 @@ export class Beam {
 
     private notes: Note[] = [];
     private extendedNoteList: Note[] = [];
+    public BeamNumber: number;
+    public BeamNumberOffsetToXML: number = 0;
+
+    constructor(beamNumber: number = 1, beamNumberOffsetToXML: number = 0) {
+        this.BeamNumber = beamNumber;
+        this.BeamNumberOffsetToXML = beamNumberOffsetToXML;
+    }
 
     public get Notes(): Note[] {
         return this.notes;

src/MusicalScore/VoiceData/Note.ts

@@ -68,6 +68,7 @@ export class Note {
     private arpeggio: Arpeggio;
     /** States whether this is a cue note (Stichnote) (smaller size). */
     private isCueNote: boolean;
+    public IsGraceNote: boolean;
     /** The stem direction asked for in XML. Not necessarily final or wanted stem direction. */
     private stemDirectionXml: StemDirectionType;
     /** The number of tremolo strokes this note has (16th tremolo = 2 strokes).