Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Adding a new MGLSource subclass that implements datasource pattern for fetching tiles on iOS #6861

Closed
JesseCrocker opened this issue Oct 31, 2016 · 17 comments
Labels
GL JS parity For feature parity with Mapbox GL JS iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS runtime styling

Comments

@JesseCrocker
Copy link
Contributor

Im considering implementing a new subclass of MGLSource(and mbgl::style::Source) that uses the datasource pattern for getting tiles into the map. The datasource would have a reference to an object that implements an interface with a single method, something like - (void)getTile:(NSUInteger)z x:(NSUInteger)x y:(NSUInteger)y callback(returnType (^)(NSData *tileData))callback

Why i think i need this: my application needs to display data that is not in vector tile or geojson format, but it's too much data to just add it all to the map as annotations. My solution to this is to generate vector tiles from the data on the fly in the application, but currently the only way I can see to get the tiles onto the map is to run an http server in my app, and add a source pointing at the local web server. Running a local server works, but it seems like a weird hack, and likely to have odd bugs or performance issues.

Im also interested in implementing the same approach for raster tiles, to enable displaying dynamically generated rasters, but that would come after vectors.

Does this seem like a reasonable approach?
Would a pull request that implements this be considered for merging?

@1ec5 1ec5 added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS runtime styling labels Oct 31, 2016
@incanus
Copy link
Contributor

incanus commented Oct 31, 2016

Some prior art here in #507.

@1ec5 1ec5 added the GL JS parity For feature parity with Mapbox GL JS label Oct 31, 2016
@1ec5
Copy link
Contributor

1ec5 commented Oct 31, 2016

Mapbox GL JS has an analogous feature in progress called “custom sources”: mapbox/mapbox-gl-js#2920.

@incanus
Copy link
Contributor

incanus commented Nov 1, 2016

I do like this idea. I say go for it @JesseCrocker 👍

@kkaefer
Copy link
Contributor

kkaefer commented Nov 1, 2016

We already have the concept of different source types in our core code. Right now, there are three basic ways to get vector data from sources: via regular tiles, via annotations, and via GeoJSON. All of these fundamentally return GeometryTile-derived objects that hold a representation of the geometry in a geometry.hpp-compatible format.

However, before you implement a completely custom source, you should consider using a GeoJSON source. Either convert your existing geometry to GeoJSON data, or if that is not possible, a shortcut may be to use the GeoJSONSource's setGeoJSON() member function. It accepts a parameter of type mapbox::geojson::geojson, which is mapbox::util::variant<geometry, feature, feature_collection>, essentially what you need anyway to be compatible with the internal geometry represenation that GeometryTile's use. This is exposed on iOS in the MGLGeoJSONSource interface: You can initialize a class of that type with MGLFeature objects that hold your geometry information.

@JesseCrocker
Copy link
Contributor Author

After looking into this more, I think the interface i would really like to see is a datasource that gets called for each tile and can supply a mapbox::geojson::geojson object for each tile, but this would violate one of the guidlines on platform/ios/DEVELOPING.md, Ensure that the symbol is pure Objective-C and does not rely on any language features specific to Objective-C++.
So i think the more platform appropriate approach is to have the data come in the form of an NSDictionary, then let MGLCustomVectorSource convert that into mapbox::geojson::geojson, im currently pursuing that approach in https://github.com/JesseCrocker/mapbox-gl-native/tree/feature/custom-vector-source

@1ec5
Copy link
Contributor

1ec5 commented Nov 1, 2016

Two primitives you may find handy are -[MGLFeaturePrivate mbglFeature] and MGLFeaturesFromMBGLFeatures(). Also, MGLCustomVectorSource and MGLGeoJSONSource have a lot of similarities that suggest they may benefit from a common abstract superclass or protocol.

@kkaefer
Copy link
Contributor

kkaefer commented Nov 1, 2016

The advantage of supplying all features is that Mapbox GL automatically creates the tiles for you

@JesseCrocker
Copy link
Contributor Author

@kkaefer Yes, i understand that, and i wish i could just use a GeoJSON source, but unfortunately my data wont all fit in memory at once, so i don't think i can use it. Parts of the data can also change frequently, and having to reload the whole dataset due to 1 part changing seems problematic. I already have data in a spatial index, so pulling out a tile of data at a time is easy to do. If there is a way I can use the existing GeoJSON source with these constraints, im all ears, I'd certainly love to avoid what might be weeks of work.

If you think this is something that wouldn't be considered for merging, I'd love to hear that also, and then i will just pursue the approach of serving vector tiles from a local http server. I really don't want to spend weeks of work getting myself into a situation where i'm having to maintain a fork.

@kkaefer
Copy link
Contributor

kkaefer commented Nov 2, 2016

Great, these are all valid points; I just wanted to make sure that no other facility that already exists can solve your problem to avoid unnecessary time investment.

@boundsj
Copy link
Contributor

boundsj commented Nov 2, 2016

i wish i could just use a GeoJSON source, but unfortunately my data wont all fit in memory at once

The Mapbox iOS SDK Beta 2 release has #6793 which allows you to update the content of a MGLGeoJSONSource.

Parts of the data can also change frequently, and having to reload the whole dataset due to 1 part changing seems problematic.

#6177 tracks making parts of GeoJSON content modifiable without needing to reload everything. This feature is not currently on the release milestone for the upcoming 3.4.0 release though.

@JesseCrocker
Copy link
Contributor Author

What branch should i point my pull request for this feature at? I branched off of release-ios-v3.4.0 in order to use changes in MGLGeoJSONSource that were in there.

@1ec5
Copy link
Contributor

1ec5 commented Nov 3, 2016

The release branch is for features that will be in the v3.4.x series. Other than targeted bug fixes that we cherry-pick, anything going into master is unlikely to make it into v3.4.x, because of some significant refactoring taking place in mbgl on master.

Given the size of this feature and the degree to which it touches mbgl, it’s probably best to develop against master. We’ll merge the release branch into master shortly so that you can take advantage of the changes to MGLGeoJSONSource.

@JesseCrocker
Copy link
Contributor Author

Ok, sounds good, I've got this working now, and once i see release-ios-v3.4.0 I'll open a pull request.

@kkaefer
Copy link
Contributor

kkaefer commented Nov 4, 2016

Yeah, please develop against master.

@1ec5
Copy link
Contributor

1ec5 commented Nov 4, 2016

We merged the release branch back to master in #6902.

@1ec5
Copy link
Contributor

1ec5 commented Nov 10, 2016

This is also being tracked for GL JS in https://github.com/mapbox/mapbox-gl-js/projects/2#card-745298.

@asheemmamoowala
Copy link
Contributor

Fixed in #9983

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
GL JS parity For feature parity with Mapbox GL JS iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS runtime styling
Projects
None yet
Development

No branches or pull requests

6 participants