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

merge GeoJSON vector tiles work #893

Closed
6 of 7 tasks
incanus opened this issue Feb 17, 2015 · 59 comments
Closed
6 of 7 tasks

merge GeoJSON vector tiles work #893

incanus opened this issue Feb 17, 2015 · 59 comments
Assignees
Labels
Milestone

Comments

@incanus
Copy link
Contributor

incanus commented Feb 17, 2015

Picking up from #507 (comment), we'll need to merge the geojsonvt branch, which will require it interfacing better with the library.

  • match vector tile object structure with main library
  • allow for live reindexing when features are added/removed
  • handle non-GeoJSON (in-memory features)
  • nailing some hopefully-basic performance kinks
  • MultiGeometry support
  • other API merging (e.g. LatLng struct)
  • figuring out how independent of or reliant on GL we want the library to be

/cc @mourner @1ec5

@incanus incanus mentioned this issue Feb 17, 2015
5 tasks
@incanus incanus added this to the iOS Beta milestone Feb 17, 2015
@incanus
Copy link
Contributor Author

incanus commented Feb 17, 2015

The library consists of geojsonvt.hpp and geojsonvt.cpp.

@incanus
Copy link
Contributor Author

incanus commented Feb 18, 2015

@kkaefer @mourner Do you guys think this should be a part of GL itself, or should we try to keep it a separate library for general use of geo data -> vector tiles?

@mourner
Copy link
Member

mourner commented Feb 18, 2015

@incanus it would be nice to have it as a separate library — easier to maintain this way, with separate tests etc. It could also grow into a useful standalone tool.

@incanus
Copy link
Contributor Author

incanus commented Feb 19, 2015

Related to #904, we can't subclass Source to make e.g. a LiveSource (my first approach), so instead I am thinking:

  • Use Source, possibly with manually-created or passed SourceInfo related to the data (GeoJSON or other) setup.
  • Adapt Source::addTile() to handle SourceType:GeoJSON (currently unimplemented) as a first measure.
  • Above relies on a new GeoJSONTileData class which works similar to VectorTileData, but which pulls tiles from the GeoJSONVT structure.
  • A Source initialized in this way (perhaps with passed GeojSON) would async run the GeoJSONVT clipping & splitting algorithm upon creation so that it would be ready for these tile queries. This shouldn't take long, but maybe it will block addTile() calls until it's done?

@incanus
Copy link
Contributor Author

incanus commented Feb 19, 2015

FYI above I was speaking of GeoJSON only because that's what GeoJSONVT currently takes (a GeoJSON string) in its conversion, but ideally GL will go right at this conversion regardless of the format or of runtime-added features. We don't really care about GeoJSON file support right now in the iOS beta.

@incanus
Copy link
Contributor Author

incanus commented Feb 20, 2015

Current status: as of 988e64e, I have some semblance of an ever-present annotations layer atop the style layers, which generates tile coverage properly. Then, instead of kicking off async web loads for tile data, it queries a passed GeoJSONVT object. I can pan and zoom and observe the proper tiles being requested out of the object, including it auto-drilling down to slice up the GeoJSON.

Next steps, roughly:

  • Bridge the GeoJSONVT Tile and TileFeature format into VectorTileData that we can render.
  • Cut my sample GeoJSON (or any passed GeoJSON, for that matter) out for now.
  • Write an API to expose direct writes/conversions into Tile and TileFeature.

@incanus
Copy link
Contributor Author

incanus commented Feb 25, 2015

Still making slow progress here. Current structure is a LiveTileData peer to VectorTileData & RasterTileData, which will support programmatic annotation adding as well as (eventually) GeoJSON file retrieval/parsing. Just as VectorTileData passes to TileParser, LiveTileData passes to LiveTileParser, which will get refactored using large parts of TileParser such as render bucket creation, but first I'm just getting through the functionality.

@incanus
Copy link
Contributor Author

incanus commented Feb 25, 2015

As of 645aeda I'm getting things to render a bit.

anigif-1424899491

@incanus
Copy link
Contributor Author

incanus commented Feb 26, 2015

Next up is some careful looking into the GeoJSONVT library and my use of move/copy and references. I got to the bottom of an issue that was causing modification to throwaway stack variables and thus losing fidelity. So... back to the porting drawing board a bit for some cleanups, then:

  • Get all shapes to render on first view.
  • Figure out what's causing pieces of PA and all of NJ to not show up (some sort of collision thing or possibly actually losing data unintentionally during conversion).
  • Cut out the hardcoded GeoJSON load and go right at the GeoJSONVT conversion routines with live-added data.
  • Add APIs to stick stuff into said conversion routines.
  • Layer iOS API on top of that plus add API for currently hardcoded annotation styling.

That should keep me busy a while longer.

@incanus
Copy link
Contributor Author

incanus commented Feb 26, 2015

Get all shapes to render on first view.

As of d36caee this now works, at least within the limits of:

Figure out what's causing pieces of PA and all of NJ to not show up

That's next.

@incanus
Copy link
Contributor Author

incanus commented Feb 26, 2015

(some sort of collision thing or possibly actually losing data unintentionally during conversion)

It's the latter, as merely drawing background plus the data is no better.

screen shot 2015-02-26 at 11 34 07 am

@mb12
Copy link

mb12 commented Feb 26, 2015

1.) Collision detection happens only for Symbol Buckets (not lines or areas).

2.) If you want render something over all areas, lines and symbols, you would have to make sure that glDepthRange gets called with the right depth range. Painter uses a variable called strata for this. You may want to review your changes around this area to debug this.

@incanus
Copy link
Contributor Author

incanus commented Feb 26, 2015

Thanks @mb12, I am fresh to this part of the code since much of the collision work so this is a good pointer. Also I think I've determined my problems are with actual geometry conversion, but I'm still digging in.

I'm super interested in what you are up to with the code base — care to get in touch on my git email or my first name @mapbox.com?

@incanus
Copy link
Contributor Author

incanus commented Feb 27, 2015

Currently taking a side trip to work on symbols instead of merely lines, since this is a higher-priority item and everything thus far has been hardcoded for lines at the expense of symbols and fills.

Longer term, we'll probably have a lines/polygons annotations layer and another, separate points layer dynamically added to the style. On iOS at least, the two groups — points and line/polygon shapes — are each relegated to their own z-ordering, which we'll likely start with. Eventually we can open up more control in the API for intermingling the two, as well as interleaving their component annotations between other layers of the map.

That is, iOS model / first goal:

— top
-
- annotation points
- annotation shapes
- base map labels
- base map shapes
- background
-
— bottom

Eventual goal, for example:

— top
-
- annotation point A
- annotation shape A'
- base map labels
- annotation shape B'
- annotation point B
- base map shapes
- background
-
— bottom

@incanus
Copy link
Contributor Author

incanus commented Feb 27, 2015

As I've gotten into this, I've found that there's enough branching logic required to support both vector tiles as current and "live tiles" of annotation data that I'm thinking it makes more sense to adapt all of the live stuff for the vector tile construct.

Right now, vector tiles are (rightly) expected to be created from PBF data, and then the various parsing buckets (fill/line/symbol) are coupled to that format, parsing and interpreting actual vector tile geometry commands individually.

Instead of adding support for "manual" geometry to the buckets as I have been (where GeoJSONVT tiles give us "tiled vector data" in 4096 extents already), I'm thinking of:

  1. Adapting VectorTile and friends to provide their geometries to consumers, moving this step-by-step logic out of the parsing buckets.
  2. Allowing instantiating of VectorTile objects from GeoJSONVT Tile objects with TileFeatures, which also are 4096 extent-based geometries.
  3. Having the geometry routines above just relate annotation Tile geometries directly behind the same abstraction as for vector tiles.

This also moves towards a model of GeoJSON or live annotation features existing in multiple layers of one tile, which is how vector tiles work now. That way we can still apply filters to get FilteredVectorTileLayer for individual styling of annotations the same as our server data.

@incanus
Copy link
Contributor Author

incanus commented Feb 27, 2015

Spinning the tile refactoring out to #928 for the next step.

@incanus
Copy link
Contributor Author

incanus commented Mar 10, 2015

#928 is dead! Back to this issue now, interfacing GeoJSONVT with GeometryTile and friends.

@incanus
Copy link
Contributor Author

incanus commented Mar 11, 2015

As of e5d8b78 we have GeoJSON fetched from local file, parsed in C++, and added as live tiles to a non-hacky rendering infrastructure.

Next up: figuring out why labels get garbled and not all shapes show up initially.

Video: https://dl.dropboxusercontent.com/u/575564/garble.mp4

@incanus
Copy link
Contributor Author

incanus commented Mar 11, 2015

Backlog:

  • Garble fix
  • Get points to render, not just lines
  • Come up with dynamic style layer addition/rearrangement/removal system
    • Layer group within hardcoded style layers
    • Separate layers for each shape overlay (to allow relative (re)ordering
    • Common layer for all point annotations (akin to Maki handling) for proper z-ordering
  • API for adding/rearranging/removing annotation layers

@mb12
Copy link

mb12 commented Mar 11, 2015

The garbling in the posted video is similar to this issue.

#832

@incanus
Copy link
Contributor Author

incanus commented Mar 11, 2015

I've also seen similar garbling when multiple Vector Sources are specified in the same style.json file.

o_O

@mourner
Copy link
Member

mourner commented Mar 16, 2015

@incanus Something along the lines of https://github.com/mapbox/hey/issues/3712. We would not look at imagery, but try to reduce density for lower zoom tiles in places where points are too crowded, so that e.g. if we have 100,000 points, we don't have to render 100,000 on z0. Duplicate points filtering is not sufficient, because it's still potentially (4096 + 64 * 2) ^ 2 = 17.8 million points.

@ljbade
Copy link
Contributor

ljbade commented Mar 17, 2015

@mourner Yeah point filtering would be great for performance and allow a developer to throw lots of points at the map without having to worry about the performance impact.

@incanus I think you are on the right track with clearing tiles. You want to be able to say - these tiles are dirty, on next redraw, fetch new ones. That way only the tiles that need to be reloaded will be and it will happen as part of the normal update/render/swap flow.

@incanus
Copy link
Contributor Author

incanus commented Mar 17, 2015

Ok, point annotation removals now work as of e4da705. That hits everything up in #893 (comment), so I think we can say point features are complete.

Time now to clean, tidy up (e.g. fbd5511#commitcomment-10224463), and get a point annotations PR in.

@incanus
Copy link
Contributor Author

incanus commented Mar 17, 2015

Oops, forgot getAnnotationsInBoundingBox — doing that first.

@incanus
Copy link
Contributor Author

incanus commented Mar 17, 2015

Done in 3ce0fb9. Moving on to cleanup & separate PR for point API.

incanus added a commit that referenced this issue Mar 17, 2015
@incanus
Copy link
Contributor Author

incanus commented Mar 17, 2015

Point annotations PR → #1018

incanus added a commit that referenced this issue Mar 18, 2015
@incanus
Copy link
Contributor Author

incanus commented Mar 18, 2015

Moving this ticket to the b2 milestone, since the real GeoJSONVT integration involves shapes; points are standalone.

@incanus incanus modified the milestones: iOS Beta 2, iOS Beta 1 Mar 18, 2015
incanus added a commit that referenced this issue Mar 18, 2015
@incanus
Copy link
Contributor Author

incanus commented Mar 19, 2015

For b2, need to port over some improvements from @mourner like mapbox/geojson-vt@6f82954 when we get back to shapes.

@incanus
Copy link
Contributor Author

incanus commented Mar 22, 2015

Note to catch up with 2.0.0 in general when we're back on this:

https://github.com/mapbox/geojson-vt#200-mar-20-2015

@incanus
Copy link
Contributor Author

incanus commented May 20, 2015

Picking up shape annotation work again for b2 in shape-annotations.

@incanus
Copy link
Contributor Author

incanus commented May 22, 2015

map.addPointAnnotation(mbgl::LatLng(45, -120), "default_marker");

map.addShapeAnnotation({
    mbgl::LatLng(44, -122),
    mbgl::LatLng(46, -122),
    mbgl::LatLng(46, -121),
    mbgl::LatLng(44, -122)
});

screen shot 2015-05-22 at 4 31 46 pm

@picciano
Copy link

pretty sweet! does that work with lines as well as closed shapes?

@incanus
Copy link
Contributor Author

incanus commented May 22, 2015

Yep, though still working through a fair number of issues. It’s looking like the first cut will support either open shapes with a variable-width/color border, closed shapes with a fill color and an optional, but fixed-width, variable-color border, and possibly cutout holes in closed shapes.

@incanus
Copy link
Contributor Author

incanus commented May 26, 2015

Currently working on multiple layers, styling, and laying foundation for relative ordering.

Just to brain dump:

  • For points, we use one style layer which is added post-parse and contains all points when added/removed at runtime. Points are individually styled by passing their sprite image association in the {sprite} property, just as we do for map style POIs.

  • For shapes, each annotation will need its own layer in the style. This is for two reasons:

    1. Each shape can be independently styled (stroke, color, alpha, etc.).
    2. Shape layers can be ordered and reordered at runtime.

    Passing styling properties in the annotation proprieties like we do with points won't work as-is since we don't parse tokens out of properties the same way we do in the symbol bucket.

    This also means that we need to modify the style each time the client adds or removes a shape layer. I think this will render alright (i.e. not flicker the basemap) since we're only modifying and invalidating the annotation tiles (fast LiveTile objects with no network component).

/cc @jfirebaugh @kkaefer @ansis

@incanus
Copy link
Contributor Author

incanus commented May 26, 2015

Another approach, an alternative to each shape having a layer in the style, is to put all shapes in a single style layer like we do with points, make line and fill buckets parse out their properties for styling (modifying the StyleLayer on the fly), and punt relative ordering of shapes to however we will solve #988, i.e. probably do it in the shaders?

@jfirebaugh
Copy link
Contributor

make line and fill buckets parse out their properties for styling

That's a huge push, outlined in mapbox/mapbox-gl-style-spec#249.

@incanus
Copy link
Contributor Author

incanus commented May 26, 2015

I was seeing it in terms of annotation layers only, but I see how this should be made generic and how it plays with data-driven styles now. Thanks for the reminder.

@incanus
Copy link
Contributor Author

incanus commented May 27, 2015

Shape work PR coming into place: #1655

@incanus
Copy link
Contributor Author

incanus commented Jun 2, 2015

Shapes are just about set for b2, so I'm back looking into this ticket's branch (geojsonvt) for shape simplification.

@incanus
Copy link
Contributor Author

incanus commented Jun 8, 2015

#1655 supersedes this now that GeoJSONVT as the underpinning of shape annotations has landed there.

@incanus incanus closed this as completed Jun 8, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants