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

Further annotation refactoring #2673

Merged
merged 4 commits into from
Oct 20, 2015
Merged

Further annotation refactoring #2673

merged 4 commits into from
Oct 20, 2015

Conversation

jfirebaugh
Copy link
Contributor

👀 @kkaefer

@@ -285,9 +285,6 @@ - (void)commonInit

_annotationImages = [NSMutableDictionary dictionary];

std::string defaultSymbolName([MGLDefaultStyleMarkerSymbolName UTF8String]);
Copy link
Contributor

Choose a reason for hiding this comment

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

If MGLDefaultStyleMarkerSymbolName is no longer needed, it doesn’t need to be declared up top anymore.

/cc @incanus

@incanus
Copy link
Contributor

incanus commented Oct 19, 2015

Cursory review looks solid. Questions:

@jfirebaugh
Copy link
Contributor Author

I view getPointAnnotationsInBounds() as a stopgap, assuming that #2082 will be implemented via #352.

If it follows the gl-js implementation, #352 will indeed use an R-tree. But I think that's a separable change.

I wish the existing annotations code could use geojson-vt-cpp for both points and lines, and not use an R-tree at all. But because the API encourages adding points one by one, rather than as a single group/layer, it's difficult/impossible to make that efficient. (Another example of how the public API is limiting the implementation options.)

  • I think per past discussion we agreed to discuss any introductions of new Boost utilities (boost::geometry::index::rtree being the addition here). Just a point of order. So let's discuss 😄

We are already using boost::geometry::index::rtree in CollisionTile.

@incanus
Copy link
Contributor

incanus commented Oct 19, 2015

I wish the existing annotations code could use geojson-vt-cpp for both points and lines, and not use an R-tree at all. But because the API encourages adding points one by one, rather than as a single group/layer, it's difficult/impossible to make that efficient. (Another example of how the public API is limiting the implementation options.)

We'll always need the ability to add points one-by-one on mobile, but perhaps we could think about the styling / management on the backend in the style of what's proposed in #1708 as we start to consider ways to make our API do things other frameworks don't provide for due to our styling advantages.

👍 on everything else.

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.

3 participants