Skip to content

Commit

Permalink
vk: rt: fix magenta light glitches
Browse files Browse the repository at this point in the history
It was due to some light samples having dot(N,L) < 0.
  • Loading branch information
w23 committed Jan 8, 2024
1 parent dee067c commit 64520ef
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 28 deletions.
4 changes: 3 additions & 1 deletion ref/vk/TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
- [x] add debugPrintfEXT to shaders
- [x] fix black dots on glass surfaces
- [ ] fix polygon light nans in logs
- [ ] magenta gliches
- [x] magenta gliches -- dot(N,L) < 0.
- [ ] validate all intermediate and final outputs against invalid values, complain into log
- [ ] disableable NaN debugging
- [ ] Better PBR math, e.g.:
- [ ] Fresnel issues (esp. with skybox)
- [ ] Just make sure that all the BRDF math is correct
Expand Down
37 changes: 26 additions & 11 deletions ref/vk/shaders/brdf.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,20 @@ float ggxV(float a2, float l_dot_n, float h_dot_l, float n_dot_v, float h_dot_v)
// return mix(base, layer, fr)
//}

void brdfComputeGltfModel(vec3 N, vec3 L, vec3 V, MaterialProperties material, out vec3 out_diffuse, out vec3 out_specular) {
void brdfComputeGltfModel(vec3 N, vec3 L, vec3 V, MaterialProperties material, out float l_dot_n, out vec3 out_diffuse, out vec3 out_specular) {
l_dot_n = max(0., dot(L, N));

if (l_dot_n == 0.) {
out_diffuse = vec3(0.);
out_specular = vec3(0.);
return;
}

const float alpha = material.roughness * material.roughness;
const float a2 = alpha * alpha;
const vec3 H = normalize(L + V);
const float h_dot_v = dot(H, V);
const float h_dot_n = dot(H, N);
const float l_dot_n = dot(L, N);
const float h_dot_l = dot(H, L);
const float n_dot_v = dot(N, V);

Expand All @@ -92,7 +99,14 @@ void brdfComputeGltfModel(vec3 N, vec3 L, vec3 V, MaterialProperties material, o

// Specular does get the real color, as its contribution is light-direction-dependent
const vec3 f0 = mix(vec3(.04), material.base_color, material.metalness);
const float fresnel_factor = pow(1. - abs(h_dot_v), 5.);
float fresnel_factor = max(0., pow(1. - abs(h_dot_v), 5.));

if (fresnel_factor < .0 || fresnel_factor > 1. || IS_INVALID(fresnel_factor)) {
debugPrintfEXT("N=(%f,%f,%f) L=(%f,%f,%f) V=(%f,%f,%f) H=(%f,%f,%f) h_dot_v=%f INVALID fresnel_factor=%f",
PRIVEC3(N), PRIVEC3(L), PRIVEC3(V), PRIVEC3(H), h_dot_v, fresnel_factor);
fresnel_factor = clamp(fresnel_factor, 0., 1.);
}

const vec3 fresnel = vec3(1.) * fresnel_factor + f0 * (1. - fresnel_factor);

// This is taken directly from glTF 2.0 spec. It seems incorrect to me: it should not include the base_color twice.
Expand All @@ -105,15 +119,15 @@ void brdfComputeGltfModel(vec3 N, vec3 L, vec3 V, MaterialProperties material, o
out_diffuse = diffuse_color * kOneOverPi * .96 * (1. - fresnel_factor);

const float ggxd = ggxD(a2, h_dot_n);
if (IS_INVALID(ggxd)) {
debugPrintfEXT("N=(%f,%f,%f) L=(%f,%f,%f) V=(%f,%f,%f) a2=%f h_dot_n=%f ggxd=%f",
if (IS_INVALID(ggxd) || ggxd < 0. /* || ggxd > 1.*/) {
debugPrintfEXT("N=(%f,%f,%f) L=(%f,%f,%f) V=(%f,%f,%f) a2=%f h_dot_n=%f INVALID ggxd=%f",
PRIVEC3(N), PRIVEC3(L), PRIVEC3(V), a2, h_dot_n, ggxd);
}

const float ggxv = ggxV(a2, l_dot_n, h_dot_l, n_dot_v, h_dot_v);
if (IS_INVALID(ggxv)) {
debugPrintfEXT("N=(%f,%f,%f) L=(%f,%f,%f) V=(%f,%f,%f) ggxv=%f",
PRIVEC3(N), PRIVEC3(L), PRIVEC3(V), ggxv);
if (IS_INVALID(ggxv) || ggxv < 0. /*|| ggxv > 1.*/) {
debugPrintfEXT("N=(%f,%f,%f) L=(%f,%f,%f) V=(%f,%f,%f) a2=%f h_dot_n=%f INVALID ggxv=%f",
PRIVEC3(N), PRIVEC3(L), PRIVEC3(V), a2, h_dot_n, ggxv);
}

out_specular = fresnel * ggxd * ggxv;
Expand Down Expand Up @@ -166,8 +180,8 @@ material = f_diffuse + f_specular
#ifdef BRDF_COMPARE
if (g_mat_gltf2) {
#endif
brdfComputeGltfModel(N, L, V, material, out_diffuse, out_specular);
const float l_dot_n = dot(L, N);
float l_dot_n;
brdfComputeGltfModel(N, L, V, material, l_dot_n, out_diffuse, out_specular);
out_diffuse *= l_dot_n;
out_specular *= l_dot_n;
#ifdef BRDF_COMPARE
Expand Down Expand Up @@ -268,7 +282,8 @@ int brdfGetSample(vec2 rnd, MaterialProperties material, vec3 view, vec3 geometr
out_direction = normalize(out_direction);

vec3 diffuse = vec3(0.), specular = vec3(0.);
brdfComputeGltfModel(shading_normal, out_direction, view, material, diffuse, specular);
float l_dot_n;
brdfComputeGltfModel(shading_normal, out_direction, view, material, l_dot_n, diffuse, specular);

/*
if (any(isnan(diffuse))) {
Expand Down
26 changes: 12 additions & 14 deletions ref/vk/shaders/light.glsl
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#extension GL_EXT_control_flow_attributes : require
#include "debug.glsl"

const float color_culling_threshold = 0;//600./color_factor;
const float shadow_offset_fudge = .1;
Expand Down Expand Up @@ -155,18 +156,15 @@ void computeLighting(vec3 P, vec3 N, vec3 view_dir, MaterialProperties material,
specular += lspecular;
#endif

if (any(isnan(diffuse)))
diffuse = vec3(1.,0.,0.);

if (any(isnan(specular)))
specular = vec3(0.,1.,0.);

if (any(lessThan(diffuse,vec3(0.))))
diffuse = vec3(1., 0., 1.);

if (any(lessThan(specular,vec3(0.))))
specular = vec3(0., 1., 1.);

//specular = vec3(0.,1.,0.);
//diffuse = vec3(0.);
if (IS_INVALID3(diffuse) || any(lessThan(diffuse,vec3(0.)))) {
debugPrintfEXT("P=(%f,%f,%f) N=(%f,%f,%f) INVALID diffuse=(%f,%f,%f)",
PRIVEC3(P), PRIVEC3(N), PRIVEC3(diffuse));
diffuse = vec3(0.);
}

if (IS_INVALID3(specular) || any(lessThan(specular,vec3(0.)))) {
debugPrintfEXT("P=(%f,%f,%f) N=(%f,%f,%f) INVALID specular=(%f,%f,%f)",
PRIVEC3(P), PRIVEC3(N), PRIVEC3(specular));
specular = vec3(0.);
}
}
7 changes: 5 additions & 2 deletions ref/vk/shaders/light_polygon.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,9 @@ void sampleEmissiveSurfaces(vec3 P, vec3 N, vec3 view_dir, MaterialProperties ma
const uint num_polygons = uint(light_grid.clusters_[cluster_index].num_polygons);
for (uint i = 0; i < num_polygons; ++i) {
const uint index = uint(light_grid.clusters_[cluster_index].polygons[i]);

//if (index != 5) // < 6 || index >= 8)
// continue;
#else
for (uint index = 0; index < lights.m.num_polygons; ++index) {
#endif
Expand Down Expand Up @@ -260,8 +263,8 @@ void sampleEmissiveSurfaces(vec3 P, vec3 N, vec3 view_dir, MaterialProperties ma
diffuse += emissive * estimate * poly_diffuse;
specular += emissive * estimate * poly_specular;

if (IS_INVALID3(specular)) {
debugPrintfEXT("%d specular=(%f,%f,%f) light=%d emissive=(%f,%f,%f) estimate=%f poly_specular=(%f,%f,%f)",
if (IS_INVALID3(specular) || any(lessThan(specular,vec3(0.)))) {
debugPrintfEXT("%d INVALID specular=(%f,%f,%f) light=%d emissive=(%f,%f,%f) estimate=%f poly_specular=(%f,%f,%f)",
__LINE__, PRIVEC3(specular), index, PRIVEC3(emissive), estimate, PRIVEC3(poly_specular));
specular = vec3(0.);
}
Expand Down

0 comments on commit 64520ef

Please sign in to comment.