openvdb icon indicating copy to clipboard operation
openvdb copied to clipboard

Half Grid Support PR

Open apradhana opened this issue 2 months ago • 4 comments

What are the most important things that I should take-away from this PR at the code-level? We want to make progress on making HalfGrid native in OpenVDB. This means:

  • We have HalfDecl.h that brings in either an OpenEXR half type or the internal half type that we have in openvdb. It's good to note at this point that this type is included in math.h and Vec3.h.
  • We have a few type-traits functions and class to handle "specialization" with Half.
  • We have an ExtendedRealGridTypes in Types.h. This is used to build NumericGridTypes, which is used to build GridTypes. The function openvdb::initialize makes sure that HalfGrid is registered (by calling GridTypes::foreach<RegisterGrid>()).
  • These tools work with HalfGrid:

What type_traits functions/idioms do you add?

  • Adds openvdb::is_floating_point which inherits from std::is_floating_point. Itd allows specialization for Half type.
  • Adds openvdb::is_signed which inherits from std::is_signed. It allows specialization for Half type.
  • Adds a trait class called ComputeTypeFor to convert Half -> float, Vec2H -> Vec2s, Vec3H -> Vec3s, Vec4H -> Vec4s`.

What medium tests did you do? - authoring sphere level set - authoring platonic solid level set - change level set background - level set measure - mesh to volume - volume to mesh - rendering half grid using vdb_render (to test interpolation)

apradhana avatar Oct 04 '25 20:10 apradhana

Thanks for the comments, @Idclip . I took these comments to heart and try to address them.

I discussed these points in a meeting with @ghurstunither on Friday. We decided to do two things:

  • Update this PR with the unit-tests that I added and pass this PR over to @ghurstunither in order to make the unit-tests to pass.
  • Get a Minimum Viable Product (MVP) for Half Grid PR, where we don't include ComputeTypeFor: https://github.com/AcademySoftwareFoundation/openvdb/pull/2106.

Hopefully between these two PRs options, we can get a version of HalfGrid in for OpenVDB 13.

apradhana avatar Oct 13 '25 17:10 apradhana

@ghurstunither : As promised, I updated this PR with all the unit-tests that I added. There are 4 unit-tests that are not passing. They are:

  1. TestLevelSetRayIntersector.cc
  2. TestMeshToVolume.cc
  3. TestTools.cc
  4. TestTree.cc

I'm passing this PR over to you.

apradhana avatar Oct 13 '25 17:10 apradhana

Thank you @apradhana / @ghurstunither - I would also try to get others to weigh in so it's not just my thoughts. @danrbailey @kmuseth @jmlait, any thoughts on the above?

Idclip avatar Oct 13 '25 19:10 Idclip

Thank you @apradhana / @ghurstunither - I would also try to get others to weigh in so it's not just my thoughts. @danrbailey @kmuseth @jmlait, any thoughts on the above?

I agree with what you said, it is somewhat arbitrary at present. On the one hand, it's probably only half that will have mixed value/compute types for the foreseeable future, so I don't want to unnecessarily hold back this feature getting released for what is limited fallout right now. That being said, there are two aspects that concern me - (1) any attempts to refactor or change the implementation in the future may subtly adjust behavior for the half type (ie round up/down in a slightly different way) which could make it harder for people to rely on using this type in practice, (2) this could also now create a demand for adding compute type logic for any new algorithm we introduce which would slow down progress substantially.

On balance, if we can come up with a slightly more consistent set of rules as to how to apply compute type, I would be happy to see this go out.

danrbailey avatar Oct 15 '25 05:10 danrbailey