-
-
Notifications
You must be signed in to change notification settings - Fork 21.3k
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
Physics interpolation (3D) #92391
Physics interpolation (3D) #92391
Conversation
Some early testing with physics ticks set to 10 Hz and physics interpolation enabled in the project settings: It works in Truck Town, but wheels act wonky when the car moves fast enough (due to the wheels spinning too fast for a single physics tick to handle). Also, the FOV change based on speed isn't aware of interpolation so it looks stuttery: simplescreenrecorder-2024-05-26_20.22.58.mp4Here it is without the FOV change: simplescreenrecorder-2024-05-26_20.23.55.mp4Interpolation doesn't seem to work in Platformer 3D and Kinematic Character though: simplescreenrecorder-2024-05-26_20.25.15.mp4simplescreenrecorder-2024-05-26_20.26.18.mp4Note that in Platformer 3D, AnimationTree is used to animate the character, but root motion is not used (unlike the TPS demo). AnimationTree with the Physics Process update mode isn't handled by physics interpolation in 3.x (which leads to stuttery character animations even if movement is smooth), so hopefully this could be implemented in 4.x. |
This now includes all the follow-up fixes/changes, so it is ready for (re-)testing and review. |
Hello, I downloaded the artifact and made a test scene with a car. Here is the scene: Testing with physics ticks set to 15 Hz, the car handling seems to behave the same with physics interpolation on and off, but the wheels shows in a strange way: physics_interpolation_test_01_edit.mp4 |
There seems some confusion here: Wheels, helicopters and fast rotating objects cannot easily be natively handled by physics interpolation. I mentioned it here (and several other places if I remember right), although it may not be in the docs yet: The problem is that in the The problem is that if an object moves more than 180 degrees in one tick, the interpolation will be wrong: Wheel moves 0-190, the interpolation will be from 190-360 (because that is the shortest arc), i.e. the interpolation will rotate in the opposite direction. You can't use intelligence to decide that from the previous move it is likely to move from 0-190, because, well it could be moving in reality from 190-360, and in some circumstances the intelligent version would be wrong. So for fast rotating objects you either have to up the tick rate, or turn off physics interpolation for that object and handle it manually on the client side (rather than server). It is possible the vehicle wheel class could be modified to handle this client side. This has already come up in users games, and they have taken the approach to switch off (using You will probably need to use |
So if an explosive force sends a box rotating quickly you'll see interpolation artifacts? Doesn't it mean that you can never be sure and will have to go back to manually interpolating all unconstrained rigid bodies if you want reliable results? Could angular velocities be taken into account when interpolating maybe? |
Yes, ideally it would be nice to modify the physics / render server to be able to send across something like a delta so we could deal with this problem correctly. The current situation is due to historical design, which didn't foresee physics interpolation. A little background So now (in 4.x at least) it is a lot more feasible to consider more intrusive engine changes to make things work as easily as possible for users (like sending delta to On the other hand we have to trade off the engine code complexity and maintenance, as e.g. sending deltas can makes physics interpolation code creep into more areas, and contributors often have to be aware of how it works in order to make changes / not break things, so there is a definite cost. There's the maintenance of 2 paths - with and without interpolation, and we ideally don't want physics interpolation to add a cost to projects that don't want to use it. |
This comment was marked as off-topic.
This comment was marked as off-topic.
I gave the latest iteration of this PR a try, here are my findings:
Is spinning VehicleWheel shrinking a problem at 60 ticks per second?Unfortunately, it's still an issue at 60 ticks per second, even at moderate speeds (< 100 km/h). It's noticeable enough that even on 60 FPS footage with the game running at 120 FPS, it can still be seen. (On this particular example, this is because I used ShareX for recording, which isn't 100% perfect in terms of frame pacing like Movie Maker mode would be.) It might be worth looking into a client-side solution (i.e. a solution implemented directly in VehicleWheel3D, where the node itself has interpolation disabled but interpolation is performed in the node). I haven't checked if VehicleWheel2D suffers from this issue yet. physics_interpolation_wheel_wobble_60_tps.mp4Are sudden position/rotation changes (e.g. caused by explosions that impact rigid bodies) a problem?From a quick test, they don't appear to be a problem, even at very low tick rates (like 10 TPS): Bouncing cylindersWarning Firefox doesn't seem to display the videos recorded with Movie Maker mode then converted to MP4 with FFmpeg for some reason. Use a Chromium-based browser, or right-click > Save Video As... and view in a media player. physics_interpolation_cylinders_10_tps.mp4Changing tick rate during the simulation doesn't incur a noticeable hitch, as long as the change is smooth (and not a sudden change like 60 -> 20 or the other way around): physics_interpolation_dynamic_tick_rate.mp4physics_interpolation_dynamic_tick_rate_sudden_change.mp4Boxes explosionRecorded with Movie Maker mode. boxes_explosion_10_tps.mp4boxes_5_tps_time_scale_05.mp4Testing projects: |
Hey I just tested it on a certain personal project and it crashes at the end, without printing the final crash error to the editor console. This is the most recent error in the debug console (NOT the cause of the crash) 15 TPS Interpolation + Crash at end: interpolated2.mp4For reference: 15 TPS No Interpolation (But no crash)not-interpolated1.mp4 |
I tested it again with the Console open this time. The final error before crashing seems to be
|
I found the log for the crash in my previous reply, and I'm a bit confused? In that previous instance there is _not_ any fatal error before crashing, it just suddenly crashes.
Full log here: godot2024-05-31T00.00.15.log |
data.client_physics_interpolation_data->timeout_physics_tick = Engine::get_singleton()->get_physics_frames() + 256; | ||
|
||
// Make sure data is up to date. | ||
update_client_physics_interpolation_data(); |
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.
This function seems to be called twice per process frame - once here and the other time due to calling tree->client_physics_interpolation_add_node_3d(...)
at the end of this function. Since update_client_physics_interpolation_data()
is not idempotent the variables global_xform_curr
and global_xform_prev
will end up having same value which results this function returning transforms that are not interpolated as (a + (b - a) * f
equals a
when a == b
.
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.
Yes but see the method, it doesn't update the previous if it's on the same tick, right?
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.
It is often called twice, yes. The function contains a tick counter which ensures it only operates once per tick.
The data must get updated, but if get_global_transform_interpolated()
is called before the tick update, it is refreshed early. The logic admittedly isn't super clear, but it works this way to prevent an order of operations bug.
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.
Yes, it is true that global_xform_prev
is updated only once per tick.
The not so obvious thing is that when update_client_physics_interpolation_data()
gets called from SceneTree::ClientPhysicsInterpolation::physics_process()
, the function Engine::get_singleton()->get_physics_frames()
is returning incorrect value due to being updated late. This way the newly changed value from get_global_transform()
is stored into global_xform_curr
even before we actually change global_xform_prev
. The next call to update_client_physics_interpolation_data()
from get_global_transform_interpolated()
we finally see that the tick has changed and we set global_xform_prev
to global_xform_curr
which is the same as get_global_transform()
and so both global_xform_prev
and global_xform_curr
end up having the same value.
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 very intriguing! Looking forward to this
Given the amount of errors in your log, it is possible you are just blowing the message queue. If you can fix the errors with the The interpolation warnings has a built in timer to prevent excessive error spam. |
Here is another test project that shows a potential issue: test-physics-interpolation-camera-godot4.mp4The NinePatchRect with the 4 corners doesn't stay centered on the ball. The issue is visible only when camera is moving. Project is using 10 physics ticks per second. Interestingly enough this issue is also reproducible in Godot 3.6-beta5. If instead of the built-in physics interpolation we use lawnjelly's smoothing-addon (https://github.com/lawnjelly/smoothing-addon/tree/4.x) then this issue disappears: test-physics-interpolation-camera-godot4-smoothing-addon.mp4Here is the Godot 4 version using the smoothing-addon: To reproduce the first video you might need to get diff --git a/scene/3d/node_3d.cpp b/scene/3d/node_3d.cpp
index 30f5d6f88a..3c36909732 100644
--- a/scene/3d/node_3d.cpp
+++ b/scene/3d/node_3d.cpp
@@ -400,18 +400,18 @@ void Node3D::_disable_client_physics_interpolation() {
}
Transform3D Node3D::_get_global_transform_interpolated(real_t p_interpolation_fraction) {
- ERR_FAIL_COND_V(is_inside_tree(), Transform3D());
+ ERR_FAIL_COND_V(!is_inside_tree(), Transform3D());
// Set in motion the mechanisms for client side interpolation if not already active.
if (!_is_physics_interpolated_client_side()) {
_set_physics_interpolated_client_side(true);
- ERR_FAIL_NULL_V(data.client_physics_interpolation_data, Transform3D());
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();
}
+ ERR_FAIL_NULL_V(data.client_physics_interpolation_data, Transform3D());
// Storing the last tick we requested client interpolation allows us to timeout
// and remove client interpolated nodes from the list to save processing.
@@ -423,9 +423,6 @@ Transform3D Node3D::_get_global_transform_interpolated(real_t p_interpolation_fr
// 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_client_physics_interpolation_data();
-
// Interpolate the current data.
const Transform3D &xform_curr = data.client_physics_interpolation_data->global_xform_curr;
const Transform3D &xform_prev = data.client_physics_interpolation_data->global_xform_prev;
|
Adds 3D fixed timestep interpolation to the rendering server. This does not yet include support for multimeshes or particles. Co-authored-by: lawnjelly <[email protected]>
This PR now includes the latest follow-up fixes that were already merged into the 3.x branch. |
when this gets merged, will it also work with third party physics engines like jolt and rapier? |
Looking at the list of modified files, it looks like this PR should work with third-party physics engines out of the box (no GodotPhysics code was changed). |
Yes, I confirm it works with Godot-Jolt and any other 3D physics engine GDExtension you can find. It's physics engine agnostic, all the physics engine has to do is tick. |
Thanks @rburing and @lawnjelly for the amazing work! 🎉 I know the community is eagerly awaiting this, please all make sure to test it well (will be in 4.4-dev1 and beyond) and report any issues to fix before 4.4. |
So I just gave this a shot on the 4.4 Dev 1 release. Setting aside not being enabled by default (I personally think enabling it by default would be a good precaution for those just getting started with the engine), it instantly makes stuff like the GDQuest TPS controller demo way smoother without any extra work needed. Fantastic PR! |
I've done a quick FPS test project. I can still notice some slight jitter on my 165hz monitor when I move the camera around. It's most apparent when I slowly walk around a small cube and keep it centered on screen. It's almost as if the camera movement is causing the jitter. Unless my character node setup isn't ideal. If I don't look around, it looks buttery smooth while moving. |
Be sure to read the documentation regarding cameras. |
this should NOT be enabled by default, as there's plenty of stuff you need to know about to properly take advantage of this and not introduce potentially massive bugs in your games |
Case in point: TokisanGames/Terrain3D#470 Physics Interpolation breaks the Terrain here. It's unclear if this is a Terrain3D issue with it not properly using Interpolation or Godot messing something up with it. Disabling Interpolation on the Terrain3D node and/or camera that it centers on does nothing.. Though given that just enabling the Physics Interpolation and absolutely disabling it in the Scene tree (by setting root node PI to off) it does seem like Godot's in some way at fault here... |
@mrjustaguy This PR has been merged, please create a new issue with a minimal reproduction project and I can take a look. (This goes for all potential issues.) |
Cant get it to work on quest 3 standalone. Is it mobile compatible? |
There's nothing that should prevent physics interpolation from working on mobile/web platforms. However, there may be XR-specific issues with physics interpolation. Does it work on a Quest 3 tethered to a PC? I suggest opening a separate issue.
While we should not change the default value to avoid breaking existing projects, I'd say we should consider enabling it by default in newly created projects using the approach described in godotengine/godot-proposals#4834. Physics interpolation really ought to be enabled in any shipping game in 2024, as you will never get 100% smooth movement without it (due to different monitor refresh rates or temporary framerate drops). It's an issue that plagues a lot of Unity games released to this day due to its 50 Hz default update rate – and modders are generally unable to fix it. The sooner you enable physics interpolation in a new project, the better you'll be able to diagnose and fix related issues before releasing your game. |
I have not tested that yet but I suspect it would behave just like in non
VR on pc. I don’t know why it’s just native on quest that it turns off
…On Mon, 16 Sep 2024 at 19:05 Hugo Locurcio ***@***.***> wrote:
Is it mobile compatible?
There's nothing that should prevent physics interpolation from working on
mobile/web platforms. However, there may be XR-specific issues with physics
interpolation. Does it work on a Quest 3 tethered to a PC?
—
Reply to this email directly, view it on GitHub
<#92391 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AQY535AJXHQHZ46HQ2IGKNDZW5P2NAVCNFSM6AAAAABIUDSULCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJUGE3TSOJVHE>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Calinou, no, under no circumstance enable this by default, you'll flood Godot's issue section with Bugs related to Physics Interpolation that aren't Godot's fault but just the fundamental principles of PI in action. People Must Know what they're getting into, and the added complexities of dealing with PI, Period. Physics Interpolation isn't and cannot ever be made into "just works" as you have to know when to reset Physics Interpolation (say teleportation of objects) and when to manually handle Interpolation when Godot doesn't understand what you're doing (say handling a gimbaled camera that's moving and rotating) to get the desired results that just work without PI. the First cannot be resolved ever, and the only "patch" would be to add a "Teleport" function that handles this and hope users figure out that it exists before experiencing Teleportation Interpolation issues |
Godot's interpolation already has the "teleport" function: reset_physics_interpolation() For scenarios where you don't want physics interpolation (or want to do your own custom interpolation), you can selectively disable interpolation on a node by node basis, and all its children inherit this property (by default): In fact, even if physics interpolation is enabled in the project, if your root node has the physics interpolation property turned off (and all other nodes are left at default), it'll be functionally similar to having physics interpolation disabled. Given these facts, I can see a better argument for having (project) physics interpolation enabled by default. The scenarios where a user would benefit from interpolation far outnumber the scenarios where users would want interpolation off. And in the scenarios where you would want interpolation off, it's far better to disable interpolation for the specific nodes involved in said scenario, rather than turning it off for the entire project. |
The main problem is the user has to KNOW they're using Physics Interpolation, something a Beginner WILL NOT KNOW, nor be informed of the implications of using it, and you totally missed the point of the 2 examples I've given above. A User that doesn't even KNOW they're using Physics Interpolation will not KNOW that they need to reset physics interpolation when teleporting, nor that there may be cases where they need to write their own Interpolation Logic, simply because they won't even KNOW that they're using the Feature, It'll just Manifest to them as Bugs that'll needlessly frustrate the unaware users Physics Interpolation can be Heavily Recommended in the Documentation with getting started (and project setting) and all with all the relevant links to the documentation regarding proper usage, but it cannot be stressed enough, this CANNOT be on by default. Don't get me wrong, I'd love to have Physics Interpolation as Standard, but this cannot be enforced by Godot, as Godot can only enforce things that are blatantly obvious to the user, or things that "Just Work" |
I am not entirely convinced that having it enabled by default is the correct thing, but this argument does not make much sense. There are thousands of things a Godot beginner will also not KNOW, and that's why the documentation exists. In contrast, how many "camera jitter bugs" would go away if interpolation was on by default? It seems like a rite of passage that could easily be dealt with now. A Godot beginner will most likely go through the "Getting Started" articles, so why can't we respect them a bit more and assume they will also read a big warning note if it is added there? |
Adds 3D physics interpolation to the rendering server, based on @lawnjelly's
This PR does not yet include support for multimeshes (#91818) or particles.
3d-physics-interpolation-just-dropped.mp4
To-do:
RenderingServer
methods and add their documentation.This PR is sponsored by My Spare Time™.
closes Add physics step interpolation in 3D (implemented in 2D since 4.3) godot-proposals#2753.