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

WebXRManager: synchronize sizing with renderer #26905

Merged
merged 3 commits into from Nov 3, 2023

Conversation

CodyJasonBennett
Copy link
Contributor

@CodyJasonBennett CodyJasonBennett commented Oct 6, 2023

Fixed #21188.

Description

Ensures that WebGLRenderer.getSize and WebGLRenderer.getDrawingBufferSize report correctly when in a WebXR session.

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
652.2 kB (161.6 kB) 652.4 kB (161.7 kB) +245 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev Filesize PR Diff
444.9 kB (107.6 kB) 445.1 kB (107.7 kB) +245 B

@Mugen87 Mugen87 added this to the r158 milestone Oct 7, 2023
@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 7, 2023

@gkjohnson Does the PR also look good to you?

@gkjohnson
Copy link
Collaborator

gkjohnson commented Oct 16, 2023

Just for context - in my use case I was using the the dimensions stored internally on the left eye camera:

const xrCamera = renderer.xr.getCamera( camera );
const leftCam = xrCamera.cameras[ 0 ];
const width = leftCam.viewport.z;
const height = leftCam.viewport.w;
tiles.setResolution( xrCamera, width, height );

I assume the glProjLayer and glBaseLayer fields will give the same values, though.

I don't know if overwriting the WebGLRenderer pixel ratio and internal canvas size is what I would do, though. Isn't it the case that things like window resize and other dom sizing events can still fire while WebXR is enabled? These are functions where people set the canvas dimensions. If this happens the renderer size and pixel ratio would become out of sync.

Does it make sense to add getSize to renderer.xr? Then users can choose the appropriate size based on the current xr state:

let dpr = 1;
const resolution = new Vector2();
if ( renderer.xr.isPresenting ) {

  renderer.xr.getSize( resolution );
  dpr = 1;

} else {

  renderer.getSize( resolution );
  dpr = renderer.getPixelRatio();

}

@CodyJasonBennett
Copy link
Contributor Author

Resizing while presenting is currently blocked, but otherwise window events wouldn't come up for immersive sessions and only inline/emulator.

this.setSize = function ( width, height, updateStyle = true ) {
if ( xr.isPresenting ) {
console.warn( 'THREE.WebGLRenderer: Can\'t change size while VR device is presenting.' );
return;
}

I would expect WebGLRenderer.getDrawingBufferSize to align with the current drawing surface, and WebGLRenderer.getSize by extension. Although I wouldn't be opposed to XR-specific methods, it breaks that expectation.

@gkjohnson
Copy link
Collaborator

Resizing while presenting is currently blocked, but otherwise window events wouldn't come up for immersive sessions and only inline/emulator.

I didn't realize that - that behavior seems a bit odd to me. Is there an issue on why that was needed? What happens if the web browser is resized during an XR session (ie in desktop VR)? This must mean the canvas doesn't have the correct scale on exiting VR? Maybe I'm misunderstanding something.

I would expect WebGLRenderer.getDrawingBufferSize to align with the current drawing surface, and WebGLRenderer.getSize by extension

This isn't how it works currently, though, I don't feel? When the draw target is set to a WebGLRenderTarget, for example, getSize and setSize still return and set the canvas size rather than the render target.

If everyone else thinks this approach is best, though, then we should merge it!

@CodyJasonBennett
Copy link
Contributor Author

CodyJasonBennett commented Oct 17, 2023

I didn't realize that - that behavior seems a bit odd to me. Is there an issue on why that was needed? What happens if the web browser is resized during an XR session (ie in desktop VR)? This must mean the canvas doesn't have the correct scale on exiting VR? Maybe I'm misunderstanding something.

I'm tracing that back to d981806 of r86 which was before WebXR -- WebVR. Looks to be related to #13225.

This isn't how it works currently, though, I don't feel? When the draw target is set to a WebGLRenderTarget, for example, getSize and setSize still return and set the canvas size rather than the render target.

It is only when targeting the underlying drawing surface or the null target (related: #26160). In apps, this is measured for presentation purposes since the final output must be compatible (e.g. post-processing, upscaling). Viewport state is not reliable in this manner since it can and will change per draw call. The same is true for render targets which normally wouldn't correspond to the drawing surface whatsoever, but with layers the internal render target is a necessary implementation detail.

@gkjohnson
Copy link
Collaborator

This must mean the canvas doesn't have the correct scale on exiting VR?

I'm having trouble setting up my oculus at the moment to test due to some account issues - but I'm still curious about what happens when you resize the page and then exit VR.

In apps, this is measured for presentation purposes since the final output must be compatible (e.g. post-processing, upscaling).

I understand why this is necessary. I'm just suggesting that it could be the apps responsibility to measure or pick what it's target drawing surface is and adjust accordingly - which could be the canvas, a render target, or XR layer. If you actually can't adjust the canvas during an XR session then this implementation makes sense.

Either way this can be tested later or reevaluated if it causes an issue. Let's merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WebXR: Provide way to get XR viewport resolution when presenting
4 participants