Dynamic lights in linear colorspace

[illwieckz]

In computeLight_fp.glsl in computeDynamicLight() function we have this code:

if( light.type == 0.0 ) {
	// point light
	L = light.center.xyz - P;
	// 2.57 ~= 8.0 ^ ( 1.0 / 2.2 ), adjusted after overbright changes
	float t = 1.0 + 2.57 * length( L ) / light.radius;
	// Quadratic attenuation function instead of linear because of overbright changes
	attenuation = 1.0 / ( t * t );
	L = normalize( L );
} else if( light.type == 1.0 ) {
	// spot light
	L = light.center - P;
	// 2.57 ~= 8.0 ^ ( 1.0 / 2.2 ), adjusted after overbright changes
	float t = 1.0 + 2.57 * length( L ) / light.radius;
	// Quadratic attenuation function instead of linear because of overbright changes
	attenuation = 1.0 / ( t * t );
	L = normalize( L );

	if( dot( L, light.direction ) <= light.angle ) {
		attenuation = 0.0;
	}
}

The comments suggest the code was modified from using a linear attenuation function to a quadratic attenuation once the complete overbright had been unlocked.

That may be wrong because if the light was not properly displayed with a not-clamped overbright, it means that maybe the values were wrong to begin with and that clamping was hiding the bug.

But my main concern isn't that. On this page we can read:

Something else that's different with gamma correction is lighting attenuation. In the real physical world, lighting attenuates closely inversely proportional to the squared distance from a light source. In normal English it simply means that the light strength is reduced over the distance to the light source squared, like below:

float attenuation = 1.0 / (distance * distance);

However, when using this equation the attenuation effect is usually way too strong, giving lights a small radius that doesn't look physically right. For that reason other attenuation functions were used (like we discussed in the basic lighting chapter) that give much more control, or the linear equivalent is used:

float attenuation = 1.0 / distance;  

The linear equivalent gives more plausible results compared to its quadratic variant without gamma correction, but when we enable gamma correction the linear attenuation looks too weak and the physically correct quadratic attenuation suddenly gives the better results. The image below shows the differences:

The cause of this difference is that light attenuation functions change brightness, and as we weren't visualizing our scene in linear space we chose the attenuation functions that looked best on our monitor, but weren't physically correct. Think of the squared attenuation function: if we were to use this function without gamma correction, the attenuation function effectively becomes: (1.0/distance²)²·² when displayed on a monitor. This creates a much larger attenuation from what we originally anticipated. This also explains why the linear equivalent makes much more sense without gamma correction as this effectively becomes (1.0/distance)²·² = 1.0/distance²·² which resembles its physical equivalent a lot more.
-- https://learnopengl.com/Advanced-Lighting/Gamma-Correction

So, if I understand it correctly, the light attenuation function should be currently linear, it should be linear when loading all our Tremulous and current Unvanquished maps, and it should be quadratic only when loading the future maps that will be processed in linear space.

That code already using the quadratic function means we can't convert it from linear to quadratic…

[illwieckz]

To add to the topic, using linear attenuation for real time lights is the usually accepted workaround for blending a light in a non-linear colorspace, it is wrong but using this trick makes possible to keep the same light value in both linear and non-linear colorspace, by using a wrong linear attenuation in non-linear colorspace, and a correct quadratic attenuation in linear colorspace.

Some games used such workaround with the non-linear colorspace, like Wolfenstein: Enemy Territory. It used the -wolf q3map2 option that tells q3map2 to do linear attenuation when raytracing the lightmap, while quake3 used the default behaviour (also accessible through the -q3 q3map2 option) of quadratic attenuation.

• Quake 3 was doing quadratic attenuation (correct) in non-linear space (not correct) so the horror is visible in plain sight.
• Wolf:ET was doing linear attenuation (not correct) in non-linear space (not correct) so this hides a bit the horror under the bed.
• Xonotic is doing quadratic attenuation (correct) in linear space (correct).

[illwieckz]

So it looks like our engine was doing the same workaround as wolf:et for dynamic lights, now we are doing as bad as quake3 q3map2 profile.

[illwieckz]

---

---

[slipher]

I guess the linear 1/r thing kind of makes sense since supposing srgb2linear(x) = x^2.2, then srgb2linear(1/r) = 1/r^2.2 which is approximately 1/r². Maybe 1/r^0.9 would be even closer.

Assuming we go with the route of using one of 2 possible global color spaces, determined by the map, it seems like going back to overbright clamping being on by default would help solve some problems. Then we could use 1/r light attenuation in non-sRGB mode (per Reaper's code comment quoted in the OP that full overbright worked poorly with 1/r lights), and we could enable bloom without issues. Since the only sRGB maps that exist will be newly built ones, we can have those specify full-range overbright in the worldspawn.

[VReaperV]

We just need to add HDR->LDR tonemapper, since we already have the HDR buffers, but currently we're just clamping the values to [0.0, 1.0]:

Daemon/src/engine/renderer/glsl_source/cameraEffects_fp.glsl

Line 45 in 9352952

color = clamp(color, 0.0, 1.0);

Which effectively nullifies this advantage of using HDR buffers.

I have a branch with a Lottes tonemapper + adaptive lighting, I'll rebase it on master and add a pr for it a bit later. Just need to clean it up and add a macro or uniform to check for HDR buffer usage.

[illwieckz]

Which effectively nullifies this advantage of using HDR buffers.

The current advantage of using HDR buffers is to not lose precision when blending multiple stages.

Using HDR buffers may have other advantages that may be nullified by such clamping, but this blending advantage is not nullified by clamping the output.

No HDR buffers, clamping:

HDR buffers, clamping:

Here with a slider: https://imgsli.com/MzQ3ODg4

[illwieckz]

it seems like going back to overbright clamping being on by default would help solve some problems.

Overbright clamping never was a feature, and the implementation we had was a bug and a regression.

[VReaperV]

The current advantage of using HDR buffers is to not lose precision when blending multiple stages.

Using HDR buffers may have other advantages that may be nullified by such clamping, but this blending advantage is not nullified by clamping the output.

Yeah, I'm talking about the HDR->LDR conversion for displaying.

[VReaperV]

There is now an HDR->LDR tonemapper implementation in #1550. It fixes the dynamic lights being clamped as well:
r_overbrightDefaultClamp off:

r_overbrightDefaultClamp off; r_tonemap on:

r_overbrightDefaultClamp off:

r_overbrightDefaultClamp off; r_tonemap on:

The code is actually still wrong, but for different reasons: the attenuation was apparently already quadratic (since later the code calls computeDeluxeLight() with attenuation * attenuation * light.color as the light colour), which I missed when I made that pr.
However, here's what happens if it's changed back to being 1 / t:
r_overbrightDefaultClamp off:

r_overbrightDefaultClamp off; r_tonemap on:

r_overbrightDefaultClamp off:

r_overbrightDefaultClamp off; r_tonemap on:

If the 2.57 in float t = 1.0 + 2.57 * length(L) / center_radius.w is changed back to 8.0:
r_overbrightDefaultClamp off:

r_overbrightDefaultClamp off; r_tonemap on:

r_overbrightDefaultClamp off:

r_overbrightDefaultClamp off; r_tonemap on:

All of those other ones still look wrong.

[VReaperV]

Also note that #1425 NUKES the random dynamic light scaling, so any changes made to dynamic lights should take that into account.

[VReaperV]

Actually, the last 2 screenshots are nearly the same as current with tonemapping:
Current:

With the dynamic lighting changes reverted:

However, without tonemapping reverting the changes still makes it look too bright:
Current:

With the dynamic lighting changes reverted:

[VReaperV]

I'm not sure why those random multipliers and additions in shaders were even there in the first place, maybe we should just NUKE those while making sure that dynamic lights we have in assets work correctly.