GLTFExporter: Fix JSON chunk padding issue.

[takahirox]

This PR fixes padding logic in stringToArrayBuffer() of GLTFExporter for multi-byte strings. The current logic doesn't take account of the possibility that .name can be multi-byte strings.

[donmccurdy]

We only use this method once — could we just call getPaddedArrayBuffer() on the result? Conceptually converting a string to an arraybuffer does not necessarily require padding.

[takahirox]

So why does even the current stringToArrayBuffer() have padded option?

[donmccurdy]

😅 Oops. Let's remove that and just do padding when we're writing a bufferView or GLB chunk.

[takahirox]

OK, I'll update,

[fernandojsg]

Yeah you're right :D I guess it was some duplicated use somewhere on the code that didn't need padding and it was removed after all :)

[takahirox]

Probably we misunderstood each other.

stringToArrayBuffer() is used once for converting from JSON string to ArrayBuffer for entire JSON chunk. And we can't use getPaddedArrayBuffer() for the JSON chunk array buffer because

https://github.com/KhronosGroup/glTF/tree/master/specification/2.0#structured-json-content

This chunk must be padded with trailing Space chars (0x20) to satisfy alignment requirements.

If we pad with 0, JSON.parse( jsonChunk ) will fail when loading.

So padding in stringToArrayBuffer() is good. Adding an padding value (0 or 0x20) option to getPaddedArrayBuffer() would be another option.

[donmccurdy]

Hm I see what you mean :/ ...hoping we can still solve this with a smaller change to getPaddedArrayBuffer though, how about this?

function getPaddedArrayBuffer( arrayBuffer, paddingByte ) {

        paddingByte = paddingByte || 0;
	var paddedLength = getPaddedBufferSize( arrayBuffer.byteLength );

	if (paddedLength !== arrayBuffer.byteLength ) {

		var paddedBuffer = new ArrayBuffer( paddedLength );
		new Uint8Array( paddedBuffer )
                  .set( [ paddingByte, paddingByte, paddingByte ], paddedLength - 3 )
                  .set( new Uint8Array( arrayBuffer ) );
		return paddedBuffer;

	}

	return arrayBuffer;

}

EDIT: Sigh. .fill() isn't supported in IE11 or Safari, so edited above.

[takahirox]

That's the one I mentioned

Adding an padding value (0 or 0x20) option to getPaddedArrayBuffer() would be another option.

[takahirox]

Came to think adding that option would be better. We don't need two padding functions. I'll update.

[takahirox]

Updated.

• Added paddingByte option to getPaddedArrayBuffer()
• Removed padded option from stringToArrayBuffer()
• Cleaned up those two functions a bit

@donmccurdy

new Uint8Array( paddedBuffer )
    .set( [ paddingByte, paddingByte, paddingByte ], paddedLength - 3 )
    .set( new Uint8Array( arrayBuffer ) );

Just in case, .set doesn't return this.

[mrdoob]

Thanks!