Skip to content

Commit

Permalink
Fix bug in BSDF code gen for HW languages (AcademySoftwareFoundation#…
Browse files Browse the repository at this point in the history
…1399)

This change list fixes a bug in code gen for HW languages, where under certain conditions the function call for a BSDF node could be generated multiple times in the same scope.

For example, if a pure reflection BSDF node, like oren_nayar_bsdf, where referenced multiple times in a graph. Then during generation of the transmission section of a GLSL surface shader the node's output variable would be emitted multiple times, giving erroneous code.

A workaround was previously to duplicate the node in the graph, as for example done in the GltfPbr graph. With this fix duplication is no longer needed.
  • Loading branch information
niklasharrysson authored Jul 10, 2023
1 parent 2a9b37a commit d7771b3
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 10 deletions.
7 changes: 1 addition & 6 deletions libraries/bxdf/gltf_pbr.mtlx
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,6 @@
<!-- Thin-film + Dielectric
Note: Due to limitations in codegen, the base layer BSDF is duplicated (#1035). -->

<oren_nayar_diffuse_bsdf name="tf_diffuse_bsdf" type="BSDF">
<input name="color" type="color3" interfacename="base_color" />
<input name="normal" type="vector3" interfacename="normal" />
</oren_nayar_diffuse_bsdf>

<dielectric_bsdf name="tf_transmission_bsdf" type="BSDF">
<input name="weight" type="float" value="1" />
<input name="tint" type="color3" interfacename="base_color" />
Expand All @@ -173,7 +168,7 @@
</generalized_schlick_bsdf>

<mix name="tf_transmission_mix" type="BSDF">
<input name="bg" type="BSDF" nodename="tf_diffuse_bsdf" />
<input name="bg" type="BSDF" nodename="diffuse_bsdf" />
<input name="fg" type="BSDF" nodename="tf_transmission_bsdf" />
<input name="mix" type="float" interfacename="transmission" />
</mix>
Expand Down
3 changes: 3 additions & 0 deletions source/MaterialXGenShader/HwShaderGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,9 @@ void HwShaderGenerator::emitFunctionCall(const ShaderNode& node, GenContext& con
emitLineBegin(stage);
emitOutput(node.getOutput(), true, true, context, stage);
emitLineEnd(stage);

// Register the node as emitted, but omit the function call.
stage.addFunctionCall(node, context, false);
}
}

Expand Down
10 changes: 7 additions & 3 deletions source/MaterialXGenShader/ShaderStage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -365,14 +365,18 @@ void ShaderStage::addFunctionDefinition(const ShaderNode& node, GenContext& cont
}
}

void ShaderStage::addFunctionCall(const ShaderNode& node, GenContext& context)
void ShaderStage::addFunctionCall(const ShaderNode& node, GenContext& context, bool emitCode)
{
// Register this function as being called in the current scope.
const ClosureContext* cct = context.getClosureContext();
const FunctionCallId id(&node, cct ? cct->getType() : 0);

_scopes.back().functions.insert(id);

node.getImplementation().emitFunctionCall(node, context, *this);
// Emit code for the function call if not omitted.
if (emitCode)
{
node.getImplementation().emitFunctionCall(node, context, *this);
}
}

bool ShaderStage::isEmitted(const ShaderNode& node, GenContext& context) const
Expand Down
5 changes: 4 additions & 1 deletion source/MaterialXGenShader/ShaderStage.h
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,10 @@ class MX_GENSHADER_API ShaderStage
void addFunctionDefinition(const ShaderNode& node, GenContext& context);

/// Add the function call for the given node.
void addFunctionCall(const ShaderNode& node, GenContext& context);
/// This will register the function as being called in the current scope, and code for the
/// function call will be added to the stage. If emitCode is set to false the code for the
/// function call will be omitted.
void addFunctionCall(const ShaderNode& node, GenContext& context, bool emitCode = true);

/// Return true if the function for the given node has been emitted in the current scope.
bool isEmitted(const ShaderNode& node, GenContext& context) const;
Expand Down

0 comments on commit d7771b3

Please sign in to comment.