From 38d7f26ea198557e30cf6d7919c3dda70bb206da Mon Sep 17 00:00:00 2001 From: isaiahdahl Date: Tue, 30 Jun 2026 11:13:37 -0700 Subject: [PATCH] Avoid line breaks starting with rhythm symbols --- playground/stores/formatter-store.ts | 5 +- src/chord_sheet_serializer.ts | 6 +- src/layout/engine/item_processor.ts | 27 +++- src/layout/engine/line_breaker.ts | 177 ++++++++++++++++++++-- test/layout/engine/item_processor.test.ts | 11 ++ test/layout/engine/line_breaker.test.ts | 87 ++++++++++- test/parser/chord_pro_parser.test.ts | 13 ++ 7 files changed, 298 insertions(+), 28 deletions(-) diff --git a/playground/stores/formatter-store.ts b/playground/stores/formatter-store.ts index 2ce797862..90d4db247 100644 --- a/playground/stores/formatter-store.ts +++ b/playground/stores/formatter-store.ts @@ -1,6 +1,6 @@ -import { Key } from '../../src'; - import { APP_EVENTS } from './init-store'; +import { Key } from '../../src'; +import { PdfFormatter } from '../../src/pdf'; import { createStore } from './store'; import { formatterConfigExamples } from '../fixtures'; import { getMeasuredHtmlDefaultConfig } from '../../src/formatter/configuration/default_config_manager'; @@ -18,7 +18,6 @@ import { ChordProFormatter, ChordsOverWordsFormatter, HtmlDivFormatter, - PdfFormatter, } from '../../src/index'; // Define the available formatter types diff --git a/src/chord_sheet_serializer.ts b/src/chord_sheet_serializer.ts index f2314782a..705d6fc78 100644 --- a/src/chord_sheet_serializer.ts +++ b/src/chord_sheet_serializer.ts @@ -223,10 +223,14 @@ class ChordSheetSerializer { lyrics, annotation, eagerChord, - isRhythmSymbol || false, + isRhythmSymbol || this.isRhythmSymbol(chordString), ); } + private isRhythmSymbol(chordString: string): boolean { + return chordString === '|' || chordString === '/'; + } + parseTag(astComponent: SerializedTag): Tag { const { name, diff --git a/src/layout/engine/item_processor.ts b/src/layout/engine/item_processor.ts index 62f357684..aa560aef2 100644 --- a/src/layout/engine/item_processor.ts +++ b/src/layout/engine/item_processor.ts @@ -139,7 +139,7 @@ export class ItemProcessor { const measurements = this.calculateMeasurements(renderedChords, lyrics, nextItem, lyricsOnly); return { - item: new ChordLyricsPair(chords, lyrics), + item: new ChordLyricsPair(chords, lyrics, splitItem.annotation, null, splitItem.isRhythmSymbol), width: measurements.totalWidth, chordHeight: measurements.chordHeight, }; @@ -275,11 +275,7 @@ export class ItemProcessor { const firstLyrics = splitLines[0]; const secondLyrics = splitLines.slice(1).join(' '); return [ - { - item: new ChordLyricsPair(item.item.chords, firstLyrics), - width: this.measurer.measureTextWidth(firstLyrics, lyricsFont), - chordHeight: item.chordHeight, - }, + this.createSplitMeasuredItem(item, firstLyrics, item.item.chords, item.item.isRhythmSymbol), { item: new ChordLyricsPair('', secondLyrics), width: this.measurer.measureTextWidth(secondLyrics, lyricsFont), @@ -288,6 +284,21 @@ export class ItemProcessor { ]; } + private createSplitMeasuredItem( + originalItem: MeasuredItem, + lyrics: string, + chords: string, + isRhythmSymbol: boolean, + ): MeasuredItem { + const annotation = originalItem.item instanceof ChordLyricsPair ? originalItem.item.annotation : null; + + return { + item: new ChordLyricsPair(chords, lyrics, annotation, null, isRhythmSymbol), + width: this.measurer.measureTextWidth(lyrics, this.config.fonts.lyrics), + chordHeight: originalItem.chordHeight, + }; + } + /** * Removes hyphens from lyrics for lyrics-only mode */ @@ -324,11 +335,11 @@ export class ItemProcessor { } if (index === 0 && lyricFragments.length === 1) { - items.push(new ChordLyricsPair(chords, fragment, annotation)); + items.push(new ChordLyricsPair(chords, fragment, annotation, null, pair.isRhythmSymbol)); } else if (index === 0 && lyricFragments.length > 1) { let commaAdjustedFragment = fragment; commaAdjustedFragment += ','; - items.push(new ChordLyricsPair(chords, commaAdjustedFragment, annotation)); + items.push(new ChordLyricsPair(chords, commaAdjustedFragment, annotation, null, pair.isRhythmSymbol)); } }); diff --git a/src/layout/engine/line_breaker.ts b/src/layout/engine/line_breaker.ts index 03f5ef69e..97f3d4b8b 100644 --- a/src/layout/engine/line_breaker.ts +++ b/src/layout/engine/line_breaker.ts @@ -45,7 +45,7 @@ export class LineBreaker { return this.handleNoSoftBreaks(items, availableWidth, line); } - const { firstChunk, secondChunk } = this.splitAtBestSoftBreak(items, softBreakIndices, totalWidth); + const { firstChunk, secondChunk } = this.splitAtBestSoftBreak(items, softBreakIndices, totalWidth, availableWidth); if (firstChunk.length === 0) { return this.breakContent(secondChunk, availableWidth, line); @@ -73,8 +73,10 @@ export class LineBreaker { return this.handleOversizedFirstItem(items, availableWidth, line); } - const firstChunk = items.slice(0, breakIndex); - const secondChunk = items.slice(breakIndex); + let firstChunk = items.slice(0, breakIndex); + let secondChunk = items.slice(breakIndex); + + ({ firstChunk, secondChunk } = this.avoidLeadingRhythmSymbol(firstChunk, secondChunk, availableWidth)); this.removeTrailingComma(firstChunk); @@ -165,30 +167,175 @@ export class LineBreaker { items: MeasuredItem[], softBreakIndices: number[], totalWidth: number, + availableWidth: number, ): { firstChunk: MeasuredItem[]; secondChunk: MeasuredItem[] } { + const bestBreak = this.findBestSoftBreak(items, softBreakIndices, totalWidth); + const softBreak = items[bestBreak.index]; + + let firstChunk = items.slice(0, bestBreak.index); + let secondChunk = items.slice(bestBreak.index + 1); + + ({ firstChunk, secondChunk } = this.avoidLeadingRhythmSymbol( + firstChunk, + secondChunk, + availableWidth, + softBreak, + )); + + this.removeTrailingComma(firstChunk); + this.capitalizeNextItem(secondChunk, secondChunk, 0); + + return { firstChunk, secondChunk }; + } + + private findBestSoftBreak(items: MeasuredItem[], softBreakIndices: number[], totalWidth: number) { const targetWidth = totalWidth / 2; const breakOptions = softBreakIndices.map((idx) => ({ index: idx, widthUpToBreak: this.getWidthUpToIndex(items, idx), })); - const bestBreak = breakOptions.reduce((best, current) => { - const currentDistance = Math.abs(current.widthUpToBreak - targetWidth); - const bestDistance = Math.abs(best.widthUpToBreak - targetWidth); - if (currentDistance === bestDistance) { - return current.index > best.index ? current : best; + return breakOptions.reduce((best, current) => this.selectBetterSoftBreak(best, current, targetWidth)); + } + + private selectBetterSoftBreak( + best: T, + current: T, + targetWidth: number, + ): T { + const currentDistance = Math.abs(current.widthUpToBreak - targetWidth); + const bestDistance = Math.abs(best.widthUpToBreak - targetWidth); + + if (currentDistance === bestDistance) { + return current.index > best.index ? current : best; + } + + return currentDistance < bestDistance ? current : best; + } + + /** + * Avoid wrapping a line so that the next visual line starts with a rhythm symbol. + * + * Prefer keeping the leading rhythm symbol(s) with the previous line, but only + * when the previous line still fits. If they do not fit, break earlier by + * moving the nearest preceding non-rhythm chord to the next line. Width checks + * are recalculated in the final line context so trailing chord-spacing is not + * counted for a line-ending rhythm symbol such as `|`. + */ + private avoidLeadingRhythmSymbol( + firstChunk: MeasuredItem[], + secondChunk: MeasuredItem[], + availableWidth: number, + separator: MeasuredItem | null = null, + ): { firstChunk: MeasuredItem[]; secondChunk: MeasuredItem[] } { + if (!this.shouldAvoidLeadingRhythmSymbol(secondChunk)) { + return { firstChunk, secondChunk }; + } + + return this.tryMoveLeadingRhythmSymbolsToFirstChunk(firstChunk, secondChunk, availableWidth, separator) || + this.movePrecedingChordToSecondChunk(firstChunk, secondChunk, separator); + } + + private shouldAvoidLeadingRhythmSymbol(secondChunk: MeasuredItem[]): boolean { + const leadingRhythmSymbolCount = this.countLeadingRhythmSymbols(secondChunk); + return this.startsWithRhythmSymbol(secondChunk) && + this.hasChordAfterLeadingRhythmSymbols(secondChunk, leadingRhythmSymbolCount); + } + + private tryMoveLeadingRhythmSymbolsToFirstChunk( + firstChunk: MeasuredItem[], + secondChunk: MeasuredItem[], + availableWidth: number, + separator: MeasuredItem | null, + ): { firstChunk: MeasuredItem[]; secondChunk: MeasuredItem[] } | null { + const leadingRhythmSymbolCount = this.countLeadingRhythmSymbols(secondChunk); + const separatorItems = separator ? [separator] : []; + const laterFirstChunk = [...firstChunk, ...separatorItems, ...secondChunk.slice(0, leadingRhythmSymbolCount)]; + const laterSecondChunk = secondChunk.slice(leadingRhythmSymbolCount); + + if (this.calculateLineContextWidth(laterFirstChunk) <= availableWidth) { + return { firstChunk: laterFirstChunk, secondChunk: laterSecondChunk }; + } + + return null; + } + + private movePrecedingChordToSecondChunk( + firstChunk: MeasuredItem[], + secondChunk: MeasuredItem[], + separator: MeasuredItem | null, + ): { firstChunk: MeasuredItem[]; secondChunk: MeasuredItem[] } { + const precedingChordIndex = this.findLastNonRhythmChordIndex(firstChunk); + + if (precedingChordIndex <= 0) { + return { firstChunk, secondChunk }; + } + + return { + firstChunk: firstChunk.slice(0, precedingChordIndex), + secondChunk: [...firstChunk.slice(precedingChordIndex), ...this.separatorItems(separator), ...secondChunk], + }; + } + + private separatorItems(separator: MeasuredItem | null): MeasuredItem[] { + return separator ? [separator] : []; + } + + private startsWithRhythmSymbol(items: MeasuredItem[]): boolean { + return this.isRhythmSymbolItem(items[0]); + } + + private countLeadingRhythmSymbols(items: MeasuredItem[]): number { + let count = 0; + + while (count < items.length && this.isRhythmSymbolItem(items[count])) { + count += 1; + } + + return count; + } + + private hasChordAfterLeadingRhythmSymbols(items: MeasuredItem[], leadingRhythmSymbolCount: number): boolean { + return this.isNonRhythmChordItem(items[leadingRhythmSymbolCount]); + } + + private findLastNonRhythmChordIndex(items: MeasuredItem[]): number { + for (let index = items.length - 1; index >= 0; index -= 1) { + if (this.isNonRhythmChordItem(items[index])) { + return index; } + } - return currentDistance < bestDistance ? current : best; - }); + return -1; + } - const firstChunk = items.slice(0, bestBreak.index); - const secondChunk = items.slice(bestBreak.index + 1); + private isRhythmSymbolItem(item: MeasuredItem | undefined): boolean { + return item?.item instanceof ChordLyricsPair && item.item.isRhythmSymbol; + } - this.removeTrailingComma(firstChunk); - this.capitalizeNextItem(secondChunk, secondChunk, 0); + private isNonRhythmChordItem(item: MeasuredItem | undefined): boolean { + return item?.item instanceof ChordLyricsPair && + !item.item.isRhythmSymbol && + (item.item.chords || '').trim() !== ''; + } - return { firstChunk, secondChunk }; + private calculateLineContextWidth(items: MeasuredItem[]): number { + return items.reduce((width, item, index) => { + const nextItem = items[index + 1] || null; + return width + this.recalculateWidthForLineContext(item, nextItem); + }, 0); + } + + private recalculateWidthForLineContext(item: MeasuredItem, nextItem: MeasuredItem | null): number { + if (item.item instanceof ChordLyricsPair) { + return this.recalculateChordLyricWidth(item, nextItem); + } + + if (!nextItem && item.item instanceof SoftLineBreak) { + return 0; + } + + return item.width; } private removeTrailingComma(items: MeasuredItem[]): void { diff --git a/test/layout/engine/item_processor.test.ts b/test/layout/engine/item_processor.test.ts index ddaca4174..f747cf775 100644 --- a/test/layout/engine/item_processor.test.ts +++ b/test/layout/engine/item_processor.test.ts @@ -496,6 +496,17 @@ describe('ItemProcessor', () => { expect(result[0]).toEqual(pair); }); + it('preserves rhythm symbols when splitting or measuring chord-lyrics pairs', () => { + const { processor } = createProcessor(); + const pair = new ChordLyricsPair('|', '', null, null, true); + const line = new Line(); + line.addChordLyricsPair(pair); + + const result = processor.measureLineItems(line); + + expect((result[0].item as ChordLyricsPair).isRhythmSymbol).toBe(true); + }); + it('returns pair when lyrics empty', () => { const { processor } = createProcessor(); const pair = createChordLyricsPair('C', ''); diff --git a/test/layout/engine/line_breaker.test.ts b/test/layout/engine/line_breaker.test.ts index e8ff22231..2d36a5dd4 100644 --- a/test/layout/engine/line_breaker.test.ts +++ b/test/layout/engine/line_breaker.test.ts @@ -145,7 +145,7 @@ class MockItemProcessor extends ItemProcessor { const chords = lyricsOnly ? '' : item.chords || ''; const lyricsWidth = this.measurer.measureTextWidth(normalizedLyrics, this.config.fonts.lyrics); const chordWidth = chords ? this.measurer.measureTextWidth(chords, this.config.fonts.chord) : 0; - const pair = new ChordLyricsPair(chords, normalizedLyrics); + const pair = new ChordLyricsPair(chords, normalizedLyrics, item.annotation, null, item.isRhythmSymbol); return { item: pair, width: Math.max(lyricsWidth, chordWidth), @@ -247,6 +247,10 @@ function createSoftBreak(content = ' '): SoftLineBreak { return new SoftLineBreak(content); } +function createRhythmSymbol(symbol: string): ChordLyricsPair { + return new ChordLyricsPair(symbol, null, null, null, true); +} + describe('LineBreaker', () => { const measurer = createMockMeasurer(); const config = createTestConfig(); @@ -407,6 +411,87 @@ describe('LineBreaker', () => { expect(totalWidth).toBeLessThanOrEqual(40 + 1); }); }); + + it('moves leading rhythm symbols to the previous line when they fit without overflow', () => { + const line = createTestLine([ + createChordLyricsPair('C', ''), + createRhythmSymbol('|'), + createChordLyricsPair('A/E', ''), + ]); + + const layouts = lineBreaker.breakLineIntoLayouts(line, 20); + + expect((layouts[0].items[layouts[0].items.length - 1].item as ChordLyricsPair).chords).toBe('|'); + expect((layouts[1].items[0].item as ChordLyricsPair).chords).toBe('A/E'); + }); + + it('preserves soft line break spacing when moving a rhythm symbol to the previous line', () => { + const line = createTestLine([ + createChordLyricsPair('C', ''), + createSoftBreak(), + createRhythmSymbol('|'), + createChordLyricsPair('A/E', ''), + ]); + + const layouts = lineBreaker.breakLineIntoLayouts(line, 30); + + expect(layouts[0].items.map(({ item }) => { + if (item instanceof ChordLyricsPair) return item.chords; + if (item instanceof SoftLineBreak) return item.content; + return ''; + })).toEqual(['C', ' ', '|']); + }); + + it('ignores trailing chord spacing when checking whether a line-ending rhythm symbol fits', () => { + const line = createTestLine([ + createChordLyricsPair('C', ''), + createRhythmSymbol('|'), + createChordLyricsPair('A/E', ''), + ]); + + itemProcessor.measureLineItemsSpy.mockImplementationOnce(() => [ + { item: new ChordLyricsPair('C', ''), width: 24, chordHeight: 10 }, + { item: createRhythmSymbol('|'), width: 32, chordHeight: 10 }, + { item: new ChordLyricsPair('A/E', ''), width: 24, chordHeight: 10 }, + ]); + + const layouts = lineBreaker.breakLineIntoLayouts(line, 32); + + expect((layouts[0].items[layouts[0].items.length - 1].item as ChordLyricsPair).chords).toBe('|'); + expect((layouts[1].items[0].item as ChordLyricsPair).chords).toBe('A/E'); + }); + + it('breaks earlier instead of overflowing to keep a rhythm symbol off the next line start', () => { + const line = createTestLine([ + createChordLyricsPair('C', ''), + createChordLyricsPair('F#m', ''), + createRhythmSymbol('|'), + createChordLyricsPair('A/E', ''), + ]); + + const layouts = lineBreaker.breakLineIntoLayouts(line, 35); + + expect((layouts[0].items[layouts[0].items.length - 1].item as ChordLyricsPair).chords).toBe('C'); + expect((layouts[1].items[0].item as ChordLyricsPair).chords).toBe('F#m'); + }); + + it('preserves soft line break spacing when breaking earlier before a rhythm symbol', () => { + const line = createTestLine([ + createChordLyricsPair('C', ''), + createChordLyricsPair('F#m', ''), + createSoftBreak(), + createRhythmSymbol('|'), + createChordLyricsPair('A/E', ''), + ]); + + const layouts = lineBreaker.breakLineIntoLayouts(line, 45); + + expect(layouts[1].items.map(({ item }) => { + if (item instanceof ChordLyricsPair) return item.chords; + if (item instanceof SoftLineBreak) return item.content; + return ''; + })).toEqual(['F#m', ' ', '|']); + }); }); describe('edge cases', () => { diff --git a/test/parser/chord_pro_parser.test.ts b/test/parser/chord_pro_parser.test.ts index c4e309e0d..b14453325 100644 --- a/test/parser/chord_pro_parser.test.ts +++ b/test/parser/chord_pro_parser.test.ts @@ -6,6 +6,7 @@ import { heredoc } from '../util/utilities'; import { ABC, CHORUS, + ChordLyricsPair, ChordProParser, LILYPOND, NONE, @@ -18,6 +19,18 @@ import { } from '../../src'; describe('ChordProParser', () => { + it('marks chord-only slash and bar tokens as rhythm symbols', () => { + const song = new ChordProParser().parse('[E][/][|][A/E]'); + const pairs = song.lines[0].items as ChordLyricsPair[]; + + expect(pairs.map((pair) => [pair.chords, pair.isRhythmSymbol])).toEqual([ + ['E', false], + ['/', true], + ['|', true], + ['A/E', false], + ]); + }); + it('parses a ChordPro chord sheet correctly', () => { const chordSheet = heredoc` {title: Let it be}