Skip to content

Conversation

adrian-kong
Copy link
Contributor

https://docs.mapbox.com/help/troubleshooting/mapbox-gl-js-performance/

  • disables sending metrics to mapbox
  • enables optimize base styles
  • enables attribute use desktop opengl
  • half required points to draw ellipse (might be better to scale this w.r.t distance on map since small circles looks ok rendering ~ 8 points)

@adrian-kong adrian-kong requested a review from a team March 20, 2023 04:43
@swiftnav-travis
Copy link

Frontend and Release Workflow Started here

@swiftnav-travis
Copy link

Frontend and Release Workflow Started here

Copy link
Contributor

@pcrumley pcrumley left a comment

Choose a reason for hiding this comment

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

This a pretty difficult thing to review other than just a rubber stamp, but it looks fine?

marking as request changes because you need to update the documentation or undo a refactor

@adrian-kong adrian-kong requested a review from pcrumley March 20, 2023 22:47
Copy link
Contributor

@pcrumley pcrumley left a comment

Choose a reason for hiding this comment

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

I kind of disagree with decision to remove the comments, i think putting it as an inline comment makes more sense but i don't think it should block merge.

@swiftnav-travis
Copy link

Frontend and Release Workflow Started here

@adrian-kong
Copy link
Contributor Author

I kind of disagree with decision to remove the comments, i think putting it as an inline comment makes more sense but i don't think it should block merge.

think it might be changed in later PRs if the optimal # of points for zoom levels is figured out
but for now might just leave default 16 pts => 20 degrees

Base automatically changed from adrian/prot_lvl to main March 22, 2023 00:30
@adrian-kong adrian-kong merged commit 6acfd17 into main Mar 22, 2023
@adrian-kong adrian-kong deleted the adrian/map-performance branch March 22, 2023 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants