fix(Expressions): fix alignment in most cases, fix distance calculation (#758, #768)

only new problem is some overlap, e.g. in Function Test Expressions - Overlap,
but the better expression placement e.g. in Beethoven (dim.) or Clementi 36/1 Pt 2 diminuendo
far outweigh this regression.

src/MusicalScore/Graphical/GraphicalUnknownExpression.ts

@@ -2,16 +2,21 @@
 import { StaffLine } from "./StaffLine";
 import { GraphicalLabel } from "./GraphicalLabel";
 import { AbstractGraphicalExpression } from "./AbstractGraphicalExpression";
-import { PlacementEnum, AbstractExpression } from "../VoiceData/Expressions";
+import { PlacementEnum } from "../VoiceData/Expressions/AbstractExpression";
+import { MultiExpression } from "../VoiceData/Expressions/MultiExpression";
 import { SkyBottomLineCalculator } from "./SkyBottomLineCalculator";
 import log from "loglevel";
 import { SourceMeasure } from "../VoiceData/SourceMeasure";
 
 export class GraphicalUnknownExpression extends AbstractGraphicalExpression {
+    public sourceMultiExpression: MultiExpression;
+    public placement: PlacementEnum;
+
     constructor(staffLine: StaffLine, label: GraphicalLabel, measure: SourceMeasure,
-                sourceMultiExpression: AbstractExpression = undefined) {
-        super(staffLine, sourceMultiExpression, measure);
+                sourceMultiExpression: MultiExpression = undefined) {
+        super(staffLine, undefined, measure);
         this.label = label;
+        this.sourceMultiExpression = sourceMultiExpression;
     }
 
     public updateSkyBottomLine(): void {

src/MusicalScore/Graphical/MusicSheetCalculator.ts

@@ -583,7 +583,8 @@ export abstract class MusicSheetCalculator {
                                                                 multiExpression.getPlacementOfFirstEntry(),
                                                                 fontHeight);
 
-        const gue: GraphicalUnknownExpression = new GraphicalUnknownExpression(staffLine, graphLabel, measures[staffIndex]?.parentSourceMeasure);
+        const gue: GraphicalUnknownExpression = new GraphicalUnknownExpression(
+            staffLine, graphLabel, measures[staffIndex]?.parentSourceMeasure, multiExpression);
         //    multiExpression); // TODO would be nice to hand over and save reference to original expression,
         //                         but MultiExpression is not an AbstractExpression.
         staffLine.AbstractExpressions.push(gue);

src/MusicalScore/Graphical/VexFlow/AlignmentManager.ts

@@ -4,7 +4,8 @@ import { VexFlowContinuousDynamicExpression } from "./VexFlowContinuousDynamicEx
 import { AbstractGraphicalExpression } from "../AbstractGraphicalExpression";
 import { PointF2D } from "../../../Common/DataObjects/PointF2D";
 import { EngravingRules } from "../EngravingRules";
-import { GraphicalContinuousDynamicExpression } from "../GraphicalContinuousDynamicExpression";
+import { PlacementEnum } from "../../VoiceData/Expressions";
+import { GraphicalUnknownExpression } from "../GraphicalUnknownExpression";
 
 export class AlignmentManager {
     private parentStaffline: StaffLine;
@@ -23,20 +24,38 @@ export class AlignmentManager {
             const currentExpression: AbstractGraphicalExpression = this.parentStaffline.AbstractExpressions[aeIdx];
             const nextExpression: AbstractGraphicalExpression = this.parentStaffline.AbstractExpressions[aeIdx + 1];
 
-            if (currentExpression?.SourceExpression === undefined ||
-                nextExpression?.SourceExpression === undefined) {
-                continue;
-                // TODO: this doesn't work yet for GraphicalUnknownExpression, because it doesn't have an AbstractExpression,
-                //   so it doesn't have a .Placement.
-                //   this lead to if (currentExpression.Placement...) crashing.
-
-                // same result:
-                // if (currentExpression instanceof GraphicalUnknownExpression ||
-                //     nextExpression instanceof GraphicalUnknownExpression) {
-                //         continue;
-                // }
+            let currentExpressionPlacement: PlacementEnum = undefined;
+            if (currentExpression?.SourceExpression) {
+                currentExpressionPlacement = currentExpression.Placement;
+            } else if (currentExpression instanceof GraphicalUnknownExpression) {
+                currentExpressionPlacement = (currentExpression as GraphicalUnknownExpression).
+                    sourceMultiExpression?.getPlacementOfFirstEntry();
+            }
+            // same for nextExpression:
+            let nextExpressionPlacement: PlacementEnum = undefined;
+            if (nextExpression?.SourceExpression) {
+                nextExpressionPlacement = nextExpression.Placement;
+            } else if (nextExpression instanceof GraphicalUnknownExpression) {
+                nextExpressionPlacement = (nextExpression as GraphicalUnknownExpression).
+                    sourceMultiExpression?.getPlacementOfFirstEntry();
             }
 
+            // if (currentExpression?.SourceExpression === undefined ||
+            //     nextExpression?.SourceExpression === undefined) {
+            //     continue;
+            //     // TODO: this doesn't work yet for GraphicalUnknownExpression, because it doesn't have an AbstractExpression,
+            //     //   so it doesn't have a .Placement.
+            //     //   this lead to if (currentExpression.Placement...) crashing.
+
+            //     // same result:
+            //     // if (currentExpression instanceof GraphicalUnknownExpression ||
+            //     //     nextExpression instanceof GraphicalUnknownExpression) {
+            //     //         continue;
+            //     // }
+            // } else {
+            //     samePlacement = currentExpression.Placement === nextExpression.Placement;
+            // }
+
             // TODO this shifts dynamics in An die Ferne Geliebte, showing that there's something wrong with the RelativePositions etc with wedges
             // if (currentExpression instanceof GraphicalContinuousDynamicExpression) {
             //     currentExpression.calcPsi();
@@ -45,9 +64,13 @@ export class AlignmentManager {
             //     nextExpression.calcPsi();
             // }
 
-            if (currentExpression.Placement === nextExpression.Placement) {
+            if (currentExpressionPlacement === nextExpressionPlacement) {
+                // if ((currentExpression as any).label?.label?.text?.startsWith("dim") ||
+                //     (nextExpression as any).label?.label?.text?.startsWith("dim")) {
+                //         console.log("here");
+                //     }
                 const dist: PointF2D = this.getDistance(currentExpression.PositionAndShape, nextExpression.PositionAndShape);
-                if (dist.x < this.rules.DynamicExpressionMaxDistance) {
+                if (Math.abs(dist.x) < this.rules.DynamicExpressionMaxDistance) {
                     // Prevent last found expression to be added twice. e.g. p<f as three close expressions
                     if (tmpList.indexOf(currentExpression) === -1) {
                         tmpList.push(currentExpression);
@@ -63,19 +86,6 @@ export class AlignmentManager {
         groups.push(tmpList);
 
         for (const aes of groups) {
-            // only align if there are wedges
-            //   fixes placement in Clementi Op. 36 No. 1 Pt 2
-            let hasWedges: boolean = false;
-            for (const ae of aes) {
-                if (ae instanceof GraphicalContinuousDynamicExpression && !(ae as GraphicalContinuousDynamicExpression).IsVerbal) {
-                    hasWedges = true;
-                    break;
-                }
-            }
-            if (!hasWedges) {
-                continue;
-            }
-
             if (aes.length > 0) {
                 // Get the median y position and shift all group members to that position
                 const centerYs: number[] = aes.map(expr => expr.PositionAndShape.Center.y);
@@ -98,7 +108,7 @@ export class AlignmentManager {
                         (expr as VexFlowContinuousDynamicExpression).shiftYPosition(-centerOffset);
                         (expr as VexFlowContinuousDynamicExpression).calcPsi();
                     } else {
-                        // TODO: The 0.8 are because the letters are a bit to far done
+                        // TODO: The 0.8 are because the letters are a bit too far done
                         expr.PositionAndShape.RelativePosition.y -= centerOffset * 0.8;
                         // note: verbal GraphicalContinuousDynamicExpressions have a label, nonverbal ones don't.
                         // take care to update and take the right bounding box for skyline.
@@ -134,6 +144,7 @@ export class AlignmentManager {
         const topBorderB: number = b.RelativePosition.y + b.BorderMarginTop;
         return new PointF2D(leftBorderB - rightBorderA,
                             topBorderB - bottomBorderA);
+                            // note: this is a distance vector, not absolute distance, otherwise we need Math.abs
     }
 
     /**