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 geojson as a valid source #145

Closed
bsudekum opened this issue Jul 30, 2015 · 12 comments
Closed

Support geojson as a valid source #145

bsudekum opened this issue Jul 30, 2015 · 12 comments

Comments

@bsudekum
Copy link

Now that mapbox-gl-native supports shapes, we should add the ability to render geojson when added as a source in stylesheet.

Feeding node-mapbox-gl-native with the (valid) style below fails with the following error:

[Thu, 30 Jul 2015 21:22:17 GMT] [info] [default] Listening on port 8889
libc++abi.dylib: terminating with uncaught exception of type std::runtime_error: source type not implemented
sh: line 1: 50433 Abort trap: 6           ./index.js
{
    "version": 7,
    "name": "Basic",
    "sources": {
        "mapbox": {
            "url": "mapbox://mapbox.mapbox-streets-v6",
            "type": "vector"
        },
        "markers": {
            "type": "geojson",
            "data": {
                "type": "FeatureCollection",
                "features": [
                    {
                        "type": "Feature",
                        "geometry": {
                            "type": "Point",
                            "coordinates": [
                                -77.03238901390978,
                                38.913188059745586
                            ]
                        },
                        "properties": {
                            "title": "Mapbox DC",
                            "marker-symbol": "monument"
                        }
                    },
                    {
                        "type": "Feature",
                        "geometry": {
                            "type": "Point",
                            "coordinates": [
                                -122.414,
                                37.776
                            ]
                        },
                        "properties": {
                            "title": "Mapbox SF",
                            "marker-symbol": "harbor"
                        }
                    }
                ]
            }
        }
    },
    "sprite": "https://www.mapbox.com/mapbox-gl-styles/sprites/sprite",
    "glyphs": "mapbox://fonts/v1/bsudekumbsudekum/{fontstack}/{range}.pbf",
    "layers": [
        {
            "id": "background",
            "type": "background",
            "paint": {
                "background-color": "#dedede"
            },
            "metadata": {},
            "interactive": true
        },
        {
            "id": "markers",
            "type": "symbol",
            "source": "markers",
            "layout": {
                "icon-image": "{marker-symbol}-12",
                "text-field": "{title}",
                "text-font": "Open Sans Semibold, Arial Unicode MS Bold",
                "text-offset": [
                    0,
                    0.6
                ],
                "text-anchor": "top"
            },
            "paint": {
                "text-size": 12
            }
        }
    ]
}

/cc @tmpsantos @mikemorris @1ec5 @incanus

@incanus
Copy link

incanus commented Jul 30, 2015

Two things impacting this:

  1. Blocking on No annotations at initial zoom level mapbox/mapbox-gl-native#1675, so shapes might not show immediately on the map until it's moved, which probably won't happen via the Node bindings & static imagery.
  2. GeoJSON is not the de facto standard on native that it is on JS (naturally) so supporting GeoJSON directly into (presumably styled? how?) geometries on native is not a common use path. It's much more common to parse the GeoJSON directly (along with whatever formats you are equally likely to use) and then add those to the map manually as annotations.

@mikemorris
Copy link
Contributor

@mikemorris
Copy link
Contributor

From @incanus via chat:

styling happens in that view controller’s delegate callbacks for the map view
and is coded there, versus coming from the data itself.
i mean, there is potential to adopt simplestyle maybe

@samanpwbb
Copy link
Contributor

@incanus @mikemorris @bsudekum i know this is totally scope creeping this issue, but it would be very useful from the perspective of app to have built-in support for geojson sources in the static API.

A Mapbox gl style template could be designed to render the simplestyle spec (in the same way we use a style template to translate vector tiles into the xray style) but that would be a relatively simple problem that would depend on node-mbgl or gl-native to treat geojson sources the same way it treats vector tile sources.

How would this look:

In a style, user can define a variable geojson source that would look something like:

var style =  {
    "version": 7,
    "name": "overlaysource",
    "sources": {
        "dynamicgeojsonsource": {
            "type": "geojson",
            "data": VariableDataThatUserPassesIn
        }
    },
    "sprite": "https://www.mapbox.com/mapbox-gl-styles/sprites/sprite",
    "glyphs": "mapbox://fonts/v1/saman-staging/{fontstack}/{range}.pbf",
    "layers": [
        {
            "id": "markers",
            "type": "symbol",
            "source": "markers",
            "layout": {
                "text-field": "{title}"
            }
        }
    ]
}

Then, it is just up to the user to know the kind of data they're going to pass in to the style (most likely via a querystring parameter like our old static API) and then write style layers to match the data. In this
case, I've set up my style to take point data with a title field

@1ec5
Copy link

1ec5 commented Jul 31, 2015

In response to:

GeoJSON is not the de facto standard on native that it is on JS (naturally) so supporting GeoJSON directly into (presumably styled? how?) geometries on native is not a common use path.

and:

i know this is totally scope creeping this issue, but it would be very useful from the perspective of app to have built-in support for geojson sources in the static API.

To square these two statements, there’s nothing stopping us from implementing GeoJSON parsing in node-mapbox-gl-native and transforming it into the data structures expected by mbgl’s existing annotation support.

This would have to be platform-specific code anyways, because GeoJSON is stored in a different data type on each platform. On iOS, where GeoJSON is not a native data type (but is used by some system frameworks), it is deserialized as an NSDictionary in Objective-C or Dictionary in Swift, whereas mbgl can only work with C++ types such as std::map. Implementing GeoJSON parsing at the mbgl level would require each platform to leave the entire GeoJSON source as a monstrous string type. This is different than the situation with gl-js, where the JavaScript environment natively supports GeoJSON as JSON as an Object.

@tmcw
Copy link

tmcw commented Aug 3, 2015

Implementing GeoJSON parsing at the mbgl level would require each platform to leave the entire GeoJSON source as a monstrous string type.

Is this true? Wouldn't we build a classic C++ class system with analogs for GeoJSON types, just like we've built for vector tile types?

@1ec5
Copy link

1ec5 commented Aug 3, 2015

Wouldn't we build a classic C++ class system with analogs for GeoJSON types, just like we've built for vector tile types?

Yes – in fact, we already have most of the analogues already in mbgl, in the form of ShapeAnnotation et al. @incanus and I are merely trying to make clear that the iOS, Android, and Linux frontends to mbgl would not be able to benefit from mbgl supporting GeoJSON as a format for on-the-fly annotations. It’d be fine to make GeoJSON just another design-time source, like the existing vector or raster sources, but this would be a very minor use case for mbgl’s non-node frontends. So I think it’d be just as fine to implement this GeoJSON support in node-mapbox-gl-native, without making any changes to mbgl.

@tmpsantos
Copy link
Contributor

Blocking on mapbox/mapbox-gl-native#1675, so shapes might not show immediately on the map until it's moved, which probably won't happen via the Node bindings & static imagery.

I'm very sorry I didn't have time to look at this yet, but it could be workarounded by using map.setStyleJSON instead of map.setStyleURL.

@tmcw
Copy link

tmcw commented Aug 24, 2015

This repository is on the path to deprecation #151 - @mikemorris should we open an equivalent ticket over at mapbox-gl-native?

@mikemorris
Copy link
Contributor

This discussion of on-the-fly annotation versus design-time GeoJSON source support should be continued in https://github.com/mapbox/mapbox-gl-native

@tmcw
Copy link

tmcw commented Aug 24, 2015

Can you open a ticket there to connect the conversations?

@jfirebaugh
Copy link
Contributor

Opened mapbox/mapbox-gl-native#2161.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

8 participants