[BUG] order of grids different when using openvdb::io::File vs openvdb::io::Stream

[Carandiru0]

Windows 10 VDB 9.0 Clang 13 LLVM

When a .vdb file is opened with openvdb::io::File and iterated, the order of the names is different then when using openvdb::io::Stream and iterating.

openvdb::io::File file(sequence_filename);
// Open the file.  This reads the file header, but not any grids.
file.open();

if (file.isOpen()) {
   std::string channel_names[3];
   uint32_t numChannels(0);

  for (openvdb::io::File::NameIterator nameIter = file.beginName(); nameIter != file.endName(); ++nameIter)
  {
    channel_names[numChannels] = nameIter.gridName();
    ++numChannels;
  }

  FMT_LOG_WARN(VOX_LOG, "{:d} channels " " [{:s}]  [{:s}]  [{:s}]", numChannels, channel_names[0], channel_names[1], 
                                                                                                       channel_names[2]);
}
file.close();

and

std::ifstream file(frame_data.path, std::ios_base::in | std::ios_base::binary);
openvdb::io::Stream const stream(file, false); // *bugfix - parameter false, disables "delayed" loading of voxel data. If delayed-loading is on, the performance tanks! speedup++
openvdb::GridPtrVecPtr const	grids(stream.getGrids());
			
file.close();
						
uint32_t const grid_count(SFM::min(3, grids->size()));
for (uint32_t grid_index = 0; grid_index < grid_count; ++grid_index) { 

   GridType::Ptr const grid(openvdb::GridBase::grid<GridType>((*grids)[grid_index]));

   std::string const name(grid->getName()); // debug breakpoint here
}

Order of grid names needs to be the same when opened with openvdb::io::File or openvdb::io::Stream

The code above is a simple example needed to reproduce the problem and observe the resulting bug.

[Idclip]

Hey @Carandiru0 thanks for reporting this. I can reproduce this issue. I believe this is because a openvdb::io::File stores grids in a std::multimap where as the constructor of openvdb::io::Stream(is, false) avoid this and directly appends grids as they are being deserialized. This fix would be to change this loop:

https://github.com/AcademySoftwareFoundation/openvdb/blob/3a38b68fd13549eafd5d5f9ed47e0fcb9ddbc9e2/openvdb/openvdb/io/Stream.cc#L115-L122

to:

        for (int32_t i = 0; i < gridCount; ++i) {
            GridDescriptor gd;
            gd.read(is);
            descriptors.push_back(gd);
            GridBase::Ptr grid = readGrid(gd, is);
            namedGrids[gd.uniqueName()] = grid;
        }
        // add grids based on multimap order
        for (auto& k : namedGrids) {
            mImpl->mGrids->push_back(k.second);
        }

An obvious side effect of this change would be breaking code that current relies on openvdb::io::Stream const stream(file, false); so my advice to you would be to sort the grid names yourself in both cases whilst we consider if we can fix this.

Finally this seems to only be an issue if you use openvdb::io::Stream with delayLoad false, is that correct? If you change your code to openvdb::io::Stream const stream(file, true);, does the orders match? I see your comment about performance when delay loading is enabled, can you elaborate on what you're seeing there? I have a suspicion as to why what you're seeing is worse but it shouldn't be so this sounds like another issue.

dev: we should probably be defining our own comparison op in said multimap for platform deterministic behaviour.