From 688dc534e59150b3fd9362e5174abbfa57f353e2 Mon Sep 17 00:00:00 2001 From: lawnjelly Date: Mon, 21 Feb 2022 07:34:42 +0000 Subject: [PATCH] Fix get_global_transform_interpolated() with multiple ticks per frame The previous and current transforms in the interpolation data were not being correctly updated in cases where two or more physics ticks occurred on a frame. This PR introduces a simple mechanism to ensure updates on interpolated spatials. --- main/main.cpp | 1 + scene/3d/spatial.cpp | 98 +++++++++++++++++++++++++-------- scene/3d/spatial.h | 9 ++- scene/animation/skeleton_ik.cpp | 4 +- scene/main/scene_tree.cpp | 36 ++++++++++++ scene/main/scene_tree.h | 9 +++ 6 files changed, 128 insertions(+), 29 deletions(-) diff --git a/main/main.cpp b/main/main.cpp index 64f5c1d41e8..ad802d01d55 100644 --- a/main/main.cpp +++ b/main/main.cpp @@ -2240,6 +2240,7 @@ bool Main::iteration() { if (OS::get_singleton()->get_main_loop()->iteration(frame_slice * time_scale)) { exit = true; + Engine::get_singleton()->_in_physics = false; break; } diff --git a/scene/3d/spatial.cpp b/scene/3d/spatial.cpp index a48819f6319..f4f23814282 100644 --- a/scene/3d/spatial.cpp +++ b/scene/3d/spatial.cpp @@ -181,6 +181,7 @@ void Spatial::_notification(int p_what) { data.parent = nullptr; data.C = nullptr; data.toplevel_active = false; + _disable_client_physics_interpolation(); } break; case NOTIFICATION_ENTER_WORLD: { data.inside_world = true; @@ -238,8 +239,8 @@ void Spatial::_notification(int p_what) { #endif } break; case NOTIFICATION_RESET_PHYSICS_INTERPOLATION: { - if (data.physics_interpolation_data) { - data.physics_interpolation_data->global_xform_prev = data.physics_interpolation_data->global_xform_curr; + if (data.client_physics_interpolation_data) { + data.client_physics_interpolation_data->global_xform_prev = data.client_physics_interpolation_data->global_xform_curr; } } break; @@ -275,20 +276,53 @@ Transform Spatial::get_transform() const { return data.local_transform; } -void Spatial::_update_physics_interpolation_data() { - if (!_is_physics_interpolated_client_side()) { - return; +// Return false to timeout and remove from the client interpolation list. +bool Spatial::update_client_physics_interpolation_data() { + if (!is_inside_tree() || !_is_physics_interpolated_client_side()) { + return false; } - ERR_FAIL_NULL(data.physics_interpolation_data); - PhysicsInterpolationData &pid = *data.physics_interpolation_data; + ERR_FAIL_NULL_V(data.client_physics_interpolation_data, false); + ClientPhysicsInterpolationData &pid = *data.client_physics_interpolation_data; uint64_t tick = Engine::get_singleton()->get_physics_frames(); - if (tick != pid.current_physics_tick) { - pid.global_xform_prev = pid.global_xform_curr; + + // Has this update been done already this tick? + // (for instance, get_global_transform_interpolated() could be called multiple times) + if (pid.current_physics_tick != tick) { + // timeout? + if (tick >= pid.timeout_physics_tick) { + return false; + } + + if (pid.current_physics_tick == (tick - 1)) { + // normal interpolation situation, there is a continuous flow of data + // from one tick to the next... + pid.global_xform_prev = pid.global_xform_curr; + } else { + // there has been a gap, we cannot sensibly offer interpolation over + // a multitick gap, so we will teleport + pid.global_xform_prev = get_global_transform(); + } pid.current_physics_tick = tick; } pid.global_xform_curr = get_global_transform(); + return true; +} + +void Spatial::_disable_client_physics_interpolation() { + // Disable any current client side interpolation + // (this can always restart as normal if you later re-attach the node to the SceneTree) + if (data.client_physics_interpolation_data) { + memdelete(data.client_physics_interpolation_data); + data.client_physics_interpolation_data = nullptr; + + SceneTree *tree = get_tree(); + if (tree && _client_physics_interpolation_spatials_list.in_list()) { + tree->client_physics_interpolation_remove_spatial(&_client_physics_interpolation_spatials_list); + } + } + _set_physics_interpolated_client_side(false); } Transform Spatial::_get_global_transform_interpolated(real_t p_interpolation_fraction) { @@ -298,22 +332,41 @@ Transform Spatial::_get_global_transform_interpolated(real_t p_interpolation_fra if (!_is_physics_interpolated_client_side()) { _set_physics_interpolated_client_side(true); - ERR_FAIL_COND_V(data.physics_interpolation_data, Transform()); - data.physics_interpolation_data = memnew(PhysicsInterpolationData); - data.physics_interpolation_data->global_xform_curr = get_global_transform(); - data.physics_interpolation_data->global_xform_prev = data.physics_interpolation_data->global_xform_curr; - data.physics_interpolation_data->current_physics_tick = Engine::get_singleton()->get_physics_frames(); + ERR_FAIL_COND_V(data.client_physics_interpolation_data, Transform()); + data.client_physics_interpolation_data = memnew(ClientPhysicsInterpolationData); + data.client_physics_interpolation_data->global_xform_curr = get_global_transform(); + data.client_physics_interpolation_data->global_xform_prev = data.client_physics_interpolation_data->global_xform_curr; + data.client_physics_interpolation_data->current_physics_tick = Engine::get_singleton()->get_physics_frames(); } + // Storing the last tick we requested client interpolation allows us to timeout + // and remove client interpolated nodes from the list to save processing. + // We use some arbitrary timeout here, but this could potentially be user defined. + + // Note: This timeout has to be larger than the number of ticks in a frame, otherwise the interpolated + // data will stop flowing before the next frame is drawn. This should only be relevant at high tick rates. + // We could alternatively do this by frames rather than ticks and avoid this problem, but then the behaviour + // would be machine dependent. + data.client_physics_interpolation_data->timeout_physics_tick = Engine::get_singleton()->get_physics_frames() + 256; + // make sure data is up to date - _update_physics_interpolation_data(); + update_client_physics_interpolation_data(); // interpolate the current data - const Transform &xform_curr = data.physics_interpolation_data->global_xform_curr; - const Transform &xform_prev = data.physics_interpolation_data->global_xform_prev; + const Transform &xform_curr = data.client_physics_interpolation_data->global_xform_curr; + const Transform &xform_prev = data.client_physics_interpolation_data->global_xform_prev; Transform res; TransformInterpolator::interpolate_transform(xform_prev, xform_curr, res, p_interpolation_fraction); + + SceneTree *tree = get_tree(); + + // This should not happen, as is_inside_tree() is checked earlier + ERR_FAIL_NULL_V(tree, res); + if (!_client_physics_interpolation_spatials_list.in_list()) { + tree->client_physics_interpolation_add_spatial(&_client_physics_interpolation_spatials_list); + } + return res; } @@ -321,7 +374,7 @@ Transform Spatial::get_global_transform_interpolated() { // Pass through if physics interpolation is switched off. // This is a convenience, as it allows you to easy turn off interpolation // without changing any code. - if (!is_physics_interpolated_and_enabled()) { + if (Engine::get_singleton()->is_in_physics_frame() || !is_physics_interpolated_and_enabled()) { return get_global_transform(); } @@ -872,7 +925,7 @@ void Spatial::_bind_methods() { } Spatial::Spatial() : - xform_change(this) { + xform_change(this), _client_physics_interpolation_spatials_list(this) { data.dirty = DIRTY_NONE; data.children_lock = 0; @@ -886,7 +939,7 @@ Spatial::Spatial() : data.disable_scale = false; data.vi_visible = true; - data.physics_interpolation_data = nullptr; + data.client_physics_interpolation_data = nullptr; #ifdef TOOLS_ENABLED data.gizmo_disabled = false; @@ -899,8 +952,5 @@ Spatial::Spatial() : } Spatial::~Spatial() { - if (data.physics_interpolation_data) { - memdelete(data.physics_interpolation_data); - data.physics_interpolation_data = nullptr; - } + _disable_client_physics_interpolation(); } diff --git a/scene/3d/spatial.h b/scene/3d/spatial.h index d7a28ef2ee4..0091fde1eab 100644 --- a/scene/3d/spatial.h +++ b/scene/3d/spatial.h @@ -55,10 +55,11 @@ class Spatial : public Node { // optionally stored if we need to do interpolation // client side (i.e. not in VisualServer) so interpolated transforms // can be read back with get_global_transform_interpolated() - struct PhysicsInterpolationData { + struct ClientPhysicsInterpolationData { Transform global_xform_curr; Transform global_xform_prev; uint64_t current_physics_tick = 0; + uint64_t timeout_physics_tick = 0; }; enum TransformDirty { @@ -69,6 +70,7 @@ class Spatial : public Node { }; mutable SelfList xform_change; + SelfList _client_physics_interpolation_spatials_list; struct Data { mutable Transform global_transform; @@ -101,7 +103,7 @@ class Spatial : public Node { List children; List::Element *C; - PhysicsInterpolationData *physics_interpolation_data; + ClientPhysicsInterpolationData *client_physics_interpolation_data; #ifdef TOOLS_ENABLED Ref gizmo; @@ -121,10 +123,10 @@ protected: _FORCE_INLINE_ void set_ignore_transform_notification(bool p_ignore) { data.ignore_notification = p_ignore; } _FORCE_INLINE_ void _update_local_transform() const; - void _update_physics_interpolation_data(); void _set_vi_visible(bool p_visible); bool _is_vi_visible() const { return data.vi_visible; } Transform _get_global_transform_interpolated(real_t p_interpolation_fraction); + void _disable_client_physics_interpolation(); void _notification(int p_what); static void _bind_methods(); @@ -162,6 +164,7 @@ public: Transform get_transform() const; Transform get_global_transform() const; Transform get_global_transform_interpolated(); + bool update_client_physics_interpolation_data(); #ifdef TOOLS_ENABLED virtual Transform get_global_gizmo_transform() const; diff --git a/scene/animation/skeleton_ik.cpp b/scene/animation/skeleton_ik.cpp index 6f3f55ed254..402cb2d760b 100644 --- a/scene/animation/skeleton_ik.cpp +++ b/scene/animation/skeleton_ik.cpp @@ -550,8 +550,8 @@ Transform SkeletonIK::_get_target_transform() { } if (target_node_override && target_node_override->is_inside_tree()) { - // Make sure to use the interpolated transform as target. This will pass through - // to get_global_transform() when physics interpolation is off, and when using interpolation, + // Make sure to use the interpolated transform as target. + // This will pass through to get_global_transform() when physics interpolation is off, and when using interpolation, // ensure that the target matches the interpolated visual position of the target when updating the IK each frame. return target_node_override->get_global_transform_interpolated(); } else { diff --git a/scene/main/scene_tree.cpp b/scene/main/scene_tree.cpp index 312a8d8488f..94b513ba1b6 100644 --- a/scene/main/scene_tree.cpp +++ b/scene/main/scene_tree.cpp @@ -104,6 +104,27 @@ SceneTreeTimer::SceneTreeTimer() { process_pause = true; } +// This should be called once per physics tick, to make sure the transform previous and current +// is kept up to date on the few spatials that are using client side physics interpolation +void SceneTree::ClientPhysicsInterpolation::physics_process() { + for (SelfList *E = _spatials_list.first(); E;) { + Spatial *spatial = E->self(); + + SelfList *current = E; + + // get the next element here BEFORE we potentially delete one + E = E->next(); + + // This will return false if the spatial has timed out .. + // i.e. If get_global_transform_interpolated() has not been called + // for a few seconds, we can delete from the list to keep processing + // to a minimum. + if (!spatial->update_client_physics_interpolation_data()) { + _spatials_list.remove(current); + } + } +} + void SceneTree::tree_changed() { tree_version++; emit_signal(tree_changed_name); @@ -498,6 +519,16 @@ bool SceneTree::is_physics_interpolation_enabled() const { return _physics_interpolation_enabled; } +void SceneTree::client_physics_interpolation_add_spatial(SelfList *p_elem) { + // This ensures that _update_physics_interpolation_data() will be called at least once every + // physics tick, to ensure the previous and current transforms are kept up to date. + _client_physics_interpolation._spatials_list.add(p_elem); +} + +void SceneTree::client_physics_interpolation_remove_spatial(SelfList *p_elem) { + _client_physics_interpolation._spatials_list.remove(p_elem); +} + bool SceneTree::iteration(float p_time) { root_lock++; @@ -510,6 +541,11 @@ bool SceneTree::iteration(float p_time) { } } + // Any objects performing client physics interpolation + // should be given an opportunity to keep their previous transforms + // up to take before each new physics tick. + _client_physics_interpolation.physics_process(); + flush_transform_notifications(); MainLoop::iteration(p_time); diff --git a/scene/main/scene_tree.h b/scene/main/scene_tree.h index 8338fddae83..1184df0a5bd 100644 --- a/scene/main/scene_tree.h +++ b/scene/main/scene_tree.h @@ -41,6 +41,7 @@ class PackedScene; class Node; +class Spatial; class Viewport; class Material; class Mesh; @@ -102,6 +103,11 @@ private: Group() { changed = false; }; }; + struct ClientPhysicsInterpolation { + SelfList::List _spatials_list; + void physics_process(); + } _client_physics_interpolation; + Viewport *root; uint64_t tree_version; @@ -411,6 +417,9 @@ public: void set_physics_interpolation_enabled(bool p_enabled); bool is_physics_interpolation_enabled() const; + void client_physics_interpolation_add_spatial(SelfList *p_elem); + void client_physics_interpolation_remove_spatial(SelfList *p_elem); + static void add_idle_callback(IdleCallback p_callback); SceneTree(); ~SceneTree();