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

Tighten the rules around bufferView.byteStride #1888

Closed
wants to merge 2 commits into from
Closed

Tighten the rules around bufferView.byteStride #1888

wants to merge 2 commits into from

Conversation

zeux
Copy link
Contributor

@zeux zeux commented Oct 21, 2020

The intention of the spec has always been to only use byteStride for
buffer views with vertex data. However, the current language was lax in
a few places, which led to logical contradictions:

  • When two accessors with non-vertex data used the same bufferView, it
    could be interpreted as requiring byteStride on that view
  • However, byteStride was only explicitly allowed for vertex attribute
    data, making it seem as if it's not valid on non-vertex views.

glTF-Validator flags bufferViews that have a byteStride but store
non-vertex data as erroneous. Based on this and the above, this change
tightens the language to explicitly only allow byteStride on vertex data
views, and changes all references to it to say the same.

This would close #1864 by explicitly outlawing interleaved data that's
not vertex data, and also relates to #1537 (that is already closed, but this
change would provide an actual resolution there).

The intention of the spec has always been to only use byteStride for
buffer views with vertex data. However, the current language was lax in
a few places, which led to logical contradictions:

- When two accessors with non-vertex data used the same bufferView, it
could be interpreted as requiring byteStride on that view
- However, byteStride was only explicitly allowed for vertex attribute
data, making it seem as if it's not valid on non-vertex views.

glTF-Validator flags bufferViews that have a byteStride but store
non-vertex data as erroneous. Based on this and the above, this change
tightens the language to explicitly only allow byteStride on vertex data
views, and changes all references to it to say the same.
@zeux
Copy link
Contributor Author

zeux commented Oct 21, 2020

If we want to keep #1864 open I can remove the line about "other types of data"; the critical part here is to make the "two or more accessor" rule more explicit about only applying to vertex data, as multiple accessors that reference the same bufferView with tightly packed data by referring to individual sections of the buffer through byte offsets should be valid.

Copy link
Contributor

@donmccurdy donmccurdy left a comment

Choose a reason for hiding this comment

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

I believe this PR reflects the original intent of the specification, and clarifies it. There does not seem to be any other interpretation that would be compatible within the glTF 2.0 lifecycle, in any case.

Looks good to me. 👍

@lexaknyazev
Copy link
Member

This clarification has been integrated into the new AsciiDoc version of the spec. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Interleaved animation data
6 participants