add MBTiles support

[incanus]

This should be straightforward based on RMMBTilesSource. However, linking to SQLite is a problem if we want to keep things simple for installation. Two thoughts right now:

1. For CocoaPods, make MBTiles suppert a subspec. Linking isn't much of a big deal here anyway since it's managed.
2. For manual install, just make a macro at the top of the file to #ifdef out the SQLite code by default. Then, if the dev wants, they can flip the macro bit and link it themselves. This gets around people having to link before use.

[radven]

Being able to have offline overlays that come built into an app is essential to some of the apps we are working on. I was hoping to use MBTiles for this, and this seems like a great feature to integrate in.

[kontextbewusst]

This would indeed be a major breakthrough... having the comfort and slickness of MapKit, combined with the flexibility of offline mbtiles sources. Great!

[thdlx]

MBTiles support would be great for my project ! MBXMapKit is a nice approach combining the SDK map kit and Mapbox. Good job team !

[incanus]

Still not able to get to this in the near term. But to lay out the strategy I was planning:

• Have a look inside MBXMapKit.m at https://github.com/mapbox/mbxmapkit/blob/a9bc909dc52d76dd61839fc40e62d3efbd1dc189/MBXMapKit.m#L83-L103 for where the layer's min/max zooms and bounds are defined. This also relies on the region that is set in https://github.com/mapbox/mbxmapkit/blob/a9bc909dc52d76dd61839fc40e62d3efbd1dc189/MBXMapKit.m#L115-L130
• The actual tile URL formulation and fetching is done in https://github.com/mapbox/mbxmapkit/blob/a9bc909dc52d76dd61839fc40e62d3efbd1dc189/MBXMapKit.m#L155-L240 per the new MapKit API. For MBTiles, URLForTilePath: would not be used and instead loadTileAtPath:result: would just hit the SQLite directly for the tile defined by path (i.e. the tile z/x/y).
• Over in the SDK, you can see in https://github.com/mapbox/mapbox-ios-sdk/blob/1517baf35c30c2a64252a834e662d00e5e7158f9/MapView/Map/RMMBTilesSource.m all of the SQLite code (called via FMDB) that is used to open the MBTiles, get the min/max zoom and bounds, and pull out tile data. In this case, the class forms whole UIImage objects from it, whereas loadTileAtPath:result: in MapKit would just act on the NSData directly.
• There is probably some work to be done around the threading model based on how MapKit works vs. how the iOS SDK (via CATiledLayer) does, but since the existing SQLite code uses the FMDatabaseQueue serial queue API for thread safety, a first cut should work pretty well.

[willsnookrz]

I've had some success playing with offline tiles on the MapKit side (i.e. not involving MBXMapKit) by subclassing MKTileOverlay. Here are the main things you'll need to do to make the API work:

Get an MKMapView set up in a view controller that implements MKMapViewDelegate protocol. The view controller should be set as the MKMapView's delegate.

In the view controller, implement - (MKOverlayRenderer *)mapView:(MKMapView *)mapView rendererForOverlay:(id<MKOverlay>)overlay so that it will return a [[MKTileOverlayRenderer alloc] initWithTileOverlay:overlay] when it gets called with your subclassed MKTileOverlay as the overlay.

Subclass MKTileOverlay with the following important bits:

- (MKMapRect)boundingMapRect
{
    // This works to test with, but you should figure out the actual bounding box
    return MKMapRectWorld;
}

- (BOOL)isGeometryFlipped
{
    // Default coordinate system is upside down relative to an MBTiles from TileMill, so flip it
    return YES;
}

- (void)loadTileAtPath:(MKTileOverlayPath)path result:(void (^)(NSData *tileData, NSError *error))result
{
    // Find your MBTiles file and open it with sqlite, FMDB, or whatever
    // Figure out if path represents a valid tile in your MBTiles file
    // Assuming it does, make this query to get the tile...
    NSString *query = [NSString stringWithFormat:@"select tile_data from tiles where zoom_level = %ld and tile_column = %ld and tile_row = %ld",(long)path.z,(long)path.x,(long)path.y];
    // Based on what comes back from your query, initialize these objects appropriately...
    NSData *myTileData;
    NSError *myErrorStatus;
    // Invoke the argument block to complete the asynchronous load...
    result(myTileData, myErrorStatus);
}

Back in your view controller, in viewDidLoad or someplace, do something kinda like this to add your overlay to the MKMapView:

_tileOverlay = [[MBTilesOverlay alloc] initWithURLTemplate:nil];
//_tileOverlay.canReplaceMapContent = YES;
[_mapView addOverlay:_tileOverlay];

[ghost]

Hi Justin,

I have some free time again, so I'm gonna take a swing at this. No promises, but I'll send you a pull request if things go well.

[incanus]

I like the approach over in https://github.com/mapbox/mbxmapkit/issues/20 of doing this with defines for the folks that really need it. Leaving this open until this is addressed and integrated either in #20 directly or the aggregate #38 pull.

[tkrajacic]

The ability to use vector-mbtiles would be awesome, though I am not sure how easy this would be to implement within this confined project. As far as I understand, there would have to be some mechanism to render those on-the-fly then. Would something like this be feasible?

[incanus]

Would something like this be feasible?

Yes, but this is a separate from MBTiles proper -- MBTiles is more the transport medium than the rendering. We're looking into vector rendering now.

[ghost]

This can wait [edit: ... until after the 0.3.0 release]

[radven]

Is "this can wait" referring to vector rendering, or MBTiles overall?

The ability to have offline maps bundled with an app seems to be a pretty central feature to MBXMapKit, at least how I envision using it.

Thoughts? Is there another more preferred way than MBTiles suggested?

[ghost]

@radven Ah, sorry for the overly terse comment there. What I had in mind was in the context of the huge pile of stuff that we're trying to merge into master from the issue79 branch (see PR #81).

MBTiles support is currently tagged as part of the 0.3.0 milestone, but it's totally not ready yet. There's some code sitting around in one of my old branches from February or so, but it needs a bunch of work. On the other hand, there's lots of stuff which is totally ready to go, including a brand new offline map downloader which I'd encourage you to take a look at.

@incanus and I are thinking that we can probably get a good 0.3.0 release out some time this week or next, but that wouldn't include MBTiles. So "this can wait" = "let's not hold up the 0.3.0 release for this feature".

[ghost]

@tkrajacic This is the MBTiles issue that I mentioned over in #104

[tkrajacic]

I'll have a shot at it!

[tkrajacic]

Just to see if I am going in a direction you like:

I think there should be a wrapper for the mbtiles file (just as there is for the MBXOfflineDatabase) for two reasons:

• Well, MBXOfflineDatabase already nicely does this and staying with good patterns is great.
• The proposed MBXMBTilesOverlay should not deal with sqlite internals imho. (Although one could argue, that this is its express purpose.

Further:

• The MBXMBTilesDatabase should be initialized with an NSURL to the file (and so should the other already present classes as well) as this is the preferred way to refer to resources according to the Apple Developer Library

[ghost]

@tkrajacic I just committed a change (https://github.com/mapbox/mbxmapkit/commit/fa58498faf92ef773f8b5e4ca661f936be32c6ed) to the issue3-mbtiles branch which reflects my understanding of how you are suggesting mbtiles ought to work. If that's what you have in mind, I like it.

[tkrajacic]

Absolutely! I was already implementing it that way. (Which now poses the problem: Do I now rebase from your branch to mine, or merge?)

[ghost]

Okay, cool. For branches, I would suggest that you do whatever is most convenient for you, so that when you're done you can submit a pull request against the issue3-mbtiles branch. I'm about out of time to spend thinking about mbtiles until probably the end of this week, or maybe next week, so there isn't any hurry.

We originally had the mbtiles feature planned as part of the 0.3.0 milestone, and that release will hopefully happen this month. I'm thinking now that mbtiles will probably end up in the 0.3.1 release, perhaps in June.

[tkrajacic]

Mhm, do you want to keep the (NSData *)dataForURL:(NSURL *)url withError:(NSError **)error signature? Wouldn't it make more sense, if the MBXMBTileOverlay would call something like (NSData *)dataForPath:(MKTileOverlayPath)path on its MBXMBTileDatabase as loadTileAtPath:... can then just forward the path? I can't imagine a scenario where using a URL would make sense. Maybe for switching out an mbtiles database with an MBXOfflineDatabase it could be useful to have this as an optional way of getting tiles?

[tkrajacic]

Should the Project contain a demo file (.mbtiles)? I have added a small (~1.5MB) file to my Project for testing (quick export from Tilemill).

MBTiles support is working fine so far on my branch but there is a lot of duplication, as I have implemented a separate overlay because mbtiles files and mapbox maps do not share that much features, and most of the stuff in MBXRasterOverlay just makes no sense for an MBTiles file as far as I can tell.

Then again, @wsnook and @incanus you know how this should fit in, so I will gladly change it to your liking.

[ghost]

Should the Project contain a demo file (.mbtiles)?

Yes. There are also a couple of small mbtiles files in this folder of my old issue3+7 branch which I created for testing with.

[ghost]

@tkrajacic Taking a look at your code so far, it looks like you're making good progress.

I'm not sure I understand what the advantage is of having MBXMBTilesDatabase split out into a separate class from MBXMBTilesOverlay. The reason we followed that pattern with MBXOfflineMapDatabase is that the MBXRasterTileOverlay class needed to be separate from the MBXOfflineMapDownloader class, and they both needed to work with MBXOfflineMapDatabase objects.

The reason for having those three classes cooperate for offline maps is that it enables MBXMapKit to hide the complicated implementation details of the offline map downloader so developers don't have to worry about their own offline maps implementation. With MBTiles, TileMill does all the work of creating the .mbtiles files, and it's not much work for the application to manage those files on its own, so the situation is really different compared to offline maps.

It seems like you could just have something like - (id)initWithMBTilesURL:(NSURL *)mbtilesURL in MBXMBTilesOverlay and keep the header files simpler. The reason I mention this is that when we put something in a header file, it becomes a part of the public API, and then we have to support it. There are big advantages in terms of support and maintenance when we avoid creating any non-essential classes and when we avoid putting anything in the headers which could be put into the implementation files instead.

Thanks for taking the time to work on this.

[tkrajacic]

Excellent points, I will refactor to your suggestions.

[ghost]

I like @property (nonatomic) BOOL shouldOverzoom;. Do you know about the overzoom code that I have in my old issue35 branch?

[tkrajacic]

Sure, that's where I stole it from iirc. ;) For now, I mostly copied some old stuff in and started massaging the code.

[ghost]

General status update for anyone watching this issue: I haven't forgotten about MBTiles, but it's also behind a couple other things in my queue.

[tkrajacic]

I don't think anyone would have thought that you'd have forgotten ;) We have seen all the progress on other projects which you probably had your part in!

I have also further enhanced my implementation of MBTiles support (not on GH yet) with asynchronous sqlite access and shared connection per overlay.

[ghost]

@tkrajacic It makes me a little nervous when you mention "asynchronous sqlite access and shared connection per overlay". Could you say a little more about that? I'm wondering if you've done any profiling to see if there is a performance gain? Also, do you feel like the new things you're working on help the code to be more readable?

As far as I could tell from the offline caching stuff, opening lots of sqlite connections and reading synchronously works very fast, and it doesn't create any noteworthy memory management issues.

I've deliberately tried to do sqlite code using a simplistic brute force approach to cut down on complexity of the code (sqlite c calls are challenging enough to read already) and to reduce the opportunities for threading related problems.

I know that the specific way I was calling libsqlite3 was thread safe, but from reading the sqlite3 documentation, I got the idea that other ways to call it might require a lot of additional safeguards of the sort included in FMDB.

[tkrajacic]

hehe, no need to get nervous @wsnook ;)

Basically, I thought it made more sense to have every MBXMBTilesOverlay have it's own database connection that its methods would use. I'm not sure, that there is much performance gain from it but I think it is just simpler and easier to read. (Sadly I also lack Instruments experience to test this seriously)

I have modeled the loading of the tiles in the same way you implemented with offline databases in a method

- (void)asyncLoadMBTilesDataForPath:(MKTileOverlayPath)path
                        workerBlock:(void(^)(NSData *, NSError **))workerBlock
                  completionHandler:(void(^)(NSData *, NSError *))completionHandler

and the asynchronous loading is done on a dispatch queue that is also a property of every MBXMBTilesOverlay instance. Therefor reading is serialized but not on the main queue.

I'll try to update my fork tomorrow so you can have a look. Maybe you can think of a better implementation, then I'll gladly change/refactor it! I have tested my implementation with quite a lot of overlays in my own app and it worked nicely so far.

[ghost]

@tkrajacic Okay, cool. I'm looking forward to seeing what you come up with.

[tkrajacic]

@wsnook My changes are on my fork ready for inspection. (issue3-mbtiles branch) Give it a spin and tell me if you would prefer a different approach.

The ideas behind my changes were:

• Reduce number of calls to sqlite API by using the same connection to the same mbtiles file.
• Use a serial dispatch queue to access an mbtiles databse in a background thread without needing locks.
• Use the same method structure to retrieve data from the database as is used for the offline cache variant. I had the idea that the worker-block could be used if the data needs preprocessing in the future like an image filter or (and there my imagination went probably a bit too wild) if the tiles might not contain image data but ::drumrolling:: vector data that needs to be rendered into images first.

[incanus]

• Reduce number of calls to sqlite API by using the same connection to the same mbtiles file.
• Use a serial dispatch queue to access an mbtiles databse in a background thread without needing locks.

This is starting to sound like FMDB reinvention territory. This is the library we use over the SDK for serial queued SQLite calls to great effect. If we are going to go this route, I think we should look into using that well-tested library than continuing to build up custom SQLite handling.

[tkrajacic]

@incanus I would agree if I would actually have done it in such an abstract way, but I think my implementation is far simpler and less abstract than FMDB.

[ghost]

@tkrajacic and @incanus I just read through through tkrajacic/mbxmapkit's issue3-mbtiles MBXMBTilesOverlay.m, and it reads pretty well. I'd want to spend some time running the code and banging on it to be sure everything holds up well, but my impression is, if this works reliably, we should probably just use it. I feel like MBTiles is an area where it's important to have some kind of support, but overthinking it too much is probably a waste of energy which could be spent on GL.

[incanus]

Ok, fair enough. Just wanted to put it on the radar. I haven't looked at @tkrajacic's code in depth yet.

[tkrajacic]

I'll happily improve my code to your suggestions. I'm sure there is still many things to improve on. I also looked at FMDB months ago but I found it overly complex at that time for this particular purpose.

@wsnook Please point out any rough edges and I'll get on it asap.

[ghost]

@tkrajacic This looks pretty good to me. I haven't run it yet, and I always like to do that with stuff which I think is working, because sometimes there are still surprises waiting to be found by some test I hadn't thought of before. If you have a Retina iPad, using one of those to quickly pan and zoom a lot is often a good way to spot problems because so many simultaneous requests get made.

I looked at your #110 pull request, and it seems like all of the commits from your fork are up there, so I think this is probably good for now unless you find any bugs which need fixing.

[tkrajacic]

Yeah, #110 should include everything. the only thing is, that I had some 1,5MB mbtiles testfile in there and as far as I understand, it is still there in the history. That's why I wrote the obscure comment on the pull request. You'll have to decide if this is acceptable or not.

[ghost]

It's been a while since this issue was last updated, so for anyone who's still watching, I recently made another MBTiles pull request, #142, which adds a simple and relatively polished MBTiles overlay with overzooming.

I wrote this code offline for use in an app that's not on GitHub. So, the reason I'm creating a new PR (vs. modifying an existing one) is that I already had the code working pretty well and I wanted to make it publicly available. MBXMBTilesOverlay is pretty self-contained, so even if #142 doesn't get merged, it should be pretty easy to copy and paste into other projects.

[tkrajacic]

Still watching here indeed! ;)

Thanks for making this available!