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

Stride forces non-vertex attribute accessors to each have their own buffer view? #1537

Closed
scurest opened this issue Jan 18, 2019 · 7 comments

Comments

@scurest
Copy link

scurest commented Jan 18, 2019

Here is what the spec says about stride

When a buffer view is used for vertex attribute data, it may have a byteStride property. This property defines the stride in bytes between each vertex.

[...]

When byteStride of referenced bufferView is not defined, it means that accessor elements are tightly packed, i.e., effective stride equals the size of the element. When byteStride is defined, it must be a multiple of the size of the accessor's component type. byteStride must be defined, when two or more accessors use the same bufferView.

[...]

bufferView.byteStride

The stride, in bytes, between vertex attributes. When this is not defined, data is tightly packed. When two or more accessors use the same bufferView, this field must be defined.

In addition, before #1501 the first quote read (note the more explicit last sentence)

Buffer view could have byteStride property. It means byte-distance between consequential elements. This field is defined only for buffer views that contain vertex attributes.

From this conclude

  1. A buffer view which is used by an accessor that is not vertex attribute data must not have a stride property defined (even if that stride property is the implied value of the size of the element).
  2. The requirement that a buffer view used by more than one accessor have a stride property defined is unconditional, ie. it applies regardless of whether the accessor hold vertex attribute data or not.
  3. It follows that only vertex attribute accessors can share buffer views; other accessors must each have their own buffer view.

I was originally going to file an issue against the validator (cc @lexaknyazev) because (1) is not flagged (example) and (2) is only flagged when the two accessors are vertex attribute data.

But I'm wondering if this is a spec issue instead with (2) instead, because

So assuming (2) is false, I think the spec should be updated to say something like

byteStride must be defined, when two or more accessors used for vertex attribute data use the same bufferView.

[...]

When two or more accessors used for vertex attribute data use the same bufferView, this field must be defined.

Also, what is the reason for (1)? I was thinking of using the stride for animation keyframes (by reusing the same buffer of keyframes just with buffer views with different strides to animate on ones, one twos, on fours, etc.).

@donmccurdy
Copy link
Contributor

I'm inclined to say that (2) might be a mistake. In particular...

When two or more accessors use the same bufferView, [byteStride] must be defined.

... seems to imply that vertex attributes cannot share a bufferView unless they are interleaved. I'm aware of no such requirement, and pretty sure our sample models would break that rule.

If so, then (1) isn't an issue, correct?

@lexaknyazev
Copy link
Member

@scurest
As far as I remember, the stride was supposed to be used only for vertex attributes because extracting interleaved data will be done by the GPU API rather than glTF client thus potentially simplifying implementation. /cc @pjcozzi

@donmccurdy

seems to imply that vertex attributes cannot share a bufferView unless they are interleaved

Vertex attributes can be non-interleaved and still use the same bufferView because accessors have their own byte offsets.

@donmccurdy
Copy link
Contributor

Vertex attributes can be non-interleaved and still use the same bufferView because accessors have their own byte offsets.

Yes, but I don't see a requirement that accessors within the same bufferView must all have the same element sizes. In that case, this spec language...

When byteStride of referenced bufferView is not defined, it means that accessor elements are tightly packed, i.e., effective stride equals the size of the element.

... implies that because the byteStride must be defined on the shared bufferView, it must also apply to all accessors pointing to that bufferView. On the other hand, if we consider (2) a mistake, omission of byteStride means that each accessor within the bufferView individually has a byte stride equal to the size of its elements.

@lexaknyazev
Copy link
Member

Only accessors of the same "real" stride can share a buffer view. This is a part of design of all modern APIs (including OpenGL with ARB_vertex_attrib_binding extension) - see illustrations in this comment.

@donmccurdy
Copy link
Contributor

donmccurdy commented Jan 22, 2019

Only accessors of the same "real" stride can share a buffer view.

Ok, I see. The validator checks for Referenced bufferView's byteStride is less than accessor element's length.. Does it check the inverse, i.e. bufferView byteStride is larger than the accessor element's length but the accessor is not padded accordingly? I would be curious whether current samples and exporters satisfy that.

Aside: We split bufferViews in three.js unless they are interleaved — it would be difficult to keep the attribute groupings intact within our own internal geometry representation, I think.

@lexaknyazev
Copy link
Member

Does it check the inverse, i.e. bufferView byteStride is larger than the accessor element's length but the accessor is not padded accordingly?

I don't think I follow this. Could you please provide an example layout?

@donmccurdy
Copy link
Contributor

Never mind, I think it's unrelated to the question here anyway.

Going back to @scurest's points, I can see where (3) seems unnecessary. If samples like AnimatedTriangle already disobey this, loosening the requirement for non-vertex-attribute data might be possible. Curious to hear what others think.

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

No branches or pull requests

3 participants