gltf-rs / gltf

A crate for loading glTF 2.0
Apache License 2.0
535 stars 124 forks source link

Fix size hint and values read with sparse iterator #411

Closed derwiath closed 10 months ago

derwiath commented 10 months ago

In #313 @wsw0108 shows that collecting positions specified using a sparse accessor triggers a panic (attempt to subtract with overflow) inside of size_hint() of SparseIter.

In this PR I have rewritten the code in the issue as two unit tests, one that verifies size_hint() and one that verifies that the positions are actually correct. Both tests fail when using latest main.

The SimpleSparseAccessor.gltf test asset contains 14 positions specified in a sparse accessor where three of them is overrideen by values specified in the sparse section of the accessor. Look at the data layout image here for an overview: gltf-test/tutorialModels/SimpleSparseAccessor.

The two tests pass after delegating the size_hint() call to the iterator for the base accessor, it knows how many items are left.

The gltf specification on Sparse Accessors mentions that a sparse accessor may not have a bufferView set

When accessor.bufferView is undefined, the sparse accessor is initialized as an array of zeros of size (size of the accessor element) * (accessor.count) bytes.

For SparseIter, base will not be set and size_hint() cannot rely on it anymore. The solution is simple, simply pass accessor.count to SparseIter so that it can use it to know how many items there are. Then it simply deducts self.counter from it as that variable keeps track of how many items have been read from the iterator.

The asset tests/box_sparse.gltf contains a sparse accessor without a base buffer view. This is used in two unit tests that the behavior is correct, even for these sorts of accessors.

alteous commented 10 months ago

Thanks for fixing this!