Fix #10441 - Omit lyrics extender line if too short #10471
Conversation
| @@ -416,8 +416,14 @@ void LyricsLineSegment::layout() | |||
| } | |||
|
|
|||
| // MELISMA vs. DASHES | |||
| static double minMelismaLen = 1 * sp; // TODO: style setting | |||
There was a problem hiding this comment.
I think it shouldn't be static since sp isn't static either. But it could be const.
There was a problem hiding this comment.
Yeah, good point, it should be const. Though I think it can still be static: in this context it means that it is a property of the class, so it doesn't need to be reinstantiated by every lyricsLine object. Even if sp isn't static
There was a problem hiding this comment.
I still think it can't be static. sp may be different every time this method is called, so minMelismaLen must also be initialized/updated every time. Now, it will be initialized the first time this method is called, but after that, the value will never change anymore.
There was a problem hiding this comment.
Mmh, you're right. I was assuming that sp is always the same within a given score, but indeed it can change. Then the variable should just be a const double
mike-spa
force-pushed
the
lyricsExtenderMinimumLength
branch
from
e0570e1
to
afc10e5
Compare
|
I wonder why no vtest fails, seems to indicate no such too short melisma is part of the tests, but maybe it better should? |
Yeah, I guess it's a relatively rare occurrence, but it may be worth having it in the tests. In general, is there a handbook or something on how to create new tests? That's something I'd like to learn. |
|
Check https://github.com/musescore/MuseScore/blob/master/vtest/README.md#add-new-test and vtest/scores/lyrics-[1-9].mscx |
Comment correct variable definition
mike-spa
force-pushed
the
lyricsExtenderMinimumLength
branch
from
afc10e5
to
1d24885
Compare
Comment
Resolves: 10441
As discussed in the related issue, the extender line for lyrics should be omitted when too short, as it serves no purpose. See the screen capture attached for a quick look. The threshold value is now hardcoded to 1 sp as suggested by @oktophonie. In future, it probably deserves a dedicated style setting.
Screencast.2022-02-06.18.43.43.mp4