Address issue 1167

If there is plenty of whitespace available, increase the right-justification padding from the constant value to something based on the distance between the notes.

src/formatter.ts

@@ -659,6 +659,14 @@ export class Formatter {
       const distances: Distance[] = contextList.map((tick: number, i: number) => {
         const context: TickContext = contextMap[tick];
         const voices = context.getTickablesByVoice();
+        // special case, if this is a single-note measure, put the note in the center
+        if (contextList.length === 1) {
+          return {
+            expectedDistance: (adjustedJustifyWidth - context.getWidth()) / 2,
+            fromTickablePx: 0,
+            maxNegativeShiftPx: 0,
+          };
+        }
         let backTickable: Tickable | undefined;
         if (i > 0) {
           const prevContext: TickContext = contextMap[contextList[i - 1]];
@@ -719,7 +727,6 @@ export class Formatter {
               } else if (typeof backTickable !== 'undefined') {
                 expectedDistance = backTickable.getVoice().softmax(maxTicks) * adjustedJustifyWidth;
               }
-
               return {
                 expectedDistance,
                 maxNegativeShiftPx,
@@ -753,10 +760,11 @@ export class Formatter {
             negativeShiftPx = Math.min(ideal.maxNegativeShiftPx, Math.abs(errorPx));
             spaceAccum += -negativeShiftPx;
           }
-
           context.setX(contextX + spaceAccum);
+        } else if (contextList.length === 1) {
+          // Special case:  If there is only one tickable, move it towards the center.
+          context.setX(context.getX() + idealDistances[index].expectedDistance);
         }
-
         // Move center aligned tickables to middle
         context.getCenterAlignedTickables().forEach((tickable: Tickable) => {
           tickable.setCenterXShift(centerX - context.getX());
@@ -771,20 +779,48 @@ export class Formatter {
       lastContext.getMetrics().notePx -
       lastContext.getMetrics().totalRightPx -
       firstContext.getMetrics().totalLeftPx;
-    let targetWidth = adjustedJustifyWidth;
-    let actualWidth = shiftToIdealDistances(calculateIdealDistances(targetWidth));
     const musicFont = Flow.DEFAULT_FONT_STACK[0];
-    const paddingMax = musicFont.lookupMetric('stave.endPaddingMax');
-    const paddingMin = musicFont.lookupMetric('stave.endPaddingMin');
+    const configMinPadding = musicFont.lookupMetric('stave.endPaddingMin');
+    const configMaxPadding = musicFont.lookupMetric('stave.endPaddingMax');
+    let targetWidth = adjustedJustifyWidth;
+    let distances = calculateIdealDistances(targetWidth);
+    let actualWidth = shiftToIdealDistances(distances);
+    // Calculate right justification by finding max of (configured value, min distance between tickables)
+    // so measures with lots of white space use it evenly, and crowded measures use at least the configured
+    // space
+    const calcMinDistance = (targetWidth: number, distances: Distance[]) => {
+      let mdCalc = targetWidth / 2;
+      if (distances.length > 1) {
+        for (let di = 1; di < distances.length; ++di) {
+          mdCalc = Math.min(distances[di].expectedDistance / 2, mdCalc);
+        }
+      }
+      return mdCalc;
+    };
+    const minDistance = calcMinDistance(targetWidth, distances);
+    const multiNote = contextList.length > 1;
+    // right justify to either the configured padding, or the min distance between notes, whichever is greatest.
+    let paddingMax = configMaxPadding;
+    // This * 2 keeps the existing formatting unless there is 'a lot' of extra whitespace, which won't break
+    // existing visual regression tests.
+    if (paddingMax * 2 < minDistance) {
+      paddingMax = minDistance;
+      L('Right padding to ' + minDistance);
+    }
+    const paddingMin = paddingMax - (configMaxPadding - configMinPadding);
     const maxX = adjustedJustifyWidth - paddingMin;
 
     let iterations = maxIterations;
-    while ((actualWidth > maxX && iterations > 0) || (actualWidth + paddingMax < maxX && iterations > 1)) {
+    while (
+      (actualWidth > maxX && iterations > 0 && multiNote) ||
+      (actualWidth + paddingMax < maxX && iterations > 1 && multiNote)
+    ) {
       // 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.
       targetWidth -= actualWidth - maxX;
-      actualWidth = shiftToIdealDistances(calculateIdealDistances(targetWidth));
+      distances = calculateIdealDistances(targetWidth);
+      actualWidth = shiftToIdealDistances(distances);
       iterations--;
     }
 

tests/formatter_tests.ts

@@ -6,6 +6,7 @@
 import { TestOptions, VexFlowTests } from './vexflow_test_helpers';
 import { Annotation } from 'annotation';
 import { Beam } from 'beam';
+import { Tables } from 'tables';
 import { Bend } from 'bend';
 import { Flow } from 'flow';
 import { FontGlyph } from 'font';
@@ -15,7 +16,7 @@ import { Registry } from 'registry';
 import { Stave } from 'stave';
 import { StaveConnector } from 'staveconnector';
 import { StaveNote } from 'stavenote';
-import { Voice } from 'voice';
+import { Voice, VoiceTime } from 'voice';
 import { MockTickable } from './mocks';
 
 const FormatterTests = {
@@ -24,6 +25,7 @@ const FormatterTests = {
     test('TickContext Building', buildTickContexts);
 
     const run = VexFlowTests.runTests;
+    run('Whitespace and justify', rightJustify);
     run('Notehead padding', noteHeadPadding);
     run('Justification and alignment with accidentals', accidentalJustification);
     run('Long measure taking full space', longMeasureProblems);
@@ -89,6 +91,35 @@ function buildTickContexts(): void {
     'Second note of voice 2 is to the right of the second note of voice 1'
   );
 }
+function rightJustify(options: TestOptions): void {
+  const y = 40;
+  const f = VexFlowTests.makeFactory(options, 1200, 300);
+  const getTickables = (time: VoiceTime, n: number, duration: string): Voice => {
+    const tickar: StaveNote[] = [];
+    let i = 0;
+    for (i = 0; i < n; ++i) {
+      tickar.push(new StaveNote({ keys: ['f/4'], duration }));
+    }
+    return new Voice(time).addTickables(tickar);
+  };
+  const renderTest = (time: VoiceTime, n: number, duration: string, x: number, width: number) => {
+    const VF = Vex.Flow;
+    const formatter = f.Formatter();
+
+    const stave = f.Stave({ x, y: 40, width });
+    // stave.addClef('treble').addTimeSignature('4/4');
+
+    const voice = getTickables(time, n, duration);
+    formatter.joinVoices([voice]).formatToStave([voice], stave);
+    stave.draw();
+    voice.draw(f.getContext(), stave);
+  };
+  renderTest({ num_beats: 4, beat_value: 4, resolution: 4 * 4096 }, 2, '2', 10, 300);
+  renderTest({ num_beats: 4, beat_value: 4, resolution: 4 * 4096 }, 1, 'w', 310, 300);
+  renderTest({ num_beats: 3, beat_value: 4, resolution: 4 * 4096 }, 3, '4', 610, 300);
+  renderTest({ num_beats: 3, beat_value: 4, resolution: 4 * 4096 }, 6, '8', 910, 300);
+  ok(true);
+}
 
 function noteHeadPadding(options: TestOptions): void {
   const registry = new Registry();
@@ -267,6 +298,14 @@ function unalignedNoteDurations2(options: TestOptions): void {
 }
 
 function justifyStaveNotes(options: TestOptions): void {
+  function glyphPixels(): number {
+    return 96 * (38 / (Flow.DEFAULT_FONT_STACK[0].getResolution() * 72));
+  }
+
+  function glyphWidth(vexGlyph: string): number {
+    const glyph: FontGlyph = Flow.DEFAULT_FONT_STACK[0].getGlyphs()[vexGlyph];
+    return (glyph.x_max - glyph.x_min) * glyphPixels();
+  }
   const f = VexFlowTests.makeFactory(options, 520, 280);
   const ctx = f.getContext();
   const score = f.EasyScore();
@@ -280,7 +319,9 @@ function justifyStaveNotes(options: TestOptions): void {
       score.voice(score.notes('(bb4 e#5 a5)/4, (d5 e5 f5)/2, (c##5 fb5 a5)/4', { stem: 'up' })),
     ];
 
-    f.Formatter().joinVoices(voices).format(voices, width);
+    f.Formatter()
+      .joinVoices(voices)
+      .format(voices, width - (Stave.defaultPadding + glyphWidth('gClef')));
 
     // Show the the width of notes via a horizontal line with red, green, yellow, blue, gray indicators.
     voices[0].getTickables().forEach((note) => Note.plotMetrics(ctx, note, y + 140)); // Bottom line.