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

client-side data #507

Closed
5 tasks
incanus opened this issue Oct 27, 2014 · 46 comments
Closed
5 tasks

client-side data #507

incanus opened this issue Oct 27, 2014 · 46 comments
Labels
feature GL JS parity For feature parity with Mapbox GL JS iOS Mapbox Maps SDK for iOS

Comments

@incanus
Copy link
Contributor

incanus commented Oct 27, 2014

Master ticket for building out client-side data (e.g. non-vector tiles). This is the last big feature we need for viable initial iOS deployment in apps.

Use cases:

  • Plot user location with e.g. Core Location
  • Show arbitrary point/polyline features
    • GeoJSON (about the only format we'll worry about parsing internally)
    • Externally-parsed formats like KML, Shapefile, etc.
    • Runtime-added features (e.g. here's an array of points; render them)

Progressive feature set:

  1. Spatial indexing and/or pre-tiling of feature data.
  2. Rendering of features with static styling (also hits user location use case).
  3. Arbitrary styling of polylines.
  4. Arbitrary imagery for points (sprite atlas etc.)
  5. Animated native views for points (a là MKAnnotationView on iOS).

Let's treat clustering (#320) as a separate milestone.

Related/obsoletes:

@abhay-agarwal
Copy link

Hey incanus, Mapbox-gl-cocoa looks and acts great. Any idea of release dates for these features? Specifically the run-time added features with arbitrary sprites (that's about all we need). Also, will these features follow the format of the RM objects (i.e. MGLAnnotation instead of RMAnnotation)?

@incanus
Copy link
Contributor Author

incanus commented Nov 21, 2014

Not sure on a timeline yet @abhay-agarwal, but stay tuned. For API, this will be more like MapKit (MKAnnotation protocol) than the SDK (RMAnnotation class).

@incanus incanus mentioned this issue Nov 24, 2014
@aleksandr-vin
Copy link

Suddenly discovered "source type not implemented" for geojson. Really need it. Subscribed.

@mourner
Copy link
Member

mourner commented Dec 9, 2014

The big GeoJSON rewrite is done — see mapbox/mapbox-gl-js#464 comments. We now need to port geojson-vt to C++11 and from there it'll be easy to add robust and fast GeoJSON support.

@incanus
Copy link
Contributor Author

incanus commented Dec 9, 2014

We now need to port geojson-vt to C++11

I'm going to start taking a crack at this today.

@incanus
Copy link
Contributor Author

incanus commented Dec 9, 2014

Making some headway on this. Believe it or not I'm using Swift to do it, since basically all of the geojson-vt code needs to be made strongly typed, and Swift's now my fastest typed language, plus playgrounds let me make small progress without explicitly having to compile or to suffer C++'s overhead in Xcode.

We'll need to build out formal structures for all of the parts used (features, points, feature types, tiles, stats, GeoJSONVT options, etc. as well as the strongly typed array/dictionary containers required by C++ and countering the looseness of JS's Int/Double numbers) and banging through it in a lightweight language is letting me move fast as I understand the code. Most of the way through tile.js & index.js thus far.

@incanus
Copy link
Contributor Author

incanus commented Dec 19, 2014

Feeling good here. Intermediary lib:

https://github.com/incanus/geojsonvt.swift

anigif-1418954950

Currently working on getting this to work with larger data, including the 100MB zips. Probably some lingering out of range errors and the like.

/cc @mourner

@mourner
Copy link
Member

mourner commented Dec 19, 2014

@incanus Can't wait!

@incanus
Copy link
Contributor Author

incanus commented Dec 19, 2014

Ok, we're looking feature-complete and I think basically bug-free at this point. My basic GeoJSON test cases work great, but zips are still taking a looooong time. Profiling reveals most of the time is spent in JSON deserialization (~20%) and dynamic object type casting (~70%, lol).

screen shot 2014-12-19 at 11 19 45 am

So, Swift has served me well here, but moving to C++ now.

anigif-1419015245

@bvjustin
Copy link

I opened an issue yesterday (#726) that's similar to this one, and the use case I described is similar to the ones listed at the top of this one, but I just want to add to this issue to spell out it out for the record here.

  • Display arbitrary raster features (non-tiled)

In plain english, here's how I describe it:

From a high level, I want to be able to render my (non-tiled) content that’s generated on the client in a Mapbox GL map at my specified z order in the rendering tree. You can imagine this as a weather satellite image above the terrain but below the roads/city name layers. And as these data are updated often, I would need to be able to animate/loop it. Any client provided content would not need to be parsed by Mapbox (it would be lower level than just providing an image). I can envision providing some texture data along with some vertices and tex coordinates, or maybe providing a custom shader or something...

@mourner
Copy link
Member

mourner commented Dec 20, 2014

most of the time is spent in JSON deserialization (~20%) and dynamic object type casting (~70%, lol).

That's completely ridiculous. I guess that's the price you pay for using a new but immature language :) Looking forward to the C++ results then!

@incanus
Copy link
Contributor Author

incanus commented Dec 20, 2014

Yeah, I'm very curious about it but not priority-curious about it. Might check it out over break and get to the bottom of it. I'd like to know if it's misuse, bug, limitation, etc. =

@incanus
Copy link
Contributor Author

incanus commented Dec 24, 2014

Ok, fully converted over to C++, albeit not yet working. But all in place, typed, modified for rapidjson, iterators, other C++isms, etc.

@incanus
Copy link
Contributor Author

incanus commented Dec 24, 2014

@springmeyer Any ideas here on the following link problems? Only happens when I add actual use of GeoJSONVT like so:

GeoJSONVT vt = GeoJSONVT(featureJSON_);

Per 8e8dd9b.

The link error doesn't happen for me when building mbgl-core nor mbgl-ios, just the iosapp.

Link error:

https://gist.github.com/incanus/e6eae06d78cadbc7b1ea

Any ideas? @ljbade recommended I maybe move the #include <rapidjson/document.h> from geojsonvt.hpp to geojsonvt.cpp and forward-declare the rapidjson types that I need, but they use templates and I got out of my depth pretty quick.

@incanus
Copy link
Contributor Author

incanus commented Dec 24, 2014

forward-declare the rapidjson types

BTW this is because all of our other uses of rapidjson pull in the header in .cpp files, not headers. I think I'm missing something about how rapidjson, which is a header-only library, gets linked.

@springmeyer
Copy link
Contributor

@incanus the problem is that the rapidjson::Document is not designed to be copied. But your usage is trying to copy the object. The linker error is telling: it says it cannot find the copy constructor symbol: in this case because it is intentionally not available. If I comment this line then the app can link:

diff --git a/src/mbgl/util/geojsonvt.cpp b/src/mbgl/util/geojsonvt.cpp
index 64e7987..5a8a0c4 100644
--- a/src/mbgl/util/geojsonvt.cpp
+++ b/src/mbgl/util/geojsonvt.cpp
@@ -362,7 +362,7 @@ std::vector<ProjectedFeature> Convert::convert(JSDocument &data, float tolerance
     } else {
         JSValue feature;
         feature.SetObject();
-        feature.AddMember("geometry", data, data.GetAllocator());
+        //feature.AddMember("geometry", data, data.GetAllocator());
         convertFeature(features, feature, tolerance);
     }

So, it seem like passing data around there (which is a rapidjson::Document instance) must be triggering a copy.

@incanus
Copy link
Contributor Author

incanus commented Dec 24, 2014

Oh awesome, thanks. Man, cryptic C++ errors. That's definitely a big help.

@ljbade
Copy link
Contributor

ljbade commented Dec 24, 2014

Cool I did not know this. Hopefully I will remember next time I see something like this.

@mikemorris
Copy link
Contributor

Man, cryptic C++ errors.

Hah, good to know, thanks @springmeyer.

@incanus
Copy link
Contributor Author

incanus commented Jan 7, 2015

It does!

tile z0-0-0 (features: 28980, points: 4444499, simplified: 10369

[...]

generate tiles up to z4: 19932.775000ms
tiles generated: 90 {
    z0:: 1
    z1:: 2
    z2:: 5
    z3:: 17
    z4:: 65
}

So, profiling is back on the list ;-)

@incanus
Copy link
Contributor Author

incanus commented Jan 7, 2015

Since I've got to step away from this for a bit, if anyone is curious, this is what I've been using to test.

https://gist.github.com/incanus/63ff08492d7f4ccf05e5

@twaysive
Copy link

twaysive commented Jan 7, 2015

This is pretty awesome.

@mourner
Copy link
Member

mourner commented Jan 8, 2015

@incanus wow, it works! Awesome. :) I hope we'll be able to get the timing down so that it's much smaller than JS timings for big data.

@akbsteam
Copy link

What is the current status of this? I need to start working with MapboxGL (its the only mapping library that will allow me to do what I need), but user created pins and lines are essential

@ljbade
Copy link
Contributor

ljbade commented Feb 12, 2015

@akbsteam This is basically the next item on our list of features to work on so keep an eye on the commits.

@akbsteam
Copy link

@ljbade that's really good news; is there a vague-hand-wavy-ball-park timeline for this?

@incanus
Copy link
Contributor Author

incanus commented Feb 12, 2015

@akbsteam I'd say on the order of 1-1.5 months.

@akbsteam
Copy link

@incanus thanks; is there anything we can do in the interim? just to get us going on it ...

@incanus
Copy link
Contributor Author

incanus commented Feb 12, 2015

I am working right now on pixel—lat/long—projected meters conversion functions in #853 which I hope to merge today. With that @ljbade and I will be looking into some interim solutions for basic overlays on the map, even if composited.

@akbsteam
Copy link

that sounds good; at this point we just want to add pins that we can tap on and lines between them, so anything that will let us do that in some fashion would be amazing

@bvjustin
Copy link

When I was digging into this a couple of months ago (I was asking @flippmoke questions literally the day before you guys stole him away from us), I eventually got it doing what I wanted it to do (see my comment/request above). Did it as a proof of concept to show the boss... Wish I had more time to devote to this and completely flesh it out. I'm anxious to see how your implementation compares!

@incanus incanus modified the milestone: iOS Beta Feb 13, 2015
This was referenced Feb 13, 2015
@incanus
Copy link
Contributor Author

incanus commented Feb 17, 2015

Lat/lon conversion stuff is in master and iOS annotations are happening in #877. In particular, I'm using the conversions to "fake it" atop the GL view as a starting point, but eventually we still need the spatial indexing provided by the geojsonvt branch, so I'll be getting back into that in #893.

@jfirebaugh
Copy link
Contributor

iOS Beta-relevant tickets are #893 and #928, removing this meta ticket from the milestone.

@jfirebaugh jfirebaugh removed this from the iOS Beta milestone Mar 6, 2015
@bleege
Copy link
Contributor

bleege commented May 27, 2015

Runtime Marker Images --> #1645

@incanus
Copy link
Contributor Author

incanus commented Jul 4, 2015

This master ticket is mostly implemented and the remaining details are tracked elsewhere and in active development. Closing.

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

No branches or pull requests