feat(Spacing): optimize vexflow voice spacing/scaling, refactor

see new EngravingRules variables: VoiceSpacingMultiplierVexflow and VoicingSpaceAddendVexflow.

SoftMaxFactorVexflow changed from 5 to 15.
Seems like a sweet spot between compression and smaller note values spaced correctly.

EngravingRules.MinSkyBottomDistBetweenSystems in compacttight mode now 1.0,
instead of 2.0, because slurs have skybottomlines now (no more collisions)

src/MusicalScore/Graphical/DrawingParameters.ts

@@ -128,7 +128,7 @@ export class DrawingParameters {
         // tight rendering mode, lower margins and safety distances between systems, staffs etc. may cause overlap.
         // these options can afterwards be finetuned by setting osmd.rules.BetweenStaffDistance for example
         this.rules.MinSkyBottomDistBetweenStaves = 1.0; // default 1.0. this can cause collisions with slurs and dynamics sometimes
-        this.rules.MinSkyBottomDistBetweenSystems = 2.0; // default 5.0
+        this.rules.MinSkyBottomDistBetweenSystems = 1.0; // default 5.0
         // note that this.rules === osmd.rules, since it's passed as a reference
 
         this.rules.BetweenStaffDistance = 2.5;

src/MusicalScore/Graphical/EngravingRules.ts

@@ -176,6 +176,9 @@ export class EngravingRules {
     private minimumStaffLineDistance: number;
     private minSkyBottomDistBetweenStaves: number;
     private minimumCrossedBeamDifferenceMargin: number;
+
+    private voiceSpacingMultiplierVexflow: number;
+    private voicingSpaceAddendVexflow: number;
     private displacedNoteMargin: number;
     private minNoteDistance: number;
     private subMeasureXSpacingThreshold: number;
@@ -435,15 +438,19 @@ export class EngravingRules {
         this.minimumCrossedBeamDifferenceMargin = 0.0001;
 
         // xSpacing Variables
+        this.voiceSpacingMultiplierVexflow = 1.0;
+        this.voicingSpaceAddendVexflow = 3.0;
         this.displacedNoteMargin = 0.1;
         this.minNoteDistance = 2.0;
         this.subMeasureXSpacingThreshold = 35;
         this.measureDynamicsMaxScalingFactor = 2.5;
-        this.wholeRestXShiftVexflow = -2.5; // VexFlow draws rest notes too far to the right
+        this.wholeRestXShiftVexflow = -1.5; // VexFlow draws rest notes too far to the right
         this.metronomeMarksDrawn = true;
         this.metronomeMarkXShift = -6; // our unit, is taken * unitInPixels
         this.metronomeMarkYShift = -0.5;
-        this.softmaxFactorVexFlow = 5;
+        this.softmaxFactorVexFlow = 15; // seems like the sweet spot. Vexflow default is 100.
+        // if too high, score gets too big, especially half notes. with half note quarter quarter, the quarters get squeezed.
+        // if too low, smaller notes aren't positioned correctly.
 
         // Render options (whether to render specific or invisible elements)
         this.alignRests = AlignRestOption.Never; // 0 = false, 1 = true, 2 = auto
@@ -1408,6 +1415,18 @@ export class EngravingRules {
     public set MinimumCrossedBeamDifferenceMargin(value: number) {
         this.minimumCrossedBeamDifferenceMargin = value;
     }
+    public get VoiceSpacingMultiplierVexflow(): number {
+        return this.voiceSpacingMultiplierVexflow;
+    }
+    public set VoiceSpacingMultiplierVexflow(value: number) {
+        this.voiceSpacingMultiplierVexflow = value;
+    }
+    public get VoicingSpaceAddendVexflow(): number {
+        return this.voicingSpaceAddendVexflow;
+    }
+    public set VoicingSpaceAddendVexflow(value: number) {
+        this.voicingSpaceAddendVexflow = value;
+    }
     public get DisplacedNoteMargin(): number {
         return this.displacedNoteMargin;
     }

src/MusicalScore/Graphical/MusicSheetCalculator.ts

@@ -221,24 +221,25 @@ export abstract class MusicSheetCalculator {
         // for each inner List in big Measure List calculate new Positions for the StaffEntries
         // and adjust Measures sizes
         // calculate max measure length for maximum zoom in.
-        let minLength: number = 0;
-        const maxInstructionsLength: number = this.rules.MaxInstructionsConstValue;
+
+        // let minLength: number = 0; // currently unused
+        // const maxInstructionsLength: number = this.rules.MaxInstructionsConstValue;
         if (this.graphicalMusicSheet.MeasureList.length > 0) {
             /** list of vertically ordered measures belonging to one bar */
             let measures: GraphicalMeasure[] = this.graphicalMusicSheet.MeasureList[0];
             let minimumStaffEntriesWidth: number = this.calculateMeasureXLayout(measures);
             minimumStaffEntriesWidth = this.calculateMeasureWidthFromLyrics(measures, minimumStaffEntriesWidth);
             MusicSheetCalculator.setMeasuresMinStaffEntriesWidth(measures, minimumStaffEntriesWidth);
-            minLength = minimumStaffEntriesWidth * 1.2 + maxInstrNameLabelLength + maxInstructionsLength;
+            // minLength = minimumStaffEntriesWidth * 1.2 + maxInstrNameLabelLength + maxInstructionsLength;
             for (let i: number = 1; i < this.graphicalMusicSheet.MeasureList.length; i++) {
                 measures = this.graphicalMusicSheet.MeasureList[i];
                 minimumStaffEntriesWidth = this.calculateMeasureXLayout(measures);
                 minimumStaffEntriesWidth = this.calculateMeasureWidthFromLyrics(measures, minimumStaffEntriesWidth);
                 MusicSheetCalculator.setMeasuresMinStaffEntriesWidth(measures, minimumStaffEntriesWidth);
-                minLength = Math.max(minLength, minimumStaffEntriesWidth * 1.2 + maxInstructionsLength);
+                // minLength = Math.max(minLength, minimumStaffEntriesWidth * 1.2 + maxInstructionsLength);
             }
         }
-        this.graphicalMusicSheet.MinAllowedSystemWidth = minLength;
+        // this.graphicalMusicSheet.MinAllowedSystemWidth = minLength; // currently unused
     }
 
     public calculateMeasureWidthFromLyrics(measuresVertical: GraphicalMeasure[], oldMinimumStaffEntriesWidth: number): number {

src/MusicalScore/Graphical/MusicSystemBuilder.ts

@@ -880,7 +880,13 @@ export class MusicSystemBuilder {
             for (let measureIndex: number = 0; measureIndex < staffLine.Measures.length; measureIndex++) {
                 const measure: GraphicalMeasure = staffLine.Measures[measureIndex];
                 measure.setPositionInStaffline(currentXPosition);
-                measure.setWidth(measure.beginInstructionsWidth + measure.minimumStaffEntriesWidth * scalingFactor + measure.endInstructionsWidth);
+                const beginInstructionsWidth: number = measure.beginInstructionsWidth;
+                // if (measureIndex === 0 && measure.staffEntries) {
+                //     if (!measure.parentSourceMeasure.hasLyrics) {
+                //         beginInstructionsWidth *= 1; // TODO the first measure in a system is always slightly too big. why? try e.g. 0.6
+                //     }
+                // }
+                measure.setWidth(beginInstructionsWidth + measure.minimumStaffEntriesWidth * scalingFactor + measure.endInstructionsWidth);
                 if (measureIndex < this.currentSystemParams.systemMeasures.length) {
                     const startLine: SystemLinesEnum = this.currentSystemParams.systemMeasures[measureIndex].beginLine;
                     const lineWidth: number = measure.getLineWidth(SystemLinesEnum.BoldThinDots);

src/MusicalScore/Graphical/VexFlow/VexFlowMusicSheetCalculator.ts

@@ -130,7 +130,10 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
     // Format the voices
     const allVoices: Vex.Flow.Voice[] = [];
     // TODO: remove the any when the new DefinitelyTyped PR is through and update released
-    const formatter: Vex.Flow.Formatter = new (Vex.Flow as any).Formatter({softmaxFactor: this.rules.SoftmaxFactorVexFlow});
+    const formatter: Vex.Flow.Formatter = new (Vex.Flow as any).Formatter({
+      // maxIterations: 2,
+      softmaxFactor: this.rules.SoftmaxFactorVexFlow
+    });
 
     for (const measure of measures) {
       if (!measure) {
@@ -156,9 +159,15 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
 
     let minStaffEntriesWidth: number = 12; // a typical measure has roughly a length of 3*StaffHeight (3*4 = 12)
     if (allVoices.length > 0) {
-      // FIXME: The following ``+ 5.0'' is temporary: it was added as a workaround for
-      // FIXME: a more relaxed formatting of voices
-      minStaffEntriesWidth = formatter.preCalculateMinTotalWidth(allVoices) / unitInPixels + 5.0;
+      // the voicing space bonus addition makes the voicing more relaxed. With a bonus of 0 the notes are basically completely squeezed together.
+      // let voicingWidthBonus: number = this.rules.VoicingSpaceBonusVexflow;
+      // if (measures[0].staffEntries?.length === 2) {
+      //   voicingWidthBonus = 3;
+      // }
+      minStaffEntriesWidth = formatter.preCalculateMinTotalWidth(allVoices) / unitInPixels
+        * this.rules.VoiceSpacingMultiplierVexflow
+        + this.rules.VoicingSpaceAddendVexflow;
+        // TODO this could use some fine-tuning. currently using *1.5 + 1 by default, results in decent spacing.
       // firstMeasure.formatVoices = (w: number) => {
       //     formatter.format(allVoices, w);
       // };

src/MusicalScore/ScoreIO/MusicSymbolModules/LyricsReader.ts

@@ -125,6 +125,9 @@ export class LyricsReader {
                                 // only add the lyric entry if not another entry has already been given:
                                 if (!currentVoiceEntry.LyricsEntries[currentLyricVerseNumber]) {
                                     currentVoiceEntry.LyricsEntries.setValue(currentLyricVerseNumber, lyricsEntry);
+                                    if (currentVoiceEntry.ParentSourceStaffEntry?.VerticalContainerParent?.ParentMeasure) {
+                                        currentVoiceEntry.ParentSourceStaffEntry.VerticalContainerParent.ParentMeasure.hasLyrics = true;
+                                    }
                                 }
                                 // save in currentInstrument the verseNumber (only once)
                                 if (!currentVoiceEntry.ParentVoice.Parent.LyricVersesNumbers[currentLyricVerseNumber]) {

src/MusicalScore/VoiceData/SourceMeasure.ts

@@ -61,6 +61,7 @@ export class SourceMeasure {
     private completeNumberOfStaves: number;
     private duration: Fraction;
     private activeTimeSignature: Fraction;
+    public hasLyrics: boolean = false;
     private staffLinkedExpressions: MultiExpression[][] = [];
     private tempoExpressions: MultiTempoExpression[] = [];
     private verticalSourceStaffEntryContainers: VerticalSourceStaffEntryContainer[] = [];