Skip to content

Commit

Permalink
Guard against invalid nodegraphs and connections in editor (AcademySo…
Browse files Browse the repository at this point in the history 
…ftwareFoundation#1503)

- Add firewall check for output->input connections which are invalid.
- Prevent nodegraph creation inside of nodegraphs which is unimplemented, and actually creates at the root unexpectedly.

## Fixes
- Add a check for output to input connect when trying to create a link. Show the appropriate message. Also some minor variable cleanup.
- Filter out nodes to display in UI when trying to add a node. If inside a nodegraph, then filter out the "nodegraph" entry.
  • Loading branch information
@kwokcb
kwokcb authored Aug 31, 2023
1 parent 6a480f0 commit 1027afb
Showing 1 changed file with 133 additions and 103 deletions.
236 changes: 133 additions & 103 deletions source/MaterialXGraphEditor/Graph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2486,154 +2486,176 @@ void Graph::AddLink(ed::PinId inputPinId, ed::PinId outputPinId)
int start_attr = int(inputPinId.Get());
UiPinPtr inputPin = getPin(outputPinId);
UiPinPtr outputPin = getPin(inputPinId);
if (inputPinId && outputPinId && (outputPin->_type == inputPin->_type))

if (!inputPin || !outputPin)
{
ed::RejectNewItem();
return;
}

// Perform type check
bool typesMatch = (outputPin->_type == inputPin->_type);
if (!typesMatch)
{
if (inputPin->_connected == false)
ed::RejectNewItem();
showLabel("Invalid connection due to mismatched types", ImColor(50, 50, 50, 255));
return;
}

if (inputPin->_connected == false)
{
int upNode = getNodeId(inputPinId);
int downNode = getNodeId(outputPinId);
UiNodePtr uiDownNode = _graphNodes[downNode];
UiNodePtr uiUpNode = _graphNodes[upNode];
if (!uiDownNode || !uiUpNode)
{
int upNode = getNodeId(inputPinId);
int downNode = getNodeId(outputPinId);
ed::RejectNewItem();
return;
}

// make sure there is an implementation for node
const mx::ShaderGenerator& shadergen = _renderer->getGenContext().getShaderGenerator();

// make sure there is an implementation for node
const mx::ShaderGenerator& shadergen = _renderer->getGenContext().getShaderGenerator();
// Prevent direct connecting from input to output
if (uiDownNode->getInput() && uiUpNode->getOutput())
{
ed::RejectNewItem();
showLabel("Direct connections between inputs and outputs is invalid", ImColor(50, 50, 50, 255));
return;
}

// Find the implementation for this nodedef if not an input or output uinode
if (_graphNodes[downNode]->getInput() && _isNodeGraph)
// Find the implementation for this nodedef if not an input or output uinode
if (uiDownNode->getInput() && _isNodeGraph)
{
ed::RejectNewItem();
showLabel("Cannot connect to inputs inside of graph", ImColor(50, 50, 50, 255));
return;
}
else if (uiUpNode->getNode())
{
mx::ShaderNodeImplPtr impl = shadergen.getImplementation(*_graphNodes[upNode]->getNode()->getNodeDef(), _renderer->getGenContext());
if (!impl)
{
ed::RejectNewItem();
showLabel("Cannot connect to inputs inside of graph", ImColor(50, 50, 50, 255));
showLabel("Invalid Connection: Node does not have an implementation", ImColor(50, 50, 50, 255));
return;
}
else if (_graphNodes[upNode]->getNode())
{
mx::ShaderNodeImplPtr impl = shadergen.getImplementation(*_graphNodes[upNode]->getNode()->getNodeDef(), _renderer->getGenContext());
if (!impl)
{
ed::RejectNewItem();
showLabel("Invalid Connection: Node does not have an implementation", ImColor(50, 50, 50, 255));
return;
}
}
}

if (ed::AcceptNewItem())
{
// Since we accepted new link, lets add one to our list of links.
Link link;
link._startAttr = start_attr;
link._endAttr = end_attr;
_currLinks.push_back(link);
_frameCount = ImGui::GetFrameCount();
_renderer->setMaterialCompilation(true);
if (ed::AcceptNewItem())
{
// Since we accepted new link, lets add one to our list of links.
Link link;
link._startAttr = start_attr;
link._endAttr = end_attr;
_currLinks.push_back(link);
_frameCount = ImGui::GetFrameCount();
_renderer->setMaterialCompilation(true);

if (_graphNodes[downNode]->getNode() || _graphNodes[downNode]->getNodeGraph())
if (uiDownNode->getNode() || uiDownNode->getNodeGraph())
{
mx::InputPtr connectingInput = nullptr;
for (UiPinPtr pin : uiDownNode->inputPins)
{
mx::InputPtr connectingInput = nullptr;
for (UiPinPtr pin : _graphNodes[downNode]->inputPins)
if (pin->_pinId == outputPinId)
{
if (pin->_pinId == outputPinId)
addNodeInput(uiDownNode, pin->_input);
// update value to be empty
if (uiDownNode->getNode() && uiDownNode->getNode()->getType() == mx::SURFACE_SHADER_TYPE_STRING)
{
addNodeInput(_graphNodes[downNode], pin->_input);
// update value to be empty
if (_graphNodes[downNode]->getNode() && _graphNodes[downNode]->getNode()->getType() == mx::SURFACE_SHADER_TYPE_STRING)
if (uiUpNode->getOutput() != nullptr)
{
if (_graphNodes[upNode]->getOutput() != nullptr)
{
pin->_input->setConnectedOutput(_graphNodes[upNode]->getOutput());
}
else if (_graphNodes[upNode]->getInput() != nullptr)
{
pin->_input->setInterfaceName(_graphNodes[upNode]->getName());
}
else
pin->_input->setConnectedOutput(uiUpNode->getOutput());
}
else if (uiUpNode->getInput() != nullptr)
{
pin->_input->setInterfaceName(uiUpNode->getName());
}
else
{
// node graph
if (uiUpNode->getNodeGraph() != nullptr)
{
// node graph
if (_graphNodes[upNode]->getNodeGraph() != nullptr)
for (UiPinPtr outPin : uiUpNode->outputPins)
{
for (UiPinPtr outPin : _graphNodes[upNode]->outputPins)
// set pin connection to correct output
if (outPin->_pinId == inputPinId)
{
// set pin connection to correct output
if (outPin->_pinId == inputPinId)
{
mx::OutputPtr outputs = _graphNodes[upNode]->getNodeGraph()->getOutput(outPin->_name);
pin->_input->setConnectedOutput(outputs);
}
mx::OutputPtr outputs = uiUpNode->getNodeGraph()->getOutput(outPin->_name);
pin->_input->setConnectedOutput(outputs);
}
}
else
{
pin->_input->setConnectedNode(_graphNodes[upNode]->getNode());
}
}
else
{
pin->_input->setConnectedNode(uiUpNode->getNode());
}
}
}
else
{
if (uiUpNode->getInput())
{
pin->_input->setInterfaceName(uiUpNode->getName());
}
else
{
if (_graphNodes[upNode]->getInput())
if (uiUpNode->getNode())
{

pin->_input->setInterfaceName(_graphNodes[upNode]->getName());
pin->_input->setConnectedNode(uiUpNode->getNode());
}
else
else if (uiUpNode->getNodeGraph())
{
if (_graphNodes[upNode]->getNode())
{
pin->_input->setConnectedNode(_graphNodes[upNode]->getNode());
}
else if (_graphNodes[upNode]->getNodeGraph())
for (UiPinPtr outPin : uiUpNode->outputPins)
{
for (UiPinPtr outPin : _graphNodes[upNode]->outputPins)
// set pin connection to correct output
if (outPin->_pinId == inputPinId)
{
// set pin connection to correct output
if (outPin->_pinId == inputPinId)
{
mx::OutputPtr outputs = _graphNodes[upNode]->getNodeGraph()->getOutput(outPin->_name);
pin->_input->setConnectedOutput(outputs);
}
mx::OutputPtr outputs = uiUpNode->getNodeGraph()->getOutput(outPin->_name);
pin->_input->setConnectedOutput(outputs);
}
}
}
}

pin->setConnected(true);
pin->_input->removeAttribute(mx::ValueElement::VALUE_ATTRIBUTE);
connectingInput = pin->_input;
break;
}

pin->setConnected(true);
pin->_input->removeAttribute(mx::ValueElement::VALUE_ATTRIBUTE);
connectingInput = pin->_input;
break;
}
// create new edge and set edge information
createEdge(_graphNodes[upNode], _graphNodes[downNode], connectingInput);
}
else if (_graphNodes[downNode]->getOutput() != nullptr)
{
mx::InputPtr connectingInput = nullptr;
_graphNodes[downNode]->getOutput()->setConnectedNode(_graphNodes[upNode]->getNode());
// create new edge and set edge information
createEdge(_graphNodes[upNode], _graphNodes[downNode], connectingInput);
}
else if (_graphNodes[downNode]->getOutput() != nullptr)
{
mx::InputPtr connectingInput = nullptr;
_graphNodes[downNode]->getOutput()->setConnectedNode(_graphNodes[upNode]->getNode());

// create new edge and set edge information
createEdge(_graphNodes[upNode], _graphNodes[downNode], connectingInput);
}
else
// create new edge and set edge information
createEdge(_graphNodes[upNode], _graphNodes[downNode], connectingInput);
}
else
{
// create new edge and set edge info
UiEdge newEdge = UiEdge(_graphNodes[upNode], _graphNodes[downNode], nullptr);
if (!edgeExists(newEdge))
{
// create new edge and set edge info
UiEdge newEdge = UiEdge(_graphNodes[upNode], _graphNodes[downNode], nullptr);
if (!edgeExists(newEdge))
{
_graphNodes[downNode]->edges.push_back(newEdge);
_currEdge.push_back(newEdge);
_graphNodes[downNode]->edges.push_back(newEdge);
_currEdge.push_back(newEdge);

// update input node num and output connections
_graphNodes[downNode]->setInputNodeNum(1);
_graphNodes[upNode]->setOutputConnection(_graphNodes[downNode]);
}
// update input node num and output connections
_graphNodes[downNode]->setInputNodeNum(1);
_graphNodes[upNode]->setOutputConnection(_graphNodes[downNode]);
}
}
}
else
{
ed::RejectNewItem();
}
}
else
{
ed::RejectNewItem();
showLabel("Invalid Connection due to Mismatch Types", ImColor(50, 50, 50, 255));
}
}

Expand Down Expand Up @@ -3479,6 +3501,7 @@ void Graph::addNodePopup(bool cursor)
std::string subs(input);
// input string length
// filter extra nodes - includes inputs, outputs, groups, and node graphs
const std::string NODEGRAPH_ENTRY = "Node Graph";
for (std::unordered_map<std::string, std::vector<std::vector<std::string>>>::iterator it = _extraNodes.begin(); it != _extraNodes.end(); ++it)
{
// filter out list of nodes
Expand All @@ -3489,6 +3512,13 @@ void Graph::addNodePopup(bool cursor)
{
std::string str(it->second[i][0]);
std::string nodeName = it->second[i][0];

// Disallow creating nested nodegraphs
if (_isNodeGraph && it->first == NODEGRAPH_ENTRY)
{
continue;
}

// allow spaces to be used to search for node names
std::replace(subs.begin(), subs.end(), ' ', '_');

Expand Down

0 comments on commit 1027afb

Please sign in to comment.