Correct shade mapping when light pos is all zeros #11567
Conversation
Was using invalid values when lighting was off.
Takes care of hrydgard#4155 for software rendering.
May need to audit more normalize() usage, if it's consistent in other places.
| }; | ||
| auto calcShadingLPos = [&](int l) { | ||
| Vec3f pos = getLPos(l); | ||
| if (pos.Length() == 0.0f) { |
There was a problem hiding this comment.
Not that it's important, but you could save a square root here by using Length2() instead (which is simply length without the square root).
There was a problem hiding this comment.
Fair point, didn't think about that.
-[Unknown]
| @@ -715,7 +715,11 @@ void GenerateVertexShaderHLSL(const VShaderID &id, char *buffer, ShaderLanguage | |||
| break; | |||
|
|
|||
| case GE_TEXMAP_ENVIRONMENT_MAP: // Shade mapping - use dots from light sources. | |||
| WRITE(p, " Out.v_texcoord = float3(u_uvscaleoffset.xy * float2(1.0 + dot(normalize(u_lightpos%i), worldnormal), 1.0 + dot(normalize(u_lightpos%i), worldnormal)) * 0.5, 1.0);\n", ls0, ls1); | |||
| { | |||
| std::string lightFactor0 = StringFromFormat("(length(u_lightpos%i) == 0.0 ? worldnormal.z : dot(normalize(u_lightpos%i), worldnormal))", ls0, ls0); | |||
There was a problem hiding this comment.
Related to the previous comment, I wonder if GPU compilers optimize length==0 to avoid the square root. Then again, square roots are pretty fast on gpu so maybe it doesn't matter. No idea if either way is faster than just comparing all three components to 0.0.
There was a problem hiding this comment.
Hm, I can't get any even slightly consistent results between:
all(equal(light.pos[%i], vec3(0.0)))length(light.pos[%i]) == 0.0dot(light.pos[%i], vec3(1.0)) == 0.0dot(light.pos[%i], light.pos[%i]) == 0.0light.pos[%i].x == 0.0 && light.pos[%i].y == 0.0 && light.pos[%i].z == 0.0
If you have any preference, happy to change it. No idea which might be faster on various drivers...
-[Unknown]
There was a problem hiding this comment.
Probably all of small enough impact to not really make a performance difference...
This fixes #4155. Well, except the slope mip mapping issue which I'm opening separately.
The correct behavior appears when normalizing it as (0, 0, 1) which effectively means using the z component of worldnormal only. A bit worried this might affect other places we normalize.
This is what it looks like now:
Correct result (PSP screenshot from @daniel229) - different frame, of course:
I think the white could be a bit too strong, but it looks close, compare to without white at all:
-[Unknown]