I found that the node has no output if the radius is smaller than the calculated voxel size (or the direct input voxel size in "size" mode). I think that's expected, but I suppose that's a case for a warning message.

Actually, even this case where the radius is larger than the voxel size has no output (on a 96m tall cube with subdiv level 6 applied):

Not for this patch, but I wonder if in combination with the volume to mesh node, this might eventually need a some sort of "max grid size" value so we don't melt people's computers. I'm surprised how well this performs actually.

My inline comments are mostly suggestions for tweaks to UI descriptions.

source/blender/makesrna/intern/rna_nodetree.c
8901	`Desired number of voxels along one axis` -> `Specify the numer of voxels along each axis`
8906	`Desired voxel side length` -> `Specify the voxel side length`
8915	`Mode for how the desired voxel size is specified` -> `How the voxel size is specified`
source/blender/nodes/geometry/nodes/node_geo_points_to_volume.cc
136	I also don't see a need for a default value here. Not sure in what case that would fail anyway.
175	0.0f
183–189	What's the idea behind creating a second grid and then merging it with the first? Maybe that deserves a comment.

Works great! I don't see any problems with it.

I mentioned this before but just want to reiterate it here:
I can see two things that would be nice to add down the line

A falloff parameter
Different modes to handle intersection of points. Might be nice to allow an additive mode as well for example

These things don't matter for this first iteration on the node, just wanted to have them written down here.

This revision is now accepted and ready to land.Jan 21 2021, 9:22 PM

cleanup
don't ignore particles based on their radius
improve ui

Harbormaster completed remote builds in B12322: Diff 33026.Jan 22 2021, 12:57 PM

Jacques Lucke (JacquesLucke) added inline comments.Jan 22 2021, 12:57 PM

source/blender/nodes/geometry/nodes/node_geo_points_to_volume.cc
136	The attribute getter methods without "try" require some kind of default in case there is an error. I could also assert, but I think I'd leave it as is.
183–189	I'll add a comment mentioning that the merge is cheap. The idea is that you don't have to worry about how `generate_volume_from_points` generates the grid. For example, if `openvdb::tools::meshToVolume` would be used, I couldn't use an existing grid, because it always outputs a new grid (see `MOD_mesh_to_volume.cc`). In this specific case I could use the existing grid, but that could change if the algorithm needs to be adapted. Furthermore, that makes the `generate_volume_from_points` more self contained, because it does not have to worry about data that might exist in the incoming grid already.

Looks good on a high level, will leave detailed review to the geometry nodes team.

Looks good to me!

The cropped text is not a unique problem, many of the new nodes have it, but I fount that node_type_size(&ntype, 170, 120, 700); gets the words to fit if you'd like to use that.

source/blender/nodes/geometry/nodes/node_geo_points_to_volume.cc
154	Hmm, maybe a more detailed comment about why subtracting .5 makes the alignment better?
213–227	We should probably settle on some standard for whether to put these functions at the top or bottom of the file. I think most of the more recent additions have been putting them at the top?]