fix(Arpeggio): fix up/down direction (wrong in Vexflow), remove Vexfl…

…ow dependency close #645 (remove Vexflow dependency from Arpeggio)
opensheetmusicdisplay · Jan 15, 2020 · 450b2d9 · 450b2d9
1 parent 90d93b9
commit 450b2d9
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 8 deletions.
diff --git a/src/MusicalScore/Graphical/VexFlow/VexFlowConverter.ts b/src/MusicalScore/Graphical/VexFlow/VexFlowConverter.ts
@@ -25,6 +25,7 @@ import { unitInPixels } from "./VexFlowMusicSheetDrawer";
 import { EngravingRules } from "../EngravingRules";
 import { Note } from "../..";
 import StaveNote = Vex.Flow.StaveNote;
+import { ArpeggioType } from "../../VoiceData";
 
 /**
  * Helper class, which contains static methods which actually convert
@@ -464,6 +465,27 @@ export class VexFlowConverter {
         }
     }
 
+    public static StrokeTypeFromArpeggioType(arpeggioType: ArpeggioType): Vex.Flow.Stroke.Type {
+        switch (arpeggioType) {
+            case ArpeggioType.ARPEGGIO_DIRECTIONLESS:
+                return Vex.Flow.Stroke.Type.ARPEGGIO_DIRECTIONLESS;
+            case ArpeggioType.BRUSH_DOWN:
+                return Vex.Flow.Stroke.Type.BRUSH_UP; // TODO somehow up and down are mixed up in Vexflow right now
+            case ArpeggioType.BRUSH_UP:
+                return Vex.Flow.Stroke.Type.BRUSH_DOWN; // TODO somehow up and down are mixed up in Vexflow right now
+            case ArpeggioType.RASQUEDO_DOWN:
+                return Vex.Flow.Stroke.Type.RASQUEDO_UP;
+            case ArpeggioType.RASQUEDO_UP:
+                return Vex.Flow.Stroke.Type.RASQUEDO_DOWN;
+            case ArpeggioType.ROLL_DOWN:
+                return Vex.Flow.Stroke.Type.ROLL_UP; // TODO somehow up and down are mixed up in Vexflow right now
+            case ArpeggioType.ROLL_UP:
+                return Vex.Flow.Stroke.Type.ROLL_DOWN; // TODO somehow up and down are mixed up in Vexflow right now
+            default:
+                return Vex.Flow.Stroke.Type.ARPEGGIO_DIRECTIONLESS;
+        }
+    }
+
     /**
      * Convert a ClefInstruction to a string represention of a clef type in VexFlow.
      *

diff --git a/src/MusicalScore/Graphical/VexFlow/VexFlowMeasure.ts b/src/MusicalScore/Graphical/VexFlow/VexFlowMeasure.ts
@@ -943,7 +943,7 @@ export class VexFlowMeasure extends GraphicalMeasure {
                     // 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) {
-                        const type: Vex.Flow.Stroke.Type = arpeggio.type;
+                        const type: Vex.Flow.Stroke.Type = VexFlowConverter.StrokeTypeFromArpeggioType(arpeggio.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.

diff --git a/src/MusicalScore/ScoreIO/VoiceGenerator.ts b/src/MusicalScore/ScoreIO/VoiceGenerator.ts
@@ -26,7 +26,7 @@ import { CollectionUtil } from "../../Util/CollectionUtil";
 import { ArticulationReader } from "./MusicSymbolModules/ArticulationReader";
 import { SlurReader } from "./MusicSymbolModules/SlurReader";
 import { Notehead } from "../VoiceData/Notehead";
-import { Arpeggio } from "../VoiceData/Arpeggio";
+import { Arpeggio, ArpeggioType } from "../VoiceData/Arpeggio";
 import { NoteType } from "../VoiceData/NoteType";
 
 export class VoiceGenerator {
@@ -165,18 +165,18 @@ export class VoiceGenerator {
               }
             }
             if (!arpeggioAlreadyExists) {
-                let arpeggioType: Vex.Flow.Stroke.Type = Vex.Flow.Stroke.Type.ARPEGGIO_DIRECTIONLESS;
+                let arpeggioType: ArpeggioType = ArpeggioType.ARPEGGIO_DIRECTIONLESS;
                 const directionAttr: Attr = arpeggioNode.attribute("direction");
                 if (directionAttr !== null) {
                   switch (directionAttr.value) {
                     case "up":
-                      arpeggioType = Vex.Flow.Stroke.Type.ROLL_UP;
+                      arpeggioType = ArpeggioType.ROLL_UP;
                       break;
                     case "down":
-                      arpeggioType = Vex.Flow.Stroke.Type.ROLL_DOWN;
+                      arpeggioType = ArpeggioType.ROLL_DOWN;
                       break;
                     default:
-                      arpeggioType = Vex.Flow.Stroke.Type.ARPEGGIO_DIRECTIONLESS;
+                      arpeggioType = ArpeggioType.ARPEGGIO_DIRECTIONLESS;
                   }
                 }
 

diff --git a/src/MusicalScore/VoiceData/Arpeggio.ts b/src/MusicalScore/VoiceData/Arpeggio.ts
@@ -2,18 +2,29 @@ import { VoiceEntry } from "./VoiceEntry";
 import { Note } from "./Note";
 
 export class Arpeggio {
-    constructor(parentVoiceEntry: VoiceEntry, type: Vex.Flow.Stroke.Type = Vex.Flow.Stroke.Type.ARPEGGIO_DIRECTIONLESS) {
+    constructor(parentVoiceEntry: VoiceEntry, type: ArpeggioType = ArpeggioType.ARPEGGIO_DIRECTIONLESS) {
         this.parentVoiceEntry = parentVoiceEntry;
         this.type = type;
         this.notes = [];
     }
 
     public parentVoiceEntry: VoiceEntry;
     public notes: Note[];
-    public type: Vex.Flow.Stroke.Type;
+    public type: ArpeggioType;
 
     public addNote(note: Note): void {
         this.notes.push(note);
         note.Arpeggio = this;
     }
 }
+
+/** Corresponds to Vex.Flow.Stroke.Type for now. But we don't want VexFlow as a dependency here. */
+export enum ArpeggioType {
+    BRUSH_DOWN = 1,
+    BRUSH_UP,
+    ROLL_DOWN,
+    ROLL_UP,
+    RASQUEDO_DOWN,
+    RASQUEDO_UP,
+    ARPEGGIO_DIRECTIONLESS
+}