From 4141824b00ac534218d7ea83ef5f8d9b590d8f26 Mon Sep 17 00:00:00 2001 From: benjamin Date: Mon, 9 Mar 2026 17:18:49 +0100 Subject: [PATCH] optimize transpose service --- .../modules/songs/services/key.helper.spec.ts | 15 ++ src/app/modules/songs/services/key.helper.ts | 4 +- .../songs/services/transpose.service.spec.ts | 67 ++++++++- .../songs/services/transpose.service.ts | 133 ++++++++++++------ src/app/services/filter.helper.spec.ts | 8 +- 5 files changed, 179 insertions(+), 48 deletions(-) create mode 100644 src/app/modules/songs/services/key.helper.spec.ts diff --git a/src/app/modules/songs/services/key.helper.spec.ts b/src/app/modules/songs/services/key.helper.spec.ts new file mode 100644 index 0000000..8879eaa --- /dev/null +++ b/src/app/modules/songs/services/key.helper.spec.ts @@ -0,0 +1,15 @@ +import {getScale, scaleMapping} from './key.helper'; + +describe('key.helper', () => { + it('should render Gb correctly', () => { + expect(scaleMapping['Gb']).toBe('G♭'); + }); + + it('should expose a sharp-based scale for D', () => { + expect(getScale('D')).toEqual(['C', 'C#', 'D', 'D#', 'E', 'F', 'F#', 'G', 'G#', 'A', 'A#', 'H']); + }); + + it('should keep flat-based spelling for Db', () => { + expect(getScale('Db')).toEqual(['C', 'Db', 'D', 'Eb', 'E', 'F', 'Gb', 'G', 'Ab', 'A', 'B', 'H']); + }); +}); diff --git a/src/app/modules/songs/services/key.helper.ts b/src/app/modules/songs/services/key.helper.ts index 7ea7a7c..905ce40 100644 --- a/src/app/modules/songs/services/key.helper.ts +++ b/src/app/modules/songs/services/key.helper.ts @@ -82,7 +82,7 @@ const scaleAssignment: {[key: string]: string[]} = { C: KEYS_MAJOR_FLAT, 'C#': KEYS_MAJOR_FLAT, Db: KEYS_MAJOR_B, - D: KEYS_MAJOR_B, + D: KEYS_MAJOR_FLAT, 'D#': KEYS_MAJOR_FLAT, Eb: KEYS_MAJOR_B, E: KEYS_MAJOR_FLAT, @@ -125,7 +125,7 @@ export const scaleMapping: {[key: string]: string} = { E: 'E', F: 'F', 'F#': 'F♯', - Gb: 'D♭', + Gb: 'G♭', G: 'G', 'G#': 'G♯', Ab: 'A♭', diff --git a/src/app/modules/songs/services/transpose.service.spec.ts b/src/app/modules/songs/services/transpose.service.spec.ts index a8642a9..9421776 100644 --- a/src/app/modules/songs/services/transpose.service.spec.ts +++ b/src/app/modules/songs/services/transpose.service.spec.ts @@ -1,6 +1,8 @@ import {TestBed} from '@angular/core/testing'; import {TransposeService} from './transpose.service'; +import {LineType} from './line-type'; +import {Line} from './line'; describe('TransposeService', () => { let service: TransposeService; @@ -12,7 +14,7 @@ describe('TransposeService', () => { it('should create map upwards', () => { const distance = service.getDistance('D', 'G'); - const map = service.getMap('D', distance); + const map = service.getMap('D', 'G', distance); if (map) { void expect(map['D']).toBe('G'); @@ -21,10 +23,71 @@ describe('TransposeService', () => { it('should create map downwards', () => { const distance = service.getDistance('G', 'D'); - const map = service.getMap('G', distance); + const map = service.getMap('G', 'D', distance); if (map) { void expect(map['G']).toBe('D'); } }); + + it('should transpose enharmonic targets by semitone distance', () => { + const distance = service.getDistance('C', 'Db'); + const map = service.getMap('C', 'Db', distance); + + expect(distance).toBe(1); + expect(map?.['C']).toBe('Db'); + expect(map?.['G']).toBe('Ab'); + }); + + it('should keep german B/H notation consistent', () => { + const distance = service.getDistance('H', 'C'); + const map = service.getMap('H', 'C', distance); + + expect(distance).toBe(1); + expect(map?.['H']).toBe('C'); + expect(map?.['B']).toBe('C#'); + }); + + it('should render unknown chords as X', () => { + const line: Line = { + type: LineType.chord, + text: '', + chords: [ + {chord: 'Q', add: 'sus4', slashChord: null, position: 0, length: 1}, + ], + }; + + const rendered = service.renderChords(line); + + expect(rendered.text).toBe('Xsus4'); + }); + + it('should render unknown slash chords as X', () => { + const line: Line = { + type: LineType.chord, + text: '', + chords: [ + {chord: 'C', add: null, slashChord: 'Q', position: 0, length: 1}, + ], + }; + + const rendered = service.renderChords(line); + + expect(rendered.text).toBe('C/X'); + }); + + it('should transpose lines with long chord positions without truncating', () => { + const line: Line = { + type: LineType.chord, + text: '', + chords: [ + {chord: 'C', add: null, slashChord: null, position: 120, length: 1}, + ], + }; + + const rendered = service.renderChords(line); + + expect(rendered.text.length).toBe(121); + expect(rendered.text.endsWith('C')).toBeTrue(); + }); }); diff --git a/src/app/modules/songs/services/transpose.service.ts b/src/app/modules/songs/services/transpose.service.ts index 369d763..b6db59f 100644 --- a/src/app/modules/songs/services/transpose.service.ts +++ b/src/app/modules/songs/services/transpose.service.ts @@ -5,16 +5,56 @@ import {Chord} from './chord'; import {Line} from './line'; type TransposeMap = {[key: string]: string}; +type ScaleVariants = [string[], string[]]; @Injectable({ providedIn: 'root', }) export class TransposeService { + private readonly keyToSemitone: Record = { + C: 0, + 'C#': 1, + Db: 1, + D: 2, + 'D#': 3, + Eb: 3, + E: 4, + F: 5, + 'F#': 6, + Gb: 6, + G: 7, + 'G#': 8, + Ab: 8, + A: 9, + 'A#': 10, + B: 10, + H: 11, + c: 0, + 'c#': 1, + db: 1, + d: 2, + 'd#': 3, + eb: 3, + e: 4, + f: 5, + 'f#': 6, + gb: 6, + g: 7, + 'g#': 8, + ab: 8, + a: 9, + 'a#': 10, + b: 10, + h: 11, + }; + + private readonly mapCache = new Map(); + public transpose(line: Line, baseKey: string, targetKey: string): Line { if (line.type !== LineType.chord || !line.chords) return line; const difference = this.getDistance(baseKey, targetKey); - const map = this.getMap(baseKey, difference); + const map = this.getMap(baseKey, targetKey, difference); const chords = difference !== 0 && map ? line.chords.map(chord => this.transposeChord(chord, map)) : line.chords; const renderedLine = this.renderLine(chords); @@ -32,50 +72,47 @@ export class TransposeService { } public getDistance(baseKey: string, targetKey: string): number { - const scale = getScaleType(baseKey); - if (!scale) { + const baseSemitone = this.keyToSemitone[baseKey]; + const targetSemitone = this.keyToSemitone[targetKey]; + + if (baseSemitone === undefined || targetSemitone === undefined) { return 0; } - const primaryBaseIndex = scale[0].indexOf(baseKey); - const primaryTargetIndex = scale[0].indexOf(targetKey); - - if (primaryBaseIndex !== -1 && primaryTargetIndex !== -1) { - return (primaryTargetIndex - primaryBaseIndex) % 12; - } - - const secondaryBaseIndex = scale[1].indexOf(baseKey); - const secondaryTargetIndex = scale[1].indexOf(targetKey); - - if (secondaryBaseIndex !== -1 && secondaryTargetIndex !== -1) { - return (secondaryTargetIndex - secondaryBaseIndex) % 12; - } - - return 0; + return (targetSemitone - baseSemitone + 12) % 12; } - public getMap(baseKey: string, difference: number): TransposeMap | null { - const scale = getScaleType(baseKey); - if (!scale) { + public getMap(baseKey: string, targetKey: string, difference: number): TransposeMap | null { + const cacheKey = `${baseKey}:${targetKey}:${difference}`; + const cachedMap = this.mapCache.get(cacheKey); + if (cachedMap) { + return cachedMap; + } + + const sourceScales = this.getScaleVariants(baseKey); + const targetScales = this.getScaleVariants(targetKey); + if (!sourceScales || !targetScales) { return null; } - const map: {[key: string]: string} = {}; - for (let i = 0; i < 12; i++) { - const source = scale[0][i]; - const mappedIndex = (i + difference + 12) % 12; - map[source] = scale[0][mappedIndex]; - } - for (let i = 0; i < 12; i++) { - const source = scale[1][i]; - const mappedIndex = (i + difference + 12) % 12; - map[source] = scale[1][mappedIndex]; - } + + const map: TransposeMap = {}; + sourceScales.forEach((sourceScale, scaleIndex) => { + const targetScale = targetScales[scaleIndex]; + for (let i = 0; i < 12; i++) { + const source = sourceScale[i]; + const mappedIndex = (i + difference + 12) % 12; + map[source] = targetScale[mappedIndex]; + } + }); + + this.mapCache.set(cacheKey, map); return map; } private transposeChord(chord: Chord, map: TransposeMap): Chord { - const translatedChord = map[chord.chord]; - const translatedSlashChord = chord.slashChord ? map[chord.slashChord] : null; + const translatedChord = map[chord.chord] ?? 'X'; + const translatedSlashChord = chord.slashChord ? map[chord.slashChord] ?? 'X' : null; + return { ...chord, chord: translatedChord, @@ -84,23 +121,39 @@ export class TransposeService { } private renderLine(chords: Chord[]): string { - let template = ' '; + const width = chords.reduce((max, chord) => { + return Math.max(max, chord.position + this.renderChord(chord).length); + }, 0); + + let template = ''.padEnd(width, ' '); chords.forEach(chord => { const pos = chord.position; const renderedChord = this.renderChord(chord); const newLength = renderedChord.length; - const pre = template.substr(0, pos); - const post = template.substr(pos + newLength); + if (template.length < pos + newLength) { + template = template.padEnd(pos + newLength, ' '); + } + + const pre = template.slice(0, pos); + const post = template.slice(pos + newLength); template = pre + renderedChord + post; }); - return template.trimRight(); + return template.trimEnd(); } - private renderChord(chord: Chord) { - return scaleMapping[chord.chord] + (chord.add ? chord.add : '') + (chord.slashChord ? '/' + scaleMapping[chord.slashChord] : ''); + private renderChord(chord: Chord): string { + const renderedChord = scaleMapping[chord.chord] ?? 'X'; + const renderedSlashChord = chord.slashChord ? scaleMapping[chord.slashChord] ?? 'X' : ''; + + return renderedChord + (chord.add ?? '') + (renderedSlashChord ? '/' + renderedSlashChord : ''); + } + + private getScaleVariants(key: string): ScaleVariants | null { + const scales = getScaleType(key); + return scales ? [scales[0], scales[1]] : null; } } diff --git a/src/app/services/filter.helper.spec.ts b/src/app/services/filter.helper.spec.ts index 9196698..4aa490b 100644 --- a/src/app/services/filter.helper.spec.ts +++ b/src/app/services/filter.helper.spec.ts @@ -5,10 +5,10 @@ describe('Filter Helper', () => { const song: Song = { title: 'Song Title', text: "This is a songtext, aa?bb!cc,dd.ee'ff", - legalOwner: '', + legalOwner: 'other', label: '', id: '', - legalType: '', + legalType: 'open', artist: '', comment: '', edits: [], @@ -18,9 +18,9 @@ describe('Filter Helper', () => { number: 1, legalOwnerId: '', origin: '', - status: '', + status: 'draft', tempo: 10, - type: '', + type: 'Misc', termsOfUse: '', };