refactor: Refactor navigation (acts-project#2768)

After acts-project#2625 turned out to be harder than initially thought I tried to factor out some changes to get them into main already.

Summary of the changes I made
- `pLimit` = path limit -> far limit = `farLimit`
- `oLimit` = overstep limit -> near limit = `nearLimit`
- replaced `DetectorNavigator` iterators with indices

Core/include/Acts/Geometry/ApproachDescriptor.hpp

@@ -43,17 +43,16 @@ class ApproachDescriptor {
   /// @param position is the position from start of the search
   /// @param direction is the direction at the start of the search
   /// @param bcheck is the boundary check directive
-  /// @param pLimit The path limit
-  /// @param oLimit The overstep limit
-  /// @param tolerance The surface tolerance
+  /// @param nearLimit The minimum distance for an intersection to be considered
+  /// @param farLimit The maximum distance for an intersection to be considered
   ///
   /// @return is a surface intersection
   virtual SurfaceIntersection approachSurface(const GeometryContext& gctx,
                                               const Vector3& position,
                                               const Vector3& direction,
                                               const BoundaryCheck& bcheck,
-                                              double pLimit, double oLimit,
-                                              double tolerance) const = 0;
+                                              double nearLimit,
+                                              double farLimit) const = 0;
 
   /// Get all the contained surfaces
   /// @return all contained surfaces of this approach descriptor

Core/include/Acts/Geometry/BoundarySurfaceT.hpp

@@ -103,13 +103,12 @@ class BoundarySurfaceT {
   ///
   /// @param gctx The current geometry context object, e.g. alignment
   /// @param pos The global position on surface
-  /// @param mom The direction on the surface
-  /// @param dir is an additional direction corrective
+  /// @param dir The direction on the surface
   ///
   /// @return The attached volume at that position
   virtual const volume_t* attachedVolume(const GeometryContext& gctx,
-                                         const Vector3& pos, const Vector3& mom,
-                                         Direction dir) const;
+                                         const Vector3& pos,
+                                         const Vector3& dir) const;
 
   /// templated onBoundary method
   ///
@@ -181,11 +180,10 @@ void BoundarySurfaceT<volume_t>::attachVolumeArray(
 
 template <class volume_t>
 const volume_t* BoundarySurfaceT<volume_t>::attachedVolume(
-    const GeometryContext& gctx, const Vector3& pos, const Vector3& mom,
-    Direction dir) const {
+    const GeometryContext& gctx, const Vector3& pos, const Vector3& dir) const {
   const volume_t* attVolume = nullptr;
   // dot product with normal vector to distinguish inside/outside
-  if ((surfaceRepresentation().normal(gctx, pos)).dot(dir * mom) > 0.) {
+  if ((surfaceRepresentation().normal(gctx, pos)).dot(dir) > 0.) {
     attVolume = m_alongVolumeArray ? m_alongVolumeArray->object(pos).get()
                                    : m_alongVolume;
   } else {

Core/include/Acts/Geometry/GenericApproachDescriptor.hpp

@@ -57,17 +57,16 @@ class GenericApproachDescriptor : public ApproachDescriptor {
   /// @param position The global position to start the approach from
   /// @param direction The momentum vector
   /// @param bcheck The boundary check prescription
-  /// @param pLimit The path limit
-  /// @param oLimit The overstep limit
-  /// @param tolerance The surface tolerance
+  /// @param nearLimit The minimum distance for an intersection to be considered
+  /// @param farLimit The maximum distance for an intersection to be considered
   ///
   /// @return : a @c SurfaceIntersection
   SurfaceIntersection approachSurface(const GeometryContext& gctx,
                                       const Vector3& position,
                                       const Vector3& direction,
                                       const BoundaryCheck& bcheck,
-                                      double pLimit, double oLimit,
-                                      double tolerance) const override;
+                                      double nearLimit,
+                                      double farLimit) const override;
 
   /// return all contained surfaces of this approach descriptor
   const std::vector<const Surface*>& containedSurfaces() const override;

Core/include/Acts/Geometry/NavigationLayer.hpp

@@ -26,7 +26,6 @@ namespace Acts {
 /// Class to be used for gaps in Volumes as a navigational link.
 /// Navigation Layers have a surface representation, but should usually never be
 /// propagated to.
-
 class NavigationLayer : public Layer {
  public:
   ///  Factory Constructor - the surface representation is given by pointer

Core/include/Acts/Navigation/DetectorNavigator.hpp

@@ -83,16 +83,6 @@ class DetectorNavigator {
     return result;
   }
 
-  void resetState(State& state, const GeometryContext& /*geoContext*/,
-                  const Vector3& /*pos*/, const Vector3& /*dir*/,
-                  const Surface* /*ssurface*/,
-                  const Surface* /*tsurface*/) const {
-    // Reset everything first
-    state = State();
-
-    // TODO fill state
-  }
-
   const Surface* currentSurface(const State& state) const {
     return state.currentSurface;
   }
@@ -202,24 +192,24 @@ class DetectorNavigator {
                    << posInfo(state, stepper) << "stepping through portal");
 
       nState.surfaceCandidates.clear();
-      nState.surfaceCandidate = nState.surfaceCandidates.cend();
+      nState.surfaceCandidateIndex = 0;
 
       nState.currentPortal->updateDetectorVolume(state.geoContext, nState);
 
       initializeTarget(state, stepper);
     }
 
-    for (; nState.surfaceCandidate != nState.surfaceCandidates.cend();
-         ++nState.surfaceCandidate) {
+    for (; nState.surfaceCandidateIndex != nState.surfaceCandidates.size();
+         ++nState.surfaceCandidateIndex) {
       // Screen output how much is left to try
       ACTS_VERBOSE(volInfo(state)
                    << posInfo(state, stepper)
-                   << std::distance(nState.surfaceCandidate,
-                                    nState.surfaceCandidates.cend())
+                   << (nState.surfaceCandidates.size() -
+                       nState.surfaceCandidateIndex)
                    << " out of " << nState.surfaceCandidates.size()
                    << " surfaces remain to try.");
       // Take the surface
-      const auto& c = *(nState.surfaceCandidate);
+      const auto& c = nState.surfaceCandidate();
       const auto& surface =
           (c.surface != nullptr) ? (*c.surface) : (c.portal->surface());
       // Screen output which surface you are on
@@ -271,7 +261,7 @@ class DetectorNavigator {
       return;
     }
 
-    if (nState.surfaceCandidate == nState.surfaceCandidates.cend()) {
+    if (nState.surfaceCandidateIndex == nState.surfaceCandidates.size()) {
       ACTS_VERBOSE(volInfo(state)
                    << posInfo(state, stepper)
                    << "no surface candidates - waiting for target call");
@@ -281,12 +271,12 @@ class DetectorNavigator {
     const Portal* nextPortal = nullptr;
     const Surface* nextSurface = nullptr;
     bool isPortal = false;
-    bool boundaryCheck = nState.surfaceCandidate->boundaryCheck;
+    bool boundaryCheck = nState.surfaceCandidate().boundaryCheck;
 
-    if (nState.surfaceCandidate->surface != nullptr) {
-      nextSurface = nState.surfaceCandidate->surface;
-    } else if (nState.surfaceCandidate->portal != nullptr) {
-      nextPortal = nState.surfaceCandidate->portal;
+    if (nState.surfaceCandidate().surface != nullptr) {
+      nextSurface = nState.surfaceCandidate().surface;
+    } else if (nState.surfaceCandidate().portal != nullptr) {
+      nextPortal = nState.surfaceCandidate().portal;
       nextSurface = &nextPortal->surface();
       isPortal = true;
     } else {
@@ -299,7 +289,7 @@ class DetectorNavigator {
     // TODO not sure about the boundary check
     auto surfaceStatus = stepper.updateSurfaceStatus(
         state.stepping, *nextSurface,
-        nState.surfaceCandidate->objectIntersection.index(),
+        nState.surfaceCandidate().objectIntersection.index(),
         state.options.direction, BoundaryCheck(boundaryCheck),
         state.options.surfaceTolerance, logger());
 
@@ -327,7 +317,7 @@ class DetectorNavigator {
         ACTS_VERBOSE(volInfo(state)
                      << posInfo(state, stepper) << "current surface set to "
                      << nState.currentSurface->geometryId());
-        ++nState.surfaceCandidate;
+        ++nState.surfaceCandidateIndex;
       }
     }
   }
@@ -426,7 +416,7 @@ class DetectorNavigator {
                 return pathToA < pathToB;
               });
     // Set the surface candidate
-    nState.surfaceCandidate = nCandidates.begin();
+    nState.surfaceCandidateIndex = 0;
   }
 
   template <typename propagator_state_t, typename stepper_t>

Core/include/Acts/Navigation/NavigationState.hpp

@@ -9,12 +9,14 @@
 #pragma once
 
 #include "Acts/Definitions/Algebra.hpp"
+#include "Acts/Definitions/Units.hpp"
 #include "Acts/Geometry/GeometryContext.hpp"
 #include "Acts/Surfaces/BoundaryCheck.hpp"
 #include "Acts/Utilities/Delegate.hpp"
 #include "Acts/Utilities/Intersection.hpp"
 
 #include <any>
+#include <cstddef>
 #include <vector>
 
 namespace Acts {
@@ -80,16 +82,20 @@ struct NavigationState {
 
   /// That are the candidate surfaces to process
   SurfaceCandidates surfaceCandidates = {};
-  SurfaceCandidates::const_iterator surfaceCandidate = surfaceCandidates.cend();
+  std::size_t surfaceCandidateIndex = 0;
 
   /// Boundary directives for surfaces
   BoundaryCheck surfaceBoundaryCheck = BoundaryCheck(true);
 
   /// An overstep tolerance
-  ActsScalar overstepTolerance = -0.1;
+  ActsScalar overstepTolerance = -100 * UnitConstants::um;
 
   /// Auxiliary attached information
   std::any auxiliary;
+
+  const SurfaceCandidate& surfaceCandidate() const {
+    return surfaceCandidates.at(surfaceCandidateIndex);
+  }
 };
 
 }  // namespace Experimental

Core/include/Acts/Navigation/NavigationStateUpdaters.hpp

@@ -12,6 +12,7 @@
 #include "Acts/Definitions/Common.hpp"
 #include "Acts/Detector/Portal.hpp"
 #include "Acts/Navigation/NavigationDelegates.hpp"
+#include "Acts/Navigation/NavigationState.hpp"
 #include "Acts/Surfaces/Surface.hpp"
 #include "Acts/Utilities/BinningType.hpp"
 #include "Acts/Utilities/Enumerate.hpp"
@@ -38,9 +39,10 @@ inline void updateCandidates(const GeometryContext& gctx,
                              NavigationState& nState) {
   const auto& position = nState.position;
   const auto& direction = nState.direction;
-  auto& nCandidates = nState.surfaceCandidates;
 
-  for (auto& c : nCandidates) {
+  NavigationState::SurfaceCandidates nextSurfaceCandidates;
+
+  for (NavigationState::SurfaceCandidate c : nState.surfaceCandidates) {
     // Get the surface representation: either native surfcae of portal
     const Surface& sRep =
         c.surface != nullptr ? *c.surface : c.portal->surface();
@@ -50,7 +52,14 @@ inline void updateCandidates(const GeometryContext& gctx,
     auto sIntersection = sRep.intersect(gctx, position, direction,
                                         c.boundaryCheck, s_onSurfaceTolerance);
     c.objectIntersection = sIntersection[c.objectIntersection.index()];
+
+    if (c.objectIntersection &&
+        c.objectIntersection.pathLength() > nState.overstepTolerance) {
+      nextSurfaceCandidates.emplace_back(std::move(c));
+    }
   }
+
+  nState.surfaceCandidates = std::move(nextSurfaceCandidates);
 }
 
 /// @brief  This sets a single object, e.g. single surface or single volume