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

Clarify byteStride restrictions #930

Merged
merged 3 commits into from
Apr 30, 2017
Merged

Clarify byteStride restrictions #930

merged 3 commits into from
Apr 30, 2017

Conversation

lexaknyazev
Copy link
Member

No syntax or semantic changes, just more verbose description.

@@ -17,7 +17,7 @@
"indices": {
"allOf": [ { "$ref": "glTFid.schema.json" } ],
"description": "The index of the accessor that contains the indices.",
"gltf_detailedDescription": "The index of the accessor that contains mesh indices. When this is not defined, the primitives should be rendered without indices using `drawArrays()`. When defined, the accessor must contain indices: the `bufferView` referenced by the accessor must have a `target` equal to 34963 (ELEMENT_ARRAY_BUFFER); a `byteStride` that is tightly packed, i.e., 0 or the byte size of `componentType` in bytes; `componentType` must be 5121 (UNSIGNED_BYTE), 5123 (UNSIGNED_SHORT) or 5125 (UNSIGNED_INT), the latter is only allowed when `OES_element_index_uint` extension is used; `type` must be `\"SCALAR\"`."
"gltf_detailedDescription": "The index of the accessor that contains mesh indices. When this is not defined, the primitives should be rendered without indices using `drawArrays()`. When defined, the accessor must contain indices: the `bufferView` referenced by the accessor should have a `target` equal to 34963 (ELEMENT_ARRAY_BUFFER); `componentType` must be 5121 (UNSIGNED_BYTE), 5123 (UNSIGNED_SHORT) or 5125 (UNSIGNED_INT), the latter may require enabling additional hardware support; `type` must be `\"SCALAR\"`."
Copy link
Contributor

Choose a reason for hiding this comment

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

a byteStride that is tightly packed, i.e., 0 or the byte size of componentType in bytes

Why is this no longer a requirement?

Copy link
Member Author

Choose a reason for hiding this comment

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

It never was a requirement. I don't think that there's any use case for this field except interleaved vertex attribs. So it would be safer to limit its usage explicitly.

Copy link
Member Author

Choose a reason for hiding this comment

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

The description of bufferView.byteStride was also refined to reflect this:

"gltf_detailedDescription": "The stride, in bytes, between vertex attributes. When this is zero, data is tightly packed.",

Copy link
Contributor

Choose a reason for hiding this comment

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

Are we saying byteStride is only for vertex attributes which means it will be ignored (or not allowed) for other data? If so, maybe this needs some description in the data alignment section?

Copy link
Member Author

Choose a reason for hiding this comment

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

From Buffers and Buffer Views:

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it clear, that in case of omitted (zero) bufferView.byteStride that restriction still applies?

My understanding from the text is that if byteStride is zero, then effectively byteStride is deduced from the type/componentType of the accessor that references it. If you are saying that accessors using the same bufferView must still use the same byteStride even when byteStride is zero, then no, I don't think that's clear, but I also don't see why anyone would do this.

Copy link
Member Author

Choose a reason for hiding this comment

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

If different accessors have different "deduced" stride, than "byte-distance between consequential elements" isn't the same accross buffer view which undermines its purpose.

More robust approach would be to require byteStride when more than one vertex attribute accessor use the same buffer view.

Copy link
Contributor

Choose a reason for hiding this comment

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

require byteStride when more than one vertex attribute accessor use the same buffer view

Sounds good to me. My guess is that, in practice, none of this will matter since I don't see why anyone would point multiple accessors to the same bufferView with different "deduced" strides.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't see why anyone would point multiple accessors to the same bufferView with different "deduced" strides

My point is that such layout must be forbidden, so implementations won't need to "fix" it before uploading to GPU.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair point. I was thinking too much about exporting.

@@ -584,7 +584,7 @@ When nor `sparse`, neither `bufferView` is defined, `min` and `max` properties c

The offset of an `accessor` into a `bufferView` (i.e., `accessor.byteOffset`) and the offset of an `accessor` into a `buffer` (i.e., `accessor.byteOffset + bufferView.byteOffset`) must be a multiple of the size of the accessor's component type.

When `byteStride` of referenced `bufferView` equals `0` (or not defined), it means that accessor elements are tightly packed, i.e., actual stride equals size of the element. When `bufferView.byteStride` is defined, it must be a multiple of the size of the accessor's component type.
When `byteStride` of referenced `bufferView` equals `0` (or not defined), it means that accessor elements are tightly packed, i.e., effective stride equals to the size of the element. Such effective stride must be the same across all accessors that use that `bufferView`. When `bufferView.byteStride` is defined, it must be a multiple of the size of the accessor's component type.

Choose a reason for hiding this comment

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

Change "equals to the" to "equals the"

@lexaknyazev lexaknyazev merged commit d122766 into 2.0 Apr 30, 2017
@lexaknyazev lexaknyazev deleted the byteStride branch April 30, 2017 11:38
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.

4 participants