Details

Reviewers

Hans Goudey (HooglyBoogly)
Jacques Lucke (JacquesLucke)

Commits

rB838c4a97f1d6: Geometry Nodes: new Volume Cube node

Summary

This patch adds a Volume Cube primitive, with a field input to set the density at each grid voxel. This can be used to create procedural SDF-based meshes using the Volume To Mesh node:

Possible future improvements:

Output an anonymous grid instead of hardcoding the name "density". Also see T91668.
Support creating multiple grids at the same time (requires adding more sockets dynamically, we don't have that yet).
Possibly allow specifying whether the grid is an fog volume or SDF.
Support for grids with different data types (vector, int, ...).
Support a selection input which is possibly evaluated at a lower resolution, so that the main grid values don't have to be evaluated at all positions.
Support different resolution modes like in the Volume to Mesh node.

Diff Detail

Repository: rB Blender

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Amos Manneschmidt (amoose136) awarded a token.Jun 15 2022, 12:31 PM

Trung Phạm (zNight) awarded a token.Jun 15 2022, 1:18 PM

Nice patch, thanks for working on that.
Some notes:

Please use clang-format and our comment style (/* Full sentence. */).
Use a new FieldContext instead of using a PointCloud. This is actually the first time we use a field context that is not based on a geometry, but that's fine or even good. I implemented it here for reference: P3009.
Generating the positions and copying it over to the volume should be parallelized. In the future this could potentially be improved by writing to the volume grid directly and by generating the positions lazily in batches.
This is also a bit related to T91668. So the node might need some versioning once that is implemented, which is fine.

source/blender/nodes/geometry/nodes/node_geo_field_to_volume.cc
56 ↗	(On Diff #52530)	Not that I know of.
59 ↗	(On Diff #52530)	Make sure to protect against division by zero (outside of this function).

This revision now requires changes to proceed.Jun 15 2022, 1:21 PM

Jacques Lucke (JacquesLucke) added inline comments.Jun 15 2022, 1:22 PM

source/blender/nodes/geometry/nodes/node_geo_field_to_volume.cc
1 ↗	(On Diff #52530)	Use the new license header: `/* SPDX-License-Identifier: GPL-2.0-or-later */`

Hans Goudey (HooglyBoogly) added a project: Geometry Nodes.Jun 15 2022, 1:59 PM

Looks to me like it needs more protection against WITH_OPENVDB not being true?

Jacob Merrill (blueprintrandom) awarded a token.Jun 15 2022, 4:31 PM

baoyu (baoyu) awarded a token.Jun 15 2022, 4:33 PM

Super Secret Goblin Prince (MasterNurmi) awarded a token.Jun 15 2022, 4:41 PM

dupoxy (dupoxy) awarded a token.Jun 15 2022, 4:42 PM

Andres Stephens (Draise) awarded a token.Jun 15 2022, 5:26 PM

Giff (GifCo) awarded a token.Jun 15 2022, 5:28 PM

Added #ifdef WITH_OPENVDB checks
Use Grid3DFieldContext and parallelise it
Bounding volume > 0 check (no divide by zero)
Rename to "Volume Grid" as per meeting notes
Remove min clamp of Volume To Mesh threshold to allow interior SDF distances to work
clang-format

Copying the field evaluation results to the OpenVDB grid isn't parallelised, the grid->getAccessor() isn't thread safe and I don't know enough about OpenVDB to know an alternative.

I suspect this is a little overkill. Your position will be virtually used, which doesn't make me feel like allocate is necessary.

*edit:

VArray<float3>::ForFunc([resolution_, bounds_min_, bounds_max_](const int index){...});

source/blender/nodes/geometry/nodes/node_geo_volume_grid.cc
58–74 ↗	(On Diff #52548)	Isn't it possible to use VArrya<int>::ForFunc and not do array allocation by position?

Iliya Katueshenock (Moder) added a comment.Jun 15 2022, 7:20 PM

This comment was removed by Iliya Katueshenock (Moder).

source/blender/nodes/geometry/nodes/node_geo_volume_grid.cc
91 ↗	(On Diff #52548)	It seems to me that the protection should change the logic of work

Francis Jasmin (Francis_Jasmin) awarded a token.Jun 15 2022, 11:37 PM

Francis Jasmin (Francis_Jasmin) added a subscriber: Francis Jasmin (Francis_Jasmin).

D0ppelgaenger (D0ppelgaenger) awarded a token.Jun 15 2022, 11:41 PM

D0ppelgaenger (D0ppelgaenger) added a subscriber: D0ppelgaenger (D0ppelgaenger).

Antoine Bagattini (Baga) awarded a token.Jun 16 2022, 10:21 AM

Jacques Lucke (JacquesLucke) mentioned this in D15160: Textures: prototype multi-function texture nodes CPU evaluation.Jun 16 2022, 10:52 AM

Angel Bueno (angelbpineda) awarded a token.Jun 16 2022, 11:11 AM

Copying the field evaluation results to the OpenVDB grid isn't parallelised, the grid->getAccessor() isn't thread safe and I don't know enough about OpenVDB to know an alternative.

I implemented that here: P3011.

source/blender/nodes/geometry/nodes/node_geo_volume_grid.cc
39 ↗	(On Diff #52548)	This comment is unnecessary.
58–74 ↗	(On Diff #52548)	Would in theory be possible, but might make performance worse right now. Think this explicit array construction is fine for now (also given that we know that all values will be needed). I'll look into generating the points in batches (and never allocating the big array) separately.
77 ↗	(On Diff #52548)	Move this to the top of the file (as in other nodes).
79 ↗	(On Diff #52548)	Shouldn't be an implicit field.
96 ↗	(On Diff #52548)	could use `int3` to store `resolution`.
128 ↗	(On Diff #52548)	It's ok when you just assume `eval_idx` to be `0` here, as we do in other places.
143 ↗	(On Diff #52548)	`const`

This revision now requires changes to proceed.Jun 16 2022, 11:58 AM

Jacques Lucke (JacquesLucke) retitled this revision from Geometry Nodes: Field to Mesh node to Geometry Nodes: Volume Grid node.Jun 16 2022, 12:02 PM

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)Jun 16 2022, 12:04 PM

Jacques Lucke (JacquesLucke) mentioned this in T93044: Single Point Node.Jun 16 2022, 12:07 PM

Arjhi Moscosa (Omeleto) added a subscriber: Arjhi Moscosa (Omeleto).Jun 16 2022, 12:50 PM

Zach Eastin (Bariboy) awarded a token.Jun 16 2022, 1:29 PM

use openvdb::tools::Dense to copy results
address other feedback

Hans Goudey (HooglyBoogly) added inline comments.Jun 16 2022, 3:06 PM

source/blender/nodes/geometry/nodes/node_geo_volume_grid.cc
120–122 ↗	(On Diff #52578)	A small improvement would be to give `Grid3DFieldContext` a `points_num()` method, which could be reused above where the positions array is built.

Add points_num() to Grid3DFieldContext

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)Jun 16 2022, 3:47 PM

Looks good to me now.

Started the buildbot here: https://builder.blender.org/admin/#/builders/18/builds/479

This revision is now accepted and ready to land.Jun 16 2022, 3:53 PM

Hans Goudey (HooglyBoogly) accepted this revision.Jun 16 2022, 3:57 PM

Hans Goudey (HooglyBoogly) added inline comments.

source/blender/nodes/geometry/nodes/node_geo_volume_grid.cc
40 ↗	(On Diff #52579)	Theoretically this should be a function in `BLI_math_base.hh`, or it could have a more specialized name. But just a thought, not a strong opinion.

We just talked in the geometry nodes chat, and Dalai talked to Brecht in person. The conclusion was that "Volume Cube" might make a bit more sense as a name.
"Cube" describes the shape of the evaluation domain, and it also describes the visual default output of the node, and the shape of the active voxels in the volume.

This revision now requires changes to proceed.Jun 16 2022, 4:25 PM

Rename to Volume Cube

Missed a function when refactoring ("volume_grid" gets used too often elsewhere to just find/replace automatically lol)

Hans Goudey (HooglyBoogly) accepted this revision.Jun 16 2022, 4:45 PM

This revision is now accepted and ready to land.Jun 16 2022, 4:45 PM

Erik Abrahamsson (erik85) added inline comments.Jun 16 2022, 4:46 PM

source/blender/nodes/NOD_geometry.h
56–57	Seems a bit out of place here? and missing `geo`?

(third time's the charm - volume_cube -> geo_volume_cube)

Deen (Bit) awarded a token.Jun 16 2022, 5:09 PM

Jacques Lucke (JacquesLucke) added inline comments.Jun 16 2022, 5:09 PM

source/blender/nodes/geometry/nodes/node_geo_volume_cube.cc
43	missing newline
74	use `this->` when calling methods

Jacques Lucke (JacquesLucke) added inline comments.Jun 16 2022, 5:11 PM

source/blender/nodes/geometry/nodes/node_geo_volume_cube.cc
77	Missing `.`

Address comments in chat

Fix "Bounds Min/Max" -> "Min/Max" in params access

João Generoso (juaogeneroso) awarded a token.Jun 16 2022, 5:36 PM

ordering in CMakeLists.txt, NOD_static_types.h

A couple fixes after some users testing the patch:

Fix that connecting to a VolumeToMesh node meshes the *outside* of the volume (as it was being treated as an SDF instead of like regular volumes. Now for SDF behaviour, i.e. negative values inside, users should multiply the density by -1). Fixed by setting the Background Value of the grid to be negative instead of positive. Fixes output face normals being flipped also.
Fix off-by-one error in grid scaling leading to changes in scale with resolution: