AcademySoftwareFoundation / OpenShadingLanguage

Advanced shading language for production GI renderers
BSD 3-Clause "New" or "Revised" License
2.05k stars 346 forks source link

Base closure pointer in layer is incorrect when using batched execute #1801

Open johnfea opened 2 months ago

johnfea commented 2 months ago

Problem

Base closure pointer in osl layer points to its parent layer closure when using batched execute while it should actually point to a diffuse closure.

void process_bsdf_closure(const ClosureColor* closure) {
const ClosureComponent* comp = closure->as_comp();
const MxLayerParams* srcparams = comp->as<MxLayerParams>();
assert(srcparams->base != closure);

minimal_test_case.zip

Steps to Reproduce

Download the attached minimal sample and issue the following commands:

g++ -DUSE_BATCHED -mavx2 -mfma -mtune=skylake -mf16c -g test2.cpp -loslcomp -loslexec -loslnoise -loslquery -lOpenImageIO -lOpenImageIO_Util ./a.out parsing mx layer closure parsing generalized schlick closure Error: base closure in mx layer points back to currently parsed closure

g++ -mavx2 -mfma -mtune=skylake -mf16c -g test2.cpp -loslcomp -loslexec -loslnoise -loslquery -lOpenImageIO -lOpenImageIO_Util ./a.out parsing mx layer closure parsing generalized schlick closure parsing diffuse closure <correct output, no problem!>

Here's .osl shader codes in the .zip:

shader layer_test(
                            color base_color = color(0.8, 0.8, 0.8),
                            color specular_tint = color(1.0),
                            float roughness = 0.5,
                            normal Tangent = normalize(dPdu),
                            output closure color BSDF = 0)
{
BSDF = base_color * diffuse(N);
color F0 = specular_tint;
color F90 = color(1.0);
BSDF = layer(generalized_schlick_bsdf(N, Tangent, color(1.0), color(0.0), roughness, roughness, F0, F90, 0.0, "ggx"),BSDF);
}
surface output_surface(closure color Surface = 0)
{
  Ci = Surface;
}

Versions

johnfea commented 1 week ago

The problem occurs only when there's this type of usage of closure color:

BSDF = layer(generalized_schlick_bsdf(),BSDF)

So a workaround is to always do this instead:

BSDF = layer(generalized_schlick_bsdf(),BSDF2)

From this one can also speculate what might happen here: base closure pointer is at some point (too late) obtained from the BSDF closure and written into the layer closure, but the BSDF closure is now the layer closure itself instead of the base closure.

This issue results in infinite loop when parsing output closure tree so it definitely needs to be fixed at some point.