Implement GraphFrame and integrate it in VisualShader

[Geometror]

This PR introduces GraphFrame, a new GraphElement designed for organizing node graphs, and integrates it into the VisualShader editor (can be split if desired).
It aims to reimplement the functionality that was lost when comment nodes were removed. While it works a bit differently (very similar to Blender) it should be a lot more predictable and much less problematic compared to the old implementation of comment nodes.

Finally closes godotengine/godot-proposals#5271.

godot.pr.frame.demo.mp4

Detailed changes:

• Add GraphFrame
  • A special graph element to which you can attach other graph elements (nodes and frames). Its rect automatically adapts to the attached elements (enclosing), if autoshrink is enabled. Only attached are moved together with the frame.
  • Can be tinted in an arbitrary color
  • GraphEdit automatically sorts the frames topologically (nested frames are supported)
  • GraphEdit automatically puts frames behind the connection layer
    • This requires to make the connection layer a non-internal child again (reintroduces GraphEdit's connection layer is not added as internal child #85005) since there is no better solution at the moment to keep it between background nodes, e.g. frames, and connectable nodes (just modifying the z_index of each node type would mess up the input and is just confusing)
• VisualShader integration
  • Remove VisualShaderNodeComment and replace it with VisualShaderNodeFrame
  • Right click context menu: Detach selected nodes from their "parent" frame
  • To attach nodes to a frame simply drag and drop them onto it.
  • Context menu option to enable/disable autoshrink and tint color (as well as choose the tint color)

API

Note for users of GraphEdit: Integrating GraphFrame in your application is a bit more complicated that the old comment nodes (where you could just check a property and it "worked" out of the box), especially for Undo-Redo, but in return it should work more reliable and be a lot more useful.

• GraphEdit
  • Methods
    • attach_graph_element_to_frame(element, frame) : Attach a node to a frame (params are string names)
    • detach_graph_element_from_frame(element) : Detach a node from its frame (param is string name)
    • get_frame(element) : Get the frame a node is attached to
  • Signals
    • frame_size_changed (frame, new_size) : Emitted when the size of a frame changes
    • nodes_linked_to_frame_request(elements, frame) : Emitted when nodes are dropped onto a frame
• GraphElement
  • Signals
    • resize_end : Emitted after releasing the mouse button after resizing the node
• GraphFrame
  • Properties
    • title : The title of the frame
    • autoshrink_enabled : Whether the frame should automatically shrink to fit its contents (attached nodes/frames)
    • autoshrink_margin : The margin to add to the frame size on each side when autoshrink is enabled
    • drag_margin : The margin around the frame where it can be dragged
    • tint_color_enabled : Whether the frame should be tinted in a color
    • tint_color : The color to tint the frame in
  • Methods
    • get_titlebar_hbox : Get the HBoxContainer of the titlebar (useful for customizing the titlebar)
  • Signals
    • autoshrink_toggled : Emitted when the autoshrink property is toggled
• VisualShader
  • Methods
    • attach_node_to_frame(node, frame) : Attach a node to a frame
    • detach_node_from_frame(node) : Detach a node from its frame

TODO:

• Theme cache
• Docs
• Address remaining TODO comments

Production edit: closes godotengine/godot-roadmap#13

[Yagich]

Amazing work! Can't wait to try it out.

I have a few questions:

1. Does it support the resizable property of GraphElement? If so, how does it behave with auto-shrink?
2. You say that GraphEdit's connection layer is not added as internal child #85005 is reintroduced, does that mean that to get the actual slot children we have to slice the get_children() call? That's quite a workflow change, but if there's no way around it I can live with it 🙂

I'll give it a spin tomorrow!

[Geometror]

Rebased and addressed all review comments.

@Yagich

1. Yes, it should. You can test it with the VS expression node which is resizable.
2. I assume you mean the connectable nodes (GraphNodes) with "slot children". Then yes, you would need to check whether the children are of type GraphElement or GraphNode (depending on whether you want the frame nodes too). Just stripping the first child from the children array would only work if you don't have any GraphFrames in there. What we could do however, is expose a method get_connection_layer_index so that you could just use that instead of type checking (but then again type checking is a bit safer and not that expensive).

[Yagich]

I've tested the PR and overall it works great! I've only been able to find one issue: if you try to attach a frame to itself, the application will hang. It's not unfixable in user code, but I wonder if it should just be a no-op if element and frame are the same when calling attach_graph_element_to_frame().

[KoBeWi]

Looks fine now (except the autoshrink_margin issue, but it's less important).
I left some style comments, otherwise it's good to go.

[lyuma]

@Geometror would it be possible to resolve the conflict with extension_api_validation / squash?

(I believe the conflict was caused by an urgent bugfix with Node::replace_by in #89992)

[Geometror]

Rebased and fixed a small resizing issue.
Initially, I wanted to split keep autoshrink always enabled (and split the free resize functionality from this PR), but I think it works well enough for now. Imo this is ready to merge :)

[akien-mga]

Thanks!

[Zylann]

This code might crash, as highlighted by this warning on my Linux CI: https://github.com/Zylann/godot_voxel/actions/runs/8560927646/job/23461023064

scene/gui/graph_frame.cpp:121:84: error: 'this' pointer is null [-Werror=nonnull]
  121 |                                         sb_panel_texture = sb_panel_flat->duplicate();
      |                                                            ~~~~~~~~~~~~~~~~~~~~~~~~^~

[akien-mga]

Reading the code, I think the two identifiers might be swapped by mistake?

[lyuma]

Remove VisualShaderNodeComment and replace it with VisualShaderNodeFrame

@Geometror Removing this part of our API is causing problems with some GDExtension plugins: it wasn't marked experimental, so this breaks all rust plugins since they hard-link to the whole API. See #90303

I think the easiest way to solve this is to add a stub class VisualShaderNodeComment : public VisualShaderNodeFrame so it will work just like the new VisualShaderNodeFrame. If there is a way to register the GDCLASS without it showing up in the resource picker, that would be best.

Even though it extends from VisualShaderNodeFrame, it might also be necessary to implement bind_compatibility_methods() for compat versions of set/get_title and set/get_description that call the real versions of those methods.

[Geometror]

@lyuma That's a good idea to derive from VisualShaderNodeFrame! I just opened #90797.

[eobet]

It would be easier to find this if when you search for "comment" it would also bring up the frame...