fix(Rest note positioning): Rest notes respect the display-step and display-octave tags now (#759)

squash-merge PR #852

* Rest notes respect the display-step and display-octave tags now

* fix rest note shift for c clef

Co-authored-by: sschmid <s.schmid@phonicscore.com>

src/MusicalScore/Graphical/VexFlow/VexFlowConverter.ts

@@ -117,8 +117,19 @@ export class VexFlowConverter {
     public static pitch(note: VexFlowGraphicalNote, pitch: Pitch): [string, string, ClefInstruction] {
         const fund: string = NoteEnum[pitch.FundamentalNote].toLowerCase();
         const acc: string = Pitch.accidentalVexflow(pitch.Accidental);
-        // The octave seems to need a shift of three FIXME?
-        const octave: number = pitch.Octave - note.Clef().OctaveOffset + 3;
+        //FIXME: The octave seems to need a shift of three?
+        //FIXME: Also rests seem to use different offsets depending on the clef.
+        let fixmeOffset: number = 3;
+        if (note.sourceNote.isRest()) {
+            fixmeOffset = 0;
+            if (note.Clef().ClefType === ClefEnum.F) {
+                fixmeOffset = 2;
+            }
+            if (note.Clef().ClefType === ClefEnum.C) {
+                fixmeOffset = 2;
+            }
+        }
+        const octave: number = pitch.Octave - note.Clef().OctaveOffset + fixmeOffset;
         const notehead: Notehead = note.sourceNote.Notehead;
         let noteheadCode: string = "";
         if (notehead) {
@@ -197,7 +208,13 @@ export class VexFlowConverter {
             // if it is a rest:
             if (note.sourceNote.isRest()) {
                 isRest = true;
-                keys = ["b/4"];
+                if (note.sourceNote.Pitch) {
+                    const restPitch: [string, string, ClefInstruction] = (note as VexFlowGraphicalNote).vfpitch;
+                    keys = [restPitch[0]];
+                    break;
+                } else {
+                    keys = ["b/4"];
+                }
                 // TODO do collision checking, place rest e.g. either below staff (A3, for stem direction below voice) or above (C5)
                 // if it is a full measure rest:
                 if (note.parentVoiceEntry.parentStaffEntry.parentMeasure.parentSourceMeasure.Duration.RealValue <= frac.RealValue) {

src/MusicalScore/Graphical/VexFlow/VexFlowGraphicalNote.ts

@@ -20,7 +20,7 @@ export class VexFlowGraphicalNote extends GraphicalNote {
         this.octaveShift = octaveShift;
         if (note.Pitch) {
             // TODO: Maybe shift to Transpose function when available
-            const drawPitch: Pitch = OctaveShift.getPitchFromOctaveShift(note.Pitch, octaveShift);
+            const drawPitch: Pitch = note.isRest() ? note.Pitch : OctaveShift.getPitchFromOctaveShift(note.Pitch, octaveShift);
             this.vfpitch = VexFlowConverter.pitch(this, drawPitch);
             this.vfpitch[1] = undefined;
         }

src/MusicalScore/ScoreIO/VoiceGenerator.ts

@@ -114,9 +114,10 @@ export class VoiceGenerator {
     this.currentStaffEntry = parentStaffEntry;
     this.currentMeasure = parentMeasure;
     //log.debug("read called:", restNote);
+
     try {
       this.currentNote = restNote
-        ? this.addRestNote(noteDuration, noteTypeXml, printObject, isCueNote, noteheadColorXml)
+        ? this.addRestNote(noteNode.element("rest"), noteDuration, noteTypeXml, printObject, isCueNote, noteheadColorXml)
         : this.addSingleNote(noteNode, noteDuration, noteTypeXml, typeDuration, normalNotes, chord, guitarPro,
                              printObject, isCueNote, stemDirectionXml, tremoloStrokes, stemColorXml, noteheadColorXml, vibratoStrokes);
       // read lyrics
@@ -477,9 +478,17 @@ export class VoiceGenerator {
    * @param divisions
    * @returns {Note}
    */
-  private addRestNote(noteDuration: Fraction, noteTypeXml: NoteType, printObject: boolean, isCueNote: boolean, noteheadColorXml: string): Note {
+  private addRestNote(node: IXmlElement, noteDuration: Fraction, noteTypeXml: NoteType,
+                      printObject: boolean, isCueNote: boolean, noteheadColorXml: string): Note {
     const restFraction: Fraction = Fraction.createFromFraction(noteDuration);
-    const restNote: Note = new Note(this.currentVoiceEntry, this.currentStaffEntry, restFraction, undefined);
+    const displayStep: IXmlElement = node.element("display-step");
+    const octave: IXmlElement = node.element("display-octave");
+    let pitch: Pitch = undefined;
+    if (displayStep && octave) {
+        const noteStep: NoteEnum = NoteEnum[displayStep.value.toUpperCase()];
+        pitch = new Pitch(noteStep, parseInt(octave.value, 10), AccidentalEnum.NONE);
+    }
+    const restNote: Note = new Note(this.currentVoiceEntry, this.currentStaffEntry, restFraction, pitch, true);
     restNote.NoteTypeXml = noteTypeXml;
     restNote.PrintObject = printObject;
     restNote.IsCueNote = isCueNote;

src/MusicalScore/VoiceData/Note.ts

@@ -17,11 +17,12 @@ import {NoteType} from "./NoteType";
  */
 export class Note {
 
-    constructor(voiceEntry: VoiceEntry, parentStaffEntry: SourceStaffEntry, length: Fraction, pitch: Pitch) {
+    constructor(voiceEntry: VoiceEntry, parentStaffEntry: SourceStaffEntry, length: Fraction, pitch: Pitch, isRest?: boolean) {
         this.voiceEntry = voiceEntry;
         this.parentStaffEntry = parentStaffEntry;
         this.length = length;
         this.pitch = pitch;
+        this.isRestFlag = isRest ?? false;
         if (pitch) {
             this.halfTone = pitch.getHalfTone();
         } else {
@@ -43,6 +44,7 @@ export class Note {
     private noteTypeXml: NoteType;
     /** The amount of notes the tuplet of this note (if there is one) replaces. */
     private normalNotes: number;
+    private isRestFlag: boolean;
     /**
      * The untransposed (!!!) source data.
      */
@@ -216,7 +218,7 @@ export class Note {
     }
 
     public isRest(): boolean {
-        return this.Pitch === undefined || this.Pitch === null;
+        return this.isRestFlag;
     }
 
     /** Note: May be dangerous to use if ParentStaffEntry.VerticalContainerParent etc is not set.