From 32e6ccb037ee686a356756351487c2105ed73031 Mon Sep 17 00:00:00 2001 From: vng Date: Fri, 3 Nov 2017 10:55:22 +0000 Subject: [PATCH] Code review fixes. --- include/engine/datafacade/datafacade_base.hpp | 2 +- include/engine/guidance/route_step.hpp | 2 - src/engine/plugins/tile.cpp | 73 +++++++------------ src/extractor/extractor.cpp | 35 +-------- unit_tests/engine/offline_facade.cpp | 1 + unit_tests/mocks/mock_datafacade.hpp | 1 + 6 files changed, 32 insertions(+), 82 deletions(-) diff --git a/include/engine/datafacade/datafacade_base.hpp b/include/engine/datafacade/datafacade_base.hpp index f382b2e58..029b1dd60 100644 --- a/include/engine/datafacade/datafacade_base.hpp +++ b/include/engine/datafacade/datafacade_base.hpp @@ -192,7 +192,7 @@ class BaseDataFacade virtual bool IsLeftHandDriving(const NodeID id) const = 0; - virtual bool IsSegregated(const NodeID) const { return false; } + virtual bool IsSegregated(const NodeID) const = 0; }; } } diff --git a/include/engine/guidance/route_step.hpp b/include/engine/guidance/route_step.hpp index d2af0e977..635444a2b 100644 --- a/include/engine/guidance/route_step.hpp +++ b/include/engine/guidance/route_step.hpp @@ -165,8 +165,6 @@ inline RouteStep &RouteStep::ElongateBy(const RouteStep &following_step) following_step.intersections.begin(), following_step.intersections.end()); - /// @todo Process is_segregated flag - return *this; } diff --git a/src/engine/plugins/tile.cpp b/src/engine/plugins/tile.cpp index a8bef13e3..965882c45 100644 --- a/src/engine/plugins/tile.cpp +++ b/src/engine/plugins/tile.cpp @@ -69,7 +69,7 @@ template struct ValueIndexer return offset; } - std::size_t indexOf(const T &value) { return value_offsets[value]; }; + std::size_t indexOf(const T &value) { return value_offsets[value]; } const std::vector &values() { return used_values; } @@ -184,16 +184,8 @@ inline void encodePoint(const FixedPoint &pt, protozero::packed_field_uint32 &ge geometry.add_element(protozero::encode_zigzag32(dy)); } -std::vector coordinatesToTileLine(const std::vector &points, - const BBox &tile_bbox) +linestring_t floatLineToTileLine(const FloatLine &geo_line, const BBox &tile_bbox) { - FloatLine geo_line; - for (auto const &c : points) - { - geo_line.emplace_back(static_cast(util::toFloating(c.lon)), - static_cast(util::toFloating(c.lat))); - } - linestring_t unclipped_line; for (auto const &pt : geo_line) @@ -212,34 +204,45 @@ std::vector coordinatesToTileLine(const std::vector boost::geometry::append(unclipped_line, point_t(px, py)); } - multi_linestring_t clipped_line; + return unclipped_line; +} +std::vector coordinatesToTileLine(const std::vector &points, + const BBox &tile_bbox) +{ + FloatLine geo_line; + for (auto const &c : points) + { + geo_line.emplace_back(static_cast(util::toFloating(c.lon)), + static_cast(util::toFloating(c.lat))); + } + + linestring_t unclipped_line = floatLineToTileLine(geo_line, tile_bbox); + + multi_linestring_t clipped_line; boost::geometry::intersection(clip_box, unclipped_line, clipped_line); std::vector result; // b::g::intersection might return a line with one point if the // original line was very short and coords were dupes - if (!clipped_line.empty()) + for (auto const &cl : clipped_line) { - for (auto const &cl : clipped_line) - { - if (cl.size() < 2) - continue; + if (cl.size() < 2) + continue; - FixedLine tile_line; - for (const auto &p : cl) - tile_line.emplace_back(p.get<0>(), p.get<1>()); + FixedLine tile_line; + for (const auto &p : cl) + tile_line.emplace_back(p.get<0>(), p.get<1>()); - result.emplace_back(std::move(tile_line)); - } + result.emplace_back(std::move(tile_line)); } return result; } /** - * Returnx the x1,y1,x2,y2 pixel coordinates of a line in a given + * Return the x1,y1,x2,y2 pixel coordinates of a line in a given * tile. * * @param start the first coordinate of the line @@ -257,26 +260,9 @@ FixedLine coordinatesToTileLine(const util::Coordinate start, geo_line.emplace_back(static_cast(util::toFloating(target.lon)), static_cast(util::toFloating(target.lat))); - linestring_t unclipped_line; - - for (auto const &pt : geo_line) - { - double px_merc = pt.x * util::web_mercator::DEGREE_TO_PX; - double py_merc = util::web_mercator::latToY(util::FloatLatitude{pt.y}) * - util::web_mercator::DEGREE_TO_PX; - // convert lon/lat to tile coordinates - const auto px = std::round( - ((px_merc - tile_bbox.minx) * util::web_mercator::TILE_SIZE / tile_bbox.width()) * - util::vector_tile::EXTENT / util::web_mercator::TILE_SIZE); - const auto py = std::round( - ((tile_bbox.maxy - py_merc) * util::web_mercator::TILE_SIZE / tile_bbox.height()) * - util::vector_tile::EXTENT / util::web_mercator::TILE_SIZE); - - boost::geometry::append(unclipped_line, point_t(px, py)); - } + linestring_t unclipped_line = floatLineToTileLine(geo_line, tile_bbox); multi_linestring_t clipped_line; - boost::geometry::intersection(clip_box, unclipped_line, clipped_line); FixedLine tile_line; @@ -285,12 +271,9 @@ FixedLine coordinatesToTileLine(const util::Coordinate start, // original line was very short and coords were dupes if (!clipped_line.empty() && clipped_line[0].size() == 2) { - if (clipped_line[0].size() == 2) + for (const auto &p : clipped_line[0]) { - for (const auto &p : clipped_line[0]) - { - tile_line.emplace_back(p.get<0>(), p.get<1>()); - } + tile_line.emplace_back(p.get<0>(), p.get<1>()); } } diff --git a/src/extractor/extractor.cpp b/src/extractor/extractor.cpp index c639facce..4e1107184 100644 --- a/src/extractor/extractor.cpp +++ b/src/extractor/extractor.cpp @@ -886,23 +886,6 @@ bool IsSegregated(std::vector v1, return false; } - /* - std::vector intersect; - std::set_intersection(v1.begin(), - v1.end(), - v2.begin(), - v2.end(), - std::back_inserter(intersect), - EdgeInfo::LessName()); - - intersect.erase(std::remove_if(intersect.begin(), - intersect.end(), - [](EdgeInfo const &info) { return info.name.empty(); }), - intersect.end()); - - return intersect.size() >= 2; - */ - // set_intersection like routine to count equal name pairs, std function is // not acceptable because of duplicates {a, a, b} ∩ {a, a, c} == {a, a}. std::vector> commons; @@ -964,21 +947,6 @@ bool IsSegregated(std::vector v1, std::min(threshold, get_length_threshold(e.first) + get_length_threshold(e.second)); return edgeLength <= threshold; - - /// @todo Process standalone U-turns. - /* - switch (commons.size()) - { - case 0: - return false; - case 1: - // ingoing + outgoing edges - if (commons.front().first->direction + commons.front().second->direction != 1) - return false; - } - - return true; - */ } size_t Extractor::FindSegregatedNodes(NodeBasedGraphFactory &factory) @@ -1071,7 +1039,6 @@ size_t Extractor::FindSegregatedNodes(NodeBasedGraphFactory &factory) edgeLength); }; - std::unordered_set processed; size_t segregated_count = 0; for (NodeID sourceID = 0; sourceID < graph.GetNumberOfNodes(); ++sourceID) @@ -1081,7 +1048,7 @@ size_t Extractor::FindSegregatedNodes(NodeBasedGraphFactory &factory) { auto &edgeData = graph.GetEdgeData(edgeID); - if (edgeData.reversed || edgeData.segregated || !processed.insert(edgeID).second) + if (edgeData.reversed || edgeData.segregated) continue; NodeID const targetID = graph.GetTarget(edgeID); diff --git a/unit_tests/engine/offline_facade.cpp b/unit_tests/engine/offline_facade.cpp index e60434f49..ec8d66ecb 100644 --- a/unit_tests/engine/offline_facade.cpp +++ b/unit_tests/engine/offline_facade.cpp @@ -366,6 +366,7 @@ class ContiguousInternalMemoryDataFacade util::guidance::EntryClass GetEntryClass(const EdgeID /*turn_id*/) const override { return {}; } bool IsLeftHandDriving(const NodeID /*id*/) const override { return false; } + bool IsSegregated(const NodeID /*id*/) const override { return false; } }; } // datafacade diff --git a/unit_tests/mocks/mock_datafacade.hpp b/unit_tests/mocks/mock_datafacade.hpp index 0bf0b36bd..2d92972bb 100644 --- a/unit_tests/mocks/mock_datafacade.hpp +++ b/unit_tests/mocks/mock_datafacade.hpp @@ -224,6 +224,7 @@ class MockBaseDataFacade : public engine::datafacade::BaseDataFacade unsigned GetWeightPrecision() const override final { return 1; } double GetWeightMultiplier() const override final { return 10.; } bool IsLeftHandDriving(const NodeID /*id*/) const override { return false; } + bool IsSegregated(const NodeID /*id*/) const override { return false; } util::guidance::TurnBearing PreTurnBearing(const EdgeID /*eid*/) const override final {