Merge pull request #1037 from AaronDavidNewman/master

Correct staff padding to include end padding
0xfe · Jun 14, 2021 · 736b654 · 736b654
2 parents 8e33074 + 4e6df4f
commit 736b654
Show file tree

Hide file tree

Showing 7 changed files with 22 additions and 6 deletions.
diff --git a/src/fonts/bravura_metrics.ts b/src/fonts/bravura_metrics.ts
@@ -4,6 +4,8 @@ export const BravuraMetrics = {
 
   stave: {
     padding: 12,
+    endPaddingMax: 10,
+    endPaddingMin: 5
   },
 
   clef: {

diff --git a/src/fonts/gonville_metrics.ts b/src/fonts/gonville_metrics.ts
@@ -3,6 +3,8 @@ export const GonvilleMetrics = {
   smufl: false,
   stave: {
     padding: 12,
+    endPaddingMax: 10,
+    endPaddingMin: 5
   },
 
   clef: {

diff --git a/src/fonts/petaluma_metrics.ts b/src/fonts/petaluma_metrics.ts
@@ -4,6 +4,8 @@ export const PetalumaMetrics = {
 
   stave: {
     padding: 15,
+    endPaddingMax: 15,
+    endPaddingMin: 7
   },
 
   clef: {

diff --git a/src/formatter.ts b/src/formatter.ts
@@ -17,7 +17,6 @@
 // See `tests/formatter_tests.js` for usage examples. The helper functions included
 // here (`FormatAndDraw`, `FormatAndDrawTab`) also serve as useful usage examples.
 
-import { Vex } from './vex';
 import { RuntimeError, midLine, log } from './util';
 import { Beam } from './beam';
 import { Flow } from './tables';
@@ -693,10 +692,13 @@ export class Formatter {
       firstContext.getMetrics().totalLeftPx;
     let targetWidth = adjustedJustifyWidth;
     let actualWidth = shiftToIdealDistances(calculateIdealDistances(targetWidth));
-    const maxX = adjustedJustifyWidth + lastContext.getMetrics().notePx;
+    const musicFont = Flow.DEFAULT_FONT_STACK[0];
+    const paddingMax = musicFont.lookupMetric('stave.endPaddingMax');
+    const paddingMin = musicFont.lookupMetric('stave.endPaddingMin');
+    const maxX = adjustedJustifyWidth + lastContext.getMetrics().notePx - paddingMin;
 
     let iterations = this.formatterOptions.maxIterations;
-    while (actualWidth > maxX && iterations > 0) {
+    while ((actualWidth > maxX && iterations > 0) || (actualWidth + paddingMax < maxX && iterations > 1)) {
       // If we couldn't fit all the notes into the jusification width, it's because the softmax-scaled
       // widths between different durations differ across stave (e.g., 1 quarter note is not the same pixel-width
       // as 4 16th-notes). Run another pass, now that we know how much to justify.
@@ -917,7 +919,7 @@ export class Formatter {
     const options: FormatOptions = { padding: 10, /*stave,*/ context: stave.getContext(), ...optionsParam };
 
     // eslint-disable-next-line
-    const justifyWidth = stave.getNoteEndX() - stave.getNoteStartX() - options.padding!;
+    const justifyWidth = stave.getNoteEndX() - stave.getNoteStartX() - Stave.defaultPadding;
     L('Formatting voices to width: ', justifyWidth);
     return this.format(voices, justifyWidth, options);
   }

diff --git a/src/stave.ts b/src/stave.ts
@@ -62,6 +62,13 @@ export class Stave extends Element {
   protected bounds: Bounds;
   protected readonly modifiers: StaveModifier[];
 
+  // This is the sum of the padding that normally goes on left + right of a stave during
+  // drawing.  Used to size staves correctly with content width
+  static get defaultPadding(): number {
+    const musicFont = Flow.DEFAULT_FONT_STACK[0];
+    return musicFont.lookupMetric('stave.padding') + musicFont.lookupMetric('stave.endPaddingMax');
+  }
+
   constructor(x: number, y: number, width: number, options?: StaveOptions) {
     super();
     this.setAttribute('type', 'Stave');

diff --git a/src/system.js b/src/system.js
@@ -6,6 +6,7 @@
 
 import { Element } from './element';
 import { Factory } from './factory';
+import { Stave } from './stave';
 import { Formatter } from './formatter';
 import { Note } from './note';
 
@@ -117,7 +118,7 @@ export class System extends Element {
     // Update the start position of all staves.
     this.parts.forEach((part) => part.stave.setNoteStartX(startX));
     const justifyWidth = this.options.noPadding
-      ? this.options.width - this.options.x
+      ? this.options.width - Stave.defaultPadding
       : this.options.width - (startX - this.options.x) - this.musicFont.lookupMetric('stave.padding');
 
     formatter.format(allVoices, this.options.noJustification ? 0 : justifyWidth);

diff --git a/tests/formatter_tests.js b/tests/formatter_tests.js
@@ -530,7 +530,7 @@ const FormatterTests = (function () {
     },
 
     mixTime: function (options) {
-      var vf = VF.Test.makeFactory(options, 420, 250);
+      var vf = VF.Test.makeFactory(options, 400 + VF.Stave.defaultPadding, 250);
       vf.getContext().scale(0.8, 0.8);
       var score = vf.EasyScore();
       var system = vf.System({