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: https://github.com/godotengine/godot/issues/27790 https://github.com/godotengine/godot/issues/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 [4c96e9676]):

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.

https://github.com/godotengine/godot/blob/3e7f638d7b574785f521beafaf52a6ad95be016f/scene/main/viewport.cpp#L496

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: https://github.com/godotengine/godot/blob/3e7f638d7b574785f521beafaf52a6ad95be016f/scene/main/viewport.cpp#L491-L506 but it's used as if it's relative to the ViewportTexture's local scene: https://github.com/godotengine/godot/blob/3e7f638d7b574785f521beafaf52a6ad95be016f/scene/main/viewport.cpp#L176-L180

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 :thinking:), 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: https://github.com/godotengine/godot/blob/3e7f638d7b574785f521beafaf52a6ad95be016f/doc/classes/ViewportTexture.xml#L18-L21

(@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. :upside_down_face:)

[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.

https://github.com/godotengine/godot/blob/e5bacbc4716ce5b54bc2ab8b3053cd5e22ffe446/scene/main/viewport.cpp#L82-L87