Fix invisible CanvasItem visibility issue

[Sauermann]

Attempt to fix #58388

Make sure that parent is visible in tree before updating Rendering server.
I believe, that NOTIFICATION_VISIBILITY_CHANGED should be sent independent on the visibility status of the parent.

[YuriSizov]

I believe, that NOTIFICATION_VISIBILITY_CHANGED should be sent independent on the visibility status of the parent.

It was like that already, _propagate_visibility_changed() sends that notification if this method call is reached. With your changes the notification will be send out twice if the parent is visible.

[Sauermann]

When should NOTIFICATION_VISIBILITY_CHANGED be sent?

• when the actual visibility of an CanvasItem on screen changes or
• when the variable CanvasItem.visible changes

[KoBeWi]

^
Both. When visibility actually changes as a result of parent node turning (in)visible or when the node itself changes its visible property. The latter is actually only needed for SceneTreeDock to update the eye icon.

Also I think the bug might not be related to CanvasItem code at all, I edited the issue with more details. The recent changes might've just exposed it.

[Sauermann]

Thanks for the explanation. I updated the patch.

[KoBeWi]

Unfortunately this creates a new bug:

[Sauermann]

CanvasItem::_propagate_visibility_changed is a bit confusing - it tries to do too much. Sorry, but I had to split this function up, in order to fix the bug.

[KoBeWi]

Now it seems to fix all issues, but the code should be checked by @reduz

[reduz]

Looks nice and tidy to me, great job!

[akien-mga]

Thanks!

[akien-mga]

This could use a dedicated backport for 3.x following #57900 (CC @timothyqiu).

The conflicts are relatively easy to solve when cherry-picking but the one thing I'm unsure is whether change_notify("visible") (not present in master) should go in _handle_visibility_change directly or stay only in set_visible.

[timothyqiu]

the one thing I'm unsure is whether change_notify("visible") (not present in master) should go in _handle_visibility_change directly or stay only in set_visible.

change_notify("visible") is only for changes of the visible property, so I believe it should stay only in set_visible.