Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Propagate uv scale and offset #1840

Closed
wants to merge 3 commits into from
Closed

Propagate uv scale and offset #1840

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8f80c83
Propagate added inputs created inside a NodeGraph
JGamache-autodesk May 27, 2024
ad8b225
Fix Windows preflight builds
JGamache-autodesk May 27, 2024
ee508a3
Merge branch 'dev_1.39' into propagate_uv_scale_and_offset
jstone-lucasfilm May 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions source/MaterialXGenMdl/Nodes/ClosureLayerNodeMdl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,4 +172,9 @@ void LayerableNodeMdl::addInputs(ShaderNode& node, GenContext& /*context*/) cons
node.addInput(StringConstantsMdl::BASE, Type::BSDF);
}

StringVec LayerableNodeMdl::addedInputNames() const
{
return {1, StringConstantsMdl::BASE};
}

MATERIALX_NAMESPACE_END
1 change: 1 addition & 0 deletions source/MaterialXGenMdl/Nodes/ClosureLayerNodeMdl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ class MX_GENMDL_API LayerableNodeMdl : public SourceCodeNodeMdl
static ShaderNodeImplPtr create();

void addInputs(ShaderNode& node, GenContext&) const override;
StringVec addedInputNames() const override;
};

MATERIALX_NAMESPACE_END
Expand Down
7 changes: 7 additions & 0 deletions source/MaterialXGenMdl/Nodes/ImageNodeMdl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ void ImageNodeMdl::addInputs(ShaderNode& node, GenContext& context) const
node.addInput(ImageNodeMdl::FLIP_V, Type::BOOLEAN)->setUniform();
}

StringVec ImageNodeMdl::addedInputNames() const
{
auto retVal = BASE::addedInputNames();
retVal.push_back(ImageNodeMdl::FLIP_V);
return retVal;
}

bool ImageNodeMdl::isEditable(const ShaderInput& input) const
{
if (input.getName() == ImageNodeMdl::FLIP_V)
Expand Down
2 changes: 2 additions & 0 deletions source/MaterialXGenMdl/Nodes/ImageNodeMdl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ class MX_GENMDL_API ImageNodeMdl : public SourceCodeNodeMdl

void addInputs(ShaderNode& node, GenContext& context) const override;

StringVec addedInputNames() const override;

bool isEditable(const ShaderInput& input) const override;

void emitFunctionCall(const ShaderNode& node, GenContext& context, ShaderStage& stage) const override;
Expand Down
15 changes: 15 additions & 0 deletions source/MaterialXGenShader/Nodes/CompoundNode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,21 @@ void CompoundNode::initialize(const InterfaceElement& element, GenContext& conte
_hash = std::hash<string>{}(_functionName);
}

void CompoundNode::addInputs(ShaderNode& node, GenContext&) const
{
// Propagate inputs added to the ShaderGraph
for (auto const& name: _rootGraph->getPropagatedAddedInputs()) {
const auto* inputSocket = _rootGraph->getInputSocket(name);
if (inputSocket)
{
ShaderInput* input = node.addInput(name, inputSocket->getType());
input->setValue(inputSocket->getValue());
}
}
}

// Note: No addedInputNames here. Not necessary unless we start nesting ShaderGraphs.

void CompoundNode::createVariables(const ShaderNode&, GenContext& context, Shader& shader) const
{
// Gather shader inputs from all child nodes
Expand Down
2 changes: 2 additions & 0 deletions source/MaterialXGenShader/Nodes/CompoundNode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ class MX_GENSHADER_API CompoundNode : public ShaderNodeImpl

void initialize(const InterfaceElement& element, GenContext& context) override;

void addInputs(ShaderNode& node, GenContext& context) const override;

void createVariables(const ShaderNode& node, GenContext& context, Shader& shader) const override;

void emitFunctionDefinition(const ShaderNode& node, GenContext& context, ShaderStage& stage) const override;
Expand Down
5 changes: 5 additions & 0 deletions source/MaterialXGenShader/Nodes/HwImageNode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ void HwImageNode::addInputs(ShaderNode& node, GenContext&) const
input->setValue(Value::createValue<Vector2>(Vector2(0.0f, 0.0f)));
}

StringVec HwImageNode::addedInputNames() const
{
return {UV_SCALE, UV_OFFSET};
}

void HwImageNode::setValues(const Node& node, ShaderNode& shaderNode, GenContext& context) const
{
// Remap uvs to normalized 0..1 space if the original UDIMs in a UDIM set
Expand Down
3 changes: 3 additions & 0 deletions source/MaterialXGenShader/Nodes/HwImageNode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ class MX_GENSHADER_API HwImageNode : public SourceCodeNode
static ShaderNodeImplPtr create();

void addInputs(ShaderNode& node, GenContext& context) const override;

StringVec addedInputNames() const override;

void setValues(const Node& node, ShaderNode& shaderNode, GenContext& context) const override;
};

Expand Down
33 changes: 33 additions & 0 deletions source/MaterialXGenShader/ShaderGraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ void ShaderGraph::createConnectedNodes(const ElementPtr& downstreamElement,
if (!newNode)
{
newNode = createNode(upstreamNode, context);
propagateAddedInputs(*newNode, newNode->addedInputNames());
}

//
Expand Down Expand Up @@ -424,6 +425,9 @@ ShaderGraphPtr ShaderGraph::create(const ShaderGraph* parent, const NodeGraph& n
// Clear classification
graph->_classification = 0;

// Allow added input propagation since this ShaderGraph is for a CompoundNode
graph->_propagateAddedInputs = true;

// Create input sockets from the nodedef
graph->addInputSockets(*nodeDef, context);

Expand All @@ -442,6 +446,35 @@ ShaderGraphPtr ShaderGraph::create(const ShaderGraph* parent, const NodeGraph& n
return graph;
}

void ShaderGraph::propagateAddedInputs(ShaderNode& node, const StringVec& addedInputs)
{
if (!_propagateAddedInputs) {
return;
}

for (auto const& name: addedInputs) {
auto* nodeInput = node.getInput(name);
if (nodeInput) {
auto* inputSocket = getInputSocket(name);
if (!inputSocket) {
// Only add an input socket once. This means that a NodeGraph
// with three embedded image nodes will only get a single
// pair of uv_scale and uv_offset inputs shared between
// all three nodes.
// TODO: Have as many uv_scale inputs as there are embedded image
// nodes. Requires a scheme to make their names unique. Bonus
// points if that name can be related to the filename input.
// NOTE: This is enough to have UDIMs working with ND_gltf_colorimage
// and ND_UsdUVTexture, so we stop here at this time.
inputSocket = addInputSocket(name, nodeInput->getType());
Copy link
Contributor

@kwokcb kwokcb May 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @JGamache-autodesk,
I don't think this is enough and is inconsistent with how other sockets are "published" using <node name>_ <input name> . This would also resolve the issue that you should be able to support different UDIM scale/offsets for different image inputs -- i.e. there is no guarantee there is only one set of UDIM information.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should definitely split the issue, especially since this PR only attempts to fix single image wrappers like those found for glTf, USD, ADSK, Arnold, etc. Image nodes wrapped in simple NodeGraph implementations.

The second part where we support multi-image nodes with multiple UDIM scales should be indeed discussed and implemented correctly, but maybe should not be a showstopper for first fixing ND_UsdUVTexture and ND_gltf_colorimage.

_propagatedAddedInputs.push_back(name);
}
inputSocket->makeConnection(nodeInput);
inputSocket->setValue(nodeInput->getValue());
}
}
}

ShaderGraphPtr ShaderGraph::create(const ShaderGraph* parent, const string& name, ElementPtr element, GenContext& context)
{
ShaderGraphPtr graph;
Expand Down
10 changes: 10 additions & 0 deletions source/MaterialXGenShader/ShaderGraph.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ class MX_GENSHADER_API ShaderGraph : public ShaderNode
static ShaderGraphPtr create(const ShaderGraph* parent, const NodeGraph& nodeGraph,
GenContext& context);

/// Get the list of added inputs to propagate further up
const StringVec& getPropagatedAddedInputs() const { return _propagatedAddedInputs; }

/// Return true if this node is a graph.
bool isAGraph() const override { return true; }

Expand Down Expand Up @@ -132,6 +135,9 @@ class MX_GENSHADER_API ShaderGraph : public ShaderNode
/// Add a node to the graph
void addNode(ShaderNodePtr node);

/// Propagate additional inputs added on a node.
void propagateAddedInputs(ShaderNode& node, const StringVec& addedInputs);

/// Add input sockets from an interface element (nodedef, nodegraph or node)
void addInputSockets(const InterfaceElement& elem, GenContext& context);

Expand Down Expand Up @@ -188,6 +194,10 @@ class MX_GENSHADER_API ShaderGraph : public ShaderNode
std::vector<ShaderNode*> _nodeOrder;
IdentifierMap _identifiers;

// Propagate or not the extra inputs added by nodes
bool _propagateAddedInputs = false;
StringVec _propagatedAddedInputs;

// Temporary storage for inputs that require color transformations
std::unordered_map<ShaderInput*, ColorSpaceTransform> _inputColorTransformMap;
// Temporary storage for inputs that require unit transformations
Expand Down
6 changes: 6 additions & 0 deletions source/MaterialXGenShader/ShaderNode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,12 @@ class MX_GENSHADER_API ShaderNode
return (!_impl || _impl->isEditable(input));
}

/// Return input names that are added by the implementation of this node
virtual StringVec addedInputNames() const
{
return _impl ? _impl->addedInputNames() : StringVec{};
}

protected:
/// Create metadata from the nodedef according to registered metadata.
void createMetadata(const NodeDef& nodeDef, GenContext& context);
Expand Down
5 changes: 5 additions & 0 deletions source/MaterialXGenShader/ShaderNodeImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ void ShaderNodeImpl::addInputs(ShaderNode&, GenContext&) const
{
}

StringVec ShaderNodeImpl::addedInputNames() const
{
return {};
}

void ShaderNodeImpl::setValues(const Node&, ShaderNode&, GenContext&) const
{
}
Expand Down
3 changes: 3 additions & 0 deletions source/MaterialXGenShader/ShaderNodeImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ class MX_GENSHADER_API ShaderNodeImpl
/// Add additional inputs on a node.
virtual void addInputs(ShaderNode& node, GenContext& context) const;

/// Return input names that are added by this implementation
virtual StringVec addedInputNames() const;

/// Set values for additional inputs on a node.
virtual void setValues(const Node& node, ShaderNode& shaderNode, GenContext& context) const;

Expand Down