Half Grid Support PR
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.hthat brings in either anOpenEXR halftype or theinternal halftype that we have inopenvdb. It's good to note at this point that this type is included inmath.handVec3.h. - We have a few type-traits functions and class to handle "specialization" with Half.
- We have an
ExtendedRealGridTypesinTypes.h. This is used to buildNumericGridTypes, which is used to buildGridTypes. The functionopenvdb::initializemakes sure thatHalfGridis registered (by callingGridTypes::foreach<RegisterGrid>()). - These tools work with
HalfGrid:
What type_traits functions/idioms do you add?
- Adds
openvdb::is_floating_pointwhich inherits fromstd::is_floating_point. Itd allows specialization forHalftype. - Adds
openvdb::is_signedwhich inherits fromstd::is_signed. It allows specialization forHalftype. - Adds a trait class called
ComputeTypeForto convertHalf -> 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)
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.
@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:
- TestLevelSetRayIntersector.cc
- TestMeshToVolume.cc
- TestTools.cc
- TestTree.cc
I'm passing this PR over to you.
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?
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.