4.6 KiB
Minutes from 70th OpenVDB TSC meeting, Nov 10th, 2020, (EDT)
Attendees: Nick A., Jeff L., Ken M., Dan B.
Additional Attendees: Johannes Meng (Intel), JT Nelson (Blender), Andre Pradhana (DW)
Regrets: Peter C.
Agenda:
-
Confirm Quorum
-
Secretary
-
AX - PR818
-
Matrix instantiation - PR723
-
Trivial types - PR865
-
Split out TypeList - PR866
-
LevelSetUtil UB Fix - PR868
-
Morphology - PR754
-
PR process
-
Unit tests
-
NanoVDB
-
Next Meeting
-
Confirm Quorum
Quorum is present.
- Secretary
Secretary is Dan Bailey.
- AX - PR818
Nick working on upstreaming floored mod changes. Requested API changes are ok to go into a subsequent release.
- Matrix instantiation - PR723
Nick has added component-wise min/max methods to matrix classes so that matrix grids can be instantiated. Matrix doesn't inherit from Tuple, would be nice to resolve this at a later date. Less-than and greater-than operators make sense in a tuple context, but not for Vec, Quat, Mat classes. Would be worth making this small breaking change to the API, perhaps adopting an Imath base class would be a good time to do this.
- Trivial types - PR865
Removed custom copy-constructors and deconstructors to make Vec, Mat and Quat classes trivial so that the NodeUnion specialization is no longer needed for these cases. Currently, they are all set to default, but discussed making them implicit and adding a comment instead. NodeUnion specialization now only used for std::string. Even though we may want to deprecate string grids, still worth keeping it around for any other custom specializations.
- Split out TypeList - PR866
Moves TypeList implemention into a new header, Nick using this quite a bit.
- LevelSetUtil UB Fix - PR868
Referencing the first element in a vector is undefined behavior if the vector is empty, so switch to using data() method. Jeff mentioned that the compiler can also agressively optimize away null pointer checks if the source was deemed to be a reference.
- Morphology - PR754
Nick to host a group code review session next week to discuss this PR in more detail.
- PR process
PRs often end up stagnating. TSC members either don't have time to review, do a cursory review or provide some suggestions for non-critical improvements and then they never get to the point of approval. Months later we return to them and then can't remember all the context.
All agreed that we should have a more formal process to merge PRs. Suggested process is to have a two-week window once a PR has first been submitted to allow the TSC time to review. Once that has passed, it can either progress to a group code review session if it's particularly large or complex or briefly presented by the author for approval one week later pending no objections.
Barrier for PR acceptance should be lower. Raising concerns or suggesting improvements shouldn't necessarily block progress of a PR. It may be fine to merge code with known but acceptable issues. Ken highlights that the master branch should be treated as in-development and making changes to newly introduced code is accepted and encouraged. Often the first usage of the code is only once it is in the master branch. Having unit tests is still required for merging.
- Unit tests
All in agreement about moving away from CppUnit, it's a simple unit test framework and is no longer being actively updated. Ken has a strong preference to use GoogleTest (or GTest), which is already in use in NanoVDB. It provides useful features such as being able to run tests using filters and until a failure occurs.
Committee is divided on how to migrate. Rewriting all the unit tests in GTest will take a lot of time and that time would be better spent on other priorities. Jeff favors having a dependency on both CppUnit and GTest and only adding new tests using GTest. Nick and Dan would prefer to get rid of CppUnit as a dependency. Dan would like to attempt a shim layer that may reduce the time spent migrating. Perhaps acceptable to have both dependencies if only one dependency per file. Migration could be done over a few months. Breaking up existing tests such as TestTools an orthogonal, but worthwhile effort.
- NanoVDB
DDA point offset issue was resolved by offsetting ray by half a voxel for point raytracing. Encapsulated in a PointRayMarch structure that hides this detail. Would be worth backporting this fix to vdb_render amongst others.
Ken feels NanoVDB has stabilized now and should be ready to merge soon.
- Next Meeting
Next meeting is November 17th, 2020. 12pm-1pm EST (GMT-5). Nick to present Morphology PR. Ken to investigate enabling Zoom screen-sharing.