Fixed alignment issues #298
Conversation
…fixed generate face normals to not crash on degenerate triangles.
lib/mergeBuffers.js
Outdated
|
|
||
| var sourceBuffer = buffer.extras._pipeline.source; | ||
| var currentBufferLength = sourceBuffer.length; | ||
| if (currentBufferLength % 4) |
There was a problem hiding this comment.
Is this 4 because of Float32 based on the implementation note here? https://github.com/KhronosGroup/glTF/tree/master/specification/1.0#bufferview-and-accessor-byte-alignment
It might be worthwhile to add a blurb summarizing why the 4-byte alignment is needed.
There was a problem hiding this comment.
Yep. A float must also start on 4-byte boundary. My particular case had something like 102 bytes of shorts containing batch ids so every float array after it in the buffer was unable to be read. The C++ converter handles this, but during the processing the padding is thrown out, so the problem creeps back in again.
| Cartesian3.cross(scratchNormalOne, scratchNormalTwo, normal); | ||
| Cartesian3.normalize(normal, normal); | ||
|
|
||
| // Check for degenerate triangles |
There was a problem hiding this comment.
When this is merged, please update #242 to note that this check is here.
|
Thanks @tfili! |
Fixed issues with merging buffers that weren't a multiple of 4.
I wrote up #299 because there are some issues that prevented me from writing a test for the degenerate triangles. It opened a can of worms that was outside the scope of this PR.