Refactor I/O#2179
Open
danrbailey wants to merge 13 commits intoAcademySoftwareFoundation:feature/iofrom
Open
Conversation
…f I/O methods Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is a no-op change - no behavior changes and no breakages to the public I/O API. All unit tests pass and this portion of the codebase is reduced by ~250 lines.
The primary goals for this PR are:
Archive::readGrid()and all the grid writing behindArchive::writeGrid()Archive::readGrid()The changes are broken out into commits so as to make this easier to review:
This is the only public API change -
GridDescriptor::read()creates a Grid from a GridDescriptor, however the Grid return type was never used in the OpenVDB codebase, so this function has been deprecated and all the places where this was previously used now callsGridDescriptor.readHeader()followed byGridDescriptor::readStreamPos()instead which does not create the Grid and mirrors the write implementation. Finally,File::readGridDescriptors()was only ever used in one place in the codebase, so this logic has been hoisted into the calling code instead.db9b6cf
787ff90
File::readGridByName()methodb3471b0
File::readPartialGrid()methodThis also removes one of the
File::readPartialGrid()variants and hoists the logic into the calling code instead.e990a15
There was no longer a good reason to use this pattern. It didn't serve a practical purpose at reducing compile times or improving binary compatibility. We support reinterpret casting a Grid pointer across ABI boundaries, but not a File or Stream object. This greatly simplifies the code.
7fe0036
5672d77
Some of the complexity in the I/O is due to handling different bounding box clipping options (no bounding box, index-space bounding box, world-space bounding box). Previously this code was checking
bbox.isSorted()to know whether to clip or not and doing an world-space -> index-space conversion early, this mainly delays that untilArchive::read()so as to reduce the number of variants. The public API remains the same after this change - it still has bbox clipping and non-bbox clipping methods, but the non-bbox clipping method now simply creates a default BBox and passes that down. In theory this introduces a tiny slowdown in checking this bbox is valid or not on every grid read (as opposed to once at the start), but the reduction in complexity vastly outweighs any performance concerns here IMO.864670c
Archive::readGrid()This change adds a new
partial = falseargument to the privateArchive::readGrid()method so thatFile::readGridPartial()can be removed entirely. Previously there was a loose agreement to keep the order of operations in both methods the same. The big change here is to move grid creation down intoArchive::readGrid()instead of doing this in the calling functions. In practical terms, the only real difference is that the exception thrown does not distinguish if it is a filename or stream if the grid type has not been registered.eb8dead