Skip to content

Conversation

lawnjelly
Copy link
Member

@lawnjelly lawnjelly commented Mar 17, 2025

Moves 3D interpolation from server to the client code (SceneTree). Complete rework of 3D physics interpolation, but using the same user API.

Forward port of #103685
Also includes forward port of #104854 and #104888 and #104920 and #105141 and #105463

Fixes #103683
Fixes #104198
Fixes #103724
Fixes #103025
Fixes #104290

Fixes IK with root motion, Wheels, Pivot relationships, OFF branches
Adds InterpolatedProperty
Adds Interpolated properties for Camera3D

This should bring master up to date with the improvements to 3D physics interpolation in 3.x.

2025-04-01.18-40-56.mp4

There are a number of near-insurmountable problems with the old server based 3D interpolation design. These were identified at an early stage as a likely outcome . The main problems with the old design come from the assumption that scene side would not require access to interpolated xforms (these cannot be retrieved from the server due to the multithread design, command queue and stalling). However, in practice many scene nodes have proven to require access to interpolated xforms in order to work, see the various bugs reported. In practice this leaves us little option but to abandon the server based design for 3D.

Luckily, moving FTI scene side in this PR makes everything conceptually easier for maintainers and users, and should make bug fixing, and interpolation of new properties and nodes far simpler in the future.

  • API compatible / backward compatible with old system
  • Solves a large number of bugs with complex cases
  • Much easier user experience with physics_interpolation_mode OFF branches
  • Accurately renders scene tree pivot relationships

Adds a SceneTreeFTI class with the intention of being used by third parties that write their own custom SceneTree, so
even custom versions of Godot can easily integrate FTI. Note this currently requires deriving from SceneTree. (If there is evidence that projects deriving from MainLoop require this functionality we can change the access to SceneTreeFTI to allow this, but this should probably be based on evidence for a need because there may be some costs or bug potential.)

Notes

  • This is now a somewhat large PR but it contains nearly all the work and bug fixes to 3.x (barring the small XR changes which I will likely leave to @Asaduji as I can't test XR as I don't have the appropriate hardware).

Two passes

There is a chicken egg problem with get_global_transform_interpolated(). Ideally we want to concatenate scene tree xforms to calculate interpolated versions once after idle and immediately prior to rendering. However that would mean getting the interpolated xform during idle would be at best one frame out of date.

The solution to that is some clever management. First of all we do a pass before idle. This ensures that any initial calls to get_global_transform_interpolated() are up to date. When we change node xforms during idle, as part of the normal propagate we add a DIRTY_GLOBAL_INTERPOLATED flag to those altered nodes.

Then during the second pass, we refresh only those nodes that have this flag set. Thus we minimize any recalculation of moved nodes.

Strictly speaking we also can make subsequent calls to get_global_transform_interpolated() correct as to previous calls which may have set transforms during idle. This can be done by checking the dirty flag, and recalculating any dirty chains. I'm not sure yet whether this will be necessary.

Discussion

See #103685

Note that this PR includes a minor fix to CPUParticles but CPUParticles physics interpolation has not been completely forwarded ported yet so constitutes a "work in progress".
GPUParticles does seem to work fine though with physics interpolation after this PR.

@lawnjelly
Copy link
Member Author

Not as tested as 3.x as I don't have many demo projects for master, but it seems to mostly be working so I've moved out of draft. The more testing with projects the better. 👍

@lawnjelly
Copy link
Member Author

I've added a proof of concept draft #104518 for giving users the ability to still use physics interpolation when using servers directly - to address any concerns that these users might be losing out.

They won't lose out, we can support this use case too fairly easily. 👍

This is a considerably simpler requirement than for scene tree interpolation.

Note that it doesn't make sense to combine the two PRs, as they work completely independently (and #104518 will benefit from further bikeshedding to ensure we are happy with the user interface / method, as I say in the PR there are several ways of doing it).

@lawnjelly lawnjelly requested a review from rburing April 17, 2025 09:40
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested again on TPS demo, 3D platformer and Truck Town, it works as expected. The particle streaking issue is fixed.

Camera FOV is now interpolated in Truck Town as well. Here's what it looks like at 10 TPS:

truck_town_10tps.mp4

This seems to already be the case in 2D in master, but not in 3D. Therefore, this PR now fully addresses godotengine/godot-proposals#9964.

@lawnjelly lawnjelly modified the milestones: 4.x, 4.5 Apr 22, 2025
@lawnjelly
Copy link
Member Author

Rebased now that #105437 is merged, and added hook to call instance_teleport() to reset motion vectors.

@Calinou
Copy link
Member

Calinou commented Apr 25, 2025

I tested this PR again and I noticed that VehicleWheels are no longer interpolated with it (both the wheel rotation and suspension). This means #72207 is fixed, but wheels will appear to update at a reduced rate according to the physics FPS. It's not too noticeable if you use untextured wheels, but it's definitely noticeable with detailed wheel models/textures.

vehicle_wheels.mp4

I suppose we'll need a dedicated soluton to interpolate VehicleWheel in any case. Does this PR change anything as for custom interpolation implementations?

@lawnjelly
Copy link
Member Author

lawnjelly commented Apr 26, 2025

(Just to make it clear, the problem in the video above referred to is the wheels, not the truck juddering at the end, that is a physics issue.)

The problems with wheels are described here as "phasing" and "throbbing":
#52846 (comment)
#52846 (comment)
and in more detail in #72207 .

This PR makes the default physics_interpolation_mode OFF for wheels (which I'm getting the impression most users were doing manually for this problem). To get the old behaviour you can simply change the physics_interpolation_mode back to INHERIT in the inspector (or via script).

We have the potential to solve this properly now we have interpolation off. This was discussed in a rendering meeting some time ago (a year or 2 I think) but the crux of the problem is that the interpolation (currently) only has available a previous and current basis in order to calculation interpolation. (If I remember right, we decided that one possible ideal solution in the future will be to optionally allow the physics to send a rotational velocity in addition to sending the basis.)

A basis can only encode a rotation between 0 and 360 degrees, thus if we need to rotate at 180-360 degrees per tick, we have a problem - the interpolation will choose the shortest arc, and will thus interpolate backwards. At 360-540 degrees per tick it will again interpolate forwards, but slower than it should move in reality. This is described as "phasing" in the post.

The problem of only having a basis available was especially acute with the previous VisualServer solution, but now we are doing FTI in the scene code, we have the potential to add bespoke solutions for wheels and objects rotating faster than 180 degrees per tick.

This would have to be investigated later, it's a somewhat tangential problem to general FTI. For now the best compromise for wheels is probably to either keep FTI off, or to alternate between on and off depending on wheel speed. Again this latter would require bespoke code.

Does this PR change anything as for custom interpolation implementations?

This makes custom implementations considerably easier, as previously the user would have to set to off manually, and call get_global_transform_interpolated() (or similar) on the parent to maintain the relationship with the parent node. With SceneTreeFTI there is no need to do the second, error prone step, as it occurs for free for you, on any branch with FTI set to OFF.

Existing custom interpolation code for wheels will likely still work as is, but it would be better to remove any get_global_transform_interpolated() code for the parent as the stock system will work more reliably with no code needed.

s->data.global_transform_interpolated.basis.orthonormalize();
}

// Upload to VisualServer the interpolated global xform.
Copy link
Member

Choose a reason for hiding this comment

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

While at it with the style comments:

Suggested change
// Upload to VisualServer the interpolated global xform.
// Upload to RenderingServer the interpolated global xform.

Sets the visibility range values for the given geometry instance. Equivalent to [member GeometryInstance3D.visibility_range_begin] and related properties.
</description>
</method>
<method name="instance_reset_physics_interpolation">
Copy link
Member

@rburing rburing Apr 26, 2025

Choose a reason for hiding this comment

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

Doesn't unbinding the methods break compatibility with (GDScript) scripts that call the methods? The alternative is to keep them bound as no-ops (under #ifndef DISABLE_DEPRECATED) and mark them as deprecated in the XML.

On second thought, just removing them is clearer, since scripts using these methods would break anyway.

Copy link
Member Author

@lawnjelly lawnjelly Apr 26, 2025

Choose a reason for hiding this comment

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

Yeah, but I suspect that's probably likely to be for the best in the long run.

Users should fix their scripts up using 4.5, this technique will no longer work (at all, physics interpolation no longer works by calling the server directly), and it's better to alert users to this sooner than have it fail silently imo.

Scripts using servers directly for 3D FTI with the 4.4 method and the 4.5 onward method won't be compatible, there's no way around this, or swapping projects between the two without change.

Realistically this is likely to be a small subset, 3D FTI was new for 4.4 anyway (only available for a few weeks, so easier to revert), and few would be using it with servers directly.

Copy link
Member

@rburing rburing left a comment

Choose a reason for hiding this comment

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

Looks like a correct forward-port from what I can tell.

Moves 3D interpolation from server to the client code (`SceneTree`).
Complete rework of 3D physics interpolation, but using the same user API.
@lawnjelly
Copy link
Member Author

lawnjelly commented Apr 26, 2025

Lots more goodies coming after this BTW, I've considerably optimized the system since in #105728 and #105681 (should be 2-10x faster), but those are best left for followups.

Had a look at the wheel code and it does seem to receive rotational velocity from the physics, so it may be simple to fix up a bespoke bit of code to handle fast rotating wheels.

Also we can interpolate more properties such as light parameters.

UPDATE:
Had a look at wheel interpolation this afternoon and now have it working, works even with fast rotating wheels. It's much easier to do now the FTI is scene side.
Will try and do a PR tomorrow. 👍

2025-04-26.19-32-17.mp4

(To be clear, this doesn't affect this PR, just mentioning because @Calinou was asking about wheels earlier, and indicating this is no longer a problem.)

@Repiteo Repiteo merged commit b7d4426 into godotengine:master Apr 28, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Apr 28, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment