Code review fixes.

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;
 };
 }
 }

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;
 }
 

src/engine/plugins/tile.cpp

@@ -69,7 +69,7 @@ template <typename T> 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<T> &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<FixedLine> coordinatesToTileLine(const std::vector<util::Coordinate> &points,
+linestring_t floatLineToTileLine(const FloatLine &geo_line, const BBox &tile_bbox)
-                                             const BBox &tile_bbox)
 {
-    FloatLine geo_line;
-    for (auto const &c : points)
-    {
-        geo_line.emplace_back(static_cast<double>(util::toFloating(c.lon)),
-                              static_cast<double>(util::toFloating(c.lat)));
-    }
-
     linestring_t unclipped_line;
 
     for (auto const &pt : geo_line)
@@ -212,34 +204,45 @@ std::vector<FixedLine> coordinatesToTileLine(const std::vector<util::Coordinate>
         boost::geometry::append(unclipped_line, point_t(px, py));
     }
 
-    multi_linestring_t clipped_line;
+    return unclipped_line;
+}
 
+std::vector<FixedLine> coordinatesToTileLine(const std::vector<util::Coordinate> &points,
+                                             const BBox &tile_bbox)
+{
+    FloatLine geo_line;
+    for (auto const &c : points)
+    {
+        geo_line.emplace_back(static_cast<double>(util::toFloating(c.lon)),
+                              static_cast<double>(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<FixedLine> 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;
+        FixedLine tile_line;
-            for (const auto &p : cl)
+        for (const auto &p : cl)
-                tile_line.emplace_back(p.get<0>(), p.get<1>());
+            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<double>(util::toFloating(target.lon)),
                           static_cast<double>(util::toFloating(target.lat)));
 
-    linestring_t unclipped_line;
+    linestring_t unclipped_line = floatLineToTileLine(geo_line, tile_bbox);
-
-    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));
-    }
 
     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>());
-            }
         }
     }
 

src/extractor/extractor.cpp

@@ -886,23 +886,6 @@ bool IsSegregated(std::vector<EdgeInfo> v1,
             return false;
     }
 
-    /*
-    std::vector<EdgeInfo> 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<std::pair<EdgeInfo const *, EdgeInfo const *>> commons;
@@ -964,21 +947,6 @@ bool IsSegregated(std::vector<EdgeInfo> 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<EdgeID> 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);

unit_tests/engine/offline_facade.cpp

@@ -366,6 +366,7 @@ class ContiguousInternalMemoryDataFacade<routing_algorithms::offline::Algorithm>
 
     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

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
     {