HemisphereLightProbe intensity wrong

[OndrejSpanel]

Description

When not using legacy lighting model, the HemisphereLightProbe intensity is wrong, the light created this way is about 3 times (or pi times, this is hard tell) brighter than it should be.

Reproduction steps

1. create a simple scene with HemisphereLight with intensity 3
2. display the scene
3. replace the light with HemisphereLightProbe
4. display the scene and notice it is much brighter
5. set HemisphereLightProbe intensity to 1
6. display the scene and notice it is the original brightness
7. use useLegacyLights = true
8. set HemisphereLightProbe intensity to 3
9. display the scene and notice it is the original brightness

Code

// Initialize scene, camera, and renderer
const scene = new THREE.Scene();
const camera = new THREE.PerspectiveCamera(75, window.innerWidth / window.innerHeight, 0.1, 1000);
const renderer = new THREE.WebGLRenderer();
renderer.setSize(window.innerWidth, window.innerHeight);
document.body.appendChild(renderer.domElement);

renderer.useLegacyLights = false;
// Add HemisphereLight
const hemiLight = new THREE.HemisphereLightProbe(0xffffff, 0x444444, 3);
hemiLight.position.set(0, 20, 0);
scene.add(hemiLight);

// Add a cube
const geometry = new THREE.BoxGeometry();
const material = new THREE.MeshStandardMaterial({ color: 0x00ff00 });
const cube = new THREE.Mesh(geometry, material);
scene.add(cube);

camera.position.z = 5;

// Animation loop
const animate = function () {
  requestAnimationFrame(animate);
  cube.rotation.x += 0.01;
  cube.rotation.y += 0.01;
  renderer.render(scene, camera);
};

animate();

Live example

• https://jsfiddle.net/OndrejSpanel/tc8L4keb/2/

Screenshots

Note the cube being so bright it looks flat:

Version

r155

Device

Desktop

Browser

Chrome

OS

Windows

[Mugen87]

It seems the issue is explained in these comments:

three.js/src/lights/AmbientLightProbe.js

Line 14 in 9272367

// without extra factor of PI in the shader, would be 2 / Math.sqrt( Math.PI );

three.js/src/lights/HemisphereLightProbe.js

Line 19 in 9272367

// without extra factor of PI in the shader, should = 1 / Math.sqrt( Math.PI );

@WestLangley Should we migrate the computations to the new default lighting mode without supporting the legacy one?

[OndrejSpanel]

By the way, how are the c0 and c1 chosen? I see the code origin is in #16223, but I did not find references to any sources. Seeing the numbers contain pi, I guess there must be some explanation.

If one would like to create similar coefficients for e.g. directional light, how would one proceed?

[Mugen87]

I agree it would be good to document how the coefficients of both light probe types are computed.

BTW: I'm not sure this was ever discussed but do we actually need HemisphereLightProbe and AmbientLightProbe at all? They produce the same result as HemisphereLight and AmbientLight but are more expensive to evaluate. I don't think we should keep them just as a demonstration of how the LightProbe class can be used.

[WestLangley]

@OndrejSpanel Yes, this update was overlooked. Thanks.

@Mugen87 asked

do we actually need HemisphereLightProbe and AmbientLightProbe at all?

Hmm... Does @donmccurdy need them as optional input to his LightProbeVolume concept?

[OndrejSpanel]

are more expensive to evaluate.

Are they? They always contribute to the one common light probe. The function getLightProbeIrradiance is called unconditionally in the shader chunk lights_fragment_begin, while hemisphere light is conditional on NUM_HEMI_LIGHTS.

I would therefore say using HemisphereLightProbe has better performance than using HemisphereLight. On the other hand, I do not see why there should be a dedicated class for it, as it does not provide anything but the constructor. A helper function computing SH coefficients would be enough.

[WestLangley]

HemisphereLightProbe and AmbientLightProbe are a poor-man's model for global illumination. Proper SH coefficients can be baked from an environment map.

Assuming these two classes are not needed as inputs to the LightProbeVolume class, I no longer see a use case for them. At the time, they were useful to me as test cases.

[Mugen87]

The function getLightProbeIrradiance is called unconditionally in the shader chunk lights_fragment_begin, while hemisphere light is conditional on NUM_HEMI_LIGHTS.

This is a bug to me. getLightProbeIrradiance() should only be executed if there are actually probes in the scene. This can easily be fixed with a new define (maybe USE_LIGHT_PROBES).

In any event, the computations of getLightProbeIrradiance() are more expensive than what getHemisphereLightIrradiance() does. You would need a quite a lot of hemisphere lights in your scene graph to change this (which I don't think is a realistic use case).

If @donmccurdy doesn't need HemisphereLightProbe and AmbientLightProbe for LightProbeVolume (which I believe is true), I vote to remove them.

[donmccurdy]

I'm fine with whatever you prefer here — it's been a while since I've been working on the LightProbeVolume idea, and I don't think removing Hemisphere or Ambient light probes would hurt that effort.