fix(Arpeggios): prevent infinite height bug, arpeggio always going across voices (#546)

vexflow standard setting is arpeggios going across all notes in all voices,
which is often unwanted.

also it causes a bug where the arpeggio extends to infinite height,
if the arpeggio starts in the 1st voice and there's a 2nd voice. see #546

fix #546

refactor drawSlurs -> renderSlurs

src/MusicalScore/Graphical/EngravingRules.ts

@@ -181,7 +181,9 @@ export class EngravingRules {
     private durationScalingDistanceDict: {[_: number]: number; } = {};
 
     private alignRests: number; // 0 = false, 1 = true, 2 = auto
-    private drawSlurs: boolean;
+    private arpeggiosGoAcrossVoices: boolean;
+    private renderArpeggios: boolean;
+    private renderSlurs: boolean;
     private coloringMode: ColoringMode;
     private coloringEnabled: boolean;
     private colorStemsLikeNoteheads: boolean;
@@ -406,7 +408,9 @@ export class EngravingRules {
 
         // Render options (whether to render specific or invisible elements)
         this.alignRests = AlignRestOption.Never; // 0 = false, 1 = true, 2 = auto
-        this.drawSlurs = true;
+        this.arpeggiosGoAcrossVoices = false; // safe option, as otherwise arpeggios will always go across all voices in Vexflow, which is often unwanted
+        this.renderArpeggios = true;
+        this.renderSlurs = true;
         this.coloringMode = ColoringMode.XML;
         this.coloringEnabled = true;
         this.colorStemsLikeNoteheads = false;
@@ -1366,11 +1370,24 @@ export class EngravingRules {
     public set AlignRests(value: number) {
         this.alignRests = value;
     }
-    public get DrawSlurs(): boolean {
-        return this.drawSlurs;
+    public get ArpeggiosGoAcrossVoices(): boolean {
+        return this.arpeggiosGoAcrossVoices;
     }
-    public set DrawSlurs(value: boolean) {
-        this.drawSlurs = value;
+    public set ArpeggiosGoAcrossVoices(value: boolean) {
+        this.arpeggiosGoAcrossVoices = value;
+    }
+    public get RenderArpeggios(): boolean {
+        return this.renderArpeggios;
+    }
+    public set RenderArpeggios(value: boolean) {
+        this.renderArpeggios = value;
+    }
+
+    public get RenderSlurs(): boolean {
+        return this.renderSlurs;
+    }
+    public set RenderSlurs(value: boolean) {
+        this.renderSlurs = value;
     }
     public get ColoringMode(): ColoringMode {
         return this.coloringMode;

src/MusicalScore/Graphical/MusicSheetCalculator.ts

@@ -728,7 +728,7 @@ export abstract class MusicSheetCalculator {
             this.optimizeRestPlacement();
             // possible Displacement of RestNotes
             this.calculateStaffEntryArticulationMarks();
-            if (EngravingRules.Rules.DrawSlurs) { // technically we should separate slurs and ties, but shouldn't be relevant for now
+            if (EngravingRules.Rules.RenderSlurs) { // technically we should separate slurs and ties, but shouldn't be relevant for now
                 // calculate Ties
                 this.calculateTieCurves();
             }
@@ -750,7 +750,7 @@ export abstract class MusicSheetCalculator {
             }
         }
         // calculate Slurs
-        if (!this.leadSheet && EngravingRules.Rules.DrawSlurs) {
+        if (!this.leadSheet && EngravingRules.Rules.RenderSlurs) {
             this.calculateSlurs();
         }
         // calculate StaffEntry Ornaments

src/MusicalScore/Graphical/VexFlow/VexFlowMeasure.ts

@@ -940,10 +940,19 @@ export class VexFlowMeasure extends GraphicalMeasure {
                 // add Arpeggio
                 if (voiceEntry.parentVoiceEntry && voiceEntry.parentVoiceEntry.Arpeggio !== undefined) {
                     const arpeggio: Arpeggio = voiceEntry.parentVoiceEntry.Arpeggio;
+                    // TODO right now our arpeggio object has all arpeggio notes from arpeggios across all voices.
+                    // see VoiceGenerator. Doesn't matter for Vexflow for now though
                     if (voiceEntry.notes && voiceEntry.notes.length > 1) {
-                        // only draw arpeggio if there's more than one note in it. otherwise Vexflow renders the arpeggio to a very high y level
                         const type: Vex.Flow.Stroke.Type = arpeggio.type;
-                        vexFlowVoiceEntry.vfStaveNote.addStroke(0, new Vex.Flow.Stroke(type));
+                        const stroke: Vex.Flow.Stroke = new Vex.Flow.Stroke(type, {
+                            all_voices: EngravingRules.Rules.ArpeggiosGoAcrossVoices
+                            // default: false. This causes arpeggios to always go across all voices, which is often unwanted.
+                            // also, this can cause infinite height of stroke, see #546
+                        });
+                        //if (arpeggio.notes.length === vexFlowVoiceEntry.notes.length) { // different workaround for endless y bug
+                        if (EngravingRules.Rules.RenderArpeggios) {
+                            vexFlowVoiceEntry.vfStaveNote.addStroke(0, stroke);
+                        }
                     } else {
                         log.debug(`[OSMD] arpeggio in measure ${this.MeasureNumber} could not be drawn.
                         voice entry had less than two notes, arpeggio is likely between voice entries, not currently supported in Vexflow.`);

src/MusicalScore/Graphical/VexFlow/VexFlowMusicSheetDrawer.ts

@@ -83,7 +83,7 @@ export class VexFlowMusicSheetDrawer extends MusicSheetDrawer {
     protected drawStaffLine(staffLine: StaffLine): void {
         super.drawStaffLine(staffLine);
         const absolutePos: PointF2D = staffLine.PositionAndShape.AbsolutePosition;
-        if (EngravingRules.Rules.DrawSlurs) {
+        if (EngravingRules.Rules.RenderSlurs) {
             this.drawSlurs(staffLine as VexFlowStaffLine, absolutePos);
         }
     }

src/MusicalScore/ScoreIO/VoiceGenerator.ts

@@ -155,6 +155,8 @@ export class VoiceGenerator {
               if (voiceEntry.Arpeggio !== undefined) {
                 arpeggioAlreadyExists = true;
                 currentArpeggio = voiceEntry.Arpeggio;
+                // TODO handle multiple arpeggios across multiple voices at same timestamp
+
                 // this.currentVoiceEntry.Arpeggio = currentArpeggio; // register the arpeggio in the current voice entry as well?
                 //   but then we duplicate information, and may have to take care not to render it multiple times
 

src/OpenSheetMusicDisplay/OpenSheetMusicDisplay.ts

@@ -324,7 +324,7 @@ export class OpenSheetMusicDisplay {
             EngravingRules.Rules.RenderLyrics = options.drawLyrics;
         }
         if (options.drawSlurs !== undefined) {
-            EngravingRules.Rules.DrawSlurs = options.drawSlurs;
+            EngravingRules.Rules.RenderSlurs = options.drawSlurs;
         }
         if (options.measureNumberInterval !== undefined) {
             EngravingRules.Rules.MeasureNumberLabelOffset = options.measureNumberInterval;