Control.PivotOffset assignment is destructive with existing Rotation or Scale

[esainane]

Tested versions

4.3.1-rc [ff9bc04], custom build (scons module_mono_enabled=yes debug_symbols=yes ccflags="-fvar-tracking -fvar-tracking-assignments")

System information

Godot v4.3.1.rc.mono (ff9bc04) - Debian GNU/Linux trixie/sid trixie - X11 - Vulkan (Mobile) - dedicated NVIDIA GeForce RTX 4060 Laptop GPU - 13th Gen Intel(R) Core(TM) i7-13650HX (20 Threads)

Issue description

Assigning to a Control.PivotOffset will change the Control's transform immediately if a non-default Scale or Rotation is in effect, rather than updating the other properties to keep the Transform constant while PivotOffset changes.

The small attached project lets one interactively compare what currently happens, versus what I expected to happen.

Controls:

• Scrollwheel scales the Godot logo
• Shift+Scrollwheel rotates the Godot logo
• Drag pans it around

Uncheck TestingContainer's "Use workaround" property in the Editor to see the current behavior.

This is the implementation of the workaround for what I believe is more developer-friendly behaviour, which just updates the Control's position to preserve the current transform, in a way I tried to make forwards-compatible:

    private static void ChangePivotOffset(Control control, Vector2 newPivot) {
        /*
         * Scale and Rotation will correctly apply themselves relative to the
         * pivot offset, so they can be freely set without altering other
         * properties.
         *
         * If there is no Scale or Rotation set, then PivotOffset can be set
         * non-destructively; the net transform will not change, and the
         * Control will not move.
         *
         * However, if Scale or Rotation is set, then changing the PivotOffset
         * will move the Control, as Scale, Rotation, and Position values are
         * kept constant, and Scale and Rotation are applied relative to the
         * PivotOffset, leading to different results.
         *
         * This function attempts to change the PivotOffset without moving the
         * Control, or otherwise causing any change in the overall Transform,
         * by adjusting the Position of the Control to compensate for the
         * change in the PivotOffset.
         */
        GD.Print("Control position before:", control.Position);
        Vector2 originPre = control.GetTransform().Origin;
        control.PivotOffset = newPivot;
        Vector2 originPost = control.GetTransform().Origin;
        GD.Print("Origin change:", originPre, " -> ", originPost, " net: ", originPost - originPre);
        GD.Print("Control position after:", control.Position);
        control.Position -= originPost - originPre;
        GD.Print("Control position after correction:", control.Position);
    }

I'm happy to implement this in C++, if someone can show me where to look. There's currently a Control::_edit_set_pivot method which does some adjustment to Position and might be equivalent, but a breakpoint on it never seems to be hit, only Control::set_pivot_offset. I could also try implementing this as a new Control method, if the existing behaviour is likely to be relied on?

Possibly related: #52383

Steps to reproduce

• Create a Control, and assign a non-default Scale and/or Rotation
• Assign a different value to PivotOffset
• Position, Scale, and Rotation are unchanged, but given that Scale and Rotation take effect from the new PivotOffset, the Control's transform has changed.

Minimal reproduction project (MRP)

test-pivot.zip

[kleonc]

I consider this correct behavior.

pivot_offset is the raw value used for calculating/composing the given Control's transform, the same as position, rotation, scale are. Changing any of these raw values should not have side effects of changing any other of these raw values. The resulting transform should be the same regardless of the order of assigning values to position/rotation/scale/pivot_offset.

The transform is calculated like:

T // translation
R // rotation
S // scale

Control.get_transform() = T(position) * T(pivot_offset) * R(rotation) * S(scale) * T(-pivot_offset)

---

Assigning to a Control.PivotOffset will change the Control's transform immediately if a non-default Scale or Rotation is in effect, rather than updating the other properties to keep the Transform constant while PivotOffset changes.

Well, changing position, rotation or scale also changes the Control's transform immediately. Why do you expect the pivot_offset to be different in that aspect and not affect the transform?

And yes, in case R(rotation) * S(scale) is an identity transformation (e.g. rotation = 0; scale = (1, 1) or rotation = PI; scale = (-1, -1)) then pivot_offset has no effect on the transform indeed as then it's a pure translation by position:

I // identity

If:
R(rotation) * S(scale) = I

Then:
Control.get_transform() = T(position) * T(pivot_offset) * R(rotation) * S(scale) * T(-pivot_offset)
                        = T(position) * T(pivot_offset) * I                      * T(-pivot_offset)
                        = T(position) * T(pivot_offset)                          * T(-pivot_offset)
                        = T(position) * I
                        = T(position)

---

It is already documented for position that it is not affected by pivot_offset. But seems like pivot_offset's docs could be clarified as well:

godot/doc/classes/Control.xml

Lines 1021 to 1026 in 3877573

	<member name="pivot_offset" type="Vector2" setter="set_pivot_offset" getter="get_pivot_offset" default="Vector2(0, 0)">
	By default, the node's pivot is its top-left corner. When you change its [member rotation] or [member scale], it will rotate or scale around this pivot. Set this property to [member size] / 2 to pivot around the Control's center.
	</member>
	<member name="position" type="Vector2" setter="_set_position" getter="get_position" default="Vector2(0, 0)">
	The node's position, relative to its containing node. It corresponds to the rectangle's top-left corner. The property is not affected by [member pivot_offset].
	</member>

---

If a new method like set_pivot_preserving_transform tweaking the position accordingly is desired, then a proposal for this should be opened (possibly with a PR implementing it).