Skip to content

Minimum Viable Product for Half Grid Support#2106

Merged
apradhana merged 32 commits intoAcademySoftwareFoundation:masterfrom
apradhana:feature/half_grid_support_mvp
Oct 30, 2025
Merged

Minimum Viable Product for Half Grid Support#2106
apradhana merged 32 commits intoAcademySoftwareFoundation:masterfrom
apradhana:feature/half_grid_support_mvp

Conversation

@apradhana
Copy link
Contributor

@apradhana apradhana commented Oct 13, 2025

This PR is trying to offer a Minimum Viable Product for Half Grid Support in OpenVDB 13.

Things covered in the PR are:

  • Native HalfGrid registration in the OpenVDB library.

@apradhana apradhana requested a review from kmuseth as a code owner October 13, 2025 17:21
@apradhana apradhana mentioned this pull request Oct 13, 2025
Open
danrbailey
danrbailey reviewed Oct 15, 2025
View reviewed changes
Copy link
Contributor

@danrbailey danrbailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good in general. However, if we do go this route and split this into two separate PRs, this first PR should disable anything for which we need a compute type algorithm, for example BoxSampler::sample(). There should be a static assert ensuring that it is not a half tree, otherwise this will (currently) do trilinear interpolation at half type which will ultimately change behavior when we subsequently add compute type.

apradhana
apradhana commented Oct 18, 2025
View reviewed changes
apradhana
apradhana commented Oct 18, 2025
View reviewed changes
danrbailey
danrbailey reviewed Oct 22, 2025
View reviewed changes
Copy link
Contributor

@danrbailey danrbailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a more thorough review of this yesterday. I think this all looks good, but my main concern remains that we are not explicitly disallowing half support for some algorithms where we haven't figured out compute type yet. I appreciate that the goal is to do like what you have done with level set sphere and to switch these algorithms from std::is_floating_point to openvdb::is_floating_point and that makes sense to me, but not all algorithms that are designed to work with floating-point grids have a static assert for std::is_floating_point unfortunately.

One such example is the LevelSetAdvection class, this currently works with any grid type and doesn't even validate that the grid must be a level set. As a result we are implicitly breaking backwards compatibility if we subsequently adapt this algorithm to use float as the compute type for HalfGrids in the future. I would be happy if we at least audit the tools and add explicit static asserts for half for any tool that doesn't already have a std::is_floating_point static assert.

Idclip
Idclip reviewed Oct 28, 2025
View reviewed changes
ghurstunither and others added 18 commits October 29, 2025 12:47
@ghurstunither @apradhana
Migrate over core changes from PR 1730
0e21a11 
Migrate over core changes from PR 1730

Signed-off-by: ghurstunither <62885595+ghurstunither@users.noreply.github.com>
@ghurstunither @apradhana
Prototyped half support. Removes ComputeType from tree & grid
264161a 
* Removes ComputeType from tree & grid.
* The same method still work with half grids.
  * Only now they infer the ComputeType through ComputeTypeFor<ValueType>::type.
* A lot of methods now let the user choose the compute type through an optional template argument.
  * Defaulting to ComputeTypeFor<ValueType>::type when user omits the parameter.

Note: The grid sampler's method sample (BoxSampler, etc) can now take an optional compute type template argument. I think this breaks backward compatibility on users with custom samplers because the GridSampler class will pass in the compute type to SamplerT::sample() now.

Signed-off-by: ghurstunither <62885595+ghurstunither@users.noreply.github.com>
@ghurstunither @apradhana
Restrict ComputeType to be computed, rather than optionally user supp…
252a4da 
…lied.

Restrict ComputeType to be computed, rather than optionally user supplied.

Signed-off-by: ghurstunither <62885595+ghurstunither@users.noreply.github.com>
@ghurstunither @apradhana
Whitespace
a9a8a63 
Whitespace

Signed-off-by: ghurstunither <62885595+ghurstunither@users.noreply.github.com>
@ghurstunither @apradhana
Update MeshToVolume.h
2c4a7d7 
Uses ComputeType during Renormalization to prevent catastrophic error with small voxel size.

Signed-off-by: ghurstunither <62885595+ghurstunither@users.noreply.github.com>
@ghurstunither @apradhana
Update FiniteDifference.h
51a3099 
ComputeType computations for finite differences.

Signed-off-by: ghurstunither <62885595+ghurstunither@users.noreply.github.com>
@ghurstunither @apradhana
More ComputeType
51bc24d 
Compute and return finite differences and other operators in terms of ComputeType.

Signed-off-by: ghurstunither <62885595+ghurstunither@users.noreply.github.com>
@ghurstunither @apradhana
Update FiniteDifference.h
0804dc8 
Removes incorrect template arguments.

Signed-off-by: ghurstunither <62885595+ghurstunither@users.noreply.github.com>
@ghurstunither @apradhana
Update Math.h
3faebe5 
Explicitly return halfs.

Signed-off-by: ghurstunither <62885595+ghurstunither@users.noreply.github.com>
@ghurstunither @apradhana
Update Math.h
a6f7fe0 
double to float.

Signed-off-by: ghurstunither <62885595+ghurstunither@users.noreply.github.com>
@ghurstunither @apradhana
Update Operators.h
67d7c85 
Fixes unused-local-typedef errors.

Signed-off-by: ghurstunither <62885595+ghurstunither@users.noreply.github.com>
@ghurstunither @apradhana
Update FiniteDifference.h
6209bfa 
Correct variables.

Signed-off-by: ghurstunither <62885595+ghurstunither@users.noreply.github.com>
@ghurstunither @apradhana
Update LevelSetTubesImpl.h
1e2d6ef 
Fixes more unused-local-typedef errors.

Signed-off-by: ghurstunither <62885595+ghurstunither@users.noreply.github.com>
@ghurstunither @apradhana
Update LevelSetMeasure.h
fcf976f 
Correct return types.

Signed-off-by: ghurstunither <62885595+ghurstunither@users.noreply.github.com>
@apradhana
Allow meshToVolume tool to work with Half Grid.
4f8cdca 
Signed-off-by: apradhana <andre.pradhana@gmail.com>
@apradhana
Add more test in TestMath.cc, Math.h, and Vec3h to make Half work with
71bdd00 
Vec3.

Signed-off-by: apradhana <andre.pradhana@gmail.com>
@apradhana
Enable volumeToMesh to work with Half.
c838a48 
Signed-off-by: apradhana <andre.pradhana@gmail.com>
@apradhana
Move HalfGrid to ExtendedRealGridTypes in openvdb.h.
193128f 
Signed-off-by: apradhana <andre.pradhana@gmail.com>
@apradhana
Add ExtendedRealTreeTypes in openvdb.h.
21e15fa 
Signed-off-by: apradhana <andre.pradhana@gmail.com>
@apradhana
Register Half Grid natively.
4f1c13e 
Signed-off-by: apradhana <andre.pradhana@gmail.com>
@apradhana
Revert the tools from Half Grid Support.
8c1e323 
Signed-off-by: apradhana <andre.pradhana@gmail.com>
@apradhana
Remove ComputeTypesFor from Types.h.
b062d4a 
Signed-off-by: apradhana <andre.pradhana@gmail.com>
@apradhana
Add testing pointer-cast with half grid type.
747fbc6 
Signed-off-by: apradhana <andre.pradhana@gmail.com>
@apradhana
Testing populatingHalfGridWithAccessor.
6d5bfe3 
Signed-off-by: apradhana <andre.pradhana@gmail.com>
@apradhana
Add test to create, write, and read HalfGrid.
d98bf5e 
Signed-off-by: apradhana <andre.pradhana@gmail.com>
@apradhana
Update license.
ee1d096 
Signed-off-by: apradhana <andre.pradhana@gmail.com>
@apradhana
Add HalfTree for OPENVDB_INSTANTIATION_ALL_TREES for explicit templat…
b1640ca 
…e instantiation.

Signed-off-by: apradhana <andre.pradhana@gmail.com>
@apradhana
Move HalfGrid from ExtendedRealGridTypes typelist to NumericGridTypes.
5a4f60e 
Signed-off-by: apradhana <andre.pradhana@gmail.com>
@apradhana
Remove template specialization of math::isNegative with half type.
8ec7681 
Signed-off-by: apradhana <andre.pradhana@gmail.com>
@apradhana
Make min_impl and max_impl to behave like std::min/std::max.
80e48d2 
Signed-off-by: apradhana <andre.pradhana@gmail.com>
@apradhana apradhana force-pushed the feature/half_grid_support_mvp branch from 501ffc8 to 80e48d2 Compare October 29, 2025 20:21
@apradhana
Merge branch 'master' into feature/half_grid_support_mvp
37f94b2 
Signed-off-by: apradhana <andre.pradhana@gmail.com>
@apradhana
Fix implicit conversion error.
9c58182 
Signed-off-by: apradhana <andre.pradhana@gmail.com>
danrbailey
danrbailey approved these changes Oct 30, 2025
View reviewed changes
Copy link
Contributor

@danrbailey danrbailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Andre, looks good! Squash before merge please. :)

@apradhana
Copy link
Contributor Author

apradhana commented Oct 30, 2025

Thank you for shepherding @ghurstunither and I to get this in, @danrbailey and @Idclip . Appreciate all the code reviews and comments.

@apradhana apradhana merged commit c69404e into AcademySoftwareFoundation:master Oct 30, 2025
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Idclip Idclip Idclip left review comments

@danrbailey danrbailey danrbailey approved these changes

@jmlait jmlait Awaiting requested review from jmlait

@richhones richhones Awaiting requested review from richhones

@ghurstunither ghurstunither Awaiting requested review from ghurstunither

@kmuseth kmuseth Awaiting requested review from kmuseth kmuseth is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@apradhana @Idclip @danrbailey @ghurstunither