Repository navigation
Conversation
sschmidTU
requested changes
Oct 28, 2026
sschmidTU
left a comment
sschmidTU
left a comment
Contributor
There was a problem hiding this comment.
Thanks a lot for this PR, and for the detailed description and the careful checks! I checked it on its base, develop 0e55fe6:
- I wrote the issue's reproduction from its description and get the same as you: the 8va now starts at x 226.82 instead of 187.04, as in the issue's measurements. This also works for an 8vb on the lower staff whose voice continues on the upper staff, for the piano as the second part of a score, and for an 8va that continues into the next system. MuseScore 4 starts the 8va and the 8vb at the same beat when it imports such a file.
- As you wrote, none of the sample images changes. Neither do the 125 scores of the LilyPond test suite, nor 242 more scores I use for testing. All 86 octave shifts in these scores start where their own staff has a note, so only the reported case changes.
- The suite passes in Firefox and Chrome (782 tests), and lint and tsc are clean. The four cross-staff tests fail on develop, in both browsers.
The change itself looks good to me. I also checked the correction for the other staff's stave, which isn't positioned yet: without it, the 8va of the upper staff is drawn at x 111 instead of 227, because the lower staff is only drawn after it. Your check of the drawn label's x catches that, which I found a very good idea!
I'd like to suggest two changes, both in the diff below:
-
Notes of other parts. I'd take the x from a note at the octave shift's time on any staff of the system, not only on the staves of its instrument: the staves of a system share the x of a time. Your test for a different instrument expects the line at the first note of the measure, a beat before the octave shift, as in the issue, while MuseScore starts it at beat 2 there as well. The loop then only needs to skip the staves that aren't drawn in the system. With this change, nothing else changes in the scores above. When a violin above the piano also has a note at beat 2, the line gets the same x from the violin's note as from the piano's other staff.
-
A smaller test, with a sample. Eight tests seem a lot to me for a case that none of the scores above has:
- The four octave shift types run the same code. An 8va of the upper staff and an 8vb of the lower staff cover both drawing orders, and the first of them needs the correction.
- The second render at zoom 0.75 draws the same positions in these scores, so it checks the same as the first one.
- The same-staff start and the clipped starts pass on develop too, and other tests cover them, e.g.
InStaffInstructionSpanners_Test, also withdrawFromMeasureNumber. bracket.start.getStave()reads VexFlow'sTextBracket. The drawn label's y shows its staff just as well, as its x shows the start.
A small sample in
test/datawould also make the case visible in the visual regression tests (npm run test:visual), where no image changes with this PR so far. The diff addstest_octaveshift_start_cross_staff_1841.musicxml: an 8va of the upper staff that starts where its voice is on the lower staff (measure 1, as in the issue), and an 8vb of the lower staff that starts where its voice is on the upper staff (measure 2). The test checks the x and y of both drawn labels:- On develop, it fails: the 8va starts at x 187 instead of 227.
- It also fails without the correction (x 111), or when the bracket is placed at the other staff (the 8vb is then drawn below the upper staff).
- With your change and the diff, the suite passes in Firefox and Chrome (775 tests), and lint and tsc are clean.
The image shows the sample on develop (above) and with your change (below):
image: pr1850_octave_shift_cross_staff_start.png
Diff (the calculator, OctaveShiftTimestamp_Test.ts and the new sample), applies to your branch
diff --git a/src/MusicalScore/Graphical/VexFlow/VexFlowMusicSheetCalculator.ts b/src/MusicalScore/Graphical/VexFlow/VexFlowMusicSheetCalculator.ts
index 329af2ab..e8eae603 100644
--- a/src/MusicalScore/Graphical/VexFlow/VexFlowMusicSheetCalculator.ts
+++ b/src/MusicalScore/Graphical/VexFlow/VexFlowMusicSheetCalculator.ts
@@ -1772,12 +1772,12 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
const graphicalOctaveShift: VexFlowOctaveShift = new VexFlowOctaveShift(octaveShift, startStaffLine.PositionAndShape);
let startStaffEntry: GraphicalStaffEntry = startMeasure.findGraphicalStaffEntryFromTimestamp(startTimeStamp);
if (!this.hasVexFlowNote(startStaffEntry) && !startIsClipped) {
- // A cross-staff voice can have its onset on another staff of this instrument.
- // Borrow its horizontal anchor, while keeping the bracket on its source staff.
+ // No note of its staff at its time, e.g. where its voice continues on the other staff of a piano:
+ // the staves of a system share the x of a time, so borrow the horizontal anchor of another staff's note there,
+ // while keeping the bracket on its source staff.
for (const measure of this.graphicalMusicSheet.MeasureList[measureIndex]) {
- if (measure.ParentStaff.ParentInstrument !== startMeasure.ParentStaff.ParentInstrument ||
- measure.ParentStaffLine?.ParentMusicSystem !== startStaffLine.ParentMusicSystem) {
- continue;
+ if (measure.ParentStaffLine?.ParentMusicSystem !== startStaffLine.ParentMusicSystem) {
+ continue; // not drawn, e.g. a hidden part
}
const entry: GraphicalStaffEntry = measure.findGraphicalStaffEntryFromTimestamp(startTimeStamp);
if (this.hasVexFlowNote(entry)) {
diff --git a/test/MusicalScore/Graphical/VexFlow/OctaveShiftTimestamp_Test.ts b/test/MusicalScore/Graphical/VexFlow/OctaveShiftTimestamp_Test.ts
index b39dd4fb..5b3eae3a 100644
--- a/test/MusicalScore/Graphical/VexFlow/OctaveShiftTimestamp_Test.ts
+++ b/test/MusicalScore/Graphical/VexFlow/OctaveShiftTimestamp_Test.ts
@@ -1,9 +1,9 @@
import { expect } from "chai";
-import { VexFlowMeasure } from "../../../../src/MusicalScore/Graphical/VexFlow/VexFlowMeasure";
-import { VexFlowOctaveShift } from "../../../../src/MusicalScore/Graphical/VexFlow/VexFlowOctaveShift";
+import { GraphicalMeasure } from "../../../../src/MusicalScore/Graphical/GraphicalMeasure";
+import { GraphicalStaffEntry } from "../../../../src/MusicalScore/Graphical/GraphicalStaffEntry";
+import { StaffLine } from "../../../../src/MusicalScore/Graphical/StaffLine";
+import { unitInPixels } from "../../../../src/MusicalScore/Graphical/VexFlow/VexFlowMusicSheetDrawer";
import { VexFlowVoiceEntry } from "../../../../src/MusicalScore/Graphical/VexFlow/VexFlowVoiceEntry";
-import { OctaveEnum } from "../../../../src/MusicalScore/VoiceData/Expressions/ContinuousExpressions/OctaveShift";
-import { FillEmptyMeasuresWithWholeRests } from "../../../../src/OpenSheetMusicDisplay/OSMDOptions";
import { OpenSheetMusicDisplay } from "../../../../src/OpenSheetMusicDisplay/OpenSheetMusicDisplay";
import { TestUtils } from "../../../Util/TestUtils";
@@ -21,107 +21,27 @@ describe("Octave shift start timestamps", () => {
container.remove();
});
- function note(staff: number, duration: number, type: string, voice: number = 1): string {
- return `<note><pitch><step>C</step><octave>4</octave></pitch><duration>${duration}</duration>
- <voice>${voice}</voice><type>${type}</type><staff>${staff}</staff></note>`;
+ /** The x of the note at beat 2 of the measure, in pixels. */
+ function beat2X(measure: GraphicalMeasure): number {
+ const entry: GraphicalStaffEntry = measure.staffEntries.find(staffEntry => staffEntry.relInMeasureTimestamp.RealValue === 0.25);
+ return (entry.graphicalVoiceEntries[0] as VexFlowVoiceEntry).vfStaveNote.getAbsoluteX();
}
- function score(sourceStaff: number = 1, targetStaff: number = 2, type: string = "down", size: number = 8): string {
- return `<?xml version="1.0" encoding="UTF-8"?>
- <score-partwise version="4.0"><part-list><score-part id="P1"><part-name>Piano</part-name>
- </score-part></part-list><part id="P1"><measure number="1"><attributes><divisions>8</divisions>
- <time><beats>4</beats><beat-type>4</beat-type></time><staves>2</staves>
- <clef number="1"><sign>F</sign><line>4</line></clef><clef number="2"><sign>F</sign><line>4</line></clef>
- </attributes>${note(sourceStaff, 8, "quarter")}<direction><direction-type>
- <octave-shift type="${type}" size="${size}"/></direction-type><staff>${sourceStaff}</staff></direction>
- ${note(targetStaff, 8, "quarter")}${note(sourceStaff, 16, "half")}
- <backup><duration>32</duration></backup><note><rest/><duration>32</duration><voice>2</voice>
- <type>whole</type><staff>${3 - sourceStaff}</staff></note></measure></part></score-partwise>`;
- }
-
- function voiceAt(measure: VexFlowMeasure, time: number): VexFlowVoiceEntry {
- return measure.staffEntries.find(entry => entry.relInMeasureTimestamp.RealValue === time)
- .graphicalVoiceEntries[0] as VexFlowVoiceEntry;
- }
-
- const shifts: {type: string, size: number, staff: number, expected: OctaveEnum}[] = [
- {type: "down", size: 8, staff: 1, expected: OctaveEnum.VA8},
- {type: "up", size: 8, staff: 2, expected: OctaveEnum.VB8},
- {type: "down", size: 15, staff: 2, expected: OctaveEnum.MA15},
- {type: "up", size: 15, staff: 1, expected: OctaveEnum.MB15}
- ];
- for (const shift of shifts) {
- it(`anchors ${shift.type} ${shift.size} at an onset on the other staff without moving the bracket there`, async () => {
- await osmd.load(score(shift.staff, 3 - shift.staff, shift.type, shift.size));
- for (const zoom of [1, 0.75]) {
- osmd.Zoom = zoom;
- osmd.render();
- const own: VexFlowMeasure = osmd.GraphicSheet.MeasureList[0][shift.staff - 1] as VexFlowMeasure;
- const other: VexFlowMeasure = osmd.GraphicSheet.MeasureList[0][2 - shift.staff] as VexFlowMeasure;
- const target: VexFlowVoiceEntry = voiceAt(other, 0.25);
- const graphical: VexFlowOctaveShift = own.ParentStaffLine.OctaveShifts[0] as VexFlowOctaveShift;
- const bracket: ReturnType<VexFlowOctaveShift["getTextBracket"]> = graphical.getTextBracket();
- expect(graphical.startNote === target.vfStaveNote, "same onset anchor").to.equal(true);
- expect(bracket.start.getAbsoluteX()).to.be.closeTo(target.vfStaveNote.getAbsoluteX(), 0.001);
- expect(bracket.start.getStave() === own.getVFStave(), "source staff").to.equal(true);
- expect(target.octaveShiftValue, "other-staff pitch remains unshifted").to.equal(OctaveEnum.NONE);
- expect(voiceAt(own, 0).octaveShiftValue, "note before the direction").to.equal(OctaveEnum.NONE);
- expect(voiceAt(own, 0.5).octaveShiftValue, "own-staff note after the direction").to.equal(shift.expected);
- expect(other.ParentStaffLine.OctaveShifts.length).to.equal(0);
- const text: SVGTextElement = Array.from(container.querySelectorAll("text"))
- .find(element => element.textContent === String(shift.size)) as SVGTextElement;
- expect(text, "actual rendered octave label").not.to.equal(undefined);
- expect(Number(text.getAttribute("x"))).to.be.closeTo(target.vfStaveNote.getAbsoluteX(), 0.001);
- }
- });
- }
-
- it("keeps an exact same-staff anchor", async () => {
- await osmd.load(score(1, 1));
- osmd.render();
- const own: VexFlowMeasure = osmd.GraphicSheet.MeasureList[0][0] as VexFlowMeasure;
- const graphical: VexFlowOctaveShift = own.ParentStaffLine.OctaveShifts[0] as VexFlowOctaveShift;
- expect(graphical.getTextBracket().start === voiceAt(own, 0.25).vfStaveNote).to.equal(true);
- });
-
- it("does not borrow another staff's onset at the old timestamp after clipping", async () => {
- const continuation: string = `<measure number="2">${note(1, 32, "whole")}
- <backup><duration>32</duration></backup>${note(2, 8, "quarter", 2)}${note(2, 8, "quarter", 2)}${note(2, 16, "half", 2)}</measure>`;
- await osmd.load(score().replace("</part>", continuation + "</part>"));
- osmd.setOptions({drawFromMeasureNumber: 2});
- osmd.render();
- const own: VexFlowMeasure = osmd.GraphicSheet.MeasureList[1][0] as VexFlowMeasure;
- const graphical: VexFlowOctaveShift = own.ParentStaffLine.OctaveShifts[0] as VexFlowOctaveShift;
- expect(graphical.getTextBracket().start === voiceAt(own, 0).vfStaveNote).to.equal(true);
- });
-
- it("keeps a clipped boundary anchor in its later measure when the first visible measure is empty", async () => {
- const continuation: string = `<measure number="2">${note(2, 32, "whole")}</measure>
- <measure number="3">${note(1, 32, "whole")}</measure>`;
- osmd.EngravingRules.FillEmptyMeasuresWithWholeRest = FillEmptyMeasuresWithWholeRests.No;
- await osmd.load(score().replace("</part>", continuation + "</part>"));
- osmd.setOptions({drawFromMeasureNumber: 2});
- osmd.render();
- const empty: VexFlowMeasure = osmd.GraphicSheet.MeasureList[1][0] as VexFlowMeasure;
- const own: VexFlowMeasure = osmd.GraphicSheet.MeasureList[2][0] as VexFlowMeasure;
- expect(empty.staffEntries.length, "first rendered source measure has no anchors").to.equal(0);
- expect(empty.ParentStaffLine === own.ParentStaffLine, "fallback is later on the same system").to.equal(true);
- const graphical: VexFlowOctaveShift = own.ParentStaffLine.OctaveShifts[0] as VexFlowOctaveShift;
- const fallback: VexFlowVoiceEntry = voiceAt(own, 0);
- expect(graphical.getTextBracket().start.getAbsoluteX()).to.be.closeTo(fallback.vfStaveNote.getAbsoluteX(), 0.001);
- expect(graphical.getTextBracket().start === fallback.vfStaveNote).to.equal(true);
- });
-
- it("does not borrow an onset from a different instrument", async () => {
- const otherPart: string = `<part id="P2"><measure number="1"><attributes><divisions>8</divisions></attributes>
- ${note(1, 8, "quarter")}${note(1, 8, "quarter")}${note(1, 16, "half")}</measure></part>`;
- const xml: string = score().replace(note(2, 8, "quarter"), "<forward><duration>8</duration></forward>")
- .replace("</part-list>", '<score-part id="P2"><part-name>Other</part-name></score-part></part-list>')
- .replace("</score-partwise>", otherPart + "</score-partwise>");
- await osmd.load(xml);
+ it("starts at its time also when its voice is on the other staff there, and stays at its own staff (#1841)", async () => {
+ await osmd.load(TestUtils.getScore("test_octaveshift_start_cross_staff_1841.musicxml"));
osmd.render();
- const own: VexFlowMeasure = osmd.GraphicSheet.MeasureList[0][0] as VexFlowMeasure;
- const graphical: VexFlowOctaveShift = own.ParentStaffLine.OctaveShifts[0] as VexFlowOctaveShift;
- expect(graphical.getTextBracket().start === voiceAt(own, 0).vfStaveNote).to.equal(true);
+ // the drawn "8" labels: the 8va of the upper staff line, then the 8vb of the lower one
+ const labels: SVGTextElement[] = Array.from(container.querySelectorAll("text")).filter(text => text.textContent === "8");
+ expect(labels.length, "8va and 8vb").to.equal(2);
+ const [vaX, vbX] = labels.map(label => Number(label.getAttribute("x")));
+ const [vaY, vbY] = labels.map(label => Number(label.getAttribute("y")));
+ const [upper, lower] = osmd.GraphicSheet.MeasureList[0].map(measure => measure.ParentStaffLine);
+ // m1: the 8va of the upper staff starts at beat 2, where its voice is on the lower staff
+ expect(vaX, "8va at beat 2").to.be.closeTo(beat2X(osmd.GraphicSheet.MeasureList[0][1]), 0.01);
+ // m2: the 8vb of the lower staff starts at beat 2, where its voice is on the upper staff
+ expect(vbX, "8vb at beat 2").to.be.closeTo(beat2X(osmd.GraphicSheet.MeasureList[1][0]), 0.01);
+ const top: (staffLine: StaffLine) => number = staffLine => staffLine.PositionAndShape.AbsolutePosition.y * unitInPixels;
+ expect(vaY, "8va above the upper staff").to.be.below(top(upper));
+ expect(vbY, "8vb below the lower staff").to.be.above(top(lower) + lower.StaffHeight * unitInPixels);
});
});
diff --git a/test/data/test_octaveshift_start_cross_staff_1841.musicxml b/test/data/test_octaveshift_start_cross_staff_1841.musicxml
new file mode 100644
index 00000000..2382b5e6
--- /dev/null
+++ b/test/data/test_octaveshift_start_cross_staff_1841.musicxml
@@ -0,0 +1,151 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE score-partwise PUBLIC "-//Recordare//DTD MusicXML 4.0 Partwise//EN" "http://www.musicxml.org/dtds/partwise.dtd">
+<score-partwise version="4.0">
+ <work>
+ <work-title>Octave shifts starting where their voice is on the other staff (#1841)</work-title>
+ </work>
+ <identification>
+ <encoding>
+ <software>hand-written OSMD test sample</software>
+ </encoding>
+ </identification>
+ <part-list>
+ <score-part id="P1">
+ <part-name>Piano</part-name>
+ </score-part>
+ </part-list>
+ <part id="P1">
+ <measure number="1">
+ <attributes>
+ <divisions>1</divisions>
+ <key>
+ <fifths>0</fifths>
+ </key>
+ <time>
+ <beats>4</beats>
+ <beat-type>4</beat-type>
+ </time>
+ <staves>2</staves>
+ <clef number="1">
+ <sign>G</sign>
+ <line>2</line>
+ </clef>
+ <clef number="2">
+ <sign>F</sign>
+ <line>4</line>
+ </clef>
+ </attributes>
+ <note>
+ <pitch>
+ <step>E</step>
+ <octave>5</octave>
+ </pitch>
+ <duration>1</duration>
+ <voice>1</voice>
+ <type>quarter</type>
+ <staff>1</staff>
+ </note>
+ <!-- the 8va of the upper staff starts at beat 2, where its voice is on the lower staff -->
+ <direction placement="above">
+ <direction-type>
+ <octave-shift type="down" size="8"/>
+ </direction-type>
+ <staff>1</staff>
+ </direction>
+ <note>
+ <pitch>
+ <step>C</step>
+ <octave>4</octave>
+ </pitch>
+ <duration>1</duration>
+ <voice>1</voice>
+ <type>quarter</type>
+ <staff>2</staff>
+ </note>
+ <note>
+ <pitch>
+ <step>G</step>
+ <octave>6</octave>
+ </pitch>
+ <duration>2</duration>
+ <voice>1</voice>
+ <type>half</type>
+ <staff>1</staff>
+ </note>
+ <direction placement="above">
+ <direction-type>
+ <octave-shift type="stop" size="8"/>
+ </direction-type>
+ <staff>1</staff>
+ </direction>
+ <backup>
+ <duration>4</duration>
+ </backup>
+ <note>
+ <rest measure="yes"/>
+ <duration>4</duration>
+ <voice>5</voice>
+ <type>whole</type>
+ <staff>2</staff>
+ </note>
+ </measure>
+ <measure number="2">
+ <note>
+ <rest measure="yes"/>
+ <duration>4</duration>
+ <voice>1</voice>
+ <type>whole</type>
+ <staff>1</staff>
+ </note>
+ <backup>
+ <duration>4</duration>
+ </backup>
+ <note>
+ <pitch>
+ <step>C</step>
+ <octave>3</octave>
+ </pitch>
+ <duration>1</duration>
+ <voice>5</voice>
+ <type>quarter</type>
+ <staff>2</staff>
+ </note>
+ <!-- the 8vb of the lower staff starts at beat 2, where its voice is on the upper staff -->
+ <direction placement="below">
+ <direction-type>
+ <octave-shift type="up" size="8"/>
+ </direction-type>
+ <staff>2</staff>
+ </direction>
+ <note>
+ <pitch>
+ <step>C</step>
+ <octave>4</octave>
+ </pitch>
+ <duration>1</duration>
+ <voice>5</voice>
+ <type>quarter</type>
+ <staff>1</staff>
+ </note>
+ <note>
+ <pitch>
+ <step>C</step>
+ <octave>2</octave>
+ </pitch>
+ <duration>2</duration>
+ <voice>5</voice>
+ <type>half</type>
+ <staff>2</staff>
+ </note>
+ <direction placement="below">
+ <direction-type>
+ <octave-shift type="stop" size="8"/>
+ </direction-type>
+ <staff>2</staff>
+ </direction>
+ <barline location="right">
+ <bar-style>light-heavy</bar-style>
+ </barline>
+ </measure>
+ </part>
+</score-partwise>
+```
</details>
Contributor
Author
|
Applied your suggested changes in e69c245, after merging develop at 618fce7. The horizontal anchor can now come from any rendered staff in the same system, while the bracket stays on its source staff. The eight tests are replaced by your MusicXML sample and one test of the drawn 8va/8vb labels. Build/lint/typecheck pass. Chromium and Firefox pass 820 tests each; restoring the develop production files makes the new test fail in each browser. Of 467 visual image pairs, only the new sample changes (894 pixels), with the other 466 identical. The PR description now reflects this revision and its validation limits. Thanks for the careful review and the sample! |
sschmidTU
approved these changes
Nov 1, 2026
sschmidTU
left a comment
sschmidTU
left a comment
Contributor
There was a problem hiding this comment.
Thanks a lot for adopting both suggestions, and for updating the description!
Your branch now has exactly the version I tested, merged with develop (618fce7), so I checked it again there:
- None of the 466 sample images changes, and the new sample is the only new image. None of the 125 scores of the LilyPond test suite and the 242 other scores changes either.
- The suite passes in Firefox and Chrome (820 tests each), and lint and tsc are clean. The new test fails on develop in both browsers: the 8va starts at x 187 instead of 227.
Looks good to me, thanks again for the PR!
Merging.
sschmidTU
added a commit
that referenced
this pull request
Nov 1, 2026
…er cuts off an octave shift, pedal or trill line: Start it at the first drawn note and end it open at the last one, like at a system break, not at its own beat in the first or last drawn measure (with a closing hook) An 8va from beat 2 of measure 1 started at beat 2 of measure 2 with drawFromMeasureNumber 2, after the first note of measure 2, and one from measure 1 into measure 3 ended at its stop's beat in measure 2 with drawUpToMeasureNumber 2, with a closing hook although it goes on. The calculation looked up the timestamp of the start or end within its measure in the first or last drawn measure (VexFlowMusicSheetCalculator.calculateSingleOctaveShift(), calculateSinglePedal(), calculateSingleWavyLine()), which gave the first or last drawn note only where that measure had no note at that beat. Now a cut-off start begins at the first note of the drawn range on the line's staff, and a cut-off end stops at the last one, drawn like the end of a line that goes on in the next system: an octave bracket open and on to the barline, a pedal line open and without the release sign (*) or change notch, a trill line on to the end of the measure. A cut-off pedal start keeps its hook or Ped. sign at the first drawn note, where playing starts, without a change notch. Found in the review of PR #1850 (pre-existing). New test sample test_drawing_range_octaveshift_pedal_trill.musicxml (a piano with an 8va over the right hand, a trill line over the left hand and a pedal line below it, from beat 2 of measure 1 to beat 3 of measure 4). Default renders don't cut off lines: no sample image changes. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1841.
When an octave-shift direction has no note on its source staff at that time, its graphical start falls back to the first note of that staff. In the issue's reproduction, this draws 8va one beat early.
Use the onset on another rendered staff in the same system for the horizontal start, including a staff of another instrument. Keep the source stave for vertical placement, and account for the borrowed stave not yet being positioned when the source staff is drawn. Pitch handling and the octave shift's staff scope stay unchanged. The existing fallback remains when no matching onset exists or the start is clipped by the drawing range.
The regression uses one MusicXML sample in
test/datawith an upper-staff 8va and a lower-staff 8vb. One rendering test checks the actual drawn labels' x and y, covering both staff drawing orders. The same sample is included in the visual regression run.Validation on Linux, Node 24, Chromium 151 and Firefox 153, after merging current
developat618fce7:npm run build(including ESLint and TypeScript): passed.npm test -- --browsers ChromeHeadlessNoSandbox,FirefoxHeadless: 820 tests per browser, 1640 passed.develop, the new rendering test fails in each browser (1638 passed, 2 expected failures). Its 8va starts at x=187.028 rather than x=226.813.generate:blessed,generate:currentandtest:visual: 467 image pairs. The new octave-shift sample is the only changed image (894 pixels, 0.1430%); all 466 other pairs are identical. The new sample was included on both sides to compare its old and corrected rendering.Windows/macOS and hosted CI were not run for this revision. The native image tool cannot discover Git metadata in isolated source copies;
618fce7identifies the blessed production sources. This targets the current VexFlow backend; the version migrations in #1139/#1673 will need the usual port. #1837 overlaps the calculator in separate dynamic-expression code.