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

Camera: Modify Camera Movement to work off of time, instead of frame rate, revisited #14809

Closed
wants to merge 5 commits into from

Conversation

PolygonalSun
Copy link
Contributor

This PR contains modified changes from these PRs:

The big difference is that there were new variables added to each of the camera's movement values (eg. pendingCameraDirection for cameraDirection) that will take a user's desired movement value, calculate what value is needed to move for the initial value to match the previous movement, and decay the inertial movement over time. It should be noted that the "pending" variables will only hold their values until consumed by the _checkInputs function is called by its corresponding camera. All of the inertial values will only be populated with the new values inside of the _checkInputs function.

The expected behavior is that the 60 FPS behavior should be the same. Any initial values added to a "pending" variable will move with respect to the same time frame as 60 FPS (eg. A pending movement value at 60 FPS will take one frame and 120 FPS will take around two frames)

@bjsplat
Copy link
Collaborator

bjsplat commented Feb 24, 2024

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Feb 24, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented Feb 24, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented Feb 24, 2024

Visualization tests for WebGPU (Experimental)
Important - these might fail sporadically. This is an optional test.

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/14809/merge/testResults/webgpuplaywright/index.html

@bjsplat
Copy link
Collaborator

bjsplat commented Feb 24, 2024

Visualization tests for webgl1 have failed. If some tests failed because the snapshots do not match, the report can be found at

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/14809/merge/testResults/webgl1/index.html

If tests were successful afterwards, this report might not be available anymore.

@sebavan
Copy link
Member

sebavan commented Feb 27, 2024

Would be great to understand what impacted the particles changes as much ?

@PolygonalSun
Copy link
Contributor Author

Would be great to understand what impacted the particles changes as much ?

I think that it's because I changed the constant frame time value from 16 to 1000 / 60

Copy link

github-actions bot commented Mar 14, 2024

This pull request has been marked as stale because it has been inactive for more than 14 days. Please update to "unstale".

@github-actions github-actions bot added stale and removed stale labels Mar 14, 2024
Copy link

This pull request has been marked as stale because it has been inactive for more than 14 days. Please update to "unstale".

@github-actions github-actions bot added the stale label Mar 30, 2024
@PolygonalSun PolygonalSun removed the stale label Apr 5, 2024
@PolygonalSun
Copy link
Contributor Author

I just want to reiterate that this PR will introduce a new variable for each of the inertial types (eg. pendingCameraDirection or pendingAlphaOffset). While the original used values will move the camera and decrease as before, the "pending" values will take the user/input provided values, scale them to the current frame rate, and add them to the inertial value (ie. old inertial value = (new pending value * scale factor) + new inertial value)

@PolygonalSun PolygonalSun marked this pull request as ready for review April 5, 2024 17:29
@PolygonalSun
Copy link
Contributor Author

PolygonalSun commented Apr 5, 2024

I just want to reiterate that this PR will introduce a new variable for each of the inertial types (eg. pendingCameraDirection or pendingAlphaOffset). While the original used values will move the camera and decrease as before, the "pending" values will take the user/input provided values, scale them to the current frame rate, and add them to the inertial value (ie. old inertial value = (new pending value * scale factor) + new inertial value)

Posting as reference for @sebavan, @bghgary, @RaananW, and @Popov72

@bjsplat
Copy link
Collaborator

bjsplat commented Apr 5, 2024

Visualization tests for WebGL 1 have failed. If some tests failed because the snapshots do not match, the report can be found at

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/14809/merge/testResults/webgl1/index.html

If tests were successful afterwards, this report might not be available anymore.

Copy link
Member

@RaananW RaananW left a comment

Choose a reason for hiding this comment

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

LGTM

A question about the breaking change - is it the way the inertia is being taken into account? i.e. - the camera movement will be subtler now?

@PolygonalSun PolygonalSun marked this pull request as draft April 18, 2024 20:22
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.

None yet

4 participants