Merge pull request #58381 from lawnjelly/fti_fix_double_ticks

Fix get_global_transform_interpolated() with multiple ticks per frame

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;
 		}
 

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) {
+			if (data.client_physics_interpolation_data) {
-				data.physics_interpolation_data->global_xform_prev = data.physics_interpolation_data->global_xform_curr;
+				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() {
+// Return false to timeout and remove from the client interpolation list.
-	if (!_is_physics_interpolated_client_side()) {
+bool Spatial::update_client_physics_interpolation_data() {
-		return;
+	if (!is_inside_tree() || !_is_physics_interpolated_client_side()) {
+		return false;
 	}
-	ERR_FAIL_NULL(data.physics_interpolation_data);
+	ERR_FAIL_NULL_V(data.client_physics_interpolation_data, false);
-	PhysicsInterpolationData &pid = *data.physics_interpolation_data;
+	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());
+		ERR_FAIL_COND_V(data.client_physics_interpolation_data, Transform());
-		data.physics_interpolation_data = memnew(PhysicsInterpolationData);
+		data.client_physics_interpolation_data = memnew(ClientPhysicsInterpolationData);
-		data.physics_interpolation_data->global_xform_curr = get_global_transform();
+		data.client_physics_interpolation_data->global_xform_curr = get_global_transform();
-		data.physics_interpolation_data->global_xform_prev = data.physics_interpolation_data->global_xform_curr;
+		data.client_physics_interpolation_data->global_xform_prev = data.client_physics_interpolation_data->global_xform_curr;
-		data.physics_interpolation_data->current_physics_tick = Engine::get_singleton()->get_physics_frames();
+		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_curr = data.client_physics_interpolation_data->global_xform_curr;
-	const Transform &xform_prev = data.physics_interpolation_data->global_xform_prev;
+	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) {
+	_disable_client_physics_interpolation();
-		memdelete(data.physics_interpolation_data);
-		data.physics_interpolation_data = nullptr;
-	}
 }

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<Node> xform_change;
+	SelfList<Spatial> _client_physics_interpolation_spatials_list;
 
 	struct Data {
 		mutable Transform global_transform;
@@ -101,7 +103,7 @@ class Spatial : public Node {
 		List<Spatial *> children;
 		List<Spatial *>::Element *C;
 
-		PhysicsInterpolationData *physics_interpolation_data;
+		ClientPhysicsInterpolationData *client_physics_interpolation_data;
 
 #ifdef TOOLS_ENABLED
 		Ref<SpatialGizmo> 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;

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
+		// Make sure to use the interpolated transform as target.
-		// to get_global_transform() when physics interpolation is off, and when using interpolation,
+		// 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 {

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<Spatial> *E = _spatials_list.first(); E;) {
+		Spatial *spatial = E->self();
+
+		SelfList<Spatial> *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<Spatial> *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<Spatial> *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);

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<Spatial>::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<Spatial> *p_elem);
+	void client_physics_interpolation_remove_spatial(SelfList<Spatial> *p_elem);
+
 	static void add_idle_callback(IdleCallback p_callback);
 	SceneTree();
 	~SceneTree();