-
-
Notifications
You must be signed in to change notification settings - Fork 35.3k
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
ViewportNode: Honor the renderer viewport #29162
Comments
ping @sunag Can you please have a look at this issue? There is a lot of potential here! |
I'll rename it to |
@sunag Can we agree on the units?
|
All should be treated as logical pixel units with the exception of
|
@sunag Thank you for the added clarity.
|
@sunag Can you respond to my questions? I am eager to get to agreement. |
Looks like a great review @WestLangley . |
Do you want to have a go at these changes to // Note that, as you have implemented it, a viewport in WebGPURenderer follows the WebGL (lower-left) convention, not the WebGPU (upper-left) convention. I expect |
@sunag Making progress! Still to do in
|
I can fix the convention in Nodes but that would not solve the use of the API in the Renderer when using |
tldr; If we choose to follow the WebGPU convention for WebGPURenderer, then the following line will set the viewport in the top-left, instead of the bottom-left. renderer.setViewport( 20, 20, insetWidth, insetHeight ); |
I vote for the WebGPU convention. |
TopLeft being 0,0 sounds good to me. |
This PR is still not resolved.
// viewport
renderer.setViewport( window.innerWidth / 8, window.innerHeight / 4, window.innerWidth / 3, window.innerHeight / 2 ); |
Description
The above image is from the webGPU MRT example, modified by setting a custom renderer viewport.
The original example uses this pattern:
I have been able to honor the viewport by using this pattern:
It is a red flag that it is necessary to inject the device DPR (2) into the formula. It is a warning that the units on the TSL functions are not correct. Some are scaled by DPR -- others are not... Or maybe the functions are fine, but the names can be improved.
Also,
viewportTopLeft
is not the top-left of the viewport. It is the fragment coordinate normalized by some quantity -- likely by the width of the frame buffer. Perhaps the nomenclature can be improved. Something that implies: "Normalized fragment coordinate relative to the top-left of the viewport".Reproduction steps
See above.
Code
See above.
Live example
n/a
Screenshots
No response
Version
r168dev
Device
Desktop
Browser
Chrome
OS
MacOS
The text was updated successfully, but these errors were encountered: