Handle accidentals on tied notes across bars correctly (#485)

Author: Danielku15

src/rendering/ScoreBarRenderer.ts

@@ -51,7 +51,7 @@ export class ScoreBarRenderer extends BarRendererBase {
     public constructor(renderer: ScoreRenderer, bar: Bar) {
         super(renderer, bar);
         this._startSpacing = false;
-        this.accidentalHelper = new AccidentalHelper(bar);
+        this.accidentalHelper = new AccidentalHelper(this);
     }
 
     public getBeatDirection(beat: Beat): BeamDirection {

src/rendering/utils/AccidentalHelper.ts

@@ -6,6 +6,7 @@ import { Note } from '@src/model/Note';
 import { NoteAccidentalMode } from '@src/model/NoteAccidentalMode';
 import { ModelUtils } from '@src/model/ModelUtils';
 import { PercussionMapper } from '../../model/PercussionMapper';
+import { ScoreBarRenderer } from '../ScoreBarRenderer';
 
 
 class BeatLines {
@@ -20,6 +21,7 @@ class BeatLines {
  */
 export class AccidentalHelper {
     private _bar: Bar;
+    private _barRenderer: ScoreBarRenderer;
 
     /**
      * a lookup list containing an info whether the notes within an octave
@@ -103,8 +105,9 @@ export class AccidentalHelper {
      */
     public minLine: number = -1000;
 
-    public constructor(bar: Bar) {
-        this._bar = bar;
+    public constructor(barRenderer: ScoreBarRenderer) {
+        this._barRenderer = barRenderer;
+        this._bar = barRenderer.bar;
     }
 
     public static getPercussionLine(bar: Bar, noteValue: number): number {
@@ -231,19 +234,15 @@ export class AccidentalHelper {
                 switch (accidentalMode) {
                     case NoteAccidentalMode.ForceSharp:
                         accidentalToSet = AccidentalType.Sharp;
-                        hasNoteAccidentalWithinOctave = true;
                         break;
                     case NoteAccidentalMode.ForceDoubleSharp:
                         accidentalToSet = AccidentalType.DoubleSharp;
-                        hasNoteAccidentalWithinOctave = true;
                         break;
                     case NoteAccidentalMode.ForceFlat:
                         accidentalToSet = AccidentalType.Flat;
-                        hasNoteAccidentalWithinOctave = true;
                         break;
                     case NoteAccidentalMode.ForceDoubleFlat:
                         accidentalToSet = AccidentalType.DoubleFlat;
-                        hasNoteAccidentalWithinOctave = true;
                         break;
                     default:
                         // if note has an accidental in the octave, we place a symbol
@@ -257,36 +256,56 @@ export class AccidentalHelper {
                         break;
                 }
 
-                // do we need an accidental on the note?
-                if (accidentalToSet !== AccidentalType.None) {
-                    // if we already have an accidental on this line we will reset it if it's the same
-                    if (this._registeredAccidentals.has(line)) {
-                        if (this._registeredAccidentals.get(line) === accidentalToSet) {
-                            accidentalToSet = AccidentalType.None;
+                // Issue #472: Tied notes across bars do not show the accidentals but also 
+                // do not register them. 
+                // https://ultimatemusictheory.com/tied-notes-with-accidentals/
+                let skipAccidental = false;
+                if (note && note.isTieDestination && note.beat.index === 0) {
+                    // candidate for skip, check further if start note is on the same line
+                    const previousRenderer = this._barRenderer.previousRenderer as ScoreBarRenderer;
+                    if (previousRenderer) {
+                        const tieOriginLine = previousRenderer.accidentalHelper.getNoteLine(note.tieOrigin!);
+                        if(tieOriginLine === line) {
+                            skipAccidental = true;
                         }
                     }
-                    // if there is no accidental on the line, and the key signature has it set already, we clear it on the note
-                    else if (hasKeySignatureAccidentalSetForNote && accidentalToSet === accidentalForKeySignature) {
-                        accidentalToSet = AccidentalType.None;
-                    }
+                }
 
-                    // register the new accidental on the line if any.
-                    if (accidentalToSet != AccidentalType.None) {
-                        this._registeredAccidentals.set(line, accidentalToSet);
-                    }
+
+                if (skipAccidental) {
+                    accidentalToSet = AccidentalType.None;
                 } else {
-                    // if we don't want an accidental, but there is already one applied, we place a naturalize accidental
-                    // and clear the registration
-                    if (this._registeredAccidentals.has(line)) {
-                        // if there is already a naturalize symbol on the line, we don't care.
-                        if (this._registeredAccidentals.get(line) === AccidentalType.Natural) {
+                    // do we need an accidental on the note?
+                    if (accidentalToSet !== AccidentalType.None) {
+                        // if we already have an accidental on this line we will reset it if it's the same
+                        if (this._registeredAccidentals.has(line)) {
+                            if (this._registeredAccidentals.get(line) === accidentalToSet) {
+                                accidentalToSet = AccidentalType.None;
+                            }
+                        }
+                        // if there is no accidental on the line, and the key signature has it set already, we clear it on the note
+                        else if (hasKeySignatureAccidentalSetForNote && accidentalToSet === accidentalForKeySignature) {
                             accidentalToSet = AccidentalType.None;
-                        } else {
-                            accidentalToSet = AccidentalType.Natural;
+                        }
+
+                        // register the new accidental on the line if any.
+                        if (accidentalToSet != AccidentalType.None) {
                             this._registeredAccidentals.set(line, accidentalToSet);
                         }
                     } else {
-                        this._registeredAccidentals.delete(line);
+                        // if we don't want an accidental, but there is already one applied, we place a naturalize accidental
+                        // and clear the registration
+                        if (this._registeredAccidentals.has(line)) {
+                            // if there is already a naturalize symbol on the line, we don't care.
+                            if (this._registeredAccidentals.get(line) === AccidentalType.Natural) {
+                                accidentalToSet = AccidentalType.None;
+                            } else {
+                                accidentalToSet = AccidentalType.Natural;
+                                this._registeredAccidentals.set(line, accidentalToSet);
+                            }
+                        } else {
+                            this._registeredAccidentals.delete(line);
+                        }
                     }
                 }
             }

test-data/visual-tests/notation-legend/tied-note-accidentals-default.png

@@ -96,15 +96,18 @@ describe('NotationLegend', () => {
     it('mixed-default', async () => { await runNotationLegendTest(`mixed-default.png`, 121, 7, false); });
     it('mixed-songbook', async () => { await runNotationLegendTest(`mixed-songbook.png`, 121, 7, true); });
 
-    async function runNotationLegendTest(referenceFileName: string, startBar: number, barCount: number, songBook: boolean): Promise<void> {
+    it('tied-note-accidentals-default', async () => { await runNotationLegendTest(`tied-note-accidentals-default.png`, 1, -1, false, 'tied-note-accidentals.gp'); });
+    it('tied-note-accidentals-songbook', async () => { await runNotationLegendTest(`tied-note-accidentals-songbook.png`, 1, -1, true, 'tied-note-accidentals.gp'); });
+
+    async function runNotationLegendTest(referenceFileName: string, startBar: number, barCount: number, songBook: boolean, fileName: string = 'notation-legend.gp'): Promise<void> {
         let settings: Settings = new Settings();
         settings.display.layoutMode = LayoutMode.Horizontal;
         settings.display.startBar = startBar;
         settings.display.barCount = barCount;
         if (songBook) {
             settings.setSongBookModeSettings();
         }
-        const inputFileData = await TestPlatform.loadFile(`test-data/visual-tests/notation-legend/notation-legend.gp`);
+        const inputFileData = await TestPlatform.loadFile(`test-data/visual-tests/notation-legend/${fileName}`);
         let score: Score = ScoreLoader.loadScoreFromBytes(inputFileData, settings);
         await VisualTestHelper.runVisualTestScore(score, `notation-legend/${referenceFileName}`, settings, [0]);
     }