Details

Reviewers

Hans Goudey (HooglyBoogly)

Commits

rB29e354519480: Geometry Nodes: String manipulation nodes

Summary

This patch adds four new nodes:

String Length (outputs length of a string)
String Substring (outputs part of a string)
Value to String (converts a value to a string)
String Join (concatenates multiple strings with a specified delimiter)
~~Text to String (converts a text data-block to a string)~~ Postponed

Diff Detail

Repository: rB Blender

Event Timeline

Erik Abrahamsson (erik85) requested review of this revision.Sep 17 2021, 1:34 AM

Erik Abrahamsson (erik85) created this revision.

This all looks pretty straightforward to me. Just a few small comments.

source/blender/makesrna/intern/rna_nodetree.c
10320 ↗	(On Diff #41953)	`Text Block` -> `Text Data-block` maybe? I don't think "Text block" is a thing.
source/blender/nodes/NOD_static_types.h
271–353	I don't think the extra space is necessary before the enum names. Not sure why it's there for the random float node.
source/blender/nodes/geometry/nodes/node_geo_string_join.cc
31–32	const
source/blender/nodes/geometry/nodes/node_geo_text_to_string.cc
36 ↗	(On Diff #41953)	What's up with this? Is the reload operator necessary?
39–42 ↗	(On Diff #41953)	Is this necessary? I would think it would be cleared by default.
68 ↗	(On Diff #41953)	I don't think the geometry class is right for this node, it doesn't work with geometry. Probably the "Input" category would make more sense.

This revision now requires changes to proceed.Sep 17 2021, 3:48 AM

Fixed the things in review comments.

Erik Abrahamsson (erik85) marked an inline comment as done.Sep 17 2021, 11:57 PM

Erik Abrahamsson (erik85) added inline comments.

source/blender/nodes/geometry/nodes/node_geo_text_to_string.cc
36 ↗	(On Diff #41953)	I will remove it for now. The node doesn't detect changes in the datablock.

You mentioned that the re-evalution operator might be complicated in chat, but I'm not sure-- couldn't it just call rna_Node_update to trigger re-evaluation?
The lack of automatic updates for the text block would feel much more purposeful with that operator.

Paul Larson (GeorgiaPacific) awarded a token.Sep 20 2021, 5:45 PM

Paul Larson (GeorgiaPacific) added a subscriber: Paul Larson (GeorgiaPacific).

Evan Wilson (EAW) awarded a token.Sep 20 2021, 9:43 PM

Removed Text To String until there is a solution to reload the Text data-block.

Hans Goudey (HooglyBoogly) accepted this revision.Sep 20 2021, 11:26 PM

Hans Goudey (HooglyBoogly) added inline comments.

source/blender/nodes/geometry/nodes/node_geo_string_join.cc
41	This can use `std::move(output)` to avoid an allocation duplicating the string.

This revision is now accepted and ready to land.Sep 20 2021, 11:26 PM

output -> std::move(output)

Requesting changes based on conversations in chat about properly supporting utf8 for the lengths.

This revision now requires changes to proceed.Sep 21 2021, 4:50 PM

Now handling UTF-8 encoding correctly.

Erik Abrahamsson (erik85) updated this revision to Diff 42194.Sep 21 2021, 8:22 PM

This revision was not accepted when it landed; it landed in state Needs Review.Sep 21 2021, 9:11 PM

Closed by commit rB29e354519480: Geometry Nodes: String manipulation nodes (authored by Erik Abrahamsson (erik85), committed by Hans Goudey (HooglyBoogly)). · Explain Why

This revision was automatically updated to reflect the committed changes.

Hans Goudey (HooglyBoogly) added a commit: rB29e354519480: Geometry Nodes: String manipulation nodes.

I forgot to accept this, oops. But it solves the problems Jacques mentioned in chat, which were, for the record:

Should the value of the multi-input be hidden? Doesn't seem to make sense to join a single string.
The String Length node shouldn't output the number of bytes, but should be utf8 aware (BLI_strlen_utf8).
str_len_fn should probably take a const std::string & if possible

I did a little cleanup to the clamping in the substring function and variable names in the commit.

Vyacheslav (hitrpr) awarded a token.Sep 29 2021, 4:34 PM

String manipulation nodes
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 42193

release/scripts/startup/nodeitems_builtins.py

source/blender/blenkernel/BKE_node.h

source/blender/blenkernel/intern/node.cc

source/blender/nodes/CMakeLists.txt

source/blender/nodes/NOD_function.h

source/blender/nodes/NOD_geometry.h

source/blender/nodes/NOD_static_types.h

source/blender/nodes/function/nodes/node_fn_string_length.cc

source/blender/nodes/function/nodes/node_fn_string_substring.cc

source/blender/nodes/function/nodes/node_fn_value_to_string.cc

source/blender/nodes/geometry/nodes/node_geo_string_join.cc

String manipulation nodesClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 42193

release/scripts/startup/nodeitems_builtins.py

source/blender/blenkernel/BKE_node.h

source/blender/blenkernel/intern/node.cc

source/blender/nodes/CMakeLists.txt

source/blender/nodes/NOD_function.h

source/blender/nodes/NOD_geometry.h

source/blender/nodes/NOD_static_types.h

source/blender/nodes/function/nodes/node_fn_string_length.cc

source/blender/nodes/function/nodes/node_fn_string_substring.cc

source/blender/nodes/function/nodes/node_fn_value_to_string.cc

source/blender/nodes/geometry/nodes/node_geo_string_join.cc

String manipulation nodes
ClosedPublic
Actions

Revision Contents
Changeset List