Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate that material variant names aren't NULL #235

Merged
merged 1 commit into from
Nov 27, 2023
Merged

Validate that material variant names aren't NULL #235

merged 1 commit into from
Nov 27, 2023

Conversation

zeux
Copy link
Contributor

@zeux zeux commented Oct 26, 2023

Some names in glTF data structures are required and some are optional.

We normally ensure that names that are required can't be NULL as a byproduct of how they are parsed: required names usually come up when they are JSON object keys, so a lack of a name means the object isn't parsed.

Material variants are an odd exception: the name is required (in fact it's the only thing that a variant has...), but it's stored as a "name" key in an array of objects because the extension needs to maintain variant order. It's natural for the user to assume that the name is not NULL as the spec requires that, but it can be NULL in non-compliant documents, so we validate this now.

Some names in glTF data structures are required and some are optional.

We normally ensure that names that are required can't be NULL as a
byproduct of how they are parsed: required names usually come up when
they are JSON object keys, so a lack of a name means the object isn't
parsed.

Material variants are an odd exception: the name is required (in fact
it's the only thing that a variant has...), but it's stored as a "name"
key in an array of objects because the extension needs to maintain
variant order. It's natural for the user to assume that the name is not
NULL as the spec requires that, but it can be NULL in non-compliant
documents, so we validate this now.
zeux added a commit to zeux/meshoptimizer that referenced this pull request Oct 26, 2023
@jkuhlmann jkuhlmann merged commit 64d6781 into jkuhlmann:master Nov 27, 2023
3 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