Remove feature state (very maybe possibly ready) #7696

peterqliu · 2018-12-12T21:40:12Z

retry of #7669

Addresses #6889

Depending on parameters specified, enables feature state removal at the source, feature, and property levels.

Effect on setFeatureState perf:

cc/ @asheemmamoowala

briefly describe the changes in this PR
write tests for all new functionality
document any changes to public APIs
post benchmark scores
manually test the debug page

src/ui/map.js

ChrisLoer · 2018-12-12T23:01:19Z

test/unit/ui/map.test.js

@@ -1518,30 +1728,6 @@ test('Map', (t) => {
        });
    });

-    t.test('no render after idle event', (t) => {


These tests also look accidentally removed.

This is just a nit now, but it looks like these two tests changed location in the file. Let's put them back where they started so it's easy to tell in the diff that nothing's changed.

…mapbox-gl-js into removeFeatureState-redux

src/source/source_state.js

ryanhamley · 2018-12-17T21:19:42Z

bench/benchmarks/remove_paint_state_feature.js

@@ -0,0 +1,76 @@
+


The three benchmark tests here look to be almost identical with the exception of what's passed to setFeatureState and removeFeatureState in bench. You could probably create an overarching remove_paint_state benchmark that has sub-tests for each individual test, in the same vein as the Expressions and Layers benchmarks.

@ryanhamley was looking to do just that! thanks for the tip, was able to crib the format from Layers. consolidated the benches here d048d22

…ecific keys

asheemmamoowala

This is nearly there! 🛩
Commit history could use some cleanup to ensure a correct and clean merge back to master.

src/source/source_state.js

test/unit/ui/map.test.js

src/ui/map.js

debug/highlightpoints.html

src/ui/map.js

src/source/source_state.js

ChrisLoer

I've left a few small comments, but overall this is looking good to me. 🚀

formatting

…practice-branch

lint squash

peterqliu · 2019-01-10T21:54:11Z

got a bit carried away with squashing-- moving things over to #7761

peterqliu added 7 commits December 12, 2018 13:20

add removeFeatureState

8faa720

add unit tests

0f3c151

add benchmark tests

e9893d4

add bench files and expected .json

1fe712f

update moving state

d85f082

add render test

971e9fe

add expected pngs

6486be6

ChrisLoer reviewed Dec 12, 2018

View reviewed changes

src/ui/map.js Outdated Show resolved Hide resolved

ChrisLoer reviewed Dec 12, 2018

View reviewed changes

src/ui/map.js Outdated Show resolved Hide resolved

restore map.js changes since last branch

c0e5b75

ChrisLoer reviewed Dec 12, 2018

View reviewed changes

put back the thing i wasn't supposed to delete

7684cec

peterqliu requested a review from asheemmamoowala December 12, 2018 23:54

peterqliu mentioned this pull request Dec 13, 2018

removeFeatureState() (not ready) #7669

Closed

6 tasks

peterqliu added 6 commits December 14, 2018 15:20

add removeFeatureState

70c3aa5

add unit tests

b2304de

add benchmark tests

96dab41

add bench files and expected .json

8e719be

add render test

c3678f1

add expected pngs

d944552

asheemmamoowala force-pushed the removeFeatureState-redux branch from 7684cec to d944552 Compare December 14, 2018 23:20

peterqliu added 3 commits December 14, 2018 15:59

Merge branch 'removeFeatureState-redux' of https://github.com/mapbox/…

6e846d8

…mapbox-gl-js into removeFeatureState-redux

simply handling of whole-feature state deletions

558e664

update tests to reflect deletion instead of null

c1130f6

peterqliu commented Dec 17, 2018

View reviewed changes

src/source/source_state.js Outdated Show resolved Hide resolved

ryanhamley reviewed Dec 17, 2018

View reviewed changes

peterqliu added 4 commits December 17, 2018 16:37

getState returns undefined's instead of nulls

e853a13

consolidate benches into subtests of one ubertest

f83bb88

add consolidated bench

d048d22

update query test for deletion

d989f29

peterqliu added 5 commits December 20, 2018 12:08

test for consecutive general-specific state deletions

821c6f4

test for consecutive general-specific state deletions

5a30abb

check for presence of whole-feature deletion before iterating over sp…

62fbe7f

…ecific keys

simplify feature-level deletion

91da949

lint

3f86807

asheemmamoowala suggested changes Dec 21, 2018

View reviewed changes

src/source/source_state.js Show resolved Hide resolved

src/source/source_state.js Outdated Show resolved Hide resolved

src/source/source_state.js Show resolved Hide resolved

src/source/source_state.js Outdated Show resolved Hide resolved

test/unit/ui/map.test.js Show resolved Hide resolved

peterqliu added 5 commits January 2, 2019 13:55

formatting

fbe6c3c

use nulls for source-level state deletion

421f846

update tests and demo

0f19f49

lint

7581f4b

squashing commits

b40cb44

peterqliu force-pushed the removeFeatureState-redux branch from 7581f4b to b40cb44 Compare January 4, 2019 22:22

ChrisLoer reviewed Jan 7, 2019

View reviewed changes

src/ui/map.js Outdated Show resolved Hide resolved

ChrisLoer reviewed Jan 7, 2019

View reviewed changes

debug/highlightpoints.html Show resolved Hide resolved