Add In-Memory Conversion to No Offset Grids - #2298
Conversation
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
apradhana
left a comment
apradhana
left a comment
There was a problem hiding this comment.
Thanks for adding this, @danrbailey .
I noticed that TopologyOnly is still ignored for no-offset files. Should we add a throw for this and add a comment saying that it's not supported in the ReadMode contract?
I went back and forth on this. TopologyOnly only reads the topology and doesn't read the value data. However, it still allocates and populates the values in the VDB (with zeros), that is to ensure that we never have partially allocated data. In the case of no-offset files, we have a full grid already cached, so we could deep copy that and then zero out all the values just so that the two match. I'm not sure I see the value in doing that though? I think what the reader is saying is that they only care about the Topology. We don't make any guarantees about what is in the value data - both zero values and actual value data satisfy that? |
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
…rnalNode::DeepCopy Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
@apradhana - this should be resolved now. All modes (TopologyOnly, Half, Bool, Mask) now work as expected with no-offset files. |
|
|
||
| GridBase::Ptr grid = cachedGrid; | ||
|
|
||
| if (readOptions.readMode == ReadMode::TopologyOnly) { |
There was a problem hiding this comment.
Please consider adding a comment a.k.a. contract where ReadMode::TopologyOnly is introduced in Codec.h that says that only topology is guaranteed and values may get background or already-cached data.
|
|
||
| GridBase::Ptr grid; | ||
| EXPECT_NO_THROW(grid = f.readGrid(srcGrid->getName(), readOptions)); | ||
| ASSERT_TRUE(grid); |
There was a problem hiding this comment.
Please consider adding a test that the returned grid has the same topology as srcGrid.
apradhana
left a comment
apradhana
left a comment
There was a problem hiding this comment.
Thanks for adding support for no-offset grids path. It looks good. I added two non-blocking comments.
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Thanks! I added a warning to the TopologyOnly section in Codec.h and extended the unit tests slightly. |
2 participants
This fixes an issue raised by @apradhana (and a few related issues).
When using VDB files that have no offsets, because they were written using a stream, the VDB grids are cached in-memory. These in-memory grids were not being converted (to half, bool, etc) if requested, as conversion was only applied during read.
This is now addressed using in-memory conversion for cached grids. In addition, this fixes clipping of cached grids and instance grids (which are effectively also cached).
Finally, I have added diagnostics when the hints were not applied such as when returning a cached grid and requesting a topology-only grid.