tangrams / tangram

WebGL map rendering engine for creative cartography
https://tangram.city
MIT License
2.22k stars 290 forks source link

Feature selection optimizations #679

Closed bcamper closed 6 years ago

bcamper commented 6 years ago

A few optimizations to improve performance for feature selection:

These only improve existing behavior, so can be included in a 0.15.x release (and we also have several PRs stacking up for a 0.16.0 release!).

bcamper commented 6 years ago

@matteblair @meetar this PR also includes this change (which I think should be considered a fix) to outline interactivity behavior: 4415142a455235e21e5cbd587364159fa45087dd

Previously, the outline of a line could never be interactive. This changes that so the outline inherits the value of interactive from the inline, but can still be overridden (as we do for line properties like cap and join). So you could set the inline to be interactive but the outline to not be (the current, but I think unintended, behavior).

But I am not sure that we should allow for only the outline to be interactive, but the inline to not be? Besides being logically weird (but maybe valid), it doesn't functionally work right now because the interactive: false inline isn't rendered in the selection pass, but the outline underneath is, so it still appears interactive.

Related, there may be some case (think we discussed this long ago) for syntax to set a non-interactive feature to actively block feature selection of features that are rendered behind, but that's well outside the scope of this PR.

nvkelso commented 6 years ago

this PR also includes this change (which I think should be considered a fix) to outline interactivity behavior: 4415142

Oh! That could be why Geraldine and I have had a hard time inspecting small road lines!

bcamper commented 6 years ago

this PR also includes this change (which I think should be considered a fix) to outline interactivity behavior: 4415142

Oh! That could be why Geraldine and I have had a hard time inspecting small road lines!

🔎

nvkelso commented 6 years ago

I should have said narrow. Some of the styles use a mix of lines and outlines that change width depending on the zoom.

matteblair commented 6 years ago

I'll take a look at how outline interactivity works in Tangram ES currently and figure out if anything will prevent us from implementing some of these behaviors.

bcamper commented 6 years ago

@matteblair @nvkelso merging this for now.

For outline interactivity, the behavior here is for the outline to now inherit the interactive value from the inline (I think this qualifies as a bug fix), but still allow it to be overridden.

We can optionally decide to only allow an interactive outline when the inline is also interactive; that is not implemented here, but is probably advisable given the unexpected behavior for this case (everything appears interactive anyway).

If there are behavior adjustments we want to make, such as the above, we can address before this is put into a release.

bcamper commented 6 years ago

@matteblair as for the other changes here, they are internal optimizations which ES may want to implement corresponding versions of, but they have no affect on rendering behavior.