Geometry Nodes: Sample Volume node #107656
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
4 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#107656
Loading…
Reference in New Issue
No description provided.
Delete Branch "erik85:volume-sample"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This PR adds an experimental
Sample Volume
node that can samplethe value of a volume at a specified position or with a field.
It supports Float, Int32, Boolean and Vector grids.
The
Grid
input expects a named attribute input with the name of the grid.WIP: Geometry Nodes: Sample Volume nodeto Geometry Nodes: Sample Volume nodeI think this will be useful! I'd expect this to be implemented as a multifunction similar to the other "sample" nodes though. An interpolation type dropdown might be nice to have as well, since I think OpenVDB makes a few available.
Potentially quite a few things here to consider I think. Pretty sure there are more than these I thought of:
typename
in front ofGridT::ConstAccessor
everywhere to get this to compile here@ -11004,0 +11013,4 @@
PropertyRNA *prop;
RNA_def_struct_sdna_from(srna, "NodeGeometrySampleVolume", "storage");
The fact that the node is implemented with OpenVDB is an implementation detail that we don't need to expose to users. Something like "How to interpolate the values from neighboring voxels" might be more helpful
@ -11004,0 +11021,4 @@
prop = RNA_def_property(srna, "grid_type", PROP_ENUM, PROP_NONE);
RNA_def_property_enum_items(prop, rna_enum_attribute_type_items);
RNA_def_property_enum_funcs(
Technically this doesn't change the data type stored in the grid, right? It's the type to use for sampling?
@ -0,0 +101,4 @@
{
StringRefNull grid_name;
if (field.node().field_inputs() != nullptr) {
Does something like this work?
Seems like it would be a bit more predictable
I think this is simpler than looping through all the field inputs. Still worth doing IMO
@ -0,0 +190,4 @@
{
const VArray<float3> &positions = params.readonly_single_input<float3>(0, "Position");
GMutableSpan dst = params.uninitialized_single_output(1, "Value");
const VolumeGridType grid_type = BKE_volume_grid_type_openvdb(*base_grid_);
I think it would be fine to use
VArraySpan
here, since the input positions would probably almost always be a span anyway. That way the rest of the function doesn't need to useVArray
@ -0,0 +195,4 @@
switch (grid_type) {
case VOLUME_GRID_FLOAT: {
sample_grid<openvdb::FloatGrid>(base_grid_, positions, mask, dst, sampling_mode_);
break;
Nit-picky thing, but the braces seem to just take up unnecessary space here.
@ -0,0 +320,4 @@
if (volume_grid == nullptr) {
char *message = BLI_sprintfN(TIP_("Grid not found in the volume: \"%s\""), grid_name.c_str());
params.error_message_add(NodeWarningType::Error, message);
params.set_default_remaining_outputs();
Memory leak! But also typically we don't give errors for this. When an attribute doesn't exist, I think we usually just give default values.
@ -0,0 +329,4 @@
/* Check that the input grid socket type matches the grid type. */
if (!((grid_type == VOLUME_GRID_FLOAT && input_grid_type == CD_PROP_FLOAT) ||
(grid_type == VOLUME_GRID_INT && input_grid_type == CD_PROP_INT32) ||
(grid_type == VOLUME_GRID_BOOLEAN && input_grid_type == CD_PROP_BOOL) ||
Maybe a simpler and more reusable way to do this would be to have a function that takes a custom data type and returns a grid type (or vice versa).
@ -0,0 +346,4 @@
NodeWarningType::Warning,
TIP_("Using the Point Sampler because of Boolean Grid type limitation"));
sampling_mode = GEO_NODE_SAMPLE_VOLUME_SAMPLING_MODE_POINT;
}
I think it's okay to just do this silently. It's not like users will really expect booleans to be interpolated.
@ -0,0 +351,4 @@
GField output_field;
if (position_value_or_field.is_field()) {
auto fn = std::make_shared<SampleVolumeFunction>(
std::move(base_grid), std::move(grid_field), sampling_mode);
The geometry nodes evaluator will do this constant field optimization by itself. Typically the multi-functions shouldn't have to know about it (they'll just get a single-element span/mask).
@ -0,0 +270,4 @@
}
BLI_assert_unreachable();
return std::nullopt;
}
It seems better to just agree that with above:
Default - unreachable
Return at the end - nothing
@ -0,0 +283,4 @@
}
const NodeGeometrySampleVolume &storage = node_storage(params.node());
const eCustomDataType input_grid_type = eCustomDataType(storage.grid_type);
auto interpolation_mode = (const GeometryNodeSampleVolumeInterpolationMode)
C-style cast there
edit: Forget it, I'm babbling.
I mean
(const GeometryNodeSampleVolumeInterpolationMode)(storage.interpolation_mode)
Really, not sure if it better.
@ -0,0 +329,4 @@
GField output_field = GField(std::move(op));
output_attribute_field(params, std::move(output_field));
params.set_default_remaining_outputs();
params.set_default_remaining_outputs();
is necessary there?@ -0,0 +4,4 @@
#include "BKE_volume.h"
#include "BLI_virtual_array.hh"
#include "NOD_add_node_search.hh"
Add newlines between includes from different modules
@ -0,0 +123,4 @@
{
using ValueT = typename GridT::ValueType;
using AccessorT = typename GridT::ConstAccessor;
const GridT::ConstPtr grid = openvdb::gridConstPtrCast<GridT>(base_grid);
I also had to add
typename
beforeGridT::ConstPtr
to get this to compile@ -0,0 +231,4 @@
switch (data_type) {
case CD_PROP_FLOAT:
return params.extract_input<Field<float>>("Grid_Float");
case CD_PROP_FLOAT3:
Braces can be removed for these switch cases
@ -0,0 +310,4 @@
const VolumeComponent *component = geometry_set.get_component_for_read<VolumeComponent>();
const Volume *volume = component->get_for_read();
BKE_volume_load(volume, DEG_get_bmain(params.depsgraph()));
const VolumeGrid *volume_grid = BKE_volume_grid_find_for_read(volume, grid_name.c_str());
Sampling the grid with its type (and only giving an error if the type is unsupported), then adding a conversion afterwards would probably be simpler.
Simpler in what way? And how do I add a conversion?
Simpler for the user, and simpler because you only need to check the grid type here. You can add a field conversion with
get_implicit_type_conversions
andfn::GField try_convert(fn::GField field, const CPPType &to_type) const;
@ -0,0 +64,4 @@
static void search_link_ops(GatherLinkSearchOpParams ¶ms)
{
if (U.experimental.use_new_volume_nodes) {
blender::nodes::search_link_ops_for_basic_node(params);
Since this node has faux multi-type sockets, it probably needs a
search_link_ops
implementation like the other sample nodes.Just a few small things! Looks good otherwise.
@ -0,0 +114,4 @@
bNodeSocket *socket_value_boolean = socket_value_float->next;
bNodeSocket *socket_value_int32 = socket_value_boolean->next;
nodeSetSocketAvailability(ntree, socket_value_vector, grid_type == CD_PROP_FLOAT3);
These need a
bke::
prefix now@ -0,0 +148,4 @@
return "";
}
static std::optional<eCustomDataType> vdb_grid_type_to_customdata_type(
I guess this might as well be
vdb_grid_type_to_cpp_type
, that's how it's used anyway.@ -0,0 +179,4 @@
const GridT::ConstPtr grid = openvdb::gridConstPtrCast<GridT>(base_grid);
AccessorT accessor = grid->getConstAccessor();
auto copy_data = [&](auto sampler) {
How about
copy_data
->sample_data
? It's not really a "copy" IMO@ -0,0 +356,4 @@
auto op = FieldOperation::Create(std::move(fn), {position_field});
GField output_field = GField(std::move(op));
if (grid_customdata_type.value() != output_field_type) {
try_convert
already does this check@ -0,0 +134,4 @@
static const StringRefNull get_grid_name(GField &field)
{
if (field.node().field_inputs() != nullptr) {
You marked this resolved twice, maybe I wasn't clear. I think you can replace this whole
get_grid_name
function with:Haha ok, I made some change the first time but missed the point and second comment.
I'm trying to think of a way it could fail, if there are other FieldInputs connected in some way. But I guess that wouldn't make any sense.
FieldInput
is just base class ofAttributeFieldInput
, other nodes is class heirs too. So,dynamic_cast
simple solve this.I was mainly thinking that it could check the wrong node and miss the AttributeFieldInput, if it's not the only node connected to the input for some reason.
c5434396b7
to70f312a954
Pull request closed