Allow Travis builds for this branch

Update CMakeLists.txt version to 5.3.1
Adapt Changelog for v5.3.1
2016-08-04 15:22:51 +02:00 · 2016-08-04 15:12:48 +02:00 · 2016-08-04 15:06:44 +02:00 · 2016-08-04 14:58:06 +02:00 · 2016-08-04 14:57:59 +02:00 · 2016-08-04 14:57:53 +02:00
8 changed files with 179 additions and 26 deletions
--- a/.travis.yml
+++ b/.travis.yml
@ -13,6 +13,7 @@ notifications:
 branches:
  only:
    - master
+    - "5.3"

 cache:
  ccache: true
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -1,3 +1,9 @@
+# 5.3.1
+  Changes from 5.3.1
+    - Bugfixes:
+      - Disabled broken lane handling for complex uturn/oneway combinations for now (190 intersections affected on the planet)
+      - Fixed a bug with overlaping geometries, which broke OSRM on recent Egypt extracts with data-modelling issues
+
 # 5.3.0
  Changes from 5.3.0-rc.3
    - Guidance
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@ -10,7 +10,7 @@ endif()
 project(OSRM C CXX)
 set(OSRM_VERSION_MAJOR 5)
 set(OSRM_VERSION_MINOR 3)
-set(OSRM_VERSION_PATCH 0)
+set(OSRM_VERSION_PATCH 1)

 # these two functions build up custom variables:
 #   OSRM_INCLUDE_PATHS and OSRM_DEFINES
--- a/features/guidance/turn-lanes.feature
+++ b/features/guidance/turn-lanes.feature
@ -736,4 +736,113 @@ Feature: Turn Lane Guidance
            | x,d       | road,road | depart,arrive | ,     |


+    Scenario: Lane Parsing Issue #2694
+        Given the node map
+            |   | c |
+            | a | b |
+            |   | d |

+        And the ways
+            | nodes | highway | turn:lanes:forward |
+            | ab    | primary | left;left\|right   |
+            | bc    | primary |                    |
+            | bd    | primary |                    |
+
+        When I route I should get
+            | waypoints | route     | turns                   | lanes                   |
+            | a,c       | ab,bc,bc  | depart,turn left,arrive | ,left:true right:false, |
+
+    # http://www.openstreetmap.org/#map=19/47.97685/7.82933&layers=D
+    @bug @todo
+    Scenario: Lane Parsing Issue #2706: None Assignments I
+        Given the node map
+            |   | f |   |   | j  |   |
+            |   |   |   |   |    |   |
+            | a | b | c |   | d  | e |
+            |   |   |   |   |    |   |
+            |   |   |   |   | i  |   |
+            |   | g |   |   | h  |   |
+
+        And the nodes
+            | node | highway         |
+            | a    | traffic_signals |
+            | i    | traffic_signals |
+
+        And the ways
+            | nodes | highway        | name           | oneway | turn:lanes:forward       |
+            | ab    | secondary      | Wiesentalstr   |        | through;left\|right      |
+            | bc    | secondary      | Wiesentalstr   |        | none\|left;through       |
+            | cd    | secondary      | Wiesentalstr   |        | none\|left;through       |
+            | de    | residential    | Wippertstr     |        |                          |
+            | fb    | secondary      | Merzhauser Str | yes    | through\|through\|right  |
+            | bg    | secondary      | Merzhauser Str | yes    |                          |
+            | hi    | secondary      | Merzhauser Str | yes    | left;reverse\|none\|none |
+            | ic    | secondary_link | Merzhauser Str | yes    |                          |
+            | id    | secondary      | Merzhauser Str | yes    | through;right\|none      |
+            | dj    | secondary      | Merzhauser Str | yes    |                          |
+
+        And the relations
+            | type        | way:from | way:to | node:via | restriction    |
+            | restriction | fb       | fb     | b        | no_left_turn   |
+            | restriction | ic       | cb     | c        | only_left_turn |
+            | restriction | id       | dc     | d        | no_left_turn   |
+
+        When I route I should get
+            | waypoints | route     | turns                   | lanes                  |
+            | h,a ||||
+            # Note: at the moment we don't care about routes, we care about the extract process triggering assertions
+
+    # https://www.openstreetmap.org/#map=19/47.99257/7.83276&layers=D
+    @bug @todo
+    Scenario: Lane Parsing Issue #2706: None Assignments II
+        Given the node map
+            |   | k | l |   |
+            | j | a | b | f |
+            | i | c | d | e |
+            |   | h | g |   |
+
+        And the ways
+            | nodes | highway        | name           | oneway | turn:lanes                           |
+            | ka    | secondary      | Eschholzstr    | yes    | left;reverse\|through\|through\|none |
+            | kj    | unclassified   | kj             | yes    |                                      |
+            | ac    | secondary      | Eschholzstr    | yes    | left;reverse\|none\|none\|none       |
+            | ch    | secondary      | Eschholzstr    | yes    |                                      |
+            | gd    | secondary      | Eschholzstr    | yes    | left;reverse\|through\|through\|none |
+            | db    | secondary      | Eschholzstr    | yes    | left;reverse\|through\|through\|none |
+            | bl    | secondary      | Eschholzstr    | yes    |                                      |
+            | fb    | residential    | Haslacher Str  | yes    | left;reverse\|left;through\|right    |
+            | ba    | secondary_link | Haslacher Str  | yes    | left;reverse\|left;through           |
+            | aj    | unclassified   | Haslacher Str  | yes    |                                      |
+            | ic    | unclassified   | Haslacher Str  | yes    | left;reverse\|left\|through          |
+            | cd    | secondary_link | Haslacher Str  | yes    | left;reverse\|left\|through          |
+            | de    | residential    | Haslacher Str  | yes    |                                      |
+
+        And the relations
+            | type        | way:from | way:to | node:via | restriction      |
+            | restriction | ka       | ac     | a        | only_straight_on |
+            | restriction | ic       | cd     | c        | only_straight_on |
+            | restriction | gd       | db     | d        | only_straight_on |
+
+        When I route I should get
+            | waypoints | route     | turns                   | lanes                  |
+            | i,e ||||
+            # Note: at the moment we don't care about routes, we care about the extract process triggering assertions
+
+    @bug @todo
+    Scenario: Lane Parsing Issue #2706: None Assignments III - Minimal reproduction recipe
+        Given the node map
+            |   |   | l |   |
+            |   | a | b |   |
+            |   |   | d |   |
+            |   |   |   |   |
+
+        And the ways
+            | nodes | highway        | name           | oneway | turn:lanes                           |
+            | db    | secondary      | Eschholzstr    | yes    | left;reverse\|through\|through\|none |
+            | bl    | secondary      | Eschholzstr    | yes    |                                      |
+            | ba    | secondary_link | Haslacher Str  | yes    |                                      |
+
+        When I route I should get
+            | waypoints | route     | turns                   | lanes                  |
+            | d,a ||||
+            # Note: at the moment we don't care about routes, we care about the extract process triggering assertions
--- a/src/extractor/extractor_callbacks.cpp
+++ b/src/extractor/extractor_callbacks.cpp
@ -193,7 +193,8 @@ void ExtractorCallbacks::ProcessWay(const osmium::Way &input_way, const Extracti
            for (auto token_itr = inner_tokens.begin(); token_itr != inner_tokens.end();
                 ++token_itr)
            {
-                auto position = std::find(osm_lane_strings, osm_lane_strings + num_osm_tags, *token_itr);
+                auto position =
+                    std::find(osm_lane_strings, osm_lane_strings + num_osm_tags, *token_itr);
                const auto translated_mask =
                    masks_by_osm_string[std::distance(osm_lane_strings, position)];
                if (translated_mask == TurnLaneType::empty)
@ -203,7 +204,10 @@ void ExtractorCallbacks::ProcessWay(const osmium::Way &input_way, const Extracti
                                                         << *token_itr << "\"";
                    return {};
                }
-                BOOST_ASSERT((lane_mask & translated_mask) == 0); // make sure the mask is valid
+
+                // In case of multiple times the same lane indicators withn a lane, as in
+                // "left;left|.."  or-ing the masks generates a single "left" enum.
+                // Which is fine since this is data issue and we can't represent it anyway.
                lane_mask |= translated_mask;
            }
            // add the lane to the description
@ -265,8 +269,8 @@ void ExtractorCallbacks::ProcessWay(const osmium::Way &input_way, const Extracti
        // name_offsets already has an offset of a new name, take the offset index as the name id
        name_id = external_memory.name_offsets.size() - 1;

-        external_memory.name_char_data.reserve(external_memory.name_char_data.size() + name_length
-                                               + destinations_length + pronunciation_length);
+        external_memory.name_char_data.reserve(external_memory.name_char_data.size() + name_length +
+                                               destinations_length + pronunciation_length);

        std::copy(parsed_way.name.c_str(),
                  parsed_way.name.c_str() + name_length,
@ -306,7 +310,9 @@ void ExtractorCallbacks::ProcessWay(const osmium::Way &input_way, const Extracti
    std::transform(input_way.nodes().begin(),
                   input_way.nodes().end(),
                   std::back_inserter(external_memory.used_node_id_list),
-                   [](const osmium::NodeRef &ref) { return OSMNodeID{static_cast<std::uint64_t>(ref.ref())}; });
+                   [](const osmium::NodeRef &ref) {
+                       return OSMNodeID{static_cast<std::uint64_t>(ref.ref())};
+                   });

    const bool is_opposite_way = TRAVEL_MODE_INACCESSIBLE == parsed_way.forward_travel_mode;

@ -338,7 +344,8 @@ void ExtractorCallbacks::ProcessWay(const osmium::Way &input_way, const Extracti
        external_memory.way_start_end_id_list.push_back(
            {OSMWayID{static_cast<std::uint32_t>(input_way.id())},
             OSMNodeID{static_cast<std::uint64_t>(input_way.nodes().back().ref())},
-             OSMNodeID{static_cast<std::uint64_t>(input_way.nodes()[input_way.nodes().size() - 2].ref())},
+             OSMNodeID{
+                 static_cast<std::uint64_t>(input_way.nodes()[input_way.nodes().size() - 2].ref())},
             OSMNodeID{static_cast<std::uint64_t>(input_way.nodes()[1].ref())},
             OSMNodeID{static_cast<std::uint64_t>(input_way.nodes()[0].ref())}});
    }
@ -372,27 +379,28 @@ void ExtractorCallbacks::ProcessWay(const osmium::Way &input_way, const Extracti
                input_way.nodes().cbegin(),
                input_way.nodes().cend(),
                [&](const osmium::NodeRef &first_node, const osmium::NodeRef &last_node) {
-                    external_memory.all_edges_list.push_back(
-                        InternalExtractorEdge(OSMNodeID{static_cast<std::uint64_t>(first_node.ref())},
-                                              OSMNodeID{static_cast<std::uint64_t>(last_node.ref())},
-                                              name_id,
-                                              backward_weight_data,
-                                              false,
-                                              true,
-                                              parsed_way.roundabout,
-                                              parsed_way.is_access_restricted,
-                                              parsed_way.is_startpoint,
-                                              parsed_way.backward_travel_mode,
-                                              true,
-                                              turn_lane_id_backward,
-                                              road_classification));
+                    external_memory.all_edges_list.push_back(InternalExtractorEdge(
+                        OSMNodeID{static_cast<std::uint64_t>(first_node.ref())},
+                        OSMNodeID{static_cast<std::uint64_t>(last_node.ref())},
+                        name_id,
+                        backward_weight_data,
+                        false,
+                        true,
+                        parsed_way.roundabout,
+                        parsed_way.is_access_restricted,
+                        parsed_way.is_startpoint,
+                        parsed_way.backward_travel_mode,
+                        true,
+                        turn_lane_id_backward,
+                        road_classification));
                });
        }

        external_memory.way_start_end_id_list.push_back(
            {OSMWayID{static_cast<std::uint32_t>(input_way.id())},
             OSMNodeID{static_cast<std::uint64_t>(input_way.nodes().back().ref())},
-             OSMNodeID{static_cast<std::uint64_t>(input_way.nodes()[input_way.nodes().size() - 2].ref())},
+             OSMNodeID{
+                 static_cast<std::uint64_t>(input_way.nodes()[input_way.nodes().size() - 2].ref())},
             OSMNodeID{static_cast<std::uint64_t>(input_way.nodes()[1].ref())},
             OSMNodeID{static_cast<std::uint64_t>(input_way.nodes()[0].ref())}});
    }
--- a/src/extractor/guidance/intersection_generator.cpp
+++ b/src/extractor/guidance/intersection_generator.cpp
@ -143,7 +143,20 @@ Intersection IntersectionGenerator::getConnectedRoads(const NodeID from_node,
    const auto valid_count =
        boost::count_if(intersection, [](const ConnectedRoad &road) { return road.entry_allowed; });
    if (0 == valid_count && uturn_could_be_valid)
-        intersection[0].entry_allowed = true;
+    {
+        // after intersections sorting by angles, find the u-turn with (from_node == to_node)
+        // that was inserted together with setting uturn_could_be_valid flag
+        std::size_t self_u_turn = 0;
+        while (self_u_turn < intersection.size()
+               && intersection[self_u_turn].turn.angle < std::numeric_limits<double>::epsilon()
+               && from_node != node_based_graph.GetTarget(intersection[self_u_turn].turn.eid))
+        {
+            ++self_u_turn;
+        }
+
+        BOOST_ASSERT(from_node == node_based_graph.GetTarget(intersection[self_u_turn].turn.eid));
+        intersection[self_u_turn].entry_allowed = true;
+    }

    return mergeSegregatedRoads(std::move(intersection));
 }
--- a/src/extractor/guidance/turn_lane_augmentation.cpp
+++ b/src/extractor/guidance/turn_lane_augmentation.cpp
@ -282,7 +282,14 @@ LaneDataVector handleNoneValueAtSimpleTurn(LaneDataVector lane_data,
        // a pgerequisite is simple turns. Larger differences should not end up here
        // an additional line at the side is only reasonable if it is targeting public
        // service vehicles. Otherwise, we should not have it
-        BOOST_ASSERT(connection_count + 1 == lane_data.size());
+        //
+        // TODO(mokob): #2730 have a look please
+        // BOOST_ASSERT(connection_count + 1 == lane_data.size());
+        //
+        if (connection_count + 1 != lane_data.size())
+        {
+            goto these_intersections_are_clearly_broken_at_the_moment;
+        }

        lane_data = mergeNoneTag(none_index, std::move(lane_data));
    }
@ -292,6 +299,9 @@ LaneDataVector handleNoneValueAtSimpleTurn(LaneDataVector lane_data,
    {
        lane_data = handleRenamingSituations(none_index, std::move(lane_data), intersection);
    }
+
+these_intersections_are_clearly_broken_at_the_moment:
+
    // finally make sure we are still sorted
    std::sort(lane_data.begin(), lane_data.end());
    return lane_data;
--- a/src/extractor/guidance/turn_lane_handler.cpp
+++ b/src/extractor/guidance/turn_lane_handler.cpp
@ -351,8 +351,14 @@ bool TurnLaneHandler::isSimpleIntersection(const LaneDataVector &lane_data,
            if (lane_data.back().tag == TurnLaneType::uturn)
                return findBestMatchForReverse(lane_data[lane_data.size() - 2].tag, intersection);

-            BOOST_ASSERT(lane_data.front().tag == TurnLaneType::uturn);
-            return findBestMatchForReverse(lane_data[1].tag, intersection);
+            // TODO(mokob): #2730 have a look please
+            // BOOST_ASSERT(lane_data.front().tag == TurnLaneType::uturn);
+            // return findBestMatchForReverse(lane_data[1].tag, intersection);
+            //
+            if (lane_data.front().tag == TurnLaneType::uturn)
+                return findBestMatchForReverse(lane_data[1].tag, intersection);
+
+            return findBestMatch(data.tag, intersection);
        }();
        std::size_t match_index = std::distance(intersection.begin(), best_match);
        all_simple &= (matched_indices.count(match_index) == 0);
Author	SHA1	Message	Date
Daniel J. Hofmann	fe491bf92c	Allow Travis builds for this branch	2016-08-04 15:22:51 +02:00
Daniel J. Hofmann	7b432b34bb	Update CMakeLists.txt version to 5.3.1	2016-08-04 15:12:48 +02:00
Daniel J. Hofmann	6983cd0de2	Adapt Changelog for v5.3.1	2016-08-04 15:06:44 +02:00
Daniel J. Hofmann	fe8177077c	Disable our added failing cucumber tests for now	2016-08-04 14:58:06 +02:00
Daniel J. Hofmann	5f339f4ed6	Fall back to generic match finding if not a reverse-lane	2016-08-04 14:57:59 +02:00
Daniel J. Hofmann	3c0b52c637	Skip handling none values for our edge cases for now.. Conflicts: src/extractor/guidance/turn_lane_augmentation.cpp	2016-08-04 14:57:53 +02:00
Daniel Patterson	877fc5b42c	Add a minimal version of the failing test case.	2016-08-04 14:57:47 +02:00
Daniel J. Hofmann	e28785e399	Reproducing breaking intersection in cucumber scenario Conflicts: src/extractor/guidance/turn_lane_augmentation.cpp	2016-08-04 14:57:39 +02:00
Daniel J. Hofmann	2c4a54ce05	Try to come up with a small test case	2016-08-04 14:57:32 +02:00
Michael Krasnyk	a8afc74590	Fix #2706 by using correct fallback u-turn Regression is due to a combination of `08248e3853` and http://www.openstreetmap.org/changeset/40938983 where in ways http://www.openstreetmap.org/way/27292481 and http://www.openstreetmap.org/way/432488408 nodes 4315134884 (part of way 432488408) 4315134891 (part of way 432488408) 4315134886 (part of way 432488408) form a u-turn that has index 0 after sorting and used as an allowed one with a reversed edge. A u-turn that corresponds to the condition uturn_could_be_valid == true has index 1 and ignored.	2016-07-31 22:46:10 +02:00
Daniel J. Hofmann	d195eee7c4	Lane Handling for multiple indications per lane as in left;left\|, fixes #2694 Before we asserted on unique lane indications per lane. Turns out the OSM data contains lane strings such as: left;left\|right Which represents two lanes as in: << > \|\| \| The two left indications _on a single lane_ look like data issue. And we can't represent this with our enum-approach at the moment. We don't want to crash there, so silently swallow this and generate a single left\|right for it.	2016-07-26 12:18:15 +02:00