Page MenuHome

Added New String Input Node
ClosedPublic

Authored by Edgar Roman Cervantes (redvant) on Feb 5 2021, 3:44 AM.

Details

Summary

This patch addresses: T84971 Create a string input node

This node has for objective be used in the new attribute workflow in the Geometry Nodes

Things to consider about my implementation:

  • I made the new node a Function node pretty similar to the existing Vector Node
  • I created a new DNA struct to store the string property and used char string[1024]. Now uses char *string.

Possible future improvements:
It was proposed to combine all the input nodes into one that could manage all types of input.

The end result looks like this:

Here is the test scene I used (updated to work with the changes):

Diff Detail

Repository
rB Blender

Event Timeline

Edgar Roman Cervantes (redvant) requested review of this revision.Feb 5 2021, 3:44 AM
Edgar Roman Cervantes (redvant) created this revision.

Thanks for the patch. Can you update it for latest master?

source/blender/makesdna/DNA_node_types.h
1144

1024 seems a bit excessive, but not sure what value it should be.
Could also use char *, but that makes the rna and node related stuff more annoying...

@Hans Goudey (HooglyBoogly) what do you think?

source/blender/nodes/function/nodes/node_fn_input_string.cc
35

"input string node" -> __func__

source/blender/makesdna/DNA_node_types.h
1144

char * sounds like a good alternative. In bNodeSocketValueString, 1024 is used as FILEMAX so long strings are probably nice to support? Though if eventually paragraphs of text are needed data-blocks will be better than entering this stuff in the little widget in the node.

Updating D10316: Added New String Input Node

Update to latest master

Hans Goudey (HooglyBoogly) requested changes to this revision.Feb 10 2021, 6:48 PM

Jacques and I talked a bit about this a couple days ago. The 1024 byte string size means that every string input node needs 1KB of memory, which is quite excessive for most use cases. That could really add up if there were a lot of string input nodes.

The string should probably be stored as a char *. That will make the RNA code and the node storage copying a bit more complicated (custom copying and freeing for the storage, custom get, set, and length functions for the string in RNA).

source/blender/nodes/function/nodes/node_fn_input_string.cc
35

This comment isn't addressed yet.

This revision now requires changes to proceed.Feb 10 2021, 6:48 PM

@Hans Goudey (HooglyBoogly) @Jacques Lucke (JacquesLucke) FYI I was talking to Sergey and we kind of agree that although it would be nice to have RNA to better handle dynamic strings. However for this particular case it is a bit of overkill.

  • Custom storage NodeInputString now uses char *
  • Merge remote-tracking branch 'origin/master' into arcpatch-D10316
  • Added layout callbacks to node_fn_input_string.cc to match with cfa48c84d06c
  • Fix to errors produced when merging the latest master

I had some problems because many changes are being made to the geometry nodes code.
I added the changes that @Hans Goudey (HooglyBoogly) asked for.
I took as reference the code used for node_shader_script, so that help me a lot.
I'm still getting familiar to all the parts involved with the nodes so I'm not sure if something is missing, so let me know.

This looks good to me. I will include some cleanups in the commit.

This revision is now accepted and ready to land.Feb 19 2021, 10:46 PM