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

Removing canvas source with animate=true leaves map in render loop #5097

Closed
gpbmike opened this issue Aug 4, 2017 · 2 comments · Fixed by #5243
Closed

Removing canvas source with animate=true leaves map in render loop #5097

gpbmike opened this issue Aug 4, 2017 · 2 comments · Fixed by #5243
Labels

Comments

@gpbmike
Copy link
Contributor

gpbmike commented Aug 4, 2017

mapbox-gl-js version: 39.1

Steps to Trigger Behavior

  1. Add canvas source / layer (with default animate=true)
  2. Remove canvas source / layer

Expected Behavior

Map should revert to state it was in prior to adding canvas source / layer initially.

Actual Behavior

Map is now in an infinite render loop and eating CPU.

Notes

This relates to a comment in another issue but I think warrants its own space.

In the following codepen, if you add and then remove a canvas layer, you can see that the map is stuck in a loaded = false state indefinitely. This is indicative of the infinite render loop mentioned in the linked comment above. If you want your CPU cycles back, you have to pause the canvas source before removing it. I think it should be auto-paused when being removed.

https://codepen.io/gpbmike/pen/prEMrX

@lbud lbud added the bug 🐞 label Aug 4, 2017
@lbud
Copy link
Contributor

lbud commented Aug 4, 2017

Thanks for the report @gpbmike! If you have time, would you be interested in contributing a fix? The SourceCache class, which is responsible for managing all sources, already checks for an optional source.onRemove method, so it should be fairly straightforward to add a corresponding method to the CanvasSource class that cancels the animationLoop.

@gpbmike
Copy link
Contributor Author

gpbmike commented Aug 4, 2017

@lbud Would love to. Thanks for pointing me in the right direction.

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

Successfully merging a pull request may close this issue.

2 participants