feat: adding visitSurfaces pattern to new detector (#2897)

This PR adds the `visitSurfaces(...)` pattern to the new Detector analog
to the pattern in the TrackingGeometry world.

This is a next step to harmonise the two branches for an eventual
transition to the new detector geometry.

It adds consistent testing of this setups as well.

---------

Co-authored-by: Alexander J. Pfleger <70842573+AJPfleger@users.noreply.github.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

Core/include/Acts/Detector/Detector.hpp

@@ -14,7 +14,9 @@
 #include "Acts/Geometry/GeometryContext.hpp"
 #include "Acts/Geometry/GeometryHierarchyMap.hpp"
 #include "Acts/Geometry/GeometryIdentifier.hpp"
+#include "Acts/Geometry/SurfaceVisitorConcept.hpp"
 #include "Acts/Navigation/NavigationDelegates.hpp"
+#include "Acts/Utilities/Concepts.hpp"
 #include "Acts/Utilities/Delegate.hpp"
 
 #include <cstddef>
@@ -100,6 +102,20 @@ class Detector : public std::enable_shared_from_this<Detector> {
   /// @return the map which can be queried with GeometryID for ranges
   const GeometryHierarchyMap<const Surface*>& sensitiveHierarchyMap() const;
 
+  /// @brief Visit all reachable surfaces of the detector
+  ///
+  /// @tparam visitor_t Type of the callable visitor
+  ///
+  /// @param visitor will be handed to each root volume,
+  /// eventually contained volumes within the root volumes are
+  /// handled by the root volume
+  template <ACTS_CONCEPT(SurfaceVisitor) visitor_t>
+  void visitSurfaces(visitor_t&& visitor) const {
+    for (const auto& v : rootVolumes()) {
+      v->template visitSurfaces<visitor_t>(std::forward<visitor_t>(visitor));
+    }
+  }
+
   /// Update the current volume of a given navigation state
   ///
   /// @param gctx is the Geometry context of the call

Core/include/Acts/Detector/DetectorVolume.hpp

@@ -10,16 +10,19 @@
 
 #include "Acts/Definitions/Algebra.hpp"
 #include "Acts/Definitions/Common.hpp"
+#include "Acts/Detector/Portal.hpp"
 #include "Acts/Detector/PortalGenerators.hpp"
 #include "Acts/Geometry/Extent.hpp"
 #include "Acts/Geometry/GeometryContext.hpp"
 #include "Acts/Geometry/GeometryIdentifier.hpp"
+#include "Acts/Geometry/SurfaceVisitorConcept.hpp"
 #include "Acts/Geometry/VolumeBounds.hpp"
 #include "Acts/Material/IVolumeMaterial.hpp"
 #include "Acts/Navigation/NavigationDelegates.hpp"
 #include "Acts/Navigation/NavigationState.hpp"
 #include "Acts/Surfaces/BoundaryCheck.hpp"
 #include "Acts/Utilities/BoundingBox.hpp"
+#include "Acts/Utilities/Concepts.hpp"
 #include "Acts/Utilities/Delegate.hpp"
 #include "Acts/Utilities/Helpers.hpp"
 
@@ -40,7 +43,6 @@ class VolumeBounds;
 namespace Experimental {
 
 class DetectorVolume;
-class Portal;
 class Detector;
 
 /// A detector volume description which can be:
@@ -282,6 +284,25 @@ class DetectorVolume : public std::enable_shared_from_this<DetectorVolume> {
   /// Const access to the detector volume updator
   const DetectorVolumeUpdater& detectorVolumeUpdater() const;
 
+  /// @brief Visit all reachable surfaces of the detector
+  ///
+  /// @tparam visitor_t Type of the callable visitor
+  ///
+  /// @param visitor will be called for each found surface,
+  /// it will be handed down to contained volumes and portals
+  template <ACTS_CONCEPT(SurfaceVisitor) visitor_t>
+  void visitSurfaces(visitor_t&& visitor) const {
+    for (const auto& s : surfaces()) {
+      visitor(s);
+    }
+    for (const auto& p : portals()) {
+      p->visitSurfaces(std::forward<visitor_t>(visitor));
+    }
+    for (const auto& v : volumes()) {
+      v->visitSurfaces(std::forward<visitor_t>(visitor));
+    }
+  }
+
   /// This method allows to udate the navigation state updator
   /// module.
   ///

Core/include/Acts/Detector/Portal.hpp

@@ -12,11 +12,13 @@
 #include "Acts/Definitions/Direction.hpp"
 #include "Acts/Geometry/GeometryContext.hpp"
 #include "Acts/Geometry/GeometryIdentifier.hpp"
+#include "Acts/Geometry/SurfaceVisitorConcept.hpp"
 #include "Acts/Navigation/NavigationDelegates.hpp"
 #include "Acts/Navigation/NavigationState.hpp"
 #include "Acts/Surfaces/BoundaryCheck.hpp"
 #include "Acts/Surfaces/RegularSurface.hpp"
 #include "Acts/Surfaces/Surface.hpp"
+#include "Acts/Utilities/Concepts.hpp"
 
 #include <array>
 #include <map>
@@ -67,6 +69,16 @@ class Portal {
   /// Non-const access to the surface reference
   RegularSurface& surface();
 
+  /// @brief Visit all reachable surfaces of the detector
+  ///
+  /// @tparam visitor_t Type of the callable visitor
+  ///
+  /// @param visitor will be called with the represented surface
+  template <ACTS_CONCEPT(SurfaceVisitor) visitor_t>
+  void visitSurfaces(visitor_t&& visitor) const {
+    visitor(m_surface.get());
+  }
+
   /// Update the current volume
   ///
   /// @param gctx is the Geometry context of this call

Tests/UnitTests/Core/Detector/DetectorTests.cpp

@@ -112,6 +112,24 @@ BOOST_AUTO_TEST_CASE(DetectorConstruction) {
   BOOST_CHECK_EQUAL(
       det012, unpackToShared<const Acts::Experimental::Detector>(*det012));
 
+  // Check surface visiting
+  // Test the visitor pattern for surfaces
+  struct CountSurfaces {
+    unsigned int counter = 0;
+
+    void operator()(const Acts::Surface* s) {
+      if (s != nullptr) {
+        counter++;
+      }
+    }
+  };
+
+  CountSurfaces countSurfaces;
+  det012->visitSurfaces(countSurfaces);
+
+  // 3 innermost, 4 middle, 4 outermost
+  BOOST_CHECK_EQUAL(countSurfaces.counter, 11u);
+
   // Check the inside function with positions
   Acts::Experimental::NavigationState nState;
   nState.position = Acts::Vector3(5., 0., 0.);

Tests/UnitTests/Core/Detector/DetectorVolumeTests.cpp

@@ -183,6 +183,24 @@ BOOST_AUTO_TEST_CASE(CuboidWithCuboid) {
   // We should have 12 candidates, 6 inner, 6 outer portals but only 3 are
   // reachable
   BOOST_CHECK_EQUAL(nState.surfaceCandidates.size(), 3u);
+
+  // Check surface visiting
+  // Test the visitor pattern for surfaces
+  struct CountSurfaces {
+    unsigned int counter = 0;
+
+    void operator()(const Acts::Surface* s) {
+      if (s != nullptr) {
+        counter++;
+      }
+    }
+  };
+
+  CountSurfaces countSurfaces;
+  outerBox->visitSurfaces(countSurfaces);
+
+  // 6 portlas outer box, 6 portals inner box
+  BOOST_CHECK_EQUAL(countSurfaces.counter, 12u);
 }
 
 BOOST_AUTO_TEST_CASE(CylinderWithSurfacesTestExtractors) {
@@ -230,6 +248,23 @@ BOOST_AUTO_TEST_CASE(CylinderWithSurfacesTestExtractors) {
   BOOST_CHECK_EQUAL(esurfaces.size(), 2u);
   BOOST_CHECK_EQUAL(esurfaces[0u], surfaces[2u].get());
   BOOST_CHECK_EQUAL(esurfaces[1u], surfaces[4u].get());
+
+  // Test the visitor pattern for surfaces
+  struct CountSurfaces {
+    unsigned int counter = 0;
+
+    void operator()(const Acts::Surface* s) {
+      if (s != nullptr) {
+        counter++;
+      }
+    }
+  };
+
+  CountSurfaces countSurfaces;
+  cylinderVolume->visitSurfaces(countSurfaces);
+
+  // 6 internal surfaces, 4 portals -> 10 surfaces counted
+  BOOST_CHECK_EQUAL(countSurfaces.counter, 10u);
 }
 
 BOOST_AUTO_TEST_SUITE_END()

Tests/UnitTests/Core/Detector/PortalTests.cpp

@@ -236,6 +236,23 @@ BOOST_AUTO_TEST_CASE(PortalMaterialTest) {
   BOOST_CHECK_THROW(Portal::fuse(portalA, portalB), std::runtime_error);
   // Same in reverse
   BOOST_CHECK_THROW(Portal::fuse(portalB, portalA), std::runtime_error);
+
+  // Test the visitor pattern for the surface
+  struct ReachSurface {
+    bool reached = false;
+
+    void operator()(const Acts::Surface* s) {
+      if (s != nullptr) {
+        reached = true;
+      }
+    }
+  };
+
+  ReachSurface reachSurface;
+  portalB->visitSurfaces(reachSurface);
+
+  // The visitor should have reached the surface
+  BOOST_CHECK(reachSurface.reached);
 }
 
 BOOST_AUTO_TEST_SUITE_END()