-
-
Notifications
You must be signed in to change notification settings - Fork 892
Added CachedTileProvider #1094
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
Added CachedTileProvider #1094
Conversation
Hi there, What it boiled down to:
I am an owner of the current plugin for caching ('flutter_map_tile_caching'), and it solves all these issues whilst providing many more advanced features. This PR may still be useful, but please consider using the more advanced and reliable plugin (as mentioned above). Thanks for the contribution :) |
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
Other maintainers, @kengu @ibrierley etc. Would like a second opinion on this, as I feel as I am biased in this situation, as I own a package that does this? Close or merge? |
I think it may need further explanation for me, as I'm a bit out of touch with the providers and caching. My experience typically has been everything currently caches ok, but naturally my experience may be different. Couple of questions for whoever... Is there any reason for this to be needed in flutter_map itself (as opposed to the ability to be added as a plugin and added to the readme with a note on the differences)? I'm typically for keeping new things as plugins unless there's a compelling reason for it. What specific benefits does it give over the existing Providers, I'm not clear about if it would be smoother and if so why ? What currently provides caching with flutter_map (without a plugin), is it purely the o.s caching images at a low level without a plugin? My crude understanding is NetworkImageWithRetry provides some caching, but am a bit unclear if that does something on top of the o.s caching iyswim. Apologies if some of this is treading over old ground :D. |
This may provide faster tile loading times, especially on slow tile servers. It would also work offline. But it won't necessarily make things 'smoother'. I think "caching" can be meant in two ways: short-term or long-term. I think short-term is currently handled OK (see below); but there is no long-term support built into 'flutter_map', which is what this PR and the existing plugin is trying to fix.
It's a bit unclear how caching currently works. I think Flutter does perform some caching automatically when
Really, the names need fixing, but that's a big breaking change (unless type definitions can be used somehow to make transitions easier). Maybe a future PR. There are two main reasons that I think this tile provider is not suitable for merging yet: This tile provider would provide long-term caching, through 'flutter_cache_manager'. But the concerning part for me is this statement on that library's:
This is a safety concern for me. I think about some user relying on an app with this tile provider, being told that the tiles will be cached safely. Let's say that user is a hiker, or something like that: when they find themselves on top of a mountain in cold weather with no Internet, they'll get their phone out for the map. And then they'll be stuck because the OS has decided to free up some space in the background. This PR also doesn't provide an easy way to manage those tiles: the only way to remove the cached tiles would be to clear the whole app cache. Therefore, I would be in favour of closing this PR. More functionality is needed, which would probably make this better for a plugin instead of a PR, which already exists. |
I agree. This PR's main cause is to provide caching. It actually does make it smoother because the tiles don't need to load every time. So let's suppose if the user has already visited a tile, it will be cached and hence when the user again goes to that particular tile, the user will get a smoother experience. The caching works in the same way as flutter_cache_manager or even native cache manager. The tile image is converted to small chunks which are then saved to OS's cache manager. OS only clears this cache (especially iOS) when the app is performing or occupying a large number of resources. Now, why would we like to clear the cache? One scenario can be that the tile design has changed and you want the user to get the latest tiles. In this case, your URL of tiles will also change and hence the provider will load from the new URL and ultimately caching that and overwriting the previous one. However, I am totally ok to close the PR if everyone thinks it won't be much useful. |
Well, personally I think I'd prefer it as a plugin, and then extend flutter_maps readme to be a bit clearer about some of the caching options and why one would choose one over the other (noting the differences between normal caching and offline caching etc). |
@kengu Any ideas?
Less grey tiles: yes. Better frame rate: not necessarily, but potentially (AFAIK from my experience with my plugin, which may be wrong).
This is still too much of a risk for me, but maybe OK for others. I know iOS is quite heavy with things like this, along with some Android vendors like Samsung.
You mentioned if the user has changed the tile layer, which I totally accept. But after a while, the storage will build up, and without an easy way for the user to clear the cache (without going through Settings), the user may become annoyed at the app. |
Ok, for now, I'm going to close this PR. Potentially in the future it can be reconsidered, but for now, this is non-essential (there are many essential PRs to merge right now) and kind-of already exists. |
I know I'm a bit late to this discussion, but perhaps one does not exclude the other? I've had one app where I've really needed the functionality provided by flutter_map_tile_caching, and I've also had other apps where I've preferred to keep things lightweight. In my current app I'm actually using the tile provider submitted in this PR, simply because it's not critical if the cache is cleared. Including it would be a convenience, and a doc comment could warn that the cache could be cleared when the OS sees fit? Then again it's not many lines of code to copy into your project either... |
I'm probably a little out of touch on caching....
|
I'm pretty out of the loop too to be honest. I'm assuming that Flutter does not cache images on disk at least, they seem to recommend using The ability to store the images in the apps directory seems to be discussed for the |
I still feel having a light weight cache tile provider would be a better option |
Flutter usually caches images until an app restart in its image providers. The custom image providers used in this repo do the same, but less reliably. The In terms of a simple tile provider using the above package, I'd be opposed to it:
Those are the main reasons it was denied originally. |
On the other hand, I would be up for making the documentation clear on how to make your own tile provider for caching using this package, including clear notices for the things I commented before, and alternatives. |
I don't feel strongly about it, like I said earlier, I think the two complement one another. If offline functionality is important then your solution is a no-brainer, but for someone like me who has continuous connectivity it carries features I don't need (and with them possibly increased overhead, and a greater risk of bugs). I understand that |
Maybe this should be part of a separate discussion... I think what's unclear currently is what any solution has over another...(ignoring file providers for the moment). It feels like there are 3+ core components of which may be desirable (any more?). Temporary cache, not guaranteed (I feel like FM should include this, even if its via OS). I.e default flutter_map I think uses network_image_provider...does this provider caching (even if via system) and does it retry (possibly also how to write your own as info, as typically in the past I have just done this anyway when I couldn't use FMs, but I'm out of date on that now). |
Probably a good idea. Open it here on GitHub, or use Discord?
If, by temporary, you mean: "until the app is closed", then we already provide this.
I would agree with this.
We kind of support this, through
Since v2, we use |
Issue Solved: Using NetworkTileProvider, the experience isn't that much smoother. I have provided the definition for CachedTileProvider which uses cached_network_image to cache the tile image so that users can get a smoother experience.