[3.x] Fix CanvasItem
visibility propagation
#58281
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #58251
I believe the logic is also wrong on
master
, see #58251 (comment). But I'm not sure if a similar fix would cancel the optimization of #57684 onmaster
. Since the issue is currently only reproducible on3.x
, this PR only targets3.x
.The name of
p_was_visible
parameter is no accurate. It's only "was visible" if the node is the source of the propagation. It's alwaysfalse
for other nodes in the propagation chain, making the name awkward. The only place that passes the parameter isset_visible()
:godot/scene/2d/canvas_item.cpp
Line 393 in 7384475
When used, it only makes sense if it's
true
:godot/scene/2d/canvas_item.cpp
Lines 365 to 367 in 7384475
So I changed it to
p_is_source
so it's easier to understand. The above code will emit the signal when it's propagating "hide" and if the current node is visible or the current node is the source of propagation.The actual cause of the issue is line 360:
godot/scene/2d/canvas_item.cpp
Lines 356 to 361 in 7384475
CanvasItem
, this syncsparent_is_visible_in_tree
with it. This should only happen for other nodes in the propagation chain, not for the node itself.p_visible
is thevisible
of the propagation source, it does not have to be the same as itsis_visible_in_tree()
. So a proper check like inNOTIFICATION_ENTER_TREE
should be done.