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

Show old tiles until new ones are downloaded #79

Closed
vinicentus opened this issue Jul 30, 2018 · 52 comments · Fixed by #572
Closed

Show old tiles until new ones are downloaded #79

vinicentus opened this issue Jul 30, 2018 · 52 comments · Fixed by #572
Labels
bug This issue reports broken functionality or another error

Comments

@vinicentus
Copy link
Contributor

When a user zooms in enough, new, more zoomed in tiles are loaded and displayed. Would it be possible to show the images of the current layer until the new pictures are loaded instead of just showing a grey map while the new tiles are loading?

@johnpryan johnpryan added the bug This issue reports broken functionality or another error label Sep 14, 2018
@johnpryan
Copy link
Collaborator

Yes, we should always keep the old tiles on the screen when zooming.

@imaNNeo
Copy link

imaNNeo commented Oct 28, 2018

I have this issue too,
When it's going to be fixed?

@johnpryan
Copy link
Collaborator

Unfortunately, I don't have an estimate on when I'll have time to fix this

@johnpryan
Copy link
Collaborator

That said, we accept pull requests!

@csjames
Copy link
Contributor

csjames commented Nov 1, 2018

What is the work required here?

@aswinmohanme
Copy link

I have this issue too, when the scrolling happens the tiles which are loading turn white which is bad UX

@johnpryan
Copy link
Collaborator

I think this will require some additional investigation into Leaflet's tile fetching strategy. I assume it eagerly loads the tiles at the surrounding zoom levels

@aswinmohanme
Copy link

Just glancing over the code I think we can use the placeholder field on the FadeInImage with the previously loaded tile, if we can pass in the old coords right ?

@ganisback
Copy link

I have the same issue, the usability is not good.

@vinicentus
Copy link
Contributor Author

Yes, we should really try to fix this, it shouldn't be too hard. I can't make any promises but if I have time I could look into doing what @aswinmohanme suggested.

@imaNNeo
Copy link

imaNNeo commented Dec 11, 2018

Guys you can use google_maps_flutter (Google Supported Officially) instead.
It is made alongside flutter release v1.0.

@RaimundWege
Copy link
Contributor

@imaNNeoFighT flutter_map has also great advantages towards google_maps_flutter or is it already possible that each marker on the map is also a widget?

@imaNNeo
Copy link

imaNNeo commented Dec 11, 2018

@RaimundWege
I think it's not possible to put a widget as a marker,
And maybe it is Google policy to prevent it because also in android you can't directly put a View as Marker on GoogleMap,
And in android to achieve this we had to use a projection that supported by GoogleMap that converts latitude, longitude to screen pixel position, maybe in future, this functionality added to FlutterGoogleMap (or maybe it exists already).

@ganisback
Copy link

@johnpryan @vinicentus did you find a good solution to resolve this usability issue? really hope this can be fixed.

@vinicentus
Copy link
Contributor Author

@ganisback I really haven't had the time to look into this...

@ibrierley
Copy link
Collaborator

ibrierley commented Dec 19, 2018

I've possibly got a workaround to help with the grey screen on zoom level changes, where I keep cached tiles for a small amount of time by default (eg 1 second), and doesn't let pruneTiles remove a tile until that amount of time has passed. It feels like it works ok for me, it's not perfect, but would be happy for someone to test it further.
I've got the tile_layer.dart file edited from a git clone, added/committed. Not sure what's best for someone to look at, a git push origin master, or a merge or something ? (I'm not familiar with git).

@johnpryan
Copy link
Collaborator

@ibrierley you can fork the repo, push your changes to that, and make a pull request

@ibrierley
Copy link
Collaborator

Thanks John, I figured it yesterday and created a pull request #190 . Good practice for me!

@johnpryan
Copy link
Collaborator

This is the highest-voted issue, so it's next on the agenda, but I haven't found anyone interested in working on it yet.

@Ishadijcks
Copy link

As someone interested in this feature, but without any experience developing flutter packages, this issue seems quite daunting.

Perhaps @johnpryan or anyone experienced with this package can give a general description of the best way to implement it and someone else can actually implement it.

I'm willing to do it, I just have no idea where to start :)

@ibrierley
Copy link
Collaborator

I had a look at this a while back ( see #190 ), but there was a bug somewhere (possibly not in the pull request, not sure) I never managed to track down. I've noted that there has been quite a few fixes and changes since that though, so that could be one alternate approach...(I'm not sure I'll have time in the near future to revisit)

I think the tile providing code and caching has changed a bit since I had a look as well.

I guess first question may be, do the image providers provide a callback now, so that we figure out when all the images are downloaded...(I don't think they did before, but maybe they've changed)?

That may help decide which is the correct approach...

@ibrierley
Copy link
Collaborator

That has been thought of already :D.

There are 2 main issues I think...(this was a while back when I looked, some of the code has changed since then)...

  1. Is there a callback/method of knowing when a new tile has been loaded ?

  2. I also think there is an issue with transitioning tiles whatever, even if you don't prune tiles at all, I think there was some flashing. Ages ago I did submit a pull request that kept tiles about for a time period to smooth it out, but there was some random bug (possibly with flutter, I couldn't see an issue with flutter_map, but it could also have been my code), where sometimes (rarely, but enough to cause a significant problem) the whole lot would go blank, so I withdrew it.

It feels far more fiddly than it should be, but there may be a simple solution out there if anyone wants a stab.

@ibrierley
Copy link
Collaborator

ibrierley commented Feb 7, 2020

Just had a quick peak to remind myself of the problems...

The callback side is ok, that can be achieved I think with an ImageProvider and resolve e.g something akin to the following and swap that in for the image value in the FadeInImage widget.

    ImageProvider newImageProvider = options.tileProvider.getImage(coords, options);
    newImageProvider.resolve(ImageConfiguration()).addListener(
      ImageStreamListener(
          (info,call) {
           // do stuff to manage whether loads are complete
            print( "Network image loaded" );
          },
      ),
    );
    return newImageProvider;
  }

Edit: I think this below I said is now incorrect...if I force the same tiles to be used with zooming, it seems to place them correctly, so the problem is probably elsewhere, but leaving it there in case someone else has similar thoughts.

However, I think that can help with zooms inbetween levels (eg a zoom from 13.2 to 13.6 or whatever), but I suspect when you zoom to the next level, eg 14, the old tiles are then calculated in the wrong position with the new level....maybe ?

I may well be wrong on that, I haven't done any debugging as I don't really know the coordinate stuff and calcs, but it feels like the problem may be somewhere in that ballpark.

It would be good if there is someone about who understands that part of the code.

@ibrierley
Copy link
Collaborator

ibrierley commented Feb 8, 2020

Actually, I think what I said before isn't correct (have edited), I do think it handles other levels ok. I think it's just not including them sometimes with the line ( if ((tile.coords.z - _level.zoom).abs() <= 1) tile ). So I think it's possible, just keeping old ones, maybe with a callback and with some code pruning changes...

I think then the tricky bit, is trying to make sure only the correct tiles get kept as backup. We probably don't want to keep 'all' the tiles if one hasn't downloaded at some point, just the ones that would be visible maybe (even better if we knew it covered an outstanding one, but that may be pushing it). Otherwise it's probably getting inefficient.....?

@ibrierley
Copy link
Collaborator

I've been having a play and have a rough proof of concept here I think, if anyone wants to try, it's not ready to go live, and has debugging etc, there are probably bugs!

https://github.com/ibrierley/flutter_map/blob/keep_old_tiles/lib/src/layer/tile_layer.dart

One of the main concerns is when there are outstanding network requests, just keeping unnecessary tiles around can build up, so any thoughts on that welcome. The version above is probably too eager to keep stuff around, so may end up causing performance issues down the line.

If nothing else it may give some others some ideas.

@edihasaj
Copy link

edihasaj commented Feb 8, 2020

I've been having a play and have a rough proof of concept here I think, if anyone wants to try, it's not ready to go live, and has debugging etc, there are probably bugs!

https://github.com/ibrierley/flutter_map/blob/keep_old_tiles/lib/src/layer/tile_layer.dart

One of the main concerns is when there are outstanding network requests, just keeping unnecessary tiles around can build up, so any thoughts on that welcome. The version above is probably too eager to keep stuff around, so may end up causing performance issues down the line.

If nothing else it may give some others some ideas.

Will this fix the bug even if it gives to much load in the network ?

@edihasaj
Copy link

edihasaj commented Feb 8, 2020

We can also think to create a timer to remove old tiles with time ? @ibrierley

@porfirioribeiro
Copy link
Contributor

Nice, i was trying to find a way to make this work, but i'm still not enough fluent with flutter_maps.

Hope to see this coming along in a good way 👍

@ibrierley
Copy link
Collaborator

@edihasaj I had pondered a timer solution, but I can't quite get my head around a logic solution that would feel solid.

Lets say you have a bad signal, and are downloading tiles. It may be 15 mins for example before you get a signal again, with some tiles loaded others not. If you remove the old tiles on a timer, all of a sudden the map you are looking at could end up displaying no tiles at all.

There may be other bugs issues as well. I'm not too familiar with flutter/dart/flutter_map, and testing for all the permutations of lousy connections, different pans/zooms etc is quite tricky.

The best way feels like some way to detect if a tile is covered by other tiles, but I feel like that may be quite fiddly/hacky/bloated as well (at least if I code it :D)

@edihasaj
Copy link

edihasaj commented Feb 8, 2020

@ibrierley, yes if we put evey tile in one place that can happen. It doesn't seem as a good idea. That will need to much code to write, as far as I can see mapbox's plugin is a copy of google_maps plugin (both for flutter) an I didn't yet find anything of how they did it, they may be doing it by using map controllers with coordinate detection.. or smth else I noticed was that they save a list of tiles that are very zoomed out and blured and then use them onscroll maybe, until regular tiles are downloaded.. 🤔

@ibrierley
Copy link
Collaborator

ibrierley commented Feb 12, 2020

Just pondering this a bit more, it feels like a creeping problem, getting a bit more complex each time. I'm wondering if it would actually be better to not have transparent placeholders until a tile is loaded, as if there's quite a few outstanding, it feels like it can make the movement grind a bit. (eg you may end up trying to display 60+ tiles of which only 8 are loaded).

So one possible alternative/addition, would be to not display a tile at all until it's loaded (and then fade in if necessary), not sure how easy that is without a bigger rewrite though atm (was looking at Offstage widgets, but couldn't get that working well with flutter_map).

Just wondering if anyone has had an experiment in that direction (either in flutter_map or in flutter in general) ?

@ibrierley
Copy link
Collaborator

Just a note, here's what I'm testing atm, https://github.com/ibrierley/flutter_map/blob/keep_old_tiles/lib/src/layer/tile_layer.dart if anyone wants a play and to give any feedback, it it's useful, a load of rubbish, or buggy, or feel free to use any of the added bits in there for ideas on another solution.

@ibrierley
Copy link
Collaborator

Sorry for the spam yesterday, thought I had got somewhere, anyway I think I have this time...

Feel free to have a play and feedback, especially if you can emulate bad connections, different tile providers etc.

https://github.com/ibrierley/flutter_map/tree/keep_old_tiles
There is only one file change in
https://raw.githubusercontent.com/ibrierley/flutter_map/keep_old_tiles/lib/src/layer/tile_layer.dart

So if you want a quick test, backup your old one (prob in your pub cache), and plop that one in.

There are 2 options there added...

greedyTileCount which will try and look ahead and grab an extra tile to help panning smoothness.
tryFallbackTiles which will try and keep old tiles around until it's been loaded.

There is also a possible bug (not decided yet, welcome to hear from others), on the sorting order of tiles by zoom, I've swapped that around, but appreciate that has been that way for some time, so I may have got that wrong.

@ibrierley
Copy link
Collaborator

Still actually a bit clunky on certain phones with finger pinching mainly, maybe it's asking too much for the amount of tiles/requests, so not sure this is still the solution. Needs some more pondering.

I'd be interested to know if the tiles not loading on edges of map issue is fixed with it tho. I've got an updated version with the latest master if anyone needs.

@ibrierley
Copy link
Collaborator

Sorry for spam again, probably last updates for a while if anyone wants to try. Code at links above, and updated with recent master changes.

It now has some "smoothing" (I use that term very loosely), to try and help with the jarring blinking of the map when it prunes very frequently. New tilelayer options below.

Try with defaults, but also try changing them if you have a problem to see if anything helps, or it's a load of old pants :D.

  /// Try and grab tiles in advance for pan direction. 1 probably a good balance.
  /// Don't set this much higher than one, or there may be too many tile requests.
  /// 0 May be better if network limited for example.
  int greedyTileCount;

  /// Keep an old tile, until the new one has downloaded
  bool useFallbackTiles;

  /// pruning tiles every move can lead to clunky flashing where prunes happen
  /// before loads, so try and back skip some prunes. Example 40ms. For some
  /// apps it may be better at 0, or even increased for very old devices.
  /// Maybe we could be intelligent and calculate if we need to back off pruning..
  /// ...tbd
  int tilePruneSmoothing;

@Karlculations
Copy link

Hi guys,

Not sure this is the best place to add this, but it's seems related. Currently I changed the styles for a map I have on MapBox, but now when I load the map in my app, about 95% of the map has the new styles, while the other 5% loads with the old styles. Zooming out fixes the issue until you zoom in again and the further you zoom the more tiles have the old style/image.

@ibrierley
Copy link
Collaborator

Just wondering if it's because it's cached he old style tiles ?

Is the new style using a different url (I would assume not cached if so)....it might be worth trying to clear the cache i.e https://stackoverflow.com/questions/47209606/how-do-i-clear-flutters-image-cache (I'm assuming that's the same cache. but i may be wrong).

If it does, it may be worth exposing that method easier in flutter_map

I suspect this is a separate problem to this issue though, so may be worth opening a new one if it's more complex.

@Karlculations
Copy link

Hi

Yeah, it was a caching issue. I changed the url to a new one and that one seems fine. Thanks.

@ibrierley
Copy link
Collaborator

For anyone following this. I've just been reworking an idea I had on this problem, and as before, would be interested in any testing...

https://github.com/ibrierley/flutter_map/tree/improve_tiles_display
(files tile_layer.dart and map/map.dart are only changes)

The approach in this one, is to discard completely the notion of 'pruning' tiles, and just display what we want in the first place with a fresh slate. If a tile is new/outstanding, we try and find the best old tile according to a strategy (it has a default currently to look up 3 levels, and down 1, but that's configurable.

It 'feels' better, seems have to display less tiles on average than the other approach. However, it deviates more from the original leaflet code (I've put a post on their repo asking for thoughts on a similar issue), and strips out a reasonable amount of code.

It needs more testing, especially if there are others with edge cases, transparent tiles, offline tiles, etc. But interested in thoughts.

@pento
Copy link
Contributor

pento commented Mar 29, 2020

@ibrierley: I've been testing your fix against a similar problem I've been having: zooming in by one step using MapController.move() causes the old tiles to be used, the new tiles are never displayed. Zooming in another step causes the new tiles to be loaded.

Eg, if MapController.zoom is currently 7 (and the center is currently set to the LatLng in center), running _mapController.move( center, 8 ) will zoom in, but it won't display the zoom level 8 tiles. Subsequently running _mapController.move( center, 9 ) will zoom in again, but this time it does load the zoom level 9 tiles.

Interestingly, zooming out doesn't appear to have the same problem.

Anyway, your improve_tiles_display branch fixes this issue, and I haven't noticed it causing any problems, so I'd love to see it land. 🙂

@ibrierley
Copy link
Collaborator

ibrierley commented Mar 29, 2020 via email

@obiwanzenobi
Copy link

obiwanzenobi commented Apr 4, 2020

Hi,

I am trying to fix that issue with a background layer of tiles.
Here is the code: https://github.com/obiwanzenobi/flutter_map/tree/feature/79-keep-old-tiles-on-downloading

Here is a gif:
zoomGif

It's my first PR here :) any feedback would be appreciated

@ibrierley
Copy link
Collaborator

(sorry for dupe comment, added from wrong account).

I think it feels solid and similar to the pull request I did a while back linked earlier. I think the only reason I wasn't happy with that general method was that at times you may get up to 30 tiles to display if some are outstanding, which could cause performance issues on certain devices, so may need testing there. I still think that route is better than the current solution though which flashes and gets annoying, and your solution is fine and would be happy with it.

The pull requests were waiting a long time, so gave up in the end (hope that one doesn't happen here), and rewrote a different solution as a plugin which may give further ideas if you wanted to explore more https://github.com/ibrierley/flutter_map_tile_layer_x (it removes the idea of any outstanding tiles, and deleting tiles completely, it just decides on which tile to display and if outstanding tries a recently seen one that is likely in cache, but that plugin is waiting on a pull request too).

So there's a couple of different routes, both have pros and cons, would be good to see something added :).

@maRci002
Copy link
Contributor

maRci002 commented Apr 4, 2020

Hello guys,
I implemented Leaflet's tile fetching /retaining strategy in PR #572 take a look.

@Chris1234567899
Copy link

Is the fix already supposed to be present in 0.9.0?
It appears to be not included yet :/

@maRci002
Copy link
Contributor

maRci002 commented Apr 7, 2020

@Chris1234567899 Make sure you run flutter pub upgrade command.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue reports broken functionality or another error
Projects
None yet