From fdc96d5b3ba685fc8e139b198a75e14af43365e7 Mon Sep 17 00:00:00 2001 From: Molly Lloyd Date: Tue, 14 Aug 2018 16:01:36 -0700 Subject: [PATCH] don't clear state on reload, just on setData (#7129) --- src/source/source_cache.js | 12 ++- src/style-spec/types.js | 1 + .../set-paint-property/expected.png | Bin 0 -> 348 bytes .../set-paint-property/style.json | 71 ++++++++++++++++++ test/unit/ui/map.test.js | 32 ++++++++ 5 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 test/integration/render-tests/feature-state/set-paint-property/expected.png create mode 100644 test/integration/render-tests/feature-state/set-paint-property/style.json diff --git a/src/source/source_cache.js b/src/source/source_cache.js index 17e8926de53..c421ec25553 100644 --- a/src/source/source_cache.js +++ b/src/source/source_cache.js @@ -75,6 +75,7 @@ class SourceCache extends Evented { // for sources with mutable data, this event fires when the underlying data // to a source is changed. (i.e. GeoJSONSource#setData and ImageSource#serCoordinates) if (this._sourceLoaded && !this._paused && e.dataType === "source" && e.sourceDataType === 'content') { + if (this._source.type === "geojson") this.resetState(); this.reload(); if (this.transform) { this.update(this.transform); @@ -217,9 +218,6 @@ class SourceCache extends Evented { } this._cache.reset(); - //Reset the SourceCache when the source has been reloaded. For GeoJSON sources, - // the `id` of features is not guaranteed to be identical when updated - this._state = new SourceFeatureState(); for (const i in this._tiles) { if (this._tiles[i].state !== "errored") this._reloadTile(i, 'reloading'); @@ -833,6 +831,14 @@ class SourceCache extends Evented { sourceLayer = sourceLayer || '_geojsonTileLayer'; return this._state.getState(sourceLayer, feature); } + + /** + * Reset the SourceFeatureState when the source has been reloaded. For GeoJSON sources, + * the `id` of features is not guaranteed to be identical when updated + */ + resetState() { + this._state = new SourceFeatureState(); + } } SourceCache.maxOverzooming = 10; diff --git a/src/style-spec/types.js b/src/style-spec/types.js index a65db4ff90f..79f2a51383d 100644 --- a/src/style-spec/types.js +++ b/src/style-spec/types.js @@ -408,3 +408,4 @@ export type LayerSpecification = | RasterLayerSpecification | HillshadeLayerSpecification | BackgroundLayerSpecification; + diff --git a/test/integration/render-tests/feature-state/set-paint-property/expected.png b/test/integration/render-tests/feature-state/set-paint-property/expected.png new file mode 100644 index 0000000000000000000000000000000000000000..382248405d009da75a2bec1bc2324a7081a92779 GIT binary patch literal 348 zcmeAS@N?(olHy`uVBq!ia0vp^4j|0I1|(Ny7TyC=Or9=|Ar*{or0Om)FffXOcyAAG zER+hAU^`HIKCAITnvGwl%w&TldJR@D4(c6rmNhvzA%JP+WbO>d$H5Ci8U_9=ceuZ< z0}g&T&GI+cc!14&y`F{Hy0mL&GLpJ&+47|~X3LyeGV|aOVZVsV`Nr3J(~r(d688Vm zelXYVPlk>5m8;f?1-lv5kM433j$Tu9Co5@nKzR21y6VtHRoW`WKBxB1&cc8d ztG6F}QKo#Yt2X!z|Nr{W;`=|`{XOU7=8tP4j2Cz6ev{afdt_aq!Q^McJ~|r?URurF h*Si|-osJG8{p^*-TuF6b`GCR2;OXk;vd$@?2>?LdoPhuU literal 0 HcmV?d00001 diff --git a/test/integration/render-tests/feature-state/set-paint-property/style.json b/test/integration/render-tests/feature-state/set-paint-property/style.json new file mode 100644 index 00000000000..c1ec1921b56 --- /dev/null +++ b/test/integration/render-tests/feature-state/set-paint-property/style.json @@ -0,0 +1,71 @@ +{ + "version": 8, + "metadata": { + "test": { + "width": 64, + "height": 64, + "operations": [ + [ + "setFeatureState", + { + "source": "geojson", + "id": "1" + }, + { + "color": "red" + } + ], + [ + "wait" + ], + [ + "setPaintProperty", + "circle", + "circle-radius", + 10 + ], + [ + "wait" + ] + ] + } + }, + "zoom": 2, + "sources": { + "geojson": { + "type": "geojson", + "data": { + "type": "Feature", + "id": "1", + "properties": { + "radius": 5 + }, + "geometry": { + "type": "Point", + "coordinates": [ + 0, + 0 + ] + } + } + } + }, + "layers": [ + { + "id": "circle", + "type": "circle", + "source": "geojson", + "paint": { + "circle-radius": ["get", "radius"], + "circle-color": [ + "coalesce", + [ + "feature-state", + "color" + ], + "black" + ] + } + } + ] +} diff --git a/test/unit/ui/map.test.js b/test/unit/ui/map.test.js index f708374943b..8211d09cfb9 100755 --- a/test/unit/ui/map.test.js +++ b/test/unit/ui/map.test.js @@ -1268,6 +1268,38 @@ test('Map', (t) => { t.end(); }); }); + + t.test('resets state if source data is updated', (t) => { + const map = createMap(t, { + style: { + "version": 8, + "sources": { + "geojson": createStyleSource() + }, + "layers": [] + } + }); + map.on('load', () => { + map.setFeatureState({ source: 'geojson', id: '12345'}, {'hover': true}); + const fState = map.getFeatureState({ source: 'geojson', id: '12345'}); + t.equal(fState.hover, true); + + map.on('data', () => { + if (map.isSourceLoaded('geojson')) { + const cleanState = map.getFeatureState({ source: 'geojson', id: '12345'}); + t.notOk(cleanState.hover); + t.end(); + } + }); + + map.getSource('geojson') + .setData({ + type: "FeatureCollection", + features: [] + }); + }); + }); + t.test('throw before loaded', (t) => { const map = createMap(t, { style: {