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

Commit 9bbb1b7

Browse files
zmiaozmiao
authored
[core] Distance expression: remove unit argument (#16434)
1 parent 0ccc9f2 commit 9bbb1b7

File tree

3 files changed

+26
-170
lines changed

3 files changed

+26
-170
lines changed

‎include/mbgl/style/expression/distance.hpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace expression {
1010

1111
class Distance final : public Expression {
1212
public:
13-
Distance(GeoJSON geoJSONSource_, Feature::geometry_type geometries_, mapbox::cheap_ruler::CheapRuler::Unit unit_);
13+
Distance(GeoJSON geoJSONSource_, Feature::geometry_type geometries_);
1414

1515
~Distance() override;
1616

@@ -30,7 +30,6 @@ class Distance final : public Expression {
3030
private:
3131
GeoJSON geoJSONSource;
3232
Feature::geometry_type geometries;
33-
mapbox::cheap_ruler::CheapRuler::Unit unit;
3433
};
3534

3635
} // namespace expression

‎src/mbgl/style/expression/distance.cpp‎

Lines changed: 25 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ namespace {
2424
const std::size_t MinPointsSize = 100;
2525
const std::size_t MinLinePointsSize = 50;
2626
const double InvalidDistance = std::numeric_limits<double>::quiet_NaN();
27+
const mapbox::cheap_ruler::CheapRuler::Unit UnitInMeters = mapbox::cheap_ruler::CheapRuler::Unit::Meters;
2728

2829
// Inclusive index range for multipoint or linestring container
2930
using IndexRange = std::pair<std::size_t, std::size_t>;
@@ -327,9 +328,8 @@ double lineToLinesDistance(const mapbox::geometry::line_string<double>& line,
327328
}
328329

329330
double pointsToGeometryDistance(const mapbox::geometry::multi_point<double>& points,
330-
const Feature::geometry_type& geoSet,
331-
mapbox::cheap_ruler::CheapRuler::Unit unit) {
332-
mapbox::cheap_ruler::CheapRuler ruler(points.front().y, unit);
331+
const Feature::geometry_type& geoSet) {
332+
mapbox::cheap_ruler::CheapRuler ruler(points.front().y, UnitInMeters);
333333
return geoSet.match(
334334
[&points, &ruler](const mapbox::geometry::point<double>& p) {
335335
return pointsToPointsDistance(mapbox::geometry::multi_point<double>{p}, points, ruler);
@@ -346,11 +346,9 @@ double pointsToGeometryDistance(const mapbox::geometry::multi_point<double>& poi
346346
[](const auto&) { return InvalidDistance; });
347347
}
348348

349-
double lineToGeometryDistance(const mapbox::geometry::line_string<double>& line,
350-
const Feature::geometry_type& geoSet,
351-
mapbox::cheap_ruler::CheapRuler::Unit unit) {
349+
double lineToGeometryDistance(const mapbox::geometry::line_string<double>& line, const Feature::geometry_type& geoSet) {
352350
assert(!line.empty());
353-
mapbox::cheap_ruler::CheapRuler ruler(line.front().y, unit);
351+
mapbox::cheap_ruler::CheapRuler ruler(line.front().y, UnitInMeters);
354352
return geoSet.match(
355353
[&line, &ruler](const mapbox::geometry::point<double>& p) {
356354
return pointsToLineDistance(mapbox::geometry::multi_point<double>{p}, line, ruler);
@@ -369,23 +367,22 @@ double lineToGeometryDistance(const mapbox::geometry::line_string<double>& line,
369367

370368
double calculateDistance(const GeometryTileFeature& feature,
371369
const CanonicalTileID& canonical,
372-
const Feature::geometry_type& geoSet,
373-
mapbox::cheap_ruler::CheapRuler::Unit unit) {
370+
const Feature::geometry_type& geoSet) {
374371
return convertGeometry(feature, canonical)
375372
.match(
376-
[&geoSet, &unit](const mapbox::geometry::point<double>& point) -> double {
377-
return pointsToGeometryDistance(mapbox::geometry::multi_point<double>{point}, geoSet, unit);
373+
[&geoSet](const mapbox::geometry::point<double>& point) -> double {
374+
return pointsToGeometryDistance(mapbox::geometry::multi_point<double>{point}, geoSet);
378375
},
379-
[&geoSet, &unit](const mapbox::geometry::multi_point<double>& points) -> double {
380-
return pointsToGeometryDistance(points, geoSet, unit);
376+
[&geoSet](const mapbox::geometry::multi_point<double>& points) -> double {
377+
return pointsToGeometryDistance(points, geoSet);
381378
},
382-
[&geoSet, &unit](const mapbox::geometry::line_string<double>& line) -> double {
383-
return lineToGeometryDistance(line, geoSet, unit);
379+
[&geoSet](const mapbox::geometry::line_string<double>& line) -> double {
380+
return lineToGeometryDistance(line, geoSet);
384381
},
385-
[&geoSet, &unit](const mapbox::geometry::multi_line_string<double>& lines) -> double {
382+
[&geoSet](const mapbox::geometry::multi_line_string<double>& lines) -> double {
386383
double dist = std::numeric_limits<double>::infinity();
387384
for (const auto& line : lines) {
388-
auto tempDist = lineToGeometryDistance(line, geoSet, unit);
385+
auto tempDist = lineToGeometryDistance(line, geoSet);
389386
if (std::isnan(tempDist)) return tempDist;
390387
dist = std::min(dist, tempDist);
391388
if (dist == 0.0 || std::isnan(dist)) return dist;
@@ -395,15 +392,7 @@ double calculateDistance(const GeometryTileFeature& feature,
395392
[](const auto&) -> double { return InvalidDistance; });
396393
}
397394

398-
struct Arguments {
399-
Arguments(GeoJSON& geojson_, mapbox::cheap_ruler::CheapRuler::Unit unit_)
400-
: geojson(std::move(geojson_)), unit(unit_) {}
401-
402-
GeoJSON geojson;
403-
mapbox::cheap_ruler::CheapRuler::Unit unit;
404-
};
405-
406-
optional<Arguments> parseValue(const style::conversion::Convertible& value, style::expression::ParsingContext& ctx) {
395+
optional<GeoJSON> parseValue(const style::conversion::Convertible& value, style::expression::ParsingContext& ctx) {
407396
if (isArray(value)) {
408397
// object value, quoted with ["Distance", GeoJSONObj, "unit(optional)"]
409398
auto length = arrayLength(value);
@@ -413,42 +402,13 @@ optional<Arguments> parseValue(const style::conversion::Convertible& value, styl
413402
return nullopt;
414403
}
415404

416-
// Parse Unit info for distance calculation, "Meters" by default
417-
mapbox::cheap_ruler::CheapRuler::Unit unit = mapbox::cheap_ruler::CheapRuler::Unit::Meters;
418-
if (length == 3) {
419-
auto input = toString(arrayMember(value, 2));
420-
if (input == nullopt) {
421-
ctx.error("Failed to parse unit argument from 'distance' expression");
422-
return nullopt;
423-
}
424-
if (*input == "meters" || *input == "metres") {
425-
unit = mapbox::cheap_ruler::CheapRuler::Unit::Meters;
426-
} else if (*input == "kilometers") {
427-
unit = mapbox::cheap_ruler::CheapRuler::Unit::Kilometers;
428-
} else if (*input == "miles") {
429-
unit = mapbox::cheap_ruler::CheapRuler::Unit::Miles;
430-
} else if (*input == "nauticalmiles") {
431-
unit = mapbox::cheap_ruler::CheapRuler::Unit::NauticalMiles;
432-
} else if (*input == "yards") {
433-
unit = mapbox::cheap_ruler::CheapRuler::Unit::Yards;
434-
} else if (*input == "feet") {
435-
unit = mapbox::cheap_ruler::CheapRuler::Unit::Feet;
436-
} else if (*input == "inches") {
437-
unit = mapbox::cheap_ruler::CheapRuler::Unit::Inches;
438-
} else {
439-
ctx.error(
440-
"'distance' expression only accepts following units: kilometers, miles, nauticalmiles, "
441-
"meters, metres, yards, feet, inches.");
442-
return nullopt;
443-
}
444-
}
445405
// Parse geometry info
446406
const auto& argument1 = arrayMember(value, 1);
447407
if (isObject(argument1)) {
448408
style::conversion::Error error;
449409
auto geojson = toGeoJSON(argument1, error);
450410
if (geojson && error.message.empty()) {
451-
return Arguments(*geojson, unit);
411+
return *geojson;
452412
}
453413
ctx.error(error.message);
454414
}
@@ -472,11 +432,8 @@ optional<Feature::geometry_type> getGeometry(const Feature& feature, mbgl::style
472432
namespace style {
473433
namespace expression {
474434

475-
Distance::Distance(GeoJSON geojson, Feature::geometry_type geometries_, mapbox::cheap_ruler::CheapRuler::Unit unit_)
476-
: Expression(Kind::Distance, type::Number),
477-
geoJSONSource(std::move(geojson)),
478-
geometries(std::move(geometries_)),
479-
unit(unit_) {}
435+
Distance::Distance(GeoJSON geojson, Feature::geometry_type geometries_)
436+
: Expression(Kind::Distance, type::Number), geoJSONSource(std::move(geojson)), geometries(std::move(geometries_)) {}
480437

481438
Distance::~Distance() = default;
482439

@@ -488,7 +445,7 @@ EvaluationResult Distance::evaluate(const EvaluationContext& params) const {
488445
}
489446
auto geometryType = params.feature->getType();
490447
if (geometryType == FeatureType::Point || geometryType == FeatureType::LineString) {
491-
auto distance = calculateDistance(*params.feature, *params.canonical, geometries, unit);
448+
auto distance = calculateDistance(*params.feature, *params.canonical, geometries);
492449
if (!std::isnan(distance)) {
493450
assert(distance >= 0.0);
494451
return distance;
@@ -503,26 +460,23 @@ ParseResult Distance::parse(const Convertible& value, ParsingContext& ctx) {
503460
return ParseResult();
504461
}
505462

506-
return parsedValue->geojson.match(
463+
return parsedValue->match(
507464
[&parsedValue, &ctx](const mapbox::geometry::geometry<double>& geometrySet) {
508465
if (auto ret = getGeometry(mbgl::Feature(geometrySet), ctx)) {
509-
return ParseResult(
510-
std::make_unique<Distance>(parsedValue->geojson, std::move(*ret), parsedValue->unit));
466+
return ParseResult(std::make_unique<Distance>(*parsedValue, std::move(*ret)));
511467
}
512468
return ParseResult();
513469
},
514470
[&parsedValue, &ctx](const mapbox::feature::feature<double>& feature) {
515471
if (auto ret = getGeometry(mbgl::Feature(feature), ctx)) {
516-
return ParseResult(
517-
std::make_unique<Distance>(parsedValue->geojson, std::move(*ret), parsedValue->unit));
472+
return ParseResult(std::make_unique<Distance>(*parsedValue, std::move(*ret)));
518473
}
519474
return ParseResult();
520475
},
521476
[&parsedValue, &ctx](const mapbox::feature::feature_collection<double>& features) {
522477
for (const auto& feature : features) {
523478
if (auto ret = getGeometry(mbgl::Feature(feature), ctx)) {
524-
return ParseResult(
525-
std::make_unique<Distance>(parsedValue->geojson, std::move(*ret), parsedValue->unit));
479+
return ParseResult(std::make_unique<Distance>(*parsedValue, std::move(*ret)));
526480
}
527481
}
528482
return ParseResult();
@@ -566,27 +520,6 @@ mbgl::Value convertValue(const mapbox::geojson::rapidjson_value& v) {
566520
return Null;
567521
}
568522

569-
std::string getUnits(const mapbox::cheap_ruler::CheapRuler::Unit& unit) {
570-
switch (unit) {
571-
case mapbox::cheap_ruler::CheapRuler::Kilometers:
572-
return "kilometers";
573-
case mapbox::cheap_ruler::CheapRuler::Miles:
574-
return "miles";
575-
case mapbox::cheap_ruler::CheapRuler::NauticalMiles:
576-
return "nauticalmiles";
577-
case mapbox::cheap_ruler::CheapRuler::Meters:
578-
return "meters";
579-
case mapbox::cheap_ruler::CheapRuler::Yards:
580-
return "yards";
581-
case mapbox::cheap_ruler::CheapRuler::Feet:
582-
return "feet";
583-
case mapbox::cheap_ruler::CheapRuler::Inches:
584-
return "inches";
585-
default:
586-
return "error";
587-
}
588-
}
589-
590523
mbgl::Value Distance::serialize() const {
591524
std::unordered_map<std::string, mbgl::Value> serialized;
592525
rapidjson::CrtAllocator allocator;
@@ -599,13 +532,13 @@ mbgl::Value Distance::serialize() const {
599532
mbgl::Log::Error(mbgl::Event::General,
600533
"Failed to serialize 'distance' expression, converted rapidJSON is not an object");
601534
}
602-
return std::vector<mbgl::Value>{{getOperator(), serialized, getUnits(unit)}};
535+
return std::vector<mbgl::Value>{{getOperator(), serialized}};
603536
}
604537

605538
bool Distance::operator==(const Expression& e) const {
606539
if (e.getKind() == Kind::Distance) {
607540
auto rhs = static_cast<const Distance*>(&e);
608-
return geoJSONSource == rhs->geoJSONSource && geometries == rhs->geometries && unit == rhs->unit;
541+
return geoJSONSource == rhs->geoJSONSource && geometries == rhs->geometries;
609542
}
610543
return false;
611544
}

‎test/style/property_expression.test.cpp‎

Lines changed: 0 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -550,82 +550,6 @@ TEST(PropertyExpression, DistanceExpression) {
550550
propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID), invalidResult);
551551
EXPECT_NEAR(491.307, evaluatedResult, 0.01);
552552

553-
// Unit: meters
554-
ss.str(std::string());
555-
ss << std::string(R"(["distance", )") << pointGeoSource << std::string(R"(, "meters" ])");
556-
expression = createExpression(ss.str().c_str());
557-
ASSERT_TRUE(expression);
558-
propExpr = std::move(expression);
559-
560-
evaluatedResult =
561-
propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID), invalidResult);
562-
EXPECT_NEAR(491.307, evaluatedResult, 0.01);
563-
564-
// Unit: kilometers
565-
ss.str(std::string());
566-
ss << std::string(R"(["distance", )") << pointGeoSource << std::string(R"(, "kilometers" ])");
567-
expression = createExpression(ss.str().c_str());
568-
ASSERT_TRUE(expression);
569-
propExpr = std::move(expression);
570-
571-
evaluatedResult =
572-
propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID), invalidResult);
573-
EXPECT_NEAR(0.491307, evaluatedResult, 0.00001);
574-
575-
// Unit: miles
576-
ss.str(std::string());
577-
ss << std::string(R"(["distance", )") << pointGeoSource << std::string(R"(, "miles" ])");
578-
expression = createExpression(ss.str().c_str());
579-
ASSERT_TRUE(expression);
580-
propExpr = std::move(expression);
581-
582-
evaluatedResult =
583-
propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID), invalidResult);
584-
EXPECT_NEAR(0.305284, evaluatedResult, 0.00001);
585-
586-
// Unit: nauticalmiles
587-
ss.str(std::string());
588-
ss << std::string(R"(["distance", )") << pointGeoSource << std::string(R"(, "nauticalmiles" ])");
589-
expression = createExpression(ss.str().c_str());
590-
ASSERT_TRUE(expression);
591-
propExpr = std::move(expression);
592-
593-
evaluatedResult =
594-
propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID), invalidResult);
595-
EXPECT_NEAR(0.265284, evaluatedResult, 0.00001);
596-
597-
// Unit: yards
598-
ss.str(std::string());
599-
ss << std::string(R"(["distance", )") << pointGeoSource << std::string(R"(, "yards" ])");
600-
expression = createExpression(ss.str().c_str());
601-
ASSERT_TRUE(expression);
602-
propExpr = std::move(expression);
603-
604-
evaluatedResult =
605-
propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID), invalidResult);
606-
EXPECT_NEAR(537.299, evaluatedResult, 0.01);
607-
608-
// Unit: feet
609-
ss.str(std::string());
610-
ss << std::string(R"(["distance", )") << pointGeoSource << std::string(R"(, "feet" ])");
611-
expression = createExpression(ss.str().c_str());
612-
ASSERT_TRUE(expression);
613-
propExpr = std::move(expression);
614-
615-
evaluatedResult =
616-
propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID), invalidResult);
617-
EXPECT_NEAR(1611.898, evaluatedResult, 0.01);
618-
619-
// Unit: inches
620-
ss.str(std::string());
621-
ss << std::string(R"(["distance", )") << pointGeoSource << std::string(R"(, "inches" ])");
622-
expression = createExpression(ss.str().c_str());
623-
ASSERT_TRUE(expression);
624-
propExpr = std::move(expression);
625-
626-
evaluatedResult =
627-
propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID), invalidResult);
628-
EXPECT_NEAR(19342.781, evaluatedResult, 0.01);
629553
}
630554

631555
// Evaluation test with Point to MultiPoint distance

0 commit comments

Comments
 (0)