improve fix and add test

Core/include/Acts/Propagator/DirectNavigator.hpp

@@ -196,20 +196,16 @@ class DirectNavigator {
       ACTS_VERBOSE("Current surface set to nullptr");
     }
 
-    // Find initial index. Because the list of surfaces has the same order
-    // regardless the direction, we need to increment the surfaceIndex manually
-    // instead of calling nextSurface
-    bool foundStartSurface = false;
-    state.surfaceIndex = 0;
-    for (const Surface* surface : state.options.surfaces) {
-      if (surface == state.currentSurface) {
-        foundStartSurface = true;
-        break;
-      }
-      state.surfaceIndex++;
-    }
-    ACTS_VERBOSE("Initial surface index set to " << state.surfaceIndex);
-    if (!foundStartSurface) {
+    // Find initial index.
+    auto found =
+        std::ranges::find(state.options.surfaces, state.options.startSurface);
+
+    if (found != state.options.surfaces.end()) {
+      // The index should be the index before the start surface, depending on
+      // the direction
+      state.surfaceIndex = std::distance(state.options.surfaces.begin(), found);
+      state.surfaceIndex += state.direction == Direction::Backward() ? 1 : -1;
+    } else {
       ACTS_DEBUG(
           "Did not find the start surface in the sequence. Assuming it is not "
           "part of the sequence. Trusting the correctness of the input "

Tests/UnitTests/Core/Propagator/DirectNavigatorTests.cpp

@@ -22,7 +22,9 @@
 #include "Acts/Propagator/Navigator.hpp"
 #include "Acts/Propagator/Propagator.hpp"
 #include "Acts/Propagator/StandardAborters.hpp"
+#include "Acts/Propagator/StraightLineStepper.hpp"
 #include "Acts/Propagator/SurfaceCollector.hpp"
+#include "Acts/Surfaces/PlaneSurface.hpp"
 #include "Acts/Tests/CommonHelpers/CylindricalTrackingGeometry.hpp"
 #include "Acts/Tests/CommonHelpers/FloatComparisons.hpp"
 
@@ -192,4 +194,91 @@ BOOST_DATA_TEST_CASE(
   runTest(rpropagator, dpropagator, pT, phi, theta, charge, index);
 }
 
+struct NavigationBreakAborter {
+  bool checkAbort(const auto& state, const auto& /*stepper*/, const auto& nav,
+                  const auto& /*logger*/) const {
+    return nav.navigationBreak(state.navigation);
+  }
+};
+
+/// Run a simple test with a sequence of surfaces to check if fwd and backward
+/// navigation works
+template <std::ranges::range ref_surfaces_t>
+void runSimpleTest(const std::vector<const Surface*>& surfaces,
+                   Direction direction, const Surface* startSurface,
+                   ref_surfaces_t expectedSurfaces) {
+  DirectNavigator dnav;
+  StraightLineStepper stepper;
+  Propagator<StraightLineStepper, DirectNavigator> prop(std::move(stepper), std::move(
+                                                                                dnav) /*, Acts::getDefaultLogger("DirectNavigatorTest", Acts::Logging::VERBOSE)*/);
+
+  using DirectActorList = ActorList<SurfaceCollector<>, NavigationBreakAborter>;
+  using DirectOptions =
+      typename Propagator<StraightLineStepper,
+                          DirectNavigator>::template Options<DirectActorList>;
+  DirectOptions pOptions(tgContext, mfContext);
+  pOptions.direction = direction;
+  pOptions.navigation.surfaces = surfaces;
+  pOptions.navigation.startSurface = startSurface;
+  auto& dCollector = pOptions.actorList.template get<SurfaceCollector<>>();
+  dCollector.selector.selectSensitive = true;
+  dCollector.selector.selectMaterial = true;
+  dCollector.selector.selectPassive = true;
+
+  // Create the start parameters in the middle of the start surface
+  BoundTrackParameters startParameters = BoundTrackParameters(
+      startSurface->getSharedPtr(),
+      {0.0_mm, 0.0_mm, 0.0_rad, 0.0_rad, 1.0 / 1.0_GeV, 0.0_ns}, std::nullopt,
+      ParticleHypothesis::muon());
+
+  // Propagate the track
+  auto result = prop.propagate(startParameters, pOptions);
+
+  // Check if the result is valid
+  BOOST_REQUIRE(result.ok());
+
+  // Check if the surfaces are the same
+  const auto& collectedSurfaceHits =
+      result->get<SurfaceCollector<>::result_type>().collected;
+  std::vector<const Surface*> collectedSurfaces;
+  std::ranges::transform(collectedSurfaceHits,
+                         std::back_inserter(collectedSurfaces),
+                         [](const auto& hit) { return hit.surface; });
+  // the initial surface is twice in the collection
+  collectedSurfaces.erase(
+      std::unique(collectedSurfaces.begin(), collectedSurfaces.end()),
+      collectedSurfaces.end());
+  BOOST_CHECK_EQUAL_COLLECTIONS(
+      collectedSurfaces.begin(), collectedSurfaces.end(),
+      expectedSurfaces.begin(), expectedSurfaces.end());
+}
+
+BOOST_AUTO_TEST_CASE(test_direct_navigator_fwd_bwd) {
+  // Create 10 surfaces at z = 0, 100, 200, ..., 900
+  std::vector<std::shared_ptr<const Acts::Surface>> surfaces;
+  for (int i = 0; i < 10; i++) {
+    Transform3 transform = Transform3::Identity();
+    transform.translate(Vector3{0.0_mm, 0.0_mm, i * 100.0_mm});
+    auto surface = Surface::makeShared<PlaneSurface>(transform, nullptr);
+    surface->assignGeometryId(
+        Acts::GeometryIdentifier().setVolume(1).setLayer(1).setSensitive(i +
+                                                                         1));
+    surfaces.push_back(surface);
+  }
+
+  // Create vector of pointers to the surfaces
+  std::vector<const Acts::Surface*> surfacePointers;
+  std::ranges::transform(surfaces, std::back_inserter(surfacePointers),
+                         [](const auto& s) { return s.get(); });
+
+  for (auto it = surfacePointers.begin(); it != surfacePointers.end(); ++it) {
+    runSimpleTest(surfacePointers, Direction::Forward(), *it,
+                  std::ranges::subrange{it, surfacePointers.end()});
+  }
+  for (auto it = surfacePointers.rbegin(); it != surfacePointers.rend(); ++it) {
+    runSimpleTest(surfacePointers, Direction::Backward(), *it,
+                  std::ranges::subrange{it, surfacePointers.rend()});
+  }
+}
+
 }  // namespace Acts::Test