From e3fc5fb1dba48b300d31eb519a7ff4d8f535a9d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= Date: Sun, 10 Mar 2019 04:59:52 +0100 Subject: [PATCH] Fix Viewport and Camera issues 1. Consider 'own_world' as well as 'world' to stop propagating enter/exit world notifications. 2. Clean & fix handling of camera currency. This fixes some random crashes and error logs in the editor; namely - when enabling/disabling own world in a Viewport; - when switching back from a subscene displayed into a main scene's Viewport; - when exiting the editor after any of them; - memory corruption (can that explain certain other seemingly unrelated crash reports?). This also fixes situations where a Viewport and its main Camera get out of sync about which World is relevant to them. --- scene/3d/camera.cpp | 28 ++++++++++++++++++---------- scene/3d/camera.h | 1 + scene/main/viewport.cpp | 36 +++++------------------------------- 3 files changed, 24 insertions(+), 41 deletions(-) diff --git a/scene/3d/camera.cpp b/scene/3d/camera.cpp index 368cebeeab06..fc350cda2e85 100644 --- a/scene/3d/camera.cpp +++ b/scene/3d/camera.cpp @@ -99,9 +99,15 @@ void Camera::_notification(int p_what) { case NOTIFICATION_ENTER_WORLD: { - bool first_camera = get_viewport()->_camera_add(this); - if (!get_tree()->is_node_being_edited(this) && (current || first_camera)) - make_current(); + // Needs to track the Viewport because it's needed on NOTIFICATION_EXIT_WORLD + // and Spatial will handle it first, including clearing its reference to the Viewport, + // therefore making it impossible to subclasses to access it + viewport = get_viewport(); + ERR_FAIL_COND(!viewport); + + bool first_camera = viewport->_camera_add(this); + if (current || first_camera) + viewport->_camera_set(this); } break; case NOTIFICATION_TRANSFORM_CHANGED: { @@ -123,17 +129,20 @@ void Camera::_notification(int p_what) { } } - get_viewport()->_camera_remove(this); + if (viewport) { + viewport->_camera_remove(this); + viewport = NULL; + } } break; case NOTIFICATION_BECAME_CURRENT: { - if (get_world().is_valid()) { - get_world()->_register_camera(this); + if (viewport) { + viewport->find_world()->_register_camera(this); } } break; case NOTIFICATION_LOST_CURRENT: { - if (get_world().is_valid()) { - get_world()->_remove_camera(this); + if (viewport) { + viewport->find_world()->_remove_camera(this); } } break; } @@ -232,8 +241,6 @@ bool Camera::is_current() const { return get_viewport()->get_camera() == this; } else return current; - - return false; } bool Camera::_can_gizmo_scale() const { @@ -651,6 +658,7 @@ Camera::Camera() { near = 0; far = 0; current = false; + viewport = NULL; force_change = false; mode = PROJECTION_PERSPECTIVE; set_perspective(70.0, 0.05, 100.0); diff --git a/scene/3d/camera.h b/scene/3d/camera.h index a531324a85c4..95f14c832756 100644 --- a/scene/3d/camera.h +++ b/scene/3d/camera.h @@ -63,6 +63,7 @@ public: private: bool force_change; bool current; + Viewport *viewport; Projection mode; diff --git a/scene/main/viewport.cpp b/scene/main/viewport.cpp index a5d86c3e1a02..8ab4ed19d806 100644 --- a/scene/main/viewport.cpp +++ b/scene/main/viewport.cpp @@ -880,7 +880,7 @@ void Viewport::_camera_set(Camera *p_camera) { if (camera == p_camera) return; - if (camera && find_world().is_valid()) { + if (camera) { camera->notification(Camera::NOTIFICATION_LOST_CURRENT); } camera = p_camera; @@ -889,7 +889,7 @@ void Viewport::_camera_set(Camera *p_camera) { else VisualServer::get_singleton()->viewport_attach_camera(viewport, RID()); - if (camera && find_world().is_valid()) { + if (camera) { camera->notification(Camera::NOTIFICATION_BECAME_CURRENT); } @@ -908,9 +908,7 @@ void Viewport::_camera_remove(Camera *p_camera) { cameras.erase(p_camera); if (camera == p_camera) { - if (camera && find_world().is_valid()) { - camera->notification(Camera::NOTIFICATION_LOST_CURRENT); - } + camera->notification(Camera::NOTIFICATION_LOST_CURRENT); camera = NULL; } } @@ -1007,7 +1005,7 @@ void Viewport::_propagate_enter_world(Node *p_node) { Viewport *v = Object::cast_to(p_node); if (v) { - if (v->world.is_valid()) + if (v->world.is_valid() || v->own_world.is_valid()) return; } } @@ -1044,7 +1042,7 @@ void Viewport::_propagate_exit_world(Node *p_node) { Viewport *v = Object::cast_to(p_node); if (v) { - if (v->world.is_valid()) + if (v->world.is_valid() || v->own_world.is_valid()) return; } } @@ -1064,23 +1062,11 @@ void Viewport::set_world(const Ref &p_world) { if (is_inside_tree()) _propagate_exit_world(this); -#ifndef _3D_DISABLED - if (find_world().is_valid() && camera) - camera->notification(Camera::NOTIFICATION_LOST_CURRENT); -#endif - world = p_world; if (is_inside_tree()) _propagate_enter_world(this); -#ifndef _3D_DISABLED - if (find_world().is_valid() && camera) - camera->notification(Camera::NOTIFICATION_BECAME_CURRENT); -#endif - - //propagate exit - if (is_inside_tree()) { VisualServer::get_singleton()->viewport_set_scenario(viewport, find_world()->get_scenario()); } @@ -2695,11 +2681,6 @@ void Viewport::set_use_own_world(bool p_world) { if (is_inside_tree()) _propagate_exit_world(this); -#ifndef _3D_DISABLED - if (find_world().is_valid() && camera) - camera->notification(Camera::NOTIFICATION_LOST_CURRENT); -#endif - if (!p_world) own_world = Ref(); else @@ -2708,13 +2689,6 @@ void Viewport::set_use_own_world(bool p_world) { if (is_inside_tree()) _propagate_enter_world(this); -#ifndef _3D_DISABLED - if (find_world().is_valid() && camera) - camera->notification(Camera::NOTIFICATION_BECAME_CURRENT); -#endif - - //propagate exit - if (is_inside_tree()) { VisualServer::get_singleton()->viewport_set_scenario(viewport, find_world()->get_scenario()); }