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

Excess tile loading in animated setView #4109

Open
1 of 2 tasks
hyperknot opened this issue Dec 27, 2015 · 10 comments
Open
1 of 2 tasks

Excess tile loading in animated setView #4109

hyperknot opened this issue Dec 27, 2015 · 10 comments

Comments

@hyperknot
Copy link
Collaborator

  • excess tile usage when zooming in: fixed with e2fbe19
  • excess tile usage when setView, {animate:true}

At a given window resolution, a fully loaded map takes 20 requests. Changing the map takes 20 new requests. Zooming out takes 20 requests. Yet, zooming in takes 48 or 56 requests.

Same with detectRetina: switching basemap, zooming out, takes 64 requests. Yet zooming in takes 192 or 208 requests.

This is not related to retina / detectRetina, it happens with non-retina maps as well.

@hyperknot hyperknot changed the title Excess tile loading when zooming in in 1.0 Excess tile loading when zooming-in in 1.0 Dec 27, 2015
@yohanboniface
Copy link
Member

@hyperknot which version, latest master or released beta2? This should be (at least partly) fixed by e2fbe19

@hyperknot
Copy link
Collaborator Author

@yohanboniface I was testing beta2, I've just tested with e2fbe19 and so far it seems to be using equal amount of requests to 0.7!

@hyperknot
Copy link
Collaborator Author

The only scenario where I see excess (around double) tile usage is when jumping to a remote point via setView + {animate:true}. It still uses about 2x the needed tiles, even when no animation should happen (jumping to the other side of the world). When turning animate off, it's not happening.

@yohanboniface
Copy link
Member

I've just tested with e2fbe19 and so far it seems to be using equal amount of requests to 0.7!

Good news :)

It still uses about 2x the needed tiles

Just to appreciate whether it's a blocker for the 1.0 release, is this behaviour also with 0.7, or specific to 1.0?

@hyperknot
Copy link
Collaborator Author

I tested it and setView animate behaviour is the same as with 0.7. Should I close this one and open an issue for setView animate tile usage?

@yohanboniface
Copy link
Member

Just rename this one :)
Also, if you can provide a minimal test case too, that'd be really helpful :)

@hyperknot
Copy link
Collaborator Author

I'm working on a test case, but it's really hard to replicate, somehow it's not happening in a minimal case.

@hyperknot hyperknot changed the title Excess tile loading when zooming-in in 1.0 Excess tile loading in 1.0 Dec 27, 2015
@yohanboniface yohanboniface changed the title Excess tile loading in 1.0 Excess tile loading in animated setView Dec 27, 2015
@hyperknot
Copy link
Collaborator Author

Did a test case here (I get 31 vs. 9 requests, but it's random):
https://output.jsbin.com/wahoxi#
with editor:
https://jsbin.com/wahoxi/edit?html,js,output

After reading Leaflet's code, I believe this is not a bug, this is what animate: true is supposed to do. Actually, animate is acting something like "forceAnimation", where if it's true, it does an animation even to the other end of the world.

if ((options && options.animate) !== true && !this.getSize().contains(offset)) { return false; }

and
if (options.animate !== true && !this.getSize().contains(offset)) {

The best/simplest solution would be to allow the user to set the pan animation threshold like zoomAnimationThreshold is done now. I believe constant multiplies of getSize() would be a reasonable unit, so instead of checking for:
this.getSize().contains(offset)
we could check for
this.getSize().multiplyBy(this.options.panAnimationThreshold).contains(offset)

@IvanSanchez
Copy link
Member

@hyperknot I'm guessing #4193 might have fixed this already. Can you please check?

@hyperknot
Copy link
Collaborator Author

@IvanSanchez this is still happening. Animated version gets 15-26 tiles, non-animated 9 always.

What do you think is the best solution here? Calculating average speed during panAnimation and disabling animation if the speed is above a given threshold?

@hyperknot hyperknot modified the milestones: 1.1, 1.0 Apr 2, 2016
@IvanSanchez IvanSanchez modified the milestones: 1.1, 1.2 May 2, 2017
@Malvoz Malvoz removed this from the 1.3 milestone Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants