Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve way name runtime styling performance #1222

Closed
danesfeder opened this issue Aug 17, 2018 · 2 comments
Closed

Improve way name runtime styling performance #1222

danesfeder opened this issue Aug 17, 2018 · 2 comments
Assignees
Labels
bug Defect to be fixed.

Comments

@danesfeder
Copy link
Contributor

danesfeder commented Aug 17, 2018

We've been seeing some performance issues with the location icon / way name below it since the way name feature was added (mainly "jumping").

I suspect this is the WaynameView bitmap generation and then being added with MapboxMap#addImage every time we need to update the text.

@tobrun mentioned when initially reviewing that PR that this was a pretty performance intensive task from a Maps SDK perspective.

If we want to keep this runtime styling approach, I think it might may sense to switch to something like mapbox/mapbox-plugins-android#401 and then figure out the "pill" background.

The only other alternative I can think of would be creating a normal Android View and anchoring it above the bottom sheet.

cc @1ec5

@danesfeder danesfeder changed the title Improve way name run-time styling performance Improve way name runtime styling performance Aug 17, 2018
@1ec5
Copy link
Contributor

1ec5 commented Aug 18, 2018

If we want to keep this runtime styling approach, I think it might may sense to switch to something like mapbox/mapbox-plugins-android#401 and then figure out the "pill" background.

Yes, I think using a symbol layer would dramatically improve the performance of this feature. However, getting a rounded rectangle to respect the text width is going to be a challenge. mapbox/mapbox-gl-style-spec#461 added an icon-text-fit property that can resize an image in either or both directions. Unfortunately, it scales the entire image, including the caps. Some potential paths forward:

  • Nine-part resizable images mapbox-gl-js#4143 would introduce options for nine-patch images. This would be the correct solution, one that would benefit a wide range of use cases on every platform. Vector icons have been proposed as a more perfect alternative, but I think nine-patch images solves 90% of the use cases out there, and the prevalence of similar APIs on each platform speaks to that tradeoff.
  • If we make the corner radius small enough and cap the label’s width, the sideways stretching won’t be quite as apparent.
  • Place the same label at the same location multiple times. Other than the topmost label, each copy of the label would be progressively shorter and wider. The number of labels determines the smoothness of the corners. There were some CSS hacks to this effect before the border-radius property was standardized in CSS3. I don’t expect that copying the labels this way would appreciably impact rendering performance.

No matter which approach we take, we should make sure to allow collisions between this label (these labels) and other symbols on the map. Otherwise, constant symbol collision detection will severely hurt performance.

The only other alternative I can think of would be creating a normal Android View and anchoring it above the bottom sheet.

This is the approach taken by the iOS navigation SDK, which is why I didn’t forsee performance becoming an issue for the Android implementation. #1173 requests the same anchoring behavior seen in the iOS navigation SDK.

By the way, mapbox/mapbox-navigation-ios#1576 added the current road’s shield to the road name label on iOS to improve comprehension. We had to render the shield into a native image, emulating the logic inside the style for selecting a shield from Streets source feature properties. Fortunately, since the Android label is part of the style, it’s just a matter of copying the feature querying result’s properties onto a new point feature.

On iOS, we chose to keep the road name visible, because sometimes the road name and route number are both important, and also because a bare shield in a bubble looked almost like a bug. Unfortunately, the runtime styling API lacks text layout capabilities: mapbox/mapbox-gl-js#4366. If the Android implementation sticks to the runtime styling API, I’d recommend showing only a shield or a name at any given time, or vertically stacking the shield and name.

/cc @vincethecoder

@danesfeder
Copy link
Contributor Author

@1ec5 thanks for the great input here - @Guardiola31337 @devotaaabel let's discuss our options here and decide on a way forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Defect to be fixed.
Projects
None yet
Development

No branches or pull requests

2 participants