Skip to content

Commit

Permalink
Changed the interface for Tipsify.tipsify and Tipsify.calculateACMR.
Browse files Browse the repository at this point in the history
  • Loading branch information
@kristiancalhoun
kristiancalhoun committed May 21, 2012
1 parent 6e6932a commit 5eeb49e
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 62 deletions.
4 changes: 3 additions & 1 deletion Source/Core/MeshFilters.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,9 @@ define([
maximumIndex = indices[j];
}
}
indexLists[i].values = Tipsify.tipsify(indices, maximumIndex, cacheCapacity || 24);
indexLists[i].values = Tipsify.tipsify({indices : indices,
maximumIndex : maximumIndex,
cacheSize : cacheCapacity});
}
}
}
Expand Down
49 changes: 32 additions & 17 deletions Source/Core/Tipsify.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ define(['./DeveloperError'], function(DeveloperError) {
/**
* Calculates the average cache miss ratio (ACMR) for a given set of indices.
*
* @param {Array} indices Lists triads of numbers corresponding to the indices of the vertices
* @param {Array} args.indices Lists triads of numbers corresponding to the indices of the vertices
* in the vertex buffer that define the mesh's triangles.
* @param {Number} maximumIndex The maximum value of the elements in <code>indices</code>.
* @param {Number} cacheSize The number of vertices that can be stored in the cache at any one time.
* @param {Number} [args.maximumIndex] The maximum value of the elements in <code>args.indices</code>.
* If not supplied, this value will be computed.
* @param {Number} [args.cacheSize=24] The number of vertices that can be stored in the cache at any one time.
*
* @exception {DeveloperError} indices is required.
* @exception {DeveloperError} indices length must be a multiple of three.
* @exception {DeveloperError} maximumIndex must be greater than zero.
* @exception {DeveloperError} cacheSize must be greater than two.
*
* @return {Number} The average cache miss ratio (ACMR).
Expand All @@ -37,7 +37,11 @@ define(['./DeveloperError'], function(DeveloperError) {
* var cacheSize = 3;
* var acmr = Tipsify.calculateACMR(indices, maxIndex, cacheSize);
*/
Tipsify.calculateACMR = function(indices, maximumIndex, cacheSize) {
Tipsify.calculateACMR = function(args) {
var indices = args.indices;
var maximumIndex = args.maximumIndex;
var cacheSize = args.cacheSize || 24;

if (!indices) {
throw new DeveloperError("indices is required.", "indices");
}
Expand All @@ -47,13 +51,23 @@ define(['./DeveloperError'], function(DeveloperError) {
if ((numIndices < 3) || (numIndices % 3 !== 0)) {
throw new DeveloperError("indices length must be a multiple of three.", "indices");
}
if (maximumIndex <= 0) {
throw new DeveloperError("maximumIndex must be greater than zero.", "maximumIndex");
}
if (cacheSize < 3) {
throw new DeveloperError("cacheSize must be greater than two.", "cachSize");
}

// Compute the maximumIndex if not given
if(!maximumIndex || maximumIndex <= 0) {

This comment has been minimized.

Sign in to view

Copy link
@pjcozzi

pjcozzi May 22, 2012

Contributor
  • This logic and the related logic in Tipsify.tipsify can still be cleaner, right? The caller either provides maximumIndex (and we use it) or they don't (and we compute it). Providing -1 shouldn't be valid. That was a C++ convention that we do not need or want. Just check typeof args.maximumIndex === 'undefined'.
  • What if args is not defined. You don't need to explicitly throw, but you can do args = args || {} and let it throw on the missing properties.
  • We only use very common abbreviations in public interfaces, e.g., funciton and argument names. args is not common enough. I've been calling arguments like this template or description, but as long as the name makes sense in the context of the function it is OK.
var currentIndex = 0;
var intoIndices = indices[currentIndex];
while (currentIndex < numIndices) {
if (intoIndices > maximumIndex) {
maximumIndex = intoIndices;
}
++currentIndex;
intoIndices = indices[currentIndex];
}
}

// Vertex time stamps
var vertexTimeStamps = [];
for ( var i = 0; i < maximumIndex + 1; i++) {
Expand All @@ -75,14 +89,14 @@ define(['./DeveloperError'], function(DeveloperError) {
/**
* Optimizes triangles for the post-vertex shader cache.
*
* @param {Array} indices Lists triads of numbers corresponding to the indices of the vertices
* @param {Array} args.indices Lists triads of numbers corresponding to the indices of the vertices
* in the vertex buffer that define the mesh's triangles.
* @param {Number} maximumIndex The maximum value of the elements in <code>indices</code>.
* @param {Number} cacheSize The number of vertices that can be stored in the cache at any one time.
* @param {Number} [args.maximumIndex] The maximum value of the elements in <code>args.indices</code>.
* If not supplied, this value will be computed.
* @param {Number} [args.cacheSize=24] The number of vertices that can be stored in the cache at any one time.
*
* @exception {DeveloperError} indices is required.
* @exception {DeveloperError} indices length must be a multiple of three.
* @exception {DeveloperError} maximumIndex must be greater than zero.
* @exception {DeveloperError} cacheSize must be greater than two.
*
* @return {Array} A list of the input indices in an optimized order.
Expand All @@ -93,7 +107,11 @@ define(['./DeveloperError'], function(DeveloperError) {
* var cacheSize = 3;
* var reorderedIndices = Tipsify.tipsify(indices, maxIndex, cacheSize);
*/
Tipsify.tipsify = function(indices, maximumIndex, cacheSize) {
Tipsify.tipsify = function(args) {
var indices = args.indices;
var maximumIndex = args.maximumIndex;
var cacheSize = args.cacheSize || 24;

var cursor;

function skipDeadEnd(vertices, deadEnd, indices, maximumIndexPlusOne) {
Expand Down Expand Up @@ -150,9 +168,6 @@ define(['./DeveloperError'], function(DeveloperError) {
if ((numIndices < 3) || (numIndices % 3 !== 0)) {
throw new DeveloperError("indices length must be a multiple of three.", "indices");
}
if ((maximumIndex <= 0) && (maximumIndex !== -1)) {
throw new DeveloperError("maximumIndex must be greater than zero.", "maximumIndex");
}
if (cacheSize < 3) {
throw new DeveloperError("cacheSize must be greater than two.", "cachSize");
}
Expand All @@ -162,7 +177,7 @@ define(['./DeveloperError'], function(DeveloperError) {
var currentIndex = 0;
var intoIndices = indices[currentIndex];
var endIndex = numIndices;
if (maximumIndex !== -1) {
if (maximumIndex > 0) {
maximumIndexPlusOne = maximumIndex + 1;
} else {
while (currentIndex < endIndex) {
Expand Down
8 changes: 6 additions & 2 deletions Specs/Core/MeshFiltersSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,11 +239,15 @@ defineSuite([
maximumIndex = indices[i];
}
}
var ACMRbefore = Tipsify.calculateACMR(indices, maximumIndex, 24);
var ACMRbefore = Tipsify.calculateACMR({indices : indices,
maximumIndex : maximumIndex,
cacheSize : 24});
expect(ACMRbefore).toBeGreaterThan(1.00);
mesh = MeshFilters.reorderForPostVertexCache(mesh);
indices = mesh.indexLists[0].values;
var ACMRafter = Tipsify.calculateACMR(indices, maximumIndex, 24);
var ACMRafter = Tipsify.calculateACMR({indices : indices,
maximumIndex : maximumIndex,
cacheSize : 24});
expect(ACMRafter).toBeLessThan(0.70);
});

Expand Down
82 changes: 40 additions & 42 deletions Specs/Core/TipsifySpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,69 +8,60 @@ defineSuite([

it("can calculate the ACMR", function() {
//Hexagon formed from 6 triangles, 7 vertices
var indices = [0, 1, 2, 0, 2, 3, 0, 3, 4, 0, 4, 5, 0, 5, 6, 0, 1, 6];
var maxIndex = 6;
var cacheSize = 3;
expect(Tipsify.calculateACMR(indices, maxIndex, cacheSize)).toEqual(2);
expect(Tipsify.calculateACMR({indices : [0, 1, 2, 0, 2, 3, 0, 3, 4, 0, 4, 5, 0, 5, 6, 0, 1, 6],
maximumIndex : 6,
cacheSize : 3})).toEqual(2);
});

it("throws when calculating ACMR (1 of 4)", function() {
it("throws when calculating ACMR (1 of 3)", function() {
expect(function() {
var indices = null;
Tipsify.calculateACMR(indices, 1, 3);
Tipsify.calculateACMR({indices : null,
maximumIndex : 1,
cacheSize : 3});
}).toThrow();
});

it("throws when calculating ACMR (2 of 4)", function() {
it("throws when calculating ACMR (2 of 3)", function() {
expect(function() {
var indices = [1, 2, 3, 4];
Tipsify.calculateACMR(indices, 1, 3);
Tipsify.calculateACMR({indices : [1, 2, 3, 4],
maximumIndex : 1,
cacheSize : 3});
}).toThrow();
});

it("throws when calculating ACMR (3 of 4)", function() {
it("throws when calculating ACMR (3 of 3)", function() {
expect(function() {
var indices = [0, 1, 2];
Tipsify.calculateACMR(indices, 0, 3);
Tipsify.calculateACMR({indices : [0, 1, 2],
maximumIndex : 2,
cacheSize : 2});
}).toThrow();
});

it("throws when calculating ACMR (4 of 4)", function() {
it("throws when executing Tipsify (1 of 4)", function() {
expect(function() {
var indices = [0, 1, 2];
Tipsify.calculateACMR(indices, 2, 2);
Tipsify.tipsify({indices : null,
maximumIndex : 1,
cacheSize : 3});
}).toThrow();
});

it("throws when executing Tipsify (1 of 5)", function() {
it("throws when executing Tipsify (2 of 4)", function() {
expect(function() {
var indices = [];
Tipsify.tipsify(indices, 1, 3);
}).toThrow();
});

it("throws when executing Tipsify (2 of 5)", function() {
expect(function() {
var indices = [1, 2, 3, 4];
Tipsify.tipsify(indices, 1, 3);
}).toThrow();
});

it("throws when executing Tipsify (3 of 5)", function() {
expect(function() {
var indices = [0, 1, 2];
Tipsify.tipsify(indices, 0, 3);
Tipsify.tipsify({indices : [1, 2, 3, 4],
maximumIndex : 1,
cacheSize : 3});
}).toThrow();
});

it("throws when executing Tipsify (4 of 5)", function() {
expect(function() {
var indices = [0, 1, 2];
Tipsify.tipsify(indices, 2, 2);
Tipsify.tipsify({indices : [0, 1, 2],
maximumIndex : 2,
cacheSize : 2});
}).toThrow();
});

it("throws when executing Tipsify (5 of 5)", function() {
it("throws when executing Tipsify (4 of 4)", function() {
expect(function() {
Tipsify.tipsify(null);
}).toThrow();
Expand All @@ -80,18 +71,25 @@ defineSuite([
var indices = [0, 1, 7, 1, 7, 8, 1, 2, 8, 2, 8, 9, 2, 3, 9, 3, 9, 10, 3, 4, 10, 4, 10, 11, 4, 5, 11, 5, 11, 12, 6, 13, 14, 6, 7, 14, 7, 14, 15, 7, 8, 15, 8, 15, 16, 8, 9, 16, 9, 16, 17, 9,
10, 17, 10, 17, 18, 10, 11, 18, 11, 18, 19, 11, 12, 19, 12, 19, 20, 13, 21, 22, 13, 14, 22, 14, 22, 23, 14, 15, 23, 15, 23, 24, 15, 16, 24, 16, 24, 25, 16, 17, 25, 17, 25, 26, 17, 18,
26, 18, 26, 27, 18, 19, 27, 19, 27, 28, 19, 20, 28];
var maximumIndex = 28;
var acmrBefore = Tipsify.calculateACMR(indices, maximumIndex, 6);
var result = Tipsify.tipsify(indices, maximumIndex, 6);
var acmrAfter = Tipsify.calculateACMR(result, maximumIndex, 6);
var acmrBefore = Tipsify.calculateACMR({indices : indices,
maximumIndex : 28,
cacheSize : 6});
var result = Tipsify.tipsify({indices : indices,
maximumIndex : 28,
cacheSize : 6});
var acmrAfter = Tipsify.calculateACMR({indices : result,
maximumIndex : 28,
cacheSize : 6});
expect(acmrAfter).toBeLessThan(acmrBefore);
});

it("can Tipsify without knowing the maximum index", function() {
var indices = [0, 1, 7, 1, 7, 8, 1, 2, 8, 2, 8, 9, 2, 3, 9, 3, 9, 10, 3, 4, 10, 4, 10, 11, 4, 5, 11, 5, 11, 12, 6, 13, 14, 6, 7, 14, 7, 14, 15, 7, 8, 15, 8, 15, 16, 8, 9, 16, 9, 16, 17, 9,
10, 17, 10, 17, 18, 10, 11, 18, 11, 18, 19, 11, 12, 19, 12, 19, 20, 13, 21, 22, 13, 14, 22, 14, 22, 23, 14, 15, 23, 15, 23, 24, 15, 16, 24, 16, 24, 25, 16, 17, 25, 17, 25, 26, 17, 18,
26, 18, 26, 27, 18, 19, 27, 19, 27, 28, 19, 20, 28];
var maximumIndex = 28;
expect(Tipsify.tipsify(indices, -1, 6)).toEqual(Tipsify.tipsify(indices, maximumIndex, 6));
expect(Tipsify.tipsify({indices : indices,
cacheSize : 6})).toEqual(Tipsify.tipsify({indices : indices,
maximumIndex : 28,
cacheSize: 6}));
});
});

0 comments on commit 5eeb49e

Please sign in to comment.