Skip to content

Conversation

@tobrun
Copy link
Member

@tobrun tobrun commented May 9, 2019

This PR updates to the latest style spec and a recent pre release build of the Mapbox maps SDK.

Todo:

  • move towards using a git submodule of gl-js instead of copy/pasting this file into this repo
  • remove old zIndex setup in favor of SymbolSortKey

@tobrun tobrun self-assigned this May 9, 2019
@tobrun tobrun force-pushed the tvn-update-style-spec branch from e68ceb7 to e89a0ba Compare May 16, 2019 14:18
@tobrun tobrun requested a review from LukasPaczos May 16, 2019 14:18
@tobrun tobrun marked this pull request as ready for review May 16, 2019 14:18
@tobrun
Copy link
Member Author

tobrun commented May 16, 2019

Note this PR also updates the Maps SDK to v7.4.0

@tobrun
Copy link
Member Author

tobrun commented May 22, 2019

Been running some benchmarks for animating symbols (the animatied cars driving on a map example):

no animation

Amount frames 296
Average FPS 55.82793574431573

SymbolLayer - animated

Amount frames 374
Average FPS 41.81102010196368

SymbolManager - before

Amount frames 209
Average FPS 25.355876562292273

SymbolManager - without zIndex (this PR)

Amount frames 233
Average FPS 28.16800860352976

Notes:

  • the low lever symbol layer is better for performance. If scale and performance are of importance use the low level symbol layer API instead
  • this improves performance a bit but was initially hoping on a larger impact.

@LukasPaczos
Copy link
Contributor

I see nitpick errors, but regardless of that, try rebasing on master if you'll see unit tests failing (#923 (comment)).

@tobrun tobrun force-pushed the tvn-update-style-spec branch from e89a0ba to 78b5380 Compare May 23, 2019 17:10
@tobrun
Copy link
Member Author

tobrun commented May 24, 2019

I see nitpick errors

that was because git submodules weren't correctly intialised on CI. This PR is ready for review.

Copy link
Contributor

@LukasPaczos LukasPaczos left a comment

Choose a reason for hiding this comment

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

🚢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants