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

struck at splash screen after adding child inside _draw() function #95810

Closed
syntaxerror247 opened this issue Aug 19, 2024 · 9 comments · Fixed by #97328
Closed

struck at splash screen after adding child inside _draw() function #95810

syntaxerror247 opened this issue Aug 19, 2024 · 9 comments · Fixed by #97328

Comments

@syntaxerror247
Copy link
Contributor

syntaxerror247 commented Aug 19, 2024

Tested versions

  • Reproducible in 4.3.stable
  • Not Reproducible in 4.2.2.stable

System information

Godot v4.3.stable

Issue description

When adding a child inside _draw(), it gets called infinitely. I think adding new child is calling _draw() again and a new child is added and it goes on and on....
This issue was not present in godot 4.2.2 and new child was added without any problem

Steps to reproduce

  • create a new control scene
  • add a script
extends Control
func _draw() -> void:
	print("draw func called")
	var n = Label.new()
	add_child(n)
  • run your scene

Minimal reproduction project (MRP)

null

@syntaxerror247
Copy link
Contributor Author

As a workaround, I created a can_draw variable and made it true after a process_frame, now adding child is not triggering _draw() func.
So maybe, it only occurs if _draw() is called(or child is added ) in same process_frame as _ready()

This is my workaround code

extends Control
var can_draw = false
func _draw() -> void:
	if not can_draw: return
	print("draw func called")
	var n = Label.new()
	add_child(n)

func _ready() -> void:
	await get_tree().process_frame
	can_draw = true
	queue_redraw()

@AThousandShips
Copy link
Member

AThousandShips commented Aug 19, 2024

To me this isn't very surprising, but since it changed since 4.2 it's something to investigate, but I don't expect there to be any reasonable use case for adding nodes in _draw without doing some tracking, so this code isn't really valid generally IMO

This is by design, as per:

godot/scene/gui/control.cpp

Lines 3299 to 3303 in da5f398

case NOTIFICATION_CHILD_ORDER_CHANGED: {
// Some parents need to know the order of the children to draw (like TabContainer),
// so we update them just in case.
queue_redraw();
} break;

Unsure why it doesn't happen in 4.2, could be that it incorrectly queues drawing immediately due to some flushing

Related:

@AThousandShips
Copy link
Member

The immediate redraw is a bug, though the provided code is still invalid, or generally not appropriate as it'll add an infinite number of children as it'll redraw itself each time

So the error with immediate redraw is an issue, but is already tracked above, so this should probably focus on the possibility of documenting that adding children redraws (though I'd say that should be expected)

@syntaxerror247
Copy link
Contributor Author

though the provided code is still invalid, or generally not appropriate as it'll add an infinite number of children as it'll redraw itself each time

I am drawing a pie chart inside _draw() and adding new labels ( which are Label nodes) everytime it redraws(or changes) and delete previous one and it was working fine in 4.2.2

@AThousandShips
Copy link
Member

AThousandShips commented Aug 19, 2024

Well that would be decent code, but your example doesn't include any removal so that's what I based my statement on 😆

The redrawing should block any queueing happening, so unsure what is broken here, if done during the draw, so unsure why this happens

@matheusmdx
Copy link
Contributor

Bisecting point to #87530 as the culprit

image


How @AThousandShips said this can be a invalid code i'll not ping the pr author, so she can decide what should be done here.

@AThousandShips
Copy link
Member

The code in this report isn't necessarily appropriate in itself, but the result of it is still invalid, so I'll go ahead and ping and I'll also take a look at the code soon, I have an idea what it might be and will do some tests

CC @YeldhamDev

@AThousandShips
Copy link
Member

Can confirm that auto translation is the cause of this, changing auto_translate_mode to disable on the root node prevents this, will try see what kind of issue is sparking this, I suspect it is caused by translation causing resizing which enters some kind of recursion

@AThousandShips
Copy link
Member

I have a tentative fix but need to identify some more details and what exactly is happening in it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Very Bad
Development

Successfully merging a pull request may close this issue.

4 participants