Merge pull request #119 from opensheetmusicdisplay/feature/staffLineElement

feat(clef): Improved conversion of ClefInstructions to VexFlow clefs

package.json

@@ -12,7 +12,8 @@
     "tslint": "tslint --type-check --project tsconfig.json \"src/**/*.ts\" \"test/**/*.ts\"",
     "typedoc": "typedoc --out ./build/docs --name OpenSheetMusicDisplay --module commonjs --target ES5 --mode file ./src/**/*.ts",
     "postinstall": "rimraf typings",
-    "prepublish": "grunt build:dist"
+    "prepublish": "grunt build:dist",
+    "start": "http-server build/demo"
   },
   "pre-commit": [
     "lint"

src/MusicalScore/Graphical/VexFlow/VexFlowConverter.ts

@@ -15,6 +15,7 @@ import {SystemLinesEnum} from "../SystemLinesEnum";
 import {FontStyles} from "../../../Common/Enums/FontStyles";
 import {Fonts} from "../../../Common/Enums/Fonts";
 import {OutlineAndFillStyleEnum} from "../DrawingEnums";
+import {Logging} from "../../../Common/Logging";
 
 /**
  * Helper class, which contains static methods which actually convert
@@ -161,28 +162,91 @@ export class VexFlowConverter {
     }
 
     /**
-     * Convert a ClefInstruction to a string representing a clef type in VexFlow
-     * @param clef
-     * @returns {string}
-     * @constructor
+     * Convert a ClefInstruction to a string represention of a clef type in VexFlow.
+     *
+     * @param clef The OSMD object to be converted representing the clef
+     * @param size The VexFlow size to be used. Can be `default` or `small`. As soon as
+     *             #118 is done, this parameter will be dispensable.
+     * @returns    A string representation of a VexFlow clef
+     * @see        https://github.com/0xfe/vexflow/blob/master/src/clef.js
+     * @see        https://github.com/0xfe/vexflow/blob/master/tests/clef_tests.js
      */
-    public static Clef(clef: ClefInstruction): {type: string, annotation: string} {
+    public static Clef(clef: ClefInstruction, size: string = "default"): { type: string, size: string, annotation: string } {
         let type: string;
-        let annotation: string = undefined;
+        let annotation: string;
 
+        // Make sure size is either "default" or "small"
+        if (size !== "default" && size !== "small") {
+            Logging.warn(`Invalid VexFlow clef size "${size}" specified. Using "default".`);
+            size = "default";
+        }
+
+        /*
+         * For all of the following conversions, OSMD uses line numbers 1-5 starting from
+         * the bottom, while VexFlow uses 0-4 starting from the top.
+         */
         switch (clef.ClefType) {
+
+            // G Clef
             case ClefEnum.G:
-                type = "treble";
+                switch (clef.Line) {
+                    case 1:
+                        type = "french"; // VexFlow line 4
+                        break;
+                    case 2:
+                        type = "treble"; // VexFlow line 3
+                        break;
+                    default:
+                        type = "treble";
+                        Logging.error(`Clef ${ClefEnum[clef.ClefType]} on line ${clef.Line} not supported by VexFlow. Using default value "${type}".`);
+                }
                 break;
+
+            // F Clef
             case ClefEnum.F:
-                type = "bass";
+                switch (clef.Line) {
+                  case 4:
+                      type = "bass"; // VexFlow line 1
+                      break;
+                  case 3:
+                      type = "baritone-f"; // VexFlow line 2
+                      break;
+                  case 5:
+                      type = "subbass"; // VexFlow line 0
+                      break;
+                  default:
+                      type = "bass";
+                      Logging.error(`Clef ${ClefEnum[clef.ClefType]} on line ${clef.Line} not supported by VexFlow. Using default value "${type}".`);
+                }
                 break;
+
+            // C Clef
             case ClefEnum.C:
-                type = "alto";
+                switch (clef.Line) {
+                  case 3:
+                      type = "alto"; // VexFlow line 2
+                      break;
+                  case 4:
+                      type = "tenor"; // VexFlow line 1
+                      break;
+                  case 1:
+                      type = "soprano"; // VexFlow line 4
+                      break;
+                  case 2:
+                      type = "mezzo-soprano"; // VexFlow line 3
+                      break;
+                  default:
+                      type = "alto";
+                      Logging.error(`Clef ${ClefEnum[clef.ClefType]} on line ${clef.Line} not supported by VexFlow. Using default value "${type}".`);
+                }
                 break;
+
+            // Percussion Clef
             case ClefEnum.percussion:
                 type = "percussion";
                 break;
+
+            // TAB Clef
             case ClefEnum.TAB:
                 type = "tab";
                 break;
@@ -198,7 +262,7 @@ export class VexFlowConverter {
                 break;
             default:
         }
-        return {type, annotation};
+        return { type, size, annotation };
     }
 
     /**

src/MusicalScore/Graphical/VexFlow/VexFlowMeasure.ts

@@ -100,8 +100,8 @@ export class VexFlowMeasure extends StaffMeasure {
      */
     public addClefAtBegin(clef: ClefInstruction): void {
         this.octaveOffset = clef.OctaveOffset;
-        let vfclef: {type: string, annotation: string} = VexFlowConverter.Clef(clef);
-        this.stave.addClef(vfclef.type, undefined, vfclef.annotation, Vex.Flow.Modifier.Position.BEGIN);
+        let vfclef: { type: string, size: string, annotation: string } = VexFlowConverter.Clef(clef, "default");
+        this.stave.addClef(vfclef.type, vfclef.size, vfclef.annotation, Vex.Flow.Modifier.Position.BEGIN);
         this.updateInstructionWidth();
     }
 
@@ -141,8 +141,8 @@ export class VexFlowMeasure extends StaffMeasure {
      * @param clef
      */
     public addClefAtEnd(clef: ClefInstruction): void {
-        let vfclef: {type: string, annotation: string} = VexFlowConverter.Clef(clef);
-        this.stave.setEndClef(vfclef.type, "small", vfclef.annotation);
+        let vfclef: { type: string, size: string, annotation: string } = VexFlowConverter.Clef(clef, "small");
+        this.stave.setEndClef(vfclef.type, vfclef.size, vfclef.annotation);
         this.updateInstructionWidth();
     }
 

test/MusicalScore/Graphical/VexFlow/VexFlowConverter_Clef_Test.ts

@@ -0,0 +1,76 @@
+import {IXmlElement} from "../../../../src/Common/FileIO/Xml";
+import {MusicSheet} from "../../../../src/MusicalScore/MusicSheet";
+import {MusicSheetReader} from "../../../../src/MusicalScore/ScoreIO/MusicSheetReader";
+
+/* tslint:disable:no-unused-expression */
+describe("Clef Converter MusicXML to VexFlow", () => {
+
+    let reader: MusicSheetReader;
+    let parser: DOMParser;
+
+    before((): void => {
+      reader = new MusicSheetReader();
+      parser = new DOMParser();
+    });
+
+    it("reads treble key", (done: MochaDone) => {
+      getMusicSheetWithClef("G").getStaffFromIndex(0);
+      done();
+    });
+
+    /**
+     * Simulates loading a [[MusicSheet]] with the specified clef.
+     *
+     * @see https://usermanuals.musicxml.com/MusicXML/Content/EL-MusicXML-clef.htm
+     */
+    function getMusicSheetWithClef(sign: string, line?: number, clefOcatveChange?: number, additional?: string, size?: string): MusicSheet {
+      let doc: Document = parser.parseFromString(getMusicXmlWithClef(sign, line, clefOcatveChange, additional, size), "text/xml");
+      chai.expect(doc).to.not.be.undefined;
+      let score: IXmlElement = new IXmlElement(doc.getElementsByTagName("score-partwise")[0]);
+      chai.expect(score).to.not.be.undefined;
+      return reader.createMusicSheet(score, "template.xml");
+    }
+
+    function getMusicXmlWithClef(sign: string, line?: number, clefOcatveChange?: number, additional?: string, size?: string): string {
+      // let modeElement: string = mode ? `<mode>${mode}</mode>` : "";
+      // let fifthsElement: string = fifths ? `<fifths>${fifths}</fifths>` : "";
+      return `<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+              <!DOCTYPE score-partwise PUBLIC
+                  "-//Recordare//DTD MusicXML 3.0 Partwise//EN"
+                  "http://www.musicxml.org/dtds/partwise.dtd">
+              <score-partwise version="3.0">
+                <part-list>
+                  <score-part id="P1">
+                    <part-name>Music</part-name>
+                  </score-part>
+                </part-list>
+                <part id="P1">
+                  <measure number="1">
+                    <attributes>
+                      <divisions>1</divisions>
+                      <key>
+
+                      </key>
+                      <time>
+                        <beats>4</beats>
+                        <beat-type>4</beat-type>
+                      </time>
+                      <clef>
+                        <sign>G</sign>
+                        <line>2</line>
+                      </clef>
+                    </attributes>
+                    <note>
+                      <pitch>
+                        <step>C</step>
+                        <octave>4</octave>
+                      </pitch>
+                      <duration>4</duration>
+                      <type>whole</type>
+                    </note>
+                  </measure>
+                </part>
+              </score-partwise>`;
+    }
+
+});