refactor: Brush over new detector geometry part 1 (#2309)

- consistent pass by value
- reformat documentation

pull changes out of #2214

Core/include/Acts/Detector/Detector.hpp

@@ -37,16 +37,16 @@ class Detector : public std::enable_shared_from_this<Detector> {
   /// @note will throw an exception if volumes vector is empty
   /// @note will throw an exception if duplicate volume names exist
   /// @note will throw an exception if the delegate is not connected
-  Detector(const std::string& name,
+  Detector(std::string name,
            std::vector<std::shared_ptr<DetectorVolume>> rootVolumes,
-           DetectorVolumeUpdator&& detectorVolumeUpdator) noexcept(false);
+           DetectorVolumeUpdator detectorVolumeUpdator) noexcept(false);
 
  public:
   /// Factory for producing memory managed instances of Detector.
   static std::shared_ptr<Detector> makeShared(
-      const std::string& name,
+      std::string name,
       std::vector<std::shared_ptr<DetectorVolume>> rootVolumes,
-      DetectorVolumeUpdator&& detectorVolumeUpdator);
+      DetectorVolumeUpdator detectorVolumeUpdator);
 
   /// Retrieve a @c std::shared_ptr for this surface (non-const version)
   ///
@@ -119,8 +119,7 @@ class Detector : public std::enable_shared_from_this<Detector> {
   /// Update the volume finder
   ///
   /// @param detectorVolumeUpdator the new volume finder
-  void updateDetectorVolumeFinder(
-      DetectorVolumeUpdator&& detectorVolumeUpdator);
+  void updateDetectorVolumeFinder(DetectorVolumeUpdator detectorVolumeUpdator);
 
   /// Const access to the volume finder
   const DetectorVolumeUpdator& detectorVolumeFinder() const;
@@ -130,7 +129,7 @@ class Detector : public std::enable_shared_from_this<Detector> {
 
  private:
   /// Name of the detector
-  std::string m_name = "Unnamed";
+  std::string m_name;
 
   /// Root volumes
   DetectorVolume::ObjectStore<std::shared_ptr<DetectorVolume>> m_rootVolumes;

Core/include/Acts/Detector/DetectorVolume.hpp

@@ -106,12 +106,12 @@ class DetectorVolume : public std::enable_shared_from_this<DetectorVolume> {
   /// @note throws exception if ghe portal general or navigation
   ///       state updator delegates are not connected
   DetectorVolume(
-      const GeometryContext& gctx, const std::string& name,
+      const GeometryContext& gctx, std::string name,
       const Transform3& transform, std::shared_ptr<VolumeBounds> bounds,
       std::vector<std::shared_ptr<Surface>> surfaces,
       std::vector<std::shared_ptr<DetectorVolume>> volumes,
-      DetectorVolumeUpdator&& detectorVolumeUpdator,
-      SurfaceCandidatesUpdator&& surfaceCandidateUpdator) noexcept(false);
+      DetectorVolumeUpdator detectorVolumeUpdator,
+      SurfaceCandidatesUpdator surfaceCandidateUpdator) noexcept(false);
 
   /// Create a detector volume - empty/gap volume constructor
   ///
@@ -125,28 +125,28 @@ class DetectorVolume : public std::enable_shared_from_this<DetectorVolume> {
   /// @note throws exception if ghe portal general or navigation
   ///       state updator delegates are not connected
   DetectorVolume(
-      const GeometryContext& gctx, const std::string& name,
+      const GeometryContext& gctx, std::string name,
       const Transform3& transform, std::shared_ptr<VolumeBounds> bounds,
-      SurfaceCandidatesUpdator&& surfaceCandidateUpdator) noexcept(false);
+      SurfaceCandidatesUpdator surfaceCandidateUpdator) noexcept(false);
 
   /// Factory method for producing memory managed instances of DetectorVolume.
   ///
   /// @note This is called by the @class DetectorVolumeFactory
   static std::shared_ptr<DetectorVolume> makeShared(
-      const GeometryContext& gctx, const std::string& name,
+      const GeometryContext& gctx, std::string name,
       const Transform3& transform, std::shared_ptr<VolumeBounds> bounds,
       std::vector<std::shared_ptr<Surface>> surfaces,
       std::vector<std::shared_ptr<DetectorVolume>> volumes,
-      DetectorVolumeUpdator&& detectorVolumeUpdator,
-      SurfaceCandidatesUpdator&& surfaceCandidateUpdator);
+      DetectorVolumeUpdator detectorVolumeUpdator,
+      SurfaceCandidatesUpdator surfaceCandidateUpdator);
 
   /// Factory method for producing memory managed instances of DetectorVolume.
   ///
   /// @note This is called by the @class DetectorVolumeFactory
   static std::shared_ptr<DetectorVolume> makeShared(
-      const GeometryContext& gctx, const std::string& name,
+      const GeometryContext& gctx, std::string name,
       const Transform3& transform, std::shared_ptr<VolumeBounds> bounds,
-      SurfaceCandidatesUpdator&& surfaceCandidateUpdator);
+      SurfaceCandidatesUpdator surfaceCandidateUpdator);
 
  public:
   /// Retrieve a @c std::shared_ptr for this surface (non-const version)
@@ -290,7 +290,7 @@ class DetectorVolume : public std::enable_shared_from_this<DetectorVolume> {
   /// @param volumes the volumes the new navigation state updator points to
   ///
   void assignSurfaceCandidatesUpdator(
-      SurfaceCandidatesUpdator&& surfaceCandidateUpdator,
+      SurfaceCandidatesUpdator surfaceCandidateUpdator,
       const std::vector<std::shared_ptr<Surface>>& surfaces = {},
       const std::vector<std::shared_ptr<DetectorVolume>>& volumes = {});
 
@@ -409,8 +409,8 @@ class DetectorVolumeFactory {
       std::shared_ptr<VolumeBounds> bounds,
       const std::vector<std::shared_ptr<Surface>>& surfaces,
       const std::vector<std::shared_ptr<DetectorVolume>>& volumes,
-      DetectorVolumeUpdator&& detectorVolumeUpdator,
-      SurfaceCandidatesUpdator&& surfaceCandidateUpdator) {
+      DetectorVolumeUpdator detectorVolumeUpdator,
+      SurfaceCandidatesUpdator surfaceCandidateUpdator) {
     auto dVolume = DetectorVolume::makeShared(
         gctx, name, transform, std::move(bounds), surfaces, volumes,
         std::move(detectorVolumeUpdator), std::move(surfaceCandidateUpdator));
@@ -421,12 +421,12 @@ class DetectorVolumeFactory {
   /// Create a detector volume - from factory
   static std::shared_ptr<DetectorVolume> construct(
       const PortalGenerator& portalGenerator, const GeometryContext& gctx,
-      const std::string& name, const Transform3& transform,
+      std::string name, const Transform3& transform,
       std::shared_ptr<VolumeBounds> bounds,
-      SurfaceCandidatesUpdator&& surfaceCandidateUpdator) {
-    auto dVolume =
-        DetectorVolume::makeShared(gctx, name, transform, std::move(bounds),
-                                   std::move(surfaceCandidateUpdator));
+      SurfaceCandidatesUpdator surfaceCandidateUpdator) {
+    auto dVolume = DetectorVolume::makeShared(
+        gctx, std::move(name), transform, std::move(bounds),
+        std::move(surfaceCandidateUpdator));
     dVolume->construct(gctx, portalGenerator);
     return dVolume;
   }

Core/include/Acts/Detector/Portal.hpp

@@ -125,8 +125,8 @@ class Portal : public std::enable_shared_from_this<Portal> {
   ///
   /// @note this overwrites the existing link
   void assignDetectorVolumeUpdator(
-      Direction dir, DetectorVolumeUpdator&& dVolumeUpdator,
-      const std::vector<std::shared_ptr<DetectorVolume>>& attachedVolumes);
+      Direction dir, DetectorVolumeUpdator dVolumeUpdator,
+      std::vector<std::shared_ptr<DetectorVolume>> attachedVolumes);
 
   /// Update the volume link, w/o directive, i.e. it relies that there's only
   /// one remaining link to be set, throws an exception if that's not the case
@@ -135,10 +135,9 @@ class Portal : public std::enable_shared_from_this<Portal> {
   /// @param attachedVolumes is the list of attached volumes for book keeping
   ///
   /// @note this overwrites the existing link
-  void assignDetectorVolumeUpdator(
-      DetectorVolumeUpdator&& dVolumeUpdator,
-      const std::vector<std::shared_ptr<DetectorVolume>>&
-          attachedVolumes) noexcept(false);
+  void assignDetectorVolumeUpdator(DetectorVolumeUpdator dVolumeUpdator,
+                                   std::vector<std::shared_ptr<DetectorVolume>>
+                                       attachedVolumes) noexcept(false);
 
   // Access to the portal targets: opposite/along normal vector
   const DetectorVolumeUpdators& detectorVolumeUpdators() const;

Core/include/Acts/Navigation/DetectorVolumeFinders.hpp

@@ -18,7 +18,7 @@
 #include "Acts/Utilities/detail/Axis.hpp"
 #include "Acts/Utilities/detail/Grid.hpp"
 
-#include <exception>
+#include <stdexcept>
 
 namespace Acts {
 namespace Experimental {

Core/include/Acts/Navigation/DetectorVolumeUpdators.hpp

@@ -16,7 +16,7 @@
 #include "Acts/Utilities/detail/Axis.hpp"
 #include "Acts/Utilities/detail/Grid.hpp"
 
-#include <exception>
+#include <stdexcept>
 
 namespace Acts {
 namespace Experimental {

Core/include/Acts/Navigation/NavigationState.hpp

@@ -80,7 +80,7 @@ struct NavigationState {
 
   /// That are the candidate surfaces to process
   SurfaceCandidates surfaceCandidates = {};
-  SurfaceCandidates::iterator surfaceCandidate = surfaceCandidates.end();
+  SurfaceCandidates::const_iterator surfaceCandidate = surfaceCandidates.cend();
 
   /// Boundary directives for surfaces
   BoundaryCheck surfaceBoundaryCheck = true;

Core/include/Acts/Navigation/NextNavigator.hpp

@@ -195,20 +195,20 @@ class NextNavigator {
                    << posInfo(state, stepper) << "stepping through portal");
 
       nState.surfaceCandidates.clear();
-      nState.surfaceCandidate = nState.surfaceCandidates.end();
+      nState.surfaceCandidate = nState.surfaceCandidates.cend();
 
       nState.currentPortal->updateDetectorVolume(state.geoContext, nState);
 
       initializeTarget(state, stepper);
     }
 
-    for (; nState.surfaceCandidate != nState.surfaceCandidates.end();
+    for (; nState.surfaceCandidate != nState.surfaceCandidates.cend();
          ++nState.surfaceCandidate) {
       // Screen output how much is left to try
       ACTS_VERBOSE(volInfo(state)
                    << posInfo(state, stepper)
                    << std::distance(nState.surfaceCandidate,
-                                    nState.surfaceCandidates.end())
+                                    nState.surfaceCandidates.cend())
                    << " out of " << nState.surfaceCandidates.size()
                    << " surfaces remain to try.");
       // Take the surface
@@ -262,7 +262,7 @@ class NextNavigator {
       return;
     }
 
-    if (nState.surfaceCandidate == nState.surfaceCandidates.end()) {
+    if (nState.surfaceCandidate == nState.surfaceCandidates.cend()) {
       ACTS_VERBOSE(volInfo(state)
                    << posInfo(state, stepper)
                    << "no surface candidates - waiting for target call");

Core/src/Detector/Detector.cpp

@@ -17,10 +17,9 @@
 #include <utility>
 
 Acts::Experimental::Detector::Detector(
-    const std::string& name,
-    std::vector<std::shared_ptr<DetectorVolume>> rootVolumes,
-    DetectorVolumeUpdator&& detectorVolumeUpdator)
-    : m_name(name),
+    std::string name, std::vector<std::shared_ptr<DetectorVolume>> rootVolumes,
+    DetectorVolumeUpdator detectorVolumeUpdator)
+    : m_name(std::move(name)),
       m_rootVolumes(std::move(rootVolumes)),
       m_detectorVolumeUpdator(std::move(detectorVolumeUpdator)) {
   if (m_rootVolumes.internal.empty()) {
@@ -67,11 +66,11 @@ Acts::Experimental::Detector::Detector(
 
 std::shared_ptr<Acts::Experimental::Detector>
 Acts::Experimental::Detector::makeShared(
-    const std::string& name,
-    std::vector<std::shared_ptr<DetectorVolume>> rootVolumes,
-    DetectorVolumeUpdator&& detectorVolumeUpdator) {
-  return std::shared_ptr<Detector>(new Detector(
-      name, std::move(rootVolumes), std::move(detectorVolumeUpdator)));
+    std::string name, std::vector<std::shared_ptr<DetectorVolume>> rootVolumes,
+    DetectorVolumeUpdator detectorVolumeUpdator) {
+  return std::shared_ptr<Detector>(
+      new Detector(std::move(name), std::move(rootVolumes),
+                   std::move(detectorVolumeUpdator)));
 }
 
 std::vector<std::shared_ptr<Acts::Experimental::DetectorVolume>>&
@@ -95,7 +94,7 @@ Acts::Experimental::Detector::volumes() const {
 }
 
 void Acts::Experimental::Detector::updateDetectorVolumeFinder(
-    DetectorVolumeUpdator&& detectorVolumeUpdator) {
+    DetectorVolumeUpdator detectorVolumeUpdator) {
   m_detectorVolumeUpdator = std::move(detectorVolumeUpdator);
 }
 

Core/src/Detector/DetectorVolume.cpp

@@ -27,13 +27,13 @@ class IVolumeMaterial;
 }  // namespace Acts
 
 Acts::Experimental::DetectorVolume::DetectorVolume(
-    const GeometryContext& gctx, const std::string& name,
-    const Transform3& transform, std::shared_ptr<VolumeBounds> bounds,
+    const GeometryContext& gctx, std::string name, const Transform3& transform,
+    std::shared_ptr<VolumeBounds> bounds,
     std::vector<std::shared_ptr<Surface>> surfaces,
     std::vector<std::shared_ptr<DetectorVolume>> volumes,
-    DetectorVolumeUpdator&& detectorVolumeUpdator,
-    SurfaceCandidatesUpdator&& surfaceCandidateUpdator)
-    : m_name(name),
+    DetectorVolumeUpdator detectorVolumeUpdator,
+    SurfaceCandidatesUpdator surfaceCandidateUpdator)
+    : m_name(std::move(name)),
       m_transform(transform),
       m_bounds(std::move(bounds)),
       m_surfaces(std::move(surfaces)),
@@ -59,33 +59,33 @@ Acts::Experimental::DetectorVolume::DetectorVolume(
 }
 
 Acts::Experimental::DetectorVolume::DetectorVolume(
-    const GeometryContext& gctx, const std::string& name,
-    const Transform3& transform, std::shared_ptr<VolumeBounds> bounds,
-    SurfaceCandidatesUpdator&& surfaceCandidateUpdator)
-    : DetectorVolume(gctx, name, transform, std::move(bounds), {}, {},
-                     tryNoVolumes(), std::move(surfaceCandidateUpdator)) {}
+    const GeometryContext& gctx, std::string name, const Transform3& transform,
+    std::shared_ptr<VolumeBounds> bounds,
+    SurfaceCandidatesUpdator surfaceCandidateUpdator)
+    : DetectorVolume(gctx, std::move(name), transform, std::move(bounds), {},
+                     {}, tryNoVolumes(), std::move(surfaceCandidateUpdator)) {}
 
 std::shared_ptr<Acts::Experimental::DetectorVolume>
 Acts::Experimental::DetectorVolume::makeShared(
-    const GeometryContext& gctx, const std::string& name,
-    const Transform3& transform, std::shared_ptr<VolumeBounds> bounds,
+    const GeometryContext& gctx, std::string name, const Transform3& transform,
+    std::shared_ptr<VolumeBounds> bounds,
     std::vector<std::shared_ptr<Surface>> surfaces,
     std::vector<std::shared_ptr<DetectorVolume>> volumes,
-    DetectorVolumeUpdator&& detectorVolumeUpdator,
-    SurfaceCandidatesUpdator&& surfaceCandidateUpdator) {
+    DetectorVolumeUpdator detectorVolumeUpdator,
+    SurfaceCandidatesUpdator surfaceCandidateUpdator) {
   return std::shared_ptr<DetectorVolume>(new DetectorVolume(
-      gctx, name, transform, std::move(bounds), std::move(surfaces),
+      gctx, std::move(name), transform, std::move(bounds), std::move(surfaces),
       std::move(volumes), std::move(detectorVolumeUpdator),
       std::move(surfaceCandidateUpdator)));
 }
 
 std::shared_ptr<Acts::Experimental::DetectorVolume>
 Acts::Experimental::DetectorVolume::makeShared(
-    const GeometryContext& gctx, const std::string& name,
-    const Transform3& transform, std::shared_ptr<VolumeBounds> bounds,
-    SurfaceCandidatesUpdator&& surfaceCandidateUpdator) {
+    const GeometryContext& gctx, std::string name, const Transform3& transform,
+    std::shared_ptr<VolumeBounds> bounds,
+    SurfaceCandidatesUpdator surfaceCandidateUpdator) {
   return std::shared_ptr<DetectorVolume>(
-      new DetectorVolume(gctx, name, transform, std::move(bounds),
+      new DetectorVolume(gctx, std::move(name), transform, std::move(bounds),
                          std::move(surfaceCandidateUpdator)));
 }
 
@@ -239,7 +239,7 @@ void Acts::Experimental::DetectorVolume::updateNavigationState(
 }
 
 void Acts::Experimental::DetectorVolume::assignSurfaceCandidatesUpdator(
-    SurfaceCandidatesUpdator&& surfaceCandidateUpdator,
+    SurfaceCandidatesUpdator surfaceCandidateUpdator,
     const std::vector<std::shared_ptr<Surface>>& surfaces,
     const std::vector<std::shared_ptr<DetectorVolume>>& volumes) {
   m_surfaceCandidatesUpdator = std::move(surfaceCandidateUpdator);

Core/src/Detector/LayerStructureBuilder.cpp

@@ -51,16 +51,19 @@ namespace {
 template <Acts::detail::AxisBoundaryType aType>
 Acts::Experimental::SurfaceCandidatesUpdator createUpdator(
     const Acts::GeometryContext& gctx,
-    const std::vector<std::shared_ptr<Acts::Surface>>& lSurfaces,
-    const std::vector<size_t>& assignToAll,
+    std::vector<std::shared_ptr<Acts::Surface>> lSurfaces,
+    std::vector<size_t> assignToAll,
     const Acts::Experimental::ProtoBinning& binning) {
   // The surface candidate updator & a generator for polyhedrons
   Acts::Experimental::SurfaceCandidatesUpdator sfCandidates;
   Acts::Experimental::detail::PolyhedronReferenceGenerator rGenerator;
   // Indexed Surface generator for this case
   Acts::Experimental::detail::IndexedSurfacesGenerator<
       decltype(lSurfaces), Acts::Experimental::IndexedSurfacesImpl>
-      isg{lSurfaces, assignToAll, {binning.binValue}, {binning.expansion}};
+      isg{std::move(lSurfaces),
+          std::move(assignToAll),
+          {binning.binValue},
+          {binning.expansion}};
   if (binning.axisType == Acts::detail::AxisType::Equidistant) {
     // Equidistant
     Acts::Experimental::detail::GridAxisGenerators::Eq<aType> aGenerator{

Core/src/Detector/Portal.cpp

@@ -91,16 +91,16 @@ void Acts::Experimental::Portal::fuse(std::shared_ptr<Portal>& other) {
 }
 
 void Acts::Experimental::Portal::assignDetectorVolumeUpdator(
-    Direction dir, DetectorVolumeUpdator&& dVolumeUpdator,
-    const std::vector<std::shared_ptr<DetectorVolume>>& attachedVolumes) {
+    Direction dir, DetectorVolumeUpdator dVolumeUpdator,
+    std::vector<std::shared_ptr<DetectorVolume>> attachedVolumes) {
   auto idx = dir.index();
   m_volumeUpdators[idx] = std::move(dVolumeUpdator);
-  m_attachedVolumes[idx] = attachedVolumes;
+  m_attachedVolumes[idx] = std::move(attachedVolumes);
 }
 
 void Acts::Experimental::Portal::assignDetectorVolumeUpdator(
-    DetectorVolumeUpdator&& dVolumeUpdator,
-    const std::vector<std::shared_ptr<DetectorVolume>>& attachedVolumes) {
+    DetectorVolumeUpdator dVolumeUpdator,
+    std::vector<std::shared_ptr<DetectorVolume>> attachedVolumes) {
   // Check and throw exceptions
   if (not m_volumeUpdators[0u].connected() and
       not m_volumeUpdators[1u].connected()) {
@@ -111,7 +111,7 @@ void Acts::Experimental::Portal::assignDetectorVolumeUpdator(
   }
   size_t idx = m_volumeUpdators[0u].connected() ? 1u : 0u;
   m_volumeUpdators[idx] = std::move(dVolumeUpdator);
-  m_attachedVolumes[idx] = attachedVolumes;
+  m_attachedVolumes[idx] = std::move(attachedVolumes);
 }
 
 void Acts::Experimental::Portal::updateDetectorVolume(