summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorzmiao <miao.zhao@mapbox.com>2020-04-25 01:12:45 +0300
committerGitHub <noreply@github.com>2020-04-24 15:12:45 -0700
commit9bbb1b721e56e07666f6b0819594c4bfda84435e (patch)
tree66ca511509874d22f61eecd41eeb098dc420d59a
parent0ccc9f26899125889f51ad006ce1cdf65d87b069 (diff)
downloadqtlocation-mapboxgl-9bbb1b721e56e07666f6b0819594c4bfda84435e.tar.gz
[core] Distance expression: remove unit argument (#16434)
-rw-r--r--include/mbgl/style/expression/distance.hpp3
-rw-r--r--src/mbgl/style/expression/distance.cpp117
-rw-r--r--test/style/property_expression.test.cpp76
3 files changed, 26 insertions, 170 deletions
diff --git a/include/mbgl/style/expression/distance.hpp b/include/mbgl/style/expression/distance.hpp
index e034b9452f..481162be03 100644
--- a/include/mbgl/style/expression/distance.hpp
+++ b/include/mbgl/style/expression/distance.hpp
@@ -10,7 +10,7 @@ namespace expression {
class Distance final : public Expression {
public:
- Distance(GeoJSON geoJSONSource_, Feature::geometry_type geometries_, mapbox::cheap_ruler::CheapRuler::Unit unit_);
+ Distance(GeoJSON geoJSONSource_, Feature::geometry_type geometries_);
~Distance() override;
@@ -30,7 +30,6 @@ public:
private:
GeoJSON geoJSONSource;
Feature::geometry_type geometries;
- mapbox::cheap_ruler::CheapRuler::Unit unit;
};
} // namespace expression
diff --git a/src/mbgl/style/expression/distance.cpp b/src/mbgl/style/expression/distance.cpp
index 78d80dc649..9992ef5394 100644
--- a/src/mbgl/style/expression/distance.cpp
+++ b/src/mbgl/style/expression/distance.cpp
@@ -24,6 +24,7 @@ namespace {
const std::size_t MinPointsSize = 100;
const std::size_t MinLinePointsSize = 50;
const double InvalidDistance = std::numeric_limits<double>::quiet_NaN();
+const mapbox::cheap_ruler::CheapRuler::Unit UnitInMeters = mapbox::cheap_ruler::CheapRuler::Unit::Meters;
// Inclusive index range for multipoint or linestring container
using IndexRange = std::pair<std::size_t, std::size_t>;
@@ -327,9 +328,8 @@ double lineToLinesDistance(const mapbox::geometry::line_string<double>& line,
}
double pointsToGeometryDistance(const mapbox::geometry::multi_point<double>& points,
- const Feature::geometry_type& geoSet,
- mapbox::cheap_ruler::CheapRuler::Unit unit) {
- mapbox::cheap_ruler::CheapRuler ruler(points.front().y, unit);
+ const Feature::geometry_type& geoSet) {
+ mapbox::cheap_ruler::CheapRuler ruler(points.front().y, UnitInMeters);
return geoSet.match(
[&points, &ruler](const mapbox::geometry::point<double>& p) {
return pointsToPointsDistance(mapbox::geometry::multi_point<double>{p}, points, ruler);
@@ -346,11 +346,9 @@ double pointsToGeometryDistance(const mapbox::geometry::multi_point<double>& poi
[](const auto&) { return InvalidDistance; });
}
-double lineToGeometryDistance(const mapbox::geometry::line_string<double>& line,
- const Feature::geometry_type& geoSet,
- mapbox::cheap_ruler::CheapRuler::Unit unit) {
+double lineToGeometryDistance(const mapbox::geometry::line_string<double>& line, const Feature::geometry_type& geoSet) {
assert(!line.empty());
- mapbox::cheap_ruler::CheapRuler ruler(line.front().y, unit);
+ mapbox::cheap_ruler::CheapRuler ruler(line.front().y, UnitInMeters);
return geoSet.match(
[&line, &ruler](const mapbox::geometry::point<double>& p) {
return pointsToLineDistance(mapbox::geometry::multi_point<double>{p}, line, ruler);
@@ -369,23 +367,22 @@ double lineToGeometryDistance(const mapbox::geometry::line_string<double>& line,
double calculateDistance(const GeometryTileFeature& feature,
const CanonicalTileID& canonical,
- const Feature::geometry_type& geoSet,
- mapbox::cheap_ruler::CheapRuler::Unit unit) {
+ const Feature::geometry_type& geoSet) {
return convertGeometry(feature, canonical)
.match(
- [&geoSet, &unit](const mapbox::geometry::point<double>& point) -> double {
- return pointsToGeometryDistance(mapbox::geometry::multi_point<double>{point}, geoSet, unit);
+ [&geoSet](const mapbox::geometry::point<double>& point) -> double {
+ return pointsToGeometryDistance(mapbox::geometry::multi_point<double>{point}, geoSet);
},
- [&geoSet, &unit](const mapbox::geometry::multi_point<double>& points) -> double {
- return pointsToGeometryDistance(points, geoSet, unit);
+ [&geoSet](const mapbox::geometry::multi_point<double>& points) -> double {
+ return pointsToGeometryDistance(points, geoSet);
},
- [&geoSet, &unit](const mapbox::geometry::line_string<double>& line) -> double {
- return lineToGeometryDistance(line, geoSet, unit);
+ [&geoSet](const mapbox::geometry::line_string<double>& line) -> double {
+ return lineToGeometryDistance(line, geoSet);
},
- [&geoSet, &unit](const mapbox::geometry::multi_line_string<double>& lines) -> double {
+ [&geoSet](const mapbox::geometry::multi_line_string<double>& lines) -> double {
double dist = std::numeric_limits<double>::infinity();
for (const auto& line : lines) {
- auto tempDist = lineToGeometryDistance(line, geoSet, unit);
+ auto tempDist = lineToGeometryDistance(line, geoSet);
if (std::isnan(tempDist)) return tempDist;
dist = std::min(dist, tempDist);
if (dist == 0.0 || std::isnan(dist)) return dist;
@@ -395,15 +392,7 @@ double calculateDistance(const GeometryTileFeature& feature,
[](const auto&) -> double { return InvalidDistance; });
}
-struct Arguments {
- Arguments(GeoJSON& geojson_, mapbox::cheap_ruler::CheapRuler::Unit unit_)
- : geojson(std::move(geojson_)), unit(unit_) {}
-
- GeoJSON geojson;
- mapbox::cheap_ruler::CheapRuler::Unit unit;
-};
-
-optional<Arguments> parseValue(const style::conversion::Convertible& value, style::expression::ParsingContext& ctx) {
+optional<GeoJSON> parseValue(const style::conversion::Convertible& value, style::expression::ParsingContext& ctx) {
if (isArray(value)) {
// object value, quoted with ["Distance", GeoJSONObj, "unit(optional)"]
auto length = arrayLength(value);
@@ -413,42 +402,13 @@ optional<Arguments> parseValue(const style::conversion::Convertible& value, styl
return nullopt;
}
- // Parse Unit info for distance calculation, "Meters" by default
- mapbox::cheap_ruler::CheapRuler::Unit unit = mapbox::cheap_ruler::CheapRuler::Unit::Meters;
- if (length == 3) {
- auto input = toString(arrayMember(value, 2));
- if (input == nullopt) {
- ctx.error("Failed to parse unit argument from 'distance' expression");
- return nullopt;
- }
- if (*input == "meters" || *input == "metres") {
- unit = mapbox::cheap_ruler::CheapRuler::Unit::Meters;
- } else if (*input == "kilometers") {
- unit = mapbox::cheap_ruler::CheapRuler::Unit::Kilometers;
- } else if (*input == "miles") {
- unit = mapbox::cheap_ruler::CheapRuler::Unit::Miles;
- } else if (*input == "nauticalmiles") {
- unit = mapbox::cheap_ruler::CheapRuler::Unit::NauticalMiles;
- } else if (*input == "yards") {
- unit = mapbox::cheap_ruler::CheapRuler::Unit::Yards;
- } else if (*input == "feet") {
- unit = mapbox::cheap_ruler::CheapRuler::Unit::Feet;
- } else if (*input == "inches") {
- unit = mapbox::cheap_ruler::CheapRuler::Unit::Inches;
- } else {
- ctx.error(
- "'distance' expression only accepts following units: kilometers, miles, nauticalmiles, "
- "meters, metres, yards, feet, inches.");
- return nullopt;
- }
- }
// Parse geometry info
const auto& argument1 = arrayMember(value, 1);
if (isObject(argument1)) {
style::conversion::Error error;
auto geojson = toGeoJSON(argument1, error);
if (geojson && error.message.empty()) {
- return Arguments(*geojson, unit);
+ return *geojson;
}
ctx.error(error.message);
}
@@ -472,11 +432,8 @@ optional<Feature::geometry_type> getGeometry(const Feature& feature, mbgl::style
namespace style {
namespace expression {
-Distance::Distance(GeoJSON geojson, Feature::geometry_type geometries_, mapbox::cheap_ruler::CheapRuler::Unit unit_)
- : Expression(Kind::Distance, type::Number),
- geoJSONSource(std::move(geojson)),
- geometries(std::move(geometries_)),
- unit(unit_) {}
+Distance::Distance(GeoJSON geojson, Feature::geometry_type geometries_)
+ : Expression(Kind::Distance, type::Number), geoJSONSource(std::move(geojson)), geometries(std::move(geometries_)) {}
Distance::~Distance() = default;
@@ -488,7 +445,7 @@ EvaluationResult Distance::evaluate(const EvaluationContext& params) const {
}
auto geometryType = params.feature->getType();
if (geometryType == FeatureType::Point || geometryType == FeatureType::LineString) {
- auto distance = calculateDistance(*params.feature, *params.canonical, geometries, unit);
+ auto distance = calculateDistance(*params.feature, *params.canonical, geometries);
if (!std::isnan(distance)) {
assert(distance >= 0.0);
return distance;
@@ -503,26 +460,23 @@ ParseResult Distance::parse(const Convertible& value, ParsingContext& ctx) {
return ParseResult();
}
- return parsedValue->geojson.match(
+ return parsedValue->match(
[&parsedValue, &ctx](const mapbox::geometry::geometry<double>& geometrySet) {
if (auto ret = getGeometry(mbgl::Feature(geometrySet), ctx)) {
- return ParseResult(
- std::make_unique<Distance>(parsedValue->geojson, std::move(*ret), parsedValue->unit));
+ return ParseResult(std::make_unique<Distance>(*parsedValue, std::move(*ret)));
}
return ParseResult();
},
[&parsedValue, &ctx](const mapbox::feature::feature<double>& feature) {
if (auto ret = getGeometry(mbgl::Feature(feature), ctx)) {
- return ParseResult(
- std::make_unique<Distance>(parsedValue->geojson, std::move(*ret), parsedValue->unit));
+ return ParseResult(std::make_unique<Distance>(*parsedValue, std::move(*ret)));
}
return ParseResult();
},
[&parsedValue, &ctx](const mapbox::feature::feature_collection<double>& features) {
for (const auto& feature : features) {
if (auto ret = getGeometry(mbgl::Feature(feature), ctx)) {
- return ParseResult(
- std::make_unique<Distance>(parsedValue->geojson, std::move(*ret), parsedValue->unit));
+ return ParseResult(std::make_unique<Distance>(*parsedValue, std::move(*ret)));
}
}
return ParseResult();
@@ -566,27 +520,6 @@ mbgl::Value convertValue(const mapbox::geojson::rapidjson_value& v) {
return Null;
}
-std::string getUnits(const mapbox::cheap_ruler::CheapRuler::Unit& unit) {
- switch (unit) {
- case mapbox::cheap_ruler::CheapRuler::Kilometers:
- return "kilometers";
- case mapbox::cheap_ruler::CheapRuler::Miles:
- return "miles";
- case mapbox::cheap_ruler::CheapRuler::NauticalMiles:
- return "nauticalmiles";
- case mapbox::cheap_ruler::CheapRuler::Meters:
- return "meters";
- case mapbox::cheap_ruler::CheapRuler::Yards:
- return "yards";
- case mapbox::cheap_ruler::CheapRuler::Feet:
- return "feet";
- case mapbox::cheap_ruler::CheapRuler::Inches:
- return "inches";
- default:
- return "error";
- }
-}
-
mbgl::Value Distance::serialize() const {
std::unordered_map<std::string, mbgl::Value> serialized;
rapidjson::CrtAllocator allocator;
@@ -599,13 +532,13 @@ mbgl::Value Distance::serialize() const {
mbgl::Log::Error(mbgl::Event::General,
"Failed to serialize 'distance' expression, converted rapidJSON is not an object");
}
- return std::vector<mbgl::Value>{{getOperator(), serialized, getUnits(unit)}};
+ return std::vector<mbgl::Value>{{getOperator(), serialized}};
}
bool Distance::operator==(const Expression& e) const {
if (e.getKind() == Kind::Distance) {
auto rhs = static_cast<const Distance*>(&e);
- return geoJSONSource == rhs->geoJSONSource && geometries == rhs->geometries && unit == rhs->unit;
+ return geoJSONSource == rhs->geoJSONSource && geometries == rhs->geometries;
}
return false;
}
diff --git a/test/style/property_expression.test.cpp b/test/style/property_expression.test.cpp
index 41f8238c70..924b0f91f6 100644
--- a/test/style/property_expression.test.cpp
+++ b/test/style/property_expression.test.cpp
@@ -550,82 +550,6 @@ TEST(PropertyExpression, DistanceExpression) {
propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID), invalidResult);
EXPECT_NEAR(491.307, evaluatedResult, 0.01);
- // Unit: meters
- ss.str(std::string());
- ss << std::string(R"(["distance", )") << pointGeoSource << std::string(R"(, "meters" ])");
- expression = createExpression(ss.str().c_str());
- ASSERT_TRUE(expression);
- propExpr = std::move(expression);
-
- evaluatedResult =
- propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID), invalidResult);
- EXPECT_NEAR(491.307, evaluatedResult, 0.01);
-
- // Unit: kilometers
- ss.str(std::string());
- ss << std::string(R"(["distance", )") << pointGeoSource << std::string(R"(, "kilometers" ])");
- expression = createExpression(ss.str().c_str());
- ASSERT_TRUE(expression);
- propExpr = std::move(expression);
-
- evaluatedResult =
- propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID), invalidResult);
- EXPECT_NEAR(0.491307, evaluatedResult, 0.00001);
-
- // Unit: miles
- ss.str(std::string());
- ss << std::string(R"(["distance", )") << pointGeoSource << std::string(R"(, "miles" ])");
- expression = createExpression(ss.str().c_str());
- ASSERT_TRUE(expression);
- propExpr = std::move(expression);
-
- evaluatedResult =
- propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID), invalidResult);
- EXPECT_NEAR(0.305284, evaluatedResult, 0.00001);
-
- // Unit: nauticalmiles
- ss.str(std::string());
- ss << std::string(R"(["distance", )") << pointGeoSource << std::string(R"(, "nauticalmiles" ])");
- expression = createExpression(ss.str().c_str());
- ASSERT_TRUE(expression);
- propExpr = std::move(expression);
-
- evaluatedResult =
- propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID), invalidResult);
- EXPECT_NEAR(0.265284, evaluatedResult, 0.00001);
-
- // Unit: yards
- ss.str(std::string());
- ss << std::string(R"(["distance", )") << pointGeoSource << std::string(R"(, "yards" ])");
- expression = createExpression(ss.str().c_str());
- ASSERT_TRUE(expression);
- propExpr = std::move(expression);
-
- evaluatedResult =
- propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID), invalidResult);
- EXPECT_NEAR(537.299, evaluatedResult, 0.01);
-
- // Unit: feet
- ss.str(std::string());
- ss << std::string(R"(["distance", )") << pointGeoSource << std::string(R"(, "feet" ])");
- expression = createExpression(ss.str().c_str());
- ASSERT_TRUE(expression);
- propExpr = std::move(expression);
-
- evaluatedResult =
- propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID), invalidResult);
- EXPECT_NEAR(1611.898, evaluatedResult, 0.01);
-
- // Unit: inches
- ss.str(std::string());
- ss << std::string(R"(["distance", )") << pointGeoSource << std::string(R"(, "inches" ])");
- expression = createExpression(ss.str().c_str());
- ASSERT_TRUE(expression);
- propExpr = std::move(expression);
-
- evaluatedResult =
- propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID), invalidResult);
- EXPECT_NEAR(19342.781, evaluatedResult, 0.01);
}
// Evaluation test with Point to MultiPoint distance