refactor beam code #9792
refactor beam code #9792
Conversation
|
I'll look at the failed test tomorrow |
|
I just want to say, I am really happy to see this happening! The cross-staff beaming is a pain currently because it can't laid out fully until we calculate system distances, but we can't do that until we do "most" beaming, so there are all sorts of dependencies and obscure bugs and glitches that result. Similar story for cross-measure beams, and most especially ones that cross systems. Would be great to be able to finally handle those, but even if this doesn't do the job right away, a major rewrite of the beam code was oong overdue, and hopefully it's a good first step to solving those other thorny problems. |
|
Rebase needed. |
|
Pushed an update with a rebase and hopefully fixed the unit tests to match the new specs. Admittedly, I did remove a few outdated unit tests and I do need to add many more unit tests in future PRs. |
Nick-Mazuk
force-pushed
the
beams
branch
4 times, most recently
from
3fea76b
to
872c8cf
Compare
fix compile errors from rebasing fix codestyle fix beam positions with non-standard noteheads fix end of flag fix codestyle fix tests checkpoint minor change test savepoint remove outdated beam tests update codestyle fixed style property type value update style preference value from int to double fix type errors when accessing style values fix all tests with beams
|
The vtests are failing, but I'd expect them to fail given that this PR includes substantial visual changes. |
|
It would seem that this has broken the case where the user has manually adjusted the beam position (in which case the y positions are stored in a the property 'fragments', but then in layout2( ) this just gets overwritten? @Nick-Mazuk |
|
|
||
| int fragmentIndex = (_direction == Direction::AUTO || _direction == Direction::DOWN) ? 0 : 1; | ||
| fragments[frag]->py1[fragmentIndex] = startAnchor.y(); | ||
| fragments[frag]->py2[fragmentIndex] = endAnchor.y(); |
There was a problem hiding this comment.
I would think we need to check the _userModified flag at this point, and not overwrite y y coordinates for fragments where that's true? @Nick-Mazuk
Resolves: #9023, #9494
This PR is a complete rewrite of the beaming code to match with @oktophonie's new beam specifications. This PR should receive his approval before merging.
Attached are a PDF of the desired results I used for testing as well as the original MuseScore file that generated those results.
beams.pdf
stems.mscz.zip
There are a few features that used to work in the old beaming code but are not yet implemented yet. Expect these in a new PR very soon:
As far as I know, the missing features do not produce crashes or other unintended side effects. Instead of waiting to submit this PR until those features are done, I'm submitting it to: