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

Improve Robustness of Input Connection Setting #1770

Open
wants to merge 12 commits into
base: dev_1.39
Choose a base branch
from
Open

Improve Robustness of Input Connection Setting #1770

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
129e14f
Add interface setting method. Make this and related methods always re…
kwokcb Apr 10, 2024
e483eb8
Add color_management file fix.
kwokcb Apr 10, 2024
f3881cc
Review fixes.
kwokcb Apr 10, 2024
5374ac9
Allow mxvalidate.py to handle folder as input.
kwokcb Apr 10, 2024
42e773e
Optimize to load stdlib once and reuse.
kwokcb Apr 10, 2024
44b1026
Revert mxvalidate.py change.
kwokcb Apr 12, 2024
9220705
Merge branch 'dev_1.39' into interface_update
jstone-lucasfilm Apr 13, 2024
e9762b6
Merge branch 'dev_1.39' into interface_update
jstone-lucasfilm Apr 30, 2024
e3fe2c2
Rename setInterfaceInput() to setConnectedInterface().
kwokcb May 10, 2024
f6f28b0
Remove reference to old method name
jstone-lucasfilm May 10, 2024
f5ca3ae
Minor spelling fix
jstone-lucasfilm May 10, 2024
46eaf21
Change API name to final name. Add in JS binding.
kwokcb May 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions resources/Materials/TestSuite/stdlib/color_management/color_management.mtlx
View file
Open in desktop
Expand Up @@ -54,14 +54,14 @@
<input name="file" type="filename" value="resources/Images/grid.png" colorspace="adobergb" />
</image>
<standard_surface name="image_adobergb_standard_surface7" type="surfaceshader">
<input name="base_color" type="color3" value="1, 1, 1" nodename="image_adobergb" />
<input name="base_color" type="color3" nodename="image_adobergb" />
jstone-lucasfilm marked this conversation as resolved.
Show resolved Hide resolved
</standard_surface>
<output name="image_adobergb_output" type="surfaceshader" nodename="image_adobergb_standard_surface7" />
<image name="image_lin_adobergb" type="color3">
<input name="file" type="filename" value="resources/Images/grid.png" colorspace="lin_adobergb" />
</image>
<standard_surface name="image_lin_adobergb_standard_surface8" type="surfaceshader">
<input name="base_color" type="color3" value="1, 1, 1" nodename="image_lin_adobergb" />
<input name="base_color" type="color3" nodename="image_lin_adobergb" />
</standard_surface>
<output name="image_lin_adobergb_output" type="surfaceshader" nodename="image_lin_adobergb_standard_surface8" />
<image name="image_srgb_displayp3" type="color3">
Expand Down Expand Up @@ -131,14 +131,14 @@
<input name="value" type="color4" value="0.5, 0.0, 0.0, 1.0" colorspace="adobergb" />
</constant>
<standard_surface name="color_adobergb_standard_surface7" type="surfaceshader">
<input name="base_color" type="color3" value="1, 1, 1" nodename="color_adobergb" channels="rgb" />
<input name="base_color" type="color3" nodename="color_adobergb" channels="rgb" />
</standard_surface>
<output name="color_adobergb_output" type="surfaceshader" nodename="color_adobergb_standard_surface7" />
<constant name="color_lin_adobergb" type="color4">
<input name="value" type="color4" value="0.5, 0.0, 0.0, 1.0" colorspace="lin_adobergb" />
</constant>
<standard_surface name="color_lin_adobergb_standard_surface8" type="surfaceshader">
<input name="base_color" type="color3" value="1, 1, 1" nodename="color_lin_adobergb" channels="rgb" />
<input name="base_color" type="color3" nodename="color_lin_adobergb" channels="rgb" />
</standard_surface>
<output name="color_lin_adobergb_output" type="surfaceshader" nodename="color_lin_adobergb_standard_surface8" />
<constant name="color_srgb_displayp3" type="color4">
Expand Down
1 change: 1 addition & 0 deletions source/JsMaterialX/JsMaterialXCore/JsInterface.cpp
View file
Open in desktop
Expand Up @@ -53,6 +53,7 @@ EMSCRIPTEN_BINDINGS(interface)
.function("getConnectedNode", &mx::Input::getConnectedNode)
.function("setConnectedOutput", &mx::Input::setConnectedOutput)
.function("getConnectedOutput", &mx::Input::getConnectedOutput)
.function("setInterfaceInput", &mx::Input::setInterfaceInput)
.function("getInterfaceInput", &mx::Input::getInterfaceInput)
.class_property("CATEGORY", &mx::Input::CATEGORY)
.class_property("DEFAULT_GEOM_PROP_ATTRIBUTE", &mx::Input::DEFAULT_GEOM_PROP_ATTRIBUTE);
Expand Down
21 changes: 21 additions & 0 deletions source/MaterialXCore/Interface.cpp
View file
Open in desktop
Expand Up @@ -52,6 +52,7 @@ void PortElement::setConnectedNode(ConstNodePtr node)
if (node)
{
setNodeName(node->getName());
removeAttribute(VALUE_ATTRIBUTE);
}
else
{
Expand Down Expand Up @@ -81,6 +82,8 @@ void PortElement::setConnectedOutput(ConstOutputPtr output)
setNodeName(parent->getName());
removeAttribute(NODE_GRAPH_ATTRIBUTE);
}

removeAttribute(VALUE_ATTRIBUTE);
}
else
{
Expand Down Expand Up @@ -301,6 +304,23 @@ NodePtr Input::getConnectedNode() const
return PortElement::getConnectedNode();
}

void Input::setInterfaceInput(const string& interfaceName)
{
if (interfaceName != EMPTY_STRING)
jstone-lucasfilm marked this conversation as resolved.
Show resolved Hide resolved
jstone-lucasfilm marked this conversation as resolved.
Show resolved Hide resolved
{
ConstGraphElementPtr graph = getAncestorOfType<GraphElement>();
if (graph && graph->getInput(interfaceName))
{
setInterfaceName(interfaceName);
removeAttribute(VALUE_ATTRIBUTE);
jstone-lucasfilm marked this conversation as resolved.
Show resolved Hide resolved
}
}
else
{
removeAttribute(INTERFACE_NAME_ATTRIBUTE);
}
}

InputPtr Input::getInterfaceInput() const
{
if (hasInterfaceName())
Expand Down Expand Up @@ -339,6 +359,7 @@ bool Input::validate(string* message) const
{
bool hasValueBinding = hasValue();
bool hasConnection = hasNodeName() || hasNodeGraphString() || hasOutputString() || hasInterfaceName();
validateRequire(!(hasValueBinding && hasConnection), res, message, "Node input binds to both a value and a connection");
jstone-lucasfilm marked this conversation as resolved.
Show resolved Hide resolved
jstone-lucasfilm marked this conversation as resolved.
Show resolved Hide resolved
validateRequire(hasValueBinding || hasConnection, res, message, "Node input binds no value or connection");
}
else if (parent->isA<NodeGraph>())
Expand Down
6 changes: 6 additions & 0 deletions source/MaterialXCore/Interface.h
View file
Open in desktop
Expand Up @@ -247,6 +247,12 @@ class MX_CORE_API Input : public PortElement
/// Return the node, if any, to which this input is connected.
NodePtr getConnectedNode() const override;

/// Set the input interface. If the interface name specified is not empty,
/// the the interface is set if a corresponding input exists in the graph or document
/// containing the input's node. If the interface name is empty then the
/// interface reference removed.
void setInterfaceInput(const string& interfaceName);
jstone-lucasfilm marked this conversation as resolved.
Show resolved Hide resolved

/// Return the input on the parent graph corresponding to the interface name
/// for this input.
InputPtr getInterfaceInput() const;
Expand Down
24 changes: 11 additions & 13 deletions source/MaterialXGraphEditor/Graph.cpp
View file
Open in desktop
Expand Up @@ -1834,7 +1834,7 @@ void Graph::copyInputs()
else if (upNode->getInput())
{

copyNode->inputPins[count]->_input->setInterfaceName(upNode->getName());
copyNode->inputPins[count]->_input->setInterfaceInput(upNode->getName());
jstone-lucasfilm marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
Expand All @@ -1860,7 +1860,7 @@ void Graph::copyInputs()
{
if (upNode->getInput())
{
copyNode->inputPins[count]->_input->setInterfaceName(upNode->getName());
copyNode->inputPins[count]->_input->setInterfaceInput(upNode->getName());
}
else
{
Expand All @@ -1869,7 +1869,6 @@ void Graph::copyInputs()
}

copyNode->inputPins[count]->setConnected(true);
copyNode->inputPins[count]->_input->removeAttribute(mx::ValueElement::VALUE_ATTRIBUTE);
jstone-lucasfilm marked this conversation as resolved.
Show resolved Hide resolved
}
else if (copyNode->getOutput() != nullptr)
{
Expand All @@ -1885,7 +1884,7 @@ void Graph::copyInputs()
{
if (pin->_input->getInterfaceInput())
{
copyNode->inputPins[count]->_input->removeAttribute(mx::ValueElement::INTERFACE_NAME_ATTRIBUTE);
copyNode->inputPins[count]->_input->setInterfaceInput(mx::EMPTY_STRING);
}
copyNode->inputPins[count]->setConnected(false);
setDefaults(copyNode->inputPins[count]->_input);
Expand Down Expand Up @@ -2625,7 +2624,7 @@ void Graph::addLink(ed::PinId startPinId, ed::PinId endPinId)
}
else if (uiUpNode->getInput() != nullptr)
{
pin->_input->setInterfaceName(uiUpNode->getName());
pin->_input->setInterfaceInput(uiUpNode->getName());
}
else
{
Expand All @@ -2651,7 +2650,7 @@ void Graph::addLink(ed::PinId startPinId, ed::PinId endPinId)
{
if (uiUpNode->getInput())
{
pin->_input->setInterfaceName(uiUpNode->getName());
pin->_input->setInterfaceInput(uiUpNode->getName());
}
else
{
Expand Down Expand Up @@ -2697,7 +2696,6 @@ void Graph::addLink(ed::PinId startPinId, ed::PinId endPinId)
}

pin->setConnected(true);
pin->_input->removeAttribute(mx::ValueElement::VALUE_ATTRIBUTE);
connectingInput = pin->_input;
break;
}
Expand Down Expand Up @@ -2777,7 +2775,7 @@ void Graph::deleteLinkInfo(int startAttr, int endAttr)
if (_graphNodes[upNode]->getInput())
{
// Remove interface value in order to set the default of the input
pin->_input->removeAttribute(mx::ValueElement::INTERFACE_NAME_ATTRIBUTE);
pin->_input->setInterfaceInput(mx::EMPTY_STRING);
setDefaults(pin->_input);
setDefaults(_graphNodes[upNode]->getInput());
}
Expand Down Expand Up @@ -2810,7 +2808,7 @@ void Graph::deleteLinkInfo(int startAttr, int endAttr)
removeEdge(downNode, upNode, pin);
if (_graphNodes[upNode]->getInput())
{
_graphNodes[downNode]->getNodeGraph()->getInput(pin->_name)->removeAttribute(mx::ValueElement::INTERFACE_NAME_ATTRIBUTE);
_graphNodes[downNode]->getNodeGraph()->getInput(pin->_name)->setInterfaceInput(mx::EMPTY_STRING);
setDefaults(_graphNodes[upNode]->getInput());
}
for (UiPinPtr connect : pin->_connections)
Expand Down Expand Up @@ -2906,8 +2904,8 @@ void Graph::deleteNode(UiNodePtr node)
}
if (node->getInput())
{
// Remove interface value in order to set the default of the input
pin->_input->removeAttribute(mx::ValueElement::INTERFACE_NAME_ATTRIBUTE);
// Remove interface in order to set the default of the input
pin->_input->setInterfaceInput(mx::EMPTY_STRING);
setDefaults(pin->_input);
setDefaults(node->getInput());
}
Expand All @@ -2916,7 +2914,7 @@ void Graph::deleteNode(UiNodePtr node)
{
if (node->getInput())
{
pin->_pinNode->getNodeGraph()->getInput(pin->_name)->removeAttribute(mx::ValueElement::INTERFACE_NAME_ATTRIBUTE);
pin->_pinNode->getNodeGraph()->getInput(pin->_name)->setInterfaceInput(mx::EMPTY_STRING);
setDefaults(node->getInput());
}
pin->_input->setConnectedNode(nullptr);
Expand Down Expand Up @@ -3320,7 +3318,7 @@ void Graph::propertyEditor()
{
_currUiNode->getInput()->setName(name);
mx::ValuePtr val = _currUiNode->getInput()->getValue();
input->setInterfaceName(name);
input->setInterfaceInput(name);
mx::InputPtr pt = input->getInterfaceInput();
}
}
Expand Down
51 changes: 51 additions & 0 deletions source/MaterialXTest/MaterialXCore/Node.cpp
View file
Open in desktop
Expand Up @@ -32,6 +32,57 @@ bool isTopologicalOrder(const std::vector<mx::ElementPtr>& elems)
return true;
}

TEST_CASE("Interface Input Validation", "[node]")
{
std::string validationErrors;

mx::FileSearchPath searchPath = mx::getDefaultDataSearchPath();
mx::DocumentPtr doc = mx::createDocument();
mx::loadLibraries({ "libraries" }, searchPath, doc);

// Test inside nodegraph
mx::GraphElementPtr nodegraph = doc->addNodeGraph("graph1");

std::vector<mx::GraphElementPtr> graphs = { doc, nodegraph };
jstone-lucasfilm marked this conversation as resolved.
Show resolved Hide resolved
for (auto graph : graphs)
{
mx::InputPtr graphInput = graph->addInput(mx::EMPTY_STRING, "color3");
mx::NodePtr addNode = graph->addNode("add", mx::EMPTY_STRING, "color3");
mx::InputPtr addInput = addNode->addInput("in1");

addInput->setValueString("3, 3, 3");
addInput->setInterfaceName(graphInput->getName());
bool valid = doc->validate(&validationErrors);
if (!valid)
{
INFO(validationErrors);
}
REQUIRE(!valid);

addInput->setInterfaceInput(graphInput->getName());
mx::InputPtr interfaceInput = addInput->getInterfaceInput();
REQUIRE((interfaceInput && interfaceInput->getNamePath() == graphInput->getNamePath()));
REQUIRE(!addInput->getValue());
valid = doc->validate(&validationErrors);
if (!valid)
{
INFO(validationErrors);
}
REQUIRE(valid);

addInput->setInterfaceInput(mx::EMPTY_STRING);
addInput->setValueString("2, 2, 2");
interfaceInput = addInput->getInterfaceInput();
REQUIRE(!interfaceInput);
valid = doc->validate(&validationErrors);
if (!valid)
{
INFO(validationErrors);
}
REQUIRE(valid);
}
}

TEST_CASE("Node", "[node]")
{
// Create a document.
Expand Down
1 change: 1 addition & 0 deletions source/PyMaterialX/PyMaterialXCore/PyInterface.cpp
View file
Open in desktop
Expand Up @@ -39,6 +39,7 @@ void bindPyInterface(py::module& mod)
.def("getDefaultGeomPropString", &mx::Input::getDefaultGeomPropString)
.def("getDefaultGeomProp", &mx::Input::getDefaultGeomProp)
.def("getConnectedNode", &mx::Input::getConnectedNode)
.def("setInterfaceInput", &mx::Input::setInterfaceInput)
.def("getInterfaceInput", &mx::Input::getInterfaceInput)
.def_readonly_static("CATEGORY", &mx::Input::CATEGORY);

Expand Down