Page MenuHome

Geometry Nodes: Fields version of Attribute Randomize as a Function Node
ClosedPublic

Authored by Johnny Matthews (guitargeek) on Sep 22 2021, 7:45 PM.

Details

Summary

This node populates a Vector,Float, Int or Bool field with random values

Vector, Float, and Int have min/max settings, which are also field aware.

All 4 have a field seed input which can be driven by the index input node, otherwise, all values are the same "random" value.

Diff Detail

Repository
rB Blender

Event Timeline

Johnny Matthews (guitargeek) requested review of this revision.Sep 22 2021, 7:45 PM
Johnny Matthews (guitargeek) created this revision.

Some notes of my thoughts on this matter:

  • I think having a single node with different datatype modes is totally fine
  • I do think we should also include the different random distributions in this node that were worked out in this patch D10459 (exactly in its latest state)
  • For a boolean output it would be nice to have a probability ratio parameter (p_True = 0.5)
  • It was mentioned somewhere that this node (randomization nodes in general) could have a specific node seed. I think that is a great idea, I brought up the issue of changing seeds between nodes in the past. This does result logically in 3 Seed inputs though:
    • node seed (randomly changed on node adding/ maybe duplication)
    • index (not exposed slider, if nothing is plugged in it used the Index field)
    • seed (seed to be changed on top of the node seed, equivalent to what this node had previously. it allows to expose a single seed to the modifier interface for example)

For the index input:

  • I would like to retain the functionality that generated points from the point distribute node to generate stable random numbers by default. So maybe the index could look for the generated hashed id first and then fallback to the Index field

@Johnny Matthews (guitargeek) I abandoned D10459, please use as you wish, the additional code was adapted from python functions.

  • It was mentioned somewhere that this node (randomization nodes in general) could have a specific node seed. I think that is a great idea, I brought up the issue of changing seeds between nodes in the past. This does result logically in 3 Seed inputs though:
    • node seed (randomly changed on node adding/ maybe duplication)
    • index (not exposed slider, if nothing is plugged in it used the Index field)
    • seed (seed to be changed on top of the node seed, equivalent to what this node had previously. it allows to expose a single seed to the modifier interface for example)

For the index input:

  • I would like to retain the functionality that generated points from the point distribute node to generate stable random numbers by default. So maybe the index could look for the generated hashed id first and then fallback to the Index field

I mentioned an idea about seed offset which seems to be similar to a combination of the second and third option you typed here. My idea was that the seed slider would actually act as an offset value for the Index field under the hood, and this behavior would be able to be disabled by a checkbox or something if you want a constant random number. What do you people think about this idea?

In real life practise random number over different index is used in way more cases than a constant random number so I just believe the default behavior should do the index thing on its own. But we still need a value for the modifier so I came up with the offset idea.

Rename to: Random Value
Added ID seed (implicit index does not work yet)
Deprecated Random Float node

Not a full review, didn't test it yet. Just got a few small notes already.

source/blender/functions/FN_field.hh
493 ↗(On Diff #42320)

Move implementation of this method to field.cc.

source/blender/makesdna/DNA_node_types.h
1228

outdated comment

source/blender/nodes/function/nodes/node_fn_random_value.cc
167 ↗(On Diff #42320)

The blender:: is not necessary.

287 ↗(On Diff #42320)

add empty line before

source/blender/makesdna/DNA_node_types.h
1231

Structs in DNA shouldn't just be renamed, because these names are written into .blend files. Just don't change the name at all here.

Johnny Matthews (guitargeek) marked 5 inline comments as done.

Correct Implicit Index Error
Reversed renaming of RNA for legacy node
Other fixes from Jaques' notes

Add field indicators to declaration

Jacques Lucke (JacquesLucke) requested changes to this revision.Sep 24 2021, 4:57 PM
Jacques Lucke (JacquesLucke) added inline comments.
source/blender/nodes/function/nodes/node_fn_random_value.cc
57 ↗(On Diff #42355)

Use dependent_field instead of field_source, because these outputs are only fields when at least one of the inputs is a field. field_source should be used when the output is a field, independent of what the inputs are.

137 ↗(On Diff #42355)

Use the new noise::hash_to_float from BLI_noise.hh. then you can also just use 0, 1 and 2 as the last input. Same in the other functions of course.

250 ↗(On Diff #42355)

That still needs a probability input.

This revision now requires changes to proceed.Sep 24 2021, 4:57 PM
Johnny Matthews (guitargeek) marked 3 inline comments as done.

Use noise::hash_to_float()
Make outputs dependent field type
Add Probability input for bool

source/blender/nodes/function/nodes/node_fn_random_value.cc
18 ↗(On Diff #42386)

Double license block.

56 ↗(On Diff #42386)

hide_value will be called implicitly in implicit_field now.

256 ↗(On Diff #42386)

The clamping shouldn't be necessary.

Remove extra header
remove hide_value()
remove clamping

Looks good, nice work Johnny. I'll land this now.

source/blender/nodes/function/nodes/node_fn_random_value.cc
251 ↗(On Diff #42389)

I'll use a switch here in the final commit.

This revision is now accepted and ready to land.Sep 24 2021, 8:36 PM
This revision was automatically updated to reflect the committed changes.