Skip to content

fix: Bend tied vibrato bug #1843

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 3 commits into from
Feb 9, 2025
Merged

fix: Bend tied vibrato bug #1843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1e1ff69
fix: wrongly aligned bends and vibratos when combined
Danielku15 Feb 9, 2025
4981422
fix: wrong Vibrato pitch generation on tied note after bend
Danielku15 Feb 9, 2025
252c2fb
fix: remove additional overflow from tab bend
Danielku15 Feb 9, 2025
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
4 changes: 2 additions & 2 deletions src/Environment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ export class Environment {
new WideBeatVibratoEffectInfo(),
new SlightBeatVibratoEffectInfo(),
new WideNoteVibratoEffectInfo(),
new SlightNoteVibratoEffectInfo(),
new SlightNoteVibratoEffectInfo(false),
new LeftHandTapEffectInfo(),
new GolpeEffectInfo(GolpeType.Finger)
],
Expand Down Expand Up @@ -558,7 +558,7 @@ export class Environment {
new WideBeatVibratoEffectInfo(),
new SlightBeatVibratoEffectInfo(),
new WideNoteVibratoEffectInfo(),
new SlightNoteVibratoEffectInfo(),
new SlightNoteVibratoEffectInfo(true),
new TapEffectInfo(),
new FadeEffectInfo(),
new HarmonicsEffectInfo(HarmonicType.Natural),
Expand Down
60 changes: 44 additions & 16 deletions src/midi/MidiFileGenerator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ export class MidiFileGenerator {
barStartTick + beatStart,
beat.playbackDuration,
phaseLength,
0,
bendAmplitude,
(tick, value) => {
this._handler.addBend(
Expand Down Expand Up @@ -480,6 +481,20 @@ export class MidiFileGenerator {
}
}

private determineChannel(track: Track, note: Note): number {
// on tied notes use the same channel as the previous note
if(note.isTieDestination) {
return this.determineChannel(track, note.tieOrigin!);
}

// on certain effects we use the secondary channel to avoid interference with other notes
if(note.hasBend || note.beat.hasWhammyBar || note.beat.vibrato !== VibratoType.None) {
return track.playbackInfo.secondaryChannel;
}

return track.playbackInfo.primaryChannel;
}

private generateNote(
note: Note,
beatStart: number,
Expand All @@ -503,10 +518,7 @@ export class MidiFileGenerator {
noteDuration.noteOnly -= brushOffset;
noteDuration.letRingEnd -= brushOffset;
const velocity: number = MidiFileGenerator.getNoteVelocity(note);
const channel: number =
note.hasBend || note.beat.hasWhammyBar || note.beat.vibrato !== VibratoType.None
? track.playbackInfo.secondaryChannel
: track.playbackInfo.primaryChannel;
const channel: number = this.determineChannel(track, note);
let initialBend: number = 0;

if (note.hasBend) {
Expand Down Expand Up @@ -808,35 +820,50 @@ export class MidiFileGenerator {
return;
}
const track: Track = note.beat.voice.bar.staff.track;
this.generateVibratorWithParams(noteStart, noteDuration.noteOnly, phaseLength, bendAmplitude, (tick, value) => {
this._handler.addNoteBend(track.index, tick, channel, noteKey, value);
});
let bendBase = 0;
// if this is a vibrato at the end of a bend, the vibrato wave needs to start at the pitch where the bend ends
if (note.isTieDestination && note.tieOrigin!.hasBend) {
const bendPoints = note.tieOrigin!.bendPoints!;
bendBase = bendPoints[bendPoints.length - 1].value;
}
this.generateVibratorWithParams(
noteStart,
noteDuration.noteOnly,
phaseLength,
bendBase,
bendAmplitude,
(tick, value) => {
this._handler.addNoteBend(track.index, tick, channel, noteKey, value);
}
);
}

public vibratoResolution: number = 16;
private generateVibratorWithParams(
noteStart: number,
noteDuration: number,
phaseLength: number,
bendBase: number,
bendAmplitude: number,
addBend: (tick: number, value: number) => void
): void {
const resolution: number = this.vibratoResolution;
const phaseHalf: number = (phaseLength / 2) | 0;
// 1st Phase stays at bend 0,
// then we have a sine wave with the given amplitude and phase length
noteStart += phaseLength;
// vibrato is a sine wave with the given amplitude and phase length
const noteEnd: number = noteStart + noteDuration;
while (noteStart < noteEnd) {
let phase: number = 0;
const phaseDuration: number = noteStart + phaseLength < noteEnd ? phaseLength : noteEnd - noteStart;
while (phase < phaseDuration) {
let bend: number = bendAmplitude * Math.sin((phase * Math.PI) / phaseHalf);
let bend: number = bendBase + bendAmplitude * Math.sin((phase * Math.PI) / phaseHalf);
addBend((noteStart + phase) | 0, MidiFileGenerator.getPitchWheel(bend));
phase += resolution;
}
noteStart += phaseLength;
}

// reset at end
addBend((noteEnd) | 0, MidiFileGenerator.getPitchWheel(bendBase));
}

/**
Expand Down Expand Up @@ -955,7 +982,11 @@ export class MidiFileGenerator {
let duration: number;
if (note.isTieOrigin && this._settings.notation.extendBendArrowsOnTiedNotes) {
let endNote: Note = note;
while (endNote.isTieOrigin && !endNote.tieDestination!.hasBend) {
while (
endNote.isTieOrigin &&
!endNote.tieDestination!.hasBend &&
endNote.tieDestination!.vibrato == VibratoType.None
) {
endNote = endNote.tieDestination!;
}
duration =
Expand Down Expand Up @@ -1316,10 +1347,7 @@ export class MidiFileGenerator {
currentTick += ticksPerBreakpoint;
}

// final bend value if needed
if (currentBendValue < nextBendValue) {
addBend(currentTick | 0, nextBendValue);
}
addBend(currentTick | 0, nextBendValue);
}

private generateTrill(
Expand Down
17 changes: 13 additions & 4 deletions src/rendering/effects/SlightNoteVibratoEffectInfo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,23 @@ import { NoteVibratoGlyph } from '@src/rendering/glyphs/NoteVibratoGlyph';
import { NotationElement } from '@src/NotationSettings';

export class SlightNoteVibratoEffectInfo extends NoteEffectInfoBase {

// for tied bends ending in a vibrato, the vibrato is drawn by the TabBendGlyph for proper alignment
private _hideOnTiedBend: boolean;

public get notationElement(): NotationElement {
return NotationElement.EffectSlightNoteVibrato;
}

protected shouldCreateGlyphForNote(note: Note): boolean {
return (
let hasVibrato =
note.vibrato === VibratoType.Slight ||
(note.isTieDestination && note.tieOrigin!.vibrato === VibratoType.Slight)
);
(note.isTieDestination && note.tieOrigin!.vibrato === VibratoType.Slight);

if (this._hideOnTiedBend && hasVibrato && note.isTieDestination && note.tieOrigin!.hasBend) {
hasVibrato = false;
}
return hasVibrato;
}

public get sizingMode(): EffectBarGlyphSizing {
Expand All @@ -28,7 +36,8 @@ export class SlightNoteVibratoEffectInfo extends NoteEffectInfoBase {
return new NoteVibratoGlyph(0, 0, VibratoType.Slight, 1.2);
}

public constructor() {
public constructor(hideOnTiedBend: boolean) {
super();
this._hideOnTiedBend = hideOnTiedBend;
}
}
47 changes: 31 additions & 16 deletions src/rendering/glyphs/TabBendGlyph.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import { TabBendRenderPoint } from '@src/rendering/glyphs/TabBendRenderPoint';
import { TabBarRenderer } from '@src/rendering/TabBarRenderer';
import { RenderingResources } from '@src/RenderingResources';
import { BendPoint } from '@src/model/BendPoint';
import { VibratoType } from '@src/model';
import { NoteVibratoGlyph } from './NoteVibratoGlyph';

export class TabBendGlyph extends Glyph {
private static readonly ArrowSize: number = 6;
Expand Down Expand Up @@ -188,7 +190,9 @@ export class TabBendGlyph extends Glyph {
break;
case BendType.BendRelease:
renderingPoints.push(new TabBendRenderPoint(0, note.bendPoints![0].value));
renderingPoints.push(new TabBendRenderPoint((BendPoint.MaxPosition / 2) | 0, note.bendPoints![1].value));
renderingPoints.push(
new TabBendRenderPoint((BendPoint.MaxPosition / 2) | 0, note.bendPoints![1].value)
);
renderingPoints.push(new TabBendRenderPoint(BendPoint.MaxPosition, note.bendPoints![3].value));
break;
case BendType.Bend:
Expand All @@ -214,7 +218,7 @@ export class TabBendGlyph extends Glyph {
let startNoteRenderer: BarRendererBase = this.renderer;
let endNote: Note = note;
let isMultiBeatBend: boolean = false;
let endNoteRenderer: TabBarRenderer | null = null;
let endNoteRenderer: BarRendererBase | null = null;
let endNoteHasBend: boolean = false;
let slurText: string = note.bendStyle === BendStyle.Gradual ? 'grad.' : '';
let endBeat: Beat | null = null;
Expand All @@ -223,17 +227,22 @@ export class TabBendGlyph extends Glyph {
endNoteRenderer = this.renderer.scoreRenderer.layout!.getRendererForBar(
this.renderer.staff.staveId,
nextNote.beat.voice.bar
) as TabBarRenderer;
);
if (!endNoteRenderer || startNoteRenderer.staff !== endNoteRenderer.staff) {
break;
}
endNote = nextNote;
isMultiBeatBend = true;
if (endNote.hasBend || !this.renderer.settings.notation.extendBendArrowsOnTiedNotes) {
if (
endNote.hasBend ||
!this.renderer.settings.notation.extendBendArrowsOnTiedNotes ||
endNote.vibrato != VibratoType.None
) {
endNoteHasBend = true;
break;
}
}

endBeat = endNote.beat;
endNoteRenderer = this.renderer.scoreRenderer.layout!.getRendererForBar(
this.renderer.staff.staveId,
Expand Down Expand Up @@ -297,18 +306,24 @@ export class TabBendGlyph extends Glyph {
secondPt = new TabBendRenderPoint(BendPoint.MaxPosition, firstPt.value);
secondPt.lineValue = firstPt.lineValue;

this.paintBend(
note,
firstPt,
secondPt,
startX,
topY,
dX,
slurText,
canvas
);
this.paintBend(note, firstPt, secondPt, startX, topY, dX, slurText, canvas);
}
}

if (endNote.vibrato !== VibratoType.None) {
const vibratoStartX = endX - cx + TabBendGlyph.ArrowSize - endNoteRenderer.x;
const vibratoStartY: number =
topY -
cy -
TabBendGlyph.BendValueHeight * renderPoints[renderPoints.length - 1].lineValue;

const vibrato = new NoteVibratoGlyph(vibratoStartX, vibratoStartY, endNote.vibrato, 1.2);
vibrato.beat = endNote.beat;
vibrato.renderer = endNoteRenderer;
vibrato.doLayout();
vibrato.paint(cx + endNoteRenderer.x, cy, canvas);
}

canvas.color = color;
}
}
Expand Down Expand Up @@ -371,7 +386,7 @@ export class TabBendGlyph extends Glyph {
canvas.fill();
arrowOffset = -arrowSize;
}
canvas.stroke();
canvas.beginPath();
if (firstPt.value === secondPt.value) {
// draw horizontal dashed line
// to really have the line ending at the right position
Expand All @@ -395,7 +410,7 @@ export class TabBendGlyph extends Glyph {
}
} else {
if (x2 > x1) {
// draw bezier lien from first to second point
// draw bezier line from first to second point
canvas.moveTo(x1, y1);
canvas.bezierCurveTo((x1 + x2) / 2, y1, x2, y1, x2, y2 + arrowOffset);
canvas.stroke();
Expand Down
7 changes: 6 additions & 1 deletion test-data/test-results.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,12 @@
img.src = '/' + result.diffFile;
})

await Promise.all([expected, actual, diff]);
try {
await Promise.all([expected, actual, diff]);
}
catch(e) {
// ignore potentially missing images
}

setupComparer(comparer, result);

Expand Down
Binary file added test-data/visual-tests/effects-and-annotations/bend-vibrato-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/effects-and-annotations/bend-vibrato-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/effects-and-annotations/bend-vibrato.gp
View file
Open in desktop
Binary file not shown.
Loading