ViewportTexture remains bright pink in some projects after upgrade to 4.0.3-stable

[z80]

Godot version

4.0.3-stable

System information

Windows 10

Issue description

In some projects ViewportTexture remains pink and doesn't render viewport content after upgrading from 4.0.2-stable to 4.0.3-stable. If a pair of files viewport.h/viewport.cpp are downgraded back to what they were in 4.0.2-stable, problem goes away.

This is how it looks in 4.0.3-stable:

And this is how it looks if viewport.h/viewport.cpp are downgraded back to what they were in 4.0.2-stable.

Steps to reproduce

Unfortunately, I cannot come up wit a simple demo project reproducing the problem. It exists in some projects and doesn't exist in others.

Minimal reproduction project

Cannot produce a simple project reproducing the problem. In simple projects it works fine.

[akien-mga]

Likely a regression from 1146172 (#75751) if it happened between 4.0.2-stable and 4.0.3-stable, CC @KoBeWi @Rindbee.

[KoBeWi]

Does it occur in 4.1 beta?

[Rindbee]

This seems to be an issue that has never been resolved.

It is related to the mechanism of resource_local_to_scene when the scene is instantiated. Resource is set up before Node when the scene is instantiated, setup_local_to_scene() is not called when setting the value of viewport_path, so you need to call setup_local_to_scene() after the node is set up.

godot/scene/resources/packed_scene.cpp

Lines 459 to 463 in ae896bb

	for (KeyValue<Ref<Resource>, Ref<Resource>> &E : resources_local_to_scene) {
	if (E.value->get_local_scene() == ret_nodes[0]) {
	E.value->setup_local_to_scene();
	}
	}

The resources cached by resources_local_to_scene require resources on the resource chain to enable resource_local_to_scene.

Node->R1->R2->R3->ViewportTexture(resource_local_to_scene)

R1, R2, and R3 all have to enable resource_local_to_scene.

godot/scene/resources/packed_scene.cpp

Lines 299 to 335 in ae896bb

	if (res.is_valid()) {
	if (res->is_local_to_scene()) {
	// In a situation where a local-to-scene resource is used in a child node of a non-editable instance,
	// we need to avoid the parent scene from overriding the resource potentially also used in the root
	// of the instantiated scene. That would to the instance having two different instances of the resource.
	// Since at this point it's too late to propagate the resource instance in the parent scene to all the relevant
	// nodes in the instance (and that would require very complex bookkepping), what we do instead is
	// tampering the resource object already there with the values from the node in the parent scene and
	// then tell this node to reference that resource.
	if (n.instance >= 0) {
	Ref<Resource> node_res = node->get(snames[nprops[j].name]);
	if (node_res.is_valid()) {
	node_res->copy_from(res);
	node_res->configure_for_local_scene(node, resources_local_to_scene);
	value = node_res;
	}
	} else {
	HashMap<Ref<Resource>, Ref<Resource>>::Iterator E = resources_local_to_scene.find(res);
	Node *base = i == 0 ? node : ret_nodes[0];
	if (E) {
	value = E->value;
	} else {
	if (p_edit_state == GEN_EDIT_STATE_MAIN) {
	//for the main scene, use the resource as is
	res->configure_for_local_scene(base, resources_local_to_scene);
	resources_local_to_scene[res] = res;
	} else {
	//for instances, a copy must be made
	Ref<Resource> local_dupe = res->duplicate_for_local_scene(base, resources_local_to_scene);
	resources_local_to_scene[res] = local_dupe;
	value = local_dupe;
	}
	}
	}
	//must make a copy, because this res is local to scene
	}
	}

[Rindbee]

Related to #74331.

[akien-mga]

Things aren't fully clear to me but while this appears to be more prominent / easier to trigger in this specific project with 4.0.3, it should be a pre-existing issue according to @Rindbee?

ViewportTexture is a complex area and any change tends to lead to subtle further issues, so given that we've reached release freeze for 4.1, it's unlikely to be fixed for the stable release, hence moving it to 4.2. As soon as we identify a good fix, this would be a good candidate to cherry-pick to 4.1.x and 4.0.x.

[Rindbee]

I realized that it doesn't seem to be that issue (although the sky texture has a similar hue to pink, it's fine). I can't determine what exactly is the cause.

@z80 Is there any other related error message?

[z80]

@Rindbee I'll check and let you know.

[z80]

@Rindbee Hello! Based on the source code it feels that error messages do not provide all the details of what's happening. I made two videos with breakpoints placed in all methods of ViewportTexture. It looks like in the failure case it is the field "vp_pending=true" when the call is "set_viewport_path_in_scene()->setup_local_to_scene()" made which does not allow it to succeed, see failure video at 3:28. The 4.0.2 version of viewport.h/cpp do not have such a field. And this call proceeds without a problem, see success video at 2.45.

This one is a success when viewport.h/cpp are downgraded to what it was in 4.0.2:
https://github.com/godotengine/godot/assets/1449921/86eb2145-9d75-4bba-8b06-591505e84216

And this one is a failure as it happens in some projects in 4.0.3:
https://github.com/godotengine/godot/assets/1449921/e22f91ce-8d00-4b26-a3e0-39168c32e332

I hope it helps.

[Rindbee]

OK, it's helpful. It is failing early in ViewportTexture::_setup_local_to_scene() (due to the target viewport not being in the tree yet), causing vp_pending to fail to reset its state. This makes the setup not yet complete and subsequent calls to ViewportTexture::_setup_local_to_scene() impossible.

vp_pending should be reset to false after ViewportTexture::_setup_local_to_scene() finishes calling (regardless of whether setup failed).