Skip to content
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

[3.x] 2D Fixed Timestep Interpolation #76252

Merged
merged 1 commit into from
Aug 2, 2023
Merged

Conversation

lawnjelly
Copy link
Member

@lawnjelly lawnjelly commented Apr 19, 2023

Adds support to canvas items, canvas lights, canvas light occluders and Camera2D.

Notes

  • No support yet for scene side get_global_transform_interpolated(), probably leave for another PR
  • No explicit support yet for particles, especially global particles (this is being prepared in separate PR)
  • On / off per node is handled by the same InterpolationMode as 3D, although there is significant different - interpolation is performed locally and passed down to children, rather than performed globally per node.
  • In order to sensibly interpolate detect sprite flips there is some extra interpolation code in TransformInterpolator
  • Switch off by default in GUI Controls
  • reset_physics_interpolation() support as in 3D
  • Adds canvas_item_transform_physics_interpolation() to allow origin shifting techniques
  • Slightly moves part of the 3D interpolation code to iteration_prepare() for consistency with 2D. I think this should be okay, but if there are any regressions it's easy to move back.
  • Compatible with hierarchical culling - hierarchical culling uses combined AABB of previous and current ticks (although this as yet does not take account of pivots affecting interpolated bounds)

@lawnjelly lawnjelly added this to the 3.6 milestone Apr 19, 2023
@lawnjelly lawnjelly force-pushed the fti_2d branch 2 times, most recently from bd6c574 to 7b4f835 Compare April 25, 2023 19:02
@lawnjelly lawnjelly marked this pull request as ready for review April 26, 2023 07:15
@lawnjelly lawnjelly requested review from a team as code owners April 26, 2023 07:15
@lawnjelly lawnjelly changed the title [3.x] 2D Fixed Timestep Interpolation [WIP] [3.x] 2D Fixed Timestep Interpolation Apr 26, 2023
@oeleo1

This comment was marked as outdated.

@lawnjelly

This comment was marked as outdated.

@lawnjelly
Copy link
Member Author

Will need a rebase now hierarchical culling is merged, I'll do this when up and running again (but is still possible to look through now).

It's a while since I wrote but one area I would like to improve (likely in a later PR) is the culling. At the moment this merges the previous and current AABB from each two adjacent ticks, with the anticipation that in most circumstances, the interpolated AABB will be between the two. However there may be exceptions with some rotations (particularly where pivots are involved), where the AABB moves outside the merged AABB, so I'm intending to look into this.

If possible I would like to avoid recalculating the AABB on every frame - which would always be correct, but stress the calculations of the AABB and the hierarchical culling unnecessarily. Consider that running at e.g. 30tps and 300fps, we would be doing 10x as many of these calculations as necessary.

Calinou added a commit to Calinou/platshoot that referenced this pull request Jul 3, 2023
@Calinou
Copy link
Member

Calinou commented Jul 3, 2023

I tested this PR locally with https://github.com/Calinou/platshoot/tree/physics-interpolation-test. The basic functionality works well, but I noticed a few issues – see the videos below.

  • Camera2D smoothing is much slower with physics interpolation enabled. I'm rendering the game at 120 FPS, and it seems to be roughly 6 times slower when the game's physics tick rate is 20 Hz. This hints at camera smoothing using the wrong delta.
  • Particles that use global coordinates move around erratically if the player also moves, such as the jetpack particles when holding Shift.
  • Particles suddenly snap into position when firing a bullet, which doesn't occur when interpolation is disabled. This may be a scripting error on my end, but I don't get any warnings despite the interpolatino warnings being enabled (not sure if it's possible for this case to be detected).

Physics FPS is set to 20 in the project (lower values cause frequent player tunneling with the tiles).

No interpolation

simplescreenrecorder-2023-07-03_12.53.20.mp4

With interpolation

simplescreenrecorder-2023-07-03_12.53.49.mp4

@lawnjelly
Copy link
Member Author

lawnjelly commented Jul 3, 2023

Camera2D smoothing is much slower with physics interpolation enabled. I'm rendering the game at 120 FPS, and it seems to be roughly 6 times slower when the game's physics tick rate is 20 Hz. This hints at camera smoothing using the wrong delta.

I'll take a look, I didn't explicitly test the camera smoothing.

Particles that use global coordinates move around erratically if the player also moves, such as the jetpack particles when holding Shift.

This is kind of expected, I spent quite a bit of time on the 3D interpolation rewriting the global particles (more than half the time I spent developing). I haven't touched the global 2D particles at all yet, working on the principle of having something working would be better than nothing at all to get users started. For global particles each particle needs to be interpolated individually if tick based, and a framework needs to be added to support this. (This is also incidentally the reason why I've been taking a while adding interpolation to 4.x, because particles is a bag of worms, and the implementation was in flux in master at the time).

I'll have a look at the 2D particles and see if they can be fixed up in a similar manner to 3D, but it might make sense for a second PR (I'll make this clear in the PR description that particles are not yet converted).

Calinou added a commit to Calinou/escape-space that referenced this pull request Jul 3, 2023
@Calinou
Copy link
Member

Calinou commented Jul 3, 2023

By the way, I ported another project if you need more ways to test it: https://github.com/Calinou/escape-space/tree/physics-interpolation-test

@lawnjelly
Copy link
Member Author

lawnjelly commented Jul 4, 2023

I had a little look at the issue with the camera smoothing being slower with interpolation switched on. The reason is that in the "platshoot" demo project above, the camera process mode is set to idle, and with interpolation, get_camera_transform() (which rather confusingly also does the smoothing) is only being called on physics ticks, because this drives the system of recording the previous and current transforms required for interpolation.

I'm not currently sure whether it makes any sense to try and apply / support smoothing per frame with interpolation on. At best you would get some kind of double smoothed "blancmange". It is doable with some rather more complex logic, but I'm not sure it makes any sense. 🤔

I suspect the best way of fixing this (at least until we get further feedback / some ideas how this might be useable) is to just emulate the physics process mode when interpolation is switched on. We could add e.g. a WARN_PRINT_ONCE that it is acting in physics process mode.

Smoothing at idle when using interpolation would also likely cause stutters, because of the same reason in 3D we recommend get_global_transform_interpolated() for manual smoothing - the target will likely only be moving once per tick.

Actually the way Camera2D currently does smoothing in get_camera_transform() seems like a bug waiting to happen. Presumably if you are smoothing, and you call it multiple times from gdscript during a frame / tick, you will get a different result each time, and it will throw off the regular smoothing. 😕

UPDATE:
Ok I've now changed it to use the physics delta for updating the smoothing when interpolation is active (so the rate of smoothing should be the same as in PROCESS_MODE_PHYSICS). There is also a WARN_PRINT_ONCE if this override has to be used (when TOOLS_ENABLED), to alert the user that their selected PROCESS_MODE_IDLE will be overridden.

@oeleo1

This comment was marked as resolved.

@Calinou
Copy link
Member

Calinou commented Jul 4, 2023

As far as I understand, what @Calinou did is enabling the FTI setting in existing projects “as is” to use FTI, which resulted in slower camera updates and particle issues.

Indeed, I didn't make any manual changes (other than removing the smoothing add-on which both projects previously used, and changing the node structure accordingly).

@lawnjelly
Copy link
Member Author

lawnjelly commented Jul 4, 2023

For the bullets, "platshoot" could potentially be improved with a little trick that I need to add to the docs for 3D:

	bullet.position = bullet_position
	add_child(bullet)
	bullet.get_node("RigidBody2D").linear_velocity = bullet_velocity

	# Trick for bullet start
	bullet.reset_physics_interpolation()
	bullet.position = bullet_position + (bullet_velocity * bullet.get_physics_process_delta_time())

Normally when adding a new object to the scene you add it and call reset_physics_interpolation(). This works for most things like characters, but for moving bullets, it causes a problem - the added object stays glued to the same spot for the first physics tick, because the previous and current transforms are identical.

In order to create a bullet "in flight":

  • add the bullet in the initial position
  • call reset_physics_interpolation() so the previous and current transforms are both the initial position
  • then set the position to what you expect it to be after the first tick (in this case by applying the linear velocity for the delta expected in that first tick). This means "T0" will be the initial position, and "T1" the position after a tick, and it will begin moving between these two right away.

This means the bullet is to some extent "ahead" of itself, but can make the visuals look nicer. The only thing is you need to be aware that the bullet will likely tunnel through anything it would have hit in that first tick, because as far as the physics is concerned, it is starting at "T+1".

Another alternative is to hide the bullet for the first tick, but this can make the input feel laggy.

@lawnjelly
Copy link
Member Author

lawnjelly commented Jul 24, 2023

It looks like the camera is lagging behind the player a few frames, especially noticeable when jumping or suddenly changing velocity, like turning around (eg. from velocity Vector2(80.0, 0.0) to Vector2(-80.0, 0.0)). I tried resetting the camera's interpolation when jumping but it doesn't help with that.

I've tracked this down to being an order of operations bug. The problem was that the Camera2D was doing the current / previous transform update (and retrieving the current transform) on the internal physics tick, which happens before the user physics tick (where the camera might be moved) and before it might get moved as a result of transform changes higher up in the tree. There was thus a one tick delay on the Camera2D in many circumstances.

Changing the current / prev update to occur on NOTIFICATION_TRANSFORM_CHANGED fixes this. I just need to make sure the updates still occur when there have been no changes to the camera transform, and make sure exactly one update happens per tick ("the spice must flow" in order for interpolation to work 😁 ).

UPDATE: Ok pushed with this fix. You will need to test the artifacts though rather than my fork as I haven't made builds yet with this change.

@lawnjelly lawnjelly marked this pull request as ready for review July 24, 2023 17:54
@Calinou
Copy link
Member

Calinou commented Jul 24, 2023

I tested this PR on https://github.com/Calinou/platshoot/tree/physics-interpolation-test again, Camera2D smoothing now works well thanks to the automatic override (there's a warning printed). Particles are still not smoothed, but it may be better to leave this for a future PR so this can be merged in 3.6 betas. The player movement itself feels very smooth.

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.

I gave the code and documentation a cursory look and it looks good to me.

@lawnjelly
Copy link
Member Author

lawnjelly commented Jul 24, 2023

Particles are still not smoothed, but it may be better to leave this for a future PR so this can be merged in 3.6 betas. The player movement itself feels very smooth.

I have CPUParticles2D working in local and global modes. But would prefer for separate PR because this involves some fundamental changes to how they work.

Both 2D and 3D particles in global mode originally previously worked in somewhat strange way (I keep meaning to ask reduz why they work like this, but perhaps it he did not write them originally). They apply the inverse of the parent transform to get to global space, instead of simply marking the nodes as using identity transform. This worked before interpolation (although strange) but with interpolation, this technique is not mathematically good. So for 3D particles I shifted it to marking the node as having identity transform, and baking the global transform into the particles.

For 2D particles, we probably need the same (indeed that is what I've done so far to get it working), but there is an added complication of y-sorting. So I might need to do some experiments here to check this particle system works with y-sorting.

For GPU 2D Particles I have no idea yet. I didn't actually have to do anything special to get them to work in 3D, but they look like they need some tweaks to work with 2D.

For 2D interpolation in 3.6 I'm envisaging we will need 2-3 PRs, plus possible extras for fixing regressions / problem areas as they come up. So just having the basic version in to start with would be good to get feedback as early as possible in betas.

@Marigem
Copy link

Marigem commented Jul 24, 2023

Camera is now perfectly following the player! Tested on 144hz monitor with physics fps set to 60 and it's perfectly smooth and responsive.

Some other bugs I noticed:

  • Setting camera offset is being weird. I use tweens to change the offset to shake and pan the camera, and it isn't shaking at all. It works fine with interpolation turned off though.
  • If parallax layers use mirroring, when the mirroring "activates", it lerps the mirrored layer to its position, instead of it being seamless.
  • Camera still can't change limits and just teleport to the new scroll position.

@lawnjelly
Copy link
Member Author

lawnjelly commented Jul 25, 2023

I'll try and check these out but:

Setting camera offset is being weird. I use tweens to change the offset to shake and pan the camera, and it isn't shaking at all. It works fine with interpolation turned off though.

As with all physics interpolation, tweens will only work if done on the physics tick (there are similar instructions for 3D). I'm not super familiar with tweens, but I'd double check this is the case in your test. Note that when using an interpolated camera, shakes only have the resolution of the physics tick rate, so if you need to use more than this resolution you would need a manually interpolated camera.

UPDATE: offset_h and offset_v seem to work fine, so I'm presuming this is some problem with the tween not operating on physics tick.

If parallax layers use mirroring, when the mirroring "activates", it lerps the mirrored layer to its position, instead of it being seamless.

I'll take a look. I had a similar problem with sprites changing direction, and the TransformInterpolator contains a special fix for these instantaneous flips. Maybe I'm not using this for the parallax layers.

Outdated - see later in thread for solution. UPDATE: although `motion_offset` is interpolated, `motion_mirroring` does not appear to be an interpolated property. Ah, it seems to be set through a "back door" to the `VisualServer` in the function `canvas_set_item_mirroring()`. There are a large number of properties that inevitably _won't be directly supported_ by the core interpolation, we'll have to evaluate on a case by case basis whether they are worth adding direct support. For now these will need manual interpolation (or move in `_process()` rather than `_physics_process()`).

To explain this rationale:

In principle almost everything in the engine could be physics interpolated. Changing modulate colors, animation blends etc etc. The problem is that each of these requires additional code that would complicate the engine codebase, and possibly create new bugs. Therefore the current aim of core interpolation is to support the main use cases (transforms, particles) that are the most tedious to deal with, and leave everything else to the user.

If there is popular demand for particular areas we can add in-built interpolation support on a case by case basis. But I think this will depend on feedback after a beta.

Camera still can't change limits and just teleport to the new scroll position.

I'll take a look. It could be it needs a manual reset_physics_interpolation() call, or maybe we can automatically detect this.

If you have MRPs for any of these it would be useful. 👍

Yes I think I'll need MRPs. I'm trying to reverse engineer the problems but the text is not enough to reproduce.

@Marigem
Copy link

Marigem commented Jul 25, 2023

I'm aware that everything like tweens needs to process on physics tick. I found when physics interpolation is enabled, if the Camera2D isn't following anything, it will ignore the offset property.
Here's an MRP for all 3 of the issues in the previous comment:

3inone.zip

How to use:

  • Press toggle limits button while camera is following player to see the limits bug. Fixed
  • Press pan up or pan down to see offset working properly (While camera is following the player)
  • Press toggle camera follow to make the camera stop following the player and break the offset. Fixed
  • Run right with the character to see the mirrored parallax layer flickering (camera must follow) Fixed

Then try with physics interpolation turned off to see that it behaves differently.

@lawnjelly
Copy link
Member Author

lawnjelly commented Jul 25, 2023

Ah much better with the MRPs. 👍

In fact now from the description I can work out what is probably going on with the offset, the logic in get_camera_transform() is probably not being run. This is why I'm thinking it might be good to move some of that logic out, because the function does not actually do what it says on the tin, it's more of an update, so the flow of data is more difficult to follow.

I suspect the parallax background is similar to a problem I had to fix when testing "jetpaca". Parallax backgrounds that perform a wraparound mod (e.g. from 0.99 -> 0) work fine without interpolation because you don't see the glitch. With interpolation, you sometimes get an interpolated frame at value e.g. 0.45. For "jetpaca" I simply turned off interpolation for these backgrounds as they were moved in process() anyway, but maybe there's a more generic way around the problem.

BTW if you change physics tick rate to e.g. 5-10 tps (rather than 60 tps) you can see more clearly what is going on. It's always a good idea to do this when fixing interpolation issues.

The toggle limits is doing exactly what we would expect with interpolation, it is gliding to the new location over a tick. If we want instantaneous movement with interpolation, we usually have to request it explicitly, with reset_physics_interpolation().

However in this particular case, the new location is controlled by the limits rather than the user calling e.g. set_position(), so calling reset_physics_interpolation() directly after setting the limit may be too early, as it only takes effect in the next get_camera_transform(). 🤔 I'll try and think of a way around this.

@oeleo1
Copy link

oeleo1 commented Jul 25, 2023

UPDATE: although motion_offset is interpolated, motion_mirroring does not appear to be an interpolated property. Ah, it seems to be set through a "back door" to the VisualServer in the function canvas_set_item_mirroring(). There are a large number of properties that inevitably won't be directly supported by the core interpolation, we'll have to evaluate on a case by case basis whether they are worth adding direct support. For now these will need manual interpolation (or move in _process() rather than _physics_process()).

That's the thing. As discussed, FTI is a physics mechanism which, when enabled, stirs everything to it in order to get things working properly. Almost no node in idle would work as expected, unless it is manually or automatically moved to physics (hence the delta in opinions about the configuration warning, among others).

This is the shift in paradigm I have been struggling with when FTI is on. It's a real shift in thinking, because in Godot idle is the default processing mode for all nodes: cameras, tweens, particles, etc. Making all of these work with FTI automatically looks like mission impossible, unless, perhaps like the Camera2D, the whole branch under an FTI enabled node is cascaded and forced to physics...

@lawnjelly
Copy link
Member Author

lawnjelly commented Jul 25, 2023

Run right with the character to see the mirrored parallax layer flickering (camera must follow)

I now have a fairly simple fix for this in parallax_layer.cpp. Will push with this.

@Marigem
Copy link

Marigem commented Jul 25, 2023

That did the trick for the mirroring.

Another thing I just noticed is that the ParallaxLayer's are a little jittery. You can test it in the MRP: Fixed

  • Put a small white ColorRect square as a child to the parallax layer (And set physics interpolation for it to Inherit)
  • Set the ParallaxLayer's motion scale to something like x = 0.3, y = 1.0
  • Set physics_fps to 20 with physics interpolation on, so the problem is really obvious
  • Set character's move_speed to 1000.0
  • Move left and right really fast and see the white square jittering
  • Compare with physics interpolation off and physics_fps set to your monitor's refresh rate.

@lawnjelly
Copy link
Member Author

lawnjelly commented Jul 26, 2023

Pan not working when camera not moving

Press pan up or pan down to see offset working properly (While camera is following the player)
Press toggle camera follow to make the camera stop following the player and break the offset.

Ok, this should now be fixed. Much as it irks me, I've mimicked the old behaviour, which actually has a bug. Like the old behaviour I'm now calling get_camera_transform() every tick, which ensures it gets called even when a new transform is not being applied to the camera.

However, this means that (like the existing non-interpolated version), get_camera_transform() can be called multiple times per tick. In fact it is usually called once when the camera is not moving, and twice when it is moving. The problem is that the camera smoothing code is also within get_camera_transform().

This means the smoothing code is applied once when the camera is not moving, and twice when it is. This is not mathematically correct (if we want to actually produce a smooth result). I've made a rather large TODO comment to point to this bug as it is something we should resolve at some point. Perhaps we can do it for non-interpolated camera too, but it will result in small change in smoothing behaviour. There are also potential for regressions fixing this in the regular camera. With interpolated camera we have carte blanche, there's no existing behaviour to break, so it would be nice to fix this in a PR before 3.6 stable.

The correct fix will probably entail deferring the smoothing until either NOTIFICATION_TRANFORM_CHANGED, or after the transforms have been flushed at the end of a tick. i.e. We may need to keep a record of active cameras and give these an opportunity for a final smooth before the tick is officially finished.

Parallax Layer Chugging

I've worked out what the problem was with the parallax layer .. they are being updated on every frame rather than every tick. This is driven by the update_scroll() in the camera. I've realised that we can actually just switch off physics interpolation on the parallax layers (like we do with Controls) and everything should just work (TM). There's no reason to interpolate these directly, as they are driven by the cameras which are already interpolated.

This also means I can probably remove the fix for wraparound that I'd previously added to the parallax layer, as this shouldn't occur any more.

Will push this fix soon.

Limits snapping

Press toggle limits button while camera is following player to see the limits bug.

Fixed. You were correctly calling reset_physics_interpolation() on the camera after setting the limits, but the problem was on my side - the limits weren't applied until after the reset. The solution was to call get_camera_transform() during the camera reset_physics_interpolation(). You can thus choose to either have the camera interpolate to the new limits (just set them), or to snap there (call reset_physics_interpolation() on the camera).

All mentioned bugs should be fixed now I think, barring particles and the double update for smoothing (which need separate PRs). 👍

@lawnjelly lawnjelly force-pushed the fti_2d branch 2 times, most recently from 9a9b08b to 68752c6 Compare July 26, 2023 09:22
@Marigem
Copy link

Marigem commented Jul 26, 2023

Just tried it and it's working good!

One thing that should be made clear is when exactly to use reset_physics_interpolation() on spawned instances
For example:

instance.global_position = some_vec2
instance.reset_physics_interpolation()
call_deferred("add_child", instance)

or

instance.global_position = some_vec2
call_deferred("add_child", instance)
instance.call_deferred("reset_physics_interpolation")

or

instance.global_position = some_vec2
call_deferred("add_child", instance)
instance.connect("ready", instance, "reset_physics_interpolation", [], CONNECT_ONESHOT)

I got the best result by putting it in the _ready function of the instanced node.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Style review pass.

core/math/transform_interpolator.cpp Outdated Show resolved Hide resolved
core/math/transform_interpolator.cpp Outdated Show resolved Hide resolved
core/math/transform_interpolator.cpp Outdated Show resolved Hide resolved
core/math/transform_interpolator.cpp Outdated Show resolved Hide resolved
core/math/transform_interpolator.cpp Outdated Show resolved Hide resolved
servers/visual/visual_server_canvas.cpp Show resolved Hide resolved
servers/visual/visual_server_canvas.cpp Outdated Show resolved Hide resolved
servers/visual/visual_server_canvas.cpp Outdated Show resolved Hide resolved
servers/visual/visual_server_canvas.cpp Outdated Show resolved Hide resolved
servers/visual/visual_server_raster.cpp Outdated Show resolved Hide resolved
Adds support to canvas items and Camera2D.
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Passes style review.

Was well tested by @Calinou and @lawnjelly, so let's merge and see how it fares under wider testing.

@akien-mga akien-mga merged commit c829572 into godotengine:3.x Aug 2, 2023
13 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants