-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Mismatch between reported viewport from three.js and R3f #1892
Comments
viewport is in three units, that means width/height would fill the screen. the other values are in pixel units, that's "size" in useThree. when you mutate something in threejs, it's not reactive, there is no way to respond. useThree-size/viewport react to a resize observer or state changes. when you want to set the viewport that is possible as well but it would be your responsibility to inform the state model. there is a const setSize = useThree(state => state.setSize)
...
setSize(width, height) |
Is the expectation for |
I'm not sure about that. However, I do think that a method such as I also want to highlight that R3F is using the word Here's the normal meaning (it's on the MS DirectX docs but it's the same meaning for OpenGL):
The important thing to realize is that the viewport is not the same thing as the The canvas is used to position the viewport on the page, and normally (by default) the viewport is set to be the full size of the canvas. However, it doesn't have to be - you can use Also, the viewport is always specified using pixels as the unit (I don't know whether that's just a convention or if it's in a specs doc somewhere but I did some poking around and this always seems to be the case for OpenGL/WebGL/DirectX) - so, again, R3F specifying it in three.js units is non-standard. |
const setSize = useThree(state => state.setSize)
...
setSize(width, height) The problem here is that Canvas resizing is expensive as it can cause browser layout recalculation, while viewport resizing is essentially free. Here's a use-case for Then I open the sidebar and it will cover part of the canvas: What if I want to shrink down the scene as the sidebar opens? So I want to animate the viewport/scene size from |
ah yes, that was what i would have suggested but you're right, this will cause layout and setViewport is faster. this is the function that gets called when size/viewport changes (the state objects): https://github.com/pmndrs/react-three-fiber/blob/master/packages/fiber/src/core/store.ts#L335-L361 any ideas how that can be cleanly separated? maybe shifting this bit: // Update renderer
gl.setPixelRatio(viewport.dpr)
gl.setSize(size.width, size.height) one line up so that it's part of the closure that would allow users to bail out? // Do not mess with the camera if it belongs to the user
if (!camera.manual && !(internal.lastProps.camera instanceof THREE.Camera)) { that would be quite the breaking change but i guess manual is rarely used. but it could pour the entire thing into your own hands. though i would suggest v8. react 18 is available as a stable release candidate and v8 is only formally beta, it's probably more stable than v7 with all the bugfixes in there. btw, im sure this will also mess with events and raycasting. |
I see two issues here:
IMO it's best to keep that responsibility to the renderer and abstract over that if needed, but this just seems to be unfortunate naming if that's not the intent behind |
What's the resolution here? Can we use a name like |
That's fair, but it's not just OpenGL semantics to use viewport with this meaning. DirectX Viewport (previously linked above) Not sure if there are any other relevant APIs but in any case, what I'm getting at with the list is that "Viewport" has a specific meaning in computer graphics literature. Using it in a different sense, especially a similar but functionality different sense as R3F is currently doing, is confusing.
That seems like a reasonable option. |
I think we're in agreement, but maybe for different reasons. To clarify my comment, yes, all graphics APIs define a viewport, and the use of the term is consistent throughout. My concern is that they all define it differently, especially with coordinates, and R3F can't make any assumptions about the environment it's running in. This is why I bring up WebGL and WebGPU for WebGLRenderer and WebGPURenderer, respectively, but some renderers like SVGRenderer don't track or expose viewport information. For those reasons, I don't think we can fix the measurements in I do question the need for |
I'm not sure if that's true. Checking through the links I posted above, it's: The x and y offsets, in WebGL terms, are the offsets into the
Actually SVG does have a viewport although not sure if it's exposed through the three.js
However that highlights that viewport also has a meaning in the DOM and it's not the same as the graphics viewport. So it is an overloaded term when doing graphics on the web, unfortunately. I guess that is why SVG calls it a Anyway: the SVG element defines a viewport (web terminology), and the Likewise in WebGL the
There is a need to get the Suggestions:
My preference is 1 but it's more complex to implement so I understand if you guys prefer another option. |
It still doesn't look like I clarified why I bring this up for WebGL and WebGPU, specifically, but I'm going to drop it for the sake of closing this issue. I trust you don't mean to come across as contrarian or pedantic, but I find continued discussion in this regard distracting.
I doubt the usefulness of 1 since we cannot rely on |
Not in the slightlest. Apologies if I came across that way, it was unintended.
That's fine with me. |
@drcmda, it seems that the intention for the use of three units was to be able to create UI in threejs scaled to the container? If const size = useThree(state => state.size)
const { factor } = useThree(state => state.viewport)
<planeGeometry args={[size.width, size.height]} scale={factor} /> IIRC interface Viewport {
/** Container horizontal offset in pixels. */
left: number
/** Container vertical offset in pixels. */
top: number
/** Container width in pixels. */
width: number
/** Container height in pixels. */
height: number
/** Container viewport aspect ratio. */
aspect: number
/** Camera distance from the origin in world space. */
distance: number
/** Camera view-space / container viewport ratio. */
factor: number
} |
I'm looking to act on this in v9 (#2338), but I'm inclined to remove this property entirely and move the current behavior to a properly named helper method elsewhere. The current behavior measures in world-space which is wrong by convention. My previous comment looked to merge it with |
Since v9 has turned into a compatibility release for React 19, this breaking change will get bumped to v10. |
I added a note to the viewport docs to note the difference with the gl viewport: https://r3f.docs.pmnd.rs/api/hooks#state-properties @CodyJasonBennett should we move the refactor to its own issue? |
The expectation that viewport = drawing buffer size is incorrect for a graphics API or a library that sits between this and a view layer. I will acknowledge that the use of viewport is ambigious as it naturally extends a context, and that is enough reason to motivate a rename, but I worry it is overzealous to commit a breaking change over pedantry alone. My suggestion would be to remove this API from R3F and move it to Drei as a helper hook for various projections. Ideally, three.js would be receptive to such a helper class if it contains specialized projection math to drop use of camera matrices and speed up certain raycasts. |
I've set up a simple sandbox here - when you click it will toggle the active viewport between two states:
viewport = [0, 0, canvas width, canvas height]
viewport = [0, 0, half canvas width, half canvas height]
After toggling, the current viewport size reported by three.js and R3F are logged to the console:
As you can see, three.js updates to show the new size. R3F continues to report the viewport as the entire canvas.
I think the issue here is actually semantics - in R3F, the word "viewport" is being used to report various stats about the state of the
canvas
element combined with the dpr. However, three.js is reporting the value ofglViewport
.Since viewport usually has a specific meaning in 3d computer graphics - it's the size of the viewing rectangle into which you will output the scene, in pixels - I'm inclined to say the three.js usage of the term is more correct here.
The text was updated successfully, but these errors were encountered: