Merge pull request #1513 from 0xfe/timesig_simplifications

Time Signature simplifications

Author: rvilarl

src/fonts/bravura_metrics.ts

@@ -242,7 +242,7 @@ export const BravuraMetrics = {
 
   // These are for numeric digits, such as in time signatures
   digits: {
-    // used by timesig
+    // used by TimeSignature object
     shiftLine: -1,
     point: 34,
 

src/fonts/leland_metrics.ts

@@ -237,7 +237,7 @@ export const LelandMetrics = {
 
   // These are for numeric digits, such as in time signatures
   digits: {
-    // used by timesig
+    // used by TimeSignature objects
     shiftLine: -1,
     point: 34,
 

src/note.ts

@@ -488,7 +488,7 @@ export abstract class Note extends Tickable {
 
   /** Accessor to isDotted. */
   isDotted(): boolean {
-    return this.getModifiersByType('Dot').length > 0;
+    return this.getModifiersByType(Category.Dot).length > 0;
   }
 
   /** Accessor to hasStem. */

src/stavemodifier.ts

@@ -90,6 +90,15 @@ export class StaveModifier extends Element {
     return this;
   }
 
+  /**
+   * Runs setYShift() for the Glyph object so that it matches the position of line for
+   * the Stave provided.  A `customShift` can also be given (measured in the same units
+   * as `setYShift` not in lines) and this will be added after all other positions are
+   * calculated from the Stave.
+   *
+   * Note that this routine only sets the yShift; it does not actually "place" (meaning
+   * draw) the Glyph on the Stave.  Call .draw() afterwards to do that.
+   */
   placeGlyphOnLine(glyph: Glyph, stave: Stave, line?: number, customShift = 0): void {
     glyph.setYShift(stave.getYForLine(line ?? 0) - stave.getYForGlyphs() + customShift);
   }

src/timesigglyph.ts

@@ -104,7 +104,7 @@ export class TimeSignatureGlyph extends Glyph {
     y = stave.getYForLine(this.timeSignature.bottomLine);
     for (let i = 0; i < this.botGlyphs.length; ++i) {
       const glyph = this.botGlyphs[i];
-      this.timeSignature.placeGlyphOnLine(glyph, stave, 0);
+      this.timeSignature.placeGlyphOnLine(glyph, stave, this.timeSignature.getLine());
       Glyph.renderOutline(ctx, glyph.getMetrics().outline, this.scale, start_x + glyph.getMetrics().x_shift, y);
       start_x += defined(glyph.getMetrics().width);
     }

src/timesignature.ts

@@ -14,7 +14,7 @@ import { defined, RuntimeError } from './util';
 
 export interface TimeSignatureInfo {
   glyph: Glyph;
-  line?: number;
+  line: number;
   num: boolean;
 }
 
@@ -36,6 +36,11 @@ const assertIsValidTimeSig = (timeSpec: string) => {
   });
 };
 
+/**
+ * A TimeSignature is a StaveModifier that can make its appropriate Glyphs directly from
+ * a provided "timeSpec" such as "4/4", "C|" (cut time), or even something more advanced
+ * such as "3/4(6/8)" or "2/4+5/8".
+ */
 export class TimeSignature extends StaveModifier {
   static get CATEGORY(): string {
     return Category.TimeSignature;
@@ -57,10 +62,13 @@ export class TimeSignature extends StaveModifier {
   }
 
   point: number;
-  bottomLine: number;
-  topLine: number;
+  bottomLine: number; // bottomLine and topLine are used to calculate the position of the
+  topLine: number; // top row of digits in a numeric TimeSignature.
 
-  protected info: TimeSignatureInfo;
+  protected timeSpec: string = '4/4';
+  protected line: number = 0;
+  protected glyph!: Glyph;
+  protected is_numeric: boolean = true;
   protected validate_args: boolean;
 
   constructor(timeSpec: string = '4/4', customPadding = 15, validate_args = true) {
@@ -69,17 +77,23 @@ export class TimeSignature extends StaveModifier {
 
     const padding = customPadding;
 
+    // point must be defined before parsing spec.
     const musicFont = Tables.currentMusicFont();
     this.point = musicFont.lookupMetric('digits.point');
+
     const fontLineShift = musicFont.lookupMetric('digits.shiftLine', 0);
     this.topLine = 2 + fontLineShift;
     this.bottomLine = 4 + fontLineShift;
     this.setPosition(StaveModifierPosition.BEGIN);
-    this.info = this.parseTimeSpec(timeSpec);
-    this.setWidth(defined(this.info.glyph.getMetrics().width));
+    this.setTimeSig(timeSpec);
     this.setPadding(padding);
   }
 
+  /**
+   * Return TimeSignatureInfo given a string, consisting of line (number),
+   * num (boolean: same as TimeSignature.getIsNumeric()), and glyph (a Glyph or
+   * TimeSignatureGlyph object).
+   */
   parseTimeSpec(timeSpec: string): TimeSignatureInfo {
     if (timeSpec === 'C' || timeSpec === 'C|') {
       const { line, code, point } = TimeSignature.glyphs[timeSpec];
@@ -97,35 +111,115 @@ export class TimeSignature extends StaveModifier {
     const parts = timeSpec.split('/');
 
     return {
+      line: 0,
       num: true,
       glyph: this.makeTimeSignatureGlyph(parts[0] ?? '', parts[1] ?? ''),
     };
   }
 
-  makeTimeSignatureGlyph(topDigits: string, botDigits: string): Glyph {
+  /**
+   * Returns a new TimeSignatureGlyph (a Glyph subclass that knows how to draw both
+   * top and bottom digits along with plus signs etc.)
+   */
+  makeTimeSignatureGlyph(topDigits: string, botDigits: string): TimeSignatureGlyph {
+    // note that 'code' is ignored by TimeSignatureGlyph when rendering.
     return new TimeSignatureGlyph(this, topDigits, botDigits, 'timeSig0', this.point);
   }
 
+  /**
+   * Returns {line, num (=getIsNumeric), glyph} --
+   * but these can also be accessed directly w/ getters and setters.
+   */
   getInfo(): TimeSignatureInfo {
-    return this.info;
+    const { line, is_numeric, glyph } = this;
+    return { line, num: is_numeric, glyph };
   }
 
+  /**
+   * Set a new time signature specification without changing customPadding, etc.
+   *
+   * The getter for this is `getTimeSpec` not `getTimeSig`.
+   */
   setTimeSig(timeSpec: string): this {
-    this.info = this.parseTimeSpec(timeSpec);
+    this.timeSpec = timeSpec;
+    const info = this.parseTimeSpec(timeSpec);
+    this.setGlyph(info.glyph);
+    this.is_numeric = info.num;
+    this.line = info.line;
     return this;
   }
 
+  /**
+   * Return the timeSpec (such as '4/4' or 'C|' or even '2/4+3/8') of the TimeSignature
+   */
+  getTimeSpec(): string {
+    return this.timeSpec;
+  }
+
+  /**
+   * Return the staff line that the TimeSignature sits on.  Generally 0 for numerator/
+   * denominator time signatures such as 3/4 and 2 for cut/common.
+   */
+  getLine(): number {
+    return this.line;
+  }
+
+  /**
+   * Set the line number that the TimeSignature sits on.  Half-values are acceptable
+   * for spaces, etc. Can be altered, for instance, for signatures that sit above the
+   * staff in large orchestral scores.
+   */
+  setLine(line: number) {
+    this.line = line;
+  }
+
+  /**
+   * Get the Glyph object used to create the time signature.  Numeric time signatures
+   * such as 3/8 have a composite Glyph stored as a single Glyph object.
+   */
+  getGlyph(): Glyph {
+    return this.glyph;
+  }
+
+  /**
+   * Set the Glyph object used to draw the time signature, and update the width of the
+   * TimeSignature to match.  The Glyph must define width in its metrics.
+   */
+  setGlyph(glyph: Glyph) {
+    this.glyph = glyph;
+    this.setWidth(defined(this.glyph.getMetrics().width));
+  }
+
+  /**
+   * Return a boolean on whether this TimeSignature is drawn with one or more numbers
+   * (such as 4/4) or not (as in cut time).
+   */
+  getIsNumeric(): boolean {
+    return this.is_numeric;
+  }
+
+  /**
+   * Set whether this TimeSignature is drawn with one or more numbers.
+   */
+  setIsNumeric(isNumeric: boolean) {
+    this.is_numeric = isNumeric;
+  }
+
+  /**
+   * Draw the time signature on a Stave using its RenderContext.  Both setStave
+   * and setContext must already be run.
+   */
   draw(): void {
     const stave = this.checkStave();
     const ctx = stave.checkContext();
     this.setRendered();
 
     this.applyStyle(ctx);
     ctx.openGroup('timesignature', this.getAttribute('id'));
-    this.info.glyph.setStave(stave);
-    this.info.glyph.setContext(ctx);
-    this.placeGlyphOnLine(this.info.glyph, stave, this.info.line);
-    this.info.glyph.renderToStave(this.x);
+    this.glyph.setStave(stave);
+    this.glyph.setContext(ctx);
+    this.placeGlyphOnLine(this.glyph, stave, this.line);
+    this.glyph.renderToStave(this.x);
     ctx.closeGroup();
     this.restoreStyle(ctx);
   }

src/timesignote.ts

@@ -3,22 +3,21 @@
 
 import { ModifierContext } from './modifiercontext';
 import { Note } from './note';
-import { TimeSignature, TimeSignatureInfo } from './timesignature';
+import { TimeSignature } from './timesignature';
 import { Category } from './typeguard';
 
 export class TimeSigNote extends Note {
   static get CATEGORY(): string {
     return Category.TimeSigNote;
   }
 
-  protected timeSigInfo: TimeSignatureInfo;
+  protected timeSig: TimeSignature;
 
   constructor(timeSpec: string, customPadding?: number) {
     super({ duration: 'b' });
 
-    const timeSignature = new TimeSignature(timeSpec, customPadding);
-    this.timeSigInfo = timeSignature.getInfo();
-    this.setWidth(this.timeSigInfo.glyph.getMetrics().width);
+    this.timeSig = new TimeSignature(timeSpec, customPadding);
+    this.setWidth(this.timeSig.getGlyph().getMetrics().width);
 
     // Note properties
     this.ignore_ticks = true;
@@ -41,12 +40,13 @@ export class TimeSigNote extends Note {
     const ctx = this.checkContext();
     this.setRendered();
 
-    if (!this.timeSigInfo.glyph.getContext()) {
-      this.timeSigInfo.glyph.setContext(ctx);
+    const tsGlyph = this.timeSig.getGlyph();
+    if (!tsGlyph.getContext()) {
+      tsGlyph.setContext(ctx);
     }
 
-    this.timeSigInfo.glyph.setStave(stave);
-    this.timeSigInfo.glyph.setYShift(stave.getYForLine(2) - stave.getYForGlyphs());
-    this.timeSigInfo.glyph.renderToStave(this.getAbsoluteX());
+    tsGlyph.setStave(stave);
+    tsGlyph.setYShift(stave.getYForLine(2) - stave.getYForGlyphs());
+    tsGlyph.renderToStave(this.getAbsoluteX());
   }
 }

tests/timesignature_tests.ts

@@ -29,6 +29,7 @@ const TimeSignatureTests = {
 
 function parser(): void {
   const timeSig = new TimeSignature();
+  equal(timeSig.getTimeSpec(), '4/4', 'default time signature is 4/4');
 
   const mustFail = ['asdf', '123/', '/10', '/', '4567', 'C+', '1+', '+1', '(3+', '+3)', '()', '(+)'];
   mustFail.forEach((invalidString) => {
@@ -38,6 +39,14 @@ function parser(): void {
   const mustPass = ['4/4', '10/12', '1/8', '1234567890/1234567890', 'C', 'C|', '+'];
   mustPass.forEach((validString) => timeSig.parseTimeSpec(validString));
 
+  timeSig.setTimeSig('4/4');
+  equal(timeSig.getIsNumeric(), true, '4/4 is numeric');
+  equal(timeSig.getLine(), 0, 'digits are on line 0');
+  timeSig.setTimeSig('C|');
+  equal(timeSig.getTimeSpec(), 'C|', 'timeSpec changed to C|');
+  equal(timeSig.getIsNumeric(), false, 'cut time is not numeric');
+  equal(timeSig.getLine(), 2, 'cut/common are on line 2');
+
   ok(true, 'all pass');
 }