Update changelog and bump OSRM version to 5.7.4

CHANGELOG.md

@@ -1,3 +1,14 @@
+# 5.7.4
+  - Changes from 5.7.3:
+   - Bug fixes:
+     - Fixes 4100: Fix steps collapsing after non-closed roundabouts
+     - Fixes 4129: Fix invalid roundabout instructions for different driving modes
+
+# 5.7.3
+  - Changes from 5.7.2:
+   - Bug fixes:
+     - Fixes 4097: .ramIndex files was able to be truncated siliently
+
 # 5.7.2
   - Changes from 5.7.1:
    - Bug fixes:

CMakeLists.txt

@@ -55,7 +55,7 @@ endif()
 project(OSRM C CXX)
 set(OSRM_VERSION_MAJOR 5)
 set(OSRM_VERSION_MINOR 7)
-set(OSRM_VERSION_PATCH 2)
+set(OSRM_VERSION_PATCH 4)
 set(OSRM_VERSION "${OSRM_VERSION_MAJOR}.${OSRM_VERSION_MINOR}.${OSRM_VERSION_PATCH}")
 
 add_definitions(-DOSRM_PROJECT_DIR="${CMAKE_CURRENT_SOURCE_DIR}")

features/guidance/roundabout-turn-bike.feature

@@ -37,3 +37,37 @@ Feature: Basic Roundabout
            | h,a       | gh,ab,ab | depart,roundabout turn left exit-3,arrive     |
            | h,d       | gh,cd,cd | depart,roundabout turn straight exit-2,arrive |
            | h,f       | gh,ef,ef | depart,roundabout turn right exit-1,arrive    |
+
+    # https://www.openstreetmap.org/way/223225602
+    Scenario: Enter and Exit with changing mode
+        Given the node map
+            """
+                a
+                b
+            h g   c d
+                e
+                f
+            """
+
+       And the ways
+            | nodes | junction   | highway     |
+            | ab    |            | residential |
+            | cd    |            | residential |
+            | ef    |            | footway     |
+            | gh    |            | footway     |
+            | bgecb | roundabout | residential |
+
+       When I route I should get
+           | waypoints | route          | turns                                                              |
+           | a,d       | ab,cd,cd       | depart,roundabout turn left exit-1,arrive                          |
+           | a,f       | ab,ef,ef,ef    | depart,roundabout turn left exit-1,notification right,arrive       |
+           | a,h       | ab,bgecb,gh,gh | depart,roundabout turn right exit-1,notification right,arrive      |
+           | d,f       | cd,ef,ef,ef    | depart,roundabout turn sharp left exit-2,notification right,arrive |
+           | d,h       | cd,gh,gh,gh    | depart,roundabout turn left exit-2,notification right,arrive       |
+           | d,a       | cd,ab,ab       | depart,roundabout turn right exit-1,arrive                         |
+           | f,h       | ef,gh,gh,gh    | depart,roundabout turn sharp left exit-3,notification right,arrive |
+           | f,a       | ef,ab,ab       | depart,roundabout turn straight exit-2,arrive                      |
+           | f,d       | ef,cd,cd       | depart,roundabout turn right exit-1,arrive                         |
+           | h,a       | gh,ab,ab       | depart,roundabout turn left exit-2,arrive                          |
+           | h,d       | gh,cd,cd       | depart,roundabout turn straight exit-1,arrive                      |
+           | h,f       | gh,bgecb,ef,ef | depart,roundabout turn right exit-1,notification right,arrive      |

include/storage/io.hpp

@@ -228,7 +228,7 @@ class FileWriter
 
     template <typename T> void WriteFrom(const T &src) { WriteFrom(&src, 1); }
 
-    template <typename T> void WriteOne(const T tmp) { WriteFrom(tmp); }
+    template <typename T> void WriteOne(const T &tmp) { WriteFrom(tmp); }
 
     void WriteElementCount32(const std::uint32_t count) { WriteOne<std::uint32_t>(count); }
     void WriteElementCount64(const std::uint64_t count) { WriteOne<std::uint64_t>(count); }

include/util/indexed_data.hpp

@@ -1,6 +1,8 @@
 #ifndef OSRM_INDEXED_DATA_HPP
 #define OSRM_INDEXED_DATA_HPP
 
+#include "storage/io.hpp"
+
 #include "util/exception.hpp"
 #include "util/string_view.hpp"
 
@@ -9,7 +11,6 @@
 #include <array>
 #include <iterator>
 #include <limits>
-#include <ostream>
 #include <string>
 #include <type_traits>
 
@@ -85,7 +86,7 @@ template <int N, typename T = std::string> struct VariableGroupBlock
     /// prefix length.
     /// Returns the block prefix length.
     template <typename Offset, typename OffsetIterator>
-    Offset WriteBlockReference(std::ostream &out,
+    Offset WriteBlockReference(storage::io::FileWriter &out,
                                Offset data_offset,
                                OffsetIterator first,
                                OffsetIterator last) const
@@ -105,7 +106,7 @@ template <int N, typename T = std::string> struct VariableGroupBlock
             prefix_length += byte_length;
         }
 
-        out.write((const char *)&refernce, sizeof(refernce));
+        out.WriteOne(refernce);
 
         return prefix_length;
     }
@@ -118,7 +119,8 @@ template <int N, typename T = std::string> struct VariableGroupBlock
     /// [first..last] is an inclusive range of block data.
     /// The length of the last item in the block is not stored.
     template <typename OffsetIterator>
-    void WriteBlockPrefix(std::ostream &out, OffsetIterator first, OffsetIterator last) const
+    void
+    WriteBlockPrefix(storage::io::FileWriter &out, OffsetIterator first, OffsetIterator last) const
     {
         for (OffsetIterator curr = first, next = std::next(first); curr != last; ++curr, ++next)
         {
@@ -127,7 +129,9 @@ template <int N, typename T = std::string> struct VariableGroupBlock
             if (byte_length == 0)
                 continue;
 
-            out.write((const char *)&data_length, byte_length);
+            // Here, we're only writing a few bytes from the 4-byte std::uint32_t,
+            // so we need to cast to (char *)
+            out.WriteFrom((const char *)&data_length, byte_length);
         }
     }
 
@@ -175,20 +179,23 @@ template <int N, typename T = std::string> struct FixedGroupBlock
     /// Write a block reference {offset}, where offset is a global block offset
     /// Returns the fixed block prefix length.
     template <typename Offset, typename OffsetIterator>
-    Offset
+    Offset WriteBlockReference(storage::io::FileWriter &out,
-    WriteBlockReference(std::ostream &out, Offset data_offset, OffsetIterator, OffsetIterator) const
+                               Offset data_offset,
+                               OffsetIterator,
+                               OffsetIterator) const
     {
         BOOST_ASSERT(data_offset <= std::numeric_limits<decltype(BlockReference::offset)>::max());
 
         BlockReference refernce{static_cast<decltype(BlockReference::offset)>(data_offset)};
-        out.write((const char *)&refernce, sizeof(refernce));
+        out.WriteOne(refernce);
 
         return BLOCK_SIZE;
     }
 
     /// Write a fixed length block prefix.
     template <typename OffsetIterator>
-    void WriteBlockPrefix(std::ostream &out, OffsetIterator first, OffsetIterator last) const
+    void
+    WriteBlockPrefix(storage::io::FileWriter &out, OffsetIterator first, OffsetIterator last) const
     {
         std::uint32_t index = 0;
         std::array<ValueType, BLOCK_SIZE> block_prefix;
@@ -200,7 +207,7 @@ template <int N, typename T = std::string> struct FixedGroupBlock
 
             block_prefix[index++] = static_cast<ValueType>(data_length);
         }
-        out.write((const char *)block_prefix.data(), block_prefix.size());
+        out.WriteFrom(block_prefix.data(), block_prefix.size());
     }
 
     /// Advances the range to an item stored in the referenced block.
@@ -244,8 +251,10 @@ template <typename GroupBlock> struct IndexedData
     bool empty() const { return blocks_number == 0; }
 
     template <typename OffsetIterator, typename DataIterator>
-    void
+    void write(storage::io::FileWriter &out,
-    write(std::ostream &out, OffsetIterator first, OffsetIterator last, DataIterator data) const
+               OffsetIterator first,
+               OffsetIterator last,
+               DataIterator data) const
     {
         static_assert(sizeof(typename DataIterator::value_type) == 1, "data basic type must char");
 
@@ -259,7 +268,7 @@ template <typename GroupBlock> struct IndexedData
         const BlocksNumberType number_of_blocks =
             number_of_elements == 0 ? 0
                                     : 1 + (std::distance(first, sentinel) - 1) / (BLOCK_SIZE + 1);
-        out.write((const char *)&number_of_blocks, sizeof(number_of_blocks));
+        out.WriteOne(number_of_blocks);
 
         // Write block references and compute the total data size that includes prefix and data
         const GroupBlock block;
@@ -273,7 +282,7 @@ template <typename GroupBlock> struct IndexedData
         }
 
         // Write the total data size
-        out.write((const char *)&data_size, sizeof(data_size));
+        out.WriteOne(data_size);
 
         // Write data blocks that are (prefix, data)
         for (OffsetIterator curr = first, next = first; next != sentinel; curr = next)
@@ -281,7 +290,8 @@ template <typename GroupBlock> struct IndexedData
             std::advance(next, std::min<diff_type>(BLOCK_SIZE, std::distance(next, sentinel)));
             block.WriteBlockPrefix(out, curr, next);
             std::advance(next, std::min<diff_type>(1, std::distance(next, sentinel)));
-            std::copy(data + *curr, data + *next, std::ostream_iterator<unsigned char>(out));
+            std::for_each(
+                data + *curr, data + *next, [&out](const auto &element) { out.WriteOne(element); });
         }
     }
 

include/util/static_rtree.hpp

@@ -19,7 +19,6 @@
 
 #include <boost/assert.hpp>
 #include <boost/filesystem.hpp>
-#include <boost/filesystem/fstream.hpp>
 #include <boost/format.hpp>
 #include <boost/iostreams/device/mapped_file.hpp>
 
@@ -204,14 +203,16 @@ class StaticRTree
                 }
             });
 
-        // open leaf file
+        std::vector<TreeNode> tree_nodes_in_level;
-        boost::filesystem::ofstream leaf_node_file(leaf_node_filename, std::ios::binary);
-
         // sort the hilbert-value representatives
         tbb::parallel_sort(input_wrapper_vector.begin(), input_wrapper_vector.end());
-        std::vector<TreeNode> tree_nodes_in_level;
 
-        // pack M elements into leaf node, write to leaf file and add child index to the parent node
+        {
+            storage::io::FileWriter leaf_node_file(leaf_node_filename,
+                                                   storage::io::FileWriter::HasNoFingerprint);
+
+            // pack M elements into leaf node, write to leaf file and add child index to the parent
+            // node
             uint64_t wrapped_element_index = 0;
             for (std::uint32_t node_index = 0; wrapped_element_index < element_count; ++node_index)
             {
@@ -238,10 +239,14 @@ class StaticRTree
                         Coordinate projected_v{
                             web_mercator::fromWGS84(Coordinate{m_coordinate_list[object.v]})};
 
-                    BOOST_ASSERT(std::abs(toFloating(projected_u.lon).operator double()) <= 180.);
+                        BOOST_ASSERT(std::abs(toFloating(projected_u.lon).operator double()) <=
-                    BOOST_ASSERT(std::abs(toFloating(projected_u.lat).operator double()) <= 180.);
+                                     180.);
-                    BOOST_ASSERT(std::abs(toFloating(projected_v.lon).operator double()) <= 180.);
+                        BOOST_ASSERT(std::abs(toFloating(projected_u.lat).operator double()) <=
-                    BOOST_ASSERT(std::abs(toFloating(projected_v.lat).operator double()) <= 180.);
+                                     180.);
+                        BOOST_ASSERT(std::abs(toFloating(projected_v.lon).operator double()) <=
+                                     180.);
+                        BOOST_ASSERT(std::abs(toFloating(projected_v.lat).operator double()) <=
+                                     180.);
 
                         rectangle.min_lon =
                             std::min(rectangle.min_lon, std::min(projected_u.lon, projected_v.lon));
@@ -264,13 +269,12 @@ class StaticRTree
                         current_leaf.minimum_bounding_rectangle);
 
                     // write leaf_node to leaf node file
-                leaf_node_file.write((char *)&current_leaf, sizeof(current_leaf));
+                    leaf_node_file.WriteOne(current_leaf);
                 }
 
                 tree_nodes_in_level.emplace_back(current_node);
             }
-        leaf_node_file.flush();
+        }
-        leaf_node_file.close();
 
         std::uint32_t processing_level = 0;
         while (1 < tree_nodes_in_level.size())
@@ -332,6 +336,7 @@ class StaticRTree
                 }
             });
 
+        {
             // open tree file
             storage::io::FileWriter tree_node_file(tree_node_filename,
                                                    storage::io::FileWriter::HasNoFingerprint);
@@ -341,6 +346,7 @@ class StaticRTree
 
             tree_node_file.WriteOne(size_of_tree);
             tree_node_file.WriteFrom(&m_search_tree[0], size_of_tree);
+        }
 
         MapLeafNodesFile(leaf_node_filename);
     }
@@ -379,6 +385,24 @@ class StaticRTree
             std::size_t num_leaves = m_leaves_region.size() / sizeof(LeafNode);
             auto data_ptr = m_leaves_region.data();
             BOOST_ASSERT(reinterpret_cast<uintptr_t>(data_ptr) % alignof(LeafNode) == 0);
+#ifndef NDEBUG
+            // Find the maximum leaf node ID and make sure we have that many from our file
+            BOOST_ASSERT(m_search_tree.size() > 0);
+            std::uint32_t max_leaf_node_id = 0;
+            // Search in reverse, the bottom of the tree is at the end of the array
+            for (auto iter = m_search_tree.rbegin(); iter != m_search_tree.rend(); iter++)
+            {
+                // If we find a non-leaf node, we can quit, we've seen the
+                // bottom level of the tree
+                if (iter->child_count > 0 && !iter->children[0].is_leaf)
+                    break;
+                for (std::uint32_t i = 0; i < iter->child_count; ++i)
+                {
+                    max_leaf_node_id = std::max(max_leaf_node_id, iter->children[i].index);
+                }
+            }
+            BOOST_ASSERT(max_leaf_node_id == num_leaves - 1);
+#endif
             m_leaves.reset(reinterpret_cast<const LeafNode *>(data_ptr), num_leaves);
         }
         catch (const std::exception &exc)

package.json

@@ -1,6 +1,6 @@
 {
   "name": "osrm",
-  "version": "5.7.2",
+  "version": "5.7.4",
   "private": false,
   "description": "The Open Source Routing Machine is a high performance routing engine written in C++14 designed to run on OpenStreetMap data.",
   "dependencies": {

src/engine/guidance/collapse_turns.cpp

@@ -53,7 +53,7 @@ double findTotalTurnAngle(const RouteStep &entry_step, const RouteStep &exit_ste
     // both angles are in the same direction, the total turn gets increased
     // 
     // a ---- b
-    //           \
+    //           \ 
     //              c
     //              |
     //              d
@@ -315,11 +315,6 @@ RouteSteps collapseTurnInstructions(RouteSteps steps)
         if (entersRoundabout(current_step->maneuver.instruction) ||
             staysOnRoundabout(current_step->maneuver.instruction))
         {
-            // Skip over all instructions within the roundabout
-            for (; current_step + 1 != steps.end(); ++current_step)
-                if (leavesRoundabout(current_step->maneuver.instruction))
-                    break;
-
             // are we done for good?
             if (current_step + 1 == steps.end())
                 break;

src/engine/guidance/post_processing.cpp

@@ -171,8 +171,10 @@ void closeOffRoundabout(const bool on_roundabout,
         if (!guidance::haveSameMode(exit_step, prev_step))
         {
             BOOST_ASSERT(leavesRoundabout(exit_step.maneuver.instruction));
-            prev_step.maneuver.instruction = exit_step.maneuver.instruction;
             if (!entersRoundabout(prev_step.maneuver.instruction))
+            {
+                prev_step.maneuver.instruction = exit_step.maneuver.instruction;
+            }
             prev_step.maneuver.exit = exit_step.maneuver.exit;
             exit_step.maneuver.instruction.type = TurnType::Notification;
             step_index--;
@@ -198,9 +200,12 @@ void closeOffRoundabout(const bool on_roundabout,
         {
             auto &propagation_step = steps[propagation_index];
             auto &next_step = steps[propagation_index + 1];
+            if (guidance::haveSameMode(propagation_step, next_step))
+            {
                 propagation_step.ElongateBy(next_step);
                 propagation_step.maneuver.exit = next_step.maneuver.exit;
                 next_step.Invalidate();
+            }
 
             if (entersRoundabout(propagation_step.maneuver.instruction))
             {

src/extractor/extraction_containers.cpp

@@ -173,7 +173,7 @@ void ExtractionContainers::WriteCharData(const std::string &file_name)
     util::UnbufferedLog log;
     log << "writing street name index ... ";
     TIMER_START(write_index);
-    boost::filesystem::ofstream file(file_name, std::ios::binary);
+    storage::io::FileWriter file(file_name, storage::io::FileWriter::HasNoFingerprint);
 
     const util::NameTable::IndexedData indexed_data;
     indexed_data.write(file, name_offsets.begin(), name_offsets.end(), name_char_data.begin());

unit_tests/util/indexed_data.cpp

@@ -1,6 +1,8 @@
 #include "util/indexed_data.hpp"
 #include "util/exception.hpp"
 
+#include <boost/filesystem.hpp>
+
 #include <boost/test/test_case_template.hpp>
 #include <boost/test/unit_test.hpp>
 
@@ -34,11 +36,18 @@ BOOST_AUTO_TEST_CASE(check_variable_group_block_bitops)
 template <typename IndexedData, typename Offsets, typename Data>
 void test_rw(const Offsets &offsets, const Data &data)
 {
-    std::stringstream sstr;
     IndexedData indexed_data;
-    indexed_data.write(sstr, offsets.begin(), offsets.end(), data.begin());
+    auto path = boost::filesystem::unique_path();
 
-    const std::string str = sstr.str();
+    {
+        storage::io::FileWriter writer(path, storage::io::FileWriter::HasNoFingerprint);
+        indexed_data.write(writer, offsets.begin(), offsets.end(), data.begin());
+    }
+
+    storage::io::FileReader reader(path, storage::io::FileReader::HasNoFingerprint);
+    auto length = reader.GetSize();
+    std::string str(length, '\0');
+    reader.ReadInto(const_cast<char *>(str.data()), length);
 
 #if 0
     std::cout << "\n" << typeid(IndexedData).name() << "\nsaved size = " << str.size() << "\n";
@@ -175,14 +184,21 @@ BOOST_AUTO_TEST_CASE(check_corrupted_memory)
 
 BOOST_AUTO_TEST_CASE(check_string_view)
 {
-    std::stringstream sstr;
+    auto path = boost::filesystem::unique_path();
     std::string name_data = "hellostringview";
     std::vector<std::uint32_t> name_offsets = {0, 5, 11, 15};
 
     IndexedData<VariableGroupBlock<16, StringView>> indexed_data;
-    indexed_data.write(sstr, name_offsets.begin(), name_offsets.end(), name_data.begin());
+    {
+        storage::io::FileWriter writer(path, storage::io::FileWriter::HasNoFingerprint);
+        indexed_data.write(writer, name_offsets.begin(), name_offsets.end(), name_data.begin());
+    }
+
+    storage::io::FileReader reader(path, storage::io::FileReader::HasNoFingerprint);
+    auto length = reader.GetSize();
+    std::string str(length, '\0');
+    reader.ReadInto(const_cast<char *>(str.data()), length);
 
-    const std::string str = sstr.str();
     indexed_data.reset(str.c_str(), str.c_str() + str.size());
 
     BOOST_CHECK_EQUAL(indexed_data.at(0), "hello");

unit_tests/util/name_table.cpp

@@ -1,6 +1,7 @@
 #include "util/name_table.hpp"
 #include "util/exception.hpp"
 
+#include <boost/filesystem.hpp>
 #include <boost/test/test_case_template.hpp>
 #include <boost/test/unit_test.hpp>
 
@@ -19,7 +20,6 @@ using namespace osrm::util;
 
 std::string PrapareNameTableData(std::vector<std::string> &data, bool fill_all)
 {
-    std::stringstream sstr;
     NameTable::IndexedData indexed_data;
     std::vector<unsigned char> name_char_data;
     std::vector<std::uint32_t> name_offsets;
@@ -54,9 +54,19 @@ std::string PrapareNameTableData(std::vector<std::string> &data, bool fill_all)
     }
     name_offsets.push_back(name_char_data.size());
 
-    indexed_data.write(sstr, name_offsets.begin(), name_offsets.end(), name_char_data.begin());
+    auto path = boost::filesystem::unique_path();
+    {
+        storage::io::FileWriter writer(path, storage::io::FileWriter::HasNoFingerprint);
+        indexed_data.write(
+            writer, name_offsets.begin(), name_offsets.end(), name_char_data.begin());
+    }
 
-    return sstr.str();
+    storage::io::FileReader reader(path, storage::io::FileReader::HasNoFingerprint);
+    auto length = reader.GetSize();
+    std::string str(length, '\0');
+    reader.ReadInto(const_cast<char *>(str.data()), length);
+
+    return str;
 }
 
 BOOST_AUTO_TEST_CASE(check_name_table_fill)