refactor: Remove representation from ObjectIntersection (acts-pro…

…ject#2760)

As the templating of the representing object was rather a burden in
acts-project#2625 I propose to remove it
all together and compose the intersection if necessary. Here I used a
`std::pair` in the `Navigator` for composition for now which may be not
optimal but it works as it should for now

Core/include/Acts/Geometry/TrackingVolume.hpp

@@ -69,17 +69,19 @@ using LayerArray = BinnedArray<LayerPtr>;
 using LayerVector = std::vector<LayerPtr>;
 
 /// Intersection with @c Layer
-using LayerIntersection = ObjectIntersection<Layer, Surface>;
+using LayerIntersection = std::pair<SurfaceIntersection, const Layer*>;
 /// Multi-intersection with @c Layer
-using LayerMultiIntersection = ObjectMultiIntersection<Layer, Surface>;
+using LayerMultiIntersection =
+    std::pair<SurfaceMultiIntersection, const Layer*>;
 
 /// BoundarySurface of a volume
 using BoundarySurface = BoundarySurfaceT<TrackingVolume>;
 /// Intersection with a @c BoundarySurface
-using BoundaryIntersection = ObjectIntersection<BoundarySurface, Surface>;
+using BoundaryIntersection =
+    std::pair<SurfaceIntersection, const BoundarySurface*>;
 /// Multi-intersection with a @c BoundarySurface
 using BoundaryMultiIntersection =
-    ObjectMultiIntersection<BoundarySurface, Surface>;
+    std::pair<SurfaceMultiIntersection, const BoundarySurface*>;
 
 /// @class TrackingVolume
 ///

Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp

@@ -662,7 +662,7 @@ class MultiEigenStepperLoop
   template <typename object_intersection_t>
   void updateStepSize(State& state, const object_intersection_t& oIntersection,
                       Direction direction, bool release = true) const {
-    const Surface& surface = *oIntersection.representation();
+    const Surface& surface = *oIntersection.object();
 
     for (auto& component : state.components) {
       auto intersection = surface.intersect(

Core/include/Acts/Propagator/Navigator.hpp

@@ -525,7 +525,7 @@ class Navigator {
         state.navigation.lastHierarchySurfaceReached = false;
         // Update volume information
         // get the attached volume information
-        auto boundary = state.navigation.navBoundary().object();
+        auto boundary = state.navigation.navBoundary().second;
         state.navigation.currentVolume = boundary->attachedVolume(
             state.geoContext, stepper.position(state.stepping),
             stepper.direction(state.stepping), state.options.direction);
@@ -568,6 +568,19 @@ class Navigator {
   }
 
  private:
+  const SurfaceIntersection& candidateIntersection(
+      const NavigationSurfaces& surfaces, std::size_t index) const {
+    return surfaces.at(index);
+  }
+  const SurfaceIntersection& candidateIntersection(
+      const NavigationLayers& surfaces, std::size_t index) const {
+    return surfaces.at(index).first;
+  }
+  const SurfaceIntersection& candidateIntersection(
+      const NavigationBoundaries& surfaces, std::size_t index) const {
+    return surfaces.at(index).first;
+  }
+
   /// @brief Status call for test surfaces (surfaces, layers, boundaries)
   ///
   /// If there are surfaces to be handled, check if the current
@@ -592,9 +605,9 @@ class Navigator {
     if (navSurfaces.empty() || navIndex == navSurfaces.size()) {
       return false;
     }
-    const auto& intersection = navSurfaces.at(navIndex);
+    const auto& intersection = candidateIntersection(navSurfaces, navIndex);
     // Take the current surface
-    auto surface = intersection.representation();
+    const auto* surface = intersection.object();
     // Check if we are at a surface
     // If we are on the surface pointed at by the index, we can make
     // it the current one to pass it to the other actors
@@ -844,9 +857,9 @@ class Navigator {
     // loop over the available navigation layer candidates
     while (state.navigation.navLayerIndex !=
            state.navigation.navLayers.size()) {
-      const auto& intersection = state.navigation.navLayer();
+      const auto& intersection = state.navigation.navLayer().first;
       // The layer surface
-      const auto* layerSurface = intersection.representation();
+      const auto* layerSurface = intersection.object();
       // We are on the layer
       if (state.navigation.currentSurface == layerSurface) {
         ACTS_VERBOSE(volInfo(state) << "We are on a layer, resolve Surfaces.");
@@ -976,15 +989,16 @@ class Navigator {
         os << state.navigation.navBoundaries.size();
         os << " boundary candidates found at path(s): ";
         for (auto& bc : state.navigation.navBoundaries) {
-          os << bc.pathLength() << "  ";
+          os << bc.first.pathLength() << "  ";
         }
         logger().log(Logging::VERBOSE, os.str());
       }
       // Set the begin index
       state.navigation.navBoundaryIndex = 0;
       if (!state.navigation.navBoundaries.empty()) {
         // Set to the first and return to the stepper
-        stepper.updateStepSize(state.stepping, state.navigation.navBoundary(),
+        stepper.updateStepSize(state.stepping,
+                               state.navigation.navBoundary().first,
                                state.options.direction, true);
         ACTS_VERBOSE(volInfo(state) << "Navigation stepSize updated to "
                                     << stepper.outputStepSize(state.stepping));
@@ -1001,9 +1015,9 @@ class Navigator {
     // Loop over the boundary surface
     while (state.navigation.navBoundaryIndex !=
            state.navigation.navBoundaries.size()) {
-      const auto& intersection = state.navigation.navBoundary();
+      const auto& intersection = state.navigation.navBoundary().first;
       // That is the current boundary surface
-      const auto* boundarySurface = intersection.representation();
+      const auto* boundarySurface = intersection.object();
       // Step towards the boundary surfrace
       auto boundaryStatus = stepper.updateSurfaceStatus(
           state.stepping, *boundarySurface, intersection.index(),
@@ -1133,8 +1147,8 @@ class Navigator {
                        const Layer* cLayer = nullptr) const {
     // get the layer and layer surface
     auto layerSurface = cLayer ? state.navigation.startSurface
-                               : state.navigation.navLayer().representation();
-    auto navLayer = cLayer ? cLayer : state.navigation.navLayer().object();
+                               : state.navigation.navLayer().first.object();
+    auto navLayer = cLayer ? cLayer : state.navigation.navLayer().second;
     // are we on the start layer
     bool onStart = (navLayer == state.navigation.startLayer);
     auto startSurface = onStart ? state.navigation.startSurface : layerSurface;
@@ -1248,18 +1262,19 @@ class Navigator {
         os << state.navigation.navLayers.size();
         os << " layer candidates found at path(s): ";
         for (auto& lc : state.navigation.navLayers) {
-          os << lc.pathLength() << "  ";
+          os << lc.first.pathLength() << "  ";
         }
         logger().log(Logging::VERBOSE, os.str());
       }
       // Set the index to the first
       state.navigation.navLayerIndex = 0;
       // Setting the step size towards first
       if (state.navigation.startLayer &&
-          state.navigation.navLayer().object() != state.navigation.startLayer) {
+          state.navigation.navLayer().second != state.navigation.startLayer) {
         ACTS_VERBOSE(volInfo(state) << "Target at layer.");
         // The stepper updates the step size ( single / multi component)
-        stepper.updateStepSize(state.stepping, state.navigation.navLayer(),
+        stepper.updateStepSize(state.stepping,
+                               state.navigation.navLayer().first,
                                state.options.direction, true);
         ACTS_VERBOSE(volInfo(state) << "Navigation stepSize updated to "
                                     << stepper.outputStepSize(state.stepping));

Core/include/Acts/Utilities/Intersection.hpp

@@ -117,38 +117,18 @@ using MultiIntersection3D =
     boost::container::static_vector<Intersection3D,
                                     s_maximumNumberOfIntersections>;
 
-/// @brief class extensions to return also the object and a representation
-template <typename object_t, typename representation_t = object_t>
+template <typename object_t>
 class ObjectIntersection {
  public:
-  /// Object intersection - symmetric setup
-  ///
-  /// @param intersection is the intersection
-  /// @param object is the object to be instersected
-  /// @param index is the intersection index
-  template <typename T = representation_t,
-            std::enable_if_t<std::is_same<T, object_t>::value, int> = 0>
-  constexpr ObjectIntersection(const Intersection3D& intersection,
-                               const object_t* object, std::uint8_t index = 0)
-      : m_intersection(intersection),
-        m_object(object),
-        m_representation(object),
-        m_index(index) {}
-
   /// Object intersection
   ///
   /// @param intersection is the intersection
   /// @param object is the object to be instersected
   /// @param representation is the object representation
   /// @param index is the intersection index
   constexpr ObjectIntersection(const Intersection3D& intersection,
-                               const object_t* object,
-                               const representation_t* representation,
-                               std::uint8_t index = 0)
-      : m_intersection(intersection),
-        m_object(object),
-        m_representation(representation),
-        m_index(index) {}
+                               const object_t* object, std::uint8_t index = 0)
+      : m_intersection(intersection), m_object(object), m_index(index) {}
 
   /// Returns whether the intersection was successful or not
   constexpr explicit operator bool() const {
@@ -173,10 +153,6 @@ class ObjectIntersection {
 
   constexpr const object_t* object() const { return m_object; }
 
-  constexpr const representation_t* representation() const {
-    return m_representation;
-  }
-
   constexpr std::uint8_t index() const { return m_index; }
 
   constexpr static ObjectIntersection invalid() { return ObjectIntersection(); }
@@ -198,59 +174,36 @@ class ObjectIntersection {
   Intersection3D m_intersection = Intersection3D::invalid();
   /// The object that was (tried to be) intersected
   const object_t* m_object = nullptr;
-  /// The representation of this object
-  const representation_t* m_representation = nullptr;
   /// The intersection index
   std::uint8_t m_index = 0;
 
   constexpr ObjectIntersection() = default;
 };
 
-/// @brief class extensions to return also the object and a representation
-template <typename object_t, typename representation_t = object_t>
+template <typename object_t>
 class ObjectMultiIntersection {
  public:
-  using SplitIntersections = boost::container::static_vector<
-      ObjectIntersection<object_t, representation_t>,
-      s_maximumNumberOfIntersections>;
-
-  /// Object intersection - symmetric setup
-  ///
-  /// @param intersections are the intersections
-  /// @param object is the object to be instersected
-  template <typename T = representation_t,
-            std::enable_if_t<std::is_same<T, object_t>::value, int> = 0>
-  constexpr ObjectMultiIntersection(const MultiIntersection3D& intersections,
-                                    const object_t* object)
-      : m_intersections(intersections),
-        m_object(object),
-        m_representation(object) {}
+  using SplitIntersections =
+      boost::container::static_vector<ObjectIntersection<object_t>,
+                                      s_maximumNumberOfIntersections>;
 
   /// Object intersection
   ///
   /// @param intersections are the intersections
   /// @param object is the object to be instersected
   /// @param representation is the object representation
   constexpr ObjectMultiIntersection(const MultiIntersection3D& intersections,
-                                    const object_t* object,
-                                    const representation_t* representation)
-      : m_intersections(intersections),
-        m_object(object),
-        m_representation(representation) {}
-
-  constexpr ObjectIntersection<object_t, representation_t> operator[](
-      std::uint8_t index) const {
-    return {m_intersections[index], m_object, m_representation, index};
+                                    const object_t* object)
+      : m_intersections(intersections), m_object(object) {}
+
+  constexpr ObjectIntersection<object_t> operator[](std::uint8_t index) const {
+    return {m_intersections[index], m_object, index};
   }
 
   constexpr std::size_t size() const { return m_intersections.size(); }
 
   constexpr const object_t* object() const { return m_object; }
 
-  constexpr const representation_t* representation() const {
-    return m_representation;
-  }
-
   constexpr SplitIntersections split() const {
     SplitIntersections result;
     for (std::size_t i = 0; i < size(); ++i) {
@@ -259,20 +212,18 @@ class ObjectMultiIntersection {
     return result;
   }
 
-  constexpr ObjectIntersection<object_t, representation_t> closest() const {
+  constexpr ObjectIntersection<object_t> closest() const {
     auto splitIntersections = split();
-    return *std::min_element(
-        splitIntersections.begin(), splitIntersections.end(),
-        ObjectIntersection<object_t, representation_t>::closestOrder);
+    return *std::min_element(splitIntersections.begin(),
+                             splitIntersections.end(),
+                             ObjectIntersection<object_t>::closestOrder);
   }
 
  private:
   /// The intersections
   MultiIntersection3D m_intersections;
   /// The object that was (tried to be) intersected
   const object_t* m_object = nullptr;
-  /// The representation of this object
-  const representation_t* m_representation = nullptr;
 };
 
 namespace detail {

Core/src/Geometry/TrackingVolume.cpp

@@ -504,9 +504,7 @@ Acts::TrackingVolume::compatibleBoundaries(
           ACTS_VERBOSE("- intersection path length "
                        << std::abs(sIntersection.pathLength())
                        << " < overstep limit " << std::abs(oLimit));
-          return BoundaryIntersection(sIntersection.intersection(), bSurface,
-                                      sIntersection.object(),
-                                      sIntersection.index());
+          return BoundaryIntersection(sIntersection, bSurface);
         }
         ACTS_VERBOSE("Can't force intersection: ");
         ACTS_VERBOSE("- intersection path length "
@@ -520,14 +518,12 @@ Acts::TrackingVolume::compatibleBoundaries(
                                     decltype(logger)>(
               sIntersection.intersection(), pLimit, oLimit,
               s_onSurfaceTolerance, logger)) {
-        return BoundaryIntersection(sIntersection.intersection(), bSurface,
-                                    sIntersection.object(),
-                                    sIntersection.index());
+        return BoundaryIntersection(sIntersection, bSurface);
       }
     }
 
     ACTS_VERBOSE("No intersection accepted");
-    return BoundaryIntersection::invalid();
+    return BoundaryIntersection(SurfaceIntersection::invalid(), bSurface);
   };
 
   /// Helper function to process boundary surfaces
@@ -548,7 +544,7 @@ Acts::TrackingVolume::compatibleBoundaries(
                                                 options.boundaryCheck);
         // Intersect and continue
         auto bIntersection = checkIntersection(bCandidate, bsIter.get());
-        if (bIntersection) {
+        if (bIntersection.first) {
           ACTS_VERBOSE(" - Proceed with surface");
           bIntersections.push_back(bIntersection);
         } else {
@@ -587,8 +583,8 @@ Acts::TrackingVolume::compatibleBoundaries(
   };
 
   std::sort(bIntersections.begin(), bIntersections.end(),
-            [&](const auto& a, const auto& b) {
-              return comparator(a.pathLength(), b.pathLength());
+            [&](const BoundaryIntersection& a, const BoundaryIntersection& b) {
+              return comparator(a.first.pathLength(), b.first.pathLength());
             });
   return bIntersections;
 }
@@ -623,9 +619,7 @@ Acts::TrackingVolume::compatibleLayers(
         if (atIntersection &&
             (atIntersection.object() != options.targetSurface) && withinLimit) {
           // create a layer intersection
-          lIntersections.push_back(LayerIntersection(
-              atIntersection.intersection(), tLayer, atIntersection.object(),
-              atIntersection.index()));
+          lIntersections.push_back(LayerIntersection(atIntersection, tLayer));
         }
       }
       // move to next one or break because you reached the end layer
@@ -634,7 +628,9 @@ Acts::TrackingVolume::compatibleLayers(
                    : tLayer->nextLayer(gctx, position, direction);
     }
     std::sort(lIntersections.begin(), lIntersections.end(),
-              LayerIntersection::forwardOrder);
+              [](const LayerIntersection& a, const LayerIntersection& b) {
+                return SurfaceIntersection::forwardOrder(a.first, b.first);
+              });
   }
   // and return
   return lIntersections;

Tests/UnitTests/Core/Propagator/NavigatorTests.cpp

@@ -508,7 +508,7 @@ BOOST_AUTO_TEST_CASE(Navigator_target_methods) {
   // The index should points to the begin
   BOOST_CHECK_EQUAL(state.navigation.navLayerIndex, 0);
   // Cache the beam pipe radius
-  double beamPipeR = perp(state.navigation.navLayer().position());
+  double beamPipeR = perp(state.navigation.navLayer().first.position());
   // step size has been updated
   CHECK_CLOSE_ABS(state.stepping.stepSize.value(), beamPipeR,
                   s_onSurfaceTolerance);