GLTFExporter: implement 4-byte data alignment for buffer #13220
Conversation
This change ensures the allocated buffer size is aligned to the next 4-byte boundary. See https://github.com/KhronosGroup/glTF/tree/master/specification/2.0#data-alignment There may also be other allocated buffers (glb output?) that require 4-byte alignment, but this fixes validation of the exported file for me. This change is cribbed from getBufferPadded.js of obj2gltf but is modified for this use case.
|
|
||
| var remainder = bufferSize % boundary; | ||
|
|
||
| var padding = (remainder === 0 ) ? 0 : boundary - remainder; |
There was a problem hiding this comment.
nit: could you remove the parens here, just: remainder === 0 ? 0 : boundary - remainder;
|
|
||
| var padding = remainder === 0 ? 0 : boundary - remainder; | ||
|
|
||
| return bufferSize + padding; |
There was a problem hiding this comment.
How about
function getPaddedBufferSize( bufferSize ) {
return Math.ceil( bufferSize / 4 ) * 4;
}There was a problem hiding this comment.
I'd have to leave that for debate. Your example is wonderfully succinct but the original is easier to read.
There was a problem hiding this comment.
The code you copied pads a buffer. You just need to calculate a length.
There was a problem hiding this comment.
Indeed, we do need to calculate a length - for a padded buffer. I appreciate the terseness of your suggested alternative from a nerdy point of view, but not regards readability. I'm not massively fussed which approach is accepted however I'm just knocking down gltf validation errors.
There was a problem hiding this comment.
Yeah, I think +1 for the short version. There are already plenty of comments explaining the purpose clearly.
There was a problem hiding this comment.
Ok, done!
|
Thanks! |
This change ensures the allocated buffer size is aligned to the next 4-byte boundary. See https://github.com/KhronosGroup/glTF/tree/master/specification/2.0#data-alignment
There may also be other allocated buffers (glb output?) that require 4-byte alignment, but this fixes validation of the exported file for me (via http://github.khronos.org/glTF-Validator/, from 27 errors down to zero).
This change is cribbed from getBufferPadded.js of obj2gltf but is modified for this use case.