mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
102.66k stars 35.37k forks source link

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

Closed hybridherbst closed 1 year ago

hybridherbst commented 1 year ago

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

Mugen87 commented 1 year ago

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

Mugen87 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@hybridherbst

In light of my previous comments: https://github.com/mrdoob/three.js/pull/18371#issuecomment-1718696634 and https://github.com/mrdoob/three.js/pull/18371#issuecomment-1720140429 ...

... 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 commented 1 year ago

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 commented 1 year ago

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.