imnodes
imnodes copied to clipboard
Internal link refactor
This pr contributes three things:
- Moves links internally from an object pool to a simple vector. This makes links truly immediate-mode.
- Rewrites link, node, pin interaction.
- Removes the attribute flag
EnableLinkDetachWithDragClick
.
@Auburn, if you are still around, wondering if you could give this branch a spin. You’ve been very good at spotting broken interaction code, and this branch rewrites everything 😓 I will leave this pr open for a while while I continue my own tests.
More details about each change below.
Move links from the object pool to a vector
This change boils down to
// in `ImNodesEditorContext`
- ImObjectPool<ImLinkData> Links;
// in `ImNodesContext`
+ ImVector<ImLink> Links;
When BeginNodeEditor
is called, the vector is resized to zero. Links are appended to the vector in ImNodes::Link()
. In EndNodeEditor
, we render them. Links are now truly immediate-mode and really simple.
Subtle bugs occurring on id reuse, such as this https://github.com/Nelarius/imnodes/issues/91 no longer apply to links. The goal is to apply this change for nodes and pins as well.
Rebuild link, node, pin interaction
- Needed to accommodate the new way of storing links in the interaction code as links are internally identified in a different manner now.
- The existing state machine was difficult to change. This pr introduces a more fine-grained state machine to make the logic of the link interaction states much simpler.
Remove the EnableLinkDetachWithDragClick attribute flag
- This attribute was removed to simplify the interaction state machine.
- Hovering over pins, links, and nodes can now be made totally exclusive, as we don’t need to detect the link + pin hovered state for this interaction.
- Using the feature felt finicky, as activation required clicking and dragging in the pin hover radius.
Test checklist
- [x] IsLinkStarted when partial link started
- [x] IsLinkDropped when link is dropped
- [x] triggers when including_detached_links defined & not from snap
- [x] triggers when including_detached_links defined & from snap
- [x] does not trigger from snap
- [x] triggers when not from snap
- [x] Repeat IsLinkDropped test with "detach link with modifier" enabled
- [x] Multi-selecting nodes should not clear link selection
- [x] Multi-selecting links should not clear node selection
- [ ] Auto-panning when dragging nodes
- [ ] Auto-panning when box-selecting nodes
- [ ] Auto-panning when dragging links
- [ ] Minimap viewport interaction
- [ ] Clicking minimap node centers on node
- [ ] Dragging viewport in minimap
- [ ] Minimap renders selected nodes
- [ ] Clicking and dragging nodes on top of minimap
- [ ] Dragging links on top of minimap
Hi, I gave this a quick test run, these are the things I noted:
- Removing
EnableLinkDetachWithDragClick
is a big loss for UX, tweaking and deleting links is much more clunky without it. I don't think I would use this new version without this functionality. -
IsLinkedCreated
works slightly differently now, it used to always return the output pin as the start and input as the end, now it is dependant on how the link was made. Not really an issue, probably makes more sense this way, just thought I'd mention it. - Snapping a link onto an occupied pin causes a debug assert
- Can not longer drag the node graph viewport around with any of the mouse buttons, not sure what it is bound to now?
Thanks for the feedback, really appreciate it! I will definitely look into adding EnableLinkDetachWithDragClick
back, I had a (wrong) hunch that it wasn't used much.
IsLinkedCreated
works slightly differently now
Yeah, that's a good point. ImNodes::Link()
is really simple now and just stores the ids in the order received. The exact behaviour of the IsLinkStarted
function could be specified.
@Nelarius Hey there, If I'm reading this PR right, it looks like we'll be able to customize the bezier curve's path from pin-pin? For example, if we have an output pin that needs to travel to an input pin to the left of it, we can make the curve arc upwards instead of connect in a straight line?
Hey @Nelarius is there any further progress on this update? Getting this and zooming functionality would be a huge upgrade for this library.
Without zooming support I'll probably have to switch to a different node library which would be a shame. I have locally merged latest onto the zooming PR and got that working. Also attempted to fix the mouse positioning bugs but not had any success.
Any input you have on this would be great! Thanks again