It seems that the bounding box for the glissando does not include the text, therefore part of the text falls outside this box (which is the rectangle being updated during dragging).
This bug can be probably solved by overriding function `QRectF& bbox()` for a glissando element so that the bbox becomes the union between the bbox of the line and the bbox of the text, if text is present. I don't know if this proposed change would affect the positioning of glissandi (collisions?).
Layout of gliss should not depend on its bbox - it's more the other way around. We don't do automatic collision avoid for other elements anyhow, but if we did, a more accurate bbox would only be a good thing.
I don't know that we need to override Glissando::bbox() to solve this, though - the bbox() is calcuated during Glissando::layout(), and it should probably just account for the text there. Although I don't see that the text is actually dealt with at all there, so maybe easier said than done?
The function bbox() of class element is virtual and by default returns _bbox. This is the function called by canvasBoundingRect(), which is called by dragging actions.
The text is drawn in lines 217-229 of libmscore\glissando.cpp: it is added horizontally and then painted with rotation angle `wi`.
Glissando's bbox is set at the end of `Glissando::layout()` (and possibly modified in `Glissando::setSize`), but I don't think that glissando's bbox is also properly taking into account for example the fact that wavy glissando uses a wiggled line slightly exceeding the bounding box of the linear glissando. I think we can use the size calculations at lines 210-229 and a little trigonometry to compute the exact bbox inside an overriding `bbox()`. In my spare time I am trying to implement it, but it is taking me a little time to understand the coordinate system used (origin coordinate of the symbol, the fact that y coordinate is negative, et cetera). I think that hopefully I can soon push a PR.
BTW, while bbox() is indeed a virtual function, you'll note it is almost never actually overridden. Elements are normally expected to calculate their _bbox members correctly in their layout() functions, so bbox() can simply return _bbox. Almost all other element types do this. See in particular TextLineSegment::layout() (the real work is done in the layout1() call), and how it tries to account for the text. It's not pretty because there a nbunch of cases to consider, but that's more or less the model I think glissando needs to follow. The only place I see an override for bbox() is for SLine, the parent class for all lines that consist of multiple segments that can be continued from one system to the next. This seems to be only to provide a special case for use in the palette.
And FWIW, I wouldn't worry about being *too* exact. After all, a rectanlgular box is a poor fit for the glissando symbol in any case - it's going to be way too big for its own good anyhow. The main things the bbox seems to be used for as far as I know are 1) redrawing the screen during drags and 2) figuring out which element you are trying to select (selecting is basically clicking within the bbox of an element). In either case, erring on the side of too big is OK for these purposes.
Text, I think, might be special. I think it uses the "tight" version of the bbox in alignment calculations. But I've never really understood how that works. Perhaps that has something to do with what is going on with the "f" in the dynamics, though.
I'm almost there. I added the bbox for the rotated text to the already existing bbox (relative to the straight line alone). See the pictures, in which the colored dots are the bounding rectangle of the text (and the magenta one is the origin of the text), while the red rectangle is the bbox of the line + the one of the text.
As you can see, the wavy line can exceed its bounding box. However, under Windows 8.1, I see the symbols generating the wavy line not completely overlapping (also in Nightly builds), see the picture. I will report this in a separate issue.
I tried to rewrite the bbox function for the glissando so that it encloses the whole line and the text.
Here is the change: https://github.com/AntonioBL/MuseScore/commit/e20bd7beba79
[ a little of trigonometry to rotate the corners of the rectangles :-) and then translate them ]
It works, but I am wondering if the calculations should instead be performed inside function Glissando::layout().
Any opinion?
Comments
I thought I had reported this one as an issue some time ago, but I only posted it as a bug report.
http://musescore.org/en/node/9614
I didn't know about that one (though I think remember the post).
Also affects tremolo. Don't know if there's anything else.
Using MuseScore 2.0 Nightly Build (5290) - Mac 10.6.8.
See this: #36251: Trace on the score page when dragging glissando
For me, only the Gliss icon with text leaves trails now.
Also, see #38221: Drag trail with dynamics starting with "f"
It seems that the bounding box for the glissando does not include the text, therefore part of the text falls outside this box (which is the rectangle being updated during dragging).
This bug can be probably solved by overriding function `QRectF& bbox()` for a glissando element so that the bbox becomes the union between the bbox of the line and the bbox of the text, if text is present. I don't know if this proposed change would affect the positioning of glissandi (collisions?).
Layout of gliss should not depend on its bbox - it's more the other way around. We don't do automatic collision avoid for other elements anyhow, but if we did, a more accurate bbox would only be a good thing.
I don't know that we need to override Glissando::bbox() to solve this, though - the bbox() is calcuated during Glissando::layout(), and it should probably just account for the text there. Although I don't see that the text is actually dealt with at all there, so maybe easier said than done?
The function bbox() of class element is virtual and by default returns _bbox. This is the function called by canvasBoundingRect(), which is called by dragging actions.
The text is drawn in lines 217-229 of libmscore\glissando.cpp: it is added horizontally and then painted with rotation angle `wi`.
Glissando's bbox is set at the end of `Glissando::layout()` (and possibly modified in `Glissando::setSize`), but I don't think that glissando's bbox is also properly taking into account for example the fact that wavy glissando uses a wiggled line slightly exceeding the bounding box of the linear glissando. I think we can use the size calculations at lines 210-229 and a little trigonometry to compute the exact bbox inside an overriding `bbox()`. In my spare time I am trying to implement it, but it is taking me a little time to understand the coordinate system used (origin coordinate of the symbol, the fact that y coordinate is negative, et cetera). I think that hopefully I can soon push a PR.
Have fun! :-).
BTW, while bbox() is indeed a virtual function, you'll note it is almost never actually overridden. Elements are normally expected to calculate their _bbox members correctly in their layout() functions, so bbox() can simply return _bbox. Almost all other element types do this. See in particular TextLineSegment::layout() (the real work is done in the layout1() call), and how it tries to account for the text. It's not pretty because there a nbunch of cases to consider, but that's more or less the model I think glissando needs to follow. The only place I see an override for bbox() is for SLine, the parent class for all lines that consist of multiple segments that can be continued from one system to the next. This seems to be only to provide a special case for use in the palette.
And FWIW, I wouldn't worry about being *too* exact. After all, a rectanlgular box is a poor fit for the glissando symbol in any case - it's going to be way too big for its own good anyhow. The main things the bbox seems to be used for as far as I know are 1) redrawing the screen during drags and 2) figuring out which element you are trying to select (selecting is basically clicking within the bbox of an element). In either case, erring on the side of too big is OK for these purposes.
Text, I think, might be special. I think it uses the "tight" version of the bbox in alignment calculations. But I've never really understood how that works. Perhaps that has something to do with what is going on with the "f" in the dynamics, though.
I'm almost there. I added the bbox for the rotated text to the already existing bbox (relative to the straight line alone). See the pictures, in which the colored dots are the bounding rectangle of the text (and the magenta one is the origin of the text), while the red rectangle is the bbox of the line + the one of the text.
As you can see, the wavy line can exceed its bounding box. However, under Windows 8.1, I see the symbols generating the wavy line not completely overlapping (also in Nightly builds), see the picture. I will report this in a separate issue.
I tried to rewrite the bbox function for the glissando so that it encloses the whole line and the text.
Here is the change:
https://github.com/AntonioBL/MuseScore/commit/e20bd7beba79
[ a little of trigonometry to rotate the corners of the rectangles :-) and then translate them ]
It works, but I am wondering if the calculations should instead be performed inside function Glissando::layout().
Any opinion?
I guess you have seen miwarre's https://github.com/musescore/MuseScore/pull/1536 ?
Glissando looks good now. Also tremolo between two notes. I will take a look to tremolo with a single note.
Fixed in branch master, commit 0c825c8ff7
fix #13879: Dragging glissando and tremolo from palette produces trail effect
Fixed in branch 2.0.2, commit ba1cac459c
fix #13879: Dragging glissando and tremolo from palette produces trail effect
Automatically closed -- issue fixed for 2 weeks with no activity.