From 9100963f00481df05bc5c68431e7154cb941c3dd Mon Sep 17 00:00:00 2001 From: Dan Bagnell Date: Wed, 14 Oct 2015 14:01:49 -0400 Subject: [PATCH] Do not use the index of the max value of an unsigned short. Fixing tests WIP. --- Source/Core/GeometryPipeline.js | 4 ++-- Source/Core/IndexDatatype.js | 4 ++-- Source/Core/TerrainProvider.js | 4 ++-- Source/Renderer/VertexArray.js | 2 +- Source/Renderer/VertexArrayFacade.js | 6 +++--- Source/Scene/BillboardCollection.js | 4 +++- Source/Scene/PolylineCollection.js | 10 +++++----- Specs/Core/GeometryPipelineSpec.js | 16 ++++++++-------- 8 files changed, 26 insertions(+), 24 deletions(-) diff --git a/Source/Core/GeometryPipeline.js b/Source/Core/GeometryPipeline.js index b42ffa337e0e..bc055a91b454 100644 --- a/Source/Core/GeometryPipeline.js +++ b/Source/Core/GeometryPipeline.js @@ -510,7 +510,7 @@ define([ // If there's an index list and more than 64K attributes, it is possible that // some indices are outside the range of unsigned short [0, 64K - 1] var numberOfVertices = Geometry.computeNumberOfVertices(geometry); - if (defined(geometry.indices) && (numberOfVertices > CesiumMath.SIXTY_FOUR_KILOBYTES)) { + if (defined(geometry.indices) && (numberOfVertices >= CesiumMath.SIXTY_FOUR_KILOBYTES)) { var oldToNewIndex = []; var newIndices = []; var currentIndex = 0; @@ -541,7 +541,7 @@ define([ newIndices.push(i); } - if (currentIndex + indicesPerPrimitive > CesiumMath.SIXTY_FOUR_KILOBYTES) { + if (currentIndex + indicesPerPrimitive >= CesiumMath.SIXTY_FOUR_KILOBYTES) { geometries.push(new Geometry({ attributes : newAttributes, indices : newIndices, diff --git a/Source/Core/IndexDatatype.js b/Source/Core/IndexDatatype.js index c05ec4ad2de8..adab021521d6 100644 --- a/Source/Core/IndexDatatype.js +++ b/Source/Core/IndexDatatype.js @@ -110,7 +110,7 @@ define([ } //>>includeEnd('debug'); - if (numberOfVertices > CesiumMath.SIXTY_FOUR_KILOBYTES) { + if (numberOfVertices >= CesiumMath.SIXTY_FOUR_KILOBYTES) { return new Uint32Array(indicesLengthOrArray); } @@ -141,7 +141,7 @@ define([ } //>>includeEnd('debug'); - if (numberOfVertices > CesiumMath.SIXTY_FOUR_KILOBYTES) { + if (numberOfVertices >= CesiumMath.SIXTY_FOUR_KILOBYTES) { return new Uint32Array(sourceArray, byteOffset, length); } diff --git a/Source/Core/TerrainProvider.js b/Source/Core/TerrainProvider.js index 46236991f59f..14b086c7de2f 100644 --- a/Source/Core/TerrainProvider.js +++ b/Source/Core/TerrainProvider.js @@ -104,8 +104,8 @@ define([ */ TerrainProvider.getRegularGridIndices = function(width, height) { //>>includeStart('debug', pragmas.debug); - if (width * height > 64 * 1024) { - throw new DeveloperError('The total number of vertices (width * height) must be less than or equal to 65536.'); + if (width * height >= 64 * 1024) { + throw new DeveloperError('The total number of vertices (width * height) must be less than 65536.'); } //>>includeEnd('debug'); diff --git a/Source/Renderer/VertexArray.js b/Source/Renderer/VertexArray.js index 196a15675794..c29bdbf29745 100644 --- a/Source/Renderer/VertexArray.js +++ b/Source/Renderer/VertexArray.js @@ -615,7 +615,7 @@ define([ var indexBuffer; var indices = geometry.indices; if (defined(indices)) { - if ((Geometry.computeNumberOfVertices(geometry) > CesiumMath.SIXTY_FOUR_KILOBYTES) && context.elementIndexUint) { + if ((Geometry.computeNumberOfVertices(geometry) >= CesiumMath.SIXTY_FOUR_KILOBYTES) && context.elementIndexUint) { indexBuffer = Buffer.createIndexBuffer({ context : context, typedArray : new Uint32Array(indices), diff --git a/Source/Renderer/VertexArrayFacade.js b/Source/Renderer/VertexArrayFacade.js index 79597b2516d4..2514ecd5afb1 100644 --- a/Source/Renderer/VertexArrayFacade.js +++ b/Source/Renderer/VertexArrayFacade.js @@ -315,12 +315,12 @@ define([ destroyVA(this); var va = this.va = []; - var numberOfVertexArrays = defined(indexBuffer) ? Math.ceil(this._size / CesiumMath.SIXTY_FOUR_KILOBYTES) : 1; + var numberOfVertexArrays = defined(indexBuffer) ? Math.ceil(this._size / (CesiumMath.SIXTY_FOUR_KILOBYTES - 1)) : 1; for ( var k = 0; k < numberOfVertexArrays; ++k) { var attributes = []; for (i = 0, length = allBuffers.length; i < length; ++i) { buffer = allBuffers[i]; - var offset = k * (buffer.vertexSizeInBytes * CesiumMath.SIXTY_FOUR_KILOBYTES); + var offset = k * (buffer.vertexSizeInBytes * (CesiumMath.SIXTY_FOUR_KILOBYTES - 1)); VertexArrayFacade._appendAttributes(attributes, buffer, offset, this._instanced); } @@ -332,7 +332,7 @@ define([ attributes : attributes, indexBuffer : indexBuffer }), - indicesCount : 1.5 * ((k !== (numberOfVertexArrays - 1)) ? CesiumMath.SIXTY_FOUR_KILOBYTES : (this._size % CesiumMath.SIXTY_FOUR_KILOBYTES)) + indicesCount : 1.5 * ((k !== (numberOfVertexArrays - 1)) ? (CesiumMath.SIXTY_FOUR_KILOBYTES - 1) : (this._size % (CesiumMath.SIXTY_FOUR_KILOBYTES - 1))) // TODO: not hardcode 1.5, this assumes 6 indices per 4 vertices (as for Billboard quads). }); } diff --git a/Source/Scene/BillboardCollection.js b/Source/Scene/BillboardCollection.js index ab6e7e792f42..a65cf5dda731 100644 --- a/Source/Scene/BillboardCollection.js +++ b/Source/Scene/BillboardCollection.js @@ -552,7 +552,9 @@ define([ return indexBuffer; } - var length = sixteenK * 6; + // Subtract 6 because the last index is reserverd for primitive restart. + // https://www.khronos.org/registry/webgl/specs/latest/2.0/#5.18 + var length = sixteenK * 6 - 6; var indices = new Uint16Array(length); for (var i = 0, j = 0; i < length; i += 6, j += 4) { indices[i] = j; diff --git a/Source/Scene/PolylineCollection.js b/Source/Scene/PolylineCollection.js index c8897226c9e5..4ba6a73543d0 100644 --- a/Source/Scene/PolylineCollection.js +++ b/Source/Scene/PolylineCollection.js @@ -787,14 +787,14 @@ define([ vbo += vertexBufferOffset[k]; - var positionHighOffset = 6 * (k * (positionSizeInBytes * CesiumMath.SIXTY_FOUR_KILOBYTES) - vbo * positionSizeInBytes);//componentsPerAttribute(3) * componentDatatype(4) + var positionHighOffset = 6 * (k * (positionSizeInBytes * (CesiumMath.SIXTY_FOUR_KILOBYTES - 1)) - vbo * positionSizeInBytes);//componentsPerAttribute(3) * componentDatatype(4) var positionLowOffset = positionSizeInBytes + positionHighOffset; var prevPositionHighOffset = positionSizeInBytes + positionLowOffset; var prevPositionLowOffset = positionSizeInBytes + prevPositionHighOffset; var nextPositionHighOffset = positionSizeInBytes + prevPositionLowOffset; var nextPositionLowOffset = positionSizeInBytes + nextPositionHighOffset; - var vertexPickColorBufferOffset = k * (pickColorSizeInBytes * CesiumMath.SIXTY_FOUR_KILOBYTES) - vbo * pickColorSizeInBytes; - var vertexTexCoordExpandWidthAndShowBufferOffset = k * (texCoordExpandWidthAndShowSizeInBytes * CesiumMath.SIXTY_FOUR_KILOBYTES) - vbo * texCoordExpandWidthAndShowSizeInBytes; + var vertexPickColorBufferOffset = k * (pickColorSizeInBytes * (CesiumMath.SIXTY_FOUR_KILOBYTES - 1)) - vbo * pickColorSizeInBytes; + var vertexTexCoordExpandWidthAndShowBufferOffset = k * (texCoordExpandWidthAndShowSizeInBytes * (CesiumMath.SIXTY_FOUR_KILOBYTES - 1)) - vbo * texCoordExpandWidthAndShowSizeInBytes; var attributes = [{ index : attributeLocations.position3DHigh, @@ -1323,7 +1323,7 @@ define([ for ( var j = 0; j < numberOfSegments; ++j) { var segmentLength = segments[j] - 1.0; for ( var k = 0; k < segmentLength; ++k) { - if (indicesCount + 4 >= CesiumMath.SIXTY_FOUR_KILOBYTES - 1) { + if (indicesCount + 4 >= CesiumMath.SIXTY_FOUR_KILOBYTES - 2) { polyline._locatorBuckets.push({ locator : bucketLocator, count : segmentIndexCount @@ -1355,7 +1355,7 @@ define([ count : segmentIndexCount }); - if (indicesCount + 4 >= CesiumMath.SIXTY_FOUR_KILOBYTES - 1) { + if (indicesCount + 4 >= CesiumMath.SIXTY_FOUR_KILOBYTES - 2) { vertexBufferOffset.push(0); indices = []; totalIndices.push(indices); diff --git a/Specs/Core/GeometryPipelineSpec.js b/Specs/Core/GeometryPipelineSpec.js index b16bb847eb96..b3ecfc138492 100644 --- a/Specs/Core/GeometryPipelineSpec.js +++ b/Specs/Core/GeometryPipelineSpec.js @@ -492,11 +492,11 @@ defineSuite([ expect(geometries.length).toEqual(2); - expect(geometries[0].attributes.position.values.length).toEqual(positions.length - 6); // Two vertices are not copied (0, 1) - expect(geometries[0].indices.length).toEqual(indices.length - 2); // One line is not copied (0, 1) + expect(geometries[0].attributes.position.values.length).toEqual(positions.length - 12); // Four vertices are not copied + expect(geometries[0].indices.length).toEqual(indices.length - 4); // Two lines are not copied - expect(geometries[1].attributes.position.values.length).toEqual(6); - expect(geometries[1].indices.length).toEqual(2); + expect(geometries[1].attributes.position.values.length).toEqual(9); + expect(geometries[1].indices.length).toEqual(4); }); it('fitToUnsignedShortIndices creates two point geometries', function() { @@ -525,11 +525,11 @@ defineSuite([ expect(geometries.length).toEqual(2); - expect(geometries[0].attributes.position.values.length).toEqual(positions.length - 3); // One vertex is not copied - expect(geometries[0].indices.length).toEqual(indices.length - 1); // One point is not copied + expect(geometries[0].attributes.position.values.length).toEqual(positions.length - 6); // Two vertices are not copied + expect(geometries[0].indices.length).toEqual(indices.length - 2); // Two points are not copied - expect(geometries[1].attributes.position.values.length).toEqual(3); - expect(geometries[1].indices.length).toEqual(1); + expect(geometries[1].attributes.position.values.length).toEqual(6); + expect(geometries[1].indices.length).toEqual(2); }); it('fitToUnsignedShortIndices throws without a geometry', function() {