🩹 Confine the zero-count early-out to the half-float path - #75
Open
MarijnS95 wants to merge 1 commit into
Open
Conversation
`read_compressed_data()` returned before touching the reader whenever `count` was zero. OpenVDB only skips the stream in `HalfReader</*IsReal=*/true, T>::read()` (Compression.h), which bails on `count < 1`; every other read goes through `readData()` -> `bloscFromStream()`/`unzipFromStream()`, and those always consume the block's `i64` size header plus its payload. Returning early therefore left the reader parked mid-block, and every subsequent read came back shifted. The desync surfaces as the `assert_eq!(-compressed_count as usize, count)` in the same function: the misplaced read picks up a `num_compressed_bytes` of 0 where a real count was expected, giving `left: 0, right: 204`. This only ever bit grids that are not stored as half floats, which matches the report in #73 that Houdini's f32 exports stayed broken while Embergen's f16 files were fixed - f16 grids take the half-float branch, where the early-out is correct. Move the early-out to that branch so it mirrors `HalfReader`, and leave a note in `read_compressed_data()` explaining why a blanket one cannot go back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.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.
read_compressed_data()returns before touching the reader whenevercountis zero, which desyncs the stream for every grid that is not stored as half floats. OpenVDB only skips the read inHalfReader</*IsReal=*/true, T>::read(), which bails oncount < 1before consuming anything; every other read goes throughreadData()intobloscFromStream()/unzipFromStream(), and those always consume the block'si64size header plus its payload regardless of the count. Returning early leaves the reader parked mid-block, so every subsequent read comes back shifted and eventually trips theassert_eq!(-compressed_count as usize, count)in that same function, reading anum_compressed_bytesof 0 where a real count was expected.#63 reports that same assertion, from before the early-out existed, and was never closed — #73 landed the blanket early-out and closed #33 instead. Since the file in #63 is one of the Embergen exports it is plausibly already covered by the half-float branch, so I am not claiming this closes it; what this fixes is the same assert firing for grids that are not half floats. That split also explains the loose end noted in #73, where Embergen's f16 exports came out fixed while the Houdini f32 exports stayed broken: f16 grids take the branch where the early-out genuinely belongs, and f32 grids take the path where OpenVDB always consumes the block. This moves the early-out into the half-float branch so it mirrors
HalfReader, and leaves a note inread_compressed_data()explaining why a blanket one cannot come back.For evidence I read Breda's 30 MB Nubis cloud-noise VDB, four
ACTIVE_MASK | BLOSCf32 grids that OpenVDB writes with zero-count blocks. Onmainit panics withleft: 0, right: 204. With this change all four grids parse, and an order-sensitive checksum over every voxel matches what the last release before #73 (vdb-rs0.6.0 from crates.io) produces, so the values are right and not merely non-crashing:The half-float path is untouched by construction, and OpenVDB's
smoke.vdbandfire.vdb(whosedensitygrids are half) give byte-identical checksums before and after. I could not find a grid that is both half-float and BLOSC to exercise that combination directly — the ASWF sample models predate BLOSC, and the Embergen files from #73 are not something I have — so that pairing rests on the early-out now sitting exactly where OpenVDB puts it rather than on a measurement.