Physics Interpolation - First use of `get_global_transform_interpolated()` can result in streaking

lawnjelly commented 2 weeks ago

Tested versions

3.6 RC 1 Likely also 4.3

System information

All

Issue description

When following a moving node using get_global_transform_interpolated(), the reported transform will act as though it was reset on the first call, rather than reporting a transform that matches the followed node movement.

The same occurs if get_global_transform_interpolated() is not called for a few seconds, then called again.

https://github.com/godotengine/godot/assets/499286/09a9b722-d67a-4b76-b47d-9311723af30e

Steps to reproduce

Create a scene with a low tick rate (to emphasize problem) with a moving object. Set a second object to follow the first by calling get_global_transform_interpolated().

Minimal reproduction project (MRP)

Run the MRP from #93339 , and click on "Show NinePatchRect" button when the ball is visible.

Discussion

This isn't really a "bug" per se, but it won't be obvious to users how to workaround, so this issue should help to track the problem should it come up, and decide whether / how to add solutions to the engine to help address the problem.

lawnjelly commented 2 weeks ago

This was first reported in #92941, but moving to separate issue as it isn't directly related. My reply is copy pasted from there.

@matmas This is not related directly to this PR I think. The reason is because:

when you first call get_global_transform_interpolated(), this sets up client interpolation system on the ball
but the ball at this point has no information on the previous transform of the ball (this is only stored in VisualServer so is not accessible)
so currently the only thing the ball can do is perform a reset on the client interpolated data the first time you call get_global_transform_interpolated()

If you want to do this with a moving start, you would currently have to "prime" the client interpolation by calling get_global_transform_interpolated() on the previous tick. This limitation is mentioned in the classref:

Note: This function creates an interpolation pump on the Spatial the first time it is called, which can respond to physics interpolation resets. If you get problems with "streaking" when initially following a Spatial, be sure to call get_global_transform_interpolated() at least once before resetting the Spatial physics interpolation.

There is no easy way around this unfortunately. We don't want to store previous transforms on every node just in case you start the interpolation pump (as it has performance implications), so the user has to manually specify which nodes will be "followed". Currently you can only do this by calling get_global_transform_interpolated() at an earlier stage.

Timeout

There is another complication. The client interpolation also has a timeout, which turns off the system after a fixed number of ticks without being used. Again this is to save processing, should the user call get_global_transform_interpolated() on a large number of different objects e.g. just once.

More user friendly solutions

I have some ideas about just setting a bool property on the node to indicate the node should always stay primed, which may be a more user friendly solution (but risks overuse). Another option is to make the timeout modifiable per node, e.g. 0 (always on) or the number of ticks before timeout.

P.S. In your MRP you can cure it by calling:

if ball:
    ball.get_global_transform_interpolated()

In the _process(). The reason this works is it keeps the client interpolation pumping for the ball, whether or not the nine patch is showing.

Sometimes for subtle issues like this we wait for reports to come in from "the wild", to decide whether it is rare (and a workaround such as above might be okay), or whether it justifies adding another property to all nodes.

matmas commented 2 weeks ago

P.S. In your MRP you can cure it by calling:
if ball:
  ball.get_global_transform_interpolated()
In the _process(). The reason this works is it keeps the client interpolation pumping for the ball, whether or not the nine patch is showing.

True. Or we can "re-prime" the client interpolation once again. This re-priming would be probably better when there is a big amount of objects any of which can be selected by the player for example.

oeleo1 commented 1 week ago

Related question @lawnjelly: now that FTI is also available for 2D, is there a reason why get_global_transform_interpolated() is not available in Node2D? The manual Camera adjustments explained in the FTI docs are relevant only for Spatial. We are also fighting streaking with the 3.6-rc1 release and what makes the exercise challenging is that it is sporadic.

lawnjelly commented 1 week ago

is there a reason why get_global_transform_interpolated() is not available in Node2D?

I'm yet to determine whether get_global_transform_interpolated() will be necessary for 2D. If you have some use cases please let me know.

To elaborate, 2D interpolation works slightly differently to 3D. In Godot in 3D, there is no scene tree hierarchy in the VisualServer. Nodes calculate their global space transform client side and send this to the server, which then performs interpolation using previous and current global transforms.

This makes it relatively easy to intercept new global transforms and store them in the scene tree in order to provide the get_global_transform_interpolated() function.

In 2D, things work differently. Local transforms are sent to VisualServer, and the hierarchy of the scene tree is also stored. This enables 2D to calculate global transforms on the fly (on each frame), and also allows far accurate physics interpolation of pivot relationships (which 3D can only approximate).

The downside is, it means that calculating interpolated global transforms in the scene side is more complex and expensive to perform than in 3D, as it has to duplicate the traversal of the scene tree and concatenate transforms all along the chain just to find final global transforms, as well as store prev and curr transforms on each node in the transform chain rather than just the node of interest.

For this reason I'm waiting for some real world use cases to justify adding this functionality. A lot of the use cases for get_global_transform_interpolated() in 3D may be possible in 2D in more elegant ways without needing the function.

oeleo1 commented 1 week ago

I don't have a use case and thanks for the explanation. Traversing the tree to accumulate transforms is a good reason, although there are functions which do just that, like is_visible_in_tree().

On another note, I find that to solve the streaking with FTI we are adding way too much reset_physics_interpolation() calls to my taste.

The typical pattern is to 1) load and instance a scene, which as a rule of thumb is a generic component at position Vector2(0,0), then 2) add it to the scene tree and 3) set its (global) position where it should be which is obviously different than Vector2(0,0). Now, with FTI we have to 4) manually reset the interpolation after adding the scene as a child node and setting its position. Without such resets on instanced and positioned scenes, we see the streaking effect sporadically.

I am not sure there is a nice way out of this. If add_child() could do it automatically for us in case FTI is on, which would make sense, we could set the position of the scene before adding it to the tree, thus saving the manual reset call. But it's a murky idea and I prefer explicit over implicit anyway. So my taste put aside, I guess I'd better consider myself happy with the overall current state of affairs with FTI.

Or maybe set_(global_)position() could do the reset automatically? Ditto for the bulk of functions changing the transform. Would that make sense?

lawnjelly commented 1 week ago

I am not sure there is a nice way out of this. If add_child() could do it automatically for us in case FTI is on, which would make sense, we could set the position of the scene before adding it to the tree, thus saving the manual reset call. But it's a murky idea and I prefer explicit over implicit anyway. So my taste put aside, I guess I'd better consider myself happy with the overall current state of affairs with FTI.

Assuming you are using 3.6 RC1, ithere is a mechanism for auto-reset when adding to the tree. For CanvasItems, NOTIFICATION_RESET_PHYSICS_INTERPOLATION is sent on NOTIFICATION_ENTER_TREE. This does mean the transform would need to be set before entering the tree for this to work.

In the case of wanting to set global transform, you might need to use the old approach of adding, setting global transform then calling reset manually, as obviously global transform makes no sense without being inside the tree (unless there is no transform applied to ancestors).

Or maybe set_(global_)position() could do the reset automatically? Ditto for the bulk of functions changing the transform. Would that make sense?

We discussed these various options at length when first adding interpolation, and Juan wanted an explicit reset_physics_interpolation() call. 99.9% of time you don't want to reset when calling set_transform() or set_global_transform().

It is possible the auto-resets for 2D can be refined in the future, but it's a complex area to do without regressions / limiting use (e.g. moving starts). Certainly not something to be changing in RC stage.

oeleo1 commented 1 week ago

Assuming you are using 3.6 RC1, ithere is a mechanism for auto-reset when adding to the tree. For CanvasItems, NOTIFICATION_RESET_PHYSICS_INTERPOLATION is sent on NOTIFICATION_ENTER_TREE. This does mean the transform would need to be set before entering the tree for this to work.

Thank you for this explanation. I didn't know that bit. It sounds like the docs would benefit from reflecting this exchange. Currently reset_physics_interpolation() is mentioned as a simple white brushstroke in rare circumstances, while it turns out that the overall picture is much more colorful :-).

lawnjelly commented 1 week ago

Thank you for this explanation. I didn't know that bit. It sounds like the docs would benefit from reflecting this exchange. Currently reset_physics_interpolation() is mentioned as a simple white brushstroke in rare circumstances, while it turns out that the overall picture is much more colorful :-).

It's kind of tricky because the situation is in flux, we have limited feedback so far, so things may change. In particular we are getting a lot more feedback from 4.x users now the physics interpolation PRs are available for testing.

We generally are quite conservative on adding to docs until we are longterm sure of the information. If we added information about resets and it changed, this could be confusing depending on which set of docs you were reading. So officially in docs at the moment, you are suggested to call reset always after initial placement or teleporting, which should work in all situations.

So it is one of those cases that to be totally informed at the moment it is necessary to keep an eye on the relevant PRs / discussion.

oeleo1 commented 1 week ago

Fair enough. I am just saying that switching to FTI, beyond ticking the settings option, turns out to be a major project overhaul wrt resetting the physics interpolation.

godotengine / godot