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

Fix WebXR Darkness and Non-Stencil Invalid GL Layer #22918

Merged
merged 1 commit into from
Nov 29, 2021

Conversation

HexaField
Copy link
Contributor

Related issue: #22558

Description

This contribution is funded by XRFoundation.

@mrdoob mrdoob added this to the r136 milestone Nov 29, 2021
@mrdoob mrdoob merged commit b63ffff into mrdoob:dev Nov 29, 2021
@mrdoob
Copy link
Owner

mrdoob commented Nov 29, 2021

Thanks!

@HexaField
Copy link
Contributor Author

I have a follow up I was just about to push that sets encoding to renderer,outputEncoding, right now this is somewhat limited. I will create a follow up PR unless you want to unmerged this? Sorry, just realised it.

@mrdoob
Copy link
Owner

mrdoob commented Nov 29, 2021

A new PR is ok 👍

@cabanier
Copy link
Contributor

It seems that the WebXR output is too light with this change.
For instance, ballshooter is much lighter in VR than it is in 2D on both the Oculus browser and desktop mac.

Maybe we should change the PR so we let authors decide if they want to use sRGB encoding.
Also, this code introduces a bug with multisampled renderbuffers because the format of the WebXR texture is still sRGB. This will cause an error on devices that would use that path. (AFAIK there are none at the moment)

@HexaField
Copy link
Contributor Author

It seems that the WebXR output is too light with this change. For instance, ballshooter is much lighter in VR than it is in 2D on both the Oculus browser and desktop mac.

Maybe we should change the PR so we let authors decide if they want to use sRGB encoding. Also, this code introduces a bug with multisampled renderbuffers because the format of the WebXR texture is still sRGB. This will cause an error on devices that would use that path. (AFAIK there are none at the moment)

A follow PR has been merged already that pulls the output encoding from the WebGLRenderer passed in.
#22919

@cabanier
Copy link
Contributor

cabanier commented Dec 17, 2021

A follow PR has been merged already that pulls the output encoding from the WebGLRenderer passed in. #22919

Ah, yes, this is better.
There is still the issue that sRGB encoding will break browsers that don't implement WEBGL_multisampled_render_to_texture. You can reproduce this by setting hasMultisampledRenderToTexture to false.
To fix this, we need to set the texture format of the WebXR projection layer to sRGB

@mrdoob
Copy link
Owner

mrdoob commented Dec 18, 2021

Didn't #22997 fix that? 🤔

@cabanier
Copy link
Contributor

cabanier commented Dec 18, 2021

Didn't #22997 fix that? 🤔

No, the texture in the projection layer is always created as RGB.

See https://github.com/mrdoob/three.js/blob/dev/src/renderers/webxr/WebXRManager.js#L284

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.

3 participants