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] Physics Interpolation - refactor Camera and fix get_camera_transform() #92784

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

lawnjelly
Copy link
Member

@lawnjelly lawnjelly commented Jun 5, 2024

  • Adds auto-resets for 3D.
  • Moves 3D Camera interpolation scene side.
  • Automatically switches get_camera_transform() to report interpolated transform during _process().
  • Fixes ClippedCamera to work with physics interpolation.
  • Fixes VisualInstance reset_physics_interpolation transforms being out of date.
  • Refactors resets, particularly for 3D.

Fixes issue mentioned in #92391 (comment)
Fixes issue mentioned in #92391 (comment)

This should fix the following Camera commands to work with physics interpolation:

is_position_behind()
project_local_ray_normal()
project_position()
project_ray_normal()
project_ray_origin()
unproject_position()

There is quite a bit of reset refactoring here to deal with moving reset bugs revealed in testing for #92391 .

Notable Changes

Teleport lists have been removed in favour of instantaneous teleport in VisualServer.
This is possible because client code now flushes any dirty transforms prior to the reset.

Notes

  • Refactors the 3D Camera interpolation to more closely match the technique used in 2D. This allows more efficient access to interpolated transforms for the querying functions.
  • Moves the physics interpolation warnings to functions within error macros to reduce duplication.
  • Adds physics interpolation warning for Camera moved in _process().
  • Allows instantaneous update of Camera parameters within _process().
  • ClippedCamera is supported, but will force physics process mode when interpolation is active (idle mode makes no sense), and shows a warning when forcing.
  • InterpolatedCamera is not supported (as deprecated).
  • ARVRCamera hopefully doesn't need any modifications (but not highly tested).

Milestones

I originally scheduled this for 3.7 as we are in feature freeze now for 3.6, and although this is not a new feature, moving the Camera interpolation scene side is a significant refactor.

However, on reflection, leaving this for 3.7 is going to be a logistical pain, as it fixes a number of significant interpolation bugs in 3.6, and we are finally getting significant testing as a result of this being kept in sync with the version in 4.x. It will also become problematic if we get 3.x and 4.x versions out of sync, as 3.x is the lead for physics interpolation, and @rburing will need this .

So it should be worth it to get in for the first 3.6 RC.

@lawnjelly lawnjelly added this to the 3.x milestone Jun 5, 2024
@lawnjelly lawnjelly force-pushed the fti_camera_to_scene branch 3 times, most recently from 2cd5163 to 2b1136d Compare June 5, 2024 19:07
@lawnjelly lawnjelly marked this pull request as ready for review June 5, 2024 19:10
@lawnjelly lawnjelly requested review from a team as code owners June 5, 2024 19:10
@lawnjelly lawnjelly mentioned this pull request Jun 6, 2024
5 tasks
@lawnjelly lawnjelly force-pushed the fti_camera_to_scene branch 2 times, most recently from ca17713 to b39ecd4 Compare June 7, 2024 09:06
@lawnjelly lawnjelly requested a review from a team as a code owner June 7, 2024 16:48
@lawnjelly lawnjelly marked this pull request as draft June 7, 2024 16:48
…rm()`

* Moves 3D Camera interpolation scene side.
* Automatically switches `get_camera_transform()` to report interpolated transform during `_process()`.
* Fixes `ClippedCamera` to work with physics interpolation.
@lawnjelly lawnjelly marked this pull request as ready for review June 9, 2024 11:20
@lawnjelly lawnjelly modified the milestones: 3.x, 3.6 Jun 9, 2024
@lawnjelly
Copy link
Member Author

This is the MRP for the PR:
Test Physics Interpolation Init3.zip

However, as it uses physics tick counter, it relies on the physics tick counter bug being fixed (see #92941 ). It won't give correct behaviour without the fix.

I'll see if I can create an MRP at later time that works around this bug.

Copy link
Member

@rburing rburing left a comment

Choose a reason for hiding this comment

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

The approach looks good and fixes issues.

@akien-mga akien-mga merged commit 30fa2e3 into godotengine:3.x Jun 18, 2024
14 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
None yet
Development

Successfully merging this pull request may close these issues.

3 participants