Add $meters_per_pixel keyword for filters and JS functions (#2144)

* Clean up StyleContext keyword logic and add $meters_per_pixel * Update tests using keywords and add case for $meters_per_pixel * Update benchmark for StyleContext changes
tangrams · Feb 8, 2020 · 78c2d2e · 78c2d2e
1 parent 2202740
commit 78c2d2e
Show file tree

Hide file tree

Showing 10 changed files with 100 additions and 123 deletions.
diff --git a/bench/src/benchStyleContext.cpp b/bench/src/benchStyleContext.cpp
@@ -104,7 +104,7 @@ struct JSTileStyleFnFixture : public benchmark::Fixture {
     void SetUp(const ::benchmark::State& state) override {
         globalSetup();
         ctx.initFunctions(*scene);
-        ctx.setKeywordZoom(10);
+        ctx.setZoom(10);
     }
     void TearDown(const ::benchmark::State& state) override {
         LOG(">>> %d", evalCnt);

diff --git a/core/src/marker/markerManager.cpp b/core/src/marker/markerManager.cpp
@@ -407,7 +407,7 @@ bool MarkerManager::buildMesh(Marker& marker, int zoom) {
     // Apply defaul draw rules defined for this style
     styler->style().applyDefaultDrawRules(*rule);
 
-    m_styleContext->setKeywordZoom(zoom);
+    m_styleContext->setZoom(zoom);
 
     bool valid = marker.evaluateRuleForContext(*m_styleContext);
 

diff --git a/core/src/scene/drawRule.cpp b/core/src/scene/drawRule.cpp
@@ -191,7 +191,7 @@ bool DrawRuleMergeSet::evaluateRuleForContext(DrawRule& rule, StyleContext& ctx)
             m_evaluated[i] = *param;
             param = &m_evaluated[i];
 
-            Stops::eval(*param->stops, param->key, ctx.getKeywordZoom(), m_evaluated[i].value);
+            Stops::eval(*param->stops, param->key, ctx.getZoom(), m_evaluated[i].value);
         }
     }
 

diff --git a/core/src/scene/filters.cpp b/core/src/scene/filters.cpp
@@ -8,6 +8,26 @@
 
 namespace Tangram {
 
+static const char* keywordGeometryString = "$geometry";
+static const char* keywordZoomString = "$zoom";
+static const char* keywordMetersPerPixelString = "$meters_per_pixel";
+
+FilterKeyword stringToFilterKeyword(const std::string& _key) {
+    if (_key == keywordGeometryString) { return FilterKeyword::geometry; }
+    if (_key == keywordZoomString) { return  FilterKeyword::zoom; }
+    if (_key == keywordMetersPerPixelString) { return FilterKeyword::meters_per_pixel; }
+    return  FilterKeyword::undefined;
+}
+
+std::string filterKeywordToString(FilterKeyword keyword) {
+    switch (keyword) {
+        case FilterKeyword::geometry: return keywordGeometryString;
+        case FilterKeyword::zoom: return keywordZoomString;
+        case FilterKeyword::meters_per_pixel: return keywordMetersPerPixelString;
+        default: return {};
+    }
+}
+
 void Filter::print(int _indent) const {
 
     switch (data.which()) {

diff --git a/core/src/scene/filters.h b/core/src/scene/filters.h
@@ -14,8 +14,13 @@ enum class FilterKeyword : uint8_t {
     undefined,
     zoom,
     geometry,
+    meters_per_pixel,
 };
 
+FilterKeyword stringToFilterKeyword(const std::string& _key);
+
+std::string filterKeywordToString(FilterKeyword keyword);
+
 struct Filter {
     struct OperatorAll {
         std::vector<Filter> operands;
@@ -83,14 +88,14 @@ struct Filter {
     // Create an 'equality' filter
     inline static Filter MatchEquality(const std::string& k, const std::vector<Value>& vals) {
         if (vals.size() == 1) {
-            return { Equality{ k, vals[0], keywordType(k) }};
+            return { Equality{k, vals[0], stringToFilterKeyword(k) }};
         } else {
-            return { EqualitySet{ k, vals, keywordType(k) }};
+            return { EqualitySet{k, vals, stringToFilterKeyword(k) }};
         }
     }
     // Create a 'range' filter
     inline static Filter MatchRange(const std::string& k, float min, float max, bool sqA) {
-        return { Range{ k, min, max, keywordType(k), sqA }};
+        return { Range{k, min, max, stringToFilterKeyword(k), sqA }};
     }
     // Create an 'existence' filter
     inline static Filter MatchExistence(const std::string& k, bool ex) {
@@ -101,15 +106,6 @@ struct Filter {
         return { Function{ id }};
     }
 
-    static FilterKeyword keywordType(const std::string& _key) {
-        if (_key == "$geometry") {
-            return FilterKeyword::geometry;
-        } else if (_key == "$zoom") {
-            return  FilterKeyword::zoom;
-        }
-        return  FilterKeyword::undefined;
-    }
-
     /* Public for testing */
     static void sort(std::vector<Filter>& filters);
     void print(int _indent = 0) const;

diff --git a/core/src/scene/styleContext.cpp b/core/src/scene/styleContext.cpp
@@ -13,9 +13,6 @@
 
 namespace Tangram {
 
-static const std::string key_geom("$geometry");
-static const std::string key_zoom("$zoom");
-
 static const std::vector<std::string> s_geometryStrings = {
     "", // unknown
     "point",
@@ -125,62 +122,54 @@ void StyleContext::setFeature(const Feature& _feature) {
 
     m_feature = &_feature;
 
-    if (m_keywordGeom != m_feature->geometryType) {
-        setKeyword(key_geom, s_geometryStrings[m_feature->geometryType]);
-        m_keywordGeom = m_feature->geometryType;
+    if (m_keywordGeometry != m_feature->geometryType) {
+        setKeyword(FilterKeyword::geometry, s_geometryStrings[m_feature->geometryType]);
+        m_keywordGeometry = m_feature->geometryType;
     }
 
     m_jsContext->setCurrentFeature(&_feature);
 }
 
-void StyleContext::setKeywordZoom(int _zoom) {
-    if (m_keywordZoom != _zoom) {
-        setKeyword(key_zoom, _zoom);
-        m_keywordZoom = _zoom;
+void StyleContext::setZoom(double zoom) {
+    if (m_zoom != zoom) {
+        setKeyword(FilterKeyword::zoom, zoom);
+        m_zoom = zoom;
+        // When new zoom is set, meters_per_pixel must be updated too.
+        double meters_per_pixel = MapProjection::metersPerPixelAtZoom(m_zoom);
+        setKeyword(FilterKeyword::meters_per_pixel, meters_per_pixel);
     }
 }
 
-void StyleContext::setKeyword(const std::string& _key, Value _val) {
-    auto keywordKey = Filter::keywordType(_key);
-    if (keywordKey == FilterKeyword::undefined) {
-        LOG("Undefined Keyword: %s", _key.c_str());
+void StyleContext::setKeyword(FilterKeyword keyword, Value value) {
+
+    Value& entry = m_keywordValues[static_cast<uint8_t>(keyword)];
+    if (entry == value) {
         return;
     }
 
-    // Unset shortcuts in case setKeyword was not called by
-    // the helper functions above.
-    if (_key == key_zoom) { m_keywordZoom = -1; }
-    if (_key == key_geom) { m_keywordGeom = -1; }
-
-    Value& entry = m_keywords[static_cast<uint8_t>(keywordKey)];
-    if (entry == _val) { return; }
+    const std::string& keywordString = filterKeywordToString(keyword);
 
     {
         JSScope jsScope(*m_jsContext);
-        JSValue value;
-        if (_val.is<std::string>()) {
-            value = jsScope.newString(_val.get<std::string>());
-        } else if (_val.is<double>()) {
-            value = jsScope.newNumber(_val.get<double>());
+        JSValue jsValue;
+        if (value.is<std::string>()) {
+            jsValue = jsScope.newString(value.get<std::string>());
+        } else if (value.is<double>()) {
+            jsValue = jsScope.newNumber(value.get<double>());
         }
-        m_jsContext->setGlobalValue(_key, std::move(value));
+        m_jsContext->setGlobalValue(keywordString, std::move(jsValue));
     }
 
-
-    entry = std::move(_val);
+    entry = std::move(value);
 }
 
-float StyleContext::getPixelAreaScale() {
+double StyleContext::getPixelAreaScale() {
     // scale the filter value with pixelsPerMeter
     // used with `px2` area filtering
-    double metersPerPixel = MapProjection::EARTH_CIRCUMFERENCE_METERS * exp2(-m_keywordZoom) / MapProjection::tileSize();
+    double metersPerPixel = MapProjection::metersPerPixelAtZoom(m_zoom);
     return metersPerPixel * metersPerPixel;
 }
 
-const Value& StyleContext::getKeyword(const std::string& _key) const {
-    return getKeyword(Filter::keywordType(_key));
-}
-
 void StyleContext::clear() {
     m_jsContext->setCurrentFeature(nullptr);
 }

diff --git a/core/src/scene/styleContext.h b/core/src/scene/styleContext.h
@@ -2,13 +2,10 @@
 
 #include "js/JavaScriptFwd.h"
 #include "scene/styleParam.h"
-#include "util/fastmap.h"
 
 #include <array>
-#include <functional>
 #include <memory>
 #include <string>
-#include <unordered_map>
 
 namespace YAML {
     class Node;
@@ -34,54 +31,50 @@ class StyleContext {
 
     ~StyleContext();
 
-    /*
-     * Set currently processed Feature
-     */
-    void setFeature(const Feature& _feature);
+    /// Set current Feature being evaluated.
+    void setFeature(const Feature& feature);
 
-    /*
-     * Set keyword for currently processed Tile
-     */
-    void setKeywordZoom(int _zoom);
+    /// Set current zoom level being evaluated.
+    void setZoom(double zoom);
 
-    /* Called from Filter::eval */
-    float getKeywordZoom() const { return m_keywordZoom; }
+    double getZoom() const {
+        return m_zoom;
+    }
 
-    /* returns meters per pixels at current style zoom */
-    float getPixelAreaScale();
+    /// Squared meters per pixels at current zoom.
+    double getPixelAreaScale();
 
-    const Value& getKeyword(FilterKeyword _key) const {
-        return m_keywords[static_cast<uint8_t>(_key)];
+    const Value& getKeyword(FilterKeyword keyword) const {
+        return m_keywordValues[static_cast<uint8_t>(keyword)];
     }
 
-    /* Called from Filter::eval */
+    /// Called from Filter::eval
     bool evalFilter(FunctionID id);
 
-    /* Called from DrawRule::eval */
-    bool evalStyle(FunctionID id, StyleParamKey _key, StyleParam::Value& _val);
+    /// Called from DrawRule::eval
+    bool evalStyle(FunctionID id, StyleParamKey key, StyleParam::Value& value);
 
-    /*
-     * Setup filter and style functions from @_scene
-     */
-    void initFunctions(const Scene& _scene);
+    /// Setup filter and style functions from a Scene.
+    void initFunctions(const Scene& scene);
 
-    /*
-     * Unset Feature handle
-     */
+    /// Unset the current Feature.
     void clear();
 
-    bool setFunctions(const std::vector<std::string>& _functions);
-    bool addFunction(const std::string& _function);
+    bool setFunctions(const std::vector<std::string>& functions);
+    bool addFunction(const std::string& function);
     void setSceneGlobals(const YAML::Node& sceneGlobals);
 
-    void setKeyword(const std::string& _key, Value _value);
-    const Value& getKeyword(const std::string& _key) const;
-
 private:
 
-    std::array<Value, 4> m_keywords;
-    int m_keywordGeom= -1;
-    int m_keywordZoom = -1;
+    void setKeyword(FilterKeyword keyword, Value value);
+
+    std::array<Value, 4> m_keywordValues;
+
+    // Cache zoom separately from keywords for easier access.
+    double m_zoom = -1;
+
+    // Geometry keyword is accessed as a string, but internally cached as an int.
+    int m_keywordGeometry = -1;
 
     int m_functionCount = 0;
 

diff --git a/core/src/tile/tileBuilder.cpp b/core/src/tile/tileBuilder.cpp
@@ -110,7 +110,7 @@ std::unique_ptr<Tile> TileBuilder::build(TileID _tileID, const TileData& _tileDa
 
     tile->initGeometry(int(m_scene.styles().size()));
 
-    m_styleContext->setKeywordZoom(_tileID.s);
+    m_styleContext->setZoom(_tileID.s);
 
     for (auto& builder : m_styleBuilder) {
         if (builder.second) { builder.second->setup(*tile); }

diff --git a/tests/unit/dukTests.cpp b/tests/unit/dukTests.cpp
@@ -44,14 +44,25 @@ TEST_CASE( "Test evalFilterFn with feature and keywords", "[Duktape][evalFilterF
 
     StyleContext ctx;
     ctx.setFeature(feature);
-    ctx.setKeyword("$zoom", 5);
+    ctx.setZoom(5);
 
     REQUIRE(ctx.setFunctions({ R"(function() { return (feature.scalerank * .5) <= ($zoom - 4); })"}));
     REQUIRE(ctx.evalFilter(0) == true);
 
-    ctx.setKeyword("$zoom", 4);
+    ctx.setZoom(4);
+    REQUIRE(ctx.evalFilter(0) == false);
+}
+
+TEST_CASE( "Test $meters_per_pixel keyword in JS function", "[Duktape]") {
+    StyleContext ctx;
+
+    REQUIRE(ctx.setFunctions({ R"(function() { return $meters_per_pixel <= 100; })"}));
+
+    ctx.setZoom(10); // $meters_per_pixel should be 152.9
     REQUIRE(ctx.evalFilter(0) == false);
 
+    ctx.setZoom(11); // $meters_per_pixel should be 76.4
+    REQUIRE(ctx.evalFilter(0) == true);
 }
 
 TEST_CASE( "Test evalFilterFn with feature and keyword geometry", "[Duktape][evalFilterFn]") {
@@ -108,27 +119,6 @@ TEST_CASE( "Test evalFilterFn with different features", "[Duktape][evalFilterFn]
     REQUIRE(ctx.evalFilter(0) == true);
 }
 
-TEST_CASE( "Test numeric keyword", "[Duktape][setKeyword]") {
-    StyleContext ctx;
-    ctx.setKeyword("$zoom", 10);
-    REQUIRE(ctx.setFunctions({ R"(function() { return $zoom === 10 })"}));
-    REQUIRE(ctx.evalFilter(0) == true);
-
-    ctx.setKeyword("$zoom", 0);
-    REQUIRE(ctx.evalFilter(0) == false);
-}
-
-TEST_CASE( "Test string keyword", "[Duktape][setKeyword]") {
-    StyleContext ctx;
-    ctx.setKeyword("$geometry", GeometryType::points);
-    REQUIRE(ctx.setFunctions({ R"(function() { return $geometry === point })"}));
-    REQUIRE(ctx.evalFilter(0) == true);
-
-    ctx.setKeyword("$geometry", "none");
-    REQUIRE(ctx.evalFilter(0) == false);
-
-}
-
 TEST_CASE( "Test evalStyleFn - StyleParamKey::order", "[Duktape][evalStyleFn]") {
     Feature feat;
     feat.props.set("sort_key", 2);