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

LightProbeGenerator: WebGL error with gl.readPixels in some cases #26765

Closed
hybridherbst opened this issue Sep 15, 2023 · 7 comments · Fixed by #26773
Closed

LightProbeGenerator: WebGL error with gl.readPixels in some cases #26765

hybridherbst opened this issue Sep 15, 2023 · 7 comments · Fixed by #26773

Comments

@hybridherbst
Copy link
Contributor

Description

When trying to use the LightProbeGenerator there are some cases where it fails.

The error message is different on
Safari: Error: WebGL: INVALID_OPERATION: readPixels: pixels is not TypeUint16
Chrome: WebGL: INVALID_OPERATION: readPixels: type HALF_FLOAT but ArrayBufferView not Uint16Array

and the coefficients being returned are all 0.

Reproduction steps

Run the fiddle in Safari or Chrome
Note WebGL: INVALID_OPERATION: readPixels: pixels is not TypeUint16 error in the console

image

Code

new EXRLoader().load('https://dl.polyhaven.org/file/ph-assets/HDRIs/exr/1k/farm_sunset_1k.exr', reflection => { 
    const size = Math.min(reflection.image.width, 512);
    const target = new THREE.WebGLCubeRenderTarget(size);
    let rt = target.fromEquirectangularTexture(renderer, reflection);
    const sampledProbe = LightProbeGenerator.fromCubeRenderTarget(renderer, rt);
    console.log("spherical harmonics", sampledProbe.sh);
});

Live example

https://jsfiddle.net/tkgobqnc/20/

Screenshots

No response

Version

r156

Device

Desktop

Browser

Chrome, Firefox, Safari, Edge

OS

MacOS

@hybridherbst hybridherbst changed the title Error on Safari with gl.readPixels in some cases LightProbeGenerator: WebGL error with gl.readPixels in some cases Sep 15, 2023
@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 15, 2023

Looking at the code, LightProbeGenerator.fromCubeRenderTarget() does not support half float cube render targets as inputs yet. Only UnsignedByteType works.

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 16, 2023

I've hacked in half float support but I don't think the result is correct. The sphere is now very bright. https://jsfiddle.net/wtbe3pug/3/

@WestLangley Does the light probe implementation support HDR? Or are the irradiance color values expected to be in the [0,1] range?

@hybridherbst
Copy link
Contributor Author

hybridherbst commented Sep 16, 2023

Thanks! I think brightness-wise it doesn't look too bad - given that this is a high-dynamic range source texture the results would be expected to be brighter too.

E.g. here's a similar thing on the modelviewer.dev page with a comparison of the same environment as HDR and converted to LDR:
https://modelviewer.dev/examples/lightingandenv/#anotherHDRExample
https://modelviewer.dev/examples/lightingandenv/#ldrEnvironmentImage

I'm not super sure about the probe result though;
I removed the extra lights and colors so that this is more comparable, and used the same image as in the model-viewer example:

https://jsfiddle.net/rh3t19pq/15/

Probe Result:
https://github.com/mrdoob/three.js/assets/2693840/ae71e6e7-a74c-47ed-9043-ae54acdf2673

Source Texture:
Screenshot 2023-09-16 at 13 12 26

(but that may be an entirely unrelated issue)

@WestLangley
Copy link
Collaborator

I've hacked in half float support but I don't think the result is correct. The sphere is now very bright. https://jsfiddle.net/wtbe3pug/3/

I edited your fiddle and highlighted my changes, adding tone mapping and exposure control: https://jsfiddle.net/793es8jq/

This very closely matches the irradiance estimated by PMREM, so it looks good to me.

Does the light probe implementation support HDR?

Your hack looks good. File a PR and we can verify.

@WestLangley
Copy link
Collaborator

@hybridherbst

In light of my previous comments: #18371 (comment) and #18371 (comment) ...

... I am curious as to your proposed workflow. How do you intend to use the HDR-generated light probe{s} in your app? What is the workflow we need to support?

@hybridherbst
Copy link
Contributor Author

I'm aware that envMap basically applies the same effect and this shouldn't be combined. The usecase for us is that there are some custom shaders that differentiate between "diffuse environment lighting" and "reflection response" and for those, the shader takes spherical harmonics for diffuse lighting and an environment map for reflections. This is relatively similar to how light probes work in Unity. Hope that makes sense!

@WestLangley
Copy link
Collaborator

WestLangley commented Sep 16, 2023

I expect LightProbeGenerator would have to be enhanced to properly accommodate HDR inputs and to prevent "ringing".

An alternative is to use low-res cube maps for your irradiance probes.

three.js support for cube-map-based light probes is also an option.

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.

3 participants