Beam / tremolo layout refactor #16223
Merged
Conversation
This file contains 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
asattely
force-pushed
the
beam-refactor
branch
5 times, most recently
from
72105f0
to
7e1eebb
Compare
This comment was marked as resolved.
This comment was marked as resolved.
asattely
force-pushed
the
beam-refactor
branch
6 times, most recently
from
9e04765
to
68976e6
Compare
asattely
force-pushed
the
beam-refactor
branch
14 times, most recently
from
345b291
to
25f0e85
Compare
oktophonie
added
vtests
asattely
force-pushed
the
beam-refactor
branch
2 times, most recently
from
fe002d0
to
1bff2d2
Compare
src/engraving/libmscore/beam.h
Outdated
| double y2; | ||
| }; | ||
|
|
||
| class BeamLayout |
There was a problem hiding this comment.
I would recommend moving this class to a separate file inside the engraving/layout folder
There was a problem hiding this comment.
All BeamLayout functionality moved out into beamlayout.cpp!
There was a problem hiding this comment.
You should also move the declaration of this class into beamlayout.h. Both of these files (beamlayout.h/cpp) should be placed in engraving/layout
There was a problem hiding this comment.
done! moved both into beamtremololayout .cpp/.h
asattely
force-pushed
the
beam-refactor
branch
3 times, most recently
from
c0605db
to
fb99cdf
Compare
so that two-note tremolos can use it as well
The vast majority of these utest fixes are due to beam positions being able to occupy a floater space outside the staff if they slant away. A few others add tremolo anchor point information which this PR added
asattely
force-pushed
the
beam-refactor
branch
2 times, most recently
from
bebc3d7
to
9cf89c5
Compare
and rebase shenanigans for PR musescore#16719
RomanPudashkin
approved these changes
Mar 22, 2023
asattely
added a commit
to asattely/MuseScore
that referenced
this pull request
Apr 11, 2023
computeUp can be called before the beam is laid out, so we need to do that if necessary
Merged
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.
Resolves: #15470
The idea of this PR is to factor out our fancy new beam positioning algorithm into its own class, which can be used by both beams and two-note tremolos. This doesn't change anything about beams, but rather radically changes two-note trems' behavior to use the beam layout algorithm.