From ad9b2b3794f33429182923f55d3451dc98063617 Mon Sep 17 00:00:00 2001 From: lawnjelly Date: Tue, 26 Apr 2022 12:59:53 +0100 Subject: [PATCH] Physics Interpolation - add helper warnings When physics interpolation is active on a node, it is essential that transforms are updated during "_physics_process()" rather than "_process()" calls, for the interpolation to give the correct result. This PR adds optional warnings for instances, cameras and multimeshes which can flag updates being incorrectly called, and thus make these problems much easier to fix. --- doc/classes/ProjectSettings.xml | 4 ++ main/main.cpp | 2 + servers/visual/rasterizer.cpp | 32 ++++++++++ servers/visual/visual_server_scene.cpp | 84 +++++++++++++++++++++++--- 4 files changed, 115 insertions(+), 7 deletions(-) diff --git a/doc/classes/ProjectSettings.xml b/doc/classes/ProjectSettings.xml index 00772ccb250..2e0401513d9 100644 --- a/doc/classes/ProjectSettings.xml +++ b/doc/classes/ProjectSettings.xml @@ -414,6 +414,10 @@ Maximum call stack allowed for debugging GDScript. + + If [code]true[/code], enables warnings which can help pinpoint where nodes are being incorrectly updated, which will result in incorrect interpolation and visual glitches. + When a node is being interpolated, it is essential that the transform is set during [method Node._physics_process] (during a physics tick) rather than [method Node._process] (during a frame). + Maximum amount of functions per frame allowed when profiling. diff --git a/main/main.cpp b/main/main.cpp index 8a45ff7db24..f2acdd0fc51 100644 --- a/main/main.cpp +++ b/main/main.cpp @@ -1224,6 +1224,8 @@ Error Main::setup(const char *execpath, int argc, char *argv[], bool p_second_ph GLOBAL_DEF("debug/settings/stdout/print_fps", false); GLOBAL_DEF("debug/settings/stdout/verbose_stdout", false); + GLOBAL_DEF("debug/settings/physics_interpolation/enable_warnings", true); + if (!OS::get_singleton()->_verbose_stdout) { // Not manually overridden. OS::get_singleton()->_verbose_stdout = GLOBAL_GET("debug/settings/stdout/verbose_stdout"); } diff --git a/servers/visual/rasterizer.cpp b/servers/visual/rasterizer.cpp index 3afcea28f71..560a854128e 100644 --- a/servers/visual/rasterizer.cpp +++ b/servers/visual/rasterizer.cpp @@ -33,6 +33,10 @@ #include "core/os/os.h" #include "core/print_string.h" +#if defined(DEBUG_ENABLED) && defined(TOOLS_ENABLED) +#include "core/project_settings.h" +#endif + Rasterizer *(*Rasterizer::_create_func)() = nullptr; Rasterizer *Rasterizer::create() { @@ -350,6 +354,16 @@ void RasterizerStorage::multimesh_instance_set_transform(RID p_multimesh, int p_ ptr[11] = t.origin.z; _multimesh_add_to_interpolation_lists(p_multimesh, *mmi); + +#if defined(DEBUG_ENABLED) && defined(TOOLS_ENABLED) + if (!Engine::get_singleton()->is_in_physics_frame()) { + static int32_t warn_count = 0; + warn_count++; + if (((warn_count % 2048) == 0) && GLOBAL_GET("debug/settings/physics_interpolation/enable_warnings")) { + WARN_PRINT("Interpolated MultiMesh transform should usually be set during physics process (possibly benign)."); + } + } +#endif return; } } @@ -498,6 +512,15 @@ void RasterizerStorage::multimesh_set_as_bulk_array_interpolated(RID p_multimesh mmi->_data_prev = p_array_prev; mmi->_data_curr = p_array; _multimesh_add_to_interpolation_lists(p_multimesh, *mmi); +#if defined(DEBUG_ENABLED) && defined(TOOLS_ENABLED) + if (!Engine::get_singleton()->is_in_physics_frame()) { + static int32_t warn_count = 0; + warn_count++; + if (((warn_count % 2048) == 0) && GLOBAL_GET("debug/settings/physics_interpolation/enable_warnings")) { + WARN_PRINT("Interpolated MultiMesh transform should usually be set during physics process (possibly benign)."); + } + } +#endif } } @@ -507,6 +530,15 @@ void RasterizerStorage::multimesh_set_as_bulk_array(RID p_multimesh, const PoolV if (mmi->interpolated) { mmi->_data_curr = p_array; _multimesh_add_to_interpolation_lists(p_multimesh, *mmi); +#if defined(DEBUG_ENABLED) && defined(TOOLS_ENABLED) + if (!Engine::get_singleton()->is_in_physics_frame()) { + static int32_t warn_count = 0; + warn_count++; + if (((warn_count % 2048) == 0) && GLOBAL_GET("debug/settings/physics_interpolation/enable_warnings")) { + WARN_PRINT("Interpolated MultiMesh transform should usually be set during physics process (possibly benign)."); + } + } +#endif return; } } diff --git a/servers/visual/visual_server_scene.cpp b/servers/visual/visual_server_scene.cpp index fd1bdc87d82..b3f6123e3ad 100644 --- a/servers/visual/visual_server_scene.cpp +++ b/servers/visual/visual_server_scene.cpp @@ -103,14 +103,37 @@ void VisualServerScene::camera_set_transform(RID p_camera, const Transform &p_tr camera->transform = p_transform.orthonormalized(); - if (_interpolation_data.interpolation_enabled && camera->interpolated) { - if (!camera->on_interpolate_transform_list) { - _interpolation_data.camera_transform_update_list_curr->push_back(p_camera); - camera->on_interpolate_transform_list = true; - } + if (_interpolation_data.interpolation_enabled) { + if (camera->interpolated) { + if (!camera->on_interpolate_transform_list) { + _interpolation_data.camera_transform_update_list_curr->push_back(p_camera); + camera->on_interpolate_transform_list = true; + } - // decide on the interpolation method .. slerp if possible - camera->interpolation_method = TransformInterpolator::find_method(camera->transform_prev.basis, camera->transform.basis); + // decide on the interpolation method .. slerp if possible + camera->interpolation_method = TransformInterpolator::find_method(camera->transform_prev.basis, camera->transform.basis); + +#if defined(DEBUG_ENABLED) && defined(TOOLS_ENABLED) + if (!Engine::get_singleton()->is_in_physics_frame()) { + // Effectively a WARN_PRINT_ONCE but after a certain number of occurrences. + static int32_t warn_count = -256; + if ((warn_count == 0) && GLOBAL_GET("debug/settings/physics_interpolation/enable_warnings")) { + WARN_PRINT("Interpolated Camera transform should usually be set during physics process (possibly benign)."); + } + warn_count++; + } +#endif + } else { +#if defined(DEBUG_ENABLED) && defined(TOOLS_ENABLED) + if (Engine::get_singleton()->is_in_physics_frame()) { + static int32_t warn_count = -256; + if ((warn_count == 0) && GLOBAL_GET("debug/settings/physics_interpolation/enable_warnings")) { + WARN_PRINT("Non-interpolated Camera transform should not usually be set during physics process (possibly benign)."); + } + warn_count++; + } +#endif + } } } @@ -808,6 +831,30 @@ void VisualServerScene::instance_set_transform(RID p_instance, const Transform & #endif instance->transform = p_transform; _instance_queue_update(instance, true); + +#if defined(DEBUG_ENABLED) && defined(TOOLS_ENABLED) + if ((_interpolation_data.interpolation_enabled && !instance->interpolated) && (Engine::get_singleton()->is_in_physics_frame())) { + static int32_t warn_count = 0; + warn_count++; + if (((warn_count % 2048) == 0) && GLOBAL_GET("debug/settings/physics_interpolation/enable_warnings")) { + String node_name; + ObjectID id = instance->object_id; + if (id != 0) { + if (ObjectDB::get_instance(id)) { + Node *node = Object::cast_to(ObjectDB::get_instance(id)); + if (node) { + node_name = "\"" + node->get_name() + "\""; + } else { + node_name = "\"unknown\""; + } + } + } + + WARN_PRINT("Non-interpolated Instance " + node_name + " transform should usually not be set during physics process (possibly benign)."); + } + } +#endif + return; } @@ -861,6 +908,29 @@ void VisualServerScene::instance_set_transform(RID p_instance, const Transform & } _instance_queue_update(instance, true); + +#if defined(DEBUG_ENABLED) && defined(TOOLS_ENABLED) + if (!Engine::get_singleton()->is_in_physics_frame()) { + static int32_t warn_count = 0; + warn_count++; + if (((warn_count % 2048) == 0) && GLOBAL_GET("debug/settings/physics_interpolation/enable_warnings")) { + String node_name; + ObjectID id = instance->object_id; + if (id != 0) { + if (ObjectDB::get_instance(id)) { + Node *node = Object::cast_to(ObjectDB::get_instance(id)); + if (node) { + node_name = "\"" + node->get_name() + "\""; + } else { + node_name = "\"unknown\""; + } + } + } + + WARN_PRINT("Interpolated Instance " + node_name + " transform should usually be set during physics process (possibly benign)."); + } + } +#endif } void VisualServerScene::InterpolationData::notify_free_camera(RID p_rid, Camera &r_camera) {