fix(Expressions): don't align expressions when there are no edges involved. manual merge of #768

src/MusicalScore/Graphical/AbstractGraphicalExpression.ts

@@ -4,6 +4,7 @@ import { StaffLine } from "./StaffLine";
 import { BoundingBox } from "./BoundingBox";
 import { AbstractExpression, PlacementEnum } from "../VoiceData/Expressions/AbstractExpression";
 import { EngravingRules } from "./EngravingRules";
+import { SourceMeasure } from "../VoiceData";
 
 export abstract class AbstractGraphicalExpression extends GraphicalObject {
     protected label: GraphicalLabel;
@@ -12,10 +13,12 @@ export abstract class AbstractGraphicalExpression extends GraphicalObject {
     protected expression: AbstractExpression;
     /** EngravingRules for positioning */
     protected rules: EngravingRules;
+    protected parentMeasure: SourceMeasure;
 
-    constructor(parentStaffline: StaffLine, expression: AbstractExpression = undefined) {
+    constructor(parentStaffline: StaffLine, expression: AbstractExpression, measure: SourceMeasure) {
         super();
         this.expression = expression;
+        this.parentMeasure = measure; // could be undefined!
         this.boundingBox = new BoundingBox(this, parentStaffline.PositionAndShape);
         this.parentStaffLine = parentStaffline;
         this.parentStaffLine.AbstractExpressions.push(this);

src/MusicalScore/Graphical/GraphicalContinuousDynamicExpression.ts

@@ -8,6 +8,7 @@ import { PlacementEnum } from "../VoiceData/Expressions/AbstractExpression";
 import { SkyBottomLineCalculator } from "./SkyBottomLineCalculator";
 import { ISqueezable } from "./ISqueezable";
 import log from "loglevel";
+import { SourceMeasure } from "../VoiceData";
 
 /**
  * This class prepares the graphical elements for a continuous expression. It calculates the wedges and
@@ -26,10 +27,10 @@ export class GraphicalContinuousDynamicExpression extends AbstractGraphicalExpre
     /**
      * Create a new instance of the GraphicalContinuousDynamicExpression
      * @param continuousDynamic The continuous dynamic instruction read via ExpressionReader
-     * @param staffLine The staffline where the exoression is attached
+     * @param staffLine The staffline where the expression is attached
      */
-    constructor(continuousDynamic: ContinuousDynamicExpression, staffLine: StaffLine) {
-        super(staffLine, continuousDynamic);
+    constructor(continuousDynamic: ContinuousDynamicExpression, staffLine: StaffLine, measure: SourceMeasure) {
+        super(staffLine, continuousDynamic, measure);
 
         this.isSplittedPart = false;
         this.notToBeRemoved = false;
@@ -92,6 +93,7 @@ export class GraphicalContinuousDynamicExpression extends AbstractGraphicalExpre
                 break;
             case PlacementEnum.Below:
                 if (!this.IsVerbal) {
+                    // console.log(`id: ${this.parentStaffLine.ParentStaff.Id}`);
                     if (this.ContinuousDynamic.DynamicType === ContDynamicEnum.crescendo) {
                         skyBottomLineCalculator.updateBottomLineWithWedge(this.lines[1].Start, this.lines[1].End);
                     } else if (this.ContinuousDynamic.DynamicType === ContDynamicEnum.diminuendo) {
@@ -263,6 +265,10 @@ export class GraphicalContinuousDynamicExpression extends AbstractGraphicalExpre
         this.PositionAndShape.RelativePosition = this.lines[0].Start;
         this.PositionAndShape.BorderMarginTop = this.lines[0].End.y - this.lines[0].Start.y;
         this.PositionAndShape.BorderMarginBottom = this.lines[1].End.y - this.lines[1].Start.y;
+        this.PositionAndShape.Center.y = (this.PositionAndShape.BorderMarginTop + this.PositionAndShape.BorderMarginBottom) / 2;
+        // TODO is the center position correct? it wasn't set before, important for AlignmentManager.alignDynamicExpressions()
+        // console.log(`relative y, center y: ${this.PositionAndShape.RelativePosition.y},${this.PositionAndShape.Center.y})`);
+
 
         if (this.ContinuousDynamic.DynamicType === ContDynamicEnum.crescendo) {
             this.PositionAndShape.BorderMarginLeft = 0;

src/MusicalScore/Graphical/GraphicalInstantaneousDynamicExpression.ts

@@ -11,7 +11,7 @@ export class GraphicalInstantaneousDynamicExpression extends AbstractGraphicalEx
     protected mMeasure: GraphicalMeasure;
 
     constructor(instantaneousDynamic: InstantaneousDynamicExpression, staffLine: StaffLine, measure: GraphicalMeasure) {
-        super(staffLine, instantaneousDynamic);
+        super(staffLine, instantaneousDynamic, measure.parentSourceMeasure);
         this.mInstantaneousDynamicExpression = instantaneousDynamic;
         this.mMeasure = measure;
     }

src/MusicalScore/Graphical/GraphicalInstantaneousTempoExpression.ts

@@ -7,7 +7,7 @@ import { AbstractGraphicalExpression } from "./AbstractGraphicalExpression";
 export class GraphicalInstantaneousTempoExpression extends AbstractGraphicalExpression {
 
     constructor(tempoExpresssion: AbstractTempoExpression, label: GraphicalLabel) {
-        super((label.PositionAndShape.Parent.DataObject as StaffLine), tempoExpresssion);
+        super((label.PositionAndShape.Parent.DataObject as StaffLine), tempoExpresssion, tempoExpresssion.parentMeasure);
         this.label = label;
     }
 

src/MusicalScore/Graphical/GraphicalUnknownExpression.ts

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

src/MusicalScore/Graphical/MusicSheetCalculator.ts

@@ -583,7 +583,9 @@ export abstract class MusicSheetCalculator {
                                                                 multiExpression.getPlacementOfFirstEntry(),
                                                                 fontHeight);
 
-        const gue: GraphicalUnknownExpression = new GraphicalUnknownExpression(staffLine, graphLabel);
+        const gue: GraphicalUnknownExpression = new GraphicalUnknownExpression(staffLine, graphLabel, measures[staffIndex]?.parentSourceMeasure);
+        //    multiExpression); // TODO would be nice to hand over and save reference to original expression,
+        //                         but MultiExpression is not an AbstractExpression.
         staffLine.AbstractExpressions.push(gue);
         }
     }
@@ -921,7 +923,7 @@ export abstract class MusicSheetCalculator {
         const endMeasure: GraphicalMeasure = this.graphicalMusicSheet.getGraphicalMeasureFromSourceMeasureAndIndex(
             graphicalContinuousDynamic.ContinuousDynamic.EndMultiExpression.SourceMeasureParent, staffIndex);
         if (!endMeasure) {
-            log.warn("Not working");
+            log.warn("MusicSheetCalculator.calculateGraphicalContinuousDynamic: No endMeasure found");
             return;
         }
 
@@ -973,7 +975,8 @@ export abstract class MusicSheetCalculator {
             lowerEndX = endPosInStaffLine.x;
 
             // must create a new Wedge
-            secondGraphicalContinuousDynamic = new GraphicalContinuousDynamicExpression(graphicalContinuousDynamic.ContinuousDynamic, endStaffLine);
+            secondGraphicalContinuousDynamic = new GraphicalContinuousDynamicExpression(
+                graphicalContinuousDynamic.ContinuousDynamic, endStaffLine, endMeasure.parentSourceMeasure);
             secondGraphicalContinuousDynamic.IsSplittedPart = true;
             graphicalContinuousDynamic.IsSplittedPart = true;
         } else {

src/MusicalScore/Graphical/VexFlow/AlignmentManager.ts

@@ -4,6 +4,7 @@ import { VexFlowContinuousDynamicExpression } from "./VexFlowContinuousDynamicEx
 import { AbstractGraphicalExpression } from "../AbstractGraphicalExpression";
 import { PointF2D } from "../../../Common/DataObjects/PointF2D";
 import { EngravingRules } from "../EngravingRules";
+import { GraphicalContinuousDynamicExpression } from "../GraphicalContinuousDynamicExpression";
 
 export class AlignmentManager {
     private parentStaffline: StaffLine;
@@ -21,6 +22,15 @@ export class AlignmentManager {
         for (let aeIdx: number = 0; aeIdx < this.parentStaffline.AbstractExpressions.length - 1; aeIdx++) {
             const currentExpression: AbstractGraphicalExpression = this.parentStaffline.AbstractExpressions[aeIdx];
             const nextExpression: AbstractGraphicalExpression = this.parentStaffline.AbstractExpressions[aeIdx + 1];
+
+            // 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();
+            // }
+            // if (nextExpression instanceof GraphicalContinuousDynamicExpression) {
+            //     nextExpression.calcPsi();
+            // }
+
             if (currentExpression.Placement === nextExpression.Placement) {
                 const dist: PointF2D = this.getDistance(currentExpression.PositionAndShape, nextExpression.PositionAndShape);
                 if (dist.x < this.rules.DynamicExpressionMaxDistance) {
@@ -39,21 +49,47 @@ 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);
+                // TODO this may not give the right position for wedges (GraphicalContinuousDynamic, !isVerbal())
                 const yIdeal: number = Math.max(...centerYs);
+                // for (const ae of aes) { // debug
+                //     if (ae.PositionAndShape.Center.y > 6) {
+                //         // dynamic positioned at edge of skybottomline
+                //         console.log(`max expression in measure ${ae.SourceExpression.parentMeasure.MeasureNumber}: `);
+                //         console.dir(aes);
+                //     }
+                // }
+
                 for (let exprIdx: number = 0; exprIdx < aes.length; exprIdx++) {
                     const expr: AbstractGraphicalExpression = aes[exprIdx];
                     const centerOffset: number = centerYs[exprIdx] - yIdeal;
+                    // TODO centerOffset is way too big sometimes, like 7.0 in An die Ferne Geliebte (measure 10, dim.)
                     // FIXME: Expressions should not behave differently.
                     if (expr instanceof VexFlowContinuousDynamicExpression) {
                         (expr as VexFlowContinuousDynamicExpression).shiftYPosition(-centerOffset);
+                        (expr as VexFlowContinuousDynamicExpression).calcPsi();
                     } else {
                         // TODO: The 0.8 are because the letters are a bit to 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.
+                        expr.PositionAndShape.calculateBoundingBox();
                     }
-                    expr.PositionAndShape.calculateBoundingBox();
                     // Squeeze wedges
                     if ((expr as VexFlowContinuousDynamicExpression).squeeze) {
                         const nextExpression: AbstractGraphicalExpression = exprIdx < aes.length - 1 ? aes[exprIdx + 1] : undefined;
@@ -80,7 +116,7 @@ export class AlignmentManager {
     private getDistance(a: BoundingBox, b: BoundingBox): PointF2D {
         const rightBorderA: number = a.RelativePosition.x + a.BorderMarginRight;
         const leftBorderB: number = b.RelativePosition.x + b.BorderMarginLeft;
-        const bottomBorderA: number = b.RelativePosition.y + a.BorderMarginBottom;
+        const bottomBorderA: number = a.RelativePosition.y + a.BorderMarginBottom;
         const topBorderB: number = b.RelativePosition.y + b.BorderMarginTop;
         return new PointF2D(leftBorderB - rightBorderA,
                             topBorderB - bottomBorderA);

src/MusicalScore/Graphical/VexFlow/VexFlowContinuousDynamicExpression.ts

@@ -5,13 +5,15 @@ import { GraphicalLabel } from "../GraphicalLabel";
 import { Label } from "../../Label";
 import { TextAlignmentEnum } from "../../../Common/Enums/TextAlignment";
 import { FontStyles } from "../../../Common/Enums/FontStyles";
+import { SourceMeasure } from "../../VoiceData/SourceMeasure";
 
 /**
  * This class extends the GraphicalContinuousDynamicExpression and creates all necessary methods for drawing
  */
 export class VexFlowContinuousDynamicExpression extends GraphicalContinuousDynamicExpression {
-    constructor(continuousDynamic: ContinuousDynamicExpression, staffLine: StaffLine, textHeight?: number) {
-        super(continuousDynamic, staffLine);
+    constructor(continuousDynamic: ContinuousDynamicExpression, staffLine: StaffLine,
+                measure: SourceMeasure, textHeight?: number) {
+        super(continuousDynamic, staffLine, measure);
         if (this.IsVerbal) {
             const sourceLabel: Label = new Label(continuousDynamic.Label);
             this.label = new GraphicalLabel(sourceLabel,

src/MusicalScore/Graphical/VexFlow/VexFlowMusicSheetCalculator.ts

@@ -522,7 +522,8 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
       const continuousDynamic: ContinuousDynamicExpression = multiExpression.StartingContinuousDynamic;
       const graphicalContinuousDynamic: VexFlowContinuousDynamicExpression = new VexFlowContinuousDynamicExpression(
         multiExpression.StartingContinuousDynamic,
-        staffLine);
+        staffLine,
+        startMeasure.parentSourceMeasure);
       graphicalContinuousDynamic.StartMeasure = startMeasure;
 
       if (!graphicalContinuousDynamic.IsVerbal && continuousDynamic.EndMultiExpression) {
@@ -702,7 +703,10 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
       for (const staffLine of musicSystem.StaffLines) {
         try {
           (<VexFlowStaffLine>staffLine).AlignmentManager.alignDynamicExpressions();
-          staffLine.AbstractExpressions.forEach(ae => ae.updateSkyBottomLine());
+          staffLine.AbstractExpressions.forEach(ae => {
+            ae.updateSkyBottomLine();
+            console.log(`id: ${staffLine.ParentStaff.Id}`);
+          });
         } catch (e) {
           // TODO still necessary when calculation of expression fails, see calculateDynamicExpressionsForMultiExpression()
           //   see calculateGraphicalContinuousDynamic(), also in MusicSheetCalculator.

src/MusicalScore/ScoreIO/MusicSymbolModules/ExpressionReader.ts

@@ -329,15 +329,20 @@ export class ExpressionReader {
                     this.openContinuousDynamicExpression.StartMultiExpression !== this.getMultiExpression) {
                     this.closeOpenContinuousDynamic();
                 }
-                const instantaneousDynamicExpression: InstantaneousDynamicExpression = new InstantaneousDynamicExpression(  expressionText,
-                                                                                                                            this.soundDynamic,
-                                                                                                                            this.placement,
-                                                                                                                            this.staffNumber);
+                const instantaneousDynamicExpression: InstantaneousDynamicExpression =
+                    new InstantaneousDynamicExpression(
+                        expressionText,
+                        this.soundDynamic,
+                        this.placement,
+                        this.staffNumber,
+                        currentMeasure);
                 this.getMultiExpression.addExpression(instantaneousDynamicExpression, "");
                 this.initialize();
                 if (this.activeInstantaneousDynamic !== undefined) {
                     this.activeInstantaneousDynamic.DynEnum = instantaneousDynamicExpression.DynEnum;
-                } else { this.activeInstantaneousDynamic = new InstantaneousDynamicExpression(expressionText, 0, PlacementEnum.NotYetDefined, 1); }
+                } else {
+                    this.activeInstantaneousDynamic = new InstantaneousDynamicExpression(expressionText, 0, PlacementEnum.NotYetDefined, 1, currentMeasure);
+                }
                 //}
             }
         }
@@ -360,7 +365,7 @@ export class ExpressionReader {
             this.directionTimestamp = Fraction.createFromFraction(inSourceMeasureCurrentFraction);
         }
         this.createNewMultiExpressionIfNeeded(currentMeasure);
-        this.addWedge(wedgeNode, currentMeasureIndex);
+        this.addWedge(wedgeNode, currentMeasure);
         this.initialize();
     }
     private createNewMultiExpressionIfNeeded(currentMeasure: SourceMeasure, timestamp: Fraction = undefined): void {
@@ -384,13 +389,17 @@ export class ExpressionReader {
             currentMeasure.TempoExpressions.push(this.currentMultiTempoExpression);
         }
     }
-    private addWedge(wedgeNode: IXmlElement, currentMeasureIndex: number): void {
+    private addWedge(wedgeNode: IXmlElement, currentMeasure: SourceMeasure): void {
         if (wedgeNode !== undefined && wedgeNode.hasAttributes) {
             const type: string = wedgeNode.attribute("type").value.toLowerCase();
             try {
                 if (type === "crescendo" || type === "diminuendo") {
-                    const continuousDynamicExpression: ContinuousDynamicExpression = new ContinuousDynamicExpression(ContDynamicEnum[type],
-                                                                                                                     this.placement, this.staffNumber);
+                    const continuousDynamicExpression: ContinuousDynamicExpression =
+                        new ContinuousDynamicExpression(
+                            ContDynamicEnum[type],
+                            this.placement,
+                            this.staffNumber,
+                            currentMeasure);
                     if (this.openContinuousDynamicExpression !== undefined) {
                         this.closeOpenContinuousDynamic();
                     }
@@ -495,18 +504,24 @@ export class ExpressionReader {
                 if (this.openContinuousDynamicExpression !== undefined && this.openContinuousDynamicExpression.EndMultiExpression === undefined) {
                     this.closeOpenContinuousDynamic();
                 }
-                const instantaneousDynamicExpression: InstantaneousDynamicExpression = new InstantaneousDynamicExpression(stringTrimmed,
-                                                                                                                          this.soundDynamic,
-                                                                                                                          this.placement,
-                                                                                                                          this.staffNumber);
+                const instantaneousDynamicExpression: InstantaneousDynamicExpression =
+                    new InstantaneousDynamicExpression(
+                        stringTrimmed,
+                        this.soundDynamic,
+                        this.placement,
+                        this.staffNumber,
+                        currentMeasure);
                 this.getMultiExpression.addExpression(instantaneousDynamicExpression, prefix);
                 return true;
             }
             if (ContinuousDynamicExpression.isInputStringContinuousDynamic(stringTrimmed)) {
-                const continuousDynamicExpression: ContinuousDynamicExpression = new ContinuousDynamicExpression( undefined,
-                                                                                                                  this.placement,
-                                                                                                                  this.staffNumber,
-                                                                                                                  stringTrimmed);
+                const continuousDynamicExpression: ContinuousDynamicExpression =
+                    new ContinuousDynamicExpression(
+                        undefined,
+                        this.placement,
+                        this.staffNumber,
+                        currentMeasure,
+                        stringTrimmed);
                 if (this.openContinuousDynamicExpression !== undefined && this.openContinuousDynamicExpression.EndMultiExpression === undefined) {
                     this.closeOpenContinuousDynamic();
                 }

src/MusicalScore/VoiceData/Expressions/AbstractExpression.ts

@@ -1,5 +1,8 @@
+import { SourceMeasure } from "../SourceMeasure";
+
 export class AbstractExpression {
     protected placement: PlacementEnum;
+    public parentMeasure: SourceMeasure; // could be undefined
 
     constructor(placement: PlacementEnum) {
         this.placement = placement;

src/MusicalScore/VoiceData/Expressions/ContinuousExpressions/ContinuousDynamicExpression.ts

@@ -1,10 +1,13 @@
 import {PlacementEnum, AbstractExpression} from "../AbstractExpression";
 import {MultiExpression} from "../MultiExpression";
 import {Fraction} from "../../../../Common/DataObjects/Fraction";
+import {SourceMeasure} from "../../SourceMeasure";
 
 export class ContinuousDynamicExpression extends AbstractExpression {
-    constructor(dynamicType: ContDynamicEnum, placement: PlacementEnum, staffNumber: number, label: string = "") {
+    constructor(dynamicType: ContDynamicEnum, placement: PlacementEnum, staffNumber: number, measure: SourceMeasure,
+                label: string = "") {
         super(placement);
+        super.parentMeasure = measure;
         this.dynamicType = dynamicType;
         this.label = label;
         this.staffNumber = staffNumber;

src/MusicalScore/VoiceData/Expressions/InstantaneousDynamicExpression.ts

@@ -4,10 +4,13 @@ import {DynamicExpressionSymbolEnum} from "./DynamicExpressionSymbolEnum";
 //import {ArgumentOutOfRangeException} from "../../Exceptions";
 import {InvalidEnumArgumentException} from "../../Exceptions";
 import log from "loglevel";
+import { SourceMeasure } from "../SourceMeasure";
 
 export class InstantaneousDynamicExpression extends AbstractExpression {
-    constructor(dynamicExpression: string, soundDynamics: number, placement: PlacementEnum, staffNumber: number) {
+    constructor(dynamicExpression: string, soundDynamics: number, placement: PlacementEnum, staffNumber: number,
+                measure: SourceMeasure) {
         super(placement);
+        super.parentMeasure = measure;
         this.dynamicEnum = DynamicEnum[dynamicExpression.toLowerCase()];
         this.soundDynamic = soundDynamics;
         this.staffNumber = staffNumber;