-
-
Notifications
You must be signed in to change notification settings - Fork 35.5k
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
PerspectiveCamera: New helper methods getFrustumBounds()
, getFrustumSize()
.
#27574
Conversation
All the other modules were doing it 🤷♂️
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
Made these review upgrades 🦾 plan to commit doc updates to match with the changes shortly |
examples/jsm/utils/CameraUtils.js
Outdated
// .applyMatrix4() -> Accounts for camera position/rotation/scale | ||
return { | ||
topLeft: _vec.set( _minTarget.x, _maxTarget.y, - distance ).applyMatrix4( camera.matrixWorld ).clone(), | ||
topRight: _vec.set( _maxTarget.x, _maxTarget.y, - distance ).applyMatrix4( camera.matrixWorld ).clone(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would return values in camera space - not world space. topRight
, topLeft
, have no meaning once the camera is rotated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems the tests developers usually perform with these coordinates happen in world space. If the vectors are now in camera space, users have to convert their data into camera space as well OR convert the frustum corner points to world space.
I wonder if this makes the workflow unnecessarily complicated.
We often compute bounding volumes in world space knowing they are only valid as long as the object does not move. I think we can make the same assumptions for the corner points as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I said, topRight
, topLeft
, have no meaning in world space. Return the values in camera space. Any user using this method knows how to convert a point from camera space into world space -- if that is what they want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See it as a stretch to say topLeft
fully looses all meaning if the camera is rotated? Agnostic of camera rotation or reference coordinate space, the topLeft vector will still be the point that is rendered to the topLeft of the screen.
Agree that if we return the results in camera-space it blunts the utility meant to be added here. Think world-space is the right play and that the nature of the top/bottom/left/right is still easily apparent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to my previous comments, I do not think .getFrustumBounds()
and .getFrustumCorners()
should return results in different coordinate frames.
And I agree, .getFrustumCorners()
provides little added utility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Bug-Reaper To avoid an impasse here, I suggest you remove getFrustumCorners()
from this PR and create a separate one. In this way, we can at least get the changes to PerspectiveCamera
merged now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay guys, gonna remove getFrustumCorners()
from this PR so we can get the goods merged ⭐️
I do believe there's tangible utility added from a method that returns the corners in world-space. Perhaps the name conventions can be improved to clearly indicate the functionality. Will take the feedback from this discussion to heart and try to keep it in mind when I get a chance to do a seperate PR for this method.
I've added a few commits to the branch to honor the latest feedback. The API in We can now focus on |
getBounds()
, frustumDimensions()
, frustumCorners()
getFrustumBounds()
, getFrustumSize()
, getFrustumCorners()
.
getFrustumBounds()
, getFrustumSize()
, getFrustumCorners()
.getFrustumBounds()
, getFrustumSize()
.
71544c6
to
c9f5641
Compare
getFrustumBounds()
, getFrustumSize()
.getFrustumBounds()
, getFrustumSize()
.
Related #27556
Description
A PerspectiveCamera's viewable margins at an arbitrary distance is a foundational piece of info you can build a lot of clean experiences from. This PR adds 2 new helper methods to PerspectiveCamera intended to make it easy for developers to create cool experiences where viewable area is a consideration.
Some common use-cases include:
Methods added to PerspectiveCamera:
1️⃣ getFrustumBounds()
Calculates the XY of the min/max bounds for the camera's viewable rectangle at a given distance. Results are copied into minTarget and maxTarget input vars.
2️⃣ getFrustumSize()
Calculates the height and width of the camera's frustum at a given distance from the camera. Copies results into the target Vector2 where x is width and y is height.
Method added to CameraUtilsTo be addressed in separate PR:getFrustumCorners()
Provides positions of the camera's frustum corners at a given distance from the camera. Returns an object with topLeft, topRight, bottomRight, and bottomLeft properties where each property is a Vector3.
*This contribution is supported by SoundSafari (shameless plug for my solo art project)