Dynamic node declarations don't update reliably #106732

New Issue

Lukas Tönne · 2023-04-09T10:14:46+02:00

Lukas Tönne commented

2023-04-09 10:14:46 +02:00

Dynamic node declarations are intended for nodes whose socket layout changes based on settings or external dependencies. Currently their only use is in node groups, future uses will include "zone" nodes such as the simulation zone.

Generating sockets based on dynamic declarations crashes easily because the general update function nodeDeclarationEnsure expect sockets to match the declaration in certain code paths, which defeats the purpose of dynamic declarations.

For example: copying a node into a different node tree may invalidate a reference, such as the output_node_id of simulation input nodes. This will cause the declaration to be cleared, while the node sockets remain unchanged. An operator like clipboard.paste will then call nodeDeclarationEnsure which crashes in update_socket_declarations.

During node tree updates the update_node_declaration_and_sockets is used to also modify the socket lists to match dynamic declarations. However, this update happens at the end of operators, where node declarations may already have been updated and crashed.

`nodeDeclarationEnsure`	`update_socket_declarations`

nodeDeclarationEnsure should run refresh_node after a build_node_declaration_dynamic. This will update sockets if they don't match the declaration.
Optional: Have a way to detect declaration changes, so nodeDeclarationEnsure can run a dynamic update. Currently it only runs dynamic updates once when the declaration is null. Dynamic nodes rely on operators calling update_socket_declarations explicitly to redo the declaration.

Dynamic node declarations are intended for nodes whose socket layout changes based on settings or external dependencies. Currently their only use is in node groups, future uses will include "zone" nodes such as the [simulation zone](https://code.blender.org/2022/11/geometry-nodes-workshop-2022/#simulation). Generating sockets based on dynamic declarations crashes easily because the general update function `nodeDeclarationEnsure` expect sockets to match the declaration in certain code paths, which defeats the purpose of dynamic declarations. For example: copying a node into a different node tree may invalidate a reference, such as the [`output_node_id`](https://projects.blender.org/blender/blender/src/commit/e665a50fb6364142198d0b8cff44b0730709e4bc/source/blender/makesdna/DNA_node_types.h#L1606) of simulation input nodes. This will cause the declaration [to be cleared](https://projects.blender.org/blender/blender/src/branch/geometry-nodes-simulation/source/blender/nodes/geometry/nodes/node_geo_simulation_input.cc#L104-L122), while the node sockets remain unchanged. An operator like `clipboard.paste` will then [call `nodeDeclarationEnsure`](https://projects.blender.org/blender/blender/src/commit/e665a50fb6364142198d0b8cff44b0730709e4bc/source/blender/editors/space_node/clipboard.cc#L241) which crashes in [`update_socket_declarations`](https://projects.blender.org/blender/blender/src/commit/e665a50fb6364142198d0b8cff44b0730709e4bc/source/blender/blenkernel/intern/node.cc#L3641). During node tree updates the `update_node_declaration_and_sockets` is used to also modify the socket lists to match dynamic declarations. However, this update happens at the end of operators, where node declarations may already have been updated and crashed. |`nodeDeclarationEnsure`|`update_socket_declarations`| |---|---| |![image](/attachments/69b8f27f-dbe1-41ee-8b92-d8851886f64f)|![image](/attachments/318c7f9c-f7eb-416f-afad-a4b4d45ef7f9)| - [ ] `nodeDeclarationEnsure` should run `refresh_node` after a `build_node_declaration_dynamic`. This will update sockets if they don't match the declaration. - [ ] Optional: Have a way to detect declaration changes, so `nodeDeclarationEnsure` can run a dynamic update. Currently it only runs dynamic updates once when the declaration is null. Dynamic nodes rely on operators calling `update_socket_declarations` explicitly to redo the declaration.

image.png

36 KiB

image.png

26 KiB

👀 1

Lukas Tönne added the

Type

To Do

label 2023-04-09 10:14:46 +02:00

Lukas Tönne added this to the Nodes & Physics project 2023-04-09 10:14:47 +02:00

~~Lukas Tönne referenced this issue 2023-04-11 15:48:47 +02:00~~

Support for simulation zones in copy operators #106812