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.

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

Right-angle grid-aligned wires in the node graph #2182

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Stargazer10101
Copy link

This PR represents an initial attempt to address [Issue #2170], specifically focusing implementing the first part of the issue :
The wires follow only right-angle horizontal and vertical paths.
The corners have been rounded.

Remaining work:
Producing a final algorithm that avoids overlapping nodes and other wires given any positioning of nodes (similar to the high-fidelity node graph UI mock up posted with the issue).

Screenshot of current implementation:
Screenshot from 2025-01-09 20-05-09

@Keavon
Copy link
Member

Keavon commented Jan 9, 2025

This looks awesome! I think the easiest low-hanging fruit change is ensuring all wires line up with the grid, so vertical sections that are halfway between an odd-number of grid spaces apart should just round up.

Then for developing the algorithm further to be "smart", I imagine the first and most crucial step is a system for coordinating between wires to avoid occupying the same vertical or horizontal run (i.e. wires overlapping for some distance, meaning it's hard to see which is which) as much as possible by choosing to bend at different places so they can run parallel instead of on top of each other.

I'll need to play with it but, assuming the overlapping wires isn't too prevalent now that they don't have curvy bends, it probably makes sense to merge this PR first and then continue adding the routing algorithms in a subsequent PR. I will aim to review this PR today or tomorrow for you. Thanks, and welcome!

@Keavon
Copy link
Member

Keavon commented Jan 10, 2025

!build

@Keavon Keavon changed the title First iteration for [#2170] - Feedback Requested Right-angle wires in the node graph Jan 10, 2025
Copy link

📦 Build Complete for 5cfbbd3
https://2e1bbda1.graphite.pages.dev

@Keavon
Copy link
Member

Keavon commented Jan 10, 2025

One issue that I'm spotting now which should be quick to fix: your implementation seems to be giving the top output from a layer stack a thick wire, when it should be thin like it is in the current version.

@Stargazer10101
Copy link
Author

Thanks for feedback @Keavon . I'm looking into this.

One issue that I'm spotting now which should be quick to fix: your implementation seems to be giving the top output from a layer stack a thick wire, when it should be thin like it is in the current version.

@Keavon
Copy link
Member

Keavon commented Jan 10, 2025

Also one more observation: if you look very closely, the way your vertical wires line up with the grid (when the spacing is an even number), you will notice that the vertical line goes entirely to the right of the grid dots when it should straddle the grid evenly. So your current implementation has it drawing at a half-line width to the right.

@Stargazer10101
Copy link
Author

One issue that I'm spotting now which should be quick to fix: your implementation seems to be giving the top output from a layer stack a thick wire, when it should be thin like it is in the current version.

This was done so as to exactly match the UI mockup shown in the issue.
This is what it looks like otherwise:
Screenshot from 2025-01-10 10-21-38

Which is not identical with this(the mockup):
image

@Stargazer10101
Copy link
Author

Also one more observation: if you look very closely, the way your vertical wires line up with the grid (when the spacing is an even number), you will notice that the vertical line goes entirely to the right of the grid dots when it should straddle the grid evenly. So your current implementation has it drawing at a half-line width to the right.

Working on this.

@Keavon
Copy link
Member

Keavon commented Jan 10, 2025

The mockup contains that tapered triangular portion at the base of the vertical wire when it exits the layer, but the actual wire itself is still a thin wire because it represents non-stacking data. Eventually I will get around to adding that triangular taper but that is just a stylistic element.

@Stargazer10101
Copy link
Author

Also one more observation: if you look very closely, the way your vertical wires line up with the grid (when the spacing is an even number), you will notice that the vertical line goes entirely to the right of the grid dots when it should straddle the grid evenly. So your current implementation has it drawing at a half-line width to the right.

I have made some changes. This is how it looks now:
Screenshot from 2025-01-10 12-35-28

Requesting your feedback.

@Keavon
Copy link
Member

Keavon commented Jan 10, 2025

That looks good, thanks. I'd be happy to merge this after the rounding of odd numbered horizontal grid spaces to avoid vertical lines halfway through the grid lines (like in your bottom center vertical line in your latest screenshot). And after the situation is resolved where a node is brought from being right of its predecessor to instead being to the left, where the wires will need to deal with cases 5 and 6 in the diagram in the original issue.

@Stargazer10101
Copy link
Author

I've made some changes. The vertical lines now lie on the grid lines. The cases 5 and 6 of the diagram are demonstrated in the bottom left of the following image:
Screenshot from 2025-01-10 14-23-58

@Keavon
Copy link
Member

Keavon commented Jan 10, 2025

!build

@Stargazer10101 Stargazer10101 marked this pull request as ready for review January 10, 2025 11:02
Copy link

📦 Build Complete for d7744dc
https://893aba6f.graphite.pages.dev

@Keavon
Copy link
Member

Keavon commented Jan 10, 2025

Nice work on case 6. Case 5 still needs to be implemented:
capture

@Keavon
Copy link
Member

Keavon commented Jan 10, 2025

There is also a zone where this isn't working as desired, but it works further to the left and further to the right. Basically, case 6 should kick in earlier instead of letting the port's output get cut off. Demonstrated in this video:

capture_2_.mp4

@Stargazer10101
Copy link
Author

Stargazer10101 commented Jan 12, 2025

I've made some changes. For the case 5 implementation was simple. For the zones where the wires behaved weirdly, I wrote a few cases (where the weird behavior occurs) individually.
The case 5 implementation has been shown on the top right of the screenshot below.
The implementations for other zones have been solved as shown in the bottom left of the screenshot below.
The wires follow a path so that they lie exactly on the grid lines, but let me know if you want them to follow a different path in a particular case.
Feedback requested.
Screenshot from 2025-01-12 21-47-28

@Keavon
Copy link
Member

Keavon commented Jan 12, 2025

!build

@Keavon Keavon changed the title Right-angle wires in the node graph Right-angle grid-aligned wires in the node graph Jan 12, 2025
Copy link

📦 Build Complete for c576ab7
https://1b25d2df.graphite.pages.dev

@Keavon
Copy link
Member

Keavon commented Jan 12, 2025

Looking a lot closer! Here are a few more edge cases:

  • Shouldn't go through its own node:

capture
capture
capture
capture

  • Shouldn't turn immediately up/down from the connector socket, it should instead pop out from the node/layer area by at least one grid cell:

capture
capture
capture

@Keavon
Copy link
Member

Keavon commented Jan 13, 2025

Marking this as a draft for now while awaiting changes. Please mark it as ready for review when you've solved them so I know to come back to this PR. Thanks!

@Keavon Keavon marked this pull request as draft January 13, 2025 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants