Skip to content

Commit

Permalink
Do not use the index of the max value of an unsigned short. Fixing te…
Browse files Browse the repository at this point in the history 
…sts WIP.
  • Loading branch information
@bagnell
bagnell committed Oct 14, 2015
1 parent 0f6a5b9 commit 9100963
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 24 deletions.
4 changes: 2 additions & 2 deletions Source/Core/GeometryPipeline.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions Source/Core/IndexDatatype.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ define([
}
//>>includeEnd('debug');

if (numberOfVertices > CesiumMath.SIXTY_FOUR_KILOBYTES) {
if (numberOfVertices >= CesiumMath.SIXTY_FOUR_KILOBYTES) {
return new Uint32Array(indicesLengthOrArray);
}

Expand Down Expand Up @@ -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);
}

Expand Down
4 changes: 2 additions & 2 deletions Source/Core/TerrainProvider.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down
2 changes: 1 addition & 1 deletion Source/Renderer/VertexArray.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
6 changes: 3 additions & 3 deletions Source/Renderer/VertexArrayFacade.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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).
});
}
Expand Down
4 changes: 3 additions & 1 deletion Source/Scene/BillboardCollection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 5 additions & 5 deletions Source/Scene/PolylineCollection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
16 changes: 8 additions & 8 deletions Specs/Core/GeometryPipelineSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down

0 comments on commit 9100963

Please sign in to comment.