Merge branch 'master' into nn-log-dup-shortcuts

CHANGELOG.md

@@ -42,6 +42,7 @@
    * FIXED: CostMatrix for trivial routes with oneways [#4626](https://github.com/valhalla/valhalla/pull/4626)
    * FIXED: some entry points to creating geotiff isochrones output did not register the geotiff driver before attempting to use it [#4628](https://github.com/valhalla/valhalla/pull/4628)
    * FIXED: libgdal wasn't installed in docker image, so it never worked in docker [#4629](https://github.com/valhalla/valhalla/pull/4629)
+   * FIXED: CostMatrix shapes for routes against trivial oneways [#4633](https://github.com/valhalla/valhalla/pull/4633)
 * **Enhancement**
    * UPDATED: French translations, thanks to @xlqian [#4159](https://github.com/valhalla/valhalla/pull/4159)
    * CHANGED: -j flag for multithreaded executables to override mjolnir.concurrency [#4168](https://github.com/valhalla/valhalla/pull/4168)
@@ -101,6 +102,7 @@
    * ADDED: isochrone proper polygon support & pbf output for isochrone [#4575](https://github.com/valhalla/valhalla/pull/4575)
    * ADDED: return isotile grid as geotiff  [#4594](https://github.com/valhalla/valhalla/pull/4594)
    * ADDED: `ignore_non_vehicular_restrictions` parameter for truck costing [#4606](https://github.com/valhalla/valhalla/pull/4606)
+   * UPDATED: tz database to 2024a [#4643](https://github.com/valhalla/valhalla/pull/4643)
    * ADDED: log shortcuts which have the same start/end node [4632](https://github.com/valhalla/valhalla/pull/4632)
 
 ## Release Date: 2023-05-11 Valhalla 3.4.0

scripts/valhalla_build_timezones

@@ -21,7 +21,7 @@ fi
 rm -rf dist
 rm -f ./timezones-with-oceans.shapefile.zip
 
-url="https://github.com/evansiroky/timezone-boundary-builder/releases/download/2023d/timezones-with-oceans.shapefile.zip"
+url="https://github.com/evansiroky/timezone-boundary-builder/releases/download/2024a/timezones-with-oceans.shapefile.zip"
 
 echo "downloading timezone polygon file." 1>&2
 curl -L -s -o ./timezones-with-oceans.shapefile.zip ${url} || error_exit "curl failed for ${url}"

src/thor/costmatrix.cc

@@ -1243,9 +1243,10 @@ std::string CostMatrix::RecostFormPath(GraphReader& graphreader,
   auto source_vertex = PointLL{source_edge.ll().lng(), source_edge.ll().lat()};
   auto target_vertex = PointLL{target_edge.ll().lng(), target_edge.ll().lat()};
   std::vector<PointLL> points;
-  for (const auto& path_edge : path_edges) {
-    auto is_first_edge = path_edge == path_edges.front();
-    auto is_last_edge = path_edge == path_edges.back();
+  for (uint32_t i = 0; i < path_edges.size(); i++) {
+    auto& path_edge = path_edges[i];
+    auto is_first_edge = i == 0;
+    auto is_last_edge = i == (path_edges.size() - 1);
 
     const auto* de = graphreader.directededge(path_edge, tile);
     auto edge_shp = tile->edgeinfo(de).shape();

test/gurka/CMakeLists.txt

@@ -11,22 +11,17 @@ if(ENABLE_DATA_TOOLS)
   # "-Werror" flag
   # Avoid adding a filename to this list
   set(TESTS_WITH_WARNINGS
-    test_elevation.cc
     test_gtfs.cc
     test_instructions_roundabout.cc
-    test_landmarks.cc
     test_languages.cc
     test_locate.cc
-    test_match.cc
     test_multi_level_loki.cc
     test_osrm_serializer.cc
     test_pbf_api.cc
     test_phonemes.cc
     test_phonemes_w_langs.cc
     test_recost.cc
     test_time_tracking.cc
-    test_traffic.cc
-    test_traffic_smoothing.cc
     )
 
   ## Add executable targets

test/gurka/test_elevation.cc

@@ -220,7 +220,7 @@ TEST(Standalone, ElevationCompareToSkadi) {
     result.Parse(route_json.c_str());
 
     for (size_t leg_index = 0; leg_index < waypoints.size() - 1; ++leg_index) {
-      auto s =
+      [[maybe_unused]] auto s =
           rapidjson::get_child_optional(result, ("/trip/legs/" + std::to_string(leg_index) + "/shape")
                                                     .c_str());
 

test/gurka/test_landmarks.cc

@@ -620,7 +620,7 @@ TEST(LandmarkTest, TestLandmarksInManeuvers) {
           "Checking landmarks in maneuver failed: cannot find the maneuver in the expected result!");
     }
     ASSERT_EQ(result_landmarks.size(), expected->second.size());
-    for (auto i = 0; i < result_landmarks.size(); ++i) {
+    for (size_t i = 0; i < result_landmarks.size(); ++i) {
       EXPECT_EQ(result_landmarks[i], expected->second[i]);
     }
   }

test/gurka/test_match.cc

@@ -262,8 +262,9 @@ uint32_t speed_from_edge(const valhalla::Api& api, bool compare_with_previous_ed
               node.cost().elapsed_cost().seconds() - node.cost().transition_cost().seconds()) /
              3600.0;
     auto new_kmh = static_cast<uint32_t>(km / h + .5);
-    if (is_valid(kmh) && compare_with_previous_edge)
+    if (is_valid(kmh) && compare_with_previous_edge) {
       EXPECT_EQ(kmh, new_kmh);
+    }
     kmh = new_kmh;
   }
   return kmh;

test/gurka/test_matrix.cc

@@ -504,8 +504,9 @@ TEST(StandAlone, CostMatrixShapes) {
   /*
   EXPECT_EQ(result.matrix().shapes(0), encoded);
   EXPECT_EQ(res_doc.Parse(res.c_str())["sources_to_targets"].GetArray()[0].GetArray()[0].GetObject()["shape"],
-  encoded); res.erase();
+  encoded);
   */
+  res.erase();
 
   // trivial route reverse
   // has a bug: https://github.com/valhalla/valhalla/issues/4433, but it's band-aided for now
@@ -517,8 +518,9 @@ TEST(StandAlone, CostMatrixShapes) {
   /*
   EXPECT_EQ(result.matrix().shapes(0), encoded);
   EXPECT_EQ(res_doc.Parse(res.c_str())["sources_to_targets"].GetArray()[0].GetArray()[0].GetObject()["shape"],
-  encoded); res.erase();
+  encoded);
   */
+  res.erase();
 
   // timedistancematrix
 
@@ -788,19 +790,33 @@ TEST(StandAlone, CostMatrixTrivialRoutes) {
       {"CD", {{"highway", "residential"}}}, {"BE", {{"highway", "residential"}}},
       {"EF", {{"highway", "residential"}}}, {"FC", {{"highway", "residential"}}},
   };
-  const auto layout = gurka::detail::map_to_coordinates(ascii_map, 100);
+  auto layout = gurka::detail::map_to_coordinates(ascii_map, 100);
   auto map =
       gurka::buildtiles(layout, ways, {}, {}, VALHALLA_BUILD_DIR "test/data/costmatrix_trivial");
 
+  std::unordered_map<std::string, std::string> options = {{"/shape_format", "polyline6"}};
+
   // test the against-oneway case
   {
-    auto matrix = gurka::do_action(valhalla::Options::sources_to_targets, map, {"1"}, {"2"}, "auto");
+    auto matrix =
+        gurka::do_action(valhalla::Options::sources_to_targets, map, {"1"}, {"2"}, "auto", options);
     EXPECT_EQ(matrix.matrix().distances(0), 2200);
+
+    std::vector<PointLL> oneway_vertices;
+    for (auto& node : {"1", "C", "F", "E", "B", "2"}) {
+      oneway_vertices.push_back(layout[node]);
+    }
+    auto encoded = encode<std::vector<PointLL>>(oneway_vertices, 1e6);
+    EXPECT_EQ(matrix.matrix().shapes(0), encoded);
   }
 
   // test the normal trivial case
   {
-    auto matrix = gurka::do_action(valhalla::Options::sources_to_targets, map, {"3"}, {"4"}, "auto");
+    auto matrix =
+        gurka::do_action(valhalla::Options::sources_to_targets, map, {"3"}, {"4"}, "auto", options);
     EXPECT_EQ(matrix.matrix().distances(0), 400);
+
+    auto encoded = encode<std::vector<PointLL>>({layout["3"], layout["4"]}, 1e6);
+    EXPECT_EQ(matrix.matrix().shapes(0), encoded);
   }
 }

test/gurka/test_traffic.cc

@@ -296,8 +296,6 @@ TEST(Traffic, CutGeoms) {
                                          uint32_t index, baldr::TrafficSpeed* current) -> void {
         baldr::GraphId tile_id(tile.header->tile_id);
         auto BD = gurka::findEdge(reader, map.nodes, "BD", "D", tile_id);
-        baldr::TrafficSpeed* existing =
-            const_cast<valhalla::baldr::TrafficSpeed*>(tile.speeds + index);
         current->breakpoint1 = 255;
         if (std::get<1>(BD) != nullptr && std::get<0>(BD).id() == index) {
           current->overall_encoded_speed = 0;

test/gurka/test_traffic_smoothing.cc

@@ -558,7 +558,7 @@ class MapMatchWithTraffic : public ::testing::Test {
       traffic_speed->breakpoint1 = 255;
     });
 
-    test::customize_historical_traffic(map.config, [](DirectedEdge& e) {
+    test::customize_historical_traffic(map.config, [](DirectedEdge&) {
       // speeds for every 5 min bucket of the week
       std::array<float, kBucketsPerWeek> historical;
       historical.fill(6);