Fixing the lights, something does not look legit in the code #320
Description
I have some doubt about our lighting being correct. While doing #301 (merge lightmap/lightgrid world and entity lighting) I spent a lot of time reading our lighting code and I found something weird.
Let's explain:
This is lighting code for BSP surface with lightmap color and deluxemap direction like we did before and we will still do after #301.
// Compute light direction in world space from deluxe map.
vec4 deluxe = texture2D(u_DeluxeMap, var_TexLight);
vec3 lightDir = normalize(2.0 * deluxe.xyz - 1.0);
// Compute light color from world space lightmap.
vec3 lightColor = texture2D(u_LightMap, var_TexLight).rgb;
// Divide by cosine term to restore original light color.
lightColor /= clamp(dot(normalize(var_Normal), lightDir), 0.3, 1.0);
color.rgb = vec3(0.0);
// Blend static light.
computeLight(lightDir, normal, viewDir, lightColor, diffuse, material, color);
// Blend dynamic lights.
computeDLights(var_Position, normal, viewDir, diffuse, material, color);This is lighting code for Entity surface with lightgrid color and lightgrid direction like we did before and will still do after #301. This is also lighting code for BSP surface with lightgrid color and lightgrid direction after #301.
// Compute light direction in world space from light grid.
vec4 texel = texture3D(u_LightGrid2, lightGridPos);
vec3 lightDir = normalize(texel.xyz - (128.0 / 255.0));
// Compute light color from lightgrid.
vec3 ambientColor, lightColor;
ReadLightGrid(texture3D(u_LightGrid1, lightGridPos), ambientColor, lightColor);
color.rgb = ambientColor * r_AmbientScale * diffuse.rgb;
// Blend static light.
computeLight(lightDir, normal, viewDir, lightColor, diffuse, material, color);
// Blend dynamic lights.
computeDLights(var_Position, normal, viewDir, diffuse, material, color);This is lighting code for BSP surface with lightgrid color and without lightgrid direction neither deluxemap direction, this code was used before #301 to lit the map when lightmapping (and deluxe mapping) was disabled.
// Compute light color from lightgrid.
vec3 ambientColor, lightColor;
ReadLightGrid(texture3D(u_LightGrid1, lightGridPos), ambientColor, lightColor);
color.rgb = lightColor.rgb * diffuse.rgb;
// Blend dynamic lights.
computeDLights(var_Position, normal, viewDir, diffuse, material, color);So, what's the issue ? We see that:
- in first case, we assign
colorwith zero (pure black) and callcomputeLightwithcolor,diffuseandlightColor; - in second case, we assign
colorwith the result of some operation withdiffuseandambientColor, then callcomputeLightwithcolor,diffuseandlightColor; - in third case, we assign
colorwith the result of some operation withdiffuseandlightColor, then do not callcomputeLightwithcolor,diffuseandlightColor.
So, I have a strong feeling the second case is doing wrong things: it looks like either we compute color from diffuse and something else, either we set color to black and call computeLight. Doing both looks suspicious.
@gimhael, what's your opinion on this?