[MU4] Fix staccato and tenuto appearing below notes when they shouldn't #10665
Conversation
(beam-side articulations)
|
Added a bit of logic for ignoring manual adjustments to stem length, so things like this are possible: (this was fixed in #4711 -- I don't know why this broke somewhere along the way but this PR returns to the functionality 4711 added) |
|
Are these vtests differences wanted? Before: |
|
But what about this? Before: |
|
Yes, these are all exactly as expected. Here's this test in 3.6:
The articulations have their anchor set differently from bar to bar, which is the reason for the differences. |
|
Yeah, the differences are due to the anchor point not being respected, which this PR fixes |
|
OK, so it seems 3.x is not affected by th at all is seems, Even if I backport this PR to 3.x I can't spot any difference, not in those 3 vtests at least. |
|
Yes, this was a regression introduced in 4.0 development (likely connected with the new beam code somewhere). |
|
I see, thanks |
src/engraving/libmscore/chord.cpp
Outdated
| if (_up) { | ||
| y = downPos() - stem()->length() - userLen; | ||
| if (beam()) { | ||
| y -= score()->styleS(Sid::beamWidth).val() * _spatium * .5; |
There was a problem hiding this comment.
This line
score()->styleS(Sid::beamWidth).val() * _spatium * .5;
is present in 4 places. It is better to add a temporary variable for it and reuse the variable 4 times
asattely
force-pushed
the
vertical-displace-articulation
branch
from
6a28552 to
65c683e
Compare
src/engraving/libmscore/chord.cpp
Outdated
| @@ -3746,18 +3746,19 @@ void Chord::layoutArticulations() | |||
| bool headSide = bottom == up(); | |||
| qreal x = centerX(); | |||
| qreal y = 0.0; | |||
|
|
|||
| const qreal halfBeamSp = score()->styleS(Sid::beamWidth).val() * _spatium() * 0.5; | |||
There was a problem hiding this comment.
_spatium() should rather be _spatium
| if (beam()) { | ||
| y -= score()->styleS(Sid::beamWidth).val() * _spatium * .5; | ||
| y -= halfBeamSp * beam()->mag(); |
There was a problem hiding this comment.
Doesn't _spatium (as part ot halfBeamSp) already contain mag? How is beam->mag() different from mag?
If really needed, should it not get added just once up where halfBeamSpis defined?
There was a problem hiding this comment.
beam->mag() can be different from mag (which is based on the magnitude of the chord) in situations like this
... as far as I can tell. So instead of using _spatium for that one I'll use score()->spatium() so that the chord magnitude isn't included in the calculation
There was a problem hiding this comment.
OK, still my last question: why not doing this only once?
There was a problem hiding this comment.
i guess we could, but beam() is not guaranteed to be non-null until the check, so we'd need to add another check where the definition is. I don't see it as too big of a deal though
There was a problem hiding this comment.
const qreal halfBeamSp = score()->styleS(Sid::beamWidth).val() * score()->spatium() * 0.5 * (beam() ? beam()-> mag() : 1) ;
asattely
force-pushed
the
vertical-displace-articulation
branch
from
65c683e to
11701a2
Compare
Before:
After:
There was just some non-direction-agnostic code in the laying out of certain articulations, so that's been fixed.