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

darker textures in WebxrSession in latest dev git #23278

Closed
arpu opened this issue Jan 19, 2022 · 10 comments · Fixed by #23288
Closed

darker textures in WebxrSession in latest dev git #23278

arpu opened this issue Jan 19, 2022 · 10 comments · Fixed by #23288

Comments

@arpu
Copy link

arpu commented Jan 19, 2022

darker textures in WebxrSession in latest dev git

To Reproduce

Steps to reproduce the behavior:
https://raw.githack.com/mrdoob/three.js/dev/examples/?q=webxr#webxr_vr_ballshooter

Screenshots

Bildschirmfoto von 2022-01-19 18-35-50

Platform:

  • Device: [Desktop]
  • OS: [Linux, Android]
  • Browser: [Chrome]
  • Three.js version: [dev, 3280feb]
@arpu
Copy link
Author

arpu commented Jan 19, 2022

revert 63f7beb fixed the problem

@cabanier
Copy link
Contributor

@Mugen87 Is it because the default framebuffer for WebXR isn't null?

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 19, 2022

The mentioned line only influences the output encode in the shader. And it is only required when rendering to the default framebuffer since WebGL does not allow to define its color space. If a render target is set, the output encode is disabled.

When using a sRGB render target, the shader writes linear values and WebGL should automatically convert to sRGB. Is the default framebuffer for WebXR a sRGB framebuffer?

@cabanier
Copy link
Contributor

It depends on the output encoding. See https://github.com/mrdoob/three.js/blob/dev/src/renderers/webxr/WebXRManager.js#L284

It's needed there because the framebuffer that is created for multisampling, is created with that encoding. Maybe it shoudn't in case of the default framebuffer?

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 19, 2022

Maybe it shoudn't in case of the default framebuffer?

Can we give this a try?

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 20, 2022

If we can't fix this in WebXRManager, we have to add a flag to the XR render targets so we can identify them in WebGLRenderer and WebGLPrograms. It would be then possible to restore the old behavior which means adding back the missing GLSL linear-sRGB conversion (the reason why the image is too dark).

Not hard to implement but a bit hacky because it would be better to let WebGL handle the color space conversion if possible.

@cabanier
Copy link
Contributor

WebXR Layers do allow you to specify the encoding. However, the oculus implementation has some bugs in this area that I'm trying to fix. For now, you can assume that you always write linear values.
However, the implementation of three before I added multisample_render_to_texture rendered to a framebuffer backed by renderbuffers which was then blitted to a non-multisample WebXR texture. This did gamma correction which resulted in lighter colors that some people preferred. I suspect that is where @HexaField's original bug and sRGB changes to the WebXRManager came from.

If we can't fix this in WebXRManager, we have to add a flag to the XR render targets so we can identify them in WebGLRenderer and WebGLPrograms.

I think that's what we need to do. Ideally, I fix the bugs in our WebXR implementation so three doesn't need this special behavior.

@mrdoob
Copy link
Owner

mrdoob commented Jan 20, 2022

which resulted in lighter colors that some people preferred

In the world of PBR 3D, it's not about "preference"... it's about "correctness" 🤓

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 20, 2022

I've filed a PR to restore the old behavior.

I agree with @mrdoob that we should head for the correct solution which means a proper sRGB workflow. If colors are perceived as too dark, devs have to update their lighting.

However, it's not clear to me whether the sRGB workflow in WebXR is right and the colors as they should be. I did not have the chance so far to test this in detail with a Quest 2.

@cabanier
Copy link
Contributor

The issue is that there is no correct behavior defined in the spec.

The Oculus browser sets the primaries to sRGB and the gamma correction should come from the texture type. However, the gamma correction seems to be incorrectly applied inside the compositor. I'm working with that team to get to the bottom of this.
Once the workflow is fixed/clarified, I will update the spec and our implementation.

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

Successfully merging a pull request may close this issue.

4 participants