Mesh To Volume fixes / refactoring #106346

This patch fixes a few problems with the current implementation of Mesh to Volume.

Main Problem

Currently Mesh to Volume creates a volume using openvdb::tools::meshToVolume which is then filled with the specified density and the class set to Fog Volume.
This is wrong because meshToVolume creates a signed distance field by default that then needs to be converted to a Fog Volume with openvdb::tools::sdfToFogVolume to get a proper Fog volume. Here is the description of what that function does:

The active and negative-valued interior half of the narrow band becomes a linear ramp from 0 to 1; the inactive interior becomes active with a constant value of 1; and the exterior, including the background and the active exterior half of the narrow band, becomes inactive with a constant value of 0. The interior, though active, remains sparse.

Here's an image of how the current behavior is, visualized with a Volume to Mesh node to the left and the volume to the right.

Here is that same file opened with this patch.
The most noticable difference is the gradient looks better and is adjustable using the interior band width.
Scrubbing the Volume to Mesh Threshold between 0 and 1 shows the size of the gradient.

This means with this change old files will no longer look the same. I don't see any way around that as the options for external band width and not filling the volume is removed (they don't make any sense for a fog volume).

Another improvement is that now interior band width can be specified using the scene unit or voxels.

An important point here is that in the future it will make more sense to use the (now experimental) node Mesh to SDF Volume if the volume will be converted back to a mesh. This skips the internal conversion from an SDF to a Fog-volume grid and gives a better result.

I can see Mesh to Volume eventually being renamed to Mesh to Fog Volume to make it more clear what it's doing.

This patch fixes a few problems with the current implementation of `Mesh to Volume`. ### Main Problem Currently `Mesh to Volume` creates a volume using `openvdb::tools::meshToVolume` which is then filled with the specified density and the class set to Fog Volume. This is wrong because `meshToVolume` creates a signed distance field by default that then needs to be converted to a Fog Volume with `openvdb::tools::sdfToFogVolume` to get a proper Fog volume. Here is the description of what that function does: > The active and negative-valued interior half of the narrow band becomes a linear ramp from 0 to 1; the inactive interior becomes active with a constant value of 1; and the exterior, including the background and the active exterior half of the narrow band, becomes inactive with a constant value of 0. The interior, though active, remains sparse. Here's an image of how the current behavior is, visualized with a Volume to Mesh node to the left and the volume to the right. ![bild](/attachments/f8930a25-90e5-4ba2-9ec0-440f20ca2850) Here is that same file opened with this patch. The most noticable difference is the gradient looks better and is adjustable using the interior band width. Scrubbing the Volume to Mesh `Threshold` between 0 and 1 shows the size of the gradient. ![bild](/attachments/0e4967b4-533d-48fb-b58b-658150e6ebfa) This means with this change old files will no longer look the same. I don't see any way around that as the options for external band width and not filling the volume is removed (they don't make any sense for a fog volume). Another improvement is that now interior band width can be specified using the scene unit or voxels. An important point here is that in the future it will make more sense to use the (now experimental) node `Mesh to SDF Volume` if the volume will be converted back to a mesh. This skips the internal conversion from an SDF to a Fog-volume grid and gives a better result. I can see `Mesh to Volume` eventually being renamed to `Mesh to Fog Volume` to make it more clear what it's doing.

bild.png

68 KiB

bild.png

617 KiB

bild.png

501 KiB

It would help if you could show in which way existing files can break with this change.

It would help if you could show in which way existing files can break with this change.

Well now the fog volume is basically a signed distance field grid that is filled from the exterior width to the interior width with constant density.

With this change it would become a fog volume with 0 values on the outside of the mesh and a gradient from 0 to 1 on the inside (gradient width is the interior band width).

Before (exterior and interior width the same value):

After (same interior width as above):

> It would help if you could show in which way existing files can break with this change. Well now the fog volume is basically a signed distance field grid that is filled from the exterior width to the interior width with constant density. With this change it would become a fog volume with 0 values on the outside of the mesh and a gradient from 0 to 1 on the inside (gradient width is the interior band width). Before (exterior and interior width the same value): ![bild](/attachments/a3cc2eb2-8a08-4a51-8a07-5d117c85dd22) After (same interior width as above): ![bild](/attachments/b2510407-38d2-4684-8a8c-b11442d2348f)

bild.png

138 KiB

bild.png

189 KiB

Maybe one way to solve it would be to keep the current Mesh To Volume node and modifier, but deprecate it and add a new Mesh to Fog Volume-node and modifier.

Maybe one way to solve it would be to keep the current `Mesh To Volume` node and modifier, but deprecate it and add a new `Mesh to Fog Volume`-node and modifier.

I think this is generally a good improvement and probably does not need versioning. The main use case of converting meshes to volumes and back only seems to improve.

I'm not fully convinced the new behavior is correct yet. sdfToFogVolume takes the interior and exterior bandwidth into account. Effectively that means that currently the density is 0 at the sphere surface and then increases linearly to 1 in the sphere center. However, I'd expect most of the sphere to have the same density. The main issue here is that fill_volume is currently implemented by increasing the interior bandwidth, which might not be necessary anymore.

When fill_volume is off, I'd expect to see a sphere in a sphere after converting the volume back to a mesh. In main that works, in this patch not yet.

I think this is generally a good improvement and probably does not need versioning. The main use case of converting meshes to volumes and back only seems to improve. I'm not fully convinced the new behavior is correct yet. `sdfToFogVolume` takes the interior and exterior bandwidth into account. Effectively that means that currently the density is 0 at the sphere surface and then increases linearly to 1 in the sphere center. However, I'd expect most of the sphere to have the same density. The main issue here is that `fill_volume` is currently implemented by increasing the interior bandwidth, which might not be necessary anymore. When `fill_volume` is off, I'd expect to see a sphere in a sphere after converting the volume back to a mesh. In `main` that works, in this patch not yet.

I'm not fully convinced the new behavior is correct yet. sdfToFogVolume takes the interior and exterior bandwidth into account. Effectively that means that currently the density is 0 at the sphere surface and then increases linearly to 1 in the sphere center. However, I'd expect most of the sphere to have the same density.

Does it really take the exterior into account? If you read the comment from the OpenVDB code in the PR description I interpret that as all of the exterior will become inactive with value 0 so it shouldn't matter how wide it is.

You're right, Fill Interior is probably not needed anymore, and to get most of the sphere the same density you would just need to decrease the interior band width near 0. There needs to be a gradient though if it is to be converted back to Mesh to get the mesh smooth (but in the case of needing a mesh maybe using SDF is almost always better?).

When fill_volume is off, I'd expect to see a sphere in a sphere after converting the volume back to a mesh. In main that works, in this patch not yet.

I don't think that's going to work because the fog volume will never have the gradient back to 0 in the middle as far as I understand.

> I'm not fully convinced the new behavior is correct yet. `sdfToFogVolume` takes the interior and exterior bandwidth into account. Effectively that means that currently the density is 0 at the sphere surface and then increases linearly to 1 in the sphere center. However, I'd expect most of the sphere to have the same density. Does it really take the exterior into account? If you read the comment from the OpenVDB code in the PR description I interpret that as all of the exterior will become inactive with value 0 so it shouldn't matter how wide it is. You're right, `Fill Interior` is probably not needed anymore, and to get most of the sphere the same density you would just need to decrease the interior band width near 0. There needs to be a gradient though if it is to be converted back to Mesh to get the mesh smooth (but in the case of needing a mesh maybe using SDF is almost always better?). > > When `fill_volume` is off, I'd expect to see a sphere in a sphere after converting the volume back to a mesh. In `main` that works, in this patch not yet. I don't think that's going to work because the fog volume will never have the gradient back to 0 in the middle as far as I understand.

From the discussion I remember in chat, I think this change makes sense. But I think it would be easier to accept if the PR description was very clear about what the "breaking" changes are on a user level.

The description also describes the problem with the current code fairly well, but it has less detail about how it's changed to improve it.

From the discussion I remember in chat, I think this change makes sense. But I think it would be easier to accept if the PR description was very clear about what the "breaking" changes are on a user level. The description also describes the problem with the current code fairly well, but it has less detail about how it's changed to improve it.

This is not very critical, but try to use extract_input instead of get_input to avoid copying values.

This is not very critical, but try to use `extract_input` instead of `get_input` to avoid copying values.

This is not very critical, but try to use extract_input instead of get_input to avoid copying values.

I don't think it works well in this case where it could be read multiple times if there are multiple objects in the Geometry input.

> This is not very critical, but try to use `extract_input` instead of `get_input` to avoid copying values. I don't think it works well in this case where it could be read multiple times if there are multiple objects in the Geometry input.

Another improvement is that now interior band width can be specified using the scene unit or voxels.
I'm not sure how I feel about this being added in the same patch. Reading the diff should give a clear idea of what's changing in the most important functional part of the patch, but the changes to MeshToVolumeSettings and adding the band width units option are making things confusing.

Generally this combination of cleanup and functional changes, and the new units option is hard for me to follow. I get the sense that all of the changes are reasonable, but it's hard to verify that by reading the diff right now.

>Another improvement is that now interior band width can be specified using the scene unit or voxels. I'm not sure how I feel about this being added in the same patch. Reading the diff should give a clear idea of what's changing in the most important functional part of the patch, but the changes to `MeshToVolumeSettings` and adding the band width units option are making things confusing. Generally this combination of cleanup and functional changes, and the new units option is hard for me to follow. I get the sense that all of the changes are reasonable, but it's hard to verify that by reading the diff right now.

I'm also not totally convinced by combining sdf_volume_grid_add_from_mesh and fog_volume_grid_add_from_mesh into the same function and differentiating their behavior with a "settings" struct. That basically creates one function with very different behavior based on its arguments. It's usually better to have different functions instead, and share their internal logic by splitting parts of their internals into separate functions, while keeping the API separate.

Maybe I'm missing some of your reasoning though? Either way, it still seems unrelated to the change of updating the mesh to fog volumes input options, which seems like the main focus of this patch.

I'm also not totally convinced by combining `sdf_volume_grid_add_from_mesh` and `fog_volume_grid_add_from_mesh` into the same function and differentiating their behavior with a "settings" struct. That basically creates one function with very different behavior based on its arguments. It's usually better to have different functions instead, and share their internal logic by splitting parts of their internals into separate functions, while keeping the API separate. Maybe I'm missing some of your reasoning though? Either way, it still seems unrelated to the change of updating the mesh to fog volumes input options, which seems like the main focus of this patch.

The main focus of this patch was to use openvdb::tools::sdfToFogVolume to create the fog volume and while doing that I realized the functions would look pretty much identical except for the call to sdfToFogVolume in the end. That's why I created the settings struct. You could be right that there are other better ways to solve it but at that point it seemed like the best I could come up with. This is similar to the Maya and Houdini node implementations in OpenVDB's repo.

The main focus of this patch was to use `openvdb::tools::sdfToFogVolume` to create the fog volume and while doing that I realized the functions would look pretty much identical except for the call to sdfToFogVolume in the end. That's why I created the settings struct. You could be right that there are other better ways to solve it but at that point it seemed like the best I could come up with. This is similar to the Maya and Houdini node implementations in OpenVDB's repo.

The main focus of this patch was to use openvdb::tools::sdfToFogVolume to create the fog volume and while doing that I realized the functions would look pretty much identical except for the call to sdfToFogVolume in the end.

I do think the patch is doing too much then. A change should usually just do one thing. Here we're combining this change, a cleanup/de-duplication change, and another new feature.

> The main focus of this patch was to use `openvdb::tools::sdfToFogVolume` to create the fog volume and while doing that I realized the functions would look pretty much identical except for the call to sdfToFogVolume in the end. I do think the patch is doing too much then. A change should usually just do one thing. Here we're combining this change, a cleanup/de-duplication change, and another new feature.

 @ -98,1 +86,3 @@
   return voxel_size;
   if (settings.use_world_space_units) {
     return (diagonal + settings.exterior_band_width * 2.0f) / (float)settings.voxels;

 @ -10162,0 +10164,4 @@
       {MESH_TO_VOLUME_UNIT_LOCAL,
        "LOCAL",
 ,
        "Local space",

 @ -10185,12 +10200,27 @@ static void def_geo_mesh_to_sdf_volume(StructRNA *srna)
       {0, NULL, 0, NULL, NULL},
   };
   static EnumPropertyItem units_items[] = {

 @ -154,0 +147,4 @@
                                           /* exterior_band_width */ mvmd->exterior_band_width,
                                           /* density */ 0.0f,
                                           /* simplify */ volume_simplify,
                                           /* fill_volume */ (bool)(mvmd->fill_volume),

 @ -104,2 +113,2 @@
     resolution.settings.voxel_size = params.get_input<float>("Voxel Size");
     if (resolution.settings.voxel_size <= 0.0f) {
   else {
     int voxels = params.get_input<int>("Half-Band Voxels");

Mesh To Volume fixes / refactoring #106346

Main Problem

Reviewers

Pull request closed

 @ -29,0 +30,4 @@
   bool fill_volume;
   bool use_world_space_units;
   bool convert_to_fog;
   bool unsigned_distance;

 @ -123,3 +157,2 @@
   const float voxel_size = geometry::volume_compute_voxel_size(
       params.depsgraph(), bounds_fn, resolution, half_band_width, mesh_to_volume_space_transform);
   auto mode = (MeshToVolumeModifierResolutionMode)storage.resolution_mode;

 @ -47,3 +40,1 @@
   b.add_input<decl::Bool>(N_("Fill Volume"))
       .default_value(true)
       .description(N_("Initialize the density grid in every cell inside the enclosed volume"));
       .description(N_("Width of the gradient inside of the mesh"));

 @ -98,0 +96,4 @@
     return nullptr;
   }
   auto unit_mode = (MeshToVolumeModifierBandUnits)storage.band_units;

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Mesh To Volume fixes / refactoring #106346

Main Problem

Reviewers

Pull request closed