Eliminate Bucket#featureIndex

mapbox · Oct 25, 2016 · e03a7b1 · e03a7b1
1 parent db828d2
commit e03a7b1
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 43 deletions.
diff --git a/js/data/bucket.js b/js/data/bucket.js
@@ -34,7 +34,6 @@ class Bucket {
         this.childLayers = options.childLayers;
 
         this.index = options.index;
-        this.featureIndex = options.featureIndex;
         this.programConfigurations = util.mapObject(this.programInterfaces, (programInterface) => {
             const result = {};
             for (const layer of options.childLayers) {
@@ -59,14 +58,14 @@ class Bucket {
         }
     }
 
-    populate(features) {
+    populate(features, options) {
         this.createArrays();
         this.recalculateStyleLayers();
 
         for (const feature of features) {
             if (this.layer.filter(feature)) {
                 this.addFeature(feature);
-                this.featureIndex.insert(feature, this.index);
+                options.featureIndex.insert(feature, this.index);
             }
         }
 

diff --git a/js/data/bucket/symbol_bucket.js b/js/data/bucket/symbol_bucket.js
@@ -95,8 +95,6 @@ class SymbolBucket extends Bucket {
     constructor(options) {
         super(options);
 
-        this.showCollisionBoxes = options.showCollisionBoxes;
-        this.overscaling = options.overscaling;
         this.collisionBoxArray = options.collisionBoxArray;
         this.symbolQuadsArray = options.symbolQuadsArray;
         this.symbolInstancesArray = options.symbolInstancesArray;
@@ -135,7 +133,7 @@ class SymbolBucket extends Bucket {
                 placementZoom * 10);
     }
 
-    populate(features, dependencies) {
+    populate(features, options) {
         this.recalculateStyleLayers();
 
         const layout = this.layer.layout;
@@ -152,8 +150,8 @@ class SymbolBucket extends Bucket {
             return;
         }
 
-        const icons = dependencies.icons;
-        const stacks = dependencies.stacks;
+        const icons = options.iconDependencies;
+        const stacks = options.glyphDependencies;
         const stack = stacks[textFont] = stacks[textFont] || {};
 
         for (const feature of features) {

diff --git a/js/source/worker_tile.js b/js/source/worker_tile.js
@@ -44,9 +44,11 @@ class WorkerTile {
         const buckets = {};
         let bucketIndex = 0;
 
-        let icons = {};
-        let stacks = {};
-        const dependencies = {icons, stacks};
+        const options = {
+            featureIndex: featureIndex,
+            iconDependencies: {},
+            glyphDependencies: {}
+        };
 
         const layerFamilies = layerIndex.familiesBySource[this.source];
         for (const sourceLayerId in layerFamilies) {
@@ -87,14 +89,12 @@ class WorkerTile {
                     childLayers: family,
                     zoom: this.zoom,
                     overscaling: this.overscaling,
-                    showCollisionBoxes: this.showCollisionBoxes,
                     collisionBoxArray: this.collisionBoxArray,
                     symbolQuadsArray: this.symbolQuadsArray,
-                    symbolInstancesArray: this.symbolInstancesArray,
-                    featureIndex: featureIndex
+                    symbolInstancesArray: this.symbolInstancesArray
                 });
 
-                bucket.populate(features, dependencies);
+                bucket.populate(features, options);
                 featureIndex.bucketLayerIDs[bucket.index] = family.map(getLayerId);
             }
         }
@@ -141,6 +141,8 @@ class WorkerTile {
         }
 
         let deps = 0;
+        let icons = Object.keys(options.iconDependencies);
+        let stacks = util.mapObject(options.glyphDependencies, (glyphs) => Object.keys(glyphs).map(Number));
 
         const gotDependency = (err) => {
             if (err) return callback(err);
@@ -154,10 +156,6 @@ class WorkerTile {
             }
         };
 
-        for (const fontName in stacks) {
-            stacks[fontName] = Object.keys(stacks[fontName]).map(Number);
-        }
-
         if (Object.keys(stacks).length) {
             actor.send('getGlyphs', {uid: this.uid, stacks: stacks}, (err, newStacks) => {
                 stacks = newStacks;
@@ -167,8 +165,6 @@ class WorkerTile {
             gotDependency();
         }
 
-        icons = Object.keys(icons);
-
         if (icons.length) {
             actor.send('getIcons', {icons: icons}, (err, newIcons) => {
                 icons = newIcons;

diff --git a/test/js/data/bucket.test.js b/test/js/data/bucket.test.js
@@ -82,16 +82,18 @@ test('Bucket', (t) => {
 
         return new Class({
             layer: layers[0],
-            childLayers: layers,
-            buffers: {},
-            featureIndex: new FeatureIndex(new TileCoord(0, 0, 0), 0, null)
+            childLayers: layers
         });
     }
 
+    function createOptions() {
+        return {featureIndex: new FeatureIndex(new TileCoord(0, 0, 0), 0, null)};
+    }
+
     t.test('add features', (t) => {
         const bucket = create();
 
-        bucket.populate([createFeature(17, 42)]);
+        bucket.populate([createFeature(17, 42)], createOptions());
 
         const testVertex = bucket.arrayGroups.test[0].layoutVertexArray;
         t.equal(testVertex.length, 1);
@@ -125,7 +127,7 @@ test('Bucket', (t) => {
             { id: 'two', type: 'circle', paint: dataDrivenPaint }
         ]});
 
-        bucket.populate([createFeature(17, 42)]);
+        bucket.populate([createFeature(17, 42)], createOptions());
 
         const v0 = bucket.arrayGroups.test[0].layoutVertexArray.get(0);
         const a0 = bucket.arrayGroups.test[0].paintVertexArrays.one.get(0);
@@ -152,7 +154,7 @@ test('Bucket', (t) => {
             ]
         });
 
-        bucket.populate([createFeature(17, 42)]);
+        bucket.populate([createFeature(17, 42)], createOptions());
 
         t.equal(bucket.arrayGroups.test[0].layoutVertexArray.bytesPerElement, 0);
         t.deepEqual(
@@ -172,7 +174,7 @@ test('Bucket', (t) => {
             }]
         });
 
-        bucket.populate([createFeature(17, 42)]);
+        bucket.populate([createFeature(17, 42)], createOptions());
 
         const v0 = bucket.arrayGroups.test[0].layoutVertexArray.get(0);
         t.equal(v0.a_map, 34);
@@ -183,7 +185,7 @@ test('Bucket', (t) => {
     t.test('reset buffers', (t) => {
         const bucket = create();
 
-        bucket.populate([createFeature(17, 42)]);
+        bucket.populate([createFeature(17, 42)], createOptions());
 
         t.equal(bucket.arrayGroups.test.length, 1);
         bucket.createArrays();
@@ -214,15 +216,15 @@ test('Bucket', (t) => {
         bucket.createArrays();
         t.ok(bucket.isEmpty());
 
-        bucket.populate([createFeature(17, 42)]);
+        bucket.populate([createFeature(17, 42)], createOptions());
         t.ok(!bucket.isEmpty());
 
         t.end();
     });
 
     t.test('getTransferables', (t) => {
         const bucket = create();
-        bucket.populate([createFeature(17, 42)]);
+        bucket.populate([createFeature(17, 42)], createOptions());
 
         const transferables = [];
         bucket.getTransferables(transferables);
@@ -239,9 +241,9 @@ test('Bucket', (t) => {
     t.test('add features after resetting buffers', (t) => {
         const bucket = create();
 
-        bucket.populate([createFeature(1, 5)]);
+        bucket.populate([createFeature(1, 5)], createOptions());
         bucket.createArrays();
-        bucket.populate([createFeature(17, 42)]);
+        bucket.populate([createFeature(17, 42)], createOptions());
 
         const testVertex = bucket.arrayGroups.test[0].layoutVertexArray;
         t.equal(testVertex.length, 1);
@@ -278,7 +280,7 @@ test('Bucket', (t) => {
     t.test('add features', (t) => {
         const bucket = create();
 
-        bucket.populate([createFeature(17, 42)]);
+        bucket.populate([createFeature(17, 42)], createOptions());
 
         const testVertex = bucket.arrayGroups.test[0].layoutVertexArray;
         t.equal(testVertex.length, 1);

diff --git a/test/js/data/symbol_bucket.test.js b/test/js/data/symbol_bucket.test.js
@@ -21,7 +21,6 @@ const feature = vt.layers.place_label.feature(10);
 const glyphs = JSON.parse(fs.readFileSync(path.join(__dirname, '/../../fixtures/fontstack-glyphs.json')));
 
 /*eslint new-cap: 0*/
-const buffers = {};
 const collisionBoxArray = new CollisionBoxArray();
 const symbolQuadsArray = new SymbolQuadsArray();
 const symbolInstancesArray = new SymbolInstancesArray();
@@ -43,26 +42,24 @@ function bucketSetup() {
     });
 
     return new SymbolBucket({
-        buffers: buffers,
         overscaling: 1,
         zoom: 0,
         collisionBoxArray: collisionBoxArray,
         symbolInstancesArray: symbolInstancesArray,
         symbolQuadsArray: symbolQuadsArray,
         layer: layer,
-        childLayers: [layer],
-        tileExtent: 4096
+        childLayers: [layer]
     });
 }
 
 test('SymbolBucket', (t) => {
     const bucketA = bucketSetup();
     const bucketB = bucketSetup();
-    const dependencies = {icons: {}, stacks: {}};
+    const options = {iconDependencies: {}, glyphDependencies: {}};
 
     // add feature from bucket A
     const a = collision.grid.keys.length;
-    bucketA.populate([feature], dependencies);
+    bucketA.populate([feature], options);
     bucketA.prepare(stacks, {});
     bucketA.place(collision);
 
@@ -71,7 +68,7 @@ test('SymbolBucket', (t) => {
 
     // add same feature from bucket B
     const a2 = collision.grid.keys.length;
-    bucketB.populate([feature], dependencies);
+    bucketB.populate([feature], options);
     bucketB.prepare(stacks, {});
     bucketB.place(collision);
     const b2 = collision.grid.keys.length;
@@ -85,9 +82,9 @@ test('SymbolBucket integer overflow', (t) => {
     t.stub(SymbolBucket, 'MAX_QUADS', 5);
 
     const bucket = bucketSetup();
-    const dependencies = {icons: {}, stacks: {}};
+    const options = {iconDependencies: {}, glyphDependencies: {}};
 
-    bucket.populate([feature], dependencies);
+    bucket.populate([feature], options);
     bucket.prepare(stacks, {});
     bucket.place(collision);