fix(Octave Shifts): Fix ottavas not generated correctly over multiple systems (#591) (PR #777)

* fix(ottava): Generate ottava over multiple stafflines, gen. skyline
Fix issue with ottava's only being generated on start and end staffline.
Further fixes for sky/bottom line generation

* Fixes for ottava's breaking across stafflines

Fix for when zoom level would break ottava's - loop from start staffline to end staffline

Co-authored-by: Justin Litten <justin@jlitten-laptop.WORKGROUP>

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);
     }
   }