mrdoob / three.js

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

Add intensity to AmbientLight? #7620

Closed bhouston closed 8 years ago

bhouston commented 8 years ago

I'd like to add intensity to AmbientLight. There is no real reason why we are currently limiting the AmbientLight to be within the range of 0-1 for RGB, technically with tone mapping we could handle any arbitrary value.

I would add it and default it to 1.0 for perfect backwards compatibility.

WestLangley commented 8 years ago

That is fine. Something like this would result, I presume.

vec3 getAmbientLightIrradiance( const in vec3 ambientLightColor, const in float ambientLightIntensity ) {

    return PI * ambientLightColor * ambientLightIntensity;

}
bhouston commented 8 years ago

We can just pre-scale the uniform ambientLightColor. A separate intensity/color is generally only required for UI purposes (and sometimes persistence if one is using a color hex) as it is hard for users to specify colors outside of the range 0-1 for RGB. Color pickers are generally just in the range of 0-1 RGB thus you couple it with an intensity.

But we can combine both color and intensity in the WebGLRenderer while updating the uniforms like I believe we do for the other lights.

bhouston commented 8 years ago

So to be clear, no changes required in the glsl code.

bhouston commented 8 years ago

BTW separate issue, I am unsure why we need to multiply ambient light by PI to get the irradiance? What unit is ambientLightColor/ambientLightIntensity in?

bhouston commented 8 years ago

If we were to make breaking changes to the lights, I'd advocate remove the PI terms from Hemisphere, and Ambient because it makes more sense to me that users set their irradiance directly (via color + intensity), rather than something that gets multiplied by PI.

WestLangley commented 8 years ago

Thinking out loud...

A hemisphere light has 2 colors. The sky is one color and the ground is the other. The hemisphere light code computes the irradiance. The computed irradiance changes with the polar angle. So in that case, the user does not set the irradiance -- the user is setting the radiance of the sky and ground via the color and intensity.

An ambient light is a hemisphere light with both colors the same, so the same logic holds.

BTW, a hemisphere light should produce the same diffuse lighting as an environment map of the same colors. Again, it is the radiance that is set.

An irradiance map is a different issue, but even there, I think the factor of PI is removed and applied later. This allows the irradiance to remain in the [0, 1] range. (I think that is how it is done.)


We need to come to an agreement as to what are the units of color and of intensity. Frostbite seems to think of color as a hue (unit-less, I think) and the units come from the intensity parameter. I, too, have always thought of color as unit-less for us.

If you want to switch to the Frostbite convention, then intenisty is in lumens or Lux, and that is not the usual units of intensity.

bhouston commented 8 years ago

This allows the irradiance to remain in the [0, 1] range.

I can not thing of a reason why irradiance should remain within the range 0-1. I think its valid range is 0 to infinity.

Frostbite seems to think of color as a hue (unit-less, I think) and the units come from the intensity parameter.

The separation of color and intensity is just for convenience. Color pickers are LDR, low dynamic range, and limited to 0-1 for the RGB channels. This isn't sufficient for lights that can have larger values than is possible in LDR. Lights can be infinitely bright. Thus to get that, we multiply a color by an intensity. The separation is a user convenience, it is only together that they really make sense as a value, because RGB( 0.1, 0.1, 0.1 ) * intensity (10 ) === RBG( 1, 1, 1 ) * intensity( 1 ) -- the decomposition isn't even well defined if you go backwards.

WestLangley commented 8 years ago

This allows the irradiance to remain in the [0, 1] range.

I meant when encoded in an RGB LDR map. I think max irradiance is PI if max color is 1, so PI is divided out before storing the value in the map.

bhouston commented 8 years ago

If you want to switch to the Frostbite convention, then intenisty is in lumens or Lux, and that is not the usual units of intensity.

Actually the standard unit for defining a light's power is lumens in Frostbite, but it has to be set as its "power" (remember I added the "poiwer" setter/getter in the other PR?) not as its "intensity" (which is lumens/solid angle, e.g. candela). Frostbite actually does it right I believe.

bhouston commented 8 years ago

I meant when encoded in an RGB LDR map.

It is super hard for me to determine what an LDR map stores because I think they are generally incorrect. They need to be HDR as Clara.io does. Once it is HDR I believe what it stores is dependent upon what type of map it is.

An preconvolved irradiance map stores irradiance directly -- because it is preintegrated, although whether it requires a PI scale depends upon how it was calculated. I believe that normally one does not need to scale a preconvolved irradiance map by PI.

An image-based light, e.g. the input to an irradiance convolution tool, I believe should store the data as candela because it is a measure of lumens per solid angle.

bhouston commented 8 years ago

An image-based light, e.g. the input to an irradiance convolution tool, I believe should store the data as candela because it is a measure of lumens per solid angle.

But I should say that nearly any IBL maps are properly calibrated to reality. Thus they are a measure of candala (lumen per solid angle) with an arbitrary and unknown scale factor applied.

WestLangley commented 8 years ago

So an an AmbientLight for us is an encoding of a radiance map of a single color. Spherical harmonics paramers also encode radiance. (Note we have a separate getIrradiance() method for SH, too.) And as mentioned, HemisphereLight encodes radiance.

That is a summary of how we are doing it now for the reasons explained above.

WestLangley commented 8 years ago

An preconvolved irradiance map stores irradiance directly -- because it is preintegrated, although whether it requires a PI scale depends upon how it was calculated. I believe that normally one does not need to scale a preconvolved irradiance map by PI.

Could be. I'll have to let you decide based on what you see in the industry. : - )

WestLangley commented 8 years ago

But I should say that nearly any IBL maps are properly calibrated to reality. Thus they are a measure of candala (lumen per solid angle) with an arbitrary and unknown scale factor applied.

I have always been operating under the assumption that IBLs store radiance -- which in photometric terms is equivalent to luminance. At least that assumption is consistent with the math we are using.

bhouston commented 8 years ago

The original IBL paper is radiance as well: http://www.pauldebevec.com/Research/IBL/debevec-siggraph98.pdf Thus we have a set of lights that are defined in radiance (IBL, Hemisphere, Ambient) and a set of lights (Point, Spot and soon Area) that are defined in terms of power/intensity.

BTW Wikipedia says that radiance used to be called "intensity", https://en.wikipedia.org/wiki/Radiance so keeping an intensity value on all lights makes sense, either its modern interpretation or its historical interpretation.

Last question, what metric does a directional light use for intensity? I think it should be radiance, although a hemispheric integration I believe isn't valid in that case for the conversion to irradiance. Which means we may have a bug? I guess I do not know how to integrate a single direction of radiance into irradiance on a surface.

bhouston commented 8 years ago

We need to add these metrics to the docs.

bhouston commented 8 years ago

I guess a DirectionalLight is a weird case. It could be viewed as a point light situated at infinity that has no falloff. In that case I guess we are specifying it as "irradiance" directly? Interesting. I guess that works.

bhouston commented 8 years ago

I've updated the docs in this commit to include the units for each light: https://github.com/bhouston/three.js/commit/4b6919a535b4895581e3674a01ab1a6a13a2f09d

WestLangley commented 8 years ago

Last question, what metric does a directional light use for intensity? I think it should be radiance, although a hemispheric integration I believe isn't value in that case for the conversion to irradiance. Which means we have a bug.

DirectionalLights have units of radiance. They are punctual lights that subtend an infinitely small solid angle. We still do the integral over the hemisphere, it is just that the integrand is a Dirac-delta function. This changes units from radiance to irradiance. For us, by definition, they generate an irradiance of PI * color * intensity * dotNL.

We should put the metrics in the docs after we switch to physical units. Currently, we are just trying to keep the units consistent. Besides, I understand you want to switch to photometric units soon.

bhouston commented 8 years ago

MentalRay (the renderer for Maya and 3DS Max) Directional Light uses lux (irradiance) as the units if you want to set its directional light results directly: http://download.autodesk.com/us/maya/2011help/mr/shaders/architectural/arch_sunsky.html

Architectural guys are requesting that UE4 allow them to specify Lux for their Directional/Sun lights: https://forums.unrealengine.com/showthread.php?83550-Lighting-Units&highlight=lux

Given that we just return the direction light intensity * color as the irradiance here without multiplying it by PI:

https://github.com/mrdoob/three.js/blob/dev/src/renderers/shaders/ShaderChunk/lights_pars.glsl#L26

I sort of think we are considering our directional lights to be specified in terms of irradiance / lux -- just like what other people want.

Interestingly, Unity3D and MentalRay and V-Ray do not have true directional lights, rather they have Sun lights that automatically incorporate haze and other physical parameters and it generates physically correct intensity results for a sun. Might be something we should explore as either a new light type SunLight (or course as a separate PR.)

WestLangley commented 8 years ago

Yes, my comments about DirectionalLights are based on what we do now, and what our mathematics is implying. Our current lights do not have real units, of course.

I have been studying the Frostbite paper to understand their conventions, and it seems OK to me to use whatever units you feel are most commonly used in the community.


I would like to get away from using the variable color for radiance/luminance or irradiance/illuminance in the code, and create new properties to hold those quantities. So, on the JS side, if we pass color * intensity to the GPU, we name it appropriately.

One other thing. The only thing that changes between materials is the BRDFs, so we do not need to recode the rendering equation for each material. The rendering equation can be specified in the template only. I think a change like that would make the GLSL easier to understand.

I think you and I are on the same page... We just need to sort out the units properly. : - )

makc commented 8 years ago

wow such discussion much science wow speaking as a user, it was always counter-intuitive to me why there is no intensity in ambient light, while other lights do have it. it is way easier to play with single float parameter to change light intensity rather than go to photoshop, drag the handle a bit, copy hex value and paste it back to code every time.