godot
godot copied to clipboard
Inconsistent behavior when rotating control nodes
Tested versions
Reproduciable in 4.3 stable, not reproduciable in 4.2.2 stable
System information
Windows 10
Issue description
Rotating a control node doesn't rotate on the pivot offset as expected if you change the node's position while rotating
Steps to reproduce
- Open the attached project in godot 4.3 and run. click anywhere to pick up the icon and right click to rotate it. you will see it does not rotate on the pivot if you try to rotate while the icon is picked up. However, it does rotate correctly if you right click without dragging
- Open the attached project in godot 4.2.2 and do the same, you will see it does rotate on the pivot as expected, in both cases.
Minimal reproduction project (MRP)
Can't fully confirm but might be related to:
- https://github.com/godotengine/godot/issues/87354
- https://github.com/godotengine/godot/issues/89497
Could be a consequence of either fix, unsure if 4.2.2 or 4.3 behavior is intended (i.e. if this is a bug, or if you were relying on bugged behavior in 4.2.2 that was fixed)
CC @kleonc @Rindbee @Sauermann
Probably because Control::set_rotation_degrees() doesn't take pivot_offset into account.
https://github.com/godotengine/godot/blob/da5f39889f155658cef7f7ec3cc1abb94e17d815/scene/gui/control.cpp#L1552-L1566
It might be better to provide a dedicated rotate method.
In 4.2.2 (and earlier) there is a discrepancy between what global position "means" in context of Control.set_global_position and Control.get_global_position.
| unscaled, unrotated | pivot at center, scaled, rotated |
|---|---|
For a TextureRect like above:
Control.get_global_positionwould return the yellow point (equivalent toglobal_transform.origin).Control.set_global_positionwould expect you to pass the red point, aka the global position of an unrotated, unscaled rect (according to the pivot).
Because of the above, doing global_position = global_position in GDScript was resulting in moving the given Control scaled/rotated around non-default pivot, this was reported in #87354. I think we all agree that's a buggy unexpected behavior.
| original | after global_position = global_positionv4.2.2.stable.official[15073afe3] |
|---|---|
global_position = global_positionis equivalent toset_global_position(get_global_position())get_global_position()returnsoriginal_yellowset_global_position(original_yellow)moves control so red is atoriginal_yellow
The above discrepancy / buggy behavior was ultimately fixed in 4.3, by #89502 (#87432 was incorrect and caused #89497). It made both Control.set_global_position and Control.get_global_position return/expect the "yellow points" (aka global_transform.origin).
Hence this is expected that MRP which is conceptually doing global_position = "red point" is working in 4.2.2, but not in 4.3.
In 4.3 a "yellow point" would need to be passed instead.
After changing main.gd in the MRP to:
extends Control
@onready var rotating: TextureButton = $Rotating
var is_dragging = false
func _process(delta: float) -> void:
if Input.is_action_just_pressed("click"):
is_dragging = !is_dragging
if Input.is_action_just_pressed("right_click"):
rotating.rotate()
if is_dragging:
# (1)
#var half_size: Vector2 = rotating.size / 2
#rotating.global_position = get_global_mouse_position() - half_size
# (2)
#var half_size_global: Vector2 = rotating.get_global_transform().basis_xform(rotating.size / 2)
#rotating.global_position = get_global_mouse_position() - half_size_global
# (3)
#var half_size_parent_global: Vector2 = (rotating.get_global_transform() * rotating.get_transform().affine_inverse()).basis_xform(rotating.size / 2)
#rotating.global_position = get_global_mouse_position() - half_size_parent_global
%RectLabel.text = "Rect: %s" % str(rotating.get_global_rect())
Here's a visualization of the behavior after uncommenting section (1) or (2):
(black shows what's assigned to rotating.global_position)
| uncommented section | v4.2.2.stable.official [15073afe3] | v4.3.stable.official [77dcf97d8] |
|---|---|---|
| (1) | ||
| (2) |
But note that even though the code from (1) seemingly works in 4.2.2, it's not guaranteed to always work. For simple case sure it does. But in general to properly obtain the unrotated/unscaled position something like (3) would need to be done instead.
Here's for example what happens after rotating and scaling the parent Main Control in the MRP, notice that for (1) mouse is no longer at the center (which is the pivot):
| uncommented section | v4.2.2.stable.official [15073afe3] |
|---|---|
| (1) | |
| (3) |
Thus it's not like doing things properly was simpler in 4.2.2.
To sum up, in 4.3 the behavior of Control.set_global_position/Control.get_global_position is more consistent, global_position = global_position doesn't move the Control.
However, note that it could have been potentially fixed the other way around. Instead of changing Control.set_global_position to expect a "yellow position" (done in #89502), an alternative would be to change Control.get_global_position to return the "red position". Whether one was preferable over the other I'm not sure.
If Control.set_global_position/Control.get_global_position used "yellow points" (current behavior in v4.3.stable.official [77dcf97d8]), then:
global_positionis equivalent toglobal_transform.origin. This is consistent withNode2DandNode3D.global_positionis not equivalent toparent_global_transform * position. This is inconsistent withNode2DandNode3D.
If Control.set_global_position/Control.get_global_position used "red points" (alternative to current behavior), then:
global_positionis not equivalent toglobal_transform.origin. This is not consistent withNode2DandNode3D.global_positionis equivalent toparent_global_transform * position. This is consistent withNode2DandNode3D.
In the end we can't make everything consistent with Node2D/Node3D because of how for Control there's an additional pivot "between" its parent and local coordinate spaces. So, again, not sure if one is preferable over the other.
Regardless of whether we will leave the behavior as in v4.3.stable.official [77dcf97d8], or change it to the alternative (after concluding it's better for some reason), we could of course potentially consider adding some helper methods/properties to Control (or just documenting better? :thinking:). So suggestions/proposals about what could be improved are of course welcomed (implementation shouldn't be a problem). :slightly_smiling_face:
When the pivot is not at the default position (top left corner, i.e. position), rotating around the pivot should change the entire transform (correcting position/global_position).
This may be more mathematical, but the values may not be very intuitive.
relevant in version v4.4.dev2.official [97ef3c837]. When moving a node, its pivot_offset position starts to be read as default, although I have it set to the center of the node. The problem is not detected if you change the local position, not the global one
Note I've opened #101719 which changes the behavior to the alternative mentioned in https://github.com/godotengine/godot/issues/95798#issuecomment-2297502534.