Skip to content

Handle accidentals on tied notes across bars correctly #485

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 31, 2020
Merged

Handle accidentals on tied notes across bars correctly #485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/rendering/ScoreBarRenderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
75 changes: 47 additions & 28 deletions src/rendering/utils/AccidentalHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
}
}
}
Expand Down
Binary file added test-data/visual-tests/notation-legend/tied-note-accidentals-default.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added test-data/visual-tests/notation-legend/tied-note-accidentals-songbook.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added test-data/visual-tests/notation-legend/tied-note-accidentals.gp
View file
Open in desktop
Binary file not shown.
7 changes: 5 additions & 2 deletions test/visualTests/features/NotationLegend.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
Expand Down