Browse Source

fix(Pedal): Fix error when no notes in end measure; don't incline pedal line upwards

sschmid 4 năm trước cách đây
mục cha
commit
4b26ea9610

+ 22 - 7
src/MusicalScore/Graphical/VexFlow/VexFlowMusicSheetCalculator.ts

@@ -1036,12 +1036,14 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
       let endStaffEntry: GraphicalStaffEntry = endMeasure.findGraphicalStaffEntryFromTimestamp(endTimeStamp);
       if (!endStaffEntry) { // fix for rendering range set
         endStaffEntry = endMeasure.staffEntries[endMeasure.staffEntries.length - 1];
+        // TODO can be undefined if no notes in end measure
       }
       graphicalPedal.setStartNote(startStaffEntry);
 
       if (endStaffLine !== startStaffLine) {
         if(graphicalPedal.pedalSymbol === MusicSymbol.PEDAL_SYMBOL){
           graphicalPedal.setEndNote(endStaffEntry);
+          graphicalPedal.setEndMeasure(endMeasure);
           graphicalPedal.ReleaseText = " ";
           graphicalPedal.CalculateBoundingBox();
           this.calculatePedalSkyBottomLine(graphicalPedal.startVfVoiceEntry, graphicalPedal.endVfVoiceEntry, graphicalPedal, startStaffLine);
@@ -1052,6 +1054,7 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
           const firstNote: GraphicalStaffEntry = nextPedalFirstMeasure.staffEntries[0];
           nextPedal.setStartNote(firstNote);
           nextPedal.setEndNote(endStaffEntry);
+          graphicalPedal.setEndMeasure(endMeasure);
           endStaffLine.Pedals.push(nextPedal);
           nextPedal.CalculateBoundingBox();
           nextPedal.DepressText = " ";
@@ -1063,6 +1066,7 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
           }
           const lastNoteOfFirstShift: GraphicalStaffEntry = lastMeasureOfFirstShift.staffEntries[lastMeasureOfFirstShift.staffEntries.length - 1];
           graphicalPedal.setEndNote(lastNoteOfFirstShift);
+          graphicalPedal.setEndMeasure(endMeasure);
 
           const systemsInBetweenCount: number = endStaffLine.ParentMusicSystem.Id - startStaffLine.ParentMusicSystem.Id;
           if (systemsInBetweenCount > 0) {
@@ -1092,6 +1096,7 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
 
               nextPedal.setStartNote(firstNote);
               nextPedal.setEndNote(lastNote);
+              graphicalPedal.setEndMeasure(endMeasure);
               nextPedalStaffline.Pedals.push(nextPedal);
               nextPedal.CalculateBoundingBox();
               this.calculatePedalSkyBottomLine(nextPedal.startVfVoiceEntry, nextPedal.endVfVoiceEntry, nextPedal, nextPedalStaffline);
@@ -1102,6 +1107,7 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
         }
       } else {
         graphicalPedal.setEndNote(endStaffEntry);
+        graphicalPedal.setEndMeasure(endMeasure);
         graphicalPedal.CalculateBoundingBox();
         this.calculatePedalSkyBottomLine(graphicalPedal.startVfVoiceEntry, graphicalPedal.endVfVoiceEntry, graphicalPedal, startStaffLine);
       }
@@ -1288,6 +1294,10 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
 
   private calculatePedalSkyBottomLine(startVfVoiceEntry: VexFlowVoiceEntry, endVfVoiceEntry: VexFlowVoiceEntry,
     vfPedal: VexFlowPedal, parentStaffline: StaffLine): void {
+      let endBbox: BoundingBox = endVfVoiceEntry?.PositionAndShape;
+      if (!endBbox) {
+        endBbox = vfPedal.endMeasure.PositionAndShape;
+      }
       //Just for shorthand. Easier readability below
       const PEDAL_STYLES_ENUM: any = Vex.Flow.PedalMarking.Styles;
       const pedalMarking: any = vfPedal.getPedalMarking();
@@ -1309,7 +1319,7 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
       if (vfPedal.EndSymbolPositionAndShape) {
         //Width of the Ped. symbol
         stopX = startX + 3.4;
-        const startX2: number = endVfVoiceEntry.PositionAndShape.AbsolutePosition.x - pedalMarkingMarginXOffset;
+        const startX2: number = endBbox.AbsolutePosition.x - pedalMarkingMarginXOffset;
         //Width of * symbol
         const stopX2: number = startX2 + 1.5;
 
@@ -1320,7 +1330,7 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
         if (!vfPedal.DepressText) {
           footroom = Math.max(footroom, footroom2);
         }
-        if (startVfVoiceEntry.parentStaffEntry.parentMeasure !== endVfVoiceEntry.parentStaffEntry.parentMeasure) {
+        if (startVfVoiceEntry.parentStaffEntry.parentMeasure !== endVfVoiceEntry.parentStaffEntry.parentMeasure && vfPedal.endNote) {
           //TODO: look into using the vexflow setLine instead of modifying the whole stave if possible
           footroom = Math.max((vfPedal.endNote.getStave().options as any).bottom_text_position, footroom);
           (vfPedal.endNote.getStave().options as any).bottom_text_position = footroom;
@@ -1333,14 +1343,14 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
           case PEDAL_STYLES_ENUM.BRACKET_OPEN_END:
           case PEDAL_STYLES_ENUM.BRACKET_OPEN_BOTH:
           case PEDAL_STYLES_ENUM.MIXED_OPEN_END:
-            stopX = endVfVoiceEntry.PositionAndShape.AbsolutePosition.x + endVfVoiceEntry.PositionAndShape.BorderRight - pedalMarkingMarginXOffset;
+            stopX = endBbox.AbsolutePosition.x + endBbox.BorderRight - pedalMarkingMarginXOffset;
           break;
           default:
-            stopX = endVfVoiceEntry.PositionAndShape.AbsolutePosition.x + endVfVoiceEntry.PositionAndShape.BorderLeft - pedalMarkingMarginXOffset;
+            stopX = endBbox.AbsolutePosition.x + endBbox.BorderLeft - pedalMarkingMarginXOffset;
           break;
         }
         //Take into account in-staff clefs associated with the staff entry (they modify the bounding box position)
-        const vfClefBefore: Vex.Flow.ClefNote = (endVfVoiceEntry.parentStaffEntry as VexFlowStaffEntry).vfClefBefore;
+        const vfClefBefore: Vex.Flow.ClefNote = (endVfVoiceEntry?.parentStaffEntry as VexFlowStaffEntry)?.vfClefBefore;
         if (vfClefBefore) {
           const clefWidth: number = vfClefBefore.getWidth() / 10;
           stopX += clefWidth;
@@ -1353,18 +1363,23 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
         //Whatever is currently lower - the set render height of the begin vf stave, the set render height of the end vf stave,
         //or the bottom line. Use that as the render height of both staves
         footroom = Math.max(footroom, (vfPedal.startNote.getStave().options as any).bottom_text_position);
-        if (startVfVoiceEntry.parentStaffEntry.parentMeasure !== endVfVoiceEntry.parentStaffEntry.parentMeasure) {
+        if (startVfVoiceEntry.parentStaffEntry.parentMeasure !== endVfVoiceEntry?.parentStaffEntry.parentMeasure && vfPedal.endNote) {
           footroom = Math.max(footroom, (vfPedal.endNote.getStave().options as any).bottom_text_position);
           (vfPedal.endNote.getStave().options as any).bottom_text_position = footroom;
         }
         (vfPedal.startNote.getStave().options as any).bottom_text_position = footroom;
+        if (startX > stopX) { // TODO hotfix for skybottomlinecalculator after pedal no endNote fix
+          const newStart: number = stopX;
+          stopX = startX;
+          startX = newStart;
+        }
         parentStaffline.SkyBottomLineCalculator.updateBottomLineInRange(startX, stopX, footroom + bottomLineYOffset);
         //This list will contain only previously processed Pedals - aka, those previous in the sheet music
         for (const otherPedal of parentStaffline.Pedals) {
           const vfOtherPedal: VexFlowPedal = otherPedal as VexFlowPedal;
           //If there is a pedal that ends on our begin measure, we need to update its begin measure to render at the new height
           //So we don't get lopsided pedal brackets
-          if (vfOtherPedal.endVfVoiceEntry.parentStaffEntry.parentMeasure === startVfVoiceEntry.parentStaffEntry.parentMeasure) {
+          if (vfOtherPedal.endVfVoiceEntry?.parentStaffEntry.parentMeasure === startVfVoiceEntry.parentStaffEntry.parentMeasure) {
             //Since we've already checked for max height for this stave above, we are certain that this pedal will not render higher than it should
             //(e.g. colliding with stuff)
             (vfOtherPedal.startNote.getStave().options as any).bottom_text_position = footroom;

+ 13 - 0
src/MusicalScore/Graphical/VexFlow/VexFlowPedal.ts

@@ -5,6 +5,8 @@ import { VexFlowVoiceEntry } from "./VexFlowVoiceEntry";
 import { GraphicalPedal } from "../GraphicalPedal";
 import { Pedal } from "../../VoiceData/Expressions/ContinuousExpressions/Pedal";
 import { MusicSymbol } from "../MusicSymbol";
+import { GraphicalMeasure } from "../GraphicalMeasure";
+import { VexFlowMeasure } from "./VexFlowMeasure";
 
 /**
  * The vexflow adaptation of a pedal marking
@@ -19,6 +21,7 @@ export class VexFlowPedal extends GraphicalPedal {
     public ReleaseText: string;
     public startVfVoiceEntry: VexFlowVoiceEntry;
     public endVfVoiceEntry: VexFlowVoiceEntry;
+    public endMeasure: GraphicalMeasure;
 
     public EndSymbolPositionAndShape: BoundingBox = undefined;
     /**
@@ -82,6 +85,9 @@ export class VexFlowPedal extends GraphicalPedal {
      */
     public setEndNote(graphicalStaffEntry: GraphicalStaffEntry): boolean {
         // this is duplicate code from setStartNote, but if we make one general method, we add a lot of branching.
+        if (!graphicalStaffEntry) {
+            return false;
+        }
         for (const gve of graphicalStaffEntry.graphicalVoiceEntries) {
             const vve: VexFlowVoiceEntry = (gve as VexFlowVoiceEntry);
             if (vve?.vfStaveNote) {
@@ -93,6 +99,10 @@ export class VexFlowPedal extends GraphicalPedal {
         return false; // couldn't find an endNote
     }
 
+    public setEndMeasure(graphicalMeasure: GraphicalMeasure): void {
+        this.endMeasure = graphicalMeasure;
+    }
+
     public CalculateBoundingBox(): void {
         //TODO?
     }
@@ -101,6 +111,9 @@ export class VexFlowPedal extends GraphicalPedal {
      */
     public getPedalMarking(): Vex.Flow.PedalMarking {
         const pedalMarking: Vex.Flow.PedalMarking = new Vex.Flow.PedalMarking([this.startNote, this.endNote]);
+        if (this.endMeasure) {
+            (pedalMarking as any).setEndStave((this.endMeasure as VexFlowMeasure).getVFStave());
+        }
         pedalMarking.setStyle(this.vfStyle);
         pedalMarking.setLine(-1);
         pedalMarking.setCustomText(this.DepressText, this.ReleaseText);

+ 25 - 6
src/VexFlowPatch/src/pedalmarking.js

@@ -116,6 +116,10 @@ export class PedalMarking extends Element {
     };
   }
 
+  setEndStave(stave) {
+    this.endStave = stave;
+  }
+
   // Set custom text for the `depress`/`release` pedal markings. No text is
   // set if the parameter is falsy.
   setCustomText(depress, release) {
@@ -150,8 +154,13 @@ export class PedalMarking extends Element {
       // Each note triggers the opposite pedal action
       is_pedal_depressed = !is_pedal_depressed;
       // Get the initial coordinates for the note
-      let x = note.getNoteHeadBeginX();
-      if (!is_pedal_depressed) {
+      let x = 0;
+      if (note) {
+        x = note.getNoteHeadBeginX();
+      }
+      if (!note) {
+        x = this.endStave.end_x;
+      } else if (!is_pedal_depressed) {
         switch(pedal.style) {
           case PedalMarking.Styles.BRACKET_OPEN_END:
           case PedalMarking.Styles.BRACKET_OPEN_BOTH:
@@ -171,13 +180,23 @@ export class PedalMarking extends Element {
         }
       }
 
-      const y = note.getStave().getYForBottomText(pedal.line + 3);
+      let stave = this.endStave; // if !note
+      if (note) {
+        stave = note.getStave();
+      }
+      let y = stave.getYForBottomText(pedal.line + 3);
+      if (prev_y) { // compiler complains if we shorten this
+        if (prev_y > y) { // don't slope pedal marking upwards (nonstandard)
+          y = prev_y;
+        }
+      }
 
       // Throw if current note is positioned before the previous note
       if (x < prev_x) {
-        throw new Vex.RERR(
-          'InvalidConfiguration', 'The notes provided must be in order of ascending x positions'
-        );
+        // TODO this unnecessarily throws for missing endNote fix
+        // throw new Vex.RERR(
+        //   'InvalidConfiguration', 'The notes provided must be in order of ascending x positions'
+        // );
       }
 
       // Determine if the previous or next note are the same