fix #67031: incomplete glissando copied #2101
Conversation
| } | ||
| // glissando with no end element can happen during copy/paste | ||
| for (Spanner* spanner : n->spannerFor()) { | ||
| if (spanner->type() == Element::Type::GLISSANDO && spanner->endElement() == nullptr) { |
There was a problem hiding this comment.
It's probably worth removing any spanner with no end element here. Glissando or not
|
@MarcSabatella : Well, I cannot think of any 'hidden pitfall' in this change. About the opposite case, when a glissando end note is copied and the initial note is not, I would expect the code for the old glissando format (lines 2672 and foll.) to trigger in and connect the end note to a 'guessed' initial note. No idea if this really happens and, in case, if it is good or bad. If this is not good and the chosen solution is to remove the glissando entirely, it is necessary to also clear the |
|
For whatever reason, it doesn't seems other spanners get into this situation. Also, originally I was thinking the place to clean up glissandi would be checkSpanner(), since that's where other spanner cleanups are performed. But glissandi are not part of the spanner map and are not normally dealt with in checkSpanner, it seemed easier to handle them here. I'd be kind of nervous about messing with the behavior of other spanners, as there were a lot of problems with copy/paste of spanners last year where fixing one bug created another, and I'd hate to break anything. Plus, we (or at least I) want to support one-side ties. But I can make this change if people think it is safe. @wschweer ? |
MarcSabatella
force-pushed
the
67031-copy-incomplete-glissando
branch
from
a9ec8e7
to
423c24a
Compare
|
Updated, now works for textline too. I re-verified copies with start elements work as expected for both element types. To be honest, I have no idea where they get cleaned up for textlines, but no matter, it seems they do. |
…ssando fix #67031: incomplete glissando copied
Copy/paste of an incomplete gliss - eg, if the start note was part of selection but end note was not - left you with an "orphaned" forward gliss that wouldn't render but also wouldn't allow a new gliss to be added. See issue report https://musescore.org/en/node/67031 and original discussion in https://musescore.org/en/node/66881
My change here checks for and removes such one-ended glissandi.
@mgavioli : does this change look good to you? It does seem to work for the case at hand, and ordinary copy/paste also still works. Not sure if there is some other case where it would be harmful to delete a glissando with no end note here. Also, I don't see any bad effects from copy and paste of the end note of a gliss without the start, so I didn't add code to deal with that.