Nodes: unify static and dynamic declarations #113742

Jacques Lucke · 2023-10-15T13:44:45+02:00

Jacques Lucke commented

2023-10-15 13:44:45 +02:00

This helps solving the problem encountered in #113553. The problem is that we currently can't support link-drag-search for nodes which have a dynamic declaration.

With this patch, there is only a single declare function per node type, instead of the separate declare and declare_dynamic functions. The new declare function has access to the node and tree. However, both are allowed to be null. The final node declaration has a flag for whether it depends on the node context or not.

Nodes that previously had a dynamic declaration should now create as much of the declaration as possible that does not depend on the node. This allows code like for link-drag-search to take those sockets into account even if the other sockets are dynamic.

For node declarations that have dynamic types (e.g. Switch node), we can also add extra information to the static node declaration, like the identifier of the socket with the dynamic type. This is not part of this patch though.

I can think of two main alternatives to the approach implemented here:

Define two separate functions for dynamic nodes. One that creates the "static declaration" without node context, and on that creates the actual declaration with node context.
Have a single declare function that generates "build instructions" for the actual node declaration. So instead of building the final declaration directly, one can for example add a socket whose type depends on a specific rna path in the node. The actual node declaration is then automatically generated based on the build instructions. This becomes quite a bit more tricky with dynamic amounts of sockets and introduces another indirection between declarations and what sockets the node actually has.

I found the approach implemented in this patch to lead to the least amount of boilerplate (doesn't require a seperate "build instructions" data structure) and code duplication (socket properties are still only defined in one place). At the same time, it offers more flexibility to how nodes can be dynamic.

This helps solving the problem encountered in #113553. The problem is that we currently can't support link-drag-search for nodes which have a dynamic declaration. With this patch, there is only a single `declare` function per node type, instead of the separate `declare` and `declare_dynamic` functions. The new `declare` function has access to the node and tree. However, both are allowed to be null. The final node declaration has a flag for whether it depends on the node context or not. Nodes that previously had a dynamic declaration should now create as much of the declaration as possible that does not depend on the node. This allows code like for link-drag-search to take those sockets into account even if the other sockets are dynamic. For node declarations that have dynamic types (e.g. Switch node), we can also add extra information to the static node declaration, like the identifier of the socket with the dynamic type. This is not part of this patch though. I can think of two main alternatives to the approach implemented here: * Define two separate functions for dynamic nodes. One that creates the "static declaration" without node context, and on that creates the actual declaration with node context. * Have a single declare function that generates "build instructions" for the actual node declaration. So instead of building the final declaration directly, one can for example add a socket whose type depends on a specific rna path in the node. The actual node declaration is then automatically generated based on the build instructions. This becomes quite a bit more tricky with dynamic amounts of sockets and introduces another indirection between declarations and what sockets the node actually has. I found the approach implemented in this patch to lead to the least amount of boilerplate (doesn't require a seperate "build instructions" data structure) and code duplication (socket properties are still only defined in one place). At the same time, it offers more flexibility to how nodes can be dynamic.

Jacques Lucke added 1 commit 2023-10-15 13:45:01 +02:00

7a3143cefc unify dynamic and static declarations

Jacques Lucke added 1 commit 2023-10-15 14:18:19 +02:00

buildbot/vexp-code-patch-coordinator Build done. Details

05a9c2b80c cleanup

Jacques Lucke commented

2023-10-15 14:20:14 +02:00

@blender-bot build

Jacques Lucke requested review from Lukas Tönne 2023-10-15 14:20:28 +02:00

Jacques Lucke requested review from Iliya Katushenock 2023-10-15 14:20:29 +02:00

Jacques Lucke requested review from Hans Goudey 2023-10-15 14:20:29 +02:00

Hans Goudey approved these changes 2023-10-15 18:24:01 +02:00

Hans Goudey left a comment

The change in the code is simpler than I expected, that looks good. I like that we can automatically detect when the declaration is context dependent. Nice to avoid redundancy that way.

The big question this raises is how we store things like dynamic socket types or amounts in the "static" declaration, or I guess even the extent to which we have to. One benefit to storing more of that sort of thing is that it could make the link-drag-search definitions simpler. But I'm confident it's possible to solve that in a nice way, so I'll accept this now.

The change in the code is simpler than I expected, that looks good. I like that we can automatically detect when the declaration is context dependent. Nice to avoid redundancy that way. The big question this raises is how we store things like dynamic socket types or amounts in the "static" declaration, or I guess even the extent to which we have to. One benefit to storing more of that sort of thing is that it could make the link-drag-search definitions simpler. But I'm confident it's possible to solve that in a nice way, so I'll accept this now.

Iliya Katushenock approved these changes 2023-10-15 19:15:43 +02:00

Iliya Katushenock left a comment

Would be good to note in somethere the fact that any declaration should be able to be built at least single time without any context.

Iliya Katushenock commented

2023-10-15 19:15:54 +02:00

It seems, what link searcher now have to create dummy node with settings (the best settings for other socket to create connection to a dynamically-typed) and build context-dependent declaration\

Iliya Katushenock referenced this pull request

2023-10-15 19:47:09 +02:00

Refactoring: Nodes: Dynamic declaration for dynamically-typed nodes #113553

Jacques Lucke commented

2023-10-15 20:25:35 +02:00

Would be good to note in somethere the fact that any declaration should be able to be built at least single time without any context.

One should always be able build it without context (not just a single time).

It seems, what link searcher now have to create dummy node with settings (the best settings for other socket to create connection to a dynamically-typed) and build context-dependent declaration

The goal is that all information that link-drag-search needs can be put into the context-free declaration. However, that is not fully implemented by this patch as mentioned in the description.

> Would be good to note in somethere the fact that any declaration should be able to be built at least single time without any context. One should always be able build it without context (not just a single time). > It seems, what link searcher now have to create dummy node with settings (the best settings for other socket to create connection to a dynamically-typed) and build context-dependent declaration The goal is that all information that link-drag-search needs can be put into the context-free declaration. However, that is not fully implemented by this patch as mentioned in the description.

Jacques Lucke merged commit 38813a7441 into main

2023-10-15 20:28:32 +02:00

Jacques Lucke referenced this issue from a commit

2023-10-15 20:28:33 +02:00

Nodes: unify static and dynamic declarations

Jacques Lucke deleted branch unify-declarations

2023-10-15 20:28:34 +02:00

Iliya Katushenock referenced this pull request

2023-11-18 13:18:25 +01:00

WIP: Refractor: Compositor: Render layer node to dynamic declaration #114629

Sign in to join this conversation.

No reviewers