Viewport Texture loosing node_path reference if SubViewport is SubScene root

[nerdlibfront]

Godot version

v4.1.3.stable.official [f06b6836a]

System information

macOS Ventura 13.5.2 - Godot v4.1.3.stable - Forward+ renderer - Apple Silicon M1 Pro

Issue description

What doesn't work
If using the Path to a SubViewport, that is the root of its own Scene, as a viewport_path of a render texture this reference is lost after restarting the editor and hitting play/saving scene.

How should it work
The viewport_path should not be updated by the editor on load as the target node path is a valid SubViewport Node.

Workaround
If the SubViewport itself is not the root of a subscene, the editor does not loose reference. So the tree structure can be changed that this works.
This is working, though it's not obvious that the scene structure is relevant here and this will throw of users who are new to viewport textures.

Possible related issues
This might be related to issues in Godot 3:
#27790
#83898

Steps to reproduce

• Load the minimal reproduction project
• Hit start or save
• reference in BrakingReference/Sprite3D::texture::viewport_path is lost (you can git diff to see the change in reference or just see that the render texture is not rendered)

Minimal reproduction project

Reproduction project is located here: https://github.com/nerdlibfront/godot-4.1-viewport-scene-issue
Or here: ViewportSceneIssue.zip ViewportSceneIssue.zip

[kleonc]

Minimal reproduction project

Reproduction project is located here: https://github.com/nerdlibfront/godot-4.1-viewport-scene-issue
Or here: ViewportSceneIssue.zip

Note the included .zip archive contains already broken main.tscn (with viewport_path = NodePath(".") saved in it) so needs tweaking before using it to reproduce.
Linked repo contains the proper path saved in there: https://github.com/nerdlibfront/godot-4.1-viewport-scene-issue/blob/094df2bcce5bd2e60d76398994207f0d0e92acdc/main.tscn#L22-L23.

Steps to reproduce

• Load the minimal reproduction project

• Hit start or save

Can't reproduce like that if only main.tscn is opened:

I can reproduce like this (same in v4.1.3.stable.official [f06b6836a] and v4.2.beta5.official [4c96e96]):

1. Close all scenes.
2. Open main.tscn.
3. Open sub_viewport.tscn.
4. Switch back to main.tscn.
   
5. Run (main.tscn).

cc @Rindbee (#77209, #79201)

[nerdlibfront]

You are right. I didn't realize that the open sub_viewport.tscn was also needed to reproduce.
Here is the proper MRP as zip: ViewportSceneIssue.zip

[Rindbee]

This is mainly due to #64388.

godot/scene/main/viewport.cpp

Line 496 in 3e7f638

Node *scene_root = get_scene_file_path().is_empty() ? get_owner() : this;

1. Resources with resource_local_to_scene set to true are restricted to sharing in a single scene instance;
2. The resources in the root node (and the root node) of a sub-scene instance belong to the sub-scene instance, not to its parent scene instance.

viewport_path is the path relative to the scene root. So in this case viewport_path is . (root node of the sub-scene).

The VeiwportTexture and the target Viewport are not in the same scene instance, so this case is expected.

[Rindbee]

So the recommended node tree looks like this:

[kleonc]

viewport_path is the path relative to the scene root.

Indeed, but it's a property of the ViewportTexture and hence it should probably be relative to the root of the scene the given ViewportTexture belongs to, not to the root of the scene the referenced Viewport belongs to.

ViewportTexture::path is currently set relative to the root of the scene containing the Viewport:

godot/scene/main/viewport.cpp

Lines 491 to 506 in 3e7f638

	void Viewport::_update_viewport_path() {
	if (viewport_textures.is_empty()) {
	return;
	}
	Node *scene_root = get_scene_file_path().is_empty() ? get_owner() : this;
	if (!scene_root && is_inside_tree()) {
	scene_root = get_tree()->get_edited_scene_root();
	}
	if (scene_root && (scene_root == this || scene_root->is_ancestor_of(this))) {
	NodePath path_in_scene = scene_root->get_path_to(this);
	for (ViewportTexture *E : viewport_textures) {
	E->path = path_in_scene;
	}
	}
	}

but it's used as if it's relative to the ViewportTexture's local scene:

godot/scene/main/viewport.cpp

Lines 176 to 180 in 3e7f638

	void ViewportTexture::_setup_local_to_scene(const Node *p_loc_scene) {
	// Always reset this, even if this call fails with an error.
	vp_pending = false;
	Node *vpn = p_loc_scene->get_node_or_null(path);

Meaning currently there's an unspoken assumption that Viewport and ViewportTexture both belong to the same scene. But it's not guaranteed to be the case, as e.g. this issue clearly shows.

For sure the current state is bad UX-wise. It either needs to be fixed (I'm not sure if there's anything preventing this 🤔), or if it's indeed meant to be unsupported then the user should be prevented / not allowed to get into situation like reported in here (making ViewportTexture refer to Viewport from different scene).

Also the docs need clarification, it's not stated which scene root the viewport_path is relative to:

godot/doc/classes/ViewportTexture.xml

Lines 18 to 21 in 3e7f638

	<member name="viewport_path" type="NodePath" setter="set_viewport_path_in_scene" getter="get_viewport_path_in_scene" default="NodePath(&quot;&quot;)">
	The path to the [Viewport] node to display. This is relative to the scene root, not to the node that uses the texture.
	[b]Note:[/b] In the editor, this path is automatically updated when the target viewport or one of its ancestors is renamed or moved. At runtime, the path may not be able to automatically update due to the inability to determine the scene root.
	</member>

(@Rindbee To be clear: I'm not suggesting you should do any of this. I've mentioned you because I thought you'd be familiar with / interested in this. 🙃)

[Rindbee]

Also the docs need clarification, it's not stated which scene root the viewport_path is relative to

It is relative to the VeiwportTexture's local_scene. In the same scene instance, they are the same. Yes, the case of VeiwportTexture without resource_local_to_scene enabled (I'm not sure if it makes sense) and the cases of not being in the same scene instance are not taken into account.

If we allow the VeiwportTexture and the target Viewport to be in different scene instances. Cases that may exist:

1. VeiwportTexture in main scene, Veiwport in sub scene;
2. Veiwport in main scene, VeiwportTexture in sub scene;
3. Veiwport in sub scene A, VeiwportTexture in sub scene B;

In case 2 and 3, it may be difficult to determine when the target Viewport is ready. They may be added to the tree separately.

godot/scene/main/viewport.cpp

Lines 82 to 87 in e5bacbc

	if (loc_scene->is_ready()) {
	_setup_local_to_scene(loc_scene);
	} else {
	loc_scene->connect(SNAME("ready"), callable_mp(this, &ViewportTexture::_setup_local_to_scene).bind(loc_scene), CONNECT_ONE_SHOT);
	vp_pending = true;
	}