Fix setPaintProperty to reparse buckets transitioning between flat an…

…d extruded fills (#3468) * Use setPaintProperty's optional worker roundtrip to reparse fill-extrude buckets when necessary (fixes #3386)
mapbox · Oct 27, 2016 · 965a2f3 · 965a2f3
1 parent 52b67c5
commit 965a2f3
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 9 deletions.
diff --git a/js/render/painter.js b/js/render/painter.js
@@ -272,10 +272,7 @@ class Painter {
         this.id = layer.id;
 
         let type = layer.type;
-        if (type === 'fill' &&
-            (!layer.isPaintValueFeatureConstant('fill-extrude-height') ||
-            !layer.isPaintValueZoomConstant('fill-extrude-height') ||
-            layer.getPaintValue('fill-extrude-height', {zoom: this.transform.zoom}) !== 0)) {
+        if (type === 'fill' && layer.isExtruded({zoom: this.transform.zoom})) {
             type = 'extrusion';
         }
 

diff --git a/js/style/style.js b/js/style/style.js
@@ -557,6 +557,7 @@ class Style extends Evented {
         if (util.deepEqual(layer.getPaintProperty(name, klass), value)) return this;
 
         const wasFeatureConstant = layer.isPaintValueFeatureConstant(name);
+        const wasExtruded = layer.type === 'fill' && layer.isExtruded({ zoom: this.zoom });
         layer.setPaintProperty(name, value, klass);
 
         const isFeatureConstant = !(
@@ -566,7 +567,14 @@ class Style extends Evented {
             value.property !== undefined
         );
 
-        if (!isFeatureConstant || !wasFeatureConstant) {
+        const switchBuckets = (
+            layer.type === 'fill' &&
+            name === 'fill-extrude-height' &&
+            (!wasExtruded && !!value) ||
+            (wasExtruded && value === 0)
+        );
+
+        if (!isFeatureConstant || !wasFeatureConstant || switchBuckets) {
             this._updates.layers[layerId] = true;
             if (layer.source) {
                 this._updates.sources[layer.source] = true;

diff --git a/js/style/style_layer/fill_style_layer.js b/js/style/style_layer/fill_style_layer.js
@@ -46,10 +46,14 @@ class FillStyleLayer extends StyleLayer {
         }
     }
 
-    createBucket(options) {
-        if (!this.isPaintValueFeatureConstant('fill-extrude-height') ||
+    isExtruded(globalProperties) {
+        return !this.isPaintValueFeatureConstant('fill-extrude-height') ||
             !this.isPaintValueZoomConstant('fill-extrude-height') ||
-            this.getPaintValue('fill-extrude-height', {zoom: options.zoom}) !== 0) {
+            this.getPaintValue('fill-extrude-height', globalProperties) !== 0;
+    }
+
+    createBucket(options) {
+        if (this.isExtruded({zoom: options.zoom})) {
             return new FillExtrusionBucket(options);
         }
         return new FillBucket(options);

diff --git a/package.json b/package.json
@@ -56,7 +56,7 @@
     "highlight.js": "9.3.0",
     "jsdom": "^9.4.2",
     "lodash.template": "^4.4.0",
-    "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#c71bd415a7f83518890c60c1473398072be2677b",
+    "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#107e22655a346e88d2525bdc8217d342208188c5",
     "minifyify": "^7.0.1",
     "npm-run-all": "^3.0.0",
     "nyc": "^8.3.0",

diff --git a/test/js/style/style.test.js b/test/js/style/style.test.js
@@ -1194,6 +1194,34 @@ test('Style defers expensive methods', (t) => {
     });
 });
 
+test('Style updates source to switch fill[-extrusion] bucket types', (t) => {
+    // Runtime switching between non-extruded and extruded fills requires
+    // switching bucket types, so setPaintProperty in this case should trigger
+    // a worker roundtrip (as also happens when setting property functions)
+
+    const style = new Style(createStyleJSON({
+        "sources": {
+            "streets": createGeoJSONSource(),
+            "terrain": createGeoJSONSource()
+        }
+    }));
+
+    style.on('style.load', () => {
+        style.addLayer({ id: 'fill', type: 'fill', source: 'streets', paint: {}});
+        style.update();
+
+        t.notOk(style._updates.sources.streets);
+
+        style.setPaintProperty('fill', 'fill-color', 'green');
+        t.notOk(style._updates.sources.streets);
+
+        style.setPaintProperty('fill', 'fill-extrude-height', 10);
+        t.ok(style._updates.sources.streets);
+
+        t.end();
+    });
+});
+
 test('Style#query*Features', (t) => {
 
     // These tests only cover filter validation. Most tests for these methods