Skip to content

fix(usdc): decode integer-compressed float arrays#126

Merged
mxpv merged 1 commit into
mxpv:mainfrom
yohawing:fix/usdc-integer-compressed-floats
Jul 24, 2026
Merged

fix(usdc): decode integer-compressed float arrays#126
mxpv merged 1 commit into
mxpv:mainfrom
yohawing:fix/usdc-integer-compressed-floats

Conversation

@yohawing

Copy link
Copy Markdown
Contributor

Hello! I’ve been following your progress over the past six months. Thank you for your excellent work—it has made it possible for me to incorporate it into my own projects as well.
I’m developing my own 3D model viewer, and I’d like to report a bug I found.

read_floats treated the i (compressed-integers) branch of a float array's value rep as plain LZ4-compressed i32s and cast them straight to f32. Pixar's crateFile.cpp _ReadCompressedInts (see https://github.com/PixarAnimationStudios/OpenUSD/blob/release/pxr/usd/usd/crateFile.cpp) additionally runs the decompressed buffer through Usd_IntegerCompression decoding before returning the integers, because the i code is USD's compact encoding for arrays whose values happen to all be integral, not a marker for "raw i32 dump". Skipping that decode step silently produces garbage floats.

This only reproduces when every element of a float[]/double[] array is an integral value (e.g. joint weights that are all 0.0/1.0), which is exactly the case where the crate writer picks the i array encoding. A 16-element all-integral float[] written by usdcat decoded to values such as 269505630.0 and -16646399.0 instead of the original 0.0/1.0 before this fix.

Fix by routing the i branch through read_encoded_ints, the same Usd_IntegerCompression-aware decoder already used elsewhere in this file for token/path/spec integer sections, instead of the raw read_compressed LZ4-only path.

Adds a regression fixture (fixtures/integer_compressed_floats.usdc, generated with upstream usdcat from the paired .usda) plus two tests: one exercising the fixture through the public field-reading API and one asserting the fixture actually triggers the i array-value-rep code path, so a future refactor can't silently drop coverage of this branch.

`read_floats` treated the `i` (compressed-integers) branch of a float
array's value rep as plain LZ4-compressed `i32`s and cast them straight
to `f32`. Pixar's crateFile.cpp `_ReadCompressedInts` (see
https://github.com/PixarAnimationStudios/OpenUSD/blob/release/pxr/usd/usd/crateFile.cpp)
additionally runs the decompressed buffer through `Usd_IntegerCompression`
decoding before returning the integers, because the `i` code is USD's
compact encoding for arrays whose values happen to all be integral, not
a marker for "raw i32 dump". Skipping that decode step silently produces
garbage floats.

This only reproduces when every element of a `float[]`/`double[]` array
is an integral value (e.g. joint weights that are all `0.0`/`1.0`), which
is exactly the case where the crate writer picks the `i` array encoding.
A 16-element all-integral `float[]` written by `usdcat` decoded to values
such as `269505630.0` and `-16646399.0` instead of the original `0.0`/`1.0`
before this fix.

Fix by routing the `i` branch through `read_encoded_ints`, the same
Usd_IntegerCompression-aware decoder already used elsewhere in this file
for token/path/spec integer sections, instead of the raw `read_compressed`
LZ4-only path.

Adds a regression fixture (`fixtures/integer_compressed_floats.usdc`,
generated with upstream `usdcat` from the paired `.usda`) plus two tests:
one exercising the fixture through the public field-reading API and one
asserting the fixture actually triggers the `i` array-value-rep code path,
so a future refactor can't silently drop coverage of this branch.
@mxpv
mxpv force-pushed the fix/usdc-integer-compressed-floats branch from 77d7d80 to c56469a Compare July 23, 2026 03:45
@mxpv

mxpv commented Jul 24, 2026

Copy link
Copy Markdown
Owner

Thanks!

@mxpv
mxpv merged commit b0044a2 into mxpv:main Jul 24, 2026
5 checks passed
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