Move tempo changes if rehearsal mark on the same segment #29775
Conversation
miiizen
force-pushed
the
27078-rehearsalMarkTempoChange
branch
from
08447d6 to
d9de077
Compare
miiizen
force-pushed
the
27078-rehearsalMarkTempoChange
branch
2 times, most recently
from
ce513df to
e85b9f9
Compare
Align over time sig first, then move out of rehearsal mark's way if needed
miiizen
force-pushed
the
27078-rehearsalMarkTempoChange
branch
from
e85b9f9 to
577344d
Compare
| } | ||
|
|
||
| const RehearsalMark* rehearsalMark = toRehearsalMark(segment->findAnnotation(ElementType::REHEARSAL_MARK, track(), track())); | ||
| if (!rehearsalMark || !m_alignRightOfRehearsalMark) { |
There was a problem hiding this comment.
The second condition is unnecessary cause you've already checked it
| RectF thisBbox = ldata()->bbox().translated(pos()); | ||
| RectF rehearsalMarkBbox = rehearsalMark ? rehearsalMark->ldata()->bbox().translated(rehearsalMark->pos()) : RectF(); | ||
|
|
||
| if (muse::RealIsEqualOrLess(rehearsalMarkBbox.bottom(), thisBbox.top())) { |
There was a problem hiding this comment.
You may need to differentiate this check if they are below the staff (which is not as rare as it used to be now as we've got the system-objects-below-bottom-option-thingy)
| padding = toTempoText(itemAfter)->fontMetrics().xHeight(); | ||
| } else if (itemAfter->isGradualTempoChangeSegment()) { | ||
| Text* startText = toGradualTempoChangeSegment(itemAfter)->text(); | ||
| padding = startText ? startText->fontMetrics().xHeight() : item->spatium(); |
There was a problem hiding this comment.
Back when I did the masking work, I found out that fontMetrics().xHeight() is an insanely expensive function (almost the entire time taken to compute the whole masking was due to xHeight() alone). Here it will be less of a problem cause this is not a function that will be called thousands of times. But still, if it's not absolutely essential to rely on the exact font metrics, I think we better avoid it. Here we just need the padding value to scale with the font size, like we did with masking, and it's not really necessary to know the exact x-height for that, so you could reuse the (quite unelegant, but very fast) solution I did in MaskingLayout::maskBarlineForText (see fontSizeScaleFactor and collisionPadding).
Resolves: #27078
This PR moves tempo change lines to the right of rehearsal marks which start on the same segment. It also ensures lines which end on the same segment as rehearsal marks end before the rehearsal mark. Finally, tempo change lines are extended up to the next snapped tempo change line, as they were previously with snapped tempo text.