Make lyric lines selectable and editable #31429
Conversation
miiizen
force-pushed
the
31398-editLyricLines
branch
from
fcfa166 to
ec2563e
Compare
mike-spa
left a comment
mike-spa
left a comment
There was a problem hiding this comment.
Really great job, only minor comments
| // This trill is on a cross-staff chord. Don't try to rebase its anchors when dragging. | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
Good one, thanks for the tidy up
| } | ||
| } | ||
|
|
||
| undoChangeProperty(Pid::GENERATED, false); |
There was a problem hiding this comment.
This feels like a slightly weird place to do this, because:
- I think it's already handled somewhere at a lower level, EngravingObject::undoChangeProperty if I'm not mistaken. In other words any time any property is edited the element is also automatically un-flagged as generated, so I'm not 100% sure this is needed.
- If it only applies to lyrics lines should it perhaps go there, instead?
| if (el->isLyricsLine()) { | ||
| LyricsLine* separator = toLyricsLine(el); | ||
| m_separator = separator; | ||
| score()->addUnmanagedSpanner(separator); |
There was a problem hiding this comment.
Have you actually understood what "unmanaged spanners" are? Cause I never had the time to properly look into it so I really never did 😅
There was a problem hiding this comment.
No not really! We could probably remove this distinction at this point.
| void mu::engraving::PartialLyricsLine::setIsEndMelisma(bool val) | ||
| { | ||
| m_isEndMelisma = val; | ||
| styleChanged(); |
There was a problem hiding this comment.
Why styleChanged()? It looks a bit unrelated. If it's just about resetting some properties I'd consider doing that directly
| initElementStyle(&lyricsLineElementStyle); | ||
| setAnchor(Spanner::Anchor::SEGMENT); | ||
| m_nextLyrics = 0; | ||
| setGenerated(true); // no need to save it, as it can be re-generated |
There was a problem hiding this comment.
GENERATED is also a flag I believe, so you could add the flag to the constructor's arguments instead of setting it here
There was a problem hiding this comment.
Generated needs to be set to true after initElementStyle as any call to undoChangeProperty sets generated to false. The base class constructor where flags are set is called first.
| return result; | ||
| } | ||
|
|
||
| Lyrics* searchNextLyrics(Segment* s, staff_idx_t staffIdx, int verse, PlacementV p) |
There was a problem hiding this comment.
I am not a big fan of this utils class, as it ends up containing a miscellaneous of things that are completely unrelated to each other, whereas I've always felt that most of these utiliy functions could just be in their related DOM class, perhaps as static functions. But let's rethink about that another time, I've no problem with this staying here for now.
2 participants
Resolves: #31398
This PR makes lyrics lines selectable in the score. Their position can be changed via offset in properties or by dragging. They can be made short or longer without reanchoring the start/end points.
Thickness is also now configurable.
Screen.Recording.2025-12-11.at.16.48.08.mov