refactor: OrientedSurface improvements (#3026)

This PR:

1. Makes `OrientedSurface` a struct with member names
2. Remove the `OrientedSurfaces` typedef over a vector
3. Add aliases `AlongNormal` and `OppositeNormal` to the `Direction` type

I don't think this is breaking.

Core/include/Acts/Definitions/Direction.hpp

@@ -33,6 +33,9 @@ class Direction final {
   static constexpr auto Backward = Value::Negative;
   static constexpr auto Forward = Value::Positive;
 
+  static constexpr auto OppositeNormal = Value::Negative;
+  static constexpr auto AlongNormal = Value::Positive;
+
   /// This turns a signed value into a direction. Will assert on zero.
   ///
   /// @param scalar is the signed value

Core/include/Acts/Geometry/ConeVolumeBounds.hpp

@@ -113,7 +113,7 @@ class ConeVolumeBounds : public VolumeBounds {
   /// It will throw an exception if the orientation prescription is not adequate
   ///
   /// @return a vector of surfaces bounding this volume
-  OrientedSurfaces orientedSurfaces(
+  std::vector<OrientedSurface> orientedSurfaces(
       const Transform3& transform = Transform3::Identity()) const final;
 
   /// Construct bounding box for this shape

Core/include/Acts/Geometry/CuboidVolumeBounds.hpp

@@ -112,7 +112,7 @@ class CuboidVolumeBounds : public VolumeBounds {
   /// It will throw an exception if the orientation prescription is not adequate
   ///
   /// @return a vector of surfaces bounding this volume
-  OrientedSurfaces orientedSurfaces(
+  std::vector<OrientedSurface> orientedSurfaces(
       const Transform3& transform = Transform3::Identity()) const override;
 
   /// Construct bounding box for this shape

Core/include/Acts/Geometry/CutoutCylinderVolumeBounds.hpp

@@ -101,7 +101,7 @@ class CutoutCylinderVolumeBounds : public VolumeBounds {
   /// It will throw an exception if the orientation prescription is not adequate
   ///
   /// @return a vector of surfaces bounding this volume
-  OrientedSurfaces orientedSurfaces(
+  std::vector<OrientedSurface> orientedSurfaces(
       const Transform3& transform = Transform3::Identity()) const override;
 
   /// Construct bounding box for this shape

Core/include/Acts/Geometry/CylinderVolumeBounds.hpp

@@ -161,7 +161,7 @@ class CylinderVolumeBounds : public VolumeBounds {
   /// It will throw an exception if the orientation prescription is not adequate
   ///
   /// @return a vector of surfaces bounding this volume
-  OrientedSurfaces orientedSurfaces(
+  std::vector<OrientedSurface> orientedSurfaces(
       const Transform3& transform = Transform3::Identity()) const override;
 
   /// Construct bounding box for this shape

Core/include/Acts/Geometry/GenericCuboidVolumeBounds.hpp

@@ -77,7 +77,7 @@ class GenericCuboidVolumeBounds : public VolumeBounds {
   /// It will throw an exception if the orientation prescription is not adequate
   ///
   /// @return a vector of surfaces bounding this volume
-  OrientedSurfaces orientedSurfaces(
+  std::vector<OrientedSurface> orientedSurfaces(
       const Transform3& transform = Transform3::Identity()) const override;
 
   /// Construct bounding box for this shape

Core/include/Acts/Geometry/TrapezoidVolumeBounds.hpp

@@ -124,7 +124,7 @@ class TrapezoidVolumeBounds : public VolumeBounds {
   /// It will throw an exception if the orientation prescription is not adequate
   ///
   /// @return a vector of surfaces bounding this volume
-  OrientedSurfaces orientedSurfaces(
+  std::vector<OrientedSurface> orientedSurfaces(
       const Transform3& transform = Transform3::Identity()) const override;
 
   /// Construct bounding box for this shape

Core/include/Acts/Geometry/VolumeBounds.hpp

@@ -26,8 +26,10 @@ class Surface;
 class VolumeBounds;
 class Direction;
 
-using OrientedSurface = std::pair<std::shared_ptr<RegularSurface>, Direction>;
-using OrientedSurfaces = std::vector<OrientedSurface>;
+struct OrientedSurface {
+  std::shared_ptr<RegularSurface> surface;
+  Direction direction;
+};
 
 // Planar definitions to help construct the boundary surfaces
 static const Transform3 s_planeXY = Transform3::Identity();
@@ -101,7 +103,7 @@ class VolumeBounds {
   /// It will throw an exception if the orientation prescription is not adequate
   ///
   /// @return a vector of surfaces bounding this volume
-  virtual OrientedSurfaces orientedSurfaces(
+  virtual std::vector<OrientedSurface> orientedSurfaces(
       const Transform3& transform = Transform3::Identity()) const = 0;
 
   /// Construct bounding box for this shape

Core/src/Detector/PortalGenerators.cpp

@@ -33,16 +33,16 @@ Acts::Experimental::generatePortals(
   std::vector<std::shared_ptr<Portal>> portals;
   for (auto [i, oSurface] : enumerate(orientedSurfaces)) {
     // Create a portal from the surface
-    auto portal = std::make_shared<Portal>(oSurface.first);
+    auto portal = std::make_shared<Portal>(oSurface.surface);
     // Create a shared link instance & delegate
     auto singleLinkImpl =
         std::make_unique<const SingleDetectorVolumeImpl>(dVolume.get());
     DetectorVolumeUpdater singleLink;
     singleLink.connect<&SingleDetectorVolumeImpl::update>(
         std::move(singleLinkImpl));
     // Update the volume link and the store
-    portal->assignDetectorVolumeUpdater(oSurface.second, std::move(singleLink),
-                                        {dVolume});
+    portal->assignDetectorVolumeUpdater(oSurface.direction,
+                                        std::move(singleLink), {dVolume});
     // Portal is prepared
     portals.push_back(std::move(portal));
   }

Core/src/Geometry/AbstractVolume.cpp

@@ -35,12 +35,12 @@ void Acts::AbstractVolume::createBoundarySurfaces() {
   for (auto& osf : orientedSurfaces) {
     AbstractVolume* opposite = nullptr;
     AbstractVolume* along = nullptr;
-    if (osf.second == Direction::Negative) {
+    if (osf.direction == Direction::Negative) {
       opposite = this;
     } else {
       along = this;
     }
     m_boundarySurfaces.push_back(std::make_shared<const Boundary>(
-        std::move(osf.first), opposite, along));
+        std::move(osf.surface), opposite, along));
   }
 }

Core/src/Geometry/ConeVolumeBounds.cpp

@@ -91,9 +91,9 @@ ConeVolumeBounds::ConeVolumeBounds(ActsScalar cylinderR, ActsScalar alpha,
   checkConsistency();
 }
 
-Acts::OrientedSurfaces Acts::ConeVolumeBounds::orientedSurfaces(
+std::vector<Acts::OrientedSurface> Acts::ConeVolumeBounds::orientedSurfaces(
     const Transform3& transform) const {
-  OrientedSurfaces oSurfaces;
+  std::vector<OrientedSurface> oSurfaces;
   oSurfaces.reserve(6);
 
   // Create an inner Cone
@@ -102,13 +102,13 @@ Acts::OrientedSurfaces Acts::ConeVolumeBounds::orientedSurfaces(
     auto innerCone =
         Surface::makeShared<ConeSurface>(innerConeTrans, m_innerConeBounds);
     oSurfaces.push_back(
-        OrientedSurface(std::move(innerCone), Direction::Forward));
+        OrientedSurface{std::move(innerCone), Direction::AlongNormal});
   } else if (m_innerCylinderBounds != nullptr) {
     // Or alternatively the inner Cylinder
     auto innerCylinder =
         Surface::makeShared<CylinderSurface>(transform, m_innerCylinderBounds);
     oSurfaces.push_back(
-        OrientedSurface(std::move(innerCylinder), Direction::Forward));
+        OrientedSurface{std::move(innerCylinder), Direction::AlongNormal});
   }
 
   // Create an outer Cone
@@ -117,13 +117,13 @@ Acts::OrientedSurfaces Acts::ConeVolumeBounds::orientedSurfaces(
     auto outerCone =
         Surface::makeShared<ConeSurface>(outerConeTrans, m_outerConeBounds);
     oSurfaces.push_back(
-        OrientedSurface(std::move(outerCone), Direction::Backward));
+        OrientedSurface{std::move(outerCone), Direction::OppositeNormal});
   } else if (m_outerCylinderBounds != nullptr) {
     // or alternatively an outer Cylinder
     auto outerCylinder =
         Surface::makeShared<CylinderSurface>(transform, m_outerCylinderBounds);
     oSurfaces.push_back(
-        OrientedSurface(std::move(outerCylinder), Direction::Backward));
+        OrientedSurface{std::move(outerCylinder), Direction::OppositeNormal});
   }
 
   // Set a disc at Zmin
@@ -133,15 +133,15 @@ Acts::OrientedSurfaces Acts::ConeVolumeBounds::orientedSurfaces(
     auto negativeDisc = Surface::makeShared<DiscSurface>(negativeDiscTrans,
                                                          m_negativeDiscBounds);
     oSurfaces.push_back(
-        OrientedSurface(std::move(negativeDisc), Direction::Forward));
+        OrientedSurface{std::move(negativeDisc), Direction::AlongNormal});
   }
 
   // Set a disc at Zmax
   auto positiveDiscTrans = transform * Translation3(0., 0., get(eHalfLengthZ));
   auto positiveDisc =
       Surface::makeShared<DiscSurface>(positiveDiscTrans, m_positiveDiscBounds);
   oSurfaces.push_back(
-      OrientedSurface(std::move(positiveDisc), Direction::Backward));
+      OrientedSurface{std::move(positiveDisc), Direction::OppositeNormal});
 
   if (m_sectorBounds) {
     RotationMatrix3 sectorRotation;
@@ -156,7 +156,7 @@ Acts::OrientedSurfaces Acts::ConeVolumeBounds::orientedSurfaces(
     auto negSectorPlane =
         Surface::makeShared<PlaneSurface>(negSectorAbsTrans, m_sectorBounds);
     oSurfaces.push_back(
-        OrientedSurface(std::move(negSectorPlane), Direction::Positive));
+        OrientedSurface{std::move(negSectorPlane), Direction::AlongNormal});
 
     Transform3 posSectorRelTrans{sectorRotation};
     posSectorRelTrans.prerotate(
@@ -166,7 +166,7 @@ Acts::OrientedSurfaces Acts::ConeVolumeBounds::orientedSurfaces(
         Surface::makeShared<PlaneSurface>(posSectorAbsTrans, m_sectorBounds);
 
     oSurfaces.push_back(
-        OrientedSurface(std::move(posSectorPlane), Direction::Negative));
+        OrientedSurface{std::move(posSectorPlane), Direction::OppositeNormal});
   }
   return oSurfaces;
 }

Core/src/Geometry/CuboidVolumeBounds.cpp

@@ -44,41 +44,44 @@ CuboidVolumeBounds& CuboidVolumeBounds::operator=(
   return *this;
 }
 
-Acts::OrientedSurfaces Acts::CuboidVolumeBounds::orientedSurfaces(
+std::vector<Acts::OrientedSurface> Acts::CuboidVolumeBounds::orientedSurfaces(
     const Transform3& transform) const {
-  OrientedSurfaces oSurfaces;
+  std::vector<OrientedSurface> oSurfaces;
   oSurfaces.reserve(6);
   // Face surfaces xy -------------------------------------
   //   (1) - at negative local z
   auto sf = Surface::makeShared<PlaneSurface>(
       transform * Translation3(0., 0., -get(eHalfLengthZ)), m_xyBounds);
-  oSurfaces.push_back(OrientedSurface(std::move(sf), Direction::Positive));
+  oSurfaces.push_back(OrientedSurface{std::move(sf), Direction::AlongNormal});
   //   (2) - at positive local z
   sf = Surface::makeShared<PlaneSurface>(
       transform * Translation3(0., 0., get(eHalfLengthZ)), m_xyBounds);
-  oSurfaces.push_back(OrientedSurface(std::move(sf), Direction::Negative));
+  oSurfaces.push_back(
+      OrientedSurface{std::move(sf), Direction::OppositeNormal});
   // Face surfaces yz -------------------------------------
   //   (3) - at negative local x
   sf = Surface::makeShared<PlaneSurface>(
       transform * Translation3(-get(eHalfLengthX), 0., 0.) * s_planeYZ,
       m_yzBounds);
-  oSurfaces.push_back(OrientedSurface(std::move(sf), Direction::Positive));
+  oSurfaces.push_back(OrientedSurface{std::move(sf), Direction::AlongNormal});
   //   (4) - at positive local x
   sf = Surface::makeShared<PlaneSurface>(
       transform * Translation3(get(eHalfLengthX), 0., 0.) * s_planeYZ,
       m_yzBounds);
-  oSurfaces.push_back(OrientedSurface(std::move(sf), Direction::Negative));
+  oSurfaces.push_back(
+      OrientedSurface{std::move(sf), Direction::OppositeNormal});
   // Face surfaces zx -------------------------------------
   //   (5) - at negative local y
   sf = Surface::makeShared<PlaneSurface>(
       transform * Translation3(0., -get(eHalfLengthY), 0.) * s_planeZX,
       m_zxBounds);
-  oSurfaces.push_back(OrientedSurface(std::move(sf), Direction::Positive));
+  oSurfaces.push_back(OrientedSurface{std::move(sf), Direction::AlongNormal});
   //   (6) - at positive local y
   sf = Surface::makeShared<PlaneSurface>(
       transform * Translation3(0., get(eHalfLengthY), 0.) * s_planeZX,
       m_zxBounds);
-  oSurfaces.push_back(OrientedSurface(std::move(sf), Direction::Negative));
+  oSurfaces.push_back(
+      OrientedSurface{std::move(sf), Direction::OppositeNormal});
 
   return oSurfaces;
 }

Core/src/Geometry/CutoutCylinderVolumeBounds.cpp

@@ -48,9 +48,10 @@ bool Acts::CutoutCylinderVolumeBounds::inside(const Acts::Vector3& gpos,
   return !insideRInner || !insideZInner;  // we are not, inside bounds
 }
 
-Acts::OrientedSurfaces Acts::CutoutCylinderVolumeBounds::orientedSurfaces(
+std::vector<Acts::OrientedSurface>
+Acts::CutoutCylinderVolumeBounds::orientedSurfaces(
     const Transform3& transform) const {
-  OrientedSurfaces oSurfaces;
+  std::vector<OrientedSurface> oSurfaces;
 
   if (get(eMinR) == 0.) {
     oSurfaces.resize(6);  // exactly six surfaces (no choke inner cover)
@@ -62,13 +63,13 @@ Acts::OrientedSurfaces Acts::CutoutCylinderVolumeBounds::orientedSurfaces(
   auto outer =
       Surface::makeShared<CylinderSurface>(transform, m_outerCylinderBounds);
   oSurfaces.at(tubeOuterCover) =
-      OrientedSurface(std::move(outer), Direction::Negative);
+      OrientedSurface{std::move(outer), Direction::OppositeNormal};
 
   // Inner (cutout) cylinder envelope
   auto cutoutInner =
       Surface::makeShared<CylinderSurface>(transform, m_cutoutCylinderBounds);
   oSurfaces.at(tubeInnerCover) =
-      OrientedSurface(std::move(cutoutInner), Direction::Positive);
+      OrientedSurface{std::move(cutoutInner), Direction::AlongNormal};
 
   // z position of the pos and neg choke points
   double hlChoke = (get(eHalfLengthZ) - get(eHalfLengthZcutout)) * 0.5;
@@ -79,13 +80,13 @@ Acts::OrientedSurfaces Acts::CutoutCylinderVolumeBounds::orientedSurfaces(
     auto posInner = Surface::makeShared<CylinderSurface>(posChokeTrf,
                                                          m_innerCylinderBounds);
     oSurfaces.at(index7) =
-        OrientedSurface(std::move(posInner), Direction::Positive);
+        OrientedSurface{std::move(posInner), Direction::AlongNormal};
 
     auto negChokeTrf = transform * Translation3(Vector3(0, 0, -zChoke));
     auto negInner = Surface::makeShared<CylinderSurface>(negChokeTrf,
                                                          m_innerCylinderBounds);
     oSurfaces.at(index6) =
-        OrientedSurface(std::move(negInner), Direction::Positive);
+        OrientedSurface{std::move(negInner), Direction::AlongNormal};
   }
 
   // Two Outer disks
@@ -94,29 +95,29 @@ Acts::OrientedSurfaces Acts::CutoutCylinderVolumeBounds::orientedSurfaces(
   auto posOutDisc =
       Surface::makeShared<DiscSurface>(posOutDiscTrf, m_outerDiscBounds);
   oSurfaces.at(positiveFaceXY) =
-      OrientedSurface(std::move(posOutDisc), Direction::Negative);
+      OrientedSurface{std::move(posOutDisc), Direction::OppositeNormal};
 
   auto negOutDiscTrf =
       transform * Translation3(Vector3(0, 0, -get(eHalfLengthZ)));
   auto negOutDisc =
       Surface::makeShared<DiscSurface>(negOutDiscTrf, m_outerDiscBounds);
   oSurfaces.at(negativeFaceXY) =
-      OrientedSurface(std::move(negOutDisc), Direction::Positive);
+      OrientedSurface{std::move(negOutDisc), Direction::AlongNormal};
 
   // Two Inner disks
   auto posInDiscTrf =
       transform * Translation3(Vector3(0, 0, get(eHalfLengthZcutout)));
   auto posInDisc =
       Surface::makeShared<DiscSurface>(posInDiscTrf, m_innerDiscBounds);
   oSurfaces.at(index5) =
-      OrientedSurface(std::move(posInDisc), Direction::Positive);
+      OrientedSurface{std::move(posInDisc), Direction::AlongNormal};
 
   auto negInDiscTrf =
       transform * Translation3(Vector3(0, 0, -get(eHalfLengthZcutout)));
   auto negInDisc =
       Surface::makeShared<DiscSurface>(negInDiscTrf, m_innerDiscBounds);
   oSurfaces.at(index4) =
-      OrientedSurface(std::move(negInDisc), Direction::Negative);
+      OrientedSurface{std::move(negInDisc), Direction::OppositeNormal};
 
   return oSurfaces;
 }

Core/src/Geometry/CylinderVolumeBounds.cpp

@@ -62,7 +62,7 @@ CylinderVolumeBounds::CylinderVolumeBounds(const RadialBounds& rBounds,
 
 std::vector<OrientedSurface> CylinderVolumeBounds::orientedSurfaces(
     const Transform3& transform) const {
-  OrientedSurfaces oSurfaces;
+  std::vector<OrientedSurface> oSurfaces;
   oSurfaces.reserve(6);
 
   Translation3 vMinZ(0., 0., -get(eHalfLengthZ));
@@ -90,24 +90,24 @@ std::vector<OrientedSurface> CylinderVolumeBounds::orientedSurfaces(
   // [0] Bottom Disc (negative z)
   auto dSurface = Surface::makeShared<DiscSurface>(transMinZ, m_discBounds);
   oSurfaces.push_back(
-      OrientedSurface(std::move(dSurface), Direction::Positive));
+      OrientedSurface{std::move(dSurface), Direction::AlongNormal});
   // [1] Top Disc (positive z)
   dSurface = Surface::makeShared<DiscSurface>(transMaxZ, m_discBounds);
   oSurfaces.push_back(
-      OrientedSurface(std::move(dSurface), Direction::Negative));
+      OrientedSurface{std::move(dSurface), Direction::OppositeNormal});
 
   // [2] Outer Cylinder
   auto cSurface =
       Surface::makeShared<CylinderSurface>(transform, m_outerCylinderBounds);
   oSurfaces.push_back(
-      OrientedSurface(std::move(cSurface), Direction::Negative));
+      OrientedSurface{std::move(cSurface), Direction::OppositeNormal});
 
   // [3] Inner Cylinder (optional)
   if (m_innerCylinderBounds != nullptr) {
     cSurface =
         Surface::makeShared<CylinderSurface>(transform, m_innerCylinderBounds);
     oSurfaces.push_back(
-        OrientedSurface(std::move(cSurface), Direction::Positive));
+        OrientedSurface{std::move(cSurface), Direction::AlongNormal});
   }
 
   // [4] & [5] - Sectoral planes (optional)
@@ -122,7 +122,7 @@ std::vector<OrientedSurface> CylinderVolumeBounds::orientedSurfaces(
     auto pSurface =
         Surface::makeShared<PlaneSurface>(sp1Transform, m_sectorPlaneBounds);
     oSurfaces.push_back(
-        OrientedSurface(std::move(pSurface), Direction::Positive));
+        OrientedSurface{std::move(pSurface), Direction::AlongNormal});
     // sectorPlane 2 (positive phi)
     const Transform3 sp2Transform =
         Transform3(transform *
@@ -133,7 +133,7 @@ std::vector<OrientedSurface> CylinderVolumeBounds::orientedSurfaces(
     pSurface =
         Surface::makeShared<PlaneSurface>(sp2Transform, m_sectorPlaneBounds);
     oSurfaces.push_back(
-        OrientedSurface(std::move(pSurface), Direction::Negative));
+        OrientedSurface{std::move(pSurface), Direction::OppositeNormal});
   }
   return oSurfaces;
 }