Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent errors when using ViewportTexture #75751

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions doc/classes/Node.xml
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,13 @@
Returns [code]true[/code] if the local system is the multiplayer authority of this node.
</description>
</method>
<method name="is_node_ready" qualifiers="const">
<return type="bool" />
<description>
Returns [code]true[/code] if the node is ready, i.e. it's inside scene tree and all its children are initialized.
[method request_ready] resets it back to [code]false[/code].
</description>
</method>
<method name="is_physics_processing" qualifiers="const">
<return type="bool" />
<description>
Expand Down
2 changes: 1 addition & 1 deletion doc/classes/ViewportTexture.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<description>
Displays the content of a [Viewport] node as a dynamic [Texture2D]. This can be used to mix controls, 2D, and 3D elements in the same scene.
To create a ViewportTexture in code, use the [method Viewport.get_texture] method on the target viewport.
[b]Note:[/b] When local to scene, this texture uses [method Resource.setup_local_to_scene] to set the proxy texture and flags in the local viewport.
[b]Note:[/b] When local to scene, this texture uses [method Resource.setup_local_to_scene] to set the proxy texture and flags in the local viewport. Local to scene viewport textures will return incorrect data until the scene root is ready (see [signal Node.ready]).
</description>
<tutorials>
<link title="GUI in 3D Demo">https://godotengine.org/asset-library/asset/127</link>
Expand Down
5 changes: 5 additions & 0 deletions scene/main/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2786,6 +2786,10 @@ bool Node::is_displayed_folded() const {
return data.display_folded;
}

bool Node::is_ready() const {
return !data.ready_first;
}
Comment on lines +2789 to +2791
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we expose this to the bindings?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It needs a different name, because RichTextLabel already has is_ready() 🙃

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch... The RTL one should be named is_done_processing or similar... but that would break compat.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok exposed it as is_node_ready().


void Node::request_ready() {
data.ready_first = true;
}
Expand Down Expand Up @@ -2932,6 +2936,7 @@ void Node::_bind_methods() {
ClassDB::bind_method(D_METHOD("queue_free"), &Node::queue_free);

ClassDB::bind_method(D_METHOD("request_ready"), &Node::request_ready);
ClassDB::bind_method(D_METHOD("is_node_ready"), &Node::is_ready);

ClassDB::bind_method(D_METHOD("set_multiplayer_authority", "id", "recursive"), &Node::set_multiplayer_authority, DEFVAL(true));
ClassDB::bind_method(D_METHOD("get_multiplayer_authority"), &Node::get_multiplayer_authority);
Expand Down
1 change: 1 addition & 0 deletions scene/main/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ class Node : public Object {
bool can_process_notification(int p_what) const;
bool is_enabled() const;

bool is_ready() const;
void request_ready();

static void print_orphan_nodes();
Expand Down
72 changes: 53 additions & 19 deletions scene/main/viewport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@
#include "servers/rendering/rendering_server_globals.h"

void ViewportTexture::setup_local_to_scene() {
if (vp_pending) {
return;
}

Node *loc_scene = get_local_scene();
if (!loc_scene) {
return;
Expand All @@ -72,22 +76,11 @@ void ViewportTexture::setup_local_to_scene() {

vp = nullptr;

Node *vpn = loc_scene->get_node(path);
ERR_FAIL_COND_MSG(!vpn, "ViewportTexture: Path to node is invalid.");

vp = Object::cast_to<Viewport>(vpn);

ERR_FAIL_COND_MSG(!vp, "ViewportTexture: Path to node does not point to a viewport.");

vp->viewport_textures.insert(this);

ERR_FAIL_NULL(RenderingServer::get_singleton());
if (proxy_ph.is_valid()) {
RS::get_singleton()->texture_proxy_update(proxy, vp->texture_rid);
RS::get_singleton()->free(proxy_ph);
if (loc_scene->is_ready()) {
_setup_local_to_scene(loc_scene);
} else {
ERR_FAIL_COND(proxy.is_valid()); // Should be invalid.
proxy = RS::get_singleton()->texture_proxy_create(vp->texture_rid);
loc_scene->connect(SNAME("ready"), callable_mp(this, &ViewportTexture::_setup_local_to_scene).bind(loc_scene), CONNECT_ONE_SHOT);
vp_pending = true;
}
}

Expand All @@ -108,17 +101,32 @@ NodePath ViewportTexture::get_viewport_path_in_scene() const {
}

int ViewportTexture::get_width() const {
ERR_FAIL_COND_V_MSG(!vp, 0, "Viewport Texture must be set to use it.");
if (!vp) {
if (!vp_pending) {
ERR_PRINT("Viewport Texture must be set to use it.");
}
return 0;
}
return vp->size.width;
}

int ViewportTexture::get_height() const {
ERR_FAIL_COND_V_MSG(!vp, 0, "Viewport Texture must be set to use it.");
if (!vp) {
if (!vp_pending) {
ERR_PRINT("Viewport Texture must be set to use it.");
}
return 0;
}
return vp->size.height;
}

Size2 ViewportTexture::get_size() const {
ERR_FAIL_COND_V_MSG(!vp, Size2(), "Viewport Texture must be set to use it.");
if (!vp) {
if (!vp_pending) {
ERR_PRINT("Viewport Texture must be set to use it.");
}
return Size2();
}
return vp->size;
}

Expand All @@ -135,10 +143,36 @@ bool ViewportTexture::has_alpha() const {
}

Ref<Image> ViewportTexture::get_image() const {
ERR_FAIL_COND_V_MSG(!vp, Ref<Image>(), "Viewport Texture must be set to use it.");
if (!vp) {
if (!vp_pending) {
ERR_PRINT("Viewport Texture must be set to use it.");
}
return Ref<Image>();
}
return RS::get_singleton()->texture_2d_get(vp->texture_rid);
}

void ViewportTexture::_setup_local_to_scene(const Node *p_loc_scene) {
Node *vpn = p_loc_scene->get_node(path);
ERR_FAIL_COND_MSG(!vpn, "ViewportTexture: Path to node is invalid.");

vp = Object::cast_to<Viewport>(vpn);

ERR_FAIL_COND_MSG(!vp, "ViewportTexture: Path to node does not point to a viewport.");

vp->viewport_textures.insert(this);

ERR_FAIL_NULL(RenderingServer::get_singleton());
if (proxy_ph.is_valid()) {
RS::get_singleton()->texture_proxy_update(proxy, vp->texture_rid);
RS::get_singleton()->free(proxy_ph);
} else {
ERR_FAIL_COND(proxy.is_valid()); // Should be invalid.
proxy = RS::get_singleton()->texture_proxy_create(vp->texture_rid);
}
vp_pending = false;
}

void ViewportTexture::_bind_methods() {
ClassDB::bind_method(D_METHOD("set_viewport_path_in_scene", "path"), &ViewportTexture::set_viewport_path_in_scene);
ClassDB::bind_method(D_METHOD("get_viewport_path_in_scene"), &ViewportTexture::get_viewport_path_in_scene);
Expand Down
3 changes: 3 additions & 0 deletions scene/main/viewport.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ class ViewportTexture : public Texture2D {

friend class Viewport;
Viewport *vp = nullptr;
bool vp_pending = false;

void _setup_local_to_scene(const Node *p_loc_scene);

mutable RID proxy_ph;
mutable RID proxy;
Expand Down