Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

Expose LocationIndicatorLayer and an alternative rendering mode based on it #319

Merged
merged 7 commits into from
Apr 21, 2020

Conversation

LukasPaczos
Copy link
Member

@LukasPaczos LukasPaczos commented Apr 10, 2020

This PR exposes a new LocationComponent's rendering mode that can be activated using the options flag:

LocationComponentActivationOptions.Builder#useSpecializedLocationLayer

The flag defaults to false, but when set, it uses a dedicated layer to render the puck instead of using the stack symbol and circle layers. The specialized implementation is fully compatible with the traditional impl, except for:

  • Constants like FOREGROUND_LAYER or LOCATION_SOURCE which are either no-op or return different types in this mode.
  • All options that alter the image ID, like LocationComponentOptions#foregroundName are ignored. There's a simple alternative in the form of LocationComponentOptions#foregroundDrawable and other though.

This branch is currently based on mapbox/mapbox-gl-native#16388 and also requires a fix from mapbox/mapbox-gl-native#16391 to be completely stable. When all those changes are released, I'll retarget the submodule pin.

TODO:

  • IndicatorLocationLayerRenderer tests.
  • Implement location click listener. Because the new layer is not feature-based and we cannot simply query the map, there's no straightforward way to achieve this. We're considering different approaches, like the indicator layer returning its dimensions, so that we can calculate the hitbox. This is TBD and will be resolved or declared incompatible before this lands.
  • Changelog.

/cc @chloekraw @galinelle

@LukasPaczos LukasPaczos requested a review from a team April 10, 2020 16:07
@LukasPaczos LukasPaczos marked this pull request as ready for review April 14, 2020 17:28
@chloekraw chloekraw requested a review from a team April 15, 2020 05:30
@LukasPaczos LukasPaczos force-pushed the lp-location-layer-2 branch 3 times, most recently from 983f9c6 to 3e1b238 Compare April 15, 2020 12:28
Copy link
Member

@tobrun tobrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great

@LukasPaczos
Copy link
Member Author

There are still 2 PRs in flight that I'm waiting for before merging - mapbox/mapbox-gl-native#16400 and mapbox/mapbox-gl-native#16404.

If needed, we could also merge this PR and release an alpha without the location click support that relies on mapbox/mapbox-gl-native#16400 and follow up in the next pre-release.

@LukasPaczos
Copy link
Member Author

Changelog entry is below @chloekraw if you want to give it a pass.

<changelog>Introduced `LocationComponentActivationOptions#useSpecializedLocationLayer` flag. When enabled, the `LocationComponent` uses a designated map layer to render the location puck which can greatly increase the performance and reduce the rendering overhead. Check the flag documentation for more details on compatibility.</changelog>

Otherwise, I bumped all of the deps and I'll merge tomorrow after QA.

@LukasPaczos
Copy link
Member Author

Here's a quick comparison between the default rendering (symbol layers):
ezgif com-optimize (19)

and the specialized location layer:
ezgif com-optimize (20)

/cc @mapbox/navigation-android

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants