Physics Interpolation - Auto-reset on set_physics_interpolation_mode()

[lawnjelly]

Fixes historical bug where auto-reset wasn't working correctly. Also fixes process modes on Cameras when mode is changed.

Forward port of #101218 (see that PR for more details)
Helps address #101192

Doesn't address hidden nodes - these will likely be addressed generally as part of unhiding as the problem of whether or not to reset on unhiding is common to several situations.

[Calinou]

Code looks good to me.

[KeyboardDanni]

This does not completely fix the bug. Any nodes that start hidden will still trigger the issue (which is a potential epilepsy risk, I might add).

Doesn't address hidden nodes - these will likely be addressed generally as part of unhiding as the problem of whether or not to reset on unhiding is common to several situations.

They should always be reset, because the existing data is uninitialized. And by uninitialized I mean they vibrate constantly forever between the world origin and their starting position. We're going from no interpolation to having interpolation, so it doesn't make sense that they should have interpolation anyway. It's better for an interpolated object to be "unprimed" than to shake violently.

I touched on this in the comments on my PR.

[lawnjelly]

This does not completely fix the bug. Any nodes that start hidden will still trigger the issue (which is a potential epilepsy risk, I might add).

Yes, that's what the PR says, and what I pointed out here: #101192 (comment) .

If a node is hidden, then at the risk of pointing out the obvious, then you won't see it vibrating or anything else, because it is, well, hidden. If a node is hidden, then it's children are also hidden.

As I explained when you first asked about this in rocketchat, in Godot the general paradigm is we don't do a load of processing on hidden nodes. If 99% of the nodes in a scene are hidden, we try to reduce as much processing as possible on them so the game runs faster.

Instead what we do is when unhiding them we make sure things are up to date so they will appear in the right place. This often has to be propagated, because unhiding a node may also unhide all its children. If one child is hidden that has 1000 children, that's a lot of propagation that would have to take place unnecessarily for moving hidden nodes.

This is something that reduz decided early on in Godot, and it is common in game engines.

Physics interpolation does pose some challenges here, because we rely on a stream of previous and current transforms. In most cases this means we will eventually end up doing a reset (and propagating) when unhiding nodes, but there are some subtleties to take care of, particularly trying to best deal with unhiding nodes with a moving start.

In rocketchat (as I understood it) you were suggesting your idea that we should instead update hidden nodes so that their previous and current transforms would be correct when they were (eventually) unhidden. That would indeed solve the problem yes, you are correct, but it would have the unfortunate side-effect of making some games run at 10fps instead of 60fps, and I'm not sure users would be happy with that trade off.

Just to refresh, this is what I posted on Jan 7th when you were asking about this problem:

lawnjelly
10:43 AM
I replied on the issue.
It's not to do with performance just the need for manual resets and feature request for auto-reset.
Ideally the previous and current positions should always be updated for physics interpolation, regardless of object visibility or whether interpolation is enabled or disabled.
We don't do this because it would cane performance.
Godot doesn't update stuff that isn't visible in general, with the idea that there's no point in doing work on e.g. 5/6 of a > level that isn't visible.
This is generally a sensible strategy, although it obviously causes problems for fixed timestep interpolation which requires a steady stream of prev / curr transforms.
That's why when unhiding, there's two choices:

• reset (which works for static objects)
• priming, which is required for moving objects so as not to cause a glitch

Cameras following client interpolated nodes is a high priority case of this, and we explicitly do priming in order to prevent glitches.
The aim is to gradually add more auto-resets though, and unhiding is one of the commoner cases.
BTW we can't simply prime all hidden objects as that would be equivalent of just updating them all the time, which is what we aim to prevent (to save performance).
But priming a limited number of camera followed objects is feasible, and what we do currently.
The problem when unhiding a moving node therefore becomes, we need to know the previous transform, otherwise we will get a "glitch" as it stays in the same place over the physics frame.
This is a difficult problem to solve, and is currently left to the user.
The other problem is users hiding and unhiding objects on the same tick. Without FTI, that is fine, but if we auto-reset, this will cause a glitch.
That may be a pill we have to swallow though, as it's not a typical or sensible pattern to use in games.
Or perhaps we can detect this, if it does prove a problem, and prevent auto-reset in these cases.

lawnjelly
11:22 AM
OK I'm adding a fix for the interpolation mode change at runtime:
#101218
Link Preview

[3.x] Physics Interpolation - Auto-reset on set_physics_interpolation_mode() by lawnjelly · Pull Request #101218 · godotengine/godot
Fixes historical bug where auto-reset wasn't working correctly. Partly addresses #101192 for 3.x Notes Looking back through the history this appears just to be a historical bug in a rarely use...
GitHub
It's likely rarely used, but may well just be a historical bug so easy enough to fix.

lawnjelly
11:40 AM
For unhiding I did consider a bunch of options a while back but decided to delay to get more feedback from users.

I'll have a little relook and see whether it might be a good time to add it. It's not super fresh in my memory (as the main work I did 3 years ago) so no promises, there might be some big problem I forgot. 😁

lawnjelly
11:46 AM
Ah looks like there's already an auto-reset for unhiding canvas items.
Have to look at the MRP from the issue to work out which bit isn't working.

KeyboardDanni
2:58 PM
lawnjelly My use case, as I described in the issue, is to enable physics interpolation for high refresh rate monitors, and disable otherwise, as physics interpolation gives very smooth scrolling on gaming displays but causes problems on 60hz because of the refresh rate closely matching the physics rate. I would not consider this rarely used, as it's one of the big reasons I'm using Godot in the first place.
I was under the assumption that this was a supported use case because it can be adjusted at runtime, but there's corner cases that prevent this from being implemented in a way that "just works". I would like to assume that all nodes will always update their previous transforms, even if hidden or physics interpolation is disabled globally. This makes manual lerping handling easier to understand for those cases that need manual intervention, and it removes the corner cases that cause bugs when toggling interpolation globally.
Do we have hard numbers on whether updating previous transforms on hidden nodes impacts performance? I don't think premature optimization should get in the way of things working properly. BTW, this visibility check seems to only happen for 2D nodes, not 3D ones. So the behavior is already inconsistent.

KeyboardDanni
3:16 PM
Alternatively, it'd be nice if we had an "escape hatch" which would force reset previous transforms for all objects on demand, something like force_reset_physics_interpolation() which would exist alongside the normal reset_physics_interpolation(), so that we can keep the existing, performance-conscious behavior while still allowing a second path on the side to override that.
This would have its own code, and possibly notifications as well

lawnjelly
4:44 PM
I would like to assume that all nodes will always update their previous transforms, even if hidden or physics interpolation is disabled globally.

KeyboardDanni You can't assume this.

KeyboardDanni
4:45 PM
Then how do you propose that this bug be fixed? Because to me, it is a bug.

lawnjelly
4:46 PM
Which bug?

KeyboardDanni
4:46 PM
#101192
Link Preview

Graphics shake violently if physics interpolation is enabled at runtime · Issue #101192 · godotengine/godot
Tested versions Reproducible in dev 216b330 System information Godot v4.4.dev (7d645fbea) - Windows 11 (build 22631) - Multi-window, 2 monitors - OpenGL 3 (Compatibility) - NVIDIA GeForce RTX 3070 ...
GitHub

lawnjelly
4:46 PM
I did a PR.

KeyboardDanni
4:46 PM
PR won't fix the case where some nodes start off invisible
I would like a force_reset_physics_interpolation() that also affects invisible nodes

lawnjelly
4:48 PM
Would benefit from a simpler MRP, preferably for 3.x as I'm not super familiar with 4.x.

KeyboardDanni
4:48 PM
I have a simple MRP already, but I can port it to 3.x if need be

lawnjelly
4:49 PM
Sure that would be useful. I'll try and look at it, I have some other work now.

KeyboardDanni
4:50 PM
I have a workaround in place for my own project (modified the engine to remove the visibility checks), but I'll see if I can produce a more elegant solution for a PR
About to submit a PR that solves the other issue (camera stopping when switching interpolation on)
That one should be much safer to merge

lawnjelly
4:52 PM
The camera stopping was pretty simple, it's just there was no record of the current camera in 3.x so I'll probably have to work out what to do there. In 4.x you can e.g. grab the current camera from the viewports and make them current.

KeyboardDanni
4:53 PM
Yeah, but I have a more elegant fix

lawnjelly
4:54 PM
I'm sure the hide thing for your case is not too tricky. It's more the moving case that is difficult.

KeyboardDanni
4:54 PM
Basically the problem is that the current code reacts to changes in process callback by enabling/disabling the appropriate internal process notifications, but not to physics interpolation changing
Camera3D handles this case while Camera2D does not

lawnjelly
4:54 PM
Arg having some internet problems here, apologies if I go offline.
Maybe I forgot to add this to 2D, 2D is rather newer.

KeyboardDanni
4:55 PM
In general the Camera2D code needs some cleanup (I feel it should do processing as normal regardless of interpolation mode, and update just the scroll in process)
There's already a FIXME in the code for it I think, at least as it pertains to smoothing

lawnjelly
4:57 PM
Oh yeah Camera2D might need _physics_interpolated_changed().

KeyboardDanni
4:57 PM
Already on it
PR forthcoming

lawnjelly
4:57 PM
But I think that may be in addition to the process change for the global switch.
I'll check it out. 👍

KeyboardDanni
4:58 PM
Just to be sure though, set_process_internal() has a thread guard on it. Are we certain it's okay to call this function from _physics_interpolated_changed()? Are they guaranteed to happen in the same thread?
The thread concern is why I shied away from this approach initially

lawnjelly
4:59 PM
I would emphasize to just fix the bug in one PR, and do any "cleanup" in another as it makes it easier to review BTW.
Wherever possible, because we need to check for regressions etc for any cleanup, so simpler the better usually.

KeyboardDanni
4:59 PM
Yeah the cleanup will happen separately, especially because it'd technically change how the camera moves

lawnjelly
5:01 PM
Who knows, I don't work on 4.x. 😀

KeyboardDanni
5:24 PM
I'm assuming you've got ongoing projects in 3.x?

KeyboardDanni
5:31 PM
Hmm, _physics_interpolated_changed() doesn't seem to be the right function. I want it to react to set_physics_interpolation_enabled() in the scene tree but _physics_interpolated_changed() is called when the node's own interpolation mode changes, which isn't the case here
Well, probably a good idea to have code to handle that anyway but doesn't solve the original issue

KeyboardDanni
8:57 PM
cc lawnjelly #101245
Link Preview

Fix changing physics interpolation in SceneTree at runtime by KeyboardDanni · Pull Request #101245 · godotengine/godot
Fixes #101192 and #101195 If physics interpolation is disabled, newly created and updated objects don't update their interpolation data, which can cause severe problems if the setting is enable...
GitHub

[akien-mga]

Thanks!

[KeyboardDanni]

Thank you for the response.

Just to clarify though, my PR isn't updating hidden nodes every frame. It only does this when switching the SceneTree's physics interpolation mode. I ended up going with a different system than what we had discussed in RocketChat. I suspect this is where communication broke down.

Basically, the only performance impact from my PR would be when the SceneTree's physics interpolation mode is switched (which won't be often). In effect it is doing the same thing as your PR, but also affecting invisible nodes.

If a node is hidden, then at the risk of pointing out the obvious, then you won't see it vibrating or anything else, because it is, well, hidden. If a node is hidden, then it's children are also hidden.

Not while it's hidden, no. But when it's unhidden it'll start vibrating.

Instead what we do is when unhiding them we make sure things are up to date so they will appear in the right place. This often has to be propagated, because unhiding a node may also unhide all its children. If one child is hidden that has 1000 children, that's a lot of propagation that would have to take place unnecessarily for moving hidden nodes.

When you mention unhiding, do you mean that this will be handled for the user? Or will the user still have to do manual physics resets? If it's the former, I could see this as being a more elegant solution. I'm assuming this would work based off some sort of timestamp?

[lawnjelly]

Just to clarify though, my PR isn't updating hidden nodes every frame. It only does this when switching the SceneTree's physics interpolation mode. I ended up going with a different system than what we had discussed in RocketChat. I suspect this is where communication broke down.

We were aware of how your PR worked, but preferred to go with the existing Godot paradigm that I've attempted to describe above (don't update hidden, update on showing). I did try to communicate this, but apologies if this was misunderstood, I may have assumed some knowledge of Godot core and the client / server architecture.

When you mention unhiding, do you mean that this will be handled for the user?

Yes.

[KeyboardDanni]

Thanks for the clarification!