Control node 'resized' event fires twice for every resize in CanvasItemEditor

ZweiEuro commented 2 months ago

Tested versions

Reproducable in: 0bf12956a10d216c46368f26b00f6999d000c1d8 , which is 4.2.1-rc

System information

Archlinux

Issue description

On a control node, the resized event fires twice on every resize

Steps to reproduce

New Project
New 2D Scene
Add Control Node
Attach Script
connect signal resized
Write this into the script:
```
@tool
extends Control
```

var calls : int = 0

func _on_resized(): calls = calls + 1 print(calls, ": ", size)


7. (restart godot, because for me i kept getting ` core/object/object.cpp:1140 - Error calling from signal 'resized' to callable: 'Control(Control.gd)::_on_resized': Method not found.`, a restart fixes this; when using the MRP this won't matter)

8. drag one corner around in any way

You will see an output like:

1: (332, 213) 2: (332, 212) 3: (332, 214) 4: (332, 212) 5: (333, 216) 6: (332, 212) 7: (334, 217) 8: (332, 212) 9: (336, 219) 10: (332, 212) 11: (339, 222) 12: (332, 212) 13: (344, 225) 14: (332, 212) 15: (352, 233) 16: (332, 212) 17: (368, 245) 18: (332, 212) 19: (387, 255) 20: (332, 212) 21: (404, 263) 22: (332, 212) 23: (418, 271) 24: (332, 212) 25: (430, 277)


where every 2nd time the function gets called, its the same, original size.

#### this _doesn't_ happen if its done via code, only when dragged in the editor

@tool extends Control

func _process(delta): size.x += 1

func _on_resized(): print(size)



### Minimal reproduction project (MRP)

[godot.zip](https://github.com/user-attachments/files/15686057/godot.zip)

nenad-jalsovec commented 2 months ago

I confirm this behavior in v4.2.2.stable.official [15073afe3]

AptetutBen commented 2 months ago

I also confirm this is happening in v4.2.2.stable official. With mine I am adding nodes to a Margin node in code and waiting for the signal for the resize,

enetheru commented 1 month ago

This annoys the crap out of me so I decided to do some digging to isolate the problem case. Ended up having to create a simple gdextion so i can get a call stack when resize is called with the original size.

Expected behaviour is when the resize call gives the new size: ... Control::_size_changed control.cpp:1752 Control::set_size control.cpp:1479 Control::_edit_set_rect control.cpp:145 CanvasItemEditor::_gui_input_resize canvas_item_editor_plugin.cpp:1869 CanvasItemEditor::_gui_input_viewport canvas_item_editor_plugin.cpp:2651 ...

Unexpected behaviour is when the resize is called with the original values: ... Control::_size_changed control.cpp:1752 Control::_edit_set_state control.cpp:122 CanvasItemEditor::_gui_input_resize canvas_item_editor_plugin.cpp:1793 CanvasItemEditor::_gui_input_viewport canvas_item_editor_plugin.cpp:2651 ...

There is another case of seeing a resize with unexpected values and that is when the control layout is set to anchor points and any of the wide or full presets, it also triggers a resize event when moved in those circumstances.

I'm still looking at it, but thats what I have so far.

enetheru commented 1 month ago

I'm guessing here, but I think it is undoing changes, and then re-doing them with the new drag state. which is why the original values interleave the new ones.

There is a bunch of state in the canvas item editor already, it would probably be easiest to keep more so that the undo isn't necessary. There appears only to be a rect, a vector and a transform that it gets from the original.

I dunno though not very familiar with internals.

akien-mga commented 1 month ago

Seems reproducible in latest master.

CC @KoBeWi

KoBeWi commented 1 month ago

The state when resizing is calculated based on initial state, not current state, hence it's necessary to reset it. I managed to fix it for Control, but unfortunately the same code does not work for e.g. Sprite2D:

https://github.com/user-attachments/assets/bfaae3b2-a9dc-4b4d-a804-7c77210bf2e7

Though why is double resized signal even a problem?

enetheru commented 1 month ago

Though why is double resized signal even a problem?

My context is that I'm creating addons and tool scripts that derive from control and friends. For me at least:

it's unexpected, and led to multiple days of unnecessary work trying to figure out if I'd done something wrong.
It halves performance when relying on signals to re-calculate interior things.
I find that there are multiple additional reasons the state reset happens in the middle of an action, which makes working around the duplicate with 'incorrect data' signals non trivial to me.

How did you implement your fix? I was going to have a go at it, capturing the initial state in the DRAG_NONE portion of the code, and using that instead of resetting each drag event.

KoBeWi commented 1 month ago

Here's a diff of my fix:

fix.txt

enetheru commented 1 month ago

The "fix" for this appears to require some interesting changes than I can't afford.

godotengine / godot