Skip to content

Commit

Permalink
Avoid calling loadGeometry twice for every feature
Browse files Browse the repository at this point in the history
  • Loading branch information
@jfirebaugh
jfirebaugh committed Sep 2, 2017
1 parent a70411b commit 80d9396
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 51 deletions.
10 changes: 6 additions & 4 deletions src/data/bucket/circle_bucket.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type {Bucket, IndexedFeature, PopulateParameters, SerializedBucket} from
import type {ProgramInterface} from '../program_configuration';
import type StyleLayer from '../../style/style_layer';
import type {StructArray} from '../../util/struct_array';
import type Point from '@mapbox/point-geometry';

const circleInterface = {
layoutAttributes: [
Expand Down Expand Up @@ -79,8 +80,9 @@ class CircleBucket implements Bucket {
populate(features: Array<IndexedFeature>, options: PopulateParameters) {
for (const {feature, index, sourceLayerIndex} of features) {
if (this.layers[0].filter(feature)) {
this.addFeature(feature);
options.featureIndex.insert(feature, index, sourceLayerIndex, this.index);
const geometry = loadGeometry(feature);
this.addFeature(feature, geometry);
options.featureIndex.insert(feature, geometry, index, sourceLayerIndex, this.index);
}
}
}
Expand Down Expand Up @@ -114,8 +116,8 @@ class CircleBucket implements Bucket {
this.segments.destroy();
}

addFeature(feature: VectorTileFeature) {
for (const ring of loadGeometry(feature)) {
addFeature(feature: VectorTileFeature, geometry: Array<Array<Point>>) {
for (const ring of geometry) {
for (const point of ring) {
const x = point.x;
const y = point.y;
Expand Down
10 changes: 6 additions & 4 deletions src/data/bucket/fill_bucket.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type {Bucket, IndexedFeature, PopulateParameters, SerializedBucket} from
import type {ProgramInterface} from '../program_configuration';
import type StyleLayer from '../../style/style_layer';
import type {StructArray} from '../../util/struct_array';
import type Point from '@mapbox/point-geometry';

const fillInterface = {
layoutAttributes: [
Expand Down Expand Up @@ -72,8 +73,9 @@ class FillBucket implements Bucket {
populate(features: Array<IndexedFeature>, options: PopulateParameters) {
for (const {feature, index, sourceLayerIndex} of features) {
if (this.layers[0].filter(feature)) {
this.addFeature(feature);
options.featureIndex.insert(feature, index, sourceLayerIndex, this.index);
const geometry = loadGeometry(feature);
this.addFeature(feature, geometry);
options.featureIndex.insert(feature, geometry, index, sourceLayerIndex, this.index);
}
}
}
Expand Down Expand Up @@ -112,8 +114,8 @@ class FillBucket implements Bucket {
this.segments2.destroy();
}

addFeature(feature: VectorTileFeature) {
for (const polygon of classifyRings(loadGeometry(feature), EARCUT_MAX_RINGS)) {
addFeature(feature: VectorTileFeature, geometry: Array<Array<Point>>) {
for (const polygon of classifyRings(geometry, EARCUT_MAX_RINGS)) {
let numVertices = 0;
for (const ring of polygon) {
numVertices += ring.length;
Expand Down
10 changes: 6 additions & 4 deletions src/data/bucket/fill_extrusion_bucket.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type {Bucket, IndexedFeature, PopulateParameters, SerializedBucket} from
import type {ProgramInterface} from '../program_configuration';
import type StyleLayer from '../../style/style_layer';
import type {StructArray} from '../../util/struct_array';
import type Point from '@mapbox/point-geometry';

const fillExtrusionInterface = {
layoutAttributes: [
Expand Down Expand Up @@ -85,8 +86,9 @@ class FillExtrusionBucket implements Bucket {
populate(features: Array<IndexedFeature>, options: PopulateParameters) {
for (const {feature, index, sourceLayerIndex} of features) {
if (this.layers[0].filter(feature)) {
this.addFeature(feature);
options.featureIndex.insert(feature, index, sourceLayerIndex, this.index);
const geometry = loadGeometry(feature);
this.addFeature(feature, geometry);
options.featureIndex.insert(feature, geometry, index, sourceLayerIndex, this.index);
}
}
}
Expand Down Expand Up @@ -120,8 +122,8 @@ class FillExtrusionBucket implements Bucket {
this.segments.destroy();
}

addFeature(feature: VectorTileFeature) {
for (const polygon of classifyRings(loadGeometry(feature), EARCUT_MAX_RINGS)) {
addFeature(feature: VectorTileFeature, geometry: Array<Array<Point>>) {
for (const polygon of classifyRings(geometry, EARCUT_MAX_RINGS)) {
let numVertices = 0;
for (const ring of polygon) {
numVertices += ring.length;
Expand Down
9 changes: 5 additions & 4 deletions src/data/bucket/line_bucket.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,9 @@ class LineBucket implements Bucket {
populate(features: Array<IndexedFeature>, options: PopulateParameters) {
for (const {feature, index, sourceLayerIndex} of features) {
if (this.layers[0].filter(feature)) {
this.addFeature(feature);
options.featureIndex.insert(feature, index, sourceLayerIndex, this.index);
const geometry = loadGeometry(feature);
this.addFeature(feature, geometry);
options.featureIndex.insert(feature, geometry, index, sourceLayerIndex, this.index);
}
}
}
Expand Down Expand Up @@ -165,14 +166,14 @@ class LineBucket implements Bucket {
this.segments.destroy();
}

addFeature(feature: VectorTileFeature) {
addFeature(feature: VectorTileFeature, geometry: Array<Array<Point>>) {
const layout = this.layers[0].layout;
const join = this.layers[0].getLayoutValue('line-join', {zoom: this.zoom}, feature);
const cap = layout['line-cap'];
const miterLimit = layout['line-miter-limit'];
const roundLimit = layout['line-round-limit'];

for (const line of loadGeometry(feature)) {
for (const line of geometry) {
this.addLine(line, feature, join, cap, miterLimit, roundLimit);
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/data/feature_index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,9 @@ class FeatureIndex {
this.featureIndexArray = featureIndexArray || new FeatureIndexArray();
}

insert(feature: VectorTileFeature, featureIndex: number, sourceLayerIndex: number, bucketIndex: number) {
insert(feature: VectorTileFeature, geometry: Array<Array<Point>>, featureIndex: number, sourceLayerIndex: number, bucketIndex: number) {
const key = this.featureIndexArray.length;
this.featureIndexArray.emplaceBack(featureIndex, sourceLayerIndex, bucketIndex);
const geometry = loadGeometry(feature);

for (let r = 0; r < geometry.length; r++) {
const ring = geometry[r];
Expand Down
29 changes: 9 additions & 20 deletions test/unit/data/fill_bucket.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,10 @@ const StyleLayer = require('../../../src/style/style_layer');
const vt = new VectorTile(new Protobuf(fs.readFileSync(path.join(__dirname, '/../../fixtures/mbsv5-6-18-23.vector.pbf'))));
const feature = vt.layers.water.feature(0);

function createFeature(points) {
return {
properties: {
'foo': 1
},
loadGeometry: function() {
return points;
}
};
}

function createPolygon(numPoints) {
const points = [];
for (let i = 0; i < numPoints; i++) {
points.push(new Point(i / numPoints, i / numPoints));
points.push(new Point(2048 + 256 * Math.cos(i / numPoints * 2 * Math.PI, 2048 + 256 * Math.sin(i / numPoints * 2 * Math.PI))));
}
return points;
}
Expand All @@ -37,18 +26,18 @@ test('FillBucket', (t) => {
const layer = new StyleLayer({ id: 'test', type: 'fill', layout: {} });
const bucket = new FillBucket({ layers: [layer] });

bucket.addFeature(createFeature([[
bucket.addFeature({}, [[
new Point(0, 0),
new Point(10, 10)
]]));
]]);

bucket.addFeature(createFeature([[
bucket.addFeature({}, [[
new Point(0, 0),
new Point(10, 10),
new Point(10, 20)
]]));
]]);

bucket.addFeature(feature);
bucket.addFeature(feature, feature.loadGeometry());

t.end();
});
Expand Down Expand Up @@ -76,13 +65,13 @@ test('FillBucket segmentation', (t) => {

// first add an initial, small feature to make sure the next one starts at
// a non-zero offset
bucket.addFeature(createFeature([createPolygon(10)]));
bucket.addFeature({}, [createPolygon(10)]);

// add a feature that will break across the group boundary
bucket.addFeature(createFeature([
bucket.addFeature({}, [
createPolygon(128),
createPolygon(128)
]));
]);

// Each polygon must fit entirely within a segment, so we expect the
// first segment to include the first feature and the first polygon
Expand Down
14 changes: 3 additions & 11 deletions test/unit/data/line_bucket.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,6 @@ const StyleLayer = require('../../../src/style/style_layer');
const vt = new VectorTile(new Protobuf(fs.readFileSync(path.join(__dirname, '/../../fixtures/mbsv5-6-18-23.vector.pbf'))));
const feature = vt.layers.road.feature(0);

function createFeature(points) {
return {
loadGeometry: function() {
return points;
}
};
}

function createLine(numPoints) {
const points = [];
for (let i = 0; i < numPoints; i++) {
Expand Down Expand Up @@ -100,7 +92,7 @@ test('LineBucket', (t) => {
new Point(0, 0)
], polygon);

bucket.addFeature(feature);
bucket.addFeature(feature, feature.loadGeometry());

t.end();
});
Expand All @@ -115,10 +107,10 @@ test('LineBucket segmentation', (t) => {

// first add an initial, small feature to make sure the next one starts at
// a non-zero offset
bucket.addFeature(createFeature([createLine(10)]));
bucket.addFeature({}, [createLine(10)]);

// add a feature that will break across the group boundary
bucket.addFeature(createFeature([createLine(128)]));
bucket.addFeature({}, [createLine(128)]);

// Each polygon must fit entirely within a segment, so we expect the
// first segment to include the first feature and the first polygon
Expand Down
4 changes: 2 additions & 2 deletions test/unit/data/load_geometry.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ test('loadGeometry', (t) => {

test('loadGeometry extent error', (t) => {
const feature = vt.layers.road.feature(0);
feature.extent = 2048;
feature.extent = 1024;

let numWarnings = 0;

Expand All @@ -33,7 +33,7 @@ test('loadGeometry extent error', (t) => {
}
};

loadGeometry(feature, 15);
loadGeometry(feature);

t.equal(numWarnings, 1);

Expand Down

0 comments on commit 80d9396

Please sign in to comment.