Page MenuHome

OpenVDB mesh/level set conversion, filters and CSG operations
ClosedPublic

Authored by Pablo Dobarro (pablodp606) on Jul 29 2019, 3:11 PM.

Details

Summary

This is the OpenVDB code that is being used in the sculpt branch by the voxel remesher and the remesh modifier. To start porting the voxel remesher, the mesh/level set conversion functionality needs to be in master first.

An alternative version that can also work for remeshing is in D4980

Done by @Martin Felke (scorpion81) and @Pablo Dobarro (pablodp606)

Diff Detail

Repository
rB Blender
Branch
sculpt-openvdb (branched from master)
Build Status
Buildable 4236
Build 4236: arc lint + arc unit

Event Timeline

Brecht Van Lommel (brecht) requested changes to this revision.Jul 31 2019, 12:53 AM

Mostly nitpicky comments.

If openvdb_capi.h keeps growing we should split it up, but seems manageable for now.

intern/openvdb/intern/openvdb_level_set.cc
42

If the size of a vector is known in advance, it should used since it's more efficient than push_back which will cause multiple allocations and copies.

std::vector<openvdb::Vec3s> points(totvertices);
for (unsigned int i = 0; i < totvertices; i++) {
  points[i] = openvdb::Vec3s(vertices[i * 3], vertices[i * 3 + 1], vertices[i * 3 + 2])
}
89

unsigned int -> size_t, it's better to use the same type as .size().

119

this->grid && is redundant.

141–146

Why is this passing iterations / 100.0? If it's not actually a number of iterations that we are specifying, this seems misleading.

154–155

Either remove or explain commented out code.

In this case there is no copy, just a reference, so the name is misleading.

intern/openvdb/intern/openvdb_level_set.h
40

The member function names in this class don't need an OpenVDB_ or OpenVDB_level_set_ prefix, it's redundant.

40

The grid pointer here should be passed by reference:

void OpenVDB_level_set_set_grid(const openvdb::FloatGrid::Ptr& grid)

Not a performance issue in this case, but copying shared pointers like these is not a cheap operation in general, so best to have the habit of passing them around more efficiently.

This goes for all the OpenVDB ::Ptr types.

41

This function is not implemented?

53

This function is not implemented?

69

int -> OpenVDBLevelSet_FilterBias

intern/openvdb/openvdb_capi.cc
24–26

I see no reason for this to exist at all, seems fine to remove. But then OpenVDBIntGrid and OpenVDBVectorGrid can be removed as well?

284

struct is not needed here.

364

Commented out code should have an explaining comment, or get removed.

intern/openvdb/openvdb_capi.h
68

Add empty line above this one.

90–91

For type safetype and clarity you could instead use this:

enum OpenVDBLevelSet_FilterType filter_type;
enum OpenVDBLevelSet_FilterBias filter_bias;
92

Comment style: /* Parameter for gaussian, median, mean. */

This revision now requires changes to proceed.Jul 31 2019, 12:53 AM
Pablo Dobarro (pablodp606) updated this revision to Diff 16734.
  • Update for revision

I also updated the sculpt branch to be compatible with these changes.

Brecht Van Lommel (brecht) requested changes to this revision.Aug 2 2019, 4:21 AM
Brecht Van Lommel (brecht) added inline comments.
intern/openvdb/intern/openvdb_level_set.cc
39

Add const

147–148

Add const for both

intern/openvdb/intern/openvdb_level_set.h
38–39

This should be:

const openvdb::FloatGrid::Ptr &get_grid();
void set_grid(const openvdb::FloatGrid::Ptr& grid);

The const is important, we don't want callers to be able to modifier the grid pointer inside OpenVDBLevelSet, we just want to avoid copying it when it's not needed.

45

Add const

55–56

Add const for both

intern/openvdb/intern/openvdb_transform.h
33

Add const

34

Use const openvdb::math::Transform::Ptr&

This revision now requires changes to proceed.Aug 2 2019, 4:21 AM
This revision is now accepted and ready to land.Aug 4 2019, 1:21 PM

Hi @Pablo Dobarro (pablodp606) and @Brecht Van Lommel (brecht) ,

I've made a comparison between the OpenVDB Remesh add-on and the Sculpt Mode Features Remesh modifier's voxel mode. I've also discussed this with @Martin Felke (scorpion81) . The OpenVDB Remesh add-on has a different smoothing / rounding / voxel meshing method resulting in significantly less Moiré / interference patterns on the mesh surface, while maintaining sufficient detail.

I really hope this can be looked at before merging into master. Thanks.

I've attached a visual comparison.