-
Notifications
You must be signed in to change notification settings - Fork 435
Resolve geompropvalue names through nested node graphs in HW codegen #2957
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,17 +13,77 @@ | |
|
|
||
| MATERIALX_NAMESPACE_BEGIN | ||
|
|
||
| static const string GEOMPROP = "geomprop"; | ||
|
|
||
| ShaderNodeImplPtr HwGeomPropValueNode::create() | ||
| { | ||
| return std::make_shared<HwGeomPropValueNode>(); | ||
| } | ||
|
|
||
| void HwGeomPropValueNode::createVariables(const ShaderNode& node, GenContext&, Shader& shader) const | ||
| // Find the ShaderInput that contains the actual name of the geomprop to read | ||
| static const ShaderInput* getGeomPropValueInput(const ShaderNode& node, const GenContext& context) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Niklas can speak better than I on threading but I don't think we should add any static methods in code generation. Maybe add this as a method on |
||
| { | ||
| // We should always have the "geomprop" input locally on this node | ||
| const ShaderInput* geomPropInput = node.getInput(GEOMPROP); | ||
| if (!geomPropInput) | ||
| { | ||
| throw ExceptionShaderGenError("No 'geomprop' input found on geompropvalue node '" + node.getName() + "'."); | ||
| } | ||
|
|
||
| // Depth into the compound instance stack, measured from the innermost | ||
| // instance. Incremented each time we cross a graph boundary so that nested | ||
| // compounds resolve to the correct enclosing instance. | ||
| size_t depth = 0; | ||
|
|
||
| while (geomPropInput) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Checking that this will work if this has to read through a functional nodegraph (i.e. the graph connection is a nodedef implementation). I think it does since shader gen embeds implementation graphs. |
||
| { | ||
| // If it has a local value set, then we've found the ShaderInput and can return that | ||
| ConstValuePtr value = geomPropInput->getValue(); | ||
| if (value) | ||
| { | ||
| return geomPropInput; | ||
| } | ||
|
|
||
| // If we don't have a local value, then we may have an incoming connection from a containing NodeGraph | ||
| // Note : the "geomprop" input on the base node is "uniform" so it cannot be driven by the output from | ||
| // another node. | ||
| const ShaderOutput* upstreamPort = geomPropInput->getConnection(); | ||
| if (!upstreamPort) | ||
| { | ||
| return nullptr; | ||
| } | ||
|
|
||
| const ShaderNode* upstreamNode = upstreamPort->getNode(); | ||
| if (!upstreamNode || !upstreamNode->isAGraph()) | ||
| { | ||
| return nullptr; | ||
| } | ||
|
|
||
| // We've reached a graph input socket. Step out to the matching compound | ||
| // instance, walking the instance stack outward in tandem with the graph | ||
| // boundary we just crossed, so that nested compounds resolve to the | ||
| // correct instance rather than always the innermost one. | ||
| const ShaderNode* compoundInstance = context.getCompoundInstanceNode(depth++); | ||
| if (!compoundInstance) | ||
| { | ||
| return nullptr; | ||
| } | ||
|
|
||
| // Consider the input on the compound instance to be the new potential | ||
| // source for the value of "geomprop". The next iteration either finds a | ||
| // value here or continues climbing. | ||
| geomPropInput = compoundInstance->getInput(upstreamPort->getName()); | ||
| } | ||
|
|
||
| return nullptr; | ||
| } | ||
|
|
||
| void HwGeomPropValueNode::createVariables(const ShaderNode& node, GenContext& context, Shader& shader) const | ||
| { | ||
| const ShaderInput* geomPropInput = getGeomPropValueInput(node, context); | ||
| if (!geomPropInput || !geomPropInput->getValue()) | ||
| { | ||
| throw ExceptionShaderGenError("No 'geomprop' parameter found on geompropvalue node '" + node.getName() + "'. Don't know what property to bind"); | ||
| throw ExceptionShaderGenError("No 'geomprop' value found for geompropvalue node '" + node.getName() + "'. Don't know what property to bind"); | ||
| } | ||
| const string geomProp = geomPropInput->getValue()->getValueString(); | ||
| const ShaderOutput* output = node.getOutput(); | ||
|
|
@@ -39,10 +99,10 @@ void HwGeomPropValueNode::emitFunctionCall(const ShaderNode& node, GenContext& c | |
| { | ||
| const HwShaderGenerator& shadergen = static_cast<const HwShaderGenerator&>(context.getShaderGenerator()); | ||
|
|
||
| const ShaderInput* geomPropInput = node.getInput(GEOMPROP); | ||
| const ShaderInput* geomPropInput = getGeomPropValueInput(node, context); | ||
| if (!geomPropInput) | ||
| { | ||
| throw ExceptionShaderGenError("No 'geomprop' parameter found on geompropvalue node '" + node.getName() + "'. Don't know what property to bind"); | ||
| throw ExceptionShaderGenError("No 'geomprop' value found for geompropvalue node '" + node.getName() + "'. Don't know what property to bind"); | ||
| } | ||
| const string geomname = geomPropInput->getValue()->getValueString(); | ||
| const string variable = HW::T_IN_GEOMPROP + "_" + geomname; | ||
|
|
@@ -76,12 +136,12 @@ ShaderNodeImplPtr HwGeomPropValueNodeAsUniform::create() | |
| return std::make_shared<HwGeomPropValueNodeAsUniform>(); | ||
| } | ||
|
|
||
| void HwGeomPropValueNodeAsUniform::createVariables(const ShaderNode& node, GenContext&, Shader& shader) const | ||
| void HwGeomPropValueNodeAsUniform::createVariables(const ShaderNode& node, GenContext& context, Shader& shader) const | ||
| { | ||
| const ShaderInput* geomPropInput = node.getInput(GEOMPROP); | ||
| const ShaderInput* geomPropInput = getGeomPropValueInput(node, context); | ||
| if (!geomPropInput || !geomPropInput->getValue()) | ||
| { | ||
| throw ExceptionShaderGenError("No 'geomprop' parameter found on geompropvalue node '" + node.getName() + "'. Don't know what property to bind"); | ||
| throw ExceptionShaderGenError("No 'geomprop' value found for geompropvalue node '" + node.getName() + "'. Don't know what property to bind"); | ||
| } | ||
| const string geomProp = geomPropInput->getValue()->getValueString(); | ||
| ShaderStage& ps = shader.getStage(Stage::PIXEL); | ||
|
|
@@ -94,10 +154,10 @@ void HwGeomPropValueNodeAsUniform::emitFunctionCall(const ShaderNode& node, GenC | |
| DEFINE_SHADER_STAGE(stage, Stage::PIXEL) | ||
| { | ||
| const ShaderGenerator& shadergen = context.getShaderGenerator(); | ||
| const ShaderInput* geomPropInput = node.getInput(GEOMPROP); | ||
| const ShaderInput* geomPropInput = getGeomPropValueInput(node, context); | ||
| if (!geomPropInput) | ||
| { | ||
| throw ExceptionShaderGenError("No 'geomprop' parameter found on geompropvalue node '" + node.getName() + "'. Don't know what property to bind"); | ||
| throw ExceptionShaderGenError("No 'geomprop' value found for geompropvalue node '" + node.getName() + "'. Don't know what property to bind"); | ||
| } | ||
| const string attrName = geomPropInput->getValue()->getValueString(); | ||
| shadergen.emitLineBegin(stage); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -491,6 +491,7 @@ bool CgltfLoader::load(const FilePath& filePath, MeshList& meshList, bool texcoo | |
| texcoordStream = mesh->generateTextureCoordinates(positionStream); | ||
| mesh->addStream(texcoordStream); | ||
| } | ||
| mesh->addStreamAlias("i_" + MeshStream::GEOMETRY_PROPERTY_ATTRIBUTE + "_st", texcoordStream->getName()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This assumes a assumption on both the code gen and geometry loader side to "know" about adding "_st" postfix alias. Would it be possible to reflect this mapping within code generation so renderers can pull the appropriate given name or alias name ? |
||
| if (!vec4TangentStream) | ||
| { | ||
| MeshStreamPtr tangentStream = mesh->generateTangents(positionStream, normalStream, texcoordStream); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think you can use
GeomProp::CATEGORY