Merge pull request #1511 from rvilarl/fix/1504

Fix/1504 Consider actual clef width.

Author: ronyeh

src/clef.ts

@@ -143,7 +143,7 @@ export class Clef extends StaveModifier {
 
     this.setPosition(StaveModifierPosition.BEGIN);
     this.setType(type, size, annotation);
-    this.setWidth(Tables.currentMusicFont().lookupMetric(`clef_${this.size}.width`));
+    this.setWidth(Glyph.getWidth(this.clef.code, this.clef.point, `clef_${this.size}`));
     L('Creating clef:', type);
   }
 

src/stave.ts

@@ -854,6 +854,45 @@ export class Stave extends Element {
   }
 
   static formatBegModifiers(staves: Stave[]): void {
+    const adjustCategoryStartX = (category: Category) => {
+      let minStartX = 0;
+      // Calculate min start X for the category
+      staves.forEach((stave) => {
+        const modifiers = stave.getModifiers(StaveModifierPosition.BEGIN, category);
+        // Consider only the first instance
+        if (modifiers.length > 0 && modifiers[0].getX() > minStartX) minStartX = modifiers[0].getX();
+      });
+      let adjustX = 0;
+      staves.forEach((stave) => {
+        adjustX = 0;
+        const modifiers = stave.getModifiers(StaveModifierPosition.BEGIN, category);
+        // Calculate adjustement required for the stave
+        modifiers.forEach((modifier) => {
+          if (minStartX - modifier.getX() > adjustX) adjustX = minStartX - modifier.getX();
+        });
+        const allModifiers = stave.getModifiers(StaveModifierPosition.BEGIN);
+        let bAdjust = false;
+        // Apply adjustment to all the modifiers in and beyond the category
+        allModifiers.forEach((modifier) => {
+          if (modifier.getCategory() === category) bAdjust = true;
+          if (bAdjust && adjustX > 0) modifier.setX(modifier.getX() + adjustX);
+        });
+        // Apply adjustment also to note start.
+        stave.setNoteStartX(stave.getNoteStartX() + adjustX);
+      });
+    };
+
+    // Make sure that staves are formatted
+    staves.forEach((stave) => {
+      if (!stave.formatted) stave.format();
+    });
+    // Align Clefs
+    adjustCategoryStartX(Category.Clef);
+    // Align key signatures
+    adjustCategoryStartX(Category.KeySignature);
+    // Align time signatures
+    adjustCategoryStartX(Category.TimeSignature);
+
     let maxX = 0;
     // align note start
     staves.forEach((stave) => {
@@ -878,20 +917,5 @@ export class Stave extends Element {
         if ((modifier as Barline).getType() == BarlineType.REPEAT_BEGIN) modifier.setX(maxX);
       });
     });
-
-    maxX = 0;
-    // Align time signatures
-    staves.forEach((stave) => {
-      const modifiers = stave.getModifiers(StaveModifierPosition.BEGIN, Category.TimeSignature);
-      modifiers.forEach((modifier) => {
-        if (modifier.getX() > maxX) maxX = modifier.getX();
-      });
-    });
-    staves.forEach((stave) => {
-      const modifiers = stave.getModifiers(StaveModifierPosition.BEGIN, Category.TimeSignature);
-      modifiers.forEach((modifier) => {
-        modifier.setX(maxX);
-      });
-    });
   }
 }

tests/key_clef_tests.ts

@@ -81,13 +81,16 @@ function keys(options: TestOptions, contextBuilder: ContextBuilder): void {
       keySig = new KeySignature(keys[sharp]);
       keySig.addToStave(staves[i + clefs.length]);
     }
+  }
+
+  Stave.formatBegModifiers(staves);
 
+  for (i = 0; i < clefs.length; i++) {
     staves[i].setContext(ctx);
     staves[i].draw();
     staves[i + clefs.length].setContext(ctx);
     staves[i + clefs.length].draw();
   }
-
   ok(true, 'all pass');
 }
 

tests/percussion_tests.ts

@@ -100,7 +100,7 @@ function drawNotes(options: TestOptions, contextBuilder: ContextBuilder): void {
 function createSingleMeasureTest(setup: (f: Factory) => void) {
   return (options: TestOptions): void => {
     const f = VexFlowTests.makeFactory(options, 500);
-    const stave = f.Stave().addClef('percussion');
+    const stave = f.Stave().addClef('percussion').setTimeSignature('4/4');
     setup(f);
     f.Formatter().joinVoices(f.getVoices()).formatToStave(f.getVoices(), stave);
     f.draw();

tests/timesignature_tests.ts

@@ -153,6 +153,8 @@ function multiple(options: TestOptions, contextBuilder: ContextBuilder): void {
   const stave2 = new Stave(15, 110, 300).addClef('treble').addTimeSignature('4/4').setContext(ctx).draw();
   const stave3 = new Stave(15, 220, 300).addClef('bass').addTimeSignature('4/4').setContext(ctx).draw();
 
+  Stave.formatBegModifiers([stave1, stave2, stave3]);
+
   new StaveConnector(stave1, stave2).setType('single').setContext(ctx).draw();
   new StaveConnector(stave2, stave3).setType('single').setContext(ctx).draw();
   new StaveConnector(stave2, stave3).setType('brace').setContext(ctx).draw();