blender
/
blender
134
424
Fork 606
Code Issues 6.3k Pull Requests 773 Projects 19 Wiki Activity

Geometry Nodes: Sample Volume node #107656

Closed
Erik Abrahamsson wants to merge 17 commits from erik85:volume-sample into main
pull from: erik85:volume-sample
merge into: blender:main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 1 Commits 17 Files Changed 9 +5 -7
1 changed files with 5 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit da6ac38680 - Show all commits

12
source/blender/nodes/geometry/nodes/node_geo_sample_volume.cc
View File

@ -252,7 +252,7 @@ static void output_attribute_field(GeoNodeExecParams &params, GField field)
}
}
static std::optional<eCustomDataType> openvdb_grid_type_to_customdata_type(
static std::optional<eCustomDataType> vdb_grid_type_to_customdata_type(
const VolumeGridType grid_type)
{
switch (grid_type) {
@ -268,7 +268,6 @@ static std::optional<eCustomDataType> openvdb_grid_type_to_customdata_type(
default:
break;
}
BLI_assert_unreachable();
return std::nullopt;
}
#endif /* WITH_OPENVDB */
erik85 marked this conversation as resolved Outdated
Iliya Katushenock commented 2023-05-09 20:22:34 +02:00
Outdated
Review
Copy Link

It seems better to just agree that with above:
Default - unreachable
Return at the end - nothing

    case VOLUME_GRID_MASK:
      return CD_PROP_BOOL;
    default:
      BLI_assert_unreachable();
      break;
  }
  return std::nullopt;
}
It seems better to just agree that with above: Default - unreachable Return at the end - nothing ```Cpp case VOLUME_GRID_MASK: return CD_PROP_BOOL; default: BLI_assert_unreachable(); break; } return std::nullopt; } ```
@ -283,8 +282,7 @@ static void node_geo_exec(GeoNodeExecParams params)
}
const NodeGeometrySampleVolume &storage = node_storage(params.node());
const eCustomDataType input_grid_type = eCustomDataType(storage.grid_type);
auto interpolation_mode = (const GeometryNodeSampleVolumeInterpolationMode)
storage.interpolation_mode;
auto interpolation_mode = GeometryNodeSampleVolumeInterpolationMode(storage.interpolation_mode);
mod_moder marked this conversation as resolved Outdated
Iliya Katushenock commented 2023-05-09 20:22:35 +02:00
Outdated
Review
Copy Link

C-style cast there

C-style cast there
Erik Abrahamsson commented 2023-05-10 17:17:03 +02:00
Outdated
Review
Copy Link

edit: Forget it, I'm babbling.

edit: Forget it, I'm babbling.
Iliya Katushenock commented 2023-05-10 17:18:43 +02:00
Outdated
Review
Copy Link

I mean (const GeometryNodeSampleVolumeInterpolationMode)(storage.interpolation_mode)
Really, not sure if it better.

I mean `(const GeometryNodeSampleVolumeInterpolationMode)(storage.interpolation_mode)` Really, not sure if it better.
GField grid_field = get_input_attribute_field(params, input_grid_type);
const StringRefNull grid_name = get_grid_name(grid_field);
@ -306,8 +304,9 @@ static void node_geo_exec(GeoNodeExecParams params)
const VolumeGridType grid_type = BKE_volume_grid_type_openvdb(*base_grid);
/* Check that the input grid socket type matches the grid type. */
if (openvdb_grid_type_to_customdata_type(grid_type).value() != input_grid_type) {
std::optional<eCustomDataType> grid_customdata_type = vdb_grid_type_to_customdata_type(
grid_type);
if (!grid_customdata_type.has_value() || grid_customdata_type.value() != input_grid_type) {
params.set_default_remaining_outputs();
params.error_message_add(
NodeWarningType::Error,
@ -329,7 +328,6 @@ static void node_geo_exec(GeoNodeExecParams params)
GField output_field = GField(std::move(op));
output_attribute_field(params, std::move(output_field));
params.set_default_remaining_outputs();
#else
params.set_default_remaining_outputs();
erik85 marked this conversation as resolved Outdated
Hans Goudey commented 2023-05-09 14:24:51 +02:00
Outdated
Review
Copy Link

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).

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).
Iliya Katushenock commented 2023-05-09 20:22:38 +02:00
Outdated
Review
Copy Link

params.set_default_remaining_outputs(); is necessary there?

`params.set_default_remaining_outputs();` is necessary there?
params.error_message_add(NodeWarningType::Error,