Rework GraphEdit connections (drawing, API, optimizations)

[Geometror]

Supersedes #83508.

Detailed changes

API

• Add method get_closest_connection_at_point which return the closest connection at a given point in GraphEdit's local screen space (with the given maximum search distance). See the docs for a usage example.
• Add method get_connections_intersecting_with_rect
• Add method reset_all_connection_activity

Visual

• Adjust theming/visual appearance of connection lines
  • Add a tiny rim so that they can be distinguished better when they cross/overlay each other (stripped from Redesign the graph editor for visual shaders #85017)
    • For easy adjustments a connection_rim_color theme property is added
  • Hovering a connection now highlights it (theme item connection_hover_tint_color)
  • The highlight color when dragging a connection onto a valid port is now a theme item (theme item connection_valid_target_tint_color)
• Dragging connection line improvements
  • Show the correct color gradient when hovering over a valid port (it was just a solid color before)
    
  • Fix instability while zooming
    When zooming/panning fast this could happen before:
    

Optimizations, Fixes & Refactoring

• Optimize connection drawing/handling

  • Introduce connections HashMap (node -> its connections)
    • The old logic iterated over all connections for every GraphNode, now only the relevant connections are checked where possible.
  • Each connection now caches the line, the start and end positions (in screen space) as well as an AABB. This allows for updating/drawing only those connections visible on the screen (this had a huge performance impact for large graphs). It's also used to accelerate fetching the connection from a screen position/rect intersection which is introduced in this PR.
  • Further improved performance (especially when panning and moving nodes)

• Connection lines are now drawn using Line2D nodes instead of using draw_polyline calls in only one CanvasItem

  • The performance concerns I had turned out to be unfounded, this method even allows for some nice optimizations
  • The reason for this was flexibility: e.g. drawing e.g. type-dependent connection line styles (double/triple/quadruple lines, dashed) with a shader requires passing per-line uniforms to the shaders

• Connection line shader

  • Connection lines use a dedicated canvas item shader now
  • The default shader can be overridden to allow for application specific shaders (e.g. to implement connection type dependent styles)
    • The method is not yet exposed
  • Parameters/uniforms currently set by GraphEdit:
    • int from_type
    • int to_type
    • vec4 rim_color
    • float line_width
  • This allows for type-dependent connection line styles, like you can see here:
    Keep in mind that the following is just a quick/arbitrary POC using the shader override and not included in this PR
    

• Drawing the lines for the minimap is now completely separate, drawing the lines via a shader is not sensible here (this also paves the way for loosening up the coupling between the minimap and GraphEdit itself and allows for optimizations since the minimap is currently a bottleneck)

• Change signature of get_connection_list (now returns the list instead of using a return parameter) [only used internally]

• Fixes Deleting GraphNodes when handling delete_nodes_request causes connection visuals to leak #36716.

TODO

• Decide what to do with the anti aliased property since AA is basically free now
• Expose the connection line shader override (maybe in a followup PR?)

[nicolasgustafsson]

The active connection line looks a little ugly when dragging in the gif you posted, is it possible to draw it behind the port?

[YuriSizov]

Out of curiosity, why did you decide to supersede your other PR instead of pushing an update to it? Also what exactly breaks compatibility here?

[Calinou]

Decide what to do with the anti aliased property since AA is basically free now

I'd probably leave it in, as I remember people using GraphNode/GraphEdit in games with a pixel art style.

[Geometror]

@nicolasgustafsson There is definitely some room for improvement there, but since the whole line needs to be on top we can't draw it behind the port. However, I have some ideas, e.g. you could fade the rim (which would require passing the line length as an uniform).

@YuriSizov Well, although this PR includes the features of the other one, the approach for updating/rendering the connections is quite different. Since that was a big part of the other PR and @ats already approved it, I thought it would be better to open a separate PR instead of updating the existing one because a lot has changed.
Regarding the breaks-compat tag: it's just minor things like the following, so I added it just in case:

• I'm still fiddling with the connection lines thickness property there (the shader and the width of the generated polyline need to be coordinated/tuned); currently it has a different scale (which should better resemble the width in pixels) and therefore an adjusted default value to compensate this
• the unexposed, but public method get_top_layer (which seems to be unused as I just have noticed) returns the top layer which no longer draws the dragged connection/handles and filters the input for it
• the anti-aliasing property currently only anti-aliases the connections in the minimap (of course this point depends on our decision regarding that property)
• well, some of the improvements (like the rim or the applied gradient on hovering a port) change the functionality and aren't really optional (although you can turn the rim off by specifying a transparent rim color)

@Calinou Well in that case a connection line with AA turned off might be still too smooth, for something like this you're able to override the default connection line shader and achieve a true pixelated look. You can do basically everything you like as you can see in the example below ;)

[YuriSizov]

I thought it would be better to open a separate PR instead of updating the existing one because a lot has changed.

From the production point of view, you can just dismiss a stale review to indicate that your work needs to be reviewed again (and of course I would assume you leave a comment with an explanation of updates anyway). So for everyone else's benefit this wasn't necessary. But if it helps you, then opening a new PR is fine. Feel free to close the old one if you're replacing it.

As for breaking changes, if they are purely cosmetic and don't change public API in any way, we don't need to label it as breaking changes.

[AThousandShips]

As the old PR didn't have review stuff on it I'd be fine but otherwise I'd suggest avoiding superseding PRs unless most of the feedback on it is significantly outdated, to not lose a lot of progress in the feedback there

[QbieShay]

There is definitely some room for improvement there, but since the whole line needs to be on top we can't draw it behind the port. However, I have some ideas, e.g. you could fade the rim (which would require passing the line length as an uniform).

Why does it need to draw on top of the nodes?

[Geometror]

Well, in a dense graph you would only see tiny segments of your dragged connection line which makes it quite unfun/a bit awkward to use. (Every graph editor I tried so far draws it over everything else while dragging, so Godot isn't an exception here)

[EnlightenedOne]

Cant you just redraw the connecting circle along with the line over the top of everything in order to have both? Slightly more faff granted but that is what Blender does:

[YuriSizov]

Overall code seems fine, good job! Functionality-wise it all seems to work. Helper methods for geometry seem fine as well. So once the feedback is addressed, this should be good to go.

---

I also noticed an issue with visual shaders as I was testing. Reported here #87337

[Geometror]

I think I addressed all the review comments :)

[YuriSizov]

Builds failing because constness change needs to be added to the extension compatibility file

[YuriSizov]

Aside from the compatibility check, it's all good 👍

[Geometror]

I also forgot to adjust the commit message. Both should be fixed now.

[AThousandShips]

You need to add a compatibility method

[Geometror]

@AThousandShips I'm not too familiar with the compat method binding stuff, but I think I got it now :)

[YuriSizov]

Thanks!

[MewPurPur]

I found this in git blame as I remembered that it upsets Aaronfranke's attempt to implement a disable_2d build option, forcing it to make an exception for Line2D. Would it make sense to use draw_polyline on a Control with the same shader instead?

Connection lines are now drawn using Line2D nodes instead of using draw_polyline calls in only one CanvasItem

The performance concerns I had turned out to be unfounded, this method even allows for some nice optimizations
The reason for this was flexibility: e.g. drawing e.g. type-dependent connection line styles (double/triple/quadruple lines, dashed) with a shader requires passing per-line uniforms to the shaders

[Geometror]

draw_polyline is not sufficient (line caps and UVs are needed), but since Line2D uses another abstraction (LineBuilder) we could do something like that (basically by using the same drawing code as in Line2D).