Refactor of BRDF interface

Currently, BRDFs can be selected using the specular and diffuse function interfaces. Fantastic! However, the signature of these functions is cumbersome, to say the least:

float specular(const in vec3 L, const in vec3 N, const in vec3 V, const in float roughness);
float specular(const in vec3 L, const in vec3 N, const in vec3 V, const in float roughness, const in float fresnel);
float specular(const in vec3 L, const in vec3 N, const in vec3 V, const in float NoV, const in float NoL, const in float roughness, const in float fresnel);

This forces every BDRF function that implements this interface to replicate these 3 signatures, even if most of the params are not used. This leads to a lot of dead and redundant code. For example:

float specularBlinnPhongRoughnes(const in vec3 L, const in vec3 N, const in vec3 V, const in float NoV, const in float NoL, const in float roughness, const in float fresnel) {
    return specularBlinnPhongRoughnes(L, N, V, roughness);
}

The other major issue with this system is extensibility. If we add a BRDF which requires a new param, a new interface would be needed and a placeholder implementation for it would be required in each file under lighting/specular or lighting diffuse.

For example the ward BRDF (used for cloth rendering) has a different signature

float specularWard(const in vec3 L, const in vec3 N, const in vec3 V, const in vec3 fiber, const in float shinyParallel, const in float shinyPerpendicular);`

And is therefore unusable with the current specular interface.

We propose to encapsulate all params in a ShadingData struct:

struct ShadingData {
   vec3 L;
   vec3 N;
   vec3 V;
   float NoV;
   float NoL;
   float roughness;
   float fresnel;

#if defined(SHADING_MODEL_CLOTH)
   vec3 fiber;
   float shinyParallel;
   float shinyPerpendicular;
#endif
}

The function interface becomes:

float specular(ShadingData shadingData);

We can also get rid of all redundant function overrides in the BRDF implementations.

Note that this pattern is already partially implemented in the Material struct

struct Material {
...
   // Cache
    vec3    V;
    vec3    R;
    float   NoV;
};

These fields should be moved to the new ShadingData struct. They don't belong in the Material struct, as they are not material properties, but by-products of the shading calculations.

Let me know how you feel about this @patriciogonzalezvivo and I can start a PR with this if you like it.

PS: There's a hidden agenda behind this. I'm planning significant improvements of the PBR pipeline, and this will make my life, and hopefully the life of future contributors, much easier.

patriciogonzalezvivo / lygia

Refactor of BRDF interface #211