Adds the distribute points in volume node.
Uses almost entirely the work by @Kenzie (kenziemac130) , so full credit to them for the node.
Adds a stable id field output to the grid mode, which uses the float3::hash to hash
each points position.
Details
Details
- Reviewers
Hans Goudey (HooglyBoogly) Jacques Lucke (JacquesLucke) - Maniphest Tasks
- T85898: Geometry Nodes: Distribute Points in Volume
Diff Detail
Diff Detail
- Repository
- rB Blender
- Branch
- T85898-points-in-volume (branched from master)
- Build Status
Buildable 17397 Build 17397: arc lint + arc unit
Event Timeline
Comment Actions
- fix rest of merge conflict shenanigans
- update to use modify_geometry_sets, removed shallow transform stuff :(
- update to work with latest master, remove stable id
Comment Actions
Sorry for the delay in reviewing!
- The code is certainly much simpler now!
- There is no stability with the random distribution mode, but I think that's okay. We're using the OpenVDB implementation anyway, and the node is still helpful without it.
- Did you verify that the stable ID didn't work with the grid mode? It seems like a straightforward idea to hash the positions, similarly to the distribute on faces node.
- I think the node should be wide enough by default to display the full name. Something similar is done for the other distribute node.
- It looks like the points are offset from the origin, maybe by half of the voxel size?
- I see there is some handling of that in the code, but maybe it's not working as expected? Maybe convert_to_grid_index_space is helpful.
- Generally I would expect to have a point at the origin.
- Let me know if I'm forgetting something here.
| source/blender/nodes/geometry/nodes/node_geo_distribute_points_in_volume.cc | ||
|---|---|---|
| 110 | This can probably look like the conversion in ParticleList for the points to volume node. | |
| 119–120 | It would be helpful to have a comment about the choice of random number generator here. | |
| 150 | This can use continue rather than indenting the rest of the loop. | |
| 206 | This should still have a keep_only to remove the volume from the geometry set. | |
Comment Actions
Honestly I didn't, I saw it was removed in the distribute points on faces node and assumed that the stable id node being developed would do the same as what I was accomplishing with stable id. Whilst I never tested it I still had doubts as I can't image a hash of a double would produce the same value across frames, as surely the grid calculations result in small discrepancies due to floating point errors?
- I think the node should be wide enough by default to display the full name. Something similar is done for the other distribute node.
Will address this
- It looks like the points are offset from the origin, maybe by half of the voxel size?
- I see there is some handling of that in the code, but maybe it's not working as expected? Maybe convert_to_grid_index_space is helpful.
- Generally I would expect to have a point at the origin.
I think this can be addressed, but I'm not sure the expected behaviour is a point at the origin. When changing the spacing by a multiple of 2 (eg from 0.3 to 0.6) the positions will stay the same but with more points filling in between, however if its changed by a different amount the the positions will change completely. I'll see if I can fix it though
Comment Actions
I saw it was removed in the distribute points on faces node
The change for that node was to output it as a named attribute instead of an anonymous attribute, the other distribute node still outputs that data. But yeah, it's worth testing if the hash works I think.
Comment Actions
Hi, yes please! I'm sorry I've not been updating it, life has got in the way unfortunately. I think it's almost done to be honest, there's not a ton left to fix I don't think.
Comment Actions
Ok, im trying to update the patch here but it doesn't let me, I think I need to create a new patch, will credit you and Kenzie