refactor: Surface takes shared ownership of SurfacePlacementBase

Core/include/Acts/Geometry/VolumePlacementBase.hpp

@@ -133,17 +133,18 @@ class VolumePlacementBase {
   virtual const Transform3& portalLocalToGlobal(
       const GeometryContext& gctx, const std::size_t portalIdx) const = 0;
 
-  /// Pointer to the `SurfacePlacementBase` object aligning the i-th portal
-  /// (May be nullptr if index exceeds the number of portals)
+  /// Shared pointer to the `SurfacePlacementBase` object aligning the i-th
+  /// portal (May be nullptr if index exceeds the number of portals)
   /// @param portalIdx: Internal index of the portal surface [0 - number of portals)
-  /// @returns Pointer to the i-th portal placement
-  const detail::PortalPlacement* portalPlacement(
+  /// @returns Shared pointer to the i-th portal placement
+  std::shared_ptr<const detail::PortalPlacement> portalPlacement(
       const std::size_t portalIdx) const;
-  /// Pointer to the `SurfacePlacementBase` object aligning the i-th portal
-  /// (May be nullptr if index exceeds the number of portals)
+  /// Shared pointer to the `SurfacePlacementBase` object aligning the i-th
+  /// portal (May be nullptr if index exceeds the number of portals)
   /// @param portalIdx: Internal index of the portal surface [0 - number of portals)
-  /// @returns Pointer to the i-th portal placement
-  detail::PortalPlacement* portalPlacement(const std::size_t portalIdx);
+  /// @returns Shared pointer to the i-th portal placement
+  std::shared_ptr<detail::PortalPlacement> portalPlacement(
+      const std::size_t portalIdx);
 
  protected:
   /// Constructs the transform from the portal's frame into the
@@ -160,6 +161,6 @@ class VolumePlacementBase {
 
  private:
   /// Resource allocation of the SurfacePlacements to align the portals
-  std::vector<std::unique_ptr<detail::PortalPlacement>> m_portalPlacements{};
+  std::vector<std::shared_ptr<detail::PortalPlacement>> m_portalPlacements{};
 };
 }  // namespace Acts

Core/include/Acts/Geometry/detail/PortalPlacement.hpp

@@ -24,6 +24,15 @@ class PortalPlacement final : public SurfacePlacementBase {
   ///  placement
   friend class Acts::VolumePlacementBase;
 
+  /// Factory: constructs a PortalPlacement and assigns it to the surface.
+  ///
+  /// Must be used instead of direct construction so that @c shared_from_this()
+  /// is valid when @c assignSurfacePlacement is called.
+  static std::shared_ptr<PortalPlacement> create(
+      std::size_t portalIdx, const Transform3& portalTrf,
+      const VolumePlacementBase* parent,
+      std::shared_ptr<RegularSurface> surface);
+
   /// Returns the transform to switch from the portal reference
   /// frame into the experiment's global frame taking the alignment
   /// correction into account

Core/include/Acts/Surfaces/CylinderSurface.hpp

@@ -74,6 +74,9 @@ class CylinderSurface : public RegularSurface {
   ///        ensures the life-time of the `SurfacePlacementBase`
   ///        and that the `Surface` is actually owned by
   ///        the `SurfacePlacementBase` instance
+  CylinderSurface(std::shared_ptr<const CylinderBounds> cbounds,
+                  std::shared_ptr<const SurfacePlacementBase> placement);
+
   CylinderSurface(std::shared_ptr<const CylinderBounds> cbounds,
                   const SurfacePlacementBase& placement);
 

Core/include/Acts/Surfaces/DiscSurface.hpp

@@ -94,6 +94,9 @@ class DiscSurface : public RegularSurface {
   ///        ensures the life-time of the `SurfacePlacementBase`
   ///        and that the `Surface` is actually owned by
   ///        the `SurfacePlacementBase` instance
+  explicit DiscSurface(std::shared_ptr<const DiscBounds> dbounds,
+                       std::shared_ptr<const SurfacePlacementBase> placement);
+
   explicit DiscSurface(std::shared_ptr<const DiscBounds> dbounds,
                        const SurfacePlacementBase& placement);
 

Core/include/Acts/Surfaces/LineSurface.hpp

@@ -65,6 +65,9 @@ class LineSurface : public Surface {
   ///        ensures the life-time of the `SurfacePlacementBase`
   ///        and that the `Surface` is actually owned by
   ///        the `SurfacePlacementBase` instance
+  explicit LineSurface(std::shared_ptr<const LineBounds> lbounds,
+                       std::shared_ptr<const SurfacePlacementBase> placement);
+
   explicit LineSurface(std::shared_ptr<const LineBounds> lbounds,
                        const SurfacePlacementBase& placement);
 

Core/include/Acts/Surfaces/PlaneSurface.hpp

@@ -62,6 +62,9 @@ class PlaneSurface : public RegularSurface {
   ///        ensures the life-time of the `SurfacePlacementBase`
   ///        and that the `Surface` is actually owned by
   ///        the `SurfacePlacementBase` instance
+  PlaneSurface(std::shared_ptr<const PlanarBounds> pbounds,
+               std::shared_ptr<const SurfacePlacementBase> placement);
+
   PlaneSurface(std::shared_ptr<const PlanarBounds> pbounds,
                const SurfacePlacementBase& placement);
 

Core/include/Acts/Surfaces/Surface.hpp

@@ -29,6 +29,7 @@
 #include <string>
 #include <string_view>
 #include <utility>
+#include <variant>
 
 namespace Acts {
 
@@ -91,14 +92,20 @@ class Surface : public virtual GeometryObject,
   /// @param other Source surface for copy.
   Surface(const Surface& other) noexcept = default;
 
-  /// Constructor from SurfacePlacement: Element proxy
+  /// Constructor from SurfacePlacement: Element proxy with shared ownership
+  ///
+  /// @param placement Shared pointer to the surface placement
+  /// @note The Surface takes shared ownership of the placement. The placement
+  ///       must be managed by a shared_ptr before calling this constructor
+  ///       (i.e. @c shared_from_this() must be valid on it).
+  explicit Surface(
+      std::shared_ptr<const SurfacePlacementBase> placement) noexcept;
+
+  /// Constructor from SurfacePlacement: Element proxy (raw reference)
   ///
   /// @param placement Reference to the surface placement
-  /// @note The Surface does not take any ownership over the
-  ///       `SurfacePlacementBase` it is expected that the user
-  ///        ensures the life-time of the `SurfacePlacementBase`
-  ///        and that the `Surface` is actually owned by
-  ///        the `SurfacePlacementBase` instance
+  /// @note Prefer the @c shared_ptr overload; this raw-reference path is kept
+  ///       for backward compatibility and will be removed in a future release.
   explicit Surface(const SurfacePlacementBase& placement) noexcept;
 
   /// Copy constructor with optional shift
@@ -233,7 +240,18 @@ class Surface : public virtual GeometryObject,
       const;
 
   /// Assign a placement object which may dynamically align the surface in space
+  /// (shared ownership — preferred)
+  /// @param placement Shared pointer to the placement object
+  void assignSurfacePlacement(
+      std::shared_ptr<const SurfacePlacementBase> placement);
+
+  /// Assign a placement object which may dynamically align the surface in space
+  /// (raw reference — deprecated)
   /// @param placement: Placement object defining the surface's position
+  /// @deprecated Pass a @c shared_ptr<const SurfacePlacementBase> instead.
+  [[deprecated(
+      "Pass a shared_ptr<const SurfacePlacementBase> instead; this overload "
+      "will be removed in a future release")]]
   void assignSurfacePlacement(const SurfacePlacementBase& placement);
 
   /// Assign the surface material description
@@ -538,9 +556,18 @@ class Surface : public virtual GeometryObject,
   /// (translation, rotation) the surface in global space
   CloneablePtr<const Transform3> m_transform{};
 
+ protected:
+  /// Helper to extract the raw SurfacePlacementBase pointer regardless of
+  /// which variant path (deprecated raw pointer or new shared_ptr) is active.
+  /// Returns nullptr if no placement is set.
+  const SurfacePlacementBase* placementPtr() const noexcept;
+
  private:
-  /// Pointer to the a SurfacePlacement
-  const SurfacePlacementBase* m_placement{nullptr};
+  /// Placement storage: either empty (monostate), a raw pointer (deprecated
+  /// backward-compat path), or a shared_ptr (new owning path).
+  std::variant<std::monostate, const SurfacePlacementBase*,
+               std::shared_ptr<const SurfacePlacementBase>>
+      m_placement;
 
   /// The associated layer Layer - layer in which the Surface is be embedded,
   /// nullptr if not associated

Core/include/Acts/Surfaces/SurfacePlacementBase.hpp

@@ -11,6 +11,8 @@
 #include "Acts/Definitions/Algebra.hpp"
 #include "Acts/Geometry/GeometryContext.hpp"
 
+#include <memory>
+
 namespace Acts {
 class Surface;
 /// @brief The `SurfacePlacementBase` is an API proxy to model the dynamic
@@ -29,7 +31,8 @@ class Surface;
 ///        `SurfacePlacementBase::localToGlobalTransform`. There the user needs
 ///        to unpack the GeometryContext, look-up the appropriate cached
 ///        transform and return it back to the Acts library
-class SurfacePlacementBase {
+class SurfacePlacementBase
+    : public std::enable_shared_from_this<SurfacePlacementBase> {
  public:
   /// @brief Virtual default constructor
   virtual ~SurfacePlacementBase() = default;
@@ -48,10 +51,20 @@ class SurfacePlacementBase {
   ///       Acts::Surface::surfacePlacement method return a pointer to
   ///       this object.
   /// @return Reference to a surface that represents this detector element
+  /// @deprecated Use createSurface() to produce a Surface that owns this
+  ///             placement. This method will be removed in a future release.
+  [[deprecated(
+      "surface() is deprecated; use createSurface() to produce a Surface that "
+      "takes shared ownership of the placement")]]
   virtual const Surface& surface() const = 0;
 
   /// @copydoc surface
   /// @return Reference to a surface that represents this detector element
+  /// @deprecated Use createSurface() to produce a Surface that owns this
+  ///             placement. This method will be removed in a future release.
+  [[deprecated(
+      "surface() is deprecated; use createSurface() to produce a Surface that "
+      "takes shared ownership of the placement")]]
   virtual Surface& surface() = 0;
 
   /// @brief Returns whether the placement corresponds to a surface on which

Core/src/Geometry/PortalPlacement.cpp

@@ -21,7 +21,16 @@
       m_parent{parent},
       m_portalIdx{portalIdx} {
   assert(m_surface != nullptr);
-  m_surface->assignSurfacePlacement(*this);
+}
+
+std::shared_ptr<PortalPlacement> PortalPlacement::create(
+    const std::size_t portalIdx, const Transform3& portalTrf,
+    const VolumePlacementBase* parent,
+    std::shared_ptr<RegularSurface> surface) {
+  auto placement =
+      std::make_shared<PortalPlacement>(portalIdx, portalTrf, parent, surface);
+  surface->assignSurfacePlacement(placement);
+  return placement;
 }
 
 Transform3 PortalPlacement::assembleFullTransform(

Core/src/Geometry/VolumePlacementBase.cpp

@@ -46,19 +46,19 @@ void VolumePlacementBase::makePortalsAlignable(
         globalToLocalTransform(gctx) *
         portalSurface->localToGlobalTransform(gctx);
 
-    m_portalPlacements.emplace_back(std::make_unique<detail::PortalPlacement>(
+    m_portalPlacements.push_back(detail::PortalPlacement::create(
         portalIdx, portalToVolTrf, this, portalSurface));
   }
 }
 
-const detail::PortalPlacement* VolumePlacementBase::portalPlacement(
-    const std::size_t portalIdx) const {
-  return m_portalPlacements.at(portalIdx).get();
+std::shared_ptr<const detail::PortalPlacement>
+VolumePlacementBase::portalPlacement(const std::size_t portalIdx) const {
+  return m_portalPlacements.at(portalIdx);
 }
 
-detail::PortalPlacement* VolumePlacementBase::portalPlacement(
+std::shared_ptr<detail::PortalPlacement> VolumePlacementBase::portalPlacement(
     const std::size_t portalIdx) {
-  return m_portalPlacements.at(portalIdx).get();
+  return m_portalPlacements.at(portalIdx);
 }
 
 std::size_t VolumePlacementBase::nPortalPlacements() const {

Core/src/Surfaces/CylinderSurface.cpp

@@ -51,6 +51,14 @@ CylinderSurface::CylinderSurface(const Transform3& transform, double radius,
       m_bounds(std::make_shared<const CylinderBounds>(
           radius, halfz, halfphi, avphi, bevelMinZ, bevelMaxZ)) {}
 
+CylinderSurface::CylinderSurface(
+    std::shared_ptr<const CylinderBounds> cbounds,
+    std::shared_ptr<const SurfacePlacementBase> placement)
+    : RegularSurface{std::move(placement)}, m_bounds(std::move(cbounds)) {
+  throw_assert(m_bounds, "CylinderBounds must not be nullptr");
+  throw_assert(placementPtr(), "SurfacePlacementBase must not be nullptr");
+}
+
 CylinderSurface::CylinderSurface(std::shared_ptr<const CylinderBounds> cbounds,
                                  const SurfacePlacementBase& placement)
     : RegularSurface{placement}, m_bounds(std::move(cbounds)) {

Core/src/Surfaces/DiscSurface.cpp

@@ -61,6 +61,13 @@ DiscSurface::DiscSurface(const Transform3& transform,
                          std::shared_ptr<const DiscBounds> dbounds)
     : RegularSurface(transform), m_bounds(std::move(dbounds)) {}
 
+DiscSurface::DiscSurface(std::shared_ptr<const DiscBounds> dbounds,
+                         std::shared_ptr<const SurfacePlacementBase> placement)
+    : RegularSurface{std::move(placement)}, m_bounds(std::move(dbounds)) {
+  throw_assert(m_bounds, "nullptr as DiscBounds");
+  throw_assert(placementPtr(), "SurfacePlacementBase must not be nullptr");
+}
+
 DiscSurface::DiscSurface(std::shared_ptr<const DiscBounds> dbounds,
                          const SurfacePlacementBase& placement)
     : RegularSurface{placement}, m_bounds(std::move(dbounds)) {

Core/src/Surfaces/LineSurface.cpp

@@ -33,6 +33,13 @@ LineSurface::LineSurface(const Transform3& transform,
                          std::shared_ptr<const LineBounds> lbounds)
     : Surface(transform), m_bounds(std::move(lbounds)) {}
 
+LineSurface::LineSurface(std::shared_ptr<const LineBounds> lbounds,
+                         std::shared_ptr<const SurfacePlacementBase> placement)
+    : Surface{std::move(placement)}, m_bounds(std::move(lbounds)) {
+  throw_assert(m_bounds, "LineBounds must not be nullptr");
+  throw_assert(placementPtr(), "SurfacePlacementBase must not be nullptr");
+}
+
 LineSurface::LineSurface(std::shared_ptr<const LineBounds> lbounds,
                          const SurfacePlacementBase& placement)
     : Surface{placement}, m_bounds(std::move(lbounds)) {

Core/src/Surfaces/PlaneSurface.cpp

@@ -41,6 +41,14 @@ PlaneSurface::PlaneSurface(const GeometryContext& gctx,
                            const Transform3& transform)
     : RegularSurface(gctx, other, transform), m_bounds(other.m_bounds) {}
 
+PlaneSurface::PlaneSurface(
+    std::shared_ptr<const PlanarBounds> pbounds,
+    std::shared_ptr<const SurfacePlacementBase> placement)
+    : RegularSurface{std::move(placement)}, m_bounds(std::move(pbounds)) {
+  throw_assert(m_bounds, "PlaneBounds must not be nullptr");
+  throw_assert(placementPtr(), "SurfacePlacementBase must not be nullptr");
+}
+
 PlaneSurface::PlaneSurface(std::shared_ptr<const PlanarBounds> pbounds,
                            const SurfacePlacementBase& placement)
     : RegularSurface{placement}, m_bounds(std::move(pbounds)) {