Fix point label placement to stick near the point object #3511
Conversation
lhiginbotham
commented
Oct 23, 2022
lhiginbotham
commented
- fixes Name label for point object is not aligned properly #3400 which notes that the labels for points are attached to a world coordinate, rather than a screenspace coordinate
- the issue came about when points were changed from having a world-fixed size to now having a screen-fixed size, such that the annotation remains the same size when changing the zoom level of the map editor
- one minor issue with this fix is that the painterScale used seems to be one "frame" behind whenever we are re-rendering due to a zoom level change; this corrects itself on the next non-cached repaint (for example, by panning the map editor)
- I have a prototype of a different fix where this one-frame lag doesn't occur but a different issue occurs; the bounding box of the label does not match where it is actually rendered and thus Qt graphics does not allow it to repaint after something was drawn over top of it
|
This fixes the issue such that the label tracks the Point object annotation better with zoom level changes. Do note the issue that I mentioned about the scale factor being "one frame behind" during zoom-triggered repaints. Video of that: label.webmMaybe this fix is good enough for now since it vastly improves the previous case, especially if we do not have any ideas on how to fix the one frame lag. |
src/tiled/objectselectionitem.cpp
Outdated
| bounds = rotateAt(screenPos, mObject->rotation()).mapRect(bounds); | ||
| if (mObject->shape() == MapObject::Point) { | ||
| // Use screenspace offset, since the Point annotation rescales with the zoom-level | ||
| setPos(screenPos - QPointF{ 0, bounds.height() / renderer.painterScale()}); |
There was a problem hiding this comment.
The problem is that the position set with setPos will be scaled with the view, even if this item will not. Previously this is what we wanted, but now that we don't, we should not use setPos to center the label above the object but rather apply an offset when rendering the label (and to the bounding rect of this item). Of course, this should still only be done for point objects, since for all other objects we do want the position of the label to be affected by the scaling.
In case of the point object, we still need to use setPos to set the item at the screenPos of the label object, and the rest of the position adjustment should happen within this item. Then we don't need to worry about the painter scale here, and we will not have a "frame delay".
There was a problem hiding this comment.
@bjorn how does this look now? In my other branch I used a similar approach but didn't realize that the repaint was misbehaving because the bounding box was not also translated.
lhiginbotham
force-pushed
the
lhiginbotham/fix-point-label-placement
branch
from
8de5de8
to
168d899
Compare
lhiginbotham
force-pushed
the
lhiginbotham/fix-point-label-placement
branch
2 times, most recently
from
2bae3a0
to
7da0dd7
Compare
src/tiled/objectselectionitem.cpp
Outdated
| QPointF pos((bounds.left() + bounds.right()) / 2, bounds.top()); | ||
|
|
||
| if (auto mapScene = static_cast<MapScene*>(scene())) | ||
| pos += mapScene->absolutePositionForLayer(*mObject->objectGroup()); |
There was a problem hiding this comment.
This call is here to take into account layer offsets as well as the parallax offset. It should be taken into account also for point objects.
There was a problem hiding this comment.
@bjorn ah okay, don't know how I missed this. I tried it out with and without the absolutePositionForLayer added, it is working better now after your suggestion. Thanks!
- fixes mapeditor#3400 which notes that the labels for points are attached to a world coordinate, rather than a screenspace coordinate - the issue came about when points were changed from having a world-fixed size to now having a screen-fixed size, such that the annotation remains the same size when changing the zoom level of the map editor
lhiginbotham
force-pushed
the
lhiginbotham/fix-point-label-placement
branch
from
7da0dd7
to
37d6f9c
Compare
* Share common code between points and other objects. * Offset the bounding rect and the text position instead of using an additional screenspace offset member.
bjorn
force-pushed
the
lhiginbotham/fix-point-label-placement
branch
from
5f8c077
to
fd291ff
Compare
