This patch turns attribute search in string input sockets off by default,
and adds a function is_attribute_name() to SocketDeclaration to enable them.
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
Event Timeline
Comment Actions
So far this looks great.
I think this should also disable logging of the list of attributes when it isn't needed though. The changes in geometry_nodes_eval_log.cc should be relatively simple, the declaration could be passed to log_value_for_sockets possibly.
This comment was removed by Erik Abrahamsson (erik85).
Comment Actions
I came up with a way to disable attribute logging without passing the NodeDeclaration.
Comment Actions
Looks good to me, just requesting a couple small changes. Best if Jacques sees this too though.
| source/blender/nodes/intern/geometry_nodes_eval_log.cc | ||
|---|---|---|
| 163 | If this was used a lot, the = false might be helfpul, but since it's just called once, I would suggest leaving the default out for now. | |
| 361 | Maybe it would be more correct to call this "log_attributes" | |
Comment Actions
Why is this part of NodeDeclaration and not SocketDeclaration? Feels like it would make more sense for this to be part of the socket declaration, because then a node can have some inputs that need attribute search and other inputs that don't need it.
| source/blender/nodes/intern/geometry_nodes_eval_log.cc | ||
|---|---|---|
| 362 | This should check if any of the passed in sockets needs attribute search. Only checking sockets[0] is not enough. FYI, multiple sockets are passed in for the same geometry when they have the same value, e.g. when a geometry is passed through reroute nodes, group inputs/outputs, ... | |
Comment Actions
Rewritten to be per socket instead of per node.
It might be totally unnecessary to add the SocketDeclaration to DNA, but it was the only way I could think of.
Comment Actions
Think I'd prefer it if we can do this without adding declaration to sockets for now. Need to think about more about the implications with respect to the life-time of the declaration, behavior when copying nodes, ...
| source/blender/nodes/NOD_node_declaration.hh | ||
|---|---|---|
| 91 | This should be in decl::String because it is specific to string sockets. | |
Comment Actions
I committed a simpler version of the patch for now: rBd1fcf93f039b0546dfd01c33daf50bd135e34344.