refactor: and fix new detector geometry

Core/include/Acts/Detector/Detector.hpp

@@ -1,6 +1,6 @@
 // This file is part of the Acts project.
 //
-// Copyright (C) 2022 CERN for the benefit of the Acts project
+// Copyright (C) 2022-2023 CERN for the benefit of the Acts project
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -22,34 +22,27 @@
 #include <vector>
 
 namespace Acts {
-
 namespace Experimental {
 
 class Detector : public std::enable_shared_from_this<Detector> {
  protected:
   /// Create a detector from volumes
   ///
   /// @param name the detecor name
-  /// @param volumes the objets contained by this detector
-  /// @param volumeFinder is a Delegate to find the assocaited volume
+  /// @param rootVolume the root detector volume
+  /// @param detectorVolumeUpdator is a Delegate to find the assocaited volume
   ///
   /// @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,
-           const std::vector<std::shared_ptr<DetectorVolume>>& volumes,
-           DetectorVolumeUpdator&& volumeFinder) noexcept(false);
+  Detector(const std::string& name, std::shared_ptr<DetectorVolume> rootVolume,
+           DetectorVolumeUpdator&& detectorVolumeUpdator) noexcept(false);
 
  public:
   /// Factory for producing memory managed instances of Detector.
-  /// Will forward all parameters and will attempt to find a suitable
-  /// constructor.
-  ///
-  /// @tparam Args the arguments that will be forwarded
-  template <typename... Args>
-  static std::shared_ptr<Detector> makeShared(Args&&... args) {
-    return std::shared_ptr<Detector>(new Detector(std::forward<Args>(args)...));
-  }
+  static std::shared_ptr<Detector> makeShared(
+      const std::string& name, std::shared_ptr<DetectorVolume> rootVolume,
+      DetectorVolumeUpdator&& detectorVolumeUpdator);
 
   /// Retrieve a @c std::shared_ptr for this surface (non-const version)
   ///
@@ -73,7 +66,17 @@ class Detector : public std::enable_shared_from_this<Detector> {
   /// @return The shared pointer
   std::shared_ptr<const Detector> getSharedPtr() const;
 
-  /// Non-const access to the volumes
+  /// Non-const access to the root volume
+  ///
+  /// @return the root volume shared pointer
+  const std::shared_ptr<DetectorVolume>& rootVolumePtr();
+
+  /// Const access to the root volume
+  ///
+  /// @return a vector to const DetectorVolume raw pointers
+  const DetectorVolume* rootVolume() const;
+
+  /// Non-const access to the root volume
   ///
   /// @return the volumes shared pointer store
   std::vector<std::shared_ptr<DetectorVolume>>& volumePtrs();
@@ -111,8 +114,9 @@ class Detector : public std::enable_shared_from_this<Detector> {
 
   /// Update the volume finder
   ///
-  /// @param mVolumeFinder the new volume finder
-  void updateDetectorVolumeFinder(DetectorVolumeUpdator&& mVolumeFinder);
+  /// @param detectorVolumeUpdator the new volume finder
+  void updateDetectorVolumeFinder(
+      DetectorVolumeUpdator&& detectorVolumeUpdator);
 
   /// Const access to the volume finder
   const DetectorVolumeUpdator& detectorVolumeFinder() const;
@@ -124,37 +128,18 @@ class Detector : public std::enable_shared_from_this<Detector> {
   /// Name of the detector
   std::string m_name = "Unnamed";
 
+  /// Root volume
+  std::shared_ptr<DetectorVolume> m_rootVolume;
+
   /// Volume store (internal/external)
   DetectorVolume::ObjectStore<std::shared_ptr<DetectorVolume>> m_volumes;
 
   /// A volume finder delegate
-  DetectorVolumeUpdator m_volumeFinder;
+  DetectorVolumeUpdator m_detectorVolumeUpdator;
 
   /// Name/index map to find volumes by name and detect duplicates
   std::unordered_map<std::string, size_t> m_volumeNameIndex;
 };
 
-inline std::vector<std::shared_ptr<DetectorVolume>>& Detector::volumePtrs() {
-  return m_volumes.internal;
-}
-
-inline const std::vector<const DetectorVolume*>& Detector::volumes() const {
-  return m_volumes.external;
-}
-
-inline void Detector::updateDetectorVolumeFinder(
-    DetectorVolumeUpdator&& mVolumeFinder) {
-  m_volumeFinder = std::move(mVolumeFinder);
-}
-
-inline const DetectorVolumeUpdator& Detector::detectorVolumeFinder() const {
-  return m_volumeFinder;
-}
-
-inline const std::string& Detector::name() const {
-  return m_name;
-}
-
 }  // namespace Experimental
-
 }  // namespace Acts

Core/include/Acts/Detector/DetectorVolume.hpp

@@ -1,6 +1,6 @@
 // This file is part of the Acts project.
 //
-// Copyright (C) 2022 CERN for the benefit of the Acts project
+// Copyright (C) 2022-2023 CERN for the benefit of the Acts project
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -10,6 +10,7 @@
 
 #include "Acts/Definitions/Algebra.hpp"
 #include "Acts/Definitions/Common.hpp"
+#include "Acts/Detector/PortalGenerators.hpp"
 #include "Acts/Geometry/Extent.hpp"
 #include "Acts/Geometry/GeometryContext.hpp"
 #include "Acts/Geometry/VolumeBounds.hpp"
@@ -36,11 +37,6 @@ class DetectorVolume;
 class Portal;
 class Detector;
 
-/// The Portal genertor definition
-using PortalGenerator = Delegate<std::vector<std::shared_ptr<Portal>>(
-    const Transform3&, const VolumeBounds&,
-    const std::shared_ptr<DetectorVolume>&)>;
-
 /// A detector volume description which can be:
 ///
 /// @note A detector volume holds non-const objects internally
@@ -80,7 +76,8 @@ class DetectorVolume : public std::enable_shared_from_this<DetectorVolume> {
     /// Store constructor
     ///
     /// @param objects are the ones copied into the internal store
-    ObjectStore(const std::vector<internal_type>& objects) : internal(objects) {
+    ObjectStore(std::vector<internal_type> objects)
+        : internal(std::move(objects)) {
       external = unpack_shared_const_vector(internal);
     }
 
@@ -96,6 +93,7 @@ class DetectorVolume : public std::enable_shared_from_this<DetectorVolume> {
   /// @param bounds the volume bounds
   /// @param surfaces are the contained surfaces of this volume
   /// @param volumes are the containes volumes of this volume
+  /// @param detectorVolumeUpdator is a Delegate to find the assocaited volume
   /// @param surfaceCandidateUpdator the navigation state updator for surfaces/portals
   ///
   /// @note throws exception if misconfigured: no bounds
@@ -106,6 +104,7 @@ class DetectorVolume : public std::enable_shared_from_this<DetectorVolume> {
       const Transform3& transform, std::unique_ptr<VolumeBounds> bounds,
       const std::vector<std::shared_ptr<Surface>>& surfaces,
       const std::vector<std::shared_ptr<DetectorVolume>>& volumes,
+      DetectorVolumeUpdator&& detectorVolumeUpdator,
       SurfaceCandidatesUpdator&& surfaceCandidateUpdator) noexcept(false);
 
   /// Create a detector volume - empty/gap volume constructor
@@ -125,17 +124,23 @@ class DetectorVolume : public std::enable_shared_from_this<DetectorVolume> {
       SurfaceCandidatesUpdator&& surfaceCandidateUpdator) noexcept(false);
 
   /// Factory method for producing memory managed instances of DetectorVolume.
-  /// Will forward all parameters and will attempt to find a suitable
-  /// constructor.
   ///
   /// @note This is called by the @class DetectorVolumeFactory
+  static std::shared_ptr<DetectorVolume> makeShared(
+      const GeometryContext& gctx, const std::string& name,
+      const Transform3& transform, std::unique_ptr<VolumeBounds> bounds,
+      const std::vector<std::shared_ptr<Surface>>& surfaces,
+      const std::vector<std::shared_ptr<DetectorVolume>>& volumes,
+      DetectorVolumeUpdator&& detectorVolumeUpdator,
+      SurfaceCandidatesUpdator&& surfaceCandidateUpdator);
+
+  /// Factory method for producing memory managed instances of DetectorVolume.
   ///
-  /// @tparam Args the arguments that will be forwarded
-  template <typename... Args>
-  static std::shared_ptr<DetectorVolume> makeShared(Args&&... args) {
-    return std::shared_ptr<DetectorVolume>(
-        new DetectorVolume(std::forward<Args>(args)...));
-  }
+  /// @note This is called by the @class DetectorVolumeFactory
+  static std::shared_ptr<DetectorVolume> makeShared(
+      const GeometryContext& gctx, const std::string& name,
+      const Transform3& transform, std::unique_ptr<VolumeBounds> bounds,
+      SurfaceCandidatesUpdator&& surfaceCandidateUpdator);
 
  public:
   /// Retrieve a @c std::shared_ptr for this surface (non-const version)
@@ -186,15 +191,23 @@ class DetectorVolume : public std::enable_shared_from_this<DetectorVolume> {
   /// @return const reference to the volume bounds object
   const VolumeBounds& volumeBounds() const;
 
-  /// Inside/outside method
+  /// Check if a point is inside this volume. Subvolumes will not be checked.
   ///
   /// @param gctx the geometry context
   /// @param position the position for the inside check
-  /// @param excludeInserts steers whether inserted volumes overwrite this
   ///
   /// @return a bool to indicate inside/outside
-  bool inside(const GeometryContext& gctx, const Vector3& position,
-              bool excludeInserts = true) const;
+  bool inside(const GeometryContext& gctx, const Vector3& position) const;
+
+  /// Check if a point is exclusively inside this volume i.e. this point is not
+  /// inside a subvolume.
+  ///
+  /// @param gctx the geometry context
+  /// @param position the position for the inside check
+  ///
+  /// @return a bool to indicate inside/outside
+  bool exclusivelyInside(const GeometryContext& gctx,
+                         const Vector3& position) const;
 
   /// The Extent for this volume
   ///
@@ -260,6 +273,9 @@ class DetectorVolume : public std::enable_shared_from_this<DetectorVolume> {
   /// @return a vector to const DetectorVolume raw pointers
   const std::vector<const DetectorVolume*>& volumes() const;
 
+  /// Const access to the detector volume updator
+  const DetectorVolumeUpdator& detectorVolumeUpdator() const;
+
   /// This method allows to udate the navigation state updator
   /// module.
   ///
@@ -360,6 +376,8 @@ class DetectorVolume : public std::enable_shared_from_this<DetectorVolume> {
   /// BoundingBox
   std::shared_ptr<const BoundingBox> m_boundingBox;
 
+  DetectorVolumeUpdator m_detectorVolumeUpdator;
+
   /// The navigation state updator
   SurfaceCandidatesUpdator m_surfaceCandidatesUpdator;
 
@@ -373,96 +391,36 @@ class DetectorVolume : public std::enable_shared_from_this<DetectorVolume> {
   const Detector* m_detector = nullptr;
 };
 
-inline const Transform3& DetectorVolume::transform(
-    const GeometryContext& /*gctx*/) const {
-  return m_transform;
-}
-
-inline Vector3 DetectorVolume::center(const GeometryContext& gctx) const {
-  return transform(gctx).translation();
-}
-
-inline const VolumeBounds& DetectorVolume::volumeBounds() const {
-  return (*m_bounds.get());
-}
-
-inline std::vector<std::shared_ptr<Portal>>& DetectorVolume::portalPtrs() {
-  return m_portals.internal;
-}
-
-inline std::vector<std::shared_ptr<Surface>>& DetectorVolume::surfacePtrs() {
-  return m_surfaces.internal;
-}
-
-inline std::vector<std::shared_ptr<DetectorVolume>>&
-DetectorVolume::volumePtrs() {
-  return m_volumes.internal;
-}
-
-inline const std::vector<const Portal*>& DetectorVolume::portals() const {
-  return m_portals.external;
-}
-
-inline const std::vector<const Surface*>& DetectorVolume::surfaces() const {
-  return m_surfaces.external;
-}
-
-inline const std::vector<const DetectorVolume*>& DetectorVolume::volumes()
-    const {
-  return m_volumes.external;
-}
-
-inline const SurfaceCandidatesUpdator&
-DetectorVolume::surfaceCandidatesUpdator() const {
-  return m_surfaceCandidatesUpdator;
-}
-
-inline void DetectorVolume::assignVolumeMaterial(
-    std::shared_ptr<IVolumeMaterial> material) {
-  m_volumeMaterial = std::move(material);
-}
-
-inline std::shared_ptr<IVolumeMaterial> DetectorVolume::volumeMaterialPtr() {
-  return m_volumeMaterial;
-}
-
-inline const IVolumeMaterial* DetectorVolume::volumeMaterial() const {
-  return m_volumeMaterial.get();
-}
-
-inline const GeometryIdentifier& DetectorVolume::geometryId() const {
-  return m_geometryId;
-}
-
-inline const std::string& DetectorVolume::name() const {
-  return m_name;
-}
-
-inline void DetectorVolume::assignDetector(const Detector& detector) {
-  m_detector = &detector;
-}
-
-inline const Detector* DetectorVolume::detector() const {
-  return m_detector;
-}
-
 /// @brief  A detector volume factory which first constructs the detector volume
 /// and then constructs the portals. This ensures that the std::shared_ptr
 /// holding the detector volume is not weak when assigning to the portals.
 class DetectorVolumeFactory {
  public:
   /// Create a detector volume - from factory
-  /// @param portalGenerator the volume portal generator
-  /// @param gctx the geometry context for construction and potential contextual store
-  /// @param args the arguments forwarded to the detector volume contructor
-  ///
-  /// @return a shared object DetectorVolume
-  template <typename... Args>
   static std::shared_ptr<DetectorVolume> construct(
       const PortalGenerator& portalGenerator, const GeometryContext& gctx,
-      Args&&... args) {
+      const std::string& name, const Transform3& transform,
+      std::unique_ptr<VolumeBounds> bounds,
+      const std::vector<std::shared_ptr<Surface>>& surfaces,
+      const std::vector<std::shared_ptr<DetectorVolume>>& volumes,
+      DetectorVolumeUpdator&& detectorVolumeUpdator,
+      SurfaceCandidatesUpdator&& surfaceCandidateUpdator) {
+    auto dVolume = DetectorVolume::makeShared(
+        gctx, name, transform, std::move(bounds), surfaces, volumes,
+        std::move(detectorVolumeUpdator), std::move(surfaceCandidateUpdator));
+    dVolume->construct(gctx, portalGenerator);
+    return dVolume;
+  }
+
+  /// 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::unique_ptr<VolumeBounds> bounds,
+      SurfaceCandidatesUpdator&& surfaceCandidateUpdator) {
     auto dVolume =
-        DetectorVolume::makeShared(gctx, std::forward<Args>(args)...);
+        DetectorVolume::makeShared(gctx, name, transform, std::move(bounds),
+                                   std::move(surfaceCandidateUpdator));
     dVolume->construct(gctx, portalGenerator);
     return dVolume;
   }