Refactor Client Data Source Interfaces (#2043)

* Fix onFeaturePick in Android demo

Property map is null when pick result is empty

* Refactor MapData and ClientGeoJsonSource to prevent repeated tiling operations

* Use CXX_STANDARD CMake property instead of explicit compiler flag

Setting '-std=c++14' as a public compile flag causes compile commands for all files to include that flag - this breaks compiling Obj-C (not Obj-C++) files on iOS

* Create builder structs for ClientGeoJsonSource to reduce data copies

* Update nativeAddFeature for geometry builders

* Update Android demo MainActivity for MapData changes

* Refactor TGMapData to reduce re-tiling and data copies

* Update demo MapViewController for TGMapData changes

* Simplify iOS MapData classes

* Consolidate TGMapFeature subclasses and add docs

* Apply some clang-tidy suggestions

* Fixup merge

* fixup: generate tiles after setting geojson (iOS)

Author: matteblair

core/CMakeLists.txt

@@ -277,10 +277,7 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
 endif()
 
 # C++14 is required for compiling tangram-core and for using the interface headers.
-# Making this option PUBLIC declares it as a requirement for targets linking against tangram-core.
-target_compile_options(tangram-core
-  PUBLIC -std=c++14
-)
+set_property(TARGET tangram-core PROPERTY CXX_STANDARD 14)
 
 # We include GLSL shader sources into the library by generating header files with the source text
 # printed into a string constant. A CMake script generates one of these headers for each shader source

core/include/tangram/data/clientGeoJsonSource.h

@@ -5,7 +5,6 @@
 
 #include <mutex>
 
-
 namespace Tangram {
 
 class Platform;
@@ -14,34 +13,61 @@ struct Properties;
 
 struct ClientGeoJsonData;
 
+struct PolylineBuilderData;
+
+struct PolylineBuilder {
+    PolylineBuilder();
+    ~PolylineBuilder();
+    void beginPolyline(size_t numberOfPoints);
+    void addPoint(LngLat point);
+    std::unique_ptr<PolylineBuilderData> data;
+};
+
+struct PolygonBuilderData;
+
+struct PolygonBuilder {
+    PolygonBuilder();
+    ~PolygonBuilder();
+    void beginPolygon(size_t numberOfRings);
+    void beginRing(size_t numberOfPoints);
+    void addPoint(LngLat point);
+    std::unique_ptr<PolygonBuilderData> data;
+};
+
 class ClientGeoJsonSource : public TileSource {
 
 public:
 
     ClientGeoJsonSource(Platform& _platform, const std::string& _name,
                         const std::string& _url, bool generateCentroids = false,
                         TileSource::ZoomOptions _zoomOptions = {});
-    ~ClientGeoJsonSource();
+
+    ~ClientGeoJsonSource() override;
 
     // http://www.iana.org/assignments/media-types/application/geo+json
-    virtual const char* mimeType() const override { return "application/geo+json"; };
+    const char* mimeType() const override { return "application/geo+json"; };
 
     // Add geometry from a GeoJSON string
     void addData(const std::string& _data);
-    void addPoint(const Properties& _tags, LngLat _point);
-    void addLine(const Properties& _tags, const Coordinates& _line);
-    void addPoly(const Properties& _tags, const std::vector<Coordinates>& _poly);
-    void generateLabelCentroidFeature();
 
-    virtual void loadTileData(std::shared_ptr<TileTask> _task, TileTaskCb _cb) override;
+    void addPointFeature(Properties&& properties, LngLat coordinates);
+
+    void addPolylineFeature(Properties&& properties, PolylineBuilder&& polyline);
+
+    void addPolygonFeature(Properties&& properties, PolygonBuilder&& polygon);
+
+    // Transform added feature data into tiles.
+    void generateTiles();
+
+    void loadTileData(std::shared_ptr<TileTask> _task, TileTaskCb _cb) override;
     std::shared_ptr<TileTask> createTask(TileID _tileId) override;
 
-    virtual void cancelLoadingTile(TileTask& _task) override {};
-    virtual void clearData() override;
+    void cancelLoadingTile(TileTask& _task) override {};
+    void clearData() override;
 
 protected:
 
-    virtual std::shared_ptr<TileData> parse(const TileTask& _task) const override;
+    std::shared_ptr<TileData> parse(const TileTask& _task) const override;
 
     std::unique_ptr<ClientGeoJsonData> m_store;
 

core/src/data/clientGeoJsonSource.cpp

@@ -36,15 +36,51 @@ geojsonvt::Options options() {
 
 struct ClientGeoJsonData {
     std::unique_ptr<geojsonvt::GeoJSONVT> tiles;
-    mapbox::geometry::feature_collection<double> features;
+    geometry::feature_collection<double> features;
     std::vector<Properties> properties;
 };
 
+struct PolylineBuilderData : mapbox::geometry::line_string<double> {};
+
+PolylineBuilder::PolylineBuilder() {
+    data = std::make_unique<PolylineBuilderData>();
+}
+
+PolylineBuilder::~PolylineBuilder() = default;
+
+void PolylineBuilder::beginPolyline(size_t numberOfPoints) {
+    data->reserve(numberOfPoints);
+}
+
+void PolylineBuilder::addPoint(Tangram::LngLat point) {
+    data->emplace_back(point.longitude, point.latitude);
+}
+
+struct PolygonBuilderData : mapbox::geometry::polygon<double> {};
+
+PolygonBuilder::PolygonBuilder() {
+    data = std::make_unique<PolygonBuilderData>();
+}
+
+PolygonBuilder::~PolygonBuilder() = default;
+
+void PolygonBuilder::beginPolygon(size_t numberOfRings) {
+    data->reserve(numberOfRings);
+}
+
+void PolygonBuilder::beginRing(size_t numberOfPoints) {
+    data->emplace_back();
+    data->back().reserve(numberOfPoints);
+}
+
+void PolygonBuilder::addPoint(LngLat point) {
+    data->back().emplace_back(point.longitude, point.latitude);
+}
+
 std::shared_ptr<TileTask> ClientGeoJsonSource::createTask(TileID _tileId) {
     return std::make_shared<TileTask>(_tileId, shared_from_this());
 }
 
-
 // TODO: pass scene's resourcePath to constructor to be used with `stringFromFile`
 ClientGeoJsonSource::ClientGeoJsonSource(Platform& _platform, const std::string& _name,
                                          const std::string& _url, bool _generateCentroids,
@@ -63,6 +99,7 @@ ClientGeoJsonSource::ClientGeoJsonSource(Platform& _platform, const std::string&
                 LOGE("Unable to retrieve data from '%s': %s", _url.c_str(), response.error);
             } else {
                 addData(std::string(response.content.begin(), response.content.end()));
+                generateTiles();
             }
             m_hasPendingData = false;
         };
@@ -72,7 +109,7 @@ ClientGeoJsonSource::ClientGeoJsonSource(Platform& _platform, const std::string&
 
 }
 
-ClientGeoJsonSource::~ClientGeoJsonSource() {}
+ClientGeoJsonSource::~ClientGeoJsonSource() = default;
 
 struct add_centroid {
 
@@ -81,10 +118,7 @@ struct add_centroid {
     bool operator()(const geometry::polygon<double>& geom) {
         if (geom.empty()) { return false; }
         pt = centroid(geom.front().begin(), geom.front().end()-1);
-        if (std::isnan(pt.x) || std::isnan(pt.y)) {
-            return false;
-        }
-        return true;
+        return !(std::isnan(pt.x) || std::isnan(pt.y));
     }
 
     bool operator()(const geometry::multi_polygon<double>& geom) {
@@ -115,7 +149,7 @@ struct prop_visitor {
     Properties& props;
     std::string& key;
     void operator()(std::string v) {
-        props.set(key, v);
+        props.set(key, std::move(v));
     }
     void operator()(bool v) {
         props.set(key, double(v));
@@ -136,47 +170,22 @@ struct prop_visitor {
     }
 };
 
-void ClientGeoJsonSource::generateLabelCentroidFeature() {
-    for (const auto &feat : m_store->features) {
-        geometry::point<double> centroid;
-        const auto& properties = m_store->properties[feat.id.get<uint64_t>()];
-        if (geometry::geometry<double>::visit(feat.geometry, add_centroid{ centroid })) {
-            uint64_t id = m_store->features.size();
-            m_store->features.emplace_back(centroid, id);
-            m_store->properties.push_back(properties);
-            auto& props = m_store->properties.back();
-            props.set("label_placement", 1.0);
-        }
-    }
-}
-
-void ClientGeoJsonSource::addData(const std::string& _data) {
+void ClientGeoJsonSource::generateTiles() {
 
     std::lock_guard<std::mutex> lock(m_mutexStore);
 
-    const auto json = geojson::parse(_data);
-    auto features = geojsonvt::geojson::visit(json, geojsonvt::ToFeatureCollection{});
-
-    for (auto& feature : features) {
-
-        feature.id = uint64_t(m_store->properties.size());
-        m_store->properties.emplace_back();
-        Properties& props = m_store->properties.back();
-
-        for (const auto& prop : feature.properties) {
-            auto key = prop.first;
-            prop_visitor visitor = {props, key};
-            mapbox::util::apply_visitor(visitor, prop.second);
-        }
-        feature.properties.clear();
-    }
-
-    m_store->features.insert(m_store->features.end(),
-                             std::make_move_iterator(features.begin()),
-                             std::make_move_iterator(features.end()));
-
     if (m_generateCentroids) {
-        generateLabelCentroidFeature();
+        for (const auto &feat : m_store->features) {
+            geometry::point<double> centroid;
+            const auto& properties = m_store->properties[feat.id.get<uint64_t>()];
+            if (geometry::geometry<double>::visit(feat.geometry, add_centroid{ centroid })) {
+                uint64_t id = m_store->features.size();
+                m_store->features.emplace_back(centroid, id);
+                m_store->properties.push_back(properties);
+                auto& props = m_store->properties.back();
+                props.set("label_placement", 1.0);
+            }
+        }
     }
 
     m_store->tiles = std::make_unique<geojsonvt::GeoJSONVT>(m_store->features, options());
@@ -210,65 +219,61 @@ void ClientGeoJsonSource::clearData() {
     m_generation++;
 }
 
-void ClientGeoJsonSource::addPoint(const Properties& _tags, LngLat _point) {
+void ClientGeoJsonSource::addData(const std::string& _data) {
 
     std::lock_guard<std::mutex> lock(m_mutexStore);
 
-    geometry::point<double> geom { _point.longitude, _point.latitude };
+    const auto json = geojson::parse(_data);
+    auto features = geojsonvt::geojson::visit(json, geojsonvt::ToFeatureCollection{});
 
-    uint64_t id = m_store->features.size();
+    for (auto& feature : features) {
 
-    m_store->features.emplace_back(geom, id);
-    m_store->properties.emplace_back(_tags);
+        feature.id = uint64_t(m_store->properties.size());
+        m_store->properties.emplace_back();
+        Properties& props = m_store->properties.back();
 
-    m_store->tiles = std::make_unique<geojsonvt::GeoJSONVT>(m_store->features, options());
-    m_generation++;
-}
+        for (const auto& prop : feature.properties) {
+            auto key = prop.first;
+            prop_visitor visitor = {props, key};
+            mapbox::util::apply_visitor(visitor, prop.second);
+        }
+        feature.properties.clear();
+    }
 
-void ClientGeoJsonSource::addLine(const Properties& _tags, const Coordinates& _line) {
+    m_store->features.insert(m_store->features.end(),
+                             std::make_move_iterator(features.begin()),
+                             std::make_move_iterator(features.end()));
+}
 
+void ClientGeoJsonSource::addPointFeature(Properties&& properties, LngLat coordinates) {
 
     std::lock_guard<std::mutex> lock(m_mutexStore);
 
-    geometry::line_string<double> geom;
-    for (auto& p : _line) {
-        geom.emplace_back(p.longitude, p.latitude);
-    }
+    geometry::point<double> geom {coordinates.longitude, coordinates.latitude};
 
     uint64_t id = m_store->features.size();
-
     m_store->features.emplace_back(geom, id);
-    m_store->properties.emplace_back(_tags);
-
-    m_store->tiles = std::make_unique<geojsonvt::GeoJSONVT>(m_store->features, options());
-    m_generation++;
+    m_store->properties.emplace_back(properties);
 }
 
-void ClientGeoJsonSource::addPoly(const Properties& _tags, const std::vector<Coordinates>& _poly) {
-
+void ClientGeoJsonSource::addPolylineFeature(Properties&& properties, PolylineBuilder&& polyline) {
 
     std::lock_guard<std::mutex> lock(m_mutexStore);
 
-    geometry::polygon<double> geom;
-    for (auto& ring : _poly) {
-        geom.emplace_back();
-        auto &line = geom.back();
-        for (auto& p : ring) {
-            line.emplace_back(p.longitude, p.latitude);
-        }
-    }
-
     uint64_t id = m_store->features.size();
+    auto geom = std::move(polyline.data);
+    m_store->features.emplace_back(*geom, id);
+    m_store->properties.emplace_back(properties);
+}
 
-    m_store->features.emplace_back(geom, id);
-    m_store->properties.emplace_back(_tags);
+void ClientGeoJsonSource::addPolygonFeature(Properties&& properties, PolygonBuilder&& polygon) {
 
-    if (m_generateCentroids) {
-        generateLabelCentroidFeature();
-    }
+    std::lock_guard<std::mutex> lock(m_mutexStore);
 
-    m_store->tiles = std::make_unique<geojsonvt::GeoJSONVT>(m_store->features, options());
-    m_generation++;
+    uint64_t id = m_store->features.size();
+    auto geom = std::move(polygon.data);
+    m_store->features.emplace_back(*geom, id);
+    m_store->properties.emplace_back(properties);
 }
 
 struct add_geometry {
@@ -356,7 +361,6 @@ std::shared_ptr<TileData> ClientGeoJsonSource::parse(const TileTask& _task) cons
         }
     }
 
-
     return data;
 }