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

flesh out annotation add/remove client API #992

Closed
incanus opened this issue Mar 12, 2015 · 14 comments
Closed

flesh out annotation add/remove client API #992

incanus opened this issue Mar 12, 2015 · 14 comments
Assignees
Milestone

Comments

@incanus
Copy link
Contributor

incanus commented Mar 12, 2015

This is the API where platforms (i.e. iOS MGLMapView internals) talk to core for manipulating point and shape annotations.

Points

  • Set default point annotation symbol name (so system falls back to e.g. a red filled Mapbox marker)
  • Add single point annotation as LatLng with optional symbol name (from style sprite sheet) upfront
  • Add multiples of same (more performant than individual adds in a loop)
  • Remove single point annotation
  • Remove multiples (more performant than individual removes in a loop)
  • Tell single point annotation to move to top in z-order (e.g. selection state)

Shapes

  • Add single shape annotation as std::vector<LatLng> and draw properties (stroke width, stroke color, fill color) upfront. Need a way to specify interior polygon holes.
  • Add multiples of same
  • Remove single shape annotation
  • Remove multiples

Both

  • Obtaining annotations in a given LatLng bounding box (enables e.g. -annotationsInMapRect:)
  • Obtaining bounding box fully encompassing all currently-added annotations (enables e.g. annotationVisibleRect)
  • Neither of these take symbology into account, just LatLng, meaning zooming to exactly a bounding box might have a marker partway off the edge of the screen.

TBD for iOS b1

  • Allow explicit relative ordering of shape annotations within shape annotation layer (e.g. -insertOverlay:aboveOverlay: or -insertOverlay:atIndex:)
  • Query with callback for updated symbol name each time point annotation becomes visible again, to give opportunity to update?
  • Query with callback for updated stroke/fill properties in same way?

iOS b2 or beyond

  • Interlacing of shape overlays with roads & labels base map layers (or other arbitrary layers). This is like MapKit's -addOverlay:level: with values like MKOverlayLevelAboveRoads or MKOverlayLevelAboveLabels. In our case, this would more likely be a style layer name passed as a string. Also enables e.g. -overlaysInLevel:.

/cc @1ec5

@incanus incanus self-assigned this Mar 12, 2015
@incanus incanus added this to the iOS Beta 1 milestone Mar 12, 2015
@1ec5
Copy link
Contributor

1ec5 commented Mar 12, 2015

The plan is to have MGLMapView track MGLAnnotationViews in a dictionary, similar to what I had before 9c46c6c, but using string UUIDs as keys instead of MGLAnnotations. That’ll allow us to support multiple annotations at the same location. Methods such as -addAnnotation: and -addAnnotations: are well positioned for the C++ API @incanus is creating.

@incanus
Copy link
Contributor Author

incanus commented Mar 12, 2015

Some brain-dumping on the behind-the-scenes stuff that needs to happen for annotation management.

Storage

  • We hold a map of uuid to Annotation object
    • Better than making uuid a member of Annotation because we'd otherwise frequently have to std::find on uuid
    • Annotation class
      • AnnotationType (Point or Shape)
      • AnnotationGeometry (std::vector<std::vector<Coordinate>>)
        • Point just has one point inside this (possibly add convenience getter)
      • ProjectedFeature takes care of projection math upfront to convert to unit space (do we need this?)
  • Unified std::unordered_map<Tile::ID, LiveTile>
    • Multiple LiveTileLayers in each tile
    • Last/top one is for all point annotations
    • Others below are for one shape per layer
      • Represents shape stacking order in render
  • Maintain min/max LatLng for all annotations, updating at add/remove time
    • Enables annotationVisibleRect
  • Annotation-in-region searching
    • Descend unified map to determine highest-numbered zoom level containing tiles that intersect
    • Iterate those tiles
      • Completely contained: all annotations match
      • Partially contained: iterate annotations and compare LatLng
      • Maybe this is naïve

Addition process

  • Add annotation(s)
    • Creates Annotation(s)
    • Generates tiles for feature(s)
    • Stores std::vector<Tile::ID> in annotation for future cleanup & querying
  • Iterate current tiles and copy feature(s) from new tiles
    • Generate new tiles where necessary
    • Reference each LiveTileFeature in the Annotation object by Tile::ID map
    • Could order iteration based on proximity either side of current viewport zoom
  • Possible future optimization: LRU cache cleaning
    • Would require keeping around affected Tile::ID and using list to intersect viewport and possibly recreate removed tiles
    • Once Tile::ID is added to unified map, it's never removed, just has its LiveTile nulled out. Then we always know the intersections with the viewport that may need a tile re-request in future
  • Ping for map update() (deferred for multiple addition)

Removal process

  • Get passed a uuid
  • Find Annotation object in map
  • Pull referenced Tile::ID list
    • Iterate each tile
    • If feature count is one, destroy tile
    • Else remove appropriate LiveTileFeature
  • Ping for map update() (deferred for multiple removal)

@incanus
Copy link
Contributor Author

incanus commented Mar 13, 2015

Misunderstanding of annotationVisibleRect above, which is not the rect containing all annotations, but rather is used for viewport animations:

http://stackoverflow.com/questions/8882231/mkmapview-zoom-to-annotations-annotationvisiblerect

The original intention isn't supported directly in MapKit:

Obtaining bounding box fully encompassing all currently-added annotations

It can only zoom to those:

mapView.showAnnotations(mapView.annotations, animated: true)

So we should add -showAnnotations:animated:, as well as possibly exposing the intermediary -boundingBoxForAnnotations: that we will have to create.

@incanus
Copy link
Contributor Author

incanus commented Mar 13, 2015

Annotations client API coming together as:

typedef const std::string AnnotationID;

typedef const std::vector<const LatLng> AnnotationSegment;

struct BoundingBox {
    LatLng sw = {0, 0};
    LatLng ne = {0, 0};
};

void setDefaultPointAnnotationSymbol(const std::string);

AnnotationID addPointAnnotation(const LatLng, const std::string symbol = "");
const std::vector<AnnotationID> addPointAnnotations(const std::vector<LatLng>, const std::vector<const std::string> symbols = {{}});

void movePointAnnotationToFront(AnnotationID);
void reorderPointAnnotations();

AnnotationID addShapeAnnotation(const std::vector<AnnotationSegment>);
const std::vector<AnnotationID> addShapeAnnotations(const std::vector<const std::vector<AnnotationSegment>>);

void removeAnnotation(AnnotationID);
void removeAnnotations(const std::vector<AnnotationID>);

const std::vector<AnnotationID> getAnnotationsInBoundingBox(BoundingBox) const;
const BoundingBox getBoundingBoxForAnnotations(const std::vector<AnnotationID>) const;

@friedbunny
Copy link
Contributor

What will reorderPointAnnotations() do?

@incanus
Copy link
Contributor Author

incanus commented Mar 13, 2015

The idea there is the reversal of movePointAnnotationToFront(), to "resettle" them back into their default order per #988.

@incanus
Copy link
Contributor Author

incanus commented Mar 13, 2015

Kind of a way to separate z-ordering from some sort of selection state, because platforms might expect single or multiple state selection, but we should leave it up to them what this means for z-ordering.

@friedbunny
Copy link
Contributor

Ah, I see — the name seems slightly unintuitive to me, but beyond tossing in "zIndex" someplace (meh) I don't have any suggestions.

Would an individual reorderPointAnnotation(AnnotationID) be too finicky? Aside from the name, anyway.

@friedbunny
Copy link
Contributor

I guess you'd be getting down to marker z-indices at that point, which I can't really think of pressing use case for.

@incanus
Copy link
Contributor Author

incanus commented Mar 13, 2015

@1ec5 This is in now, at least stubbed out. We'll drop the z-indexing related stuff for now per #991 (comment) so this takes care of adds, removes, regions, and basic symbology.

// Annotations
void setDefaultPointAnnotationSymbol(const std::string&);
uint64_t addPointAnnotation(const LatLng, const std::string& symbol = "");
std::vector<const uint64_t> addPointAnnotations(const std::vector<LatLng>, const std::vector<const std::string>& symbols = {{}});
uint64_t addShapeAnnotation(const std::vector<AnnotationSegment>);
std::vector<const uint64_t> addShapeAnnotations(const std::vector<const std::vector<AnnotationSegment>>);
void removeAnnotation(const uint64_t);
void removeAnnotations(const std::vector<const uint64_t>);
std::vector<const uint64_t> getAnnotationsInBoundingBox(BoundingBox) const;
BoundingBox getBoundingBoxForAnnotations(const std::vector<const uint64_t>) const;

@incanus
Copy link
Contributor Author

incanus commented Mar 13, 2015

BTW moving to uint64_t tokens for the annotation IDs.

@incanus
Copy link
Contributor Author

incanus commented Mar 16, 2015

Work on this is basically wrapped into #893, but is progressing. We have point annotation addition and dynamic symbology.

@incanus
Copy link
Contributor Author

incanus commented Mar 17, 2015

Ok, point annotation add, remove, dynamic symbology, and bbox querying is in as of 3ce0fb9. The point API remains as described above, other than returned ID's actually being uint32_t. So I'm going to move this ticket over to the b2 milestone for future shape work.

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

incanus commented Jun 1, 2015

We've got all the critical stuff here captured elsewhere and/or the API is all built out in existing points and soon-to-lands shapes (#1655).

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

No branches or pull requests

3 participants