fix: do not absorb the main chord into a preceding grace-note chord#346
Merged
Conversation
3 tasks
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28514 / 36648 |
| Functions | 74.2% | 6352 / 8556 |
| Branches | 50.7% | 22680 / 44751 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 84.4% | 7233 / 8568 |
| Functions | 74.9% | 2512 / 3353 |
| Branches | 52.3% | 6302 / 12048 |
Core HTML report | API HTML report
Commit f63f4ffc43348a795ab4cee065066d00f9cc6d20.
gen-quality
|
4c44177 to
302bca3
Compare
2989c24 to
de0d806
Compare
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28514 / 36648 |
| Functions | 74.2% | 6352 / 8556 |
| Branches | 50.7% | 22680 / 44751 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 84.6% | 7247 / 8570 |
| Functions | 74.9% | 2512 / 3353 |
| Branches | 52.4% | 6316 / 12052 |
Core HTML report | API HTML report
Commit 2a3d559e0cf3ddf2d6cb9feb8656ed06a93927c0.
gen-quality
|
Grace notes occupy no ticks, so a grace-note chord shares its tick position with the chord it ornaments; the writer's tick-only chord boundary detection stamped a spurious <chord/> on the main chord's first note. A grace/non-grace transition is now also a chord boundary. Pins the 2 corpus files this flips to PASS (224 total).
de0d806 to
541a835
Compare
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28514 / 36648 |
| Functions | 74.2% | 6352 / 8556 |
| Branches | 50.7% | 22680 / 44751 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 84.6% | 7247 / 8570 |
| Functions | 74.9% | 2512 / 3353 |
| Branches | 52.4% | 6316 / 12052 |
Core HTML report | API HTML report
Commit 3725731bd7af63c9ca08ff2520d905641f08fa8e.
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
Bug fix related to grace notes not advancing the tick time position.
Summary
Grace notes occupy no ticks (#318), so a grace-note chord shares its
tickTimePositionwiththe chord it ornaments.
MeasureWriter's chord-boundary detection compared tick positionsonly, so the first note of the main chord was treated as a continuation of the grace chord
and got a spurious
<chord/>tag — musically corrupting the output by merging the realchord into the zero-duration grace chord.
The fix tracks whether the previous chord note was a grace note: a grace/non-grace
transition at the same tick is also a chord boundary.
Stacked on #344 to keep the
roundtrip-baseline.txtadditions conflict-free; the 2 filesthis flips are pinned (222 -> 224).
Testing
GraceChordThenChordAtSameTickMeasureWriter test (fails before, passes after)References