Skip to content

Commit

Permalink
Drop smallest inner rings for earcut performance (#2622)
Browse files Browse the repository at this point in the history
* Add unit tests for existing classifyRings functionality

* Add maxRings argument to classifyRings

* Winding order fix, set a max ring threshold to 100 (for now)

* Satisfy the eslint beast

* Bump EARCUT_MAX_RINGS to 500

* Use quickselect instead of sort in classifyRings

* Refactoring

* Respond to PR review

Thanks to @yhahn, @jakepruitt and @mourner for all the help on this effort!
  • Loading branch information
yhahn authored and lucaswoj committed May 26, 2016
1 parent 8a7f936 commit 3d39142
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 6 deletions.
3 changes: 2 additions & 1 deletion js/data/bucket/fill_bucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ var util = require('../../util/util');
var loadGeometry = require('../load_geometry');
var earcut = require('earcut');
var classifyRings = require('../../util/classify_rings');
var EARCUT_MAX_RINGS = 500;

module.exports = FillBucket;

Expand Down Expand Up @@ -32,7 +33,7 @@ FillBucket.prototype.programInterfaces = {

FillBucket.prototype.addFeature = function(feature) {
var lines = loadGeometry(feature);
var polygons = classifyRings(lines);
var polygons = classifyRings(lines, EARCUT_MAX_RINGS);
for (var i = 0; i < polygons.length; i++) {
this.addPolygon(polygons[i]);
}
Expand Down
25 changes: 20 additions & 5 deletions js/util/classify_rings.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
'use strict';

module.exports = classifyRings;
var quickselect = require('quickselect');

// classifies an array of rings into polygons with outer rings and holes

function classifyRings(rings) {
module.exports = function classifyRings(rings, maxRings) {
var len = rings.length;

if (len <= 1) return [rings];
Expand All @@ -14,9 +13,11 @@ function classifyRings(rings) {
ccw;

for (var i = 0; i < len; i++) {
var area = signedArea(rings[i]);
var area = calculateSignedArea(rings[i]);
if (area === 0) continue;

rings[i].area = Math.abs(area);

if (ccw === undefined) ccw = area < 0;

if (ccw === area < 0) {
Expand All @@ -29,10 +30,24 @@ function classifyRings(rings) {
}
if (polygon) polygons.push(polygon);

// Earcut performance degrages with the # of rings in a polygon. For this
// reason, we limit strip out all but the `maxRings` largest rings.
if (maxRings > 1) {
for (var j = 0; j < polygons.length; j++) {
if (polygons[j].length <= maxRings) continue;
quickselect(polygons[j], maxRings, 1, polygon.length - 1, compareAreas);
polygons[j] = polygon.slice(0, maxRings);
}
}

return polygons;
};

function compareAreas(a, b) {
return b.area - a.area;
}

function signedArea(ring) {
function calculateSignedArea(ring) {
var sum = 0;
for (var i = 0, len = ring.length, j = len - 1, p1, p2; i < len; j = i++) {
p1 = ring[i];
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"pbf": "^1.3.2",
"pngjs": "^2.2.0",
"point-geometry": "^0.0.0",
"quickselect": "^1.0.0",
"request": "^2.39.0",
"resolve-url": "^0.2.1",
"shelf-pack": "^1.0.0",
Expand Down
151 changes: 151 additions & 0 deletions test/js/util/classify_rings.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
'use strict';

var test = require('tap').test;
var fs = require('fs');
var path = require('path');
var Protobuf = require('pbf');
var VectorTile = require('vector-tile').VectorTile;
var classifyRings = require('../../../js/util/classify_rings');

// Load a fill feature from fixture tile.
var vt = new VectorTile(new Protobuf(new Uint8Array(fs.readFileSync(path.join(__dirname, '/../../fixtures/mbsv5-6-18-23.vector.pbf')))));
var feature = vt.layers.water.feature(0);

test('classifyRings', function(assert) {
var geometry;
var classified;

geometry = [
[
{x:0, y:0},
{x:0, y:40},
{x:40, y:40},
{x:40, y:0},
{x:0, y:0}
]
];
classified = classifyRings(geometry);
assert.equal(classified.length, 1, '1 polygon');
assert.equal(classified[0].length, 1, 'polygon 1 has 1 exterior');

geometry = [
[
{x:0, y:0},
{x:0, y:40},
{x:40, y:40},
{x:40, y:0},
{x:0, y:0}
],
[
{x:60, y:0},
{x:60, y:40},
{x:100, y:40},
{x:100, y:0},
{x:60, y:0}
]
];
classified = classifyRings(geometry);
assert.equal(classified.length, 2, '2 polygons');
assert.equal(classified[0].length, 1, 'polygon 1 has 1 exterior');
assert.equal(classified[1].length, 1, 'polygon 2 has 1 exterior');

geometry = [
[
{x:0, y:0},
{x:0, y:40},
{x:40, y:40},
{x:40, y:0},
{x:0, y:0}
],
[
{x:10, y:10},
{x:20, y:10},
{x:20, y:20},
{x:10, y:10}
]
];
classified = classifyRings(geometry);
assert.equal(classified.length, 1, '1 polygon');
assert.equal(classified[0].length, 2, 'polygon 1 has 1 exterior, 1 interior');

geometry = feature.loadGeometry();
classified = classifyRings(geometry);
assert.equal(classified.length, 2, '2 polygons');
assert.equal(classified[0].length, 1, 'polygon 1 has 1 exterior');
assert.equal(classified[1].length, 10, 'polygon 2 has 1 exterior, 9 interior');

assert.end();
});

test('classifyRings + maxRings', function(t) {

function createGeometry(options) {
var geometry = [
// Outer ring, area = 3200
[ {x:0, y:0}, {x:0, y:40}, {x:40, y:40}, {x:40, y:0}, {x:0, y:0} ],
// Inner ring, area = 100
[ {x:30, y:30}, {x:32, y:30}, {x:32, y:32}, {x:30, y:30} ],
// Inner ring, area = 4
[ {x:10, y:10}, {x:20, y:10}, {x:20, y:20}, {x:10, y:10} ]
];
if (options && options.reverse) {
geometry[0].reverse();
geometry[1].reverse();
geometry[2].reverse();
}
return geometry;
}


t.test('maxRings=undefined', function(t) {
var geometry = sortRings(classifyRings(createGeometry()));
t.equal(geometry.length, 1);
t.equal(geometry[0].length, 3);
t.equal(geometry[0][0].area, 3200);
t.equal(geometry[0][1].area, 100);
t.equal(geometry[0][2].area, 4);
t.end();
});

t.test('maxRings=2', function(t) {
var geometry = sortRings(classifyRings(createGeometry(), 2));
t.equal(geometry.length, 1);
t.equal(geometry[0].length, 2);
t.equal(geometry[0][0].area, 3200);
t.equal(geometry[0][1].area, 100);
t.end();
});

t.test('maxRings=2, reversed geometry', function(t) {
var geometry = sortRings(classifyRings(createGeometry({reverse: true}), 2));
t.equal(geometry.length, 1);
t.equal(geometry[0].length, 2);
t.equal(geometry[0][0].area, 3200);
t.equal(geometry[0][1].area, 100);
t.end();
});

t.test('maxRings=5, geometry from fixture', function(t) {
var geometry = sortRings(classifyRings(feature.loadGeometry(), 5));
t.equal(geometry.length, 2);
t.equal(geometry[0].length, 1);
t.equal(geometry[1].length, 5);

var areas = geometry[1].map(function(ring) { return ring.area; });
t.deepEqual(areas, [2763951, 21600, 8298, 4758, 3411]);
t.end();
});

t.end();
});

function sortRings(geometry) {
for (var i = 0; i < geometry.length; i++) {
geometry[i] = geometry[i].sort(compareAreas);
}
return geometry;
}

function compareAreas(a, b) {
return b.area - a.area;
}

0 comments on commit 3d39142

Please sign in to comment.