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

shape annotations #1655

Closed
wants to merge 91 commits into from
Closed

shape annotations #1655

wants to merge 91 commits into from

Conversation

incanus
Copy link
Contributor

@incanus incanus commented May 27, 2015

Runtime shape annotations.

  • C++ API for adding single shape
  • C++ API for adding multiples at once
  • Runtime styling at annotation create time
  • Exposes StyleProperties et al. to Map users
  • C++ API for reordering
  • C++ API for removing
  • Cocoa API (including selecting only points)
  • Remove perf penalty when adding points and shapes at same time, since two invalidations occur
  • Track shapes across style changes (Switching styles turns annotations invisible #1488)
  • Bring in GeoJSONVT poly simplification - we could get away with punting for a bit here in exchange for performance were it not for problems rendering shapes that span tiles. I do feel good about the state of where I left the GeoJSONVT work and could probably pretty quickly get it into shape to farm out the tiling of shape features instead of the current stopgap naive tiling based on it.
  • Polygons with cutout holes
  • Shapes around antemeridian?
  • Cocoa API docs (currently blocking tests)

Problems/checkpoints:

  • Adding shape annotations before style is finished loading misses rendering them. This is because MapContext::updateAnnotationTiles() returns early. This doesn't affect points as their styling is all wrapped up in the style parsing step (since the variable parts are covered in the runtime {sprite} property), the conclusion of which performs a render. Worked around right now with a hack to add after a delay upon startup.
  • Threading sanity check.
  • General sanity check. This has been messy work.

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

mbgl::FillProperties fillProperties;
fillProperties.fill_color = mbgl::Color({{ 1, 0, 0, 1 }});
fillProperties.stroke_color = mbgl::Color({{ 0, 0, 0, 1 }});
fillProperties.opacity = 0.25;

mbgl::StyleProperties shapeProperties;
shapeProperties.set<mbgl::FillProperties>(fillProperties);

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

screen shot 2015-05-27 at 3 23 04 pm

@incanus incanus added feature in progress ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold labels May 27, 2015
@incanus incanus self-assigned this May 27, 2015
@incanus incanus added this to the iOS Beta 2 milestone May 27, 2015
@@ -150,6 +150,27 @@ int main() {
map.setStyleURL(newStyle.first);
view.setWindowTitle(newStyle.second);

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember to pull these changes out before merging.

Copy link
Contributor Author

@incanus incanus May 27, 2015 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@incanus incanus mentioned this pull request May 27, 2015
7 tasks
@incanus
Copy link
Contributor Author

incanus commented May 28, 2015

Things feel good for some real-world data, too:

screen shot 2015-05-28 at 2 49 47 pm

@incanus
Copy link
Contributor Author

incanus commented May 28, 2015

Bring in GeoJSONVT poly simplification - we could get away with punting for a bit here in exchange for performance were it not for problems rendering shapes that span tiles. I do feel good about the state of where I left the GeoJSONVT work and could probably pretty quickly get it into shape to farm out the tiling of shape features instead of the current stopgap naive tiling based on it.

This should actually be pretty easily fixed. In shape adds, I'm just overflowing the tile extent and assuming all shapes belong in only one tile, as a naive extension of the point add. That's why this happens:

anigif-1432856096

Fixing that up now as a staged approach, so that we don't necessarily need the GeoJSONVT simplification stuff in the first cut. This is a ~2,500 point polyline and it performs well now.

@incanus
Copy link
Contributor Author

incanus commented May 29, 2015

Taking a break from feature clipping, which is a bit of going in circles. I may just punt and bring in the GeoJSONVT work which takes care of this now instead of later, bringing along simplification for the ride. My reluctance to do so there is a bottleneck around the GeoJSONVT object vending tiles to to the annotation manager — maybe not a valid concern.

In the break I am going to rough out the Cocoa adding/removing/styling API since that part won't change and we are successfully using the direct C++ one right now.

@incanus
Copy link
Contributor Author

incanus commented May 29, 2015

BTW current impact of bad feature clipping is:

  • Lines: small gaps at tile borders.

    screen shot 2015-05-29 at 4 03 10 pm

  • Polys: disappear.

    screen shot 2015-05-29 at 4 03 20 pm

    screen shot 2015-05-29 at 4 03 23 pm


features.push_back(create(tags, projectedType, *geometry));

printf("features now has %lu items\n", (unsigned long)features.size());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prints to the console in Release mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@incanus
Copy link
Contributor Author

incanus commented Jun 11, 2015

Split GeoJSONVT out to https://github.com/mapbox/geojson-vt-cpp. Currently just dupe-copied here still; no formal build or dep process.

@interface MGLMultiPoint (MGLPolygon)

- (instancetype)initWithCoordinates:(CLLocationCoordinate2D *)coords count:(NSUInteger)count;
- (BOOL)intersectsOverlayBounds:(MGLCoordinateBounds)overlayBounds;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of a sneaky way of getting access to these methods. The declarations should go in a private header in platform/ios/.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1ec5 added a commit that referenced this pull request Jun 15, 2015
…ons"

This reverts commit 2435c1a (#1710), which needs to be revisited in light of #1655, which is a much higher priority at the moment.
@incanus
Copy link
Contributor Author

incanus commented Jun 16, 2015

336x199px-ll-ffcde97d_edge

@incanus incanus closed this Jun 16, 2015
@incanus incanus removed in progress ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold labels Jun 16, 2015
@jfirebaugh jfirebaugh deleted the shape-annotations branch June 19, 2015 00:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants