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

refs #893 #992: point annotations API #1018

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions include/mbgl/map/annotation.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
#ifndef MBGL_MAP_ANNOTATIONS
#define MBGL_MAP_ANNOTATIONS

#include <mbgl/map/map.hpp>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be a forward declaration instead of an include?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This leads to not seeing AnnotationSegment, and that can't be forward declared because it is used in getPoint(). That method can't be moved inside of annotation.cpp because then you get a redefinition error between the forward declaration of AnnotationSegment and the one brought into annotation.cpp with #include <mbgl/map/map.hpp>. This went really deep and doing it like this seemed the only way to keep things happy.

Copy link
Contributor

Choose a reason for hiding this comment

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

The underlying problem is the cyclic dependency between AnnotationManager and Map. It looks like you can break the cycle by removing Map& map from AnnotationManager and passing the max zoom to AnnotationManager::getAnnotationsInBounds and AnnotationManager::addPointAnnotations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is weird from an API point of view — clients like Cocoa would have to query, then pass the max zoom, when it doesn't appear to need to be an argument of querying bounds for annotations (they are independent of zoom and an implementation detail).

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought clients would use the API provided by Map?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohhhh, got it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#include <mbgl/map/tile.hpp>
#include <mbgl/map/live_tile.hpp>
#include <mbgl/util/geo.hpp>
#include <mbgl/util/noncopyable.hpp>
#include <mbgl/util/std.hpp>
#include <mbgl/util/vec.hpp>

#include <string>
#include <vector>
#include <map>
#include <mutex>
#include <memory>

namespace mbgl {

class Annotation;

enum class AnnotationType : uint8_t {
Point,
Shape
};

class AnnotationManager : private util::noncopyable {
public:
AnnotationManager(Map&);

void setDefaultPointAnnotationSymbol(std::string& symbol) { defaultPointAnnotationSymbol = symbol; }
std::pair<std::vector<Tile::ID>, std::vector<uint32_t>> addPointAnnotations(std::vector<LatLng>, std::vector<std::string>& symbols);
std::vector<Tile::ID> removeAnnotations(std::vector<uint32_t>);
std::vector<uint32_t> getAnnotationsInBoundingBox(BoundingBox) const;
BoundingBox getBoundingBoxForAnnotations(std::vector<uint32_t>) const;

const std::unique_ptr<LiveTile>& getTile(Tile::ID const& id);

private:
uint32_t nextID() { return nextID_++; }
static vec2<double> projectPoint(LatLng& point);

private:
std::mutex mtx;
Map& map;
std::string defaultPointAnnotationSymbol;
std::map<uint32_t, std::unique_ptr<Annotation>> annotations;
std::map<Tile::ID, std::pair<std::vector<uint32_t>, std::unique_ptr<LiveTile>>> annotationTiles;
std::unique_ptr<LiveTile> nullTile;
uint32_t nextID_ = 0;
};

class Annotation : private util::noncopyable {
public:
Annotation(AnnotationType, std::vector<AnnotationSegment>);

LatLng getPoint() const { return geometry[0][0]; }
BoundingBox getBoundingBox() const { return bbox; }

public:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do these need to be public?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

const AnnotationType type = AnnotationType::Point;
const std::vector<AnnotationSegment> geometry;
std::map<Tile::ID, std::vector<std::weak_ptr<const LiveTileFeature>>> tileFeatures;

private:
BoundingBox bbox;
};

}

#endif
17 changes: 16 additions & 1 deletion include/mbgl/map/map.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ class GlyphAtlas;
class SpriteAtlas;
class LineAtlas;
class Environment;
class AnnotationManager;

typedef std::vector<LatLng> AnnotationSegment;
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't appear to be used by this header; should move to annotation.hpp.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reasoning here is because the shapes API will have it; you pass vectors of AnnotationSegment objects to represent lines in a shape.

Copy link
Contributor Author

Choose a reason for hiding this comment

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


class Map : private util::noncopyable {
friend class View;
Expand Down Expand Up @@ -140,13 +143,23 @@ class Map : private util::noncopyable {
inline const vec2<double> pixelForLatLng(const LatLng latLng) const { return state.pixelForLatLng(latLng); }
inline const LatLng latLngForPixel(const vec2<double> pixel) const { return state.latLngForPixel(pixel); }

// Annotations
void setDefaultPointAnnotationSymbol(std::string&);
uint32_t addPointAnnotation(LatLng, std::string& symbol);
std::vector<uint32_t> addPointAnnotations(std::vector<LatLng>, std::vector<std::string>& symbols);
void removeAnnotation(uint32_t);
void removeAnnotations(std::vector<uint32_t>);
std::vector<uint32_t> getAnnotationsInBoundingBox(BoundingBox) const;
BoundingBox getBoundingBoxForAnnotations(std::vector<uint32_t>) const;

// Debug
void setDebug(bool value);
void toggleDebug();
bool getDebug() const;

inline const TransformState &getState() const { return state; }
inline std::chrono::steady_clock::time_point getTime() const { return animationTime; }
inline util::ptr<AnnotationManager> getAnnotationManager() const { return annotationManager; }
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't look like shared ownership should be necessary here; annotationManager is around for the lifetime of the Map, so return type can be AnnotationManager&.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That requires the member variable to move from util::ptr<AnnotationManager> to AnnotationManager, and that can't be forward declared, which then gets to the same cycle with map.hpp and annotation.hpp as in https://github.com/mapbox/mapbox-gl-native/pull/1018/files#r26625693.

Copy link
Contributor

Choose a reason for hiding this comment

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

You don't have to change the member variable:

inline AnnotationManager& getAnnotationManager() const { return *annotationManager; }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohhhh, perfect. Right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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


private:
// This may only be called by the View object.
Expand All @@ -170,6 +183,8 @@ class Map : private util::noncopyable {
// the stylesheet.
void prepare();

void updateAnnotationTiles(std::vector<Tile::ID>&);

enum class Mode : uint8_t {
None, // we're not doing any processing
Continuous, // continually updating map
Expand Down Expand Up @@ -219,8 +234,8 @@ class Map : private util::noncopyable {
util::ptr<Sprite> sprite;
const std::unique_ptr<LineAtlas> lineAtlas;
util::ptr<TexturePool> texturePool;

const std::unique_ptr<Painter> painter;
util::ptr<AnnotationManager> annotationManager;

std::string styleURL;
std::string styleJSON = "";
Expand Down
3 changes: 3 additions & 0 deletions include/mbgl/util/constants.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#define MBGL_UTIL_CONSTANTS

#include <cmath>
#include <string>

namespace mbgl {

Expand All @@ -15,6 +16,8 @@ extern const double M2PI;
extern const double EARTH_RADIUS_M;
extern const double LATITUDE_MAX;

extern const std::string ANNOTATIONS_POINTS_LAYER_ID;

}

namespace debug {
Expand Down
8 changes: 8 additions & 0 deletions include/mbgl/util/geo.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ struct ProjectedMeters {
: northing(n), easting(e) {}
};

struct BoundingBox {
Copy link
Contributor

Choose a reason for hiding this comment

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

LatLngBounds, to match gl-js.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LatLng sw = {90, 180};
LatLng ne = {-90, -180};

inline BoundingBox(LatLng sw_ = {90, 180}, LatLng ne_ = {-90, -180})
: sw(sw_), ne(ne_) {}
};

}

#endif
2 changes: 1 addition & 1 deletion macosx/mapboxgl-app.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
'product_extension': 'app',
'mac_bundle': 1,
'mac_bundle_resources': [
'Icon.icns',
'Icon.icns'
],

'dependencies': [
Expand Down
190 changes: 190 additions & 0 deletions src/mbgl/map/annotation.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
#include <mbgl/map/annotation.hpp>
#include <mbgl/util/ptr.hpp>

#include <algorithm>
#include <memory>

using namespace mbgl;

Annotation::Annotation(AnnotationType type_, std::vector<AnnotationSegment> geometry_)
: type(type_),
geometry(geometry_) {
if (type == AnnotationType::Point) {
bbox = BoundingBox(getPoint(), getPoint());
} else {
LatLng sw, ne;
for (auto segment : geometry) {
for (auto point : segment) {
if (point.latitude < sw.latitude) sw.latitude = point.latitude;
Copy link
Contributor

Choose a reason for hiding this comment

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

Extract this to BoundingBox::extend(const LatLng&).

Copy link
Contributor Author

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.

if (point.latitude > ne.latitude) ne.latitude = point.latitude;
if (point.longitude < sw.longitude) sw.longitude = point.longitude;
if (point.longitude > ne.longitude) ne.longitude = point.longitude;
}
}
bbox = BoundingBox(sw, ne);
}
}

AnnotationManager::AnnotationManager(Map& map_)
: map(map_),
nullTile(util::make_unique<LiveTile>()) {}

vec2<double> AnnotationManager::projectPoint(LatLng& point) {
double sine = std::sin(point.latitude * M_PI / 180);
double x = point.longitude / 360 + 0.5;
double y = 0.5 - 0.25 * std::log((1 + sine) / (1 - sine)) / M_PI;
return vec2<double>(x, y);
}

std::pair<std::vector<Tile::ID>, std::vector<uint32_t>> AnnotationManager::addPointAnnotations(std::vector<LatLng> points, std::vector<std::string>& symbols) {

uint16_t extent = 4096;

std::vector<uint32_t> annotationIDs(points.size());
std::vector<Tile::ID> affectedTiles;

for (uint32_t i = 0; i < points.size(); ++i) {
uint32_t annotationID = nextID();

auto anno_it = annotations.emplace(annotationID, util::make_unique<Annotation>(AnnotationType::Point, std::vector<AnnotationSegment>({{ points[i] }})));

uint8_t maxZoom = map.getMaxZoom();

uint32_t z2 = 1 << maxZoom;

vec2<double> p = projectPoint(points[i]);

uint32_t x = p.x * z2;
uint32_t y = p.y * z2;

for (int8_t z = maxZoom; z >= 0; z--) {
affectedTiles.emplace_back(z, x, y);
Tile::ID tileID = affectedTiles.back();

Coordinate coordinate(extent * (p.x * z2 - x), extent * (p.y * z2 - y));

GeometryCollection geometries({{ {{ coordinate }} }});

std::map<std::string, std::string> properties = {{ "sprite", (symbols[i].length() ? symbols[i] : defaultPointAnnotationSymbol) }};

auto feature = std::make_shared<const LiveTileFeature>(FeatureType::Point,
geometries,
properties);

auto tile_it = annotationTiles.find(tileID);
if (tile_it != annotationTiles.end()) {
// get point layer & add feature
auto layer = tile_it->second.second->getMutableLayer(util::ANNOTATIONS_POINTS_LAYER_ID);
layer->addFeature(feature);
// record annotation association with tile
tile_it->second.first.push_back(annotationID);
} else {
// create point layer & add feature
util::ptr<LiveTileLayer> layer = std::make_shared<LiveTileLayer>();
layer->addFeature(feature);
// create tile & record annotation association
auto tile_pos = annotationTiles.emplace(tileID, std::make_pair(std::vector<uint32_t>({ annotationID }), util::make_unique<LiveTile>()));
// add point layer to tile
tile_pos.first->second.second->addLayer(util::ANNOTATIONS_POINTS_LAYER_ID, layer);
}

// record annotation association with tile feature
anno_it.first->second->tileFeatures.emplace(tileID, std::vector<std::weak_ptr<const LiveTileFeature>>({ feature }));

z2 /= 2;
x /= 2;
y /= 2;
}

annotationIDs.push_back(annotationID);
}

return std::make_pair(affectedTiles, annotationIDs);
}

std::vector<Tile::ID> AnnotationManager::removeAnnotations(std::vector<uint32_t> ids) {
std::vector<Tile::ID> affectedTiles;

for (auto& annotationID : ids) {
auto annotation_it = annotations.find(annotationID);
if (annotation_it != annotations.end()) {
auto& annotation = annotation_it->second;
for (auto& tile_it : annotationTiles) {
auto features_it = annotation->tileFeatures.find(tile_it.first);
if (features_it != annotation->tileFeatures.end()) {
auto layer = tile_it.second.second->getMutableLayer(util::ANNOTATIONS_POINTS_LAYER_ID);
auto& features = features_it->second;
layer->removeFeature(features[0]);
affectedTiles.push_back(tile_it.first);
}
}
annotations.erase(annotationID);
}
}

return affectedTiles;
}

std::vector<uint32_t> AnnotationManager::getAnnotationsInBoundingBox(BoundingBox queryBbox) const {
uint8_t z = map.getMaxZoom();
uint32_t z2 = 1 << z;
vec2<double> swPoint = projectPoint(queryBbox.sw);
vec2<double> nePoint = projectPoint(queryBbox.ne);

// tiles number y from top down
Tile::ID nwTile(z, swPoint.x * z2, nePoint.y * z2);
Tile::ID seTile(z, nePoint.x * z2, swPoint.y * z2);

std::vector<uint32_t> matchingAnnotations;

for (auto& tile : annotationTiles) {
Tile::ID id = tile.first;
if (id.z == z) {
if (id.x >= nwTile.x && id.x <= seTile.x && id.y >= nwTile.y && id.y <= seTile.y) {
if (id.x > nwTile.x && id.x < seTile.x && id.y > nwTile.y && id.y < seTile.y) {
// trivial accept; grab all of the tile's annotations
std::copy(tile.second.first.begin(), tile.second.first.end(), std::back_inserter(matchingAnnotations));
} else {
// check tile's annotations' bounding boxes
std::copy_if(tile.second.first.begin(), tile.second.first.end(),
std::back_inserter(matchingAnnotations), [&](const uint32_t annotationID) -> bool {
printf("checking annotation %i\n", annotationID);
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this debug output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BoundingBox annoBbox = this->annotations.find(annotationID)->second->getBoundingBox();
return (annoBbox.sw.latitude >= queryBbox.sw.latitude &&
annoBbox.ne.latitude <= queryBbox.ne.latitude &&
annoBbox.sw.longitude >= queryBbox.sw.longitude &&
annoBbox.ne.longitude <= queryBbox.ne.longitude);
});
}
}
}
}

return matchingAnnotations;
}

BoundingBox AnnotationManager::getBoundingBoxForAnnotations(std::vector<uint32_t> ids) const {
LatLng sw, ne;
for (auto id : ids) {
auto annotation_it = annotations.find(id);
if (annotation_it != annotations.end()) {
auto bbox = annotation_it->second->getBoundingBox();
if (bbox.sw.latitude < sw.latitude) sw.latitude = bbox.sw.latitude;
Copy link
Contributor

Choose a reason for hiding this comment

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

Reuse BoundingBox::extend(const LatLng&) here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if (bbox.ne.latitude > ne.latitude) ne.latitude = bbox.ne.latitude;
if (bbox.sw.longitude < sw.longitude) sw.longitude = bbox.sw.longitude;
if (bbox.ne.longitude > ne.longitude) ne.longitude = bbox.ne.longitude;
}
}

return BoundingBox(sw, ne);
}

const std::unique_ptr<LiveTile>& AnnotationManager::getTile(Tile::ID const& id) {
std::lock_guard<std::mutex> lock(mtx);

auto tile_it = annotationTiles.find(id);
if (tile_it != annotationTiles.end()) {
return tile_it->second.second;
}
return nullTile;
}
10 changes: 5 additions & 5 deletions src/mbgl/map/geometry_tile.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,22 @@ enum class FeatureType : uint8_t {

typedef std::vector<std::vector<Coordinate>> GeometryCollection;

class GeometryTileFeature : public mbgl::util::noncopyable {
class GeometryTileFeature : private util::noncopyable {
public:
virtual FeatureType getType() const = 0;
virtual mapbox::util::optional<Value> getValue(const std::string& key) const = 0;
virtual GeometryCollection getGeometries() const = 0;
};

class GeometryTileLayer : public mbgl::util::noncopyable {
class GeometryTileLayer : private util::noncopyable {
public:
virtual std::size_t featureCount() const = 0;
virtual util::ptr<const GeometryTileFeature> feature(std::size_t i) const = 0;
virtual util::ptr<const GeometryTileFeature> feature(std::size_t) const = 0;
};

class GeometryTile : public mbgl::util::noncopyable {
class GeometryTile : private util::noncopyable {
public:
virtual util::ptr<const GeometryTileLayer> getLayer(const std::string&) const = 0;
virtual util::ptr<GeometryTileLayer> getLayer(const std::string&) const = 0;
};

class GeometryTileFeatureExtractor {
Expand Down
Loading