Add support for expression test

Fix polygon within algorithm
Add Unit tests

Fix incorrect metrics folder for ios-render-test-runner job

expression-test/expression_test_parser.cpp

@@ -14,6 +14,8 @@
 
 #include <args.hxx>
 
+#include <regex>
+
 using namespace mbgl;
 using namespace mbgl::style;
 using namespace mbgl::style::conversion;
@@ -254,6 +256,19 @@ bool parseInputs(const JSValue& inputsValue, TestData& data) {
             heatmapDensity = evaluationContext["heatmapDensity"].GetDouble();
         }
 
+        // Parse canonicalID
+        optional<CanonicalTileID> canonical;
+        if (evaluationContext.HasMember("canonicalID")) {
+            assert(evaluationContext["canonicalID"].IsObject());
+            assert(
+                evaluationContext["canonicalID"].HasMember("z") && evaluationContext["canonicalID"]["z"].IsNumber() &&
+                evaluationContext["canonicalID"].HasMember("x") && evaluationContext["canonicalID"]["x"].IsNumber() &&
+                evaluationContext["canonicalID"].HasMember("y") && evaluationContext["canonicalID"]["y"].IsNumber());
+            canonical = CanonicalTileID(evaluationContext["canonicalID"]["z"].GetUint(),
+                                        evaluationContext["canonicalID"]["x"].GetUint(),
+                                        evaluationContext["canonicalID"]["y"].GetUint());
+        }
+
         // Parse availableImages
         std::set<std::string> availableImages;
         if (evaluationContext.HasMember("availableImages")) {
@@ -282,8 +297,11 @@ bool parseInputs(const JSValue& inputsValue, TestData& data) {
             feature.id = mapbox::geojson::convert<mapbox::feature::identifier>(featureObject["id"]);
         }
 
-        data.inputs.emplace_back(
-            std::move(zoom), std::move(heatmapDensity), std::move(availableImages), std::move(feature));
+        data.inputs.emplace_back(std::move(zoom),
+                                 std::move(heatmapDensity),
+                                 std::move(canonical),
+                                 std::move(availableImages),
+                                 std::move(feature));
     }
     return true;
 }
@@ -294,11 +312,11 @@ std::tuple<filesystem::path, std::vector<filesystem::path>, bool, uint32_t> pars
     args::ArgumentParser argumentParser("Mapbox GL Expression Test Runner");
 
     args::HelpFlag helpFlag(argumentParser, "help", "Display this help menu", { 'h', "help" });
-    args::Flag shuffleFlag(argumentParser, "shuffle", "Toggle shuffling the tests order",
-                           { 's', "shuffle" });
+    args::Flag shuffleFlag(argumentParser, "shuffle", "Toggle shuffling the tests order", {'s', "shuffle"});
     args::ValueFlag<uint32_t> seedValue(argumentParser, "seed", "Shuffle seed (default: random)",
                                         { "seed" });
     args::PositionalList<std::string> testNameValues(argumentParser, "URL", "Test name(s)");
+    args::ValueFlag<std::string> testFilterValue(argumentParser, "filter", "Test filter regex", {'f', "filter"});
 
     try {
         argumentParser.ParseCLI(argc, argv);
@@ -336,6 +354,7 @@ std::tuple<filesystem::path, std::vector<filesystem::path>, bool, uint32_t> pars
         paths.emplace_back(rootPath);
     }
 
+    auto testFilter = testFilterValue ? args::get(testFilterValue) : std::string{};
     // Recursively traverse through the test paths and collect test directories containing "test.json".
     std::vector<filesystem::path> testPaths;
     testPaths.reserve(paths.size());
@@ -346,6 +365,9 @@ std::tuple<filesystem::path, std::vector<filesystem::path>, bool, uint32_t> pars
         }
 
         for (auto& testPath : filesystem::recursive_directory_iterator(path)) {
+            if (!testFilter.empty() && !std::regex_search(testPath.path().string(), std::regex(testFilter))) {
+                continue;
+            }
             if (testPath.path().filename() == "test.json") {
                 testPaths.emplace_back(testPath.path());
             }

expression-test/expression_test_parser.hpp

@@ -16,14 +16,17 @@ using namespace mbgl;
 struct Input {
     Input(optional<float> zoom_,
           optional<double> heatmapDensity_,
+          optional<CanonicalTileID> canonical_,
           std::set<std::string> availableImages_,
           Feature feature_)
         : zoom(std::move(zoom_)),
           heatmapDensity(std::move(heatmapDensity_)),
+          canonical(std::move(canonical_)),
           availableImages(std::move(availableImages_)),
           feature(std::move(feature_)) {}
     optional<float> zoom;
     optional<double> heatmapDensity;
+    optional<CanonicalTileID> canonical;
     std::set<std::string> availableImages;
     Feature feature;
 };

expression-test/expression_test_runner.cpp

@@ -104,8 +104,14 @@ TestRunOutput runExpressionTest(TestData& data, const std::string& rootPath, con
         std::vector<Value> outputs;
         if (!data.inputs.empty()) {
             for (const auto& input : data.inputs) {
-                auto evaluationResult =
-                    expression->evaluate(input.zoom, input.feature, input.heatmapDensity, input.availableImages);
+                mbgl::style::expression::EvaluationResult evaluationResult;
+                if (input.canonical) {
+                    evaluationResult = expression->evaluate(
+                        input.zoom, input.feature, input.heatmapDensity, input.availableImages, *input.canonical);
+                } else {
+                    evaluationResult =
+                        expression->evaluate(input.zoom, input.feature, input.heatmapDensity, input.availableImages);
+                }
                 if (!evaluationResult) {
                     std::unordered_map<std::string, Value> error{{"error", Value{evaluationResult.error().message}}};
                     outputs.emplace_back(Value{std::move(error)});

include/mbgl/style/expression/expression.hpp

@@ -191,6 +191,11 @@ class Expression {
                               const Feature& feature,
                               optional<double> colorRampParameter,
                               const std::set<std::string>& availableImages) const;
+    EvaluationResult evaluate(optional<float> zoom,
+                              const Feature& feature,
+                              optional<double> colorRampParameter,
+                              const std::set<std::string>& availableImages,
+                              const CanonicalTileID& canonical) const;
     EvaluationResult evaluate(optional<mbgl::Value> accumulated, const Feature& feature) const;
 
     /**

include/mbgl/style/expression/within.hpp

@@ -31,10 +31,10 @@ class Within final : public Expression {
         return false;
     }
 
-    std::vector<optional<Value>> possibleOutputs() const override { return {{false}}; }
+    std::vector<optional<Value>> possibleOutputs() const override { return {{true}, {false}}; }
 
     mbgl::Value serialize() const override;
-    std::string getOperator() const override { return "Within"; }
+    std::string getOperator() const override { return "within"; }
 
 private:
     GeoJSON geoJSONSource;

src/mbgl/style/expression/expression.cpp

@@ -9,8 +9,16 @@ namespace expression {
 class GeoJSONFeature : public GeometryTileFeature {
 public:
     const Feature& feature;
+    mutable optional<GeometryCollection> geometry;
 
     GeoJSONFeature(const Feature& feature_) : feature(feature_) {}
+    GeoJSONFeature(const Feature& feature_, const CanonicalTileID& canonical) : feature(feature_) {
+        geometry = convertGeometry(feature.geometry, canonical);
+        // https://github.com/mapbox/geojson-vt-cpp/issues/44
+        if (getType() == FeatureType::Polygon) {
+            geometry = fixupPolygons(*geometry);
+        }
+    }
 
     FeatureType getType() const override  {
         return apply_visitor(ToFeatureType(), feature.geometry);
@@ -24,6 +32,11 @@ class GeoJSONFeature : public GeometryTileFeature {
         }
         return optional<mbgl::Value>();
     }
+    const GeometryCollection& getGeometries() const override {
+        if (geometry) return *geometry;
+        geometry = GeometryCollection();
+        return *geometry;
+    }
 };
 
 EvaluationResult Expression::evaluate(optional<float> zoom,
@@ -41,6 +54,17 @@ EvaluationResult Expression::evaluate(optional<float> zoom,
     return this->evaluate(EvaluationContext(zoom, &f, colorRampParameter).withAvailableImages(&availableImages));
 }
 
+EvaluationResult Expression::evaluate(optional<float> zoom,
+                                      const Feature& feature,
+                                      optional<double> colorRampParameter,
+                                      const std::set<std::string>& availableImages,
+                                      const CanonicalTileID& canonical) const {
+    GeoJSONFeature f(feature, canonical);
+    return this->evaluate(EvaluationContext(zoom, &f, colorRampParameter)
+                              .withAvailableImages(&availableImages)
+                              .withCanonicalTileID(&canonical));
+}
+
 EvaluationResult Expression::evaluate(optional<mbgl::Value> accumulated, const Feature& feature) const {
     GeoJSONFeature f(feature);
     return this->evaluate(EvaluationContext(accumulated, &f));

src/mbgl/style/expression/within.cpp

@@ -7,50 +7,70 @@
 #include <mbgl/util/logging.hpp>
 #include <mbgl/util/string.hpp>
 
+#include <rapidjson/document.h>
+#include <mbgl/style/conversion/json.hpp>
+
 namespace mbgl {
 namespace {
 
-double isLeft(mbgl::Point<double> P0, mbgl::Point<double> P1, mbgl::Point<double> P2) {
-    return ((P1.x - P0.x) * (P2.y - P0.y) - (P2.x - P0.x) * (P1.y - P0.y));
+class PolygonFeature : public GeometryTileFeature {
+public:
+    const Feature& feature;
+    mutable optional<GeometryCollection> geometry;
+
+    PolygonFeature(const Feature& feature_, const CanonicalTileID& canonical) : feature(feature_) {
+        geometry = convertGeometry(feature.geometry, canonical);
+        assert(geometry);
+        geometry = fixupPolygons(*geometry);
+    }
+
+    FeatureType getType() const override { return FeatureType::Polygon; }
+    const PropertyMap& getProperties() const override { return feature.properties; }
+    FeatureIdentifier getID() const override { return feature.id; }
+    optional<mbgl::Value> getValue(const std::string& /*key*/) const override { return optional<mbgl::Value>(); }
+    const GeometryCollection& getGeometries() const override {
+        assert(geometry);
+        return *geometry;
+    }
+};
+
+bool rayIntersect(const mbgl::Point<double>& p, const mbgl::Point<double>& p1, const mbgl::Point<double>& p2) {
+    return ((p1.y > p.y) != (p2.y > p.y)) && (p.x < (p2.x - p1.x) * (p.y - p1.y) / (p2.y - p1.y) + p1.x);
 }
 
-// winding number algorithm for checking if point inside a ploygon or not.
-// http://geomalgorithms.com/a03-_inclusion.html#wn_PnPoly()
-bool pointWithinPolygons(mbgl::Point<double> point, const mapbox::geometry::polygon<double>& polys) {
-    //  wn = the winding number (=0 only when point is outside)
-    int wn = 0;
-    for (auto poly : polys) {
-        auto size = poly.size();
-        // loop through every edge of the polygon
+// ray casting algorithm for detecting if point is in polygon
+bool pointWithinPolygon(const mbgl::Point<double>& point, const mapbox::geometry::polygon<double>& polygon) {
+    bool within = false;
+    for (auto ring : polygon) {
+        auto size = ring.size();
+        // loop through every edge of the ring
         for (decltype(size) i = 0; i < size - 1; ++i) {
-            if (poly[i].y <= point.y) {
-                if (poly[i + 1].y > point.y) { // upward horizontal crossing from point to edge E(poly[i], poly[i+1])
-                    if (isLeft(poly[i], poly[i + 1], point) > 0) {
-                        ++wn;
-                    }
-                }
-            } else {
-                if (poly[i + 1].y <= point.y) { // downward crossing
-                    if (isLeft(poly[i], poly[i + 1], point) < 0) {
-                        --wn;
-                    }
-                }
+            if (rayIntersect(point, ring[i], ring[i + 1])) {
+                within = !within;
             }
         }
-        if (wn != 0) {
-            return true;
-        }
     }
-    return wn != 0;
+    return within;
+}
+
+bool pointWithinPolygons(const mbgl::Point<double>& point, const mapbox::geometry::multi_polygon<double>& polygons) {
+    for (auto polygon : polygons) {
+        auto within = pointWithinPolygon(point, polygon);
+        if (within) return true;
+    }
+    return false;
 }
 
 bool pointsWithinPolygons(const mbgl::GeometryTileFeature& feature,
                           const mbgl::CanonicalTileID& canonical,
                           const mbgl::GeoJSON& geoJson) {
     return geoJson.match(
         [&feature, &canonical](const mapbox::geometry::geometry<double>& geometrySet) -> bool {
-            return geometrySet.match(
-                [&feature, &canonical](const mapbox::geometry::polygon<double>& polygons) -> bool {
+            mbgl::Feature f(geometrySet);
+            PolygonFeature polyFeature(f, canonical);
+            auto refinedGeoSet = convertGeometry(polyFeature, canonical);
+            return refinedGeoSet.match(
+                [&feature, &canonical](const mapbox::geometry::multi_polygon<double>& polygons) -> bool {
                     return convertGeometry(feature, canonical)
                         .match(
                             [&polygons](const mapbox::geometry::point<double>& point) -> bool {
@@ -68,6 +88,24 @@ bool pointsWithinPolygons(const mbgl::GeometryTileFeature& feature,
                             },
                             [](const auto&) -> bool { return false; });
                 },
+                [&feature, &canonical](const mapbox::geometry::polygon<double>& polygon) -> bool {
+                    return convertGeometry(feature, canonical)
+                        .match(
+                            [&polygon](const mapbox::geometry::point<double>& point) -> bool {
+                                return pointWithinPolygon(point, polygon);
+                            },
+                            [&polygon](const mapbox::geometry::multi_point<double>& points) -> bool {
+                                auto result = false;
+                                for (const auto& p : points) {
+                                    result = pointWithinPolygon(p, polygon);
+                                    if (!result) {
+                                        return result;
+                                    }
+                                }
+                                return result;
+                            },
+                            [](const auto&) -> bool { return false; });
+                },
                 [](const auto&) -> bool { return false; });
         },
         [](const auto&) -> bool { return false; });
@@ -109,11 +147,11 @@ EvaluationResult Within::evaluate(const EvaluationContext& params) const {
         return false;
     }
     auto geometryType = params.feature->getType();
-    // Currently only support Point/Points in polygon
+    // Currently only support Point/Points in Polygon/Polygons
     if (geometryType == FeatureType::Point) {
         return pointsWithinPolygons(*params.feature, *params.canonical, geoJSONSource);
     } else {
-        mbgl::Log::Warning(mbgl::Event::General, "Within expression currently only support 'Point' geometry type");
+        mbgl::Log::Warning(mbgl::Event::General, "within expression currently only support 'Point' geometry type");
     }
     return false;
 }
@@ -122,7 +160,7 @@ ParseResult Within::parse(const Convertible& value, ParsingContext& ctx) {
     if (isArray(value)) {
         // object value, quoted with ["Within", value]
         if (arrayLength(value) != 2) {
-            ctx.error("'Within' expression requires exactly one argument, but found " +
+            ctx.error("'within' expression requires exactly one argument, but found " +
                       util::toString(arrayLength(value) - 1) + " instead.");
             return ParseResult();
         }
@@ -136,8 +174,37 @@ ParseResult Within::parse(const Convertible& value, ParsingContext& ctx) {
     return ParseResult();
 }
 
+Value valueConverter(const mapbox::geojson::rapidjson_value& v) {
+    if (v.IsDouble()) {
+        return v.GetDouble();
+    }
+    if (v.IsString()) {
+        return std::string(v.GetString());
+    }
+    if (v.IsArray()) {
+        std::vector<Value> result;
+        for (const auto& m : v.GetArray()) {
+            result.push_back(valueConverter(m));
+        }
+        return result;
+    }
+    // Ignore other types as valid geojson only contains above types.
+    return Null;
+}
+
 mbgl::Value Within::serialize() const {
-    return std::vector<mbgl::Value>{{getOperator()}, {mapbox::geojson::stringify(geoJSONSource)}};
+    std::unordered_map<std::string, Value> serialized;
+    rapidjson::CrtAllocator allocator;
+    const mapbox::geojson::rapidjson_value value = mapbox::geojson::convert(geoJSONSource, allocator);
+    if (value.IsObject()) {
+        for (const auto& m : value.GetObject()) {
+            serialized.emplace(m.name.GetString(), valueConverter(m.value));
+        }
+    } else {
+        mbgl::Log::Error(mbgl::Event::General,
+                         "Failed to serialize 'within' expression, converted rapidJSON is not an object");
+    }
+    return std::vector<mbgl::Value>{{getOperator(), *fromExpressionValue<mbgl::Value>(serialized)}};
 }
 
 } // namespace expression