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

attribution_control._updateAttribution causing Style Recalculation and Layout #4447

Closed
Scarysize opened this issue Mar 17, 2017 · 6 comments
Closed
Assignees

Comments

@Scarysize
Copy link

Seems the attribution control is updated every time .setData is called on a GeoJSON source (which makes sense). This update causes the browser to recalculate styles and layout. This is fine most of the time, because you are calling .setData once. Though when animating features on the map in a animation loop (requestAnimationFrame), this costs valuable milliseconds each frame. I observed between 0.5 and 4ms (!).

In my application I'm doing something similar like the example below does (actual implementation-wise pretty much exactly that).

Versions I experienced this:

  • 0.32.1
  • 0.34.0

Steps to Trigger Behavior

  1. Visit https://www.mapbox.com/bites/00318/
  2. Open DevTools with Timeline and record a few seconds while points are being animated
  3. Check for purple "Recalculate Style" & "Layout" bars in the flame chart

Expected Behavior

Don't trigger Style Recalculation and Layout every frame

Actual Behavior

Above mentioned is happening every


Some screenshots taken on a MBP 13" (mid 2015), recent Chrome

screen shot 2017-03-17 at 22 14 01

screen shot 2017-03-17 at 22 14 09

screen shot 2017-03-17 at 22 17 43

@lucaswoj
Copy link
Contributor

lucaswoj commented Mar 17, 2017

Nice catch! We're always looking for perf improvements to setData. We'll tackle this in the next few days.

@mollymerp
Copy link
Contributor

Thanks @Scarysize ! That demo you linked to is using an older version of mapbox-gl-js and I think the behavior is different now since we refactored our eventing for sourcedata #4347 . Its true that Recalculate Style (points to AttributionControl#_updateEditLink on my Timeline) was being erroneously called, and I've made a PR to correct that #4448

I haven't been able to reproduce what you're seeing with the most recent release, so I'm cautiously optimistic that this has been resolved.

@Scarysize
Copy link
Author

Great, thanks for the fast reaction. I will give it a try as soon as the PR is through. 👌

@mollymerp
Copy link
Contributor

@Scarysize we just merged the fix into master! Mind letting us know if this helps with your issue?

@Scarysize
Copy link
Author

@mollymerp Thanks for the heads-up. Looks good so far, no more style recalculation or layout every frame! Glad this could be resolved so fast, I guess this can be closed now 🎉

@mollymerp
Copy link
Contributor

Great to hear! thanks for the followup 😸

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

No branches or pull requests

3 participants