Optimize "Map#setFilter" performance for "hover effect" use case

[lucaswoj]

Creating performant hover effects in GL JS is hard. One solution to this problem is making our primitives (Map#queryRenderedFeatures, Map#setFilter, GeoJSONSource#setData, ...) faster. Another solution is adding a primitive optimized for the hover usecase.

Suppose we allowed filtering features by id within the shaders. With a buffer size increase equivalent to that of a single data-driven styling property, we could have zero-overhead filtering on even the largest data sets.

A single filter id could be passed to the shader as a uniform, with some common boilerplate encapsulated within #pragma.

We could use this optimized filtering pathway automatically when a filter in the form ['==', 'id', *] is used or provide a new filterId style property.

cc @jfirebaugh @mourner @peterqliu @tmcw @tristen

[lucaswoj]

This is a specific proposal for #200

[jfirebaugh]

What would the shader do based on the uniform value? Is the idea that it would implement style syntax similar to #200 (comment)?

[lucaswoj]

The shader would not draw a feature unless the feature's id (i.e. feature.properties.id) matches the feature id filter (ie. filter: ['==', 'id', 5] or featureIdFilter: 5)

The shader implementation would look roughly like this:

if (u_feature_id_filter === a_feature_id) {
    gl_FragColor = vec4(0.0);
}

This could be a stepping stone to the syntax proposed in #200 or a standalone feature. The style syntax in #200 is more ergonomic for our users and offers less room for a fast implementation.

[jfirebaugh]

Got it. So at least at first it's purely an optimization for existing functionality -- map.setFilter(layerID, ['==', 'id', n]) would be special-cased to re-render with the new uniform value, rather than reparsing tiles.

Looking at our current hover example, it wouldn't be able to benefit from this optimization, because it uses the "name" property rather than id. It also appears to be fast enough as currently implemented.

Have we seen a lot of other cases where this pathway would be used?

[anandthakker]

@lucaswoj It's possible that the with some small changes, the properties-only-update primitives over in #2812 could also be used for a (definitely less good, but possibly more expedient?) temporary solution to the hover problem: you could have a style layer like paint: { color: { property: '__mapbox_gl_hovered_feature', type: 'categorical', stops: [ [ true, 'blue' ], [ false, 'red' ] ] } }, and then with a bit of hackery, populate __mapbox_gl_hovered_feature property on mouse move.

Maybe this is just as much work as the approach you're proposing here, I dunno. Just mentioning it because I was considering this hack just this morning

[lucaswoj]

Looking at our current hover example, it wouldn't be able to benefit from this optimization, because it uses the "name" property rather than id.

Right. We'd have to modify the example to assign ids to features and filter based on id instead of name.

It also appears to be fast enough as currently implemented.

On on a state-of-the-art Macbook Pro and a data set containing 50 features, the queryRenderedFeatures -> setFilter workflow is fast enough.

Hover implementations that must work with larger data sets use the queryRenderedFeatures -> setData workflow which, although faster, breaks features across tile boundaries. See, for example, the map in https://www.mapbox.com/blog/data-driven-fill-color/

[jfirebaugh]

👍

We should implement mapbox/mapbox-gl-style-spec#391 before or alongside this, and use the "official" Vector Tile / GeoJSON id field rather than a property that happens to be named "id".

[peterqliu]

tagging on to @lucaswoj 's comment, the existing setFilter workflow slows considerably with more features in the viewport. Faster mouse interactivity for hovering/dragging/clicking in GL is definitely desirable, if only to achieve perf parity with the CSS-dependent Leaflet.

[jfirebaugh]

@peterqliu Looks like most of the time in that example is spent in queryRenderedFeatures, and my understanding of this proposal is that it wouldn't eliminate the need for queryRenderedFeatures.

[anandthakker]

Looks like most of the time in that example is spent in queryRenderedFeatures, and my understanding of this proposal is that it wouldn't eliminate the need for queryRenderedFeatures.

@jfirebaugh I was surprised to hear this, but it looks like it's because the raw pbf data is getting continually reparsed. Reason: the style update triggers a source update => reload tiles => the tiles' feature indexes lose their cached Vector Tile. This ticket's proposal notwithstanding, it does seem hover styling could get a pretty good improvement in the meantime if that cycle could be eliminated.

[anandthakker]

@peterqliu @jfirebaugh Confirmed: here's a version of that example with one (admittedly odd) change: add a second source parcel_source_hover with the same url as parcel_source, and use that for the parcel-hover layer. It's noticeably smoother! https://jsfiddle.net/7meh7d50/2/

update: my apologies -- I also forgot that the speedup included a tweak I made locally to gl-js itself to filter the sources queried by Style#queryRenderedFeatures to the ones relevant to the specified params.layers. Opening a separate issue about that.

[andrewharvey]

https://jsfiddle.net/7meh7d50/

@anandthakker Did you save this code, looks the same as the one @peterqliu linked to. However if I make your changes as described, I notice a performance improvement although it's still too slow as the hover lags behind the cursor with a noticeable trail (something not seen with the setData approach to hover).

[anandthakker]

Ah, sorry about that. Here's the saved one: https://jsfiddle.net/7meh7d50/2/ (also updated in my comment above)

[mourner]

Yeah, it's a bit faster but still not quite there if you zoom out.

I like the shader filter-by-id idea coupled with mapbox/mapbox-gl-style-spec#391 (to use feature-level ids rather than a property). Let's try it out.

[anandthakker]

Argh -- much more importantly, I also forgot that the speedup included a tweak I made locally to gl-js itself, not just the example: in style.js, queryRenderedFeatures queries every source, even if a layers parameter is specified

[lucaswoj]

+1 on using feature-level ids rather than a property id. I wasn’t aware of the difference.

[ekelleyv]

On on a state-of-the-art Macbook Pro and a data set containing 50 features, the queryRenderedFeatures -> setFilter workflow is fast enough.

I can't tell if this is a more recent regression (I think it is), but this does not seem to be the case anymore. When using the state hovering example, I get a ton of lag behind the pointer (top of the line macbook, modern chrome). I recorded a video (which is not totally a fair representation as the act of recording slows things down further):
https://cl.ly/0x3R1H373Y0b

We think the behavior got worse sometime since 0.21.0, but honestly cannot say for sure. Let me know if this is worth tracking in a separate issue. That being said, a more performant way to do hover effects would be hugely beneficial for our use cases.

[mourner]

@ekelleyv currently the most performant way to do this is queryRenderedFeature + GeoJSONSource setData, which is much faster than setFilter-based approach. But we definitely want to investigate what went wrong with setFilter after 0.21.0.

[andrewharvey]

currently the most performant way to do this is queryRenderedFeature + GeoJSONSource setData, which is much faster than setFilter-based approach.

One issue with this approach is if the filter style changes the stroke, queryRenderedFeature only returns the tile clipped geometry which means the stroke doesn't always match the full feature. This can be worked around by doing a subsequent querySourceFeatures and using say turf.union and of course then redoing this on map move or zoom end because camera changes might bring in new tiles which include the feature hovered. I've implemented this and although it works most of the time it's certainly not the nicest solution or simplest thing to implement.

[ekelleyv]

This can be worked around by doing a subsequent querySourceFeatures and using say turf.union and of course then redoing this on map move or zoom end because camera changes might bring in new tiles which include the feature hovered.

This is exactly what we had implemented as well. We switched to setFilter for implementation sanity. We took a bit of a perf hit in 0.21.0, but cannot upgrade to the most recent release due to the regressions. The hit detection seems to be very fast, it is just the setting of the filter that causes dropped frames. Being able to pass in ids to the shader would be 💰

Purely for our own planning purposes, do you have any concept of where this fits on your roadmap? Any way we could be of help?

[mourner]

While we're still investigating in-shader filtering for hover effects, I'll try to find out what exactly regressed in setFilter performance. Maybe we can fight some of it back.

[mourner]

I just merged a PR that should improve setFilter performance for cases where the source is relatively small — please try it out on your use case.

For cases where the source is big (e.g. a few megabytes of county shapes), there are two remaining bottlenecks:

1. Having to repopulate WebGL buffers for the whole tile, even if there are just a few features to highlight. You can work around this by creating a second source with the same data, and using it for the highlight layer. Try it out as well and see if this makes a difference.
2. Having to serialize and transfer the tile for querying after every style update. I'm currently exploring several ways to fix that — if we don't manage to make the code avoid reserializing the data on filter changes, we can also introduce a new queryable option for sources that will disable this expensive part altogether (and you'll be able to use it when adding a second source for highlighted layers).

[chrisvoll]

@mourner @ekelleyv and I tested out the most recent master version with our maps and it's significantly better. Thanks so much for the quick turnaround!

[lucaswoj]

Retitled this issue to better reflect the work happening & avoid specifying a particular implementation.

[ezheidtmann]

I ran into this issue today on v0.32.1 with a 3MB geojson source containing 910 polygon features. In my case, I found that changing the source to a vector source improved performance immensely -- from unusable to perfectly fine. Perhaps there is some opportunity for improvements in GeoJSONSource?

[mourner]

@ezheidtmann that sounds surprising. Would you mind setting up a JSFiddle with that GeoJSON and setFilter usage so we could take a quick look?

[ezheidtmann]

Sure thing. PR has possible fix. Fiddle showing slowness is here: http://jsfiddle.net/vk2knag7/1/

[cpsubrian]

Are repeated calls to setFilter queued up? This seems to be the case for me and one thing that would improved perceived performance would be if subsequent setFilter calls on the same layer cancelled each other out, rather than all queuing up for rendering. For an example of this 'queuing' see https://terraeclipse.github.io/react-mapboxgl/?selectedKind=ButtonLayer&selectedStory=Example

If you zoom in a little (so you at least see the state labels), and move your mouse in a circular motion you can see the queuing I'm talking about. Though this is react-driven, and not pure JS, I'm pretty sure setState or something else isn't to blame here because the overlay (which operates separately from the map) updates immediately.

Note: Here the calls to setState and thus setFilter are already throttled based on requestAnimationFrame().

[cpsubrian]

Another observation, the responsiveness seems to slow down if more features go 'offscreen'.

[peterqliu]

anandthakker changed the title from Improve "Map#setFilter" performance to Optimize "Map#setFilter" performance for "hover effect" use case

setFilter is definitely worth optimizing on its own. But for the specific use case of hover effects, using setData with the desired object's whole geometry can actually be significantly more performant.

[cpsubrian]

Updated my lib (and example) to (optionally) use the setData approach and while it is definitely more performant, it routinely fails because you have to @turf/union the hovered features (if you want a solid polygon to style) and that often breaks down because of how mapbox breaks up polygons internally. For now I log the union error with a warning and fall back to just calling setData with the individual polygon parts, but this means you can't really do border or transparent styling.

I totally understand that the webgl-based rendering presents different tradeoffs, and I don't think there is anything to 'fix' here. But I definitely await setFilter performance improvements because it is just all-around the better approach over setData. The resulting code is simpler, the styling works like you would expect, and you can't get errors from trying to join strangely broken polygons.

[cpsubrian]

In the above-linked example, Tennessee breaks at certain zoom/pan, because turf/union cannot join the strange polygons that are being passed back from querySourceFeatures.

[benderlidze]

any updates on this?

[andrewharvey]

@benderlidze As a workaround until this ticket progresses check out #5040 (comment) as there is a trick of duplicating the source which improves performance of setFilter based hovers.

[jfirebaugh]

→ #6022