Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calling get_global_rect() inside an item_rect_changed callback behaves inconsistently #92449

Closed
DevJac opened this issue May 27, 2024 · 3 comments · Fixed by #92489
Closed

Calling get_global_rect() inside an item_rect_changed callback behaves inconsistently #92449

DevJac opened this issue May 27, 2024 · 3 comments · Fixed by #92489

Comments

@DevJac
Copy link

DevJac commented May 27, 2024

Tested versions

  • Reproducible in: v4.2.1.stable.fedora [b09f793]

System information

Godot v4.2.1.stable (b09f793) - Fedora Linux 39 (Workstation Edition) - X11 - GLES3 (Compatibility) - NVIDIA GeForce RTX 3060 (nvidia; 550.54.14) - AMD Ryzen 9 5950X 16-Core Processor (32 Threads)

Issue description

Calling get_global_rect() inside a item_rect_changed callback behaves inconsistently. Sometimes it gives the rectangle as it was before the change that triggered the callback, and sometimes it gives the rectangle as it was after the change that triggered the callback.

Steps to reproduce

  1. Create a new project.
  2. Create a user interface scene (with a Control node as root).
  3. Add a ColorRect as a child of the Control.
  4. Attach the following script to the ColorRect.
  5. Connect the item_rect_changed signal to the appropriate function in the script.
  6. Run the project.
  7. While the project is running, move your mouse in and out of the ColorRect.
  8. Observe the output until this bug occurs.

(Also, consider using the script in my next post which throws an error when this bug occurs.)

extends ColorRect


func _ready() -> void:
	var timer := Timer.new()
	timer.wait_time = 1
	timer.autostart = true
	timer.connect(
		'timeout',
		func move():
			var t := Time.get_ticks_msec() / 1000.0
			prints('setting position to: ', Vector2(t, t))
			position = Vector2(t, t)
	)
	add_child(timer)


func _on_item_rect_changed() -> void:
	print('in item_rect_changed callback, get_global_rect() returns: ', get_global_rect())

On my computer, the script output the following:

Godot Engine v4.2.1.stable.fedora.b09f793f5 - https://godotengine.org
OpenGL API 3.3.0 NVIDIA 550.54.14 - Compatibility - Using Device: NVIDIA - NVIDIA GeForce RTX 3060
 
setting position to:  (1.481, 1.481)
in item_rect_changed callback, get_global_rect() returns: [P: (1.481, 1.481), S: (192, 192)]
setting position to:  (2.481, 2.481)
in item_rect_changed callback, get_global_rect() returns: [P: (2.481, 2.481), S: (192, 192)]
setting position to:  (3.482, 3.482)
in item_rect_changed callback, get_global_rect() returns: [P: (3.482, 3.482), S: (192, 192)]
setting position to:  (4.483, 4.483)
in item_rect_changed callback, get_global_rect() returns: [P: (4.483, 4.483), S: (192, 192)]
setting position to:  (5.485, 5.485)
in item_rect_changed callback, get_global_rect() returns: [P: (5.485, 5.485), S: (192, 192)]
setting position to:  (6.468, 6.468)
in item_rect_changed callback, get_global_rect() returns: [P: (6.468, 6.468), S: (192, 192)]
setting position to:  (7.469, 7.469)
in item_rect_changed callback, get_global_rect() returns: [P: (7.469, 7.469), S: (192, 192)]
setting position to:  (8.469, 8.469)
in item_rect_changed callback, get_global_rect() returns: [P: (8.469, 8.469), S: (192, 192)]
setting position to:  (9.47, 9.47)
in item_rect_changed callback, get_global_rect() returns: [P: (8.469, 8.469), S: (192, 192)]
setting position to:  (10.47, 10.47)
in item_rect_changed callback, get_global_rect() returns: [P: (9.47, 9.47), S: (192, 192)]
setting position to:  (11.471, 11.471)
in item_rect_changed callback, get_global_rect() returns: [P: (10.47, 10.47), S: (192, 192)]
setting position to:  (12.471, 12.471)
in item_rect_changed callback, get_global_rect() returns: [P: (11.471, 11.471), S: (192, 192)]
setting position to:  (13.471, 13.471)
in item_rect_changed callback, get_global_rect() returns: [P: (12.471, 12.471), S: (192, 192)]
setting position to:  (14.472, 14.472)
in item_rect_changed callback, get_global_rect() returns: [P: (13.471, 13.471), S: (192, 192)]
setting position to:  (15.472, 15.472)
in item_rect_changed callback, get_global_rect() returns: [P: (14.472, 14.472), S: (192, 192)]
setting position to:  (16.473, 16.473)
in item_rect_changed callback, get_global_rect() returns: [P: (15.472, 15.472), S: (192, 192)]
setting position to:  (17.473, 17.473)
in item_rect_changed callback, get_global_rect() returns: [P: (16.473, 16.473), S: (192, 192)]
setting position to:  (18.474, 18.474)
in item_rect_changed callback, get_global_rect() returns: [P: (17.473, 17.473), S: (192, 192)]
setting position to:  (19.474, 19.474)
in item_rect_changed callback, get_global_rect() returns: [P: (18.474, 18.474), S: (192, 192)]
setting position to:  (20.475, 20.475)
in item_rect_changed callback, get_global_rect() returns: [P: (19.474, 19.474), S: (192, 192)]
setting position to:  (21.475, 21.475)
in item_rect_changed callback, get_global_rect() returns: [P: (20.475, 20.475), S: (192, 192)]
setting position to:  (22.459, 22.459)
in item_rect_changed callback, get_global_rect() returns: [P: (21.475, 21.475), S: (192, 192)]
setting position to:  (23.459, 23.459)
in item_rect_changed callback, get_global_rect() returns: [P: (22.459, 22.459), S: (192, 192)]
setting position to:  (24.46, 24.46)
in item_rect_changed callback, get_global_rect() returns: [P: (23.459, 23.459), S: (192, 192)]
setting position to:  (25.46, 25.46)
in item_rect_changed callback, get_global_rect() returns: [P: (24.46, 24.46), S: (192, 192)]
setting position to:  (26.461, 26.461)
in item_rect_changed callback, get_global_rect() returns: [P: (25.46, 25.46), S: (192, 192)]
setting position to:  (27.461, 27.461)
in item_rect_changed callback, get_global_rect() returns: [P: (26.461, 26.461), S: (192, 192)]
setting position to:  (28.461, 28.461)
in item_rect_changed callback, get_global_rect() returns: [P: (27.461, 27.461), S: (192, 192)]
setting position to:  (29.462, 29.462)
in item_rect_changed callback, get_global_rect() returns: [P: (28.461, 28.461), S: (192, 192)]
setting position to:  (30.462, 30.462)
in item_rect_changed callback, get_global_rect() returns: [P: (29.462, 29.462), S: (192, 192)]
setting position to:  (31.463, 31.463)
in item_rect_changed callback, get_global_rect() returns: [P: (30.462, 30.462), S: (192, 192)]
setting position to:  (32.463, 32.463)
in item_rect_changed callback, get_global_rect() returns: [P: (31.463, 31.463), S: (192, 192)]
setting position to:  (33.464, 33.464)
in item_rect_changed callback, get_global_rect() returns: [P: (32.463, 32.463), S: (192, 192)]
setting position to:  (34.464, 34.464)
in item_rect_changed callback, get_global_rect() returns: [P: (34.464, 34.464), S: (192, 192)]
setting position to:  (35.465, 35.465)
in item_rect_changed callback, get_global_rect() returns: [P: (35.465, 35.465), S: (192, 192)]
setting position to:  (36.465, 36.465)
in item_rect_changed callback, get_global_rect() returns: [P: (36.465, 36.465), S: (192, 192)]
setting position to:  (37.466, 37.466)
in item_rect_changed callback, get_global_rect() returns: [P: (36.465, 36.465), S: (192, 192)]
setting position to:  (38.467, 38.467)
in item_rect_changed callback, get_global_rect() returns: [P: (38.467, 38.467), S: (192, 192)]
setting position to:  (39.483, 39.483)
in item_rect_changed callback, get_global_rect() returns: [P: (38.467, 38.467), S: (192, 192)]
setting position to:  (40.484, 40.484)
in item_rect_changed callback, get_global_rect() returns: [P: (39.483, 39.483), S: (192, 192)]
setting position to:  (41.484, 41.484)
in item_rect_changed callback, get_global_rect() returns: [P: (41.484, 41.484), S: (192, 192)]
setting position to:  (42.484, 42.484)
in item_rect_changed callback, get_global_rect() returns: [P: (42.484, 42.484), S: (192, 192)]
setting position to:  (43.485, 43.485)
in item_rect_changed callback, get_global_rect() returns: [P: (42.484, 42.484), S: (192, 192)]
--- Debugging process stopped ---

Notice that sometimes multiple calls to get_global_rect() return the same value. This should not happen because the node is continuously being moved to a new location. For example:

...
setting position to:  (8.469, 8.469)
in item_rect_changed callback, get_global_rect() returns: [P: (8.469, 8.469), S: (192, 192)]
setting position to:  (9.47, 9.47)
in item_rect_changed callback, get_global_rect() returns: [P: (8.469, 8.469), S: (192, 192)]
...

and

...
setting position to:  (36.465, 36.465)
in item_rect_changed callback, get_global_rect() returns: [P: (36.465, 36.465), S: (192, 192)]
setting position to:  (37.466, 37.466)
in item_rect_changed callback, get_global_rect() returns: [P: (36.465, 36.465), S: (192, 192)]
...

I believe this script and these logs make the issue self-evident.

Minimal reproduction project (MRP)

N/A - Just attach that script to a node, connect one signal, run the project, and observe the output.

@DevJac DevJac changed the title Calling get_global_rect() inside a item_rect_changed callback sometimes give the rect as it was before the change, and sometimes as it was after the change Calling get_global_rect() inside an item_rect_changed callback behaves inconsistently May 27, 2024
@DevJac
Copy link
Author

DevJac commented May 28, 2024

Here is a slightly modified script that will fail an assertion when the error occurs (the game will halt when the error occurs).

extends ColorRect


var p: Vector2


func _ready() -> void:
	var timer := Timer.new()
	timer.wait_time = 1
	timer.autostart = true
	timer.connect(
		'timeout',
		func move():
			var t := Time.get_ticks_msec() / 1000.0
			prints('setting position to: ', Vector2(t, t))
			p = Vector2(t, t)
			position = Vector2(t, t)
	)
	add_child(timer)


func _on_item_rect_changed() -> void:
	var r := get_global_rect()
	print('in item_rect_changed callback, get_global_rect() returns: ', r)
	assert(r.position == p)

The source code for get_global_rect() is:

Rect2 Control::get_global_rect() const {
	ERR_READ_THREAD_GUARD_V(Rect2());
	Transform2D xform = get_global_transform();
	return Rect2(xform.get_origin(), xform.get_scale() * get_size());
}

Some further testing confirms that get_global_transform() returns the wrong result, which is the underlying cause of this bug.

@DevJac
Copy link
Author

DevJac commented May 28, 2024

Discussed this with @kleonc on Discord. @kleonc was able to reproduce the issue on Windows with Godot 4.2.2.stable.

Discord discussion: https://discord.com/channels/212250894228652034/762480230559383562/1245071245008044123

We realized that you must move your mouse in and out of the ColorRect in order for this bug to happen. I've updated the original instructions.

(I originally discovered this bug when experimenting with mouse_entered and mouse_exited signals. I have recreated this bug in a completely fresh project multiple times though, so setting up mouse_entered / mouse_exited signals are not necessary for this bug to occur.)

@kleonc
Copy link
Member

kleonc commented May 28, 2024

Reproducible also in current master.

We realized that you must move your mouse in and out of the ColorRect in order for this bug to happen. I've updated the original instructions.

It can be trigerred differently though, the issue seems to be that the global transform can end up incorrectly not invalidated during NOTIFICATION_RESIZED / item_rect_changed signal.

E.g.:

extends ColorRect

var p: Vector2

func _notification(what: int) -> void:
	match what:
		NOTIFICATION_RESIZED:
			print_info("NOTIFICATION_RESIZED")
		NOTIFICATION_TRANSFORM_CHANGED:
			print_info("NOTIFICATION_TRANSFORM_CHANGED")
		NOTIFICATION_LOCAL_TRANSFORM_CHANGED:
			print_info("NOTIFICATION_LOCAL_TRANSFORM_CHANGED")

func _ready() -> void:
	var timer := Timer.new()
	timer.wait_time = 1
	timer.autostart = true
	timer.connect(
		'timeout',
		func move():
			var t := Time.get_ticks_msec() / 1000.0
			print('[%4d] setting position to: %s' % [Engine.get_process_frames(), Vector2(t, t)])
			p = Vector2(t, t)
			position = Vector2(t, t)
	)
	add_child(timer)


func _on_item_rect_changed() -> void:
	print_info("item_rect_changed")
	
func print_info(label: String) -> void:
	var r := get_global_rect()
	print("[%4d] %-30s get_global_rect()=%s" % [Engine.get_process_frames(), label, r])
	if r.position != p:
		print("^".repeat(80))

Outputs:

[   0] NOTIFICATION_TRANSFORM_CHANGED get_global_rect()=[P: (0, 0), S: (40, 40)]
[  58] setting position to: (2.838, 2.838)
[  58] item_rect_changed              get_global_rect()=[P: (0, 0), S: (40, 40)]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants