yuphin / Lumen

A Vulkan Raytracing framework for various bidirectional path tracing techniques
MIT License
468 stars 29 forks source link

Error in glossy_f() #21

Closed tigrazone closed 10 months ago

tigrazone commented 10 months ago

when we read https://www.pbr-book.org/3ed-2018/Reflection_Models/Fresnel_Incidence_Effects

Spectrum FresnelBlend::f(const Vector3f &wo, const Vector3f &wi) const {
    auto pow5 = [](Float v) { return (v * v) * (v * v) * v; };
    Spectrum diffuse = (28.f/(23.f*Pi)) * Rd *
        (Spectrum(1.f) - Rs) * 
        (1 - pow5(1 - .5f * AbsCosTheta(wi))) *
        (1 - pow5(1 - .5f * AbsCosTheta(wo)));
    Vector3f wh = wi + wo;
    if (wh.x == 0 && wh.y == 0 && wh.z == 0) return Spectrum(0);
    wh = Normalize(wh);
    Spectrum specular = distribution->D(wh) /
        (4 * AbsDot(wi, wh) *
         std::max(AbsCosTheta(wi), AbsCosTheta(wo))) *
         SchlickFresnel(Dot(wi, wh));
    return diffuse + specular;
}

and if we compare with glossy_f():

vec3 glossy_f(const Material mat, const vec3 wo, const vec3 wi, const vec3 n_s,
              float hl, float nl, float nv, float beckmann_term) {
    const vec3 h = normalize(wo + wi);
    const vec3 f_diffuse =
        (28. / (23 * PI)) * vec3(mat.albedo) * (1 - mat.metalness) *
        (1 - pow5(1 - 0.5 * dot(wi, n_s))) * (1 - pow5(1 - 0.5 * dot(wo, n_s)));
    const vec3 f_specular = beckmann_term *
                            fresnel_schlick(vec3(mat.metalness), hl) /
                            (4 * hl * max(nl, nv));
    return f_specular + f_diffuse;
}

(1 - mat.metalness) is Rd mat.metalness is Rs and then right version of calc f_diffuse is

    const vec3 f_diffuse =
        (28. / (23 * PI)) * vec3(mat.albedo) * (1 - mat.metalness) * mat.metalness *
        (1 - pow5(1 - 0.5 * dot(wi, n_s))) * (1 - pow5(1 - 0.5 * dot(wo, n_s)));

Real working #define pow5(x) ((x)(x)(x)(x)(x)) can show you this error and my code is fixes it

yuphin commented 10 months ago

Rd stands for diffuse reflectance and Rs is the specular reflectance. Consequently, the BRDF for diffuse surfaces is Rd / PI (I.e albedo is Rd). So I think the current version should be correct. Regarding the pow5 fix, I'll add it when I start rework/reorganize the BSDF backend.

tigrazone commented 10 months ago

maybe this issue connected with https://github.com/yuphin/Lumen/issues/25