NOTIFICATION_VISIBILITY_CHANGED is called twice when a WindowDialog is opened

[Zylann]

Godot 3.0.2
Windows 10 64 bits

When I call popup_centered_minsize(), my dialog is receiving twice the same notification when made visible.

Repro:

1. Create a WindowDialog
2. Add a script on it which implements _notification and checks for NOTIFICATION_VISIBILITY_CHANGED
3. Add a button to open the dialog at runtime
4. Launch the game, open the dialog: notice it prints twice.

Further open/close only trigger the notification once, as expected.

Project:
VisibilityChangedCalledTwice.zip

[akien-mga]

Reopening, as #18172 was reverted (it caused a regression: #18614).

[Chaosus]

@akien-mga I really sorry

[akien-mga]

No worry :) CanvasItem is such a core component that any change to hit can mess things up in unpredictable way, as happened here. We'll have to do some more research to figure out how to fix this issue without having regressions.

[Zylann]

Any hint on this?

[quasar-kyle-2]

ran into this problem too. hoping for a fix

[qichunren]

"NOTIFICATION_VISIBILITY_CHANGED is called twice" make http send twice when I open "Templates" tab in project manager dialog. #14653

hoping for a fix.

[Schroedi]

It seems this not only affects WindowDialogs but CanvasItems in general. In the attached sample vis_signal_test.zip the first time the node becomes visible, visibility_changed() is called twice. Further, in visibility_changed() the .visible value is always true so there is no way to check the current state, is that expected behavior?

[qichunren]

I found there are two entrances in CanvasItem class that notification(NOTIFICATION_VISIBILITY_CHANGED);

First is _propagate_visibility_changed, which used by show() and hide() function.

void CanvasItem::_propagate_visibility_changed(bool p_visible) {

	notification(NOTIFICATION_VISIBILITY_CHANGED);

	if (p_visible)
		update(); //todo optimize
	else
		emit_signal(SceneStringNames::get_singleton()->hide);
	_block();

	for (int i = 0; i < get_child_count(); i++) {

		CanvasItem *c = Object::cast_to<CanvasItem>(get_child(i));

		if (c && c->visible) //should the toplevels stop propagation? i think so but..
			c->_propagate_visibility_changed(p_visible);
	}

	_unblock();
}

And second is used by CanvasItem::update()

void CanvasItem::_update_callback() {

	if (!is_inside_tree()) {
		pending_update = false;
		return;
	}

	VisualServer::get_singleton()->canvas_item_clear(get_canvas_item());
	//todo updating = true - only allow drawing here
	if (is_visible_in_tree()) { //todo optimize this!!
		if (first_draw) {
			notification(NOTIFICATION_VISIBILITY_CHANGED);
			first_draw = false;
		}
		drawing = true;
		notification(NOTIFICATION_DRAW);
		emit_signal(SceneStringNames::get_singleton()->draw);
		if (get_script_instance()) {
			get_script_instance()->call_multilevel_reversed(SceneStringNames::get_singleton()->_draw, NULL, 0);
		}
		drawing = false;
	}
	//todo updating = false
	pending_update = false; // don't change to false until finished drawing (avoid recursive update)
}

So there should be a mechanism to control notification(NOTIFICATION_VISIBILITY_CHANGED); when first become visible.