-
-
Notifications
You must be signed in to change notification settings - Fork 23.4k
Physics Interpolation - Move 3D FTI to SceneTree
#104269
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ac46f47
to
f834a25
Compare
bcf1372
to
f7fe990
Compare
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. 👍 |
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). |
f7fe990
to
a980b9c
Compare
a980b9c
to
ed0bbc6
Compare
167ae2f
to
c303c3c
Compare
cbced45
to
b615cb4
Compare
There was a problem hiding this 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.
b615cb4
to
db323cb
Compare
Rebased now that #105437 is merged, and added hook to call |
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.mp4I suppose we'll need a dedicated soluton to interpolate VehicleWheel in any case. Does this PR change anything as for custom interpolation implementations? |
(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": This PR makes the default 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 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.
This makes custom implementations considerably easier, as previously the user would have to set to off manually, and call Existing custom interpolation code for wheels will likely still work as is, but it would be better to remove any |
scene/main/scene_tree_fti.cpp
Outdated
s->data.global_transform_interpolated.basis.orthonormalize(); | ||
} | ||
|
||
// Upload to VisualServer the interpolated global xform. |
There was a problem hiding this comment.
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:
// 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"> |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this 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.
db323cb
to
ae04a3a
Compare
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: 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.) |
Thanks! |
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.
physics_interpolation_mode
OFF branchesAdds a
SceneTreeFTI
class with the intention of being used by third parties that write their own customSceneTree
, soeven custom versions of Godot can easily integrate FTI. Note this currently requires deriving from
SceneTree
. (If there is evidence that projects deriving fromMainLoop
require this functionality we can change the access toSceneTreeFTI
to allow this, but this should probably be based on evidence for a need because there may be some costs or bug potential.)Notes
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 duringidle
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 aDIRTY_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
butCPUParticles
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.