Fix incomplete segments (#144)

* Ensure segments contain at least 2 points * Ensure no duplicated shape offsets are stored
motis-project · Oct 10, 2024 · 6a37889 · 6a37889
1 parent 27f6c49
commit 6a37889
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 8 deletions.
diff --git a/src/loader/gtfs/shape_prepare.cc b/src/loader/gtfs/shape_prepare.cc
@@ -34,12 +34,15 @@ std::vector<shape_offset_t> get_offsets_by_stops(
     timetable const& tt,
     std::span<geo::latlng const> shape,
     stop_seq_t const& stop_seq) {
-  if (shape.empty()) {
+  // Need at least 1 shape point per stop
+  if (shape.size() < stop_seq.size()) {
     return {};
   }
 
   auto offsets = std::vector<shape_offset_t>(stop_seq.size());
-  auto remaining_start = cista::base_t<shape_offset_t>{0U};
+  auto remaining_start = cista::base_t<shape_offset_t>{1U};
+  // Reserve space to map each stop to a different point
+  auto max_width = shape.size() - stop_seq.size();
 
   for (auto const [i, s] : utl::enumerate(stop_seq)) {
     if (i == 0U) {
@@ -48,8 +51,11 @@ std::vector<shape_offset_t> get_offsets_by_stops(
       offsets[i] = shape_offset_t{shape.size() - 1U};
     } else {
       auto const pos = tt.locations_.coordinates_[stop{s}.location_idx()];
-      remaining_start += get_closest(pos, shape.subspan(remaining_start));
-      offsets[i] = shape_offset_t{remaining_start};
+      auto const offset =
+          get_closest(pos, shape.subspan(remaining_start, max_width + 1U));
+      offsets[i] = shape_offset_t{remaining_start + offset};
+      remaining_start += offset + 1U;
+      max_width -= offset;
     }
   }
 

diff --git a/src/rt/frun.cc b/src/rt/frun.cc
@@ -396,11 +396,14 @@ void frun::for_each_shape_point(
     }
     return absolute_range << stop_range_.from_;
   };
-  constexpr auto kInvalidLatLng = geo::latlng{200, 200};
-  auto consume_pos = [&, last_pos =
-                             kInvalidLatLng](geo::latlng const& pos) mutable {
-    if (pos != last_pos) {
+  auto start_pos = (*this)[range.from_].pos();
+  callback(start_pos);
+  auto consume_pos = [&, last_pos = std::move(start_pos), changed = false](
+                         geo::latlng const& pos,
+                         bool const force_if_unchanged = false) mutable {
+    if (pos != last_pos || (force_if_unchanged && !changed)) {
       callback(pos);
+      changed = true;
     }
     last_pos = pos;
   };
@@ -433,6 +436,7 @@ void frun::for_each_shape_point(
                  get_subshape(common_stops, trip_idx, absolute_trip_start));
     }
   }
+  consume_pos((*this)[static_cast<stop_idx_t>(range.to_ - 1)].pos(), true);
 }
 
 trip_id frun::id() const noexcept {

diff --git a/test/rt/frun_shape_test.cc b/test/rt/frun_shape_test.cc
@@ -83,6 +83,7 @@ ROUTE_1,SERVICE_1,TRIP_9,E,BLOCK_5,,
 ROUTE_1,SERVICE_1,TRIP_10,E,BLOCK_5,SHAPE_6,
 ROUTE_1,SERVICE_1,TRIP_11,E,BLOCK_6,SHAPE_7,
 ROUTE_1,SERVICE_1,TRIP_12,E,BLOCK_7,SHAPE_8,
+ROUTE_1,SERVICE_1,TRIP_13,E,BLOCK_8,SHAPE_9,
 
 # shapes.txt
 "shape_id","shape_pt_lat","shape_pt_lon","shape_pt_sequence","shape_dist_traveled"
@@ -137,6 +138,11 @@ SHAPE_8,1.0,1.0,0,0.0
 SHAPE_8,1.5,1.5,1,0.0
 SHAPE_8,2.0,2.0,2,2.0
 SHAPE_8,3.0,3.0,3,4.0
+SHAPE_9,1.0,1.0,0,
+SHAPE_9,2.5,2.5,1,
+SHAPE_9,3.625,3.625,2,
+SHAPE_9,4.0,4.0,3,
+SHAPE_9,4.0,4.0,4,
 
 # stop_times.txt
 trip_id,arrival_time,departure_time,stop_id,stop_sequence,pickup_type,drop_off_type,shape_dist_traveled
@@ -183,6 +189,11 @@ TRIP_11,13:00:00,13:00:00,O,4,0,0,4.24
 TRIP_12,10:00:00,10:00:00,A,1,0,0,0.0
 TRIP_12,11:00:00,11:00:00,M,2,0,0,2.0
 TRIP_12,12:00:00,12:00:00,N,3,0,0,4.0
+TRIP_13,10:00:00,10:00:00,A,1,0,0,
+TRIP_13,11:00:00,11:00:00,M,2,0,0,
+TRIP_13,12:00:00,12:00:00,N,3,0,0,
+TRIP_13,13:00:00,13:00:00,O,4,0,0,
+TRIP_13,13:05:00,13:05:00,O,5,0,0,
 
 )"sv;
 
@@ -648,6 +659,7 @@ TEST(
           plot_point);
 
       EXPECT_EQ((geo::polyline{
+                    {2.0F, 2.0F},
                     {2.5F, 2.5F},
                     {3.0F, 3.0F},
                 }),
@@ -681,6 +693,48 @@ TEST(
                 leg_shape);
     }
   }
+  // Segments must always contain at least 2 points
+  {
+    // Create run
+    transit_realtime::TripDescriptor td;
+    td.set_trip_id("TRIP_13");
+    auto const [r, t] = rt::gtfsrt_resolve_run(
+        date::sys_days{2024_y / January / 1}, tt, rtt, source_idx_t{0}, td);
+    ASSERT_TRUE(r.valid());
+    // Create full run
+    auto const full_run = rt::frun{tt, &rtt, r};
+
+    // Two stops close to same shape point (1.5, 1.5)
+    {
+      leg_shape.clear();
+
+      full_run.for_each_shape_point(
+          &shapes_data, interval{stop_idx_t{1U}, stop_idx_t{2U + 1U}},
+          plot_point);
+
+      EXPECT_EQ((geo::polyline{
+                    {2.0F, 2.0F},
+                    {2.5F, 2.5F},
+                    {3.625F, 3.625F},
+                    {3.0F, 3.0F},
+                }),
+                leg_shape);
+    }
+    // Duplicated end stop
+    {
+      leg_shape.clear();
+
+      full_run.for_each_shape_point(
+          &shapes_data, interval{stop_idx_t{3U}, stop_idx_t{4U + 1U}},
+          plot_point);
+
+      EXPECT_EQ((geo::polyline{
+                    {4.0F, 4.0F},
+                    {4.0F, 4.0F},
+                }),
+                leg_shape);
+    }
+  }
 }
 
 }  // namespace