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

Update link connections by dragging between ports #1569

Merged
merged 2 commits into from
Oct 13, 2023
Merged

Update link connections by dragging between ports #1569

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e5c332f
Allow updating input connections by dragging another output connectio…
erikaharrison-adsk Oct 13, 2023
173a7de
Merge branch 'main' into drag_link_update
erikaharrison-adsk Oct 13, 2023
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
307 changes: 168 additions & 139 deletions source/MaterialXGraphEditor/Graph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,7 @@ void Graph::setRenderMaterial(UiNodePtr node)
}
}

void Graph::updateMaterials(mx::InputPtr input, mx::ValuePtr value)
void Graph::updateMaterials(mx::InputPtr input /* = nullptr */, mx::ValuePtr value /* = nullptr */)
{
std::string renderablePath;
if (_currRenderNode)
Expand Down Expand Up @@ -2483,12 +2483,22 @@ void Graph::setDefaults(mx::InputPtr input)
}
}

void Graph::addLink(ed::PinId inputPinId, ed::PinId outputPinId)
void Graph::addLink(ed::PinId startPinId, ed::PinId endPinId)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found input vs. output, start vs. end a bit confusing. Have tried to make this more accurate by checking the input pin is actually an input pin.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for making that change! It's a lot clearer now.

{
int end_attr = int(outputPinId.Get());
int start_attr = int(inputPinId.Get());
UiPinPtr inputPin = getPin(outputPinId);
UiPinPtr outputPin = getPin(inputPinId);
// prefer to assume left to right - start is an output, end is an input; swap if inaccurate
if (UiPinPtr inputPin = getPin(endPinId); inputPin && inputPin->_kind != ed::PinKind::Input)
{
auto tmp = startPinId;
startPinId = endPinId;
endPinId = tmp;
}

int end_attr = int(endPinId.Get());
int start_attr = int(startPinId.Get());
ed::PinId outputPinId = startPinId;
ed::PinId inputPinId = endPinId;
UiPinPtr outputPin = getPin(outputPinId);
UiPinPtr inputPin = getPin(inputPinId);

if (!inputPin || !outputPin)
{
Expand All @@ -2505,187 +2515,206 @@ void Graph::addLink(ed::PinId inputPinId, ed::PinId outputPinId)
return;
}

if (inputPin->_connected == false)
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 no longer skip if the input pin has a connection. Note: This resulted in the rest of the work being tabbed back (and likely caused the resulting ugly diff - it looked better on my computer, sorry).

// Perform kind check
bool kindsMatch = (outputPin->_kind == inputPin->_kind);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added check that the output and input pins are not the same kinds (eg. not both Output and not both Input).

if (kindsMatch)
{
int upNode = getNodeId(inputPinId);
int downNode = getNodeId(outputPinId);
UiNodePtr uiDownNode = _graphNodes[downNode];
UiNodePtr uiUpNode = _graphNodes[upNode];
if (!uiDownNode || !uiUpNode)
{
ed::RejectNewItem();
return;
}
ed::RejectNewItem();
showLabel("Invalid connection due to same input/output kind", ImColor(50, 50, 50, 255));
return;
}

// make sure there is an implementation for node
const mx::ShaderGenerator& shadergen = _renderer->getGenContext().getShaderGenerator();
int upNode = getNodeId(outputPinId);
int downNode = getNodeId(inputPinId);
UiNodePtr uiDownNode = _graphNodes[downNode];
UiNodePtr uiUpNode = _graphNodes[upNode];
if (!uiDownNode || !uiUpNode)
{
ed::RejectNewItem();
return;
}

// 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;
}
// make sure there is an implementation for node
const mx::ShaderGenerator& shadergen = _renderer->getGenContext().getShaderGenerator();

// Find the implementation for this nodedef if not an input or output uinode
if (uiDownNode->getInput() && _isNodeGraph)
// 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 (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 (uiUpNode->getNode())
}

if (ed::AcceptNewItem())
{
// If the accepting node already has a link, remove it
if (inputPin->_connected)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the crux of the new work - if we do have a connection for our input pin, we remove the existing link associated with it.

{
mx::ShaderNodeImplPtr impl = shadergen.getImplementation(*_graphNodes[upNode]->getNode()->getNodeDef(), _renderer->getGenContext());
if (!impl)
for (auto linksItr = _currLinks.begin(); linksItr != _currLinks.end(); linksItr++)
{
ed::RejectNewItem();
showLabel("Invalid Connection: Node does not have an implementation", ImColor(50, 50, 50, 255));
return;
if (linksItr->_endAttr == end_attr)
{
// found existing link - remove it; adapted from deleteLink
// note: ed::BreakLinks doesn't work as the order ends up inaccurate
deleteLinkInfo(linksItr->_startAttr, linksItr->_endAttr);
_currLinks.erase(linksItr);
break;
}
}

}

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);
// 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 (uiDownNode->getNode() || uiDownNode->getNodeGraph())
if (uiDownNode->getNode() || uiDownNode->getNodeGraph())
{
mx::InputPtr connectingInput = nullptr;
for (UiPinPtr pin : uiDownNode->inputPins)
{
mx::InputPtr connectingInput = nullptr;
for (UiPinPtr pin : uiDownNode->inputPins)
if (pin->_pinId == inputPinId)
{
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(uiDownNode, pin->_input);
// update value to be empty
if (uiDownNode->getNode() && uiDownNode->getNode()->getType() == mx::SURFACE_SHADER_TYPE_STRING)
if (uiUpNode->getOutput() != nullptr)
{
if (uiUpNode->getOutput() != nullptr)
{
pin->_input->setConnectedOutput(uiUpNode->getOutput());
}
else if (uiUpNode->getInput() != nullptr)
{
pin->_input->setInterfaceName(uiUpNode->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 (uiUpNode->getNodeGraph() != nullptr)
for (UiPinPtr outPin : uiUpNode->outputPins)
{
for (UiPinPtr outPin : uiUpNode->outputPins)
// set pin connection to correct output
if (outPin->_pinId == outputPinId)
{
// set pin connection to correct output
if (outPin->_pinId == inputPinId)
{
mx::OutputPtr outputs = uiUpNode->getNodeGraph()->getOutput(outPin->_name);
pin->_input->setConnectedOutput(outputs);
}
mx::OutputPtr outputs = uiUpNode->getNodeGraph()->getOutput(outPin->_name);
pin->_input->setConnectedOutput(outputs);
}
}
else
{
pin->_input->setConnectedNode(uiUpNode->getNode());
}
}
else
{
pin->_input->setConnectedNode(uiUpNode->getNode());
}
}
}
else
{
if (uiUpNode->getInput())
{
pin->_input->setInterfaceName(uiUpNode->getName());
}
else
{
if (uiUpNode->getInput())
{
pin->_input->setInterfaceName(uiUpNode->getName());
}
else
if (uiUpNode->getNode())
{
if (uiUpNode->getNode())
mx::NodePtr upstreamNode = _graphNodes[upNode]->getNode();
mx::NodeDefPtr upstreamNodeDef = upstreamNode->getNodeDef();
bool isMultiOutput = upstreamNodeDef ? upstreamNodeDef->getOutputs().size() > 1 : false;

// This is purely to avoid adding a reference to an update node only 1 output,
// as currently validation consides adding this an error. Otherwise
// it will add an "output" attribute all the time.
if (!isMultiOutput)
{
mx::NodePtr upstreamNode = _graphNodes[upNode]->getNode();
mx::NodeDefPtr upstreamNodeDef = upstreamNode->getNodeDef();
bool isMultiOutput = upstreamNodeDef ? upstreamNodeDef->getOutputs().size() > 1 : false;

// This is purely to avoid adding a reference to an update node only 1 output,
// as currently validation consides adding this an error. Otherwise
// it will add an "output" attribute all the time.
if (!isMultiOutput)
{
pin->_input->setConnectedNode(uiUpNode->getNode());
}
else
pin->_input->setConnectedNode(uiUpNode->getNode());
}
else
{
for (UiPinPtr outPin : _graphNodes[upNode]->outputPins)
{
for (UiPinPtr outPin : _graphNodes[upNode]->outputPins)
// set pin connection to correct output
if (outPin->_pinId == outputPinId)
{
// set pin connection to correct output
if (outPin->_pinId == inputPinId)
mx::OutputPtr outputs = uiUpNode->getNode()->getOutput(outPin->_name);
if (!outputs)
{
mx::OutputPtr outputs = uiUpNode->getNode()->getOutput(outPin->_name);
if (!outputs)
{
outputs = uiUpNode->getNode()->addOutput(outPin->_name, pin->_input->getType());
}
pin->_input->setConnectedOutput(outputs);
outputs = uiUpNode->getNode()->addOutput(outPin->_name, pin->_input->getType());
}
pin->_input->setConnectedOutput(outputs);
}
}
}
else if (uiUpNode->getNodeGraph())
}
else if (uiUpNode->getNodeGraph())
{
for (UiPinPtr outPin : uiUpNode->outputPins)
{
for (UiPinPtr outPin : uiUpNode->outputPins)
// set pin connection to correct output
if (outPin->_pinId == outputPinId)
{
// set pin connection to correct output
if (outPin->_pinId == inputPinId)
{
mx::OutputPtr outputs = uiUpNode->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();
}
}

void Graph::removeEdge(int downNode, int upNode, UiPinPtr pin)
Expand Down Expand Up @@ -4045,12 +4074,12 @@ void Graph::drawGraph(ImVec2 mousePos)
// Add new link
if (ed::BeginCreate())
{
ed::PinId inputPinId, outputPinId, filterPinId;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also tweaked the terminology here. imgui's node editor simply returns a start/end -- could be output to input or input to output. Changed the terminology here to reflect that.

if (ed::QueryNewLink(&inputPinId, &outputPinId))
ed::PinId startPinId, endPinId, filterPinId;
if (ed::QueryNewLink(&startPinId, &endPinId))
{
if (!readOnly())
{
addLink(inputPinId, outputPinId);
addLink(startPinId, endPinId);
}
else
{
Expand Down
Loading