Update link connections by dragging between ports #1569
Conversation
…n over the port (replace the connection) from Issue 1398.
| @@ -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) | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Thanks for making that change! It's a lot clearer now.
| @@ -2505,187 +2515,206 @@ void Graph::addLink(ed::PinId inputPinId, ed::PinId outputPinId) | |||
| return; | |||
| } | |||
|
|
|||
| if (inputPin->_connected == false) | |||
| // Perform kind check | |||
| bool kindsMatch = (outputPin->_kind == inputPin->_kind); | |||
There was a problem hiding this comment.
Added check that the output and input pins are not the same kinds (eg. not both Output and not both Input).
| if (ed::AcceptNewItem()) | ||
| { | ||
| // If the accepting node already has a link, remove it | ||
| if (inputPin->_connected) |
There was a problem hiding this comment.
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.
| @@ -2505,187 +2515,206 @@ void Graph::addLink(ed::PinId inputPinId, ed::PinId outputPinId) | |||
| return; | |||
| } | |||
|
|
|||
| if (inputPin->_connected == false) | |||
There was a problem hiding this comment.
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).
| @@ -4045,12 +4074,12 @@ void Graph::drawGraph(ImVec2 mousePos) | |||
| // Add new link | |||
| if (ed::BeginCreate()) | |||
| { | |||
| ed::PinId inputPinId, outputPinId, filterPinId; | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Thanks @erikaharrison-adsk ! This looks great, looking forward to trying it out.
There was a problem hiding this comment.
Great work on this improvement, thanks @erikaharrison-adsk!
jstone-lucasfilm
changed the title
jstone-lucasfilm
changed the title
jstone-lucasfilm
changed the title
jstone-lucasfilm
merged commit eb52ec2
into
AcademySoftwareFoundation:main
…ndation#1569) Currently, one must explicitly delete an existing link before adding a new link for a given node. This change enables dragging to update a link connection, and automatically deletes the existing link on the input pin if one exists. Note: An output pin on a node may have many outgoing links, but an input pin may only have one incoming link. This behaviour is unchanged. Also: - Renames a few variables for consistency on start/end and input/output pins - Ensures you can't add a link from an input to an input, or from an output to an output - Can also drag from an output to an input and have it correctly delete existing link
Currently, one must explicitly delete an existing link before adding a new link for a given node. This change enables dragging to update a link connection, and automatically deletes the existing link on the input pin if one exists.
Note: An output pin on a node may have many outgoing links, but an input pin may only have one incoming link. This behaviour is unchanged.
Example video: https://github.com/AcademySoftwareFoundation/MaterialX/assets/85182970/7a825225-09f7-4936-abb9-f294978ebaaf
Also:
Original issue/sub-issue: #1398 (comment)