Skip to content

Validate glTF data in CgltfLoader#2975

Merged
jstone-lucasfilm merged 1 commit into
AcademySoftwareFoundation:mainfrom
jstone-lucasfilm:dev_cgltf_validation
Jun 17, 2026
Merged

Validate glTF data in CgltfLoader#2975
jstone-lucasfilm merged 1 commit into
AcademySoftwareFoundation:mainfrom
jstone-lucasfilm:dev_cgltf_validation

Conversation

@jstone-lucasfilm

Copy link
Copy Markdown
Member

This changelist adds a cgltf_validate step to CgltfLoader, rejecting invalid glTF files before their contents are used to construct meshes.

The cgltf parser is intentionally permissive, and does not verify that accessor ranges, buffer view extents, and object references are internally consistent. Validating the parsed data after buffer loading guards downstream mesh construction code against out-of-bounds reads when an invalid asset is loaded.

This changelist additionally fixes a memory leak in CgltfLoader, where a failure in buffer loading or validation would previously return without freeing the data allocated by cgltf_parse_file.

This changelist adds a `cgltf_validate` step to `CgltfLoader`, rejecting invalid glTF files before their contents are used to construct meshes.

The cgltf parser is intentionally permissive, and does not verify that accessor ranges, buffer view extents, and object references are internally consistent.  Validating the parsed data after buffer loading guards downstream mesh construction code against out-of-bounds reads when an invalid asset is loaded.

This changelist additionally fixes a memory leak in `CgltfLoader`, where a failure in buffer loading or validation would previously return without freeing the data allocated by `cgltf_parse_file`.
@jstone-lucasfilm jstone-lucasfilm requested a review from kwokcb June 10, 2026 18:55

@kwokcb kwokcb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

BTW, I noticed that `GeometryHandler::loadGeometry() which calls this method has a return code which is not checked in for GLSL, MSL, Slang (all copied code I assume).

@jstone-lucasfilm

Copy link
Copy Markdown
Member Author

Good point, @kwokcb, and we should indeed add proper error reporting to our render test suite in a future PR. For now, this means that the render test suite will silently render empty images rather than reporting invalid meshes, which isn't ideal.

@jstone-lucasfilm jstone-lucasfilm merged commit 7e7830e into AcademySoftwareFoundation:main Jun 17, 2026
36 checks passed
@jstone-lucasfilm jstone-lucasfilm deleted the dev_cgltf_validation branch June 17, 2026 21:47
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