feat: add per-staff and mid-measure transposition to mx::api#305
Merged
Conversation
Splits the transposition half out of the original combined time-signature and transposition PR so each can be reviewed and merged independently. Closes #269
4 tasks
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 | 78.9% | 6152 / 7795 |
| Functions | 64.3% | 2093 / 3254 |
| Branches | 48.1% | 5247 / 10907 |
Core HTML report | API HTML report
Commit dc7e231d3979149be61be23a6412145eb3c5c299.
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
Seems a bit ugly but I think that is a due to the fact that transposition can either be specified at the part level (i.e. start of the score in the part list) or can be changed mid-measure (which is certainly required by real scores). So our ugly semantics mirror that and the a measure 0, tickTime 0 transposition is expressed both at the part level and the measure level (if I understand correctly).
As long as it makes enough sense to use, then It looks like it solves the missing feature of the API. I'm going to call it good and merge it.
Summary
mx::corealready fully modeled the transposition shape (Transpose::number()); the gap was entirely inmx::api/mx::impl.TransposeDatagainsstaffIndex(sameINDEX_UNSPECIFIEDconvention asKeyData::staffIndex) andtickTimePosition(mirrorsKeyData::tickTimePositionfor mid-measure changes).MeasureDatagains atranspositionsvector: event-based likekeys(populated whenever a measure's<attributes>contains<transpose>, at any measure/tick), not carried-forward.PartData::transpositionconvenience field (first<transpose>at measure 0/tick 0) is unchanged for backward compatibility; the writer only falls back to it when a measure has no explicittranspositionsof its own.Testing
PartData::transpositionstill behaves exactly as before (TranspositionTest.cpp)make test: all pass (4584 assertions in 361 test cases, plus the three examples)make test-api-roundtrip: 124 passed, 0 failed (of 124 pinned; no regressions)make fmt/make checkcleanReferences