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

Support of storage tile caching #564

Closed
wants to merge 9 commits into from

Conversation

bugDim88
Copy link
Contributor

@bugDim88 bugDim88 commented Mar 27, 2020

Hello, this PR provides TileProvider that stores tile images on disk.
PR has two new main classes:

  1. StorageCachingTileProvider - tile provider that automatically switches the network and local tile source. Basic logic is if local tile has expired update period (which you can define) than provider grabs this tile from network and updates local db row with that image, else provider takes local tile.
  2. TileStorageCachingManager - singleton with static api for local db managing. StorageCachingTileProvider uses this class under the hood for local tiles.

Features:

  • automatically persist caching of browsing tiles:
/// put this layer options to FlutterMap
/// [cachedValidDuration] - valid time period since [DateTime.now]
/// which determines the need for a request for remote tile server
 TileLayerOptions(
      tileProvider: StorageCachingTileProvider(cachedValidDuration: Duration(days: 1)),
      urlTemplate: 'https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png',
      subdomains: ['a', 'b', 'c'],
    );
  • preload tiles for given 'LatLngBounds' and zoom level:
...
storageCachingTileProvider.loadTiles(bounds, minZoom, maxZoom, layerOptions)
.listen((progress)=>...);
...
  • automatically hold given local db size (uses sqlite trigger under the hood). In overflow case, manager removes the most oldest tiles:
await TileStorageCachingManager.changeMaxTileAmount(2000);

Look at AutoCachedTilesPage in example folder for more info.
Also PR has corresponding changes to README.md

Note: for local db TileStorageCachingManager uses sqflite,
not tested on IOS devices.

closes #279, #170, #276

@maRci002 maRci002 mentioned this pull request Apr 7, 2020
@alukyano
Copy link

Very nice! Works for me like a charm in IOS emulator.
BTW, can I suggest an optional parameter something like "dbname"? It can be very useful in case you have more than one layer to be cached.
Another option is current amount of tiles in cache db just for information purposes.
Anyway thanks a lot!

@avioli
Copy link
Contributor

avioli commented Apr 29, 2020

Will this work with a TileLayer that changes its URL template? I’ve got an app that switches between layers of data, using a different urlTemplate. Also for authenticated users I need to set a header for each tile request, but can’t seem to see how.

@guenth39
Copy link

Love this way of caching the tiles and a nice workaround for #636

@m-skolnick
Copy link

Works great for me on an iPhone XR. Thank you for putting in the work. This would be a great feature to add!

content: Text(
'Cache db size: ${currentCacheSize.toStringAsFixed(2)} mb.'
'\nCached tiles amount: $currentCacheAmount'
'\nSeriosly want to delete this stuf?'),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest changing these two lines:

'\nCached tiles amount: $currentCacheAmount'
 '\nSeriosly want to delete this stuf?'),

to

'\nCached tiles count: $currentCacheAmount'
'\nAre you sure you want to clear cached tiles?'),

@johnpryan
Copy link
Collaborator

is this something that could be added as a separate package? I would like to encourage a more decentralized approach to plugins and tile providers

@johnpryan
Copy link
Collaborator

Happy to add any packages to the README

@JaffaKetchup
Copy link
Member

Hello.
Is there any news on the progress of this fork being merged? I created my own fork that does a similar thing, but I prefer this method.
Thanks!

@JaffaKetchup
Copy link
Member

Hi all,
I know it has only been 6 days since my last comment here, however the worry on my mind has been said in another pull request comment: #829 (comment).

This repo seems to have no maintainers, which is bad because there are many useful (and important to me) pull requests such as this and the null-safety migration (#829).

So, I'm hoping that once that new flutter_map has been published, it will have active maintainers and you may have better luck merging this PR there. Also, this will mean changing this package to be null-safe, but I think that's probably the aim in the long run anyway.

For me, this is a waiting game, as both this and the null-safety versions are important to me.

Thanks, and I hope this repo (or some version of it) becomes active again soon, with all these new features.

@mootw
Copy link
Collaborator

mootw commented Mar 15, 2021

@JaffaKetchup TLDR: You can also use an existing ImageProvider cache like cached_network_image or network_to_file_image and supply the image provider like this:

class CachedTileProvider extends TileProvider {
  const CachedTileProvider();
  @override
  ImageProvider getImage(Coords<num> coords, TileLayerOptions options) {
    return YourCachedNetworkImageProvider(
      url: Uri.parse(getTileUrl(coords, options)),
    );
  }
}

This repo is unlikely to get merged. @johnpryan appears to be wanting to remove large dependencies like sqflite and moving those to separate plugins (that anyone can make and maintain). This allows for flutter_map to be updated more frequently and be less monolithic. I agree with this methodology and am just using a custom image caching solution that I prefer over this implementation or the old implementation anyways.

This pr should be closed and moved into an independent plugin or people who want the feature can use existing imageprovider caching plugins. There is no reason to reinvent the wheel especially for something that is so complex and use-case specific like image caching.

@JaffaKetchup
Copy link
Member

Hi @moonag,
Thanks for the tip, I was already doing something similar in my PR #798.
And, you might be right that it would be a good idea to move this to a plugin, however I also feel that this is a fundamental part of any map app, as apps are supposed to work as well as possible online and offline. I would really be happy either way, and even more so if this could again be used with the upcoming null-safety version.

@johnpryan
Copy link
Collaborator

Hi, @bugDim88. We would like to keep the core package small and reduce external dependencies. If you have a package that provides this custom tile provider I could add it to the README. Thanks!

@JaffaKetchup
Copy link
Member

Hi all,
Is it then possible that this could be made into a plugin? This feature is still really important to me, and I think it would be a shame for the work to go to waste.
Thanks.

@mootw
Copy link
Collaborator

mootw commented Mar 17, 2021

@JaffaKetchup You don't need to make a plugin for flutter_map specifically as it is trivial to add it yourself using an image caching plugin. See my image above for using an existing caching solution like cached_network_image. There is a tutorial in the readme in master now too. If you have any questions I can help you set it up. If you are already using an image caching plugin like cached_network_image or network_to_file_image just create a new class like this and replace YourCachedNetworkImageProvider with whatever caching image provider your plugin uses. It will cache your map images just like it caches the rest of the images in your app.

class CachedTileProvider extends TileProvider {
  const CachedTileProvider();
  @override
  ImageProvider getImage(Coords<num> coords, TileLayerOptions options) {
    return YourCachedNetworkImageProvider(
      url: Uri.parse(getTileUrl(coords, options)),
    );
  }
}

@JaffaKetchup
Copy link
Member

Hi @moonag and all,
Thanks for your reply. I have already implemented my own complete solution that can even be setup to download an area of tiles: flutter_map_basic_offline_support, and you can use this now, however, the download function (that is not in that repo) works but is probably inefficient and has problems downloading all the edges of tiles at different zoom levels.
A plugin that can do all of this for me whilst offering other benefits (eg. this works on the web I think? (I might be wrong about this)) I would choose over my code any day. If the flutter_map owner doesn't want to merge this directly to keep dependencies down, I can understand that, but a plugin is great way to do this.
If this plugin isn't published, it would be a shame, because I feel like wanting to cache tiles is a commonly required feature (just like marking the user's location), and therefore why should people have to write unnecessary code essentially the same as everyone else's, when this code is a complete solution (there are two plugins to mark the user's location). I could easily take this code, and make a plugin, giving full credit to the contributors on here.
Another reason why a standard caching solution is not great is the fact that you are limited by the system, which could clear the cache at any time to free-up space. Using this method, I think it's not so simple? (I might be wrong about this). Also, writing a guide in the documentation is much less convenient than just using a plugin.
I feel strongly that this is a perfectly good plugin, that yes, could be written by every developer requiring something like this, but exists now, so might as well be used.
This pull got 9 likes and hearts, and plenty of comments saying it works well and that they like this feature. This also closes three separate issues.

Yes, I can see I feel strongly about this reading this back to myself, but I still agree with it.

If anyone wants to allow me to make a plugin out of this, I will happily do so, giving full credit to you guys 👍 .

Best wishes,
Luka S.

@johnpryan
Copy link
Collaborator

johnpryan commented Mar 17, 2021

The direction I would like to head in is to keep the core package small with as few package dependencies as possible. When we add a package dependency (like cached_network_image), we promise to users that the functionality of that package works, and issues will get filed here. This also requires all users to include that dependency when the add flutter_map to their project, which they might not want.

@JaffaKetchup
Copy link
Member

Hi @johnpryan,
That's perfectly understandable and I would probably put myself in the same situation if I had a package with as many users as this. So all I'm saying is, this should be made into an external plugin which developers don't have to include.

@johnpryan
Copy link
Collaborator

@JaffaKetchup can you explain more what you mean?

@JaffaKetchup
Copy link
Member

@johnpryan
For example, there is a Plugins section at the bottom of the docs:
image

So, what I'm suggesting is converting this PR that would alter the main code and dependencies of this project to another plugin, like flutter_map_location_marker, for example. This would allow developers the choice of using this, prevent addition of more dependencies to the main code, but also allow easy integration of something like this, instead of developers having to code it themselves.
As you said yesterday: "If you have a package that provides this custom tile provider I could add it to the README.". That's all I'm suggesting, as well as keeping the extra functions:
image

@johnpryan
Copy link
Collaborator

It sounds like we agree that we want to encourage users to use whichever tile provider they like by adding a package dependency in their pubspec. I don't know quite what you mean when you say you would like to alter the main code and dependencies of this project and keep extra functions?

@JaffaKetchup
Copy link
Member

Sorry, I guess I'm not being clear. I would not like to alter the code of this repo in any way, except to add another plugin link to the README (when/if this gets converted).

What I was asking 2 hours ago after your comment was if there were any plans to make this a plugin, or if we were just going to leave this alone and waste the effort and code.

If no-one wants to make this a plugin, I am happy to, and as I've said, I will give full credit to the real people who developed this code, @bugDim88 and @m-skolnick.

And also, I apologise, I completely forgot about the "is this something that could be added as a separate package? I would like to encourage a more decentralized approach to plugins and tile providers" comment, :'D. If I'd realised this, I'd have offered sooner!

@JaffaKetchup
Copy link
Member

Hi,
I've started work on making this into a plugin. The repo is publicly available at this URL, however I don't recommend using it yet as I haven't tested it at all yet.
I've copied and pasted the two tile providers and fixed the errors IntelliSense gave me, but that's it.
Meanwhile, if @bugDim88 or anyone else would like to own this repo instead, let me know. I haven't written the README yet either, but credits will go in there if no one wants to claim this repo.
If anyone else also wants to test it, please do, it would be a great help.
Thanks!

@JaffaKetchup
Copy link
Member

Hi all,
So the new package/plugin is basically finished. What do you guys think?
Should I open a pull request to get the package listed as a plugin? Should I publish it on pub.dev? Or should I wait to see if anyone wants to own it?
I hope I've done someone a favor here :).

@m-skolnick
Copy link

@JaffaKetchup We will be exploring offline maps in the next few months. When we get to that point, I'm sure your package will be extremely useful!

@Swepilot
Copy link

Swepilot commented Apr 8, 2021

@JaffaKetchup Very interested in the pre-loading of tiles as I had do do some workarounds in my app for that. Hopefully your solution is a neater one. Will test as soon as I have time.

@johnpryan
Copy link
Collaborator

@JaffaKetchup feel free to add to the README!

@JaffaKetchup
Copy link
Member

JaffaKetchup commented Apr 8, 2021

Thanks, I've opened the PR: #869.

I've also just published it to pub.dev! https://pub.dev/packages/flutter_map_tile_caching

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

Successfully merging this pull request may close these issues.

Add TileProvider that stores images on disk