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

buffer.byteLength of GLB-stored buffer #1026

Closed
lexaknyazev opened this issue Jun 22, 2017 · 11 comments
Closed

buffer.byteLength of GLB-stored buffer #1026

lexaknyazev opened this issue Jun 22, 2017 · 11 comments
Assignees

Comments

@lexaknyazev
Copy link
Member

Must the length of GLB-stored BIN chunk be equal to buffer.byteLength? Spec doesn't define their relations. Since GLB chunk must be padded to 4-byte boundary, converting ASCII glTF to GLB may require adjusting buffer.byteLength.

Otherwise we can say that buffer.byteLength must be less then or equal to the length of BIN chunk.

CC @pjcozzi @sbtron @bghgary

@bghgary
Copy link
Contributor

bghgary commented Jun 22, 2017

I have been assuming yes. The Babylon.js glTF loader currently throws a warning if they don't match.

@lexaknyazev
Copy link
Member Author

Currently, COLLADA2GLTF adds padding in GLB container, but doesn't update buffer.byteLength . I don't see this as an issue as long as BIN chunk's length is not smaller than buffer.byteLength (otherwise it's a error, not a warning).

@pjcozzi
Copy link
Member

pjcozzi commented Jun 28, 2017

I do not recall. @tfili or @lilleyse may know and may have written that part of COLLADA2GLTF.

@lexaknyazev
Copy link
Member Author

@pjcozzi
The binary padding part was added by me recently (it wasn't the case with 1.0).

@pjcozzi
Copy link
Member

pjcozzi commented Jun 28, 2017

Ah 😜

@vpenades
Copy link
Contributor

vpenades commented Sep 12, 2017

Making Buffer's byteLength always match the actual buffer byte size would make the field redundant.

So my opinion is that byteLength field should represent the "meaningful" part of the buffer, or to be removed altogether.

The only practical use of the field is to do an early check to know if we have all the bytes. And if that's the case, a less-equal test would be good enough, there's no need for an exact match.

@zellski
Copy link
Contributor

zellski commented Sep 12, 2017

@vpenadea Surely the main use of the field is to allow preallocation of memory when loading? Buffers can be of non-trivial size...

@vpenades
Copy link
Contributor

@zellski True, I was thinking more in desktop environments where it's easy to open a file and check its size, I didn't considered streaming the file from web, where you usually don't know the file size until the end.

So then it's a matter of telling which value is more important for the scene to be processed correctly, the byteLength field, or the file size itself.

If the file size is larger than byteLength, we can simply stop filling the buffer when we've read byteLength bytes.

@pjcozzi
Copy link
Member

pjcozzi commented Oct 19, 2017

Discussed on a 3D Formats call.

If the asset uses more data than the .glb contains it is an error. If it uses less - up to 3 bytes to allow for padding - or equal, it is OK.

This allows adding padding to .glb when converting .gltf to .glb without changing the original JSON buffer part.

Requires an update to the .glb part of the spec.

@lexaknyazev volunteered to update the spec.

@stevenvergenz
Copy link

Doesn't the spec already specify that the JSON chunk is padded with space (0x20) characters? Is that no longer desirable?

@lexaknyazev
Copy link
Member Author

@stevenvergenz
All chunks in GLB must be padded to 4-byte lengths.
Clarification is that buffer.byteLength JSON property could be up to 3 bytes smaller than the length of GLB binary chunk.

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

6 participants