ViewportNode: Honor the renderer viewport

[WestLangley]

trafficstars

Description

The above image is from the webGPU MRT example, modified by setting a custom renderer viewport.

The original example uses this pattern:

const out = mix( output.renderOutput(), output, step( 0.2, viewportTopLeft.x ) );

I have been able to honor the viewport by using this pattern:

const offset = Fn( () => {

	return viewportTopLeft.x.mul( viewportResolution.x ).div( 2 ).sub( viewport.x ).div( viewport.z ); // 2 is the DPR for my device

} );

const out = mix( output.renderOutput(), output, step( 0.2, offset() ) );

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

[WestLangley]

ping @sunag Can you please have a look at this issue?

There is a lot of potential here!

[sunag]

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".

I'll rename it to viewportUV, the current name might not be as intuitive.

[WestLangley]

@sunag Can we agree on the units?

1. viewport: logical pixel units - the renderer's viewport is always in logical units
2. viewportResolution: physical or logical pixel units? the name is not clear what the units are
3. viewportCoordinate: I am not sure what you intend this to represent. It is currently normalized by something.
4. viewportTopLeft: should be unitless - in [0,1] -- the numerator and denominator must have the same units

[sunag]

All should be treated as logical pixel units with the exception of viewportUV.

1. viewport: logical pixel units - the renderer's viewport is always in logical units
2. viewportResolution: logical pixel units - width/height resolution.
3. viewportCoordinate: logical pixel units - current xy pixel position.
4. viewportTopLeft / viewportUV: should be unitless - in [0,1] -- screen space coordinates - viewportCoordinate.div( viewportResolution )

[WestLangley]

@sunag Thank you for the added clarity.

1. Good. xyzw -- relative to the top-left (or following webGL convention, bottom-left)?

2. How about renaming viewportResolution -> viewportSize. It should match viewport.zw.

3. viewportCoordinate is currently xyz. Do you intend to drop the z-coordinate? What is the value of the viewport coordinate in the upper-left corner of the viewport? Can it be fractional?

4. viewportUV - unitless in [0, 1] - relative to the top-left of the viewport? Are you going to remove the "top/bottom-left/right" ones?

[WestLangley]

@sunag Can you respond to my questions? I am eager to get to agreement.

[sunag]

1. xyzw -- relative to the top-left is great
2. Renaming viewportResolution -> viewportSize. I think it's good too, closer to what we use in the three.js API
3. viewportCoordinate is currently xyz -- I agree with drop the z-coordinate.
4. viewportUV - unitless in [0, 1] - relative to the top-left , let's remove "top/bottom-left/right"

Looks like a great review @WestLangley .

[WestLangley]

Do you want to have a go at these changes to ViewportNode? I will test it.

//

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 ViewportNode should too, so you might want to use the lower-left convention, instead.

[WestLangley]

@sunag Making progress! Still to do in ViewportNode:

1. viewportResolution -> viewportSize
2. Retain logical units throughout.
3. ViewportUV must be correct for non-full-sized viewports.
4. Agree on which convention to use: WebGL or WebGPU.

[sunag]

Agree on which convention to use: WebGL or WebGPU.

I can fix the convention in Nodes but that would not solve the use of the API in the Renderer when using WebGPURenderer.setViewport(). The WebGPU convention seems easier to use and we would advance a better standard for the next years. I know the idea of ​​backwards compatibility is strong, but could we change this item more and provide it in the migration guide? setViewport() is more common with advanced users but not everyone uses it, maybe we won't have a problem with it. @Mugen87 @mrdoob suggestions?

[WestLangley]

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 );

ref: WebGPU coordinate-systems

[Mugen87]

I vote for the WebGPU convention.

[mrdoob]

TopLeft being 0,0 sounds good to me. So I also vote for changing this to WebGPU convention 👍

[WestLangley]

This PR is still not resolved.

viewportUV.x must honor the viewport (compute the correct values) when the viewport is not full-sized.

// viewport
renderer.setViewport( window.innerWidth / 8, window.innerHeight / 4, window.innerWidth / 3, window.innerHeight / 2 );