Rework head and stem rendering for custom glyphs - #752

package.json

@@ -35,7 +35,6 @@
     "grunt-contrib-concat": "^1.0.1",
     "grunt-contrib-copy": "^1.0.0",
     "grunt-contrib-qunit": "^3.1.0",
-    "grunt-contrib-uglify": "^4.0.1",
     "grunt-contrib-watch": "^1.1.0",
     "grunt-docco": "^0.5.0",
     "grunt-eslint": "^22.0.0",

src/note.js

@@ -101,6 +101,7 @@ export class Note extends Tickable {
     this.duration = initData.duration;
     this.dots = initData.dots;
     this.noteType = initData.type;
+    this.customTypes = initData.customTypes;
 
     if (note_struct.duration_override) {
       // Custom duration
@@ -114,6 +115,7 @@ export class Note extends Tickable {
 
     // Get the glyph code for this note from the font.
     this.glyph = Flow.durationToGlyph(this.duration, this.noteType);
+    this.customGlyphs = this.customTypes.map(t => Flow.durationToGlyph(this.duration, t));
 
     if (this.positions && (typeof (this.positions) !== 'object' || !this.positions.length)) {
       throw new Vex.RuntimeError('BadArguments', 'Note keys must be array type.');

src/notehead.js

@@ -91,10 +91,12 @@ export class NoteHead extends Note {
     }
 
     this.glyph_code = this.glyph.code_head;
-    this.x_shift = head_options.x_shift;
+    this.x_shift = head_options.x_shift || 0;
     if (head_options.custom_glyph_code) {
       this.custom_glyph = true;
       this.glyph_code = head_options.custom_glyph_code;
+      this.stem_up_x_offset = head_options.stem_up_x_offset || 0;
+      this.stem_down_x_offset = head_options.stem_down_x_offset || 0;
     }
 
     this.style = head_options.style;
@@ -189,7 +191,12 @@ export class NoteHead extends Note {
     this.setRendered();
 
     const ctx = this.context;
-    const head_x = this.getAbsoluteX();
+    let head_x = this.getAbsoluteX();
+    if (this.custom_glyph) {
+      // head_x += this.x_shift;
+      head_x += this.stem_direction === Stem.UP ? this.stem_up_x_offset : this.stem_down_x_offset;
+    }
+
     const y = this.y;
 
     L("Drawing note head '", this.note_type, this.duration, "' at", head_x, y);

src/stavenote.js

@@ -270,15 +270,15 @@ export class StaveNote extends StemmableNote {
       //
       // We also extend the y for each note by a half notehead because the
       // notehead's origin is centered
-      const topNotBottomY = topNote
+      const topNoteBottomY = topNote
         .getStave()
         .getYForLine(5 - topKeys[0].line + HALF_NOTEHEAD_HEIGHT);
 
       const bottomNoteTopY = bottomNote
         .getStave()
         .getYForLine(5 - bottomKeys[bottomKeys.length - 1].line - HALF_NOTEHEAD_HEIGHT);
 
-      const areNotesColliding = bottomNoteTopY - topNotBottomY < 0;
+      const areNotesColliding = bottomNoteTopY - topNoteBottomY < 0;
 
       if (areNotesColliding) {
         xShift = topNote.getVoiceShiftWidth() + 2;
@@ -434,6 +434,8 @@ export class StaveNote extends StemmableNote {
         custom_glyph_code: noteProps.code,
         glyph_font_scale: this.render_options.glyph_font_scale,
         x_shift: noteProps.shift_right,
+        stem_up_x_offset: noteProps.stem_up_x_offset,
+        stem_down_x_offset: noteProps.stem_down_x_offset,
         line: noteProps.line,
       });
 
@@ -1094,7 +1096,6 @@ export class StaveNote extends StemmableNote {
     });
   }
 
-  // Render the stem onto the canvas
   drawStem(stemStruct) {
     // GCR TODO: I can't find any context in which this is called with the stemStruct
     // argument in the codebase or tests. Nor can I find a case where super.drawStem

src/stem.js

@@ -54,13 +54,16 @@ export class Stem extends Element {
     this.isStemlet = options.isStemlet || false;
     this.stemletHeight = options.stemletHeight || 0;
 
-    // Changing where the stem meets the head
-    this.stem_up_y_offset = options.stem_up_y_offset || 0;
-    this.stem_down_y_offset = options.stem_down_y_offset || 0;
-
     // Use to adjust the rendered height without affecting
     // the results of `.getExtents()`
     this.renderHeightAdjustment = 0;
+    this.setOptions(options);
+  }
+
+  setOptions(options) {
+    // Changing where the stem meets the head
+    this.stem_up_y_offset = options.stem_up_y_offset || 0;
+    this.stem_down_y_offset = options.stem_down_y_offset || 0;
   }
 
   // Set the x bounds for the default notehead
@@ -102,6 +105,7 @@ export class Stem extends Element {
     const isStemUp = this.stem_direction === Stem.UP;
     const ys = [this.y_top, this.y_bottom];
     const stemHeight = Stem.HEIGHT + this.stem_extension;
+
     const innerMostNoteheadY = (isStemUp ? Math.min : Math.max)(...ys);
     const outerMostNoteheadY = (isStemUp ? Math.max : Math.min)(...ys);
     const stemTipY = innerMostNoteheadY + (stemHeight * -this.stem_direction);

src/stemmablenote.js

@@ -44,6 +44,15 @@ export class StemmableNote extends Note {
     }
   }
 
+  // Get the glyph associated with the top key of this note
+  getTopGlyph() {
+    if (this.getStemDirection() === Stem.DOWN) {
+      return this.customGlyphs[this.customGlyphs.length - 1];
+    } else {
+      return this.customGlyphs[0];
+    }
+  }
+
   // Get the full length of stem
   getStemLength() {
     return Stem.HEIGHT + this.getStemExtension();
@@ -99,6 +108,11 @@ export class StemmableNote extends Note {
     if (this.stem) {
       this.stem.setDirection(direction);
       this.stem.setExtension(this.getStemExtension());
+      const glyph = this.getTopGlyph() || this.getGlyph();
+      this.stem.setOptions({
+        stem_up_y_offset: glyph.stem_up_y_offset,
+        stem_down_y_offset: glyph.stem_down_y_offset
+      });
     }
 
     this.reset();
@@ -110,6 +124,7 @@ export class StemmableNote extends Note {
     if (this.preFormatted) {
       this.preFormat();
     }
+
     return this;
   }
 

src/tables.js

@@ -100,14 +100,14 @@ Flow.keyProperties = (key, clef, params) => {
     : null;
 
   /* Check if the user specified a glyph. */
-  let code = value.code;
-  let shift_right = value.shift_right;
+  const code = value.code;
+  const shift_right = value.shift_right;
+  let extraProps = {};
   if (pieces.length > 2 && pieces[2]) {
     const glyph_name = pieces[2].toUpperCase();
     const note_glyph = Flow.keyProperties.note_glyph[glyph_name];
     if (note_glyph) {
-      code = note_glyph.code;
-      shift_right = note_glyph.shift_right;
+      extraProps = note_glyph;
     }
   }
 
@@ -121,6 +121,7 @@ Flow.keyProperties = (key, clef, params) => {
     stroke,
     shift_right,
     displaced: false,
+    ...extraProps,
   };
 };
 
@@ -177,9 +178,17 @@ Flow.keyProperties.note_values = {
   },
 };
 
+// Custom note heads
 Flow.keyProperties.note_glyph = {
   /* Diamond */
-  'D0': { code: 'v27', shift_right: -0.5 },
+  'D0': {
+    code: 'v27',
+    shift_right: 0, // deprecated for stem_{up,down}_x_offset
+    stem_up_x_offset: 0,
+    stem_down_x_offset: 0,
+    stem_up_y_offset: -1,
+    stem_down_y_offset: 0
+  },
   'D1': { code: 'v2d', shift_right: -0.5 },
   'D2': { code: 'v22', shift_right: -0.5 },
   'D3': { code: 'v70', shift_right: -0.5 },
@@ -191,14 +200,28 @@ Flow.keyProperties.note_glyph = {
   'T3': { code: 'v7d', shift_right: 0.5, stem_up_y_offset: -4, stem_down_y_offset: 4 },
 
   /* Cross */
-  'X0': { code: 'v92', shift_right: -2, stem_up_y_offset: 4, stem_down_y_offset: 4 },
+  'X0': {
+    code: 'v92',
+    stem_up_x_offset: -2,
+    stem_down_x_offset: 0,
+    stem_up_y_offset: 4,
+    stem_down_y_offset: 4
+  },
   'X1': { code: 'v95', shift_right: -0.5, stem_up_y_offset: 4, stem_down_y_offset: 4 },
   'X2': { code: 'v3e', shift_right: 0.5, stem_up_y_offset: 4, stem_down_y_offset: 4 },
-  'X3': { code: 'v3b', shift_right: -2, stem_up_y_offset: 2, stem_down_y_offset: 2 },
+  'X3': {
+    code: 'v3b',
+    shift_right: 0,
+    stem_up_x_offset: -1.2,
+    stem_down_x_offset: 0,
+    stem_up_y_offset: -1,
+    stem_down_y_offset: 2
+  },
 
   /* Square */
   'S1': { code: 'vd3', shift_right: 0 },
   'S2': { code: 'vd2', shift_right: 0 },
+
   /* Rectangle */
   'R1': { code: 'vd5', shift_right: 0 },
   'R2': { code: 'vd4', shift_right: 0 },
@@ -474,25 +497,31 @@ Flow.parseNoteData = noteData => {
   }
 
   let type = noteData.type;
+  const customTypes = [];
 
   if (type) {
     if (!(type === 'n' || type === 'r' || type === 'h' || type === 'm' || type === 's')) {
       return null;
     }
   } else {
-    type = durationStringData.type;
+    type = durationStringData.type || 'n';
 
     // If we have keys, try and check if we've got a custom glyph
     if (noteData.keys !== undefined) {
-      const result = noteData.keys[0].split('/');
-
-      // We have a custom glyph specified after the note eg. /X2
-      if (result && result.length === 3) {
-        type = result[2]; // Set the type to the custom note head
-      }
-    }
-    if (!type) {
-      type = 'n';
+      // FIXME: We're taking the custom note head data of the bottom most note
+      // in both the stem-up and stem-down cases. This causes formatting errors
+      // for stem-up custom note heads, where the shift parameters are not
+      // respected.
+      noteData.keys.forEach((k, i) => {
+        const result = k.split('/');
+        // We have a custom glyph specified after the note eg. /X2
+        if (result && result.length === 3) {
+          type = result[2]; // Set the type to the custom note head
+          customTypes[i] = type;
+        }
+      });
+
+      type = customTypes[0] || type;
     }
   }
 
@@ -514,6 +543,7 @@ Flow.parseNoteData = noteData => {
   return {
     duration: durationStringData.duration,
     type,
+    customTypes,
     dots,
     ticks,
   };
@@ -606,6 +636,8 @@ Flow.durationToGlyph = (duration, type) => {
       code_head: customGlyphTypeProperties.code,
       stem_up_y_offset: customGlyphTypeProperties.stem_up_y_offset,
       stem_down_y_offset: customGlyphTypeProperties.stem_down_y_offset,
+      stem_up_x_offset: customGlyphTypeProperties.stem_up_x_offset,
+      stem_down_x_offset: customGlyphTypeProperties.stem_down_x_offset,
     };
   }
 

tests/notehead_tests.js

@@ -9,6 +9,7 @@ VF.Test.NoteHead = (function() {
       QUnit.module('NoteHead');
       VF.Test.runTests('Basic', VF.Test.NoteHead.basic);
       VF.Test.runTests('Various Heads', VF.Test.NoteHead.variousHeads);
+      VF.Test.runTests('Drum Chord Heads', VF.Test.NoteHead.drumChordHeads);
       VF.Test.runTests('Bounding Boxes', VF.Test.NoteHead.basicBoundingBoxes);
     },
 
@@ -117,6 +118,47 @@ VF.Test.NoteHead = (function() {
       }
     },
 
+    drumChordHeads: function(options, contextBuilder) {
+      var notes = [
+        { keys: ['a/4/d0', 'g/5/x3'], duration: '4' },
+        { keys: ['a/4/x3', 'g/5/d0'], duration: '4' },
+        { keys: ['a/4/d1', 'g/5/x2'], duration: '4' },
+        { keys: ['a/4/x2', 'g/5/d1'], duration: '4' },
+        { keys: ['a/4/d2', 'g/5/x1'], duration: '4' },
+        { keys: ['a/4/x1', 'g/5/d2'], duration: '4' },
+        { keys: ['a/4/d3', 'g/5/x0'], duration: '4' },
+        { keys: ['a/4/x0', 'g/5/d3'], duration: '4' },
+
+        { keys: ['a/4/t0', 'g/5/s1'], duration: '4' },
+        { keys: ['a/4/s1', 'g/5/t0'], duration: '4' },
+        { keys: ['a/4/t1', 'g/5/s2'], duration: '4' },
+        { keys: ['a/4/s2', 'g/5/t1'], duration: '4' },
+        { keys: ['a/4/t2', 'g/5/r1'], duration: '4' },
+        { keys: ['a/4/r1', 'g/5/t2'], duration: '4' },
+        { keys: ['a/4/t3', 'g/5/r2'], duration: '4' },
+        { keys: ['a/4/r2', 'g/5/t3'], duration: '4' },
+      ];
+
+      var ctx = new contextBuilder(options.elementId, notes.length * 25 + 100, 240);
+
+      // Draw two staves, one with up-stems and one with down-stems.
+      for (var h = 0; h < 2; ++h) {
+        var stave = new VF.Stave(10, 10 + h * 120, notes.length * 25 + 75)
+          .addClef('percussion')
+          .setContext(ctx)
+          .draw();
+
+        for (var i = 0; i < notes.length; ++i) {
+          var note = notes[i];
+          note.stem_direction = (h === 0 ? -1 : 1);
+          var staveNote = NoteHead.showNote(note, stave, ctx, (i + 1) * 25);
+
+          ok(staveNote.getX() > 0, 'Note ' + i + ' has X value');
+          ok(staveNote.getYs().length > 0, 'Note ' + i + ' has Y values');
+        }
+      }
+    },
+
     basicBoundingBoxes: function(options, contextBuilder) {
       options.contextBuilder = contextBuilder;
       var c = VF.Test.NoteHead.setupContext(options, 350, 250);