Retain correct layer order throughout the layout process

mapbox · Oct 18, 2016 · 317c921 · 317c921
1 parent bcb9f12
commit 317c921
Show file tree

Hide file tree

Showing 11 changed files with 201 additions and 218 deletions.
diff --git a/js/source/geojson_worker_source.js b/js/source/geojson_worker_source.js
@@ -17,15 +17,15 @@ module.exports = GeoJSONWorkerSource;
  * This class is designed to be easily reused to support custom source types
  * for data formats that can be parsed/converted into an in-memory GeoJSON
  * representation.  To do so, create it with
- * `new GeoJSONWorkerSource(actor, styleLayers, customLoadGeoJSONFunction)`.  For a full example, see [mapbox-gl-topojson](https://github.com/developmentseed/mapbox-gl-topojson).
+ * `new GeoJSONWorkerSource(actor, layerIndex, customLoadGeoJSONFunction)`.  For a full example, see [mapbox-gl-topojson](https://github.com/developmentseed/mapbox-gl-topojson).
  *
  * @class GeoJSONWorkerSource
  * @private
  * @param {Function} [loadGeoJSON] Optional method for custom loading/parsing of GeoJSON based on parameters passed from the main-thread Source.  See {@link GeoJSONWorkerSource#loadGeoJSON}.
  */
-function GeoJSONWorkerSource (actor, styleLayers, loadGeoJSON) {
+function GeoJSONWorkerSource (actor, layerIndex, loadGeoJSON) {
     if (loadGeoJSON) { this.loadGeoJSON = loadGeoJSON; }
-    VectorTileWorkerSource.call(this, actor, styleLayers);
+    VectorTileWorkerSource.call(this, actor, layerIndex);
 }
 
 GeoJSONWorkerSource.prototype = util.inherit(VectorTileWorkerSource, /** @lends GeoJSONWorkerSource.prototype */ {

diff --git a/js/source/source.js b/js/source/source.js
@@ -122,9 +122,7 @@ exports.setType = function (name, type) {
  *
  * @class WorkerSource
  * @param {Actor} actor
- * @param {object} styleLayers An accessor provided by the Worker to get the current style layers and layer families.
- * @param {Function} styleLayers.getLayers
- * @param {Function} styleLayers.getLayerFamilies
+ * @param {StyleLayerIndex} layerIndex
  */
 
 /**

diff --git a/js/source/vector_tile_worker_source.js b/js/source/vector_tile_worker_source.js
@@ -18,9 +18,9 @@ module.exports = VectorTileWorkerSource;
  * @private
  * @param {Function} [loadVectorData] Optional method for custom loading of a VectorTile object based on parameters passed from the main-thread Source.  See {@link VectorTileWorkerSource#loadTile}.  The default implementation simply loads the pbf at `params.url`.
  */
-function VectorTileWorkerSource (actor, styleLayers, loadVectorData) {
+function VectorTileWorkerSource (actor, layerIndex, loadVectorData) {
     this.actor = actor;
-    this.styleLayers = styleLayers;
+    this.layerIndex = layerIndex;
 
     if (loadVectorData) { this.loadVectorData = loadVectorData; }
 
@@ -59,7 +59,7 @@ VectorTileWorkerSource.prototype = {
             if (!vectorTile) return callback(null, null);
 
             workerTile.vectorTile = vectorTile;
-            workerTile.parse(vectorTile, this.styleLayers.getLayerFamilies(), this.actor, (err, result, transferrables) => {
+            workerTile.parse(vectorTile, this.layerIndex.families, this.actor, (err, result, transferrables) => {
                 if (err) return callback(err);
 
                 // Not transferring rawTileData because the worker needs to retain its copy.
@@ -85,7 +85,7 @@ VectorTileWorkerSource.prototype = {
             uid = params.uid;
         if (loaded && loaded[uid]) {
             const workerTile = loaded[uid];
-            workerTile.parse(workerTile.vectorTile, this.styleLayers.getLayerFamilies(), this.actor, callback);
+            workerTile.parse(workerTile.vectorTile, this.layerIndex.families, this.actor, callback);
         }
     },
 

diff --git a/js/source/worker.js b/js/source/worker.js
@@ -1,12 +1,11 @@
 'use strict';
 
 const Actor = require('../util/actor');
-const StyleLayer = require('../style/style_layer');
+const StyleLayerIndex = require('../style/style_layer_index');
 const util = require('../util/util');
 
 const VectorTileWorkerSource = require('./vector_tile_worker_source');
 const GeoJSONWorkerSource = require('./geojson_worker_source');
-const featureFilter = require('feature-filter');
 const assert = require('assert');
 
 module.exports = function createWorker(self) {
@@ -17,8 +16,7 @@ function Worker(self) {
     this.self = self;
     this.actor = new Actor(self, this);
 
-    this.layers = {};
-    this.layerFamilies = {};
+    this.layerIndexes = {};
 
     this.workerSourceTypes = {
         vector: VectorTileWorkerSource,
@@ -38,38 +36,11 @@ function Worker(self) {
 
 util.extend(Worker.prototype, {
     'set layers': function(mapId, layerDefinitions) {
-        this.layers[mapId] = {};
-        this['update layers'](mapId, layerDefinitions);
+        this.getLayerIndex(mapId).replace(layerDefinitions);
     },
 
     'update layers': function(mapId, layerDefinitions) {
-        const layers = this.layers[mapId];
-
-        // Update ref parents
-        for (const layer of layerDefinitions) {
-            if (!layer.ref) updateLayer(layer);
-        }
-
-        // Update ref children
-        for (const layer of layerDefinitions) {
-            if (layer.ref) updateLayer(layer);
-        }
-
-        function updateLayer(layer) {
-            if (layer.type !== 'fill' && layer.type !== 'line' && layer.type !== 'circle' && layer.type !== 'symbol')
-                return;
-            const refLayer = layer.ref && layers[layer.ref];
-            let styleLayer = layers[layer.id];
-            if (styleLayer) {
-                styleLayer.set(layer, refLayer);
-            } else {
-                styleLayer = layers[layer.id] = StyleLayer.create(layer, refLayer);
-            }
-            styleLayer.updatePaintTransitions({}, {transition: false});
-            styleLayer.filter = featureFilter(styleLayer.filter);
-        }
-
-        this.layerFamilies[mapId] = createLayerFamilies(this.layers[mapId]);
+        this.getLayerIndex(mapId).update(layerDefinitions);
     },
 
     'load tile': function(mapId, params, callback) {
@@ -112,16 +83,18 @@ util.extend(Worker.prototype, {
         }
     },
 
+    getLayerIndex: function(mapId) {
+        let layerIndexes = this.layerIndexes[mapId];
+        if (!layerIndexes) {
+            layerIndexes = this.layerIndexes[mapId] = new StyleLayerIndex();
+        }
+        return layerIndexes;
+    },
+
     getWorkerSource: function(mapId, type) {
         if (!this.workerSources[mapId])
             this.workerSources[mapId] = {};
         if (!this.workerSources[mapId][type]) {
-            // simple accessor object for passing to WorkerSources
-            const layers = {
-                getLayers: () => this.layers[mapId],
-                getLayerFamilies: () => this.layerFamilies[mapId]
-            };
-
             // use a wrapped actor so that we can attach a target mapId param
             // to any messages invoked by the WorkerSource
             const actor = {
@@ -130,30 +103,9 @@ util.extend(Worker.prototype, {
                 }
             };
 
-            this.workerSources[mapId][type] = new this.workerSourceTypes[type](actor, layers);
+            this.workerSources[mapId][type] = new this.workerSourceTypes[type](actor, this.getLayerIndex(mapId));
         }
 
         return this.workerSources[mapId][type];
     }
 });
-
-function createLayerFamilies(layers) {
-    const families = {};
-
-    for (const layerId in layers) {
-        const layer = layers[layerId];
-        const parentLayerId = layer.ref || layer.id;
-        const parentLayer = layers[parentLayerId];
-
-        if (parentLayer.layout && parentLayer.layout.visibility === 'none') continue;
-
-        families[parentLayerId] = families[parentLayerId] || [];
-        if (layerId === parentLayerId) {
-            families[parentLayerId].unshift(layer);
-        } else {
-            families[parentLayerId].push(layer);
-        }
-    }
-
-    return families;
-}
diff --git a/js/source/worker_tile.js b/js/source/worker_tile.js
@@ -47,8 +47,8 @@ WorkerTile.prototype.parse = function(data, layerFamilies, actor, callback) {
 
     // Map non-ref layers to buckets.
     let bucketIndex = 0;
-    for (const layerId in layerFamilies) {
-        const layer = layerFamilies[layerId][0];
+    for (const family of layerFamilies) {
+        const layer = family[0];
         const sourceLayerId = layer.sourceLayer || '_geojsonTileLayer';
 
         assert(!layer.ref);
@@ -62,7 +62,7 @@ WorkerTile.prototype.parse = function(data, layerFamilies, actor, callback) {
         const bucket = Bucket.create({
             layer: layer,
             index: bucketIndex++,
-            childLayers: layerFamilies[layerId],
+            childLayers: family,
             zoom: this.zoom,
             overscaling: this.overscaling,
             showCollisionBoxes: this.showCollisionBoxes,

diff --git a/js/style/style_layer_index.js b/js/style/style_layer_index.js
@@ -0,0 +1,78 @@
+'use strict';
+
+const StyleLayer = require('./style_layer');
+const featureFilter = require('feature-filter');
+
+class StyleLayerIndex {
+    constructor(layers) {
+        this.families = [];
+        if (layers) {
+            this.replace(layers);
+        }
+    }
+
+    replace(layers) {
+        this._layers = {};
+        this._order = [];
+        this.update(layers);
+    }
+
+    _updateLayer(layer) {
+        const refLayer = layer.ref && this._layers[layer.ref];
+
+        let styleLayer = this._layers[layer.id];
+        if (styleLayer) {
+            styleLayer.set(layer, refLayer);
+        } else {
+            styleLayer = this._layers[layer.id] = StyleLayer.create(layer, refLayer);
+        }
+
+        styleLayer.updatePaintTransitions({}, {transition: false});
+        styleLayer.filter = featureFilter(styleLayer.filter);
+    }
+
+    update(layers) {
+        for (const layer of layers) {
+            if (!this._layers[layer.id]) {
+                this._order.push(layer.id);
+            }
+        }
+
+        // Update ref parents
+        for (const layer of layers) {
+            if (!layer.ref) this._updateLayer(layer);
+        }
+
+        // Update ref children
+        for (const layer of layers) {
+            if (layer.ref) this._updateLayer(layer);
+        }
+
+        this.families = [];
+        const byParent = {};
+
+        for (const id of this._order) {
+            const layer = this._layers[id];
+            const parent = layer.ref ? this._layers[layer.ref] : layer;
+
+            if (parent.layout && parent.layout.visibility === 'none') {
+                continue;
+            }
+
+            let family = byParent[parent.id];
+            if (!family) {
+                family = [];
+                this.families.push(family);
+                byParent[parent.id] = family;
+            }
+
+            if (layer.ref) {
+                family.push(layer);
+            } else {
+                family.unshift(layer);
+            }
+        }
+    }
+}
+
+module.exports = StyleLayerIndex;
diff --git a/package.json b/package.json
@@ -60,7 +60,7 @@
     "jsdom": "^9.4.2",
     "json-loader": "^0.5.4",
     "lodash": "^4.13.1",
-    "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#8db35f9130bce27102c5867d6542c42c074d9bfc",
+    "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#119551708b3621fb1e7066347730b1ea449ff8d6",
     "memory-fs": "^0.3.0",
     "minifyify": "^7.0.1",
     "npm-run-all": "^3.0.0",

diff --git a/test/js/source/vector_tile_worker_source.test.js b/test/js/source/vector_tile_worker_source.test.js
@@ -2,15 +2,11 @@
 
 const test = require('mapbox-gl-js-test').test;
 const VectorTileWorkerSource = require('../../../js/source/vector_tile_worker_source');
-
-const styleLayers = {
-    getLayers: function () {},
-    getLayerFamilies: function () {}
-};
+const StyleLayerIndex = require('../../../js/style/style_layer_index');
 
 test('abortTile', (t) => {
     t.test('aborts pending request', (t) => {
-        const source = new VectorTileWorkerSource(null, styleLayers);
+        const source = new VectorTileWorkerSource(null, new StyleLayerIndex());
 
         source.loadTile({
             source: 'source',
@@ -32,7 +28,7 @@ test('abortTile', (t) => {
 
 test('removeTile', (t) => {
     t.test('removes loaded tile', (t) => {
-        const source = new VectorTileWorkerSource(null, styleLayers);
+        const source = new VectorTileWorkerSource(null, new StyleLayerIndex());
 
         source.loaded = {
             source: {
@@ -55,7 +51,7 @@ test('removeTile', (t) => {
 test('redoPlacement', (t) => {
 
     t.test('on loaded tile', (t) => {
-        const source = new VectorTileWorkerSource(null, styleLayers);
+        const source = new VectorTileWorkerSource(null, new StyleLayerIndex());
         const tile = {
             redoPlacement: function(angle, pitch, showCollisionBoxes) {
                 t.equal(angle, 60);
@@ -84,7 +80,7 @@ test('redoPlacement', (t) => {
     });
 
     t.test('on loading tile', (t) => {
-        const source = new VectorTileWorkerSource(null, styleLayers);
+        const source = new VectorTileWorkerSource(null, new StyleLayerIndex());
         const tile = {};
         source.loading = {mapbox: {3: tile}};