Skip to content

Add In-Memory Conversion to No Offset Grids - #2298

Open
danrbailey wants to merge 4 commits into
AcademySoftwareFoundation:feature/iofrom
danrbailey:in_memory_conversion_no_offsets
Open

Add In-Memory Conversion to No Offset Grids#2298
danrbailey wants to merge 4 commits into
AcademySoftwareFoundation:feature/iofrom
danrbailey:in_memory_conversion_no_offsets

Conversation

@danrbailey

Copy link
Copy Markdown
Contributor

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.

Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>

@apradhana apradhana left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Comment thread openvdb/openvdb/io/File.cc Outdated
+ source.type() + "'; reading as original type");
return GridBase::Ptr();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick comment: for targetType == source.type(), we can say

if (targetType == source.type()) {
    // The grid already has the requested representation.
    return {GridBase::Ptr(), ConversionStatus::NoOp};
}

Comment thread openvdb/openvdb/io/File.cc Outdated
if constexpr (!std::is_same_v<typename SourceGridT::BuildType, ValueMask>) {
if constexpr (CanConvertType<typename SourceGridT::BuildType, TargetBuildT>::value) {
if (TargetGridT::gridType() == targetType) {
result = typename TargetGridT::Ptr(new TargetGridT(typedSource));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we special-case std::is_same_v<TargetBuildT, ValueMask> here and use a topology copy with inactive false and active true? TargetGridT(typedSource) copies the source grid’s values into the mask. For example, a nonzero background becomes true. But ReadMode::Mask should ignore the source values and create a mask based only on which voxels are active.

@danrbailey

Copy link
Copy Markdown
Contributor Author

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants