Skip to content

Enforce glTF index contract on decoded indices - #1447

Open
bjornblissing wants to merge 2 commits into
CesiumGS:mainfrom
bjornblissing:fix/draco-index-component-type
Open

bjornblissing wants to merge 2 commits into
CesiumGS:mainfrom
bjornblissing:fix/draco-index-component-type

Conversation

@bjornblissing

@bjornblissing bjornblissing commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Description

copyDecodedIndices (CesiumGltfReader/src/decodeDraco.cpp) used pIndicesAccessor->componentType to size the decoded index buffer and to pick the copy branch, without validating it first. If the accessor declared a componentType that isn't a legal glTF index type (e.g. BYTE, SHORT, FLOAT, or some other unrecognized value), computeByteSizeOfComponent() returned 0, so the index buffer was sized to zero bytes while the accessor still advertised a nonzero count, and the copy switch matched no case, silently copying nothing into that zero-length buffer — leaving behind an inconsistent, corrupted accessor.

Per the glTF spec, mesh.primitive.indices requires the accessor to have SCALAR type and an unsigned integer componentType, and getIndexAccessorView additionally rejects normalized accessors. This PR narrows the accepted componentTypes to exactly UNSIGNED_BYTE, UNSIGNED_SHORT, and UNSIGNED_INT:

  • An illegal componentType is now replaced with one derived from the decoded Draco point count (with a warning), instead of falling through to a zero-sized buffer.
  • A legal but too-narrow componentType is still widened as before.
  • type is forced to SCALAR and normalized to false, matching the spec requirements.
  • The final copy switch now only handles the same three validated types, with the default case asserting unreachability, so the validated set and the copied set can no longer drift apart.

Issue number or link

N/A

Author checklist

  • I have submitted a Contributor License Agreement (only needed once).
  • I have done a full self-review of my code.
  • I have updated CHANGES.md with a short summary of my change (for user-facing changes).
  • I have added or updated unit tests to ensure consistent code coverage as necessary.
  • I have updated the documentation as necessary.

Testing plan

Steps to reproduce the original issue:

  1. Load a glTF model with a KHR_draco_mesh_compression extension where the primitive's indices accessor declares a componentType that is not a legal glTF index type (e.g. FLOAT or a signed integer type).
  2. Before the fix, computeByteSizeOfComponent() returns 0 for that componentType, so the decoded index buffer is allocated with zero bytes while the accessor's count remains nonzero, and no data is copied into it — producing a corrupted, inconsistent accessor.
  3. After the fix, the illegal componentType is detected and replaced with one derived from the decoded Draco point count, a warning is emitted, and the index buffer is correctly sized and populated.

`copyDecodedIndices` used `pIndicesAccessor->componentType` to size
the index buffer and select the copy branch without validating it.
An unrecognized componentType made `computeByteSizeOfComponent()`
return 0, so the buffer was sized to zero and the copy switch
matched no case, leaving an inconsistent zero-length accessor still
advertising a nonzero count.

mesh.primitive.indices requires SCALAR type and an unsigned integer
componentType, and getIndexAccessorView also rejects normalized
accessors. Accept only UNSIGNED_BYTE, UNSIGNED_SHORT, and
UNSIGNED_INT. Replace an illegal componentType with one derived from
the decoded point count, widen a legal one that is too narrow, and
force SCALAR and normalized = false. The copy switch shrinks to the
same three types, so the validated and copied sets cannot drift
apart.
Cover the glTF index contract enforced when decoding Draco
indices: unknown, FLOAT, signed, and normalized componentTypes
must be corrected or rejected, and a too-narrow but legal
componentType must still be accepted. Each case asserts the
resulting accessor satisfies getIndexAccessorView, guarding
against regressions in how decoded indices are validated and
sized.

Reuses the existing CesiumMilkTruck Draco bitstream, so no new
binary test data is required.
@j9liu j9liu added this to the November 2026 Release milestone Sep 28, 2026
@j9liu
j9liu requested a review from azrogers September 28, 2026 17:26
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