feat: support cross-staff notes in the api#304
Open
webern wants to merge 1 commit into
Open
Conversation
Closes #289. A cross-staff note (a beamed run or chord that dips onto the other staff of a multi-staff part) could not be created through the api, and reading one split the beam group across staff buckets so the writer reordered the measure and broke beam adjacency. NoteData::crossStaffIndex (std::optional<int>) is the zero-based index of the staff on which the note is displayed when it differs from the staff that contains the note. Containment still governs voice membership, timing, and beam adjacency; the writer emits the override in the note's <staff> element instead of the containing staff's number. The reader detects cross-staff notation with a per-measure pre-scan: a level-1 beam group that properly closes and names more than one <staff> value is filed into its voice's home staff (the staff the voice was on when the group began), and each divergent member gets crossStaffIndex. A <chord> member always follows the bucket of the note it chords with, so a chord can no longer be split across staves. Beam groups that stay on one staff never relocate, so behavior for non-crossing files is unchanged. lysuite/ly43d_MultiStaff_StaffChange.xml now round-trips byte-faithfully and is pinned in roundtrip-baseline.txt (125 pinned). Discovery over the full corpus: 140 -> 141 PASS with zero newly-failing files. Unit suite: 363 cases, 4622 assertions, all passing.
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28513 / 36640 |
| Functions | 74.3% | 6352 / 8551 |
| Branches | 50.7% | 22678 / 44751 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 79.1% | 6215 / 7856 |
| Functions | 64.3% | 2095 / 3256 |
| Branches | 48.3% | 5312 / 10997 |
Core HTML report | API HTML report
Commit 1740e3ae6c541dc639ab39d16a75257cd49cad76.
gen-quality
|
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.
Human Summary
This seems fairly clean. If the note is to be displayed on a different staff from the one it is contained in, populate the std::optional crossStaffIndex.
Summary
Cross-staff notes (a beamed run or chord that dips onto the other staff of a multi-staff part) could not be created through the api, and reading one split the beam group across staff buckets, so the writer reordered the measure and broke beam adjacency.
NoteData::crossStaffIndex(std::optional<int>) is the zero-based index of the staff on which the note is displayed, for the rare case that it differs from the staff that contains the note. Containment still governs voice membership, timing, and beam adjacency; the writer emits the override in the note's staff element instead of the containing staff's number. Authors set one field; everything else is unchanged.On the read side, a per-measure pre-scan decides which notes are cross-staff before any bucketing happens:
crossStaffIndex. This handles groups that begin on the foreign staff (ly43d measure 1).The design follows the containment principle rather than adding a second source of truth for ownership: the field overrides only where the note is displayed, never which staff/voice owns it. Also removed a long-dead commented-out
staffIndexline inNoteFunctions.cppthat this feature supersedes.Testing
CrossStaffApiTest.cpp: authoring writes the override with beam adjacency intact (no forward, single backup); round-trip preservescrossStaffIndex; reader relocates the ly43d sticky-home shape and cross-staff chords; unbeamed staff changes and pure foreign-staff groups still bucket structurally with no overridelysuite/ly43d_MultiStaff_StaffChange.xmlnow round-trips byte-faithfully and is pinned inroundtrip-baseline.txtReferences