Merge pull request #421 from AnalyticalGraphicsInc/byte-alignment

Created accessor bufferViews should be byte aligned
CesiumGS · Sep 12, 2018 · 353cd8e · 353cd8e
2 parents 0b051aa + 7c0d4cb
commit 353cd8e
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 12 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -4,6 +4,7 @@ Change Log
 ### 2.0.1 - ????
 
 * Fixed a bug where the buffer `byteOffset` was not set properly when updating 1.0 accessor types to 2.0 allowed values. [#418](https://github.com/AnalyticalGraphicsInc/gltf-pipeline/pull/418)
+* Fixed a bug where bufferViews were not properly byte aligned when updating accessors from 1.0 to 2.0. [#421](https://github.com/AnalyticalGraphicsInc/gltf-pipeline/pull/421)
 * Fixed a bug in `removePipelineExtras` when run in the browser. [#422](https://github.com/AnalyticalGraphicsInc/gltf-pipeline/pull/422)
 
 ### 2.0.0 - 2018-08-14

diff --git a/lib/updateVersion.js b/lib/updateVersion.js
@@ -790,7 +790,8 @@ function moveByteStrideToBufferView(gltf) {
                 var accessorByteLength = accessor.count * accessorByteStride;
                 delete accessor.byteStride;
 
-                var nextAccessorByteStride = (i < accessorsLength - 1) ? computeAccessorByteStride(gltf, accessors[i + 1]) : undefined;
+                var hasNextAccessor = (i < accessorsLength - 1);
+                var nextAccessorByteStride = hasNextAccessor ? computeAccessorByteStride(gltf, accessors[i + 1]) : undefined;
                 if (accessorByteStride !== nextAccessorByteStride) {
                     var newBufferView = clone(bufferView, true);
                     if (bufferViewHasVertexAttributes[bufferViewId]) {
@@ -804,7 +805,8 @@ function moveByteStrideToBufferView(gltf) {
                         accessor.bufferView = newBufferViewId;
                         accessor.byteOffset = accessor.byteOffset - currentByteOffset;
                     }
-                    currentByteOffset = accessorByteOffset + accessorByteLength;
+                    // Set current byte offset to next accessor's byte offset
+                    currentByteOffset = hasNextAccessor ? accessors[i + 1].byteOffset : undefined;
                     currentIndex = i + 1;
                 }
             }

diff --git a/specs/lib/updateVersionSpec.js b/specs/lib/updateVersionSpec.js
@@ -443,15 +443,15 @@ describe('updateVersion', function() {
                 },
                 accessor_normal: {
                     bufferView: 'bufferViewAttributes',
-                    byteOffset: 36,
-                    componentType: WebGLConstants.FLOAT,
+                    byteOffset: 42,
+                    componentType: WebGLConstants.BYTE,
                     count: 3,
-                    type: 'VEC3'
+                    type: 'VEC2'
                 },
                 accessor_texcoord: {
                     bufferView: 'bufferViewAttributes',
-                    byteOffset: 72,
-                    componentType: WebGLConstants.FLOAT,
+                    byteOffset: 50,
+                    componentType: WebGLConstants.BYTE,
                     count: 3,
                     type: 'VEC2'
                 },
@@ -722,14 +722,14 @@ describe('updateVersion', function() {
                 var positionBufferView = gltf.bufferViews[positionAccessor.bufferView];
                 var texcoordBufferView = gltf.bufferViews[texcoordAccessor.bufferView];
                 expect(positionAccessor.bufferView).toBe(1);
-                expect(normalAccessor.bufferView).toBe(1);
+                expect(normalAccessor.bufferView).toBe(2);
                 expect(texcoordAccessor.bufferView).toBe(2);
-                expect(positionBufferView.byteLength).toBe(72);
+                expect(positionBufferView.byteLength).toBe(36);
                 expect(positionBufferView.byteOffset).toBe(12); // First unrelated buffer view is 12 bytes
                 expect(positionBufferView.byteStride).toBe(12);
-                expect(texcoordBufferView.byteLength).toBe(24);
-                expect(texcoordBufferView.byteOffset).toBe(72 + 12); // First unrelated buffer view is 12 bytes
-                expect(texcoordBufferView.byteStride).toBe(8);
+                expect(texcoordBufferView.byteLength).toBe(50 - 42 + 6); // Padding to next buffer view
+                expect(texcoordBufferView.byteOffset).toBe(42 + 12); // Byte offset of previous accessor plus byte length
+                expect(texcoordBufferView.byteStride).toBe(2);
                 expect(positionAccessor.byteStride).toBeUndefined();
                 expect(normalAccessor.byteStride).toBeUndefined();
                 expect(texcoordAccessor.byteStride).toBeUndefined();