Browse Source

fix(Arpeggio): don't draw one-note arpeggios (#617)

previously, a one-note arpeggio had a very tall arpeggio stroke "into the sky".
these arpeggios are probably incorrectly given anyways,
as a one-note arpeggio doesn't make much sense.

we don't yet support arpeggios between different voice entries.
sschmid 5 years ago
parent
commit
5f7e1834f0

+ 11 - 3
src/MusicalScore/Graphical/VexFlow/VexFlowMeasure.ts

@@ -33,7 +33,7 @@ import {TechnicalInstruction} from "../../VoiceData/Instructions/TechnicalInstru
 import {PlacementEnum} from "../../VoiceData/Expressions/AbstractExpression";
 import {VexFlowGraphicalNote} from "./VexFlowGraphicalNote";
 import {AutoBeamOptions} from "../../../OpenSheetMusicDisplay/OSMDOptions";
-import {NoteType} from "../../VoiceData";
+import {NoteType, Arpeggio} from "../../VoiceData";
 
 export class VexFlowMeasure extends GraphicalMeasure {
     constructor(staff: Staff, staffLine: StaffLine = undefined, sourceMeasure: SourceMeasure = undefined) {
@@ -936,8 +936,16 @@ export class VexFlowMeasure extends GraphicalMeasure {
 
                 // add Arpeggio
                 if (voiceEntry.parentVoiceEntry && voiceEntry.parentVoiceEntry.Arpeggio !== undefined) {
-                    const type: Vex.Flow.Stroke.Type = voiceEntry.parentVoiceEntry.Arpeggio.type;
-                    vexFlowVoiceEntry.vfStaveNote.addStroke(0, new Vex.Flow.Stroke(type));
+                    const arpeggio: Arpeggio = voiceEntry.parentVoiceEntry.Arpeggio;
+                    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));
+                    } 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.`);
+                        // TODO: create new arpeggio with all the arpeggio's notes (arpeggio.notes), perhaps with GhostNotes in a new vfStaveNote. not easy.
+                    }
                 }
 
                 this.vfVoices[voice.VoiceId].addTickable(vexFlowVoiceEntry.vfStaveNote);

+ 25 - 14
src/MusicalScore/ScoreIO/VoiceGenerator.ts

@@ -150,23 +150,34 @@ export class VoiceGenerator {
           if (this.currentVoiceEntry.Arpeggio !== undefined) { // add note to existing Arpeggio
             currentArpeggio = this.currentVoiceEntry.Arpeggio;
           } else { // create new Arpeggio
-            let arpeggioType: Vex.Flow.Stroke.Type;
-            const directionAttr: Attr = arpeggioNode.attribute("direction");
-            if (directionAttr !== null) {
-              switch (directionAttr.value) {
-                case "up":
-                  arpeggioType = Vex.Flow.Stroke.Type.ROLL_UP;
-                  break;
-                case "down":
-                  arpeggioType = Vex.Flow.Stroke.Type.ROLL_DOWN;
-                  break;
-                default:
-                  arpeggioType = Vex.Flow.Stroke.Type.ARPEGGIO_DIRECTIONLESS;
+            let arpeggioAlreadyExists: boolean = false;
+            for (const voiceEntry of this.currentStaffEntry.VoiceEntries) {
+              if (voiceEntry.Arpeggio !== undefined) {
+                arpeggioAlreadyExists = true;
+                currentArpeggio = voiceEntry.Arpeggio;
+                // we already have an arpeggio in another voice, at the current timestamp. add the notes there.
+                break;
               }
             }
+            if (!arpeggioAlreadyExists) {
+                let arpeggioType: Vex.Flow.Stroke.Type = Vex.Flow.Stroke.Type.ARPEGGIO_DIRECTIONLESS;
+                const directionAttr: Attr = arpeggioNode.attribute("direction");
+                if (directionAttr !== null) {
+                  switch (directionAttr.value) {
+                    case "up":
+                      arpeggioType = Vex.Flow.Stroke.Type.ROLL_UP;
+                      break;
+                    case "down":
+                      arpeggioType = Vex.Flow.Stroke.Type.ROLL_DOWN;
+                      break;
+                    default:
+                      arpeggioType = Vex.Flow.Stroke.Type.ARPEGGIO_DIRECTIONLESS;
+                  }
+                }
 
-            currentArpeggio = new Arpeggio(this.currentVoiceEntry, arpeggioType);
-            this.currentVoiceEntry.Arpeggio = currentArpeggio;
+                currentArpeggio = new Arpeggio(this.currentVoiceEntry, arpeggioType);
+                this.currentVoiceEntry.Arpeggio = currentArpeggio;
+            }
           }
           currentArpeggio.addNote(this.currentNote);
         }