diff --git a/Alignment/include/ActsAlignment/Kernel/Alignment.ipp b/Alignment/include/ActsAlignment/Kernel/Alignment.ipp index 4b00839f747..e8f07bb4a17 100644 --- a/Alignment/include/ActsAlignment/Kernel/Alignment.ipp +++ b/Alignment/include/ActsAlignment/Kernel/Alignment.ipp @@ -229,7 +229,7 @@ ActsAlignment::Alignment::align( for (unsigned int iDetElement = 0; iDetElement < alignOptions.alignedDetElements.size(); iDetElement++) { alignResult.idxedAlignSurfaces.emplace( - &alignOptions.alignedDetElements.at(iDetElement)->surface(), + alignOptions.alignedDetElements.at(iDetElement)->surface(), iDetElement); } ACTS_VERBOSE("There are " << alignResult.idxedAlignSurfaces.size() @@ -311,7 +311,7 @@ ActsAlignment::Alignment::align( // @todo if (alignmentParametersUpdated) { for (const auto& det : alignOptions.alignedDetElements) { - const auto& surface = &det->surface(); + const auto* surface = det->surface(); const auto& transform = det->localToGlobalTransform(alignOptions.fitOptions.geoContext); // write it to the result diff --git a/Core/include/Acts/Geometry/CuboidVolumeBuilder.hpp b/Core/include/Acts/Geometry/CuboidVolumeBuilder.hpp index ad3f7e6bdf3..7ab53c13b3d 100644 --- a/Core/include/Acts/Geometry/CuboidVolumeBuilder.hpp +++ b/Core/include/Acts/Geometry/CuboidVolumeBuilder.hpp @@ -59,7 +59,7 @@ class CuboidVolumeBuilder : public ITrackingVolumeBuilder { double thickness = 0.; /// Constructor function for optional detector elements /// Arguments are transform, rectangle bounds and thickness. - std::function( const Transform3&, std::shared_ptr, double)> detElementConstructor; }; diff --git a/Core/include/Acts/Geometry/VolumePlacementBase.hpp b/Core/include/Acts/Geometry/VolumePlacementBase.hpp index 18ebfa68ceb..73e48a4a8a2 100644 --- a/Core/include/Acts/Geometry/VolumePlacementBase.hpp +++ b/Core/include/Acts/Geometry/VolumePlacementBase.hpp @@ -133,17 +133,19 @@ 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 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 portalPlacement( + const std::size_t portalIdx); protected: /// Constructs the transform from the portal's frame into the @@ -160,6 +162,6 @@ class VolumePlacementBase { private: /// Resource allocation of the SurfacePlacements to align the portals - std::vector> m_portalPlacements{}; + std::vector> m_portalPlacements{}; }; } // namespace Acts diff --git a/Core/include/Acts/Geometry/detail/PortalPlacement.hpp b/Core/include/Acts/Geometry/detail/PortalPlacement.hpp index 1e8d9467500..85d839f9d68 100644 --- a/Core/include/Acts/Geometry/detail/PortalPlacement.hpp +++ b/Core/include/Acts/Geometry/detail/PortalPlacement.hpp @@ -8,7 +8,6 @@ #pragma once -#include "Acts/Surfaces/RegularSurface.hpp" #include "Acts/Surfaces/SurfacePlacementBase.hpp" namespace Acts { @@ -19,11 +18,29 @@ namespace detail { /// of an alignable volume with the alignment of the boundary surfaces /// associated with the Volume. class PortalPlacement final : public SurfacePlacementBase { - public: + private: /// Allow the VolumePlacementBase as only class to construct the portal /// placement friend class Acts::VolumePlacementBase; + struct ConstructTag {}; + + public: + /// Constructor to instantiate a PortalPlacement + /// @param tag: Construct tag to disallow construction from outside the @ref + /// VolumePlacementBase class + /// @param portalIdx: Internal index to associated the portal with the + /// i-th boundary surface of the volume + /// @param portalTrf: Transform from the portal's frame into the + /// volume (I.e. the orientation of the portal w.r.t. + /// volume) + /// @param parent: Pointer to the parent which is hosting the placement and also + /// providing the override transforms from the portal -> + /// experiment's frame + PortalPlacement(ConstructTag tag, const std::size_t portalIdx, + const Transform3& portalTrf, + const VolumePlacementBase* parent); + /// Returns the transform to switch from the portal reference /// frame into the experiment's global frame taking the alignment /// correction into account @@ -31,20 +48,6 @@ class PortalPlacement final : public SurfacePlacementBase { const Transform3& localToGlobalTransform( const GeometryContext& gctx) const override; - /// Returns the const reference to portal surface connected with this - /// placement instance - const RegularSurface& surface() const override; - - /// Returns the mutable reference to portal surface connected with this - /// placement instance - RegularSurface& surface() override; - - /// Returns the pointer to the hold surface - const std::shared_ptr& surfacePtr(); - - /// Returns the pointer to the hold surface - std::shared_ptr surfacePtr() const; - /// Declares the surface object to be non-sensitive bool isSensitive() const override; @@ -61,21 +64,6 @@ class PortalPlacement final : public SurfacePlacementBase { /// Delete the copy assignment operator PortalPlacement& operator=(const PortalPlacement& other) = delete; - /// Constructor to instantiate a PortalPlacement - /// @param portalIdx: Internal index to associated the portal with the - /// i-th boundary surface of the volume - /// @param portalTrf: Transform from the portal's frame into the - /// volume (I.e. the orientation of the portal w.r.t. - /// volume) - /// @param parent: Pointer to the parent which is hosting the placement and also - /// providing the override transforms from the portal -> - /// experiment's frame - /// @param surface: Pointer to the portal surface itself which is becoming alignable - /// with the construction of this PortalPlacement - PortalPlacement(const std::size_t portalIdx, const Transform3& portalTrf, - const VolumePlacementBase* parent, - std::shared_ptr surface); - private: /// Assembles the transform to switch from the portal's frame /// to the experiment's global frame which is essentially the @@ -87,10 +75,10 @@ class PortalPlacement final : public SurfacePlacementBase { /// Orientation of the portal surface w.r.t the volume Transform3 m_portalToVolumeCenter{Transform3::Identity()}; - /// Pointer to the surface held by the placement - std::shared_ptr m_surface{}; + /// Pointer to the parent managing this instance const VolumePlacementBase* m_parent{nullptr}; + /// Internal index of the portal std::size_t m_portalIdx{0ul}; }; diff --git a/Core/include/Acts/Surfaces/CylinderSurface.hpp b/Core/include/Acts/Surfaces/CylinderSurface.hpp index b9afd97ea00..7aeec4ad746 100644 --- a/Core/include/Acts/Surfaces/CylinderSurface.hpp +++ b/Core/include/Acts/Surfaces/CylinderSurface.hpp @@ -75,7 +75,7 @@ class CylinderSurface : public RegularSurface { /// and that the `Surface` is actually owned by /// the `SurfacePlacementBase` instance CylinderSurface(std::shared_ptr cbounds, - const SurfacePlacementBase& placement); + std::shared_ptr placement); /// Copy constructor /// diff --git a/Core/include/Acts/Surfaces/DiscSurface.hpp b/Core/include/Acts/Surfaces/DiscSurface.hpp index db7af3c9195..1463192c631 100644 --- a/Core/include/Acts/Surfaces/DiscSurface.hpp +++ b/Core/include/Acts/Surfaces/DiscSurface.hpp @@ -88,14 +88,9 @@ class DiscSurface : public RegularSurface { /// Constructor from SurfacePlacementBase : Element proxy /// /// @param dbounds The disc bounds describing the surface coverage - /// @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 + /// @param placement Shared pointer to the surface placement explicit DiscSurface(std::shared_ptr dbounds, - const SurfacePlacementBase& placement); + std::shared_ptr placement); /// Copy Constructor /// diff --git a/Core/include/Acts/Surfaces/LineSurface.hpp b/Core/include/Acts/Surfaces/LineSurface.hpp index 629949b93b7..a41d65415d3 100644 --- a/Core/include/Acts/Surfaces/LineSurface.hpp +++ b/Core/include/Acts/Surfaces/LineSurface.hpp @@ -59,14 +59,9 @@ class LineSurface : public Surface { /// /// @param lbounds are the bounds describing the line dimensions, they must /// not be nullptr - /// @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 + /// @param placement Shared pointer to the surface placement explicit LineSurface(std::shared_ptr lbounds, - const SurfacePlacementBase& placement); + std::shared_ptr placement); /// Copy constructor /// diff --git a/Core/include/Acts/Surfaces/PlaneSurface.hpp b/Core/include/Acts/Surfaces/PlaneSurface.hpp index ef2a24c4fff..677cea14dd3 100644 --- a/Core/include/Acts/Surfaces/PlaneSurface.hpp +++ b/Core/include/Acts/Surfaces/PlaneSurface.hpp @@ -63,7 +63,7 @@ class PlaneSurface : public RegularSurface { /// and that the `Surface` is actually owned by /// the `SurfacePlacementBase` instance PlaneSurface(std::shared_ptr pbounds, - const SurfacePlacementBase& placement); + std::shared_ptr placement); /// Constructor for Planes with (optional) shared bounds object /// diff --git a/Core/include/Acts/Surfaces/Surface.hpp b/Core/include/Acts/Surfaces/Surface.hpp index 6c84e800903..a2352a0053c 100644 --- a/Core/include/Acts/Surfaces/Surface.hpp +++ b/Core/include/Acts/Surfaces/Surface.hpp @@ -29,6 +29,7 @@ #include #include #include +#include namespace Acts { @@ -91,15 +92,13 @@ 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 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 - explicit Surface(const SurfacePlacementBase& placement) noexcept; + /// @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 placement); /// Copy constructor with optional shift /// @@ -233,8 +232,9 @@ class Surface : public virtual GeometryObject, const; /// Assign a placement object which may dynamically align the surface in space - /// @param placement: Placement object defining the surface's position - void assignSurfacePlacement(const SurfacePlacementBase& placement); + /// (shared ownership — preferred) + /// @param placement Shared pointer to the placement object + void assignSurfacePlacement(std::shared_ptr placement); /// Assign the surface material description /// @@ -539,8 +539,8 @@ class Surface : public virtual GeometryObject, CloneablePtr m_transform{}; private: - /// Pointer to the a SurfacePlacement - const SurfacePlacementBase* m_placement{nullptr}; + /// The surface placement + std::shared_ptr m_placement; /// The associated layer Layer - layer in which the Surface is be embedded, /// nullptr if not associated @@ -554,6 +554,7 @@ class Surface : public virtual GeometryObject, /// Thickness of the surface in the normal direction double m_thickness{0.}; + /// Calculate the derivative of bound track parameters w.r.t. /// alignment parameters of its reference surface (i.e. origin in global 3D /// Cartesian coordinates and its rotation represented with extrinsic Euler diff --git a/Core/include/Acts/Surfaces/SurfacePlacementBase.hpp b/Core/include/Acts/Surfaces/SurfacePlacementBase.hpp index 2589cc748f8..8a93b454089 100644 --- a/Core/include/Acts/Surfaces/SurfacePlacementBase.hpp +++ b/Core/include/Acts/Surfaces/SurfacePlacementBase.hpp @@ -11,6 +11,8 @@ #include "Acts/Definitions/Algebra.hpp" #include "Acts/Geometry/GeometryContext.hpp" +#include + 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 { public: /// @brief Virtual default constructor virtual ~SurfacePlacementBase() = default; @@ -48,16 +51,21 @@ class SurfacePlacementBase { /// Acts::Surface::surfacePlacement method return a pointer to /// this object. /// @return Reference to a surface that represents this detector element - virtual const Surface& surface() const = 0; + const Surface* surface() const; - /// @copydoc surface - /// @return Reference to a surface that represents this detector element - virtual Surface& surface() = 0; + /// Assign a surface to this placement element. + /// @note The placement will NOT take ownership of the surface. + void assignSurface(const std::shared_ptr& surface); /// @brief Returns whether the placement corresponds to a surface on which /// the measurements from the experiment are represented, i.e. it is // a detector surface /// @return True if this is a sensitive surface virtual bool isSensitive() const = 0; + + private: + // This is a raw pointer to the surface that is associated with this + // placement. That surface will eventually own this placement. + Surface* m_surface; }; } // namespace Acts diff --git a/Core/src/Geometry/CuboidVolumeBuilder.cpp b/Core/src/Geometry/CuboidVolumeBuilder.cpp index 7420aa0608b..33a0ba7493c 100644 --- a/Core/src/Geometry/CuboidVolumeBuilder.cpp +++ b/Core/src/Geometry/CuboidVolumeBuilder.cpp @@ -44,9 +44,9 @@ std::shared_ptr CuboidVolumeBuilder::buildSurface( // Create and store surface if (cfg.detElementConstructor) { - surface = Surface::makeShared( - cfg.rBounds, - *(cfg.detElementConstructor(trafo, cfg.rBounds, cfg.thickness))); + surface = Surface::makeShared(trafo, cfg.rBounds); + surface->assignSurfacePlacement( + cfg.detElementConstructor(trafo, cfg.rBounds, cfg.thickness)); surface->assignThickness(cfg.thickness); } else { surface = Surface::makeShared(trafo, cfg.rBounds); diff --git a/Core/src/Geometry/PortalPlacement.cpp b/Core/src/Geometry/PortalPlacement.cpp index 967af39922b..cdd1a5d21f7 100644 --- a/Core/src/Geometry/PortalPlacement.cpp +++ b/Core/src/Geometry/PortalPlacement.cpp @@ -12,17 +12,13 @@ namespace Acts::detail { -PortalPlacement::PortalPlacement(const std::size_t portalIdx, +PortalPlacement::PortalPlacement(ConstructTag /*tag*/, + const std::size_t portalIdx, const Transform3& portalTrf, - const VolumePlacementBase* parent, - std::shared_ptr surface) + const VolumePlacementBase* parent) : m_portalToVolumeCenter{portalTrf}, - m_surface{std::move(surface)}, m_parent{parent}, - m_portalIdx{portalIdx} { - assert(m_surface != nullptr); - m_surface->assignSurfacePlacement(*this); -} + m_portalIdx{portalIdx} {} Transform3 PortalPlacement::assembleFullTransform( const GeometryContext& gctx) const { @@ -34,14 +30,6 @@ const Transform3& PortalPlacement::localToGlobalTransform( return m_parent->portalLocalToGlobal(gctx, m_portalIdx); } -const RegularSurface& PortalPlacement::surface() const { - return *m_surface; -} - -RegularSurface& PortalPlacement::surface() { - return *m_surface; -} - bool PortalPlacement::isSensitive() const { return false; } @@ -54,11 +42,4 @@ const Transform3& PortalPlacement::portalToVolumeCenter() const { return m_portalToVolumeCenter; } -const std::shared_ptr& PortalPlacement::surfacePtr() { - return m_surface; -} - -std::shared_ptr PortalPlacement::surfacePtr() const { - return m_surface; -} } // namespace Acts::detail diff --git a/Core/src/Geometry/VolumePlacementBase.cpp b/Core/src/Geometry/VolumePlacementBase.cpp index 43b5cad46a8..bd89a254c11 100644 --- a/Core/src/Geometry/VolumePlacementBase.cpp +++ b/Core/src/Geometry/VolumePlacementBase.cpp @@ -46,19 +46,22 @@ void VolumePlacementBase::makePortalsAlignable( globalToLocalTransform(gctx) * portalSurface->localToGlobalTransform(gctx); - m_portalPlacements.emplace_back(std::make_unique( - portalIdx, portalToVolTrf, this, portalSurface)); + auto placement = std::make_shared( + detail::PortalPlacement::ConstructTag{}, portalIdx, portalToVolTrf, + this); + placement->assignSurface(portalSurface); + m_portalPlacements.push_back(std::move(placement)); } } -const detail::PortalPlacement* VolumePlacementBase::portalPlacement( - const std::size_t portalIdx) const { - return m_portalPlacements.at(portalIdx).get(); +std::shared_ptr +VolumePlacementBase::portalPlacement(const std::size_t portalIdx) const { + return m_portalPlacements.at(portalIdx); } -detail::PortalPlacement* VolumePlacementBase::portalPlacement( +std::shared_ptr VolumePlacementBase::portalPlacement( const std::size_t portalIdx) { - return m_portalPlacements.at(portalIdx).get(); + return m_portalPlacements.at(portalIdx); } std::size_t VolumePlacementBase::nPortalPlacements() const { diff --git a/Core/src/Surfaces/CMakeLists.txt b/Core/src/Surfaces/CMakeLists.txt index 97e6c542b52..b767bd15a06 100644 --- a/Core/src/Surfaces/CMakeLists.txt +++ b/Core/src/Surfaces/CMakeLists.txt @@ -31,4 +31,5 @@ target_sources( detail/AlignmentHelper.cpp detail/AnnulusBoundsHelper.cpp detail/MergeHelper.cpp + SurfacePlacementBase.cpp ) diff --git a/Core/src/Surfaces/CylinderSurface.cpp b/Core/src/Surfaces/CylinderSurface.cpp index 1afe172dc0a..11f8e4af52b 100644 --- a/Core/src/Surfaces/CylinderSurface.cpp +++ b/Core/src/Surfaces/CylinderSurface.cpp @@ -51,10 +51,10 @@ CylinderSurface::CylinderSurface(const Transform3& transform, double radius, m_bounds(std::make_shared( radius, halfz, halfphi, avphi, bevelMinZ, bevelMaxZ)) {} -CylinderSurface::CylinderSurface(std::shared_ptr cbounds, - const SurfacePlacementBase& placement) - : RegularSurface{placement}, m_bounds(std::move(cbounds)) { - // surfaces representing a detector element must have bounds +CylinderSurface::CylinderSurface( + std::shared_ptr cbounds, + std::shared_ptr placement) + : RegularSurface{std::move(placement)}, m_bounds(std::move(cbounds)) { throw_assert(m_bounds, "CylinderBounds must not be nullptr"); } diff --git a/Core/src/Surfaces/DiscSurface.cpp b/Core/src/Surfaces/DiscSurface.cpp index e14f4e41123..0136c9a79fb 100644 --- a/Core/src/Surfaces/DiscSurface.cpp +++ b/Core/src/Surfaces/DiscSurface.cpp @@ -62,8 +62,8 @@ DiscSurface::DiscSurface(const Transform3& transform, : RegularSurface(transform), m_bounds(std::move(dbounds)) {} DiscSurface::DiscSurface(std::shared_ptr dbounds, - const SurfacePlacementBase& placement) - : RegularSurface{placement}, m_bounds(std::move(dbounds)) { + std::shared_ptr placement) + : RegularSurface{std::move(placement)}, m_bounds(std::move(dbounds)) { throw_assert(m_bounds, "nullptr as DiscBounds"); } diff --git a/Core/src/Surfaces/LineSurface.cpp b/Core/src/Surfaces/LineSurface.cpp index 807d921a13a..ed4bcb7111d 100644 --- a/Core/src/Surfaces/LineSurface.cpp +++ b/Core/src/Surfaces/LineSurface.cpp @@ -34,8 +34,8 @@ LineSurface::LineSurface(const Transform3& transform, : Surface(transform), m_bounds(std::move(lbounds)) {} LineSurface::LineSurface(std::shared_ptr lbounds, - const SurfacePlacementBase& placement) - : Surface{placement}, m_bounds(std::move(lbounds)) { + std::shared_ptr placement) + : Surface{std::move(placement)}, m_bounds(std::move(lbounds)) { throw_assert(m_bounds, "LineBounds must not be nullptr"); } diff --git a/Core/src/Surfaces/PlaneSurface.cpp b/Core/src/Surfaces/PlaneSurface.cpp index cce26085362..22f450a6206 100644 --- a/Core/src/Surfaces/PlaneSurface.cpp +++ b/Core/src/Surfaces/PlaneSurface.cpp @@ -42,10 +42,10 @@ PlaneSurface::PlaneSurface(const GeometryContext& gctx, : RegularSurface(gctx, other, transform), m_bounds(other.m_bounds) {} PlaneSurface::PlaneSurface(std::shared_ptr pbounds, - const SurfacePlacementBase& placement) - : RegularSurface{placement}, m_bounds(std::move(pbounds)) { - // surfaces representing a detector element must have bounds + std::shared_ptr placement) + : RegularSurface{std::move(placement)}, m_bounds(std::move(pbounds)) { throw_assert(m_bounds, "PlaneBounds must not be nullptr"); + throw_assert(placement, "SurfacePlacementBase must not be nullptr"); } PlaneSurface::PlaneSurface(const Transform3& transform, diff --git a/Core/src/Surfaces/Surface.cpp b/Core/src/Surfaces/Surface.cpp index 3c3b8083ef3..cf148872325 100644 --- a/Core/src/Surfaces/Surface.cpp +++ b/Core/src/Surfaces/Surface.cpp @@ -12,6 +12,7 @@ #include "Acts/Surfaces/SurfaceBounds.hpp" #include "Acts/Surfaces/detail/AlignmentHelper.hpp" #include "Acts/Utilities/JacobianHelpers.hpp" +#include "Acts/Utilities/ThrowAssert.hpp" #include "Acts/Visualization/ViewConfig.hpp" #include @@ -22,8 +23,10 @@ namespace Acts { Surface::Surface(const Transform3& transform) : GeometryObject(), m_transform(std::make_unique(transform)) {} -Surface::Surface(const SurfacePlacementBase& placement) noexcept - : GeometryObject(), m_placement(&placement) {} +Surface::Surface(std::shared_ptr placement) + : GeometryObject(), m_placement(std::move(placement)) { + throw_assert(m_placement, "SurfacePlacementBase must not be nullptr"); +} Surface::Surface(const GeometryContext& gctx, const Surface& other, const Transform3& shift) noexcept @@ -305,7 +308,7 @@ FreeToPathMatrix Surface::freeToPathDerivative(const GeometryContext& gctx, } const SurfacePlacementBase* Surface::surfacePlacement() const { - return m_placement; + return m_placement.get(); } double Surface::thickness() const { @@ -330,13 +333,15 @@ Surface::surfaceMaterialSharedPtr() const { return m_surfaceMaterial; } -void Surface::assignSurfacePlacement(const SurfacePlacementBase& placement) { - m_placement = &placement; - // resetting the transform as it will be handled through the detector element - // now +void Surface::assignSurfacePlacement( + std::shared_ptr placement) { + m_placement = std::move(placement); m_transform.reset(); - // reset sensitivity flag m_isSensitive = false; + // Call assignSurface to wire the back-pointer on the placement side. + // assignSurface checks surface->surfacePlacement(): since m_placement is + // already set above, it will find `this` and return without calling back. + m_placement->assignSurface(getSharedPtr()); } void Surface::assignSurfaceMaterial( @@ -345,7 +350,7 @@ void Surface::assignSurfaceMaterial( } void Surface::associateLayer(const Layer& lay) { - m_associatedLayer = (&lay); + m_associatedLayer = &lay; } void Surface::visualize(IVisualization3D& helper, const GeometryContext& gctx, diff --git a/Core/src/Surfaces/SurfacePlacementBase.cpp b/Core/src/Surfaces/SurfacePlacementBase.cpp new file mode 100644 index 00000000000..89dd32fe3af --- /dev/null +++ b/Core/src/Surfaces/SurfacePlacementBase.cpp @@ -0,0 +1,40 @@ +// This file is part of the ACTS project. +// +// Copyright (C) 2016 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 +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +#include "Acts/Surfaces/SurfacePlacementBase.hpp" + +#include "Acts/Surfaces/Surface.hpp" + +namespace Acts { + +const Surface* SurfacePlacementBase::surface() const { + return m_surface; +} + +void SurfacePlacementBase::assignSurface( + const std::shared_ptr& surface) { + m_surface = surface.get(); + + const auto* placement = surface->surfacePlacement(); + + // If the surface has no placement, assign this one to it + if (placement == nullptr) { + surface->assignSurfacePlacement(shared_from_this()); + return; + } + + // If we get here, the surface has a placement, and it is not this one, + // it's an error + if (placement != this) { + throw std::runtime_error( + "SurfacePlacementBase: surface already has a placement. The placement " + "cannot be reassigned"); + } +} + +} // namespace Acts \ No newline at end of file diff --git a/Examples/Detectors/Common/include/ActsExamples/DetectorCommons/Aligned.hpp b/Examples/Detectors/Common/include/ActsExamples/DetectorCommons/Aligned.hpp index 59414a4d7cc..1bfc3ac8d0a 100644 --- a/Examples/Detectors/Common/include/ActsExamples/DetectorCommons/Aligned.hpp +++ b/Examples/Detectors/Common/include/ActsExamples/DetectorCommons/Aligned.hpp @@ -12,7 +12,6 @@ #include "Acts/Geometry/GeometryContext.hpp" #include "ActsExamples/DetectorCommons/AlignmentContext.hpp" -#include namespace ActsExamples { @@ -34,7 +33,7 @@ class Aligned : public DetectorElement { const auto* alignmentContext = gctx.maybeGet(); if (alignmentContext != nullptr && alignmentContext->store != nullptr) { const auto* transform = - alignmentContext->store->contextualTransform(this->surface()); + alignmentContext->store->contextualTransform(*this->surface()); // The store may only have a subset of surfaces registered if (transform != nullptr) { return *transform; diff --git a/Examples/Detectors/GenericDetector/include/ActsExamples/GenericDetector/GenericDetectorElement.hpp b/Examples/Detectors/GenericDetector/include/ActsExamples/GenericDetector/GenericDetectorElement.hpp index a47b7c08215..433a52e7364 100644 --- a/Examples/Detectors/GenericDetector/include/ActsExamples/GenericDetector/GenericDetectorElement.hpp +++ b/Examples/Detectors/GenericDetector/include/ActsExamples/GenericDetector/GenericDetectorElement.hpp @@ -14,6 +14,7 @@ #include "Acts/Surfaces/SurfacePlacementBase.hpp" #include +#include namespace Acts { class Surface; @@ -76,11 +77,44 @@ class GenericDetectorElement : public Acts::SurfacePlacementBase { /// Return local to global transform associated with this detector element const Acts::Transform3& nominalTransform() const; - /// Return surface associated with this detector element - const Acts::Surface& surface() const override; + /// Create and return the surface associated with this detector element. + /// + /// This method uses @c shared_from_this() so it must only be called after + /// the element is already managed by a @c std::shared_ptr. The returned + /// surface takes shared ownership of this detector element. + /// + /// @return Shared pointer to the created surface + std::shared_ptr createSurface(); - /// Non-cost access to surface associated with this detector element - Acts::Surface& surface() override; + /// Convenience factory: constructs the element and immediately calls + /// @c createSurface(), returning both. + /// + /// @param identifier Module identifier + /// @param transform Placement transform in 3D global space + /// @param pBounds Planar bounds of the detector element + /// @param thickness Module thickness + /// @param material Optional surface material + /// @return Pair of (shared element, shared surface) + static std::pair, + std::shared_ptr> + create(const Identifier identifier, const Acts::Transform3& transform, + std::shared_ptr pBounds, double thickness, + std::shared_ptr material = nullptr); + + /// Convenience factory: constructs the element and immediately calls + /// @c createSurface(), returning both. + /// + /// @param identifier Module identifier + /// @param transform Placement transform in 3D global space + /// @param dBounds Disc bounds of the detector element + /// @param thickness Module thickness + /// @param material Optional surface material + /// @return Pair of (shared element, shared surface) + static std::pair, + std::shared_ptr> + create(const Identifier identifier, const Acts::Transform3& transform, + std::shared_ptr dBounds, double thickness, + std::shared_ptr material = nullptr); /// The maximal thickness of the detector element wrt normal axis double thickness() const; @@ -95,13 +129,13 @@ class GenericDetectorElement : public Acts::SurfacePlacementBase { Identifier m_elementIdentifier; /// the transform for positioning in 3D space const Acts::Transform3 m_elementTransform; - /// the surface represented by it - std::shared_ptr m_elementSurface; /// the element thickness double m_elementThickness; - /// store either + /// store either planar or disc bounds std::shared_ptr m_elementPlanarBounds = nullptr; std::shared_ptr m_elementDiscBounds = nullptr; + /// optional material, applied when createSurface() is called + std::shared_ptr m_elementMaterial = nullptr; }; } // namespace ActsExamples diff --git a/Examples/Detectors/GenericDetector/src/GenericDetectorElement.cpp b/Examples/Detectors/GenericDetector/src/GenericDetectorElement.cpp index 2014bcca3d3..53f23bb7641 100644 --- a/Examples/Detectors/GenericDetector/src/GenericDetectorElement.cpp +++ b/Examples/Detectors/GenericDetector/src/GenericDetectorElement.cpp @@ -21,14 +21,10 @@ GenericDetectorElement::GenericDetectorElement( std::shared_ptr material) : m_elementIdentifier(identifier), m_elementTransform(transform), - m_elementSurface( - Acts::Surface::makeShared(pBounds, *this)), m_elementThickness(thickness), m_elementPlanarBounds(std::move(pBounds)), - m_elementDiscBounds(nullptr) { - m_elementSurface->assignSurfaceMaterial(std::move(material)); - m_elementSurface->assignThickness(thickness); -} + m_elementDiscBounds(nullptr), + m_elementMaterial(std::move(material)) {} GenericDetectorElement::GenericDetectorElement( const Identifier identifier, const Acts::Transform3& transform, @@ -36,13 +32,52 @@ GenericDetectorElement::GenericDetectorElement( std::shared_ptr material) : m_elementIdentifier(identifier), m_elementTransform(transform), - m_elementSurface( - Acts::Surface::makeShared(dBounds, *this)), m_elementThickness(thickness), m_elementPlanarBounds(nullptr), - m_elementDiscBounds(std::move(dBounds)) { - m_elementSurface->assignSurfaceMaterial(std::move(material)); - m_elementSurface->assignThickness(thickness); + m_elementDiscBounds(std::move(dBounds)), + m_elementMaterial(std::move(material)) {} + +std::shared_ptr GenericDetectorElement::createSurface() { + std::shared_ptr surf; + if (m_elementPlanarBounds) { + surf = Acts::Surface::makeShared(m_elementTransform, + m_elementPlanarBounds); + } else { + surf = Acts::Surface::makeShared(m_elementTransform, + m_elementDiscBounds); + } + assignSurface(surf); + surf->assignThickness(m_elementThickness); + if (m_elementMaterial) { + surf->assignSurfaceMaterial(m_elementMaterial); + } + return surf; +} + +std::pair, + std::shared_ptr> +GenericDetectorElement::create( + const Identifier identifier, const Acts::Transform3& transform, + std::shared_ptr pBounds, double thickness, + std::shared_ptr material) { + auto el = std::make_shared( + identifier, transform, std::move(pBounds), thickness, + std::move(material)); + auto surf = el->createSurface(); + return {std::move(el), std::move(surf)}; +} + +std::pair, + std::shared_ptr> +GenericDetectorElement::create( + const Identifier identifier, const Acts::Transform3& transform, + std::shared_ptr dBounds, double thickness, + std::shared_ptr material) { + auto el = std::make_shared( + identifier, transform, std::move(dBounds), thickness, + std::move(material)); + auto surf = el->createSurface(); + return {std::move(el), std::move(surf)}; } const Acts::Transform3& GenericDetectorElement::localToGlobalTransform( @@ -54,14 +89,6 @@ const Acts::Transform3& GenericDetectorElement::nominalTransform() const { return m_elementTransform; } -const Acts::Surface& GenericDetectorElement::surface() const { - return *m_elementSurface; -} - -Acts::Surface& GenericDetectorElement::surface() { - return *m_elementSurface; -} - double GenericDetectorElement::thickness() const { return m_elementThickness; } diff --git a/Examples/Detectors/GenericDetector/src/ProtoLayerCreator.cpp b/Examples/Detectors/GenericDetector/src/ProtoLayerCreator.cpp index dfa1fa2eb38..0b700570b76 100644 --- a/Examples/Detectors/GenericDetector/src/ProtoLayerCreator.cpp +++ b/Examples/Detectors/GenericDetector/src/ProtoLayerCreator.cpp @@ -103,7 +103,7 @@ std::vector ProtoLayerCreator::centralProtoLayers( auto moduleElement = m_cfg.detectorElementFactory( moduleTransform, moduleBounds, moduleThickness, moduleMaterialPtr); // register the surface - sVector.push_back(moduleElement->surface().getSharedPtr()); + sVector.push_back(moduleElement->createSurface()); // IF double modules exist // and the backside one (if configured to do so) if (!m_cfg.centralModuleBacksideGap.empty()) { @@ -126,7 +126,7 @@ std::vector ProtoLayerCreator::centralProtoLayers( m_cfg.detectorElementFactory(bsModuleTransform, moduleBounds, moduleThickness, moduleMaterialPtr); // register the backside surface - sVector.push_back(bsModuleElement->surface().getSharedPtr()); + sVector.push_back(bsModuleElement->createSurface()); } } @@ -253,7 +253,7 @@ std::vector ProtoLayerCreator::createProtoLayers( m_cfg.detectorElementFactory(moduleTransform, moduleBounds, moduleThickness, moduleMaterialPtr); // register the surface - esVector.push_back(moduleElement->surface().getSharedPtr()); + esVector.push_back(moduleElement->createSurface()); // now deal with the potential backside if (!m_cfg.posnegModuleBacksideGap.empty()) { // the new centers @@ -276,7 +276,7 @@ std::vector ProtoLayerCreator::createProtoLayers( bsModuleTransform, moduleBounds, moduleThickness, moduleMaterialPtr); // register the backside surface - esVector.push_back(bsModuleElement->surface().getSharedPtr()); + esVector.push_back(bsModuleElement->createSurface()); } } // counter of rings diff --git a/Examples/Detectors/GeoModelDetector/src/AlignedGeoModelDetectorElement.cpp b/Examples/Detectors/GeoModelDetector/src/AlignedGeoModelDetectorElement.cpp index 386fbb6ba2d..2c5a99857a7 100644 --- a/Examples/Detectors/GeoModelDetector/src/AlignedGeoModelDetectorElement.cpp +++ b/Examples/Detectors/GeoModelDetector/src/AlignedGeoModelDetectorElement.cpp @@ -12,6 +12,8 @@ std::shared_ptr ActsExamples::alignedGeoModelDetectorElementFactory( const PVConstLink& geoPhysVol, std::shared_ptr surface, const Acts::Transform3& sfTransform, double thickness) { - return std::make_shared( - geoPhysVol, std::move(surface), sfTransform, thickness); + auto el = std::make_shared( + geoPhysVol, sfTransform, thickness); + el->assignSurface(surface); + return el; } diff --git a/Examples/Detectors/ITkModuleSplitting/include/ActsExamples/ITkModuleSplitting/ITkModuleSplitting.hpp b/Examples/Detectors/ITkModuleSplitting/include/ActsExamples/ITkModuleSplitting/ITkModuleSplitting.hpp index 95fb3ea1620..ad387e760d6 100644 --- a/Examples/Detectors/ITkModuleSplitting/include/ActsExamples/ITkModuleSplitting/ITkModuleSplitting.hpp +++ b/Examples/Detectors/ITkModuleSplitting/include/ActsExamples/ITkModuleSplitting/ITkModuleSplitting.hpp @@ -21,14 +21,14 @@ namespace ActsExamples::ITk { template -inline std::vector> splitBarrelModule( +inline std::vector> splitBarrelModule( const Acts::GeometryContext& gctx, - const std::shared_ptr& detElement, + const std::shared_ptr& detElement, unsigned int nSegments, const element_factory_t& factory, const std::string& name, const Acts::Logger& logger = Acts::getDummyLogger()) { // Retrieve the surface - const Acts::Surface& surface = detElement->surface(); + const Acts::Surface& surface = *detElement->surface(); const Acts::SurfaceBounds& bounds = surface.bounds(); if (bounds.type() != Acts::SurfaceBounds::eRectangle || nSegments <= 1u) { ACTS_WARNING("Invalid splitting config for barrel node: " @@ -37,7 +37,7 @@ inline std::vector> splitBarrelModule( } // Output container for the submodules - std::vector> detElements = {}; + std::vector> detElements = {}; detElements.reserve(nSegments); // Get the geometric information @@ -80,7 +80,7 @@ inline std::vector> splitDiscModule( const element_factory_t& factory, const std::string& name, const Acts::Logger& logger = Acts::getDummyLogger()) { // Retrieve the surface - const Acts::Surface& surface = detElement->surface(); + const Acts::Surface& surface = *detElement->surface(); const Acts::SurfaceBounds& bounds = surface.bounds(); // Check annulus bounds origin @@ -103,7 +103,7 @@ inline std::vector> splitDiscModule( auto nSegments = splitRanges.size(); // Output container for the submodules - std::vector> detElements = {}; + std::vector> detElements = {}; detElements.reserve(nSegments); // Get the geometric information diff --git a/Examples/Detectors/TGeoDetector/include/ActsExamples/TGeoDetector/TGeoITkModuleSplitter.hpp b/Examples/Detectors/TGeoDetector/include/ActsExamples/TGeoDetector/TGeoITkModuleSplitter.hpp index ce0c2e76b1e..70e710334a2 100644 --- a/Examples/Detectors/TGeoDetector/include/ActsExamples/TGeoDetector/TGeoITkModuleSplitter.hpp +++ b/Examples/Detectors/TGeoDetector/include/ActsExamples/TGeoDetector/TGeoITkModuleSplitter.hpp @@ -63,9 +63,9 @@ class TGeoITkModuleSplitter : public ActsPlugins::ITGeoDetectorElementSplitter { /// @note If no split is performed the unsplit detector element is returned /// /// @return a vector of TGeoDetectorElement objects - std::vector> split( + std::vector> split( const Acts::GeometryContext& gctx, - std::shared_ptr detElement) + std::shared_ptr detElement) const override; private: @@ -86,10 +86,10 @@ class TGeoITkModuleSplitter : public ActsPlugins::ITGeoDetectorElementSplitter { /// @note If no split is performed the unsplit detector element is returned /// /// @return a vector of TGeoDetectorElement objects - std::vector> + std::vector> splitBarrelModule( const Acts::GeometryContext& gctx, - const std::shared_ptr& detElement, + const std::shared_ptr& detElement, unsigned int nSegments) const; /// Take a geometry context and TGeoElement in the Itk disks and split it @@ -102,10 +102,10 @@ class TGeoITkModuleSplitter : public ActsPlugins::ITGeoDetectorElementSplitter { /// @note If no split is performed the unsplit detector element is returned /// /// @return a vector of TGeoDetectorElement objects - std::vector> + std::vector> splitDiscModule( const Acts::GeometryContext& gctx, - const std::shared_ptr& detElement, + const std::shared_ptr& detElement, const std::vector& splitRanges) const; /// Contains the splitting parameters, sorted by sensor type diff --git a/Examples/Detectors/TGeoDetector/src/TGeoITkModuleSplitter.cpp b/Examples/Detectors/TGeoDetector/src/TGeoITkModuleSplitter.cpp index f2c1d656f14..007694eaac3 100644 --- a/Examples/Detectors/TGeoDetector/src/TGeoITkModuleSplitter.cpp +++ b/Examples/Detectors/TGeoDetector/src/TGeoITkModuleSplitter.cpp @@ -51,10 +51,10 @@ void TGeoITkModuleSplitter::initSplitCategories() { } /// If applicable, returns a split detector element -inline std::vector> +inline std::vector> TGeoITkModuleSplitter::split( const Acts::GeometryContext& gctx, - std::shared_ptr detElement) const { + std::shared_ptr detElement) const { // Is the current node covered by this splitter? const TGeoNode& node = detElement->tgeoNode(); auto sensorName = std::string(node.GetName()); @@ -84,15 +84,15 @@ TGeoITkModuleSplitter::split( } /// If applicable, returns a split detector element -inline std::vector> +inline std::vector> TGeoITkModuleSplitter::splitBarrelModule( const Acts::GeometryContext& gctx, - const std::shared_ptr& detElement, + const std::shared_ptr& detElement, unsigned int nSegments) const { auto name = detElement->tgeoNode().GetName(); auto factory = [&](const auto& trafo, const auto& bounds) { - return std::make_shared( + return std::make_shared( detElement->identifier(), detElement->tgeoNode(), trafo, bounds, detElement->thickness()); }; @@ -102,15 +102,15 @@ TGeoITkModuleSplitter::splitBarrelModule( } /// If applicable, returns a split detector element -inline std::vector> +inline std::vector> TGeoITkModuleSplitter::splitDiscModule( const Acts::GeometryContext& gctx, - const std::shared_ptr& detElement, + const std::shared_ptr& detElement, const std::vector& splitRanges) const { auto name = detElement->tgeoNode().GetName(); auto factory = [&](const auto& trafo, const auto& bounds) { - return std::make_shared( + return std::make_shared( detElement->identifier(), detElement->tgeoNode(), trafo, bounds, detElement->thickness()); }; diff --git a/Examples/Detectors/TelescopeDetector/include/ActsExamples/TelescopeDetector/TelescopeDetectorElement.hpp b/Examples/Detectors/TelescopeDetector/include/ActsExamples/TelescopeDetector/TelescopeDetectorElement.hpp index e2dcdf0ac7f..17d91a94f27 100644 --- a/Examples/Detectors/TelescopeDetector/include/ActsExamples/TelescopeDetector/TelescopeDetectorElement.hpp +++ b/Examples/Detectors/TelescopeDetector/include/ActsExamples/TelescopeDetector/TelescopeDetectorElement.hpp @@ -66,11 +66,8 @@ class TelescopeDetectorElement : public Acts::SurfacePlacementBase { /// Destructor ~TelescopeDetectorElement() override = default; - /// Return surface associated with this detector element - const Acts::Surface& surface() const final; - - /// Non-const access to the surface associated with this detector element - Acts::Surface& surface() final; + /// Create and return the surface associated with this detector element. + std::shared_ptr createSurface(); /// The maximal thickness of the detector element wrt normal axis double thickness() const; @@ -107,24 +104,16 @@ class TelescopeDetectorElement : public Acts::SurfacePlacementBase { std::shared_ptr m_elementTransform = nullptr; // the aligned transforms std::vector> m_alignedTransforms = {}; - /// the surface represented by it - std::shared_ptr m_elementSurface = nullptr; /// the element thickness double m_elementThickness = 0.; /// the planar bounds std::shared_ptr m_elementPlanarBounds = nullptr; /// the disc bounds std::shared_ptr m_elementDiscBounds = nullptr; + /// optional material applied when createSurface() is called + std::shared_ptr m_elementMaterial = nullptr; }; -inline const Acts::Surface& TelescopeDetectorElement::surface() const { - return *m_elementSurface; -} - -inline Acts::Surface& TelescopeDetectorElement::surface() { - return *m_elementSurface; -} - inline double TelescopeDetectorElement::thickness() const { return m_elementThickness; } diff --git a/Examples/Detectors/TelescopeDetector/src/BuildTelescopeDetector.cpp b/Examples/Detectors/TelescopeDetector/src/BuildTelescopeDetector.cpp index 9a5d31bd81c..e6dd7868b12 100644 --- a/Examples/Detectors/TelescopeDetector/src/BuildTelescopeDetector.cpp +++ b/Examples/Detectors/TelescopeDetector/src/BuildTelescopeDetector.cpp @@ -101,7 +101,7 @@ ActsExamples::buildTelescopeDetector( surfaceMaterial); } // Get the surface - auto surface = detElement->surface().getSharedPtr(); + auto surface = detElement->createSurface(); // Add the detector element to the detector store detectorStore.push_back(std::move(detElement)); // Construct the surface array (one surface contained) diff --git a/Examples/Detectors/TelescopeDetector/src/TelescopeDetectorElement.cpp b/Examples/Detectors/TelescopeDetector/src/TelescopeDetectorElement.cpp index c5efa55b59f..13dafa25afc 100644 --- a/Examples/Detectors/TelescopeDetector/src/TelescopeDetectorElement.cpp +++ b/Examples/Detectors/TelescopeDetector/src/TelescopeDetectorElement.cpp @@ -18,27 +18,36 @@ TelescopeDetectorElement::TelescopeDetectorElement( std::shared_ptr pBounds, double thickness, std::shared_ptr material) : m_elementTransform(std::move(transform)), - m_elementSurface( - Acts::Surface::makeShared(pBounds, *this)), m_elementThickness(thickness), m_elementPlanarBounds(std::move(pBounds)), - m_elementDiscBounds(nullptr) { - m_elementSurface->assignSurfaceMaterial(std::move(material)); - m_elementSurface->assignThickness(thickness); -} + m_elementDiscBounds(nullptr), + m_elementMaterial(std::move(material)) {} TelescopeDetectorElement::TelescopeDetectorElement( std::shared_ptr transform, std::shared_ptr dBounds, double thickness, std::shared_ptr material) : m_elementTransform(std::move(transform)), - m_elementSurface( - Acts::Surface::makeShared(dBounds, *this)), m_elementThickness(thickness), m_elementPlanarBounds(nullptr), - m_elementDiscBounds(std::move(dBounds)) { - m_elementSurface->assignSurfaceMaterial(std::move(material)); - m_elementSurface->assignThickness(thickness); + m_elementDiscBounds(std::move(dBounds)), + m_elementMaterial(std::move(material)) {} + +std::shared_ptr TelescopeDetectorElement::createSurface() { + std::shared_ptr surf; + if (m_elementPlanarBounds) { + surf = Acts::Surface::makeShared(*m_elementTransform, + m_elementPlanarBounds); + } else { + surf = Acts::Surface::makeShared(*m_elementTransform, + m_elementDiscBounds); + } + assignSurface(surf); + surf->assignThickness(m_elementThickness); + if (m_elementMaterial) { + surf->assignSurfaceMaterial(m_elementMaterial); + } + return surf; } } // namespace ActsExamples diff --git a/Examples/Io/EDM4hep/src/EDM4hepMeasurementInputConverter.cpp b/Examples/Io/EDM4hep/src/EDM4hepMeasurementInputConverter.cpp index 8dae5acbdc4..54886259e06 100644 --- a/Examples/Io/EDM4hep/src/EDM4hepMeasurementInputConverter.cpp +++ b/Examples/Io/EDM4hep/src/EDM4hepMeasurementInputConverter.cpp @@ -52,7 +52,7 @@ EDM4hepMeasurementInputConverter::EDM4hepMeasurementInputConverter( "DD4hepDetectorElementExtension for cellId " + std::to_string(cellId)); } - return ext->detectorElement().surface().geometryId(); + return ext->detectorElement().surface()->geometryId(); }; } diff --git a/Plugins/DD4hep/src/DD4hepDetectorSurfaceFactory.cpp b/Plugins/DD4hep/src/DD4hepDetectorSurfaceFactory.cpp index c7ed3b5942d..ba6e97d612e 100644 --- a/Plugins/DD4hep/src/DD4hepDetectorSurfaceFactory.cpp +++ b/Plugins/DD4hep/src/DD4hepDetectorSurfaceFactory.cpp @@ -102,7 +102,7 @@ DD4hepDetectorSurfaceFactory::constructSensitiveComponents( // Create the corresponding detector element auto dd4hepDetElement = m_config.detectorElementFactory( dd4hepElement, detAxis, unitLength, nullptr); - auto sSurface = dd4hepDetElement->surface().getSharedPtr(); + auto sSurface = dd4hepDetElement->createSurface(); // Measure if configured to do so if (cache.sExtent.has_value()) { auto sExtent = diff --git a/Plugins/DD4hep/src/DD4hepLayerBuilder.cpp b/Plugins/DD4hep/src/DD4hepLayerBuilder.cpp index 0fd7534ddd7..970c293912a 100644 --- a/Plugins/DD4hep/src/DD4hepLayerBuilder.cpp +++ b/Plugins/DD4hep/src/DD4hepLayerBuilder.cpp @@ -406,7 +406,7 @@ std::shared_ptr DD4hepLayerBuilder::createSensitiveSurface( DD4hepDetectorElementExtension(dd4hepDetElement))); // return the surface - return dd4hepDetElement->surface().getSharedPtr(); + return dd4hepDetElement->createSurface(); } Transform3 DD4hepLayerBuilder::convertTransform( diff --git a/Plugins/Geant4/include/ActsPlugins/Geant4/Geant4DetectorElement.hpp b/Plugins/Geant4/include/ActsPlugins/Geant4/Geant4DetectorElement.hpp index e02727d850c..627ff92a289 100644 --- a/Plugins/Geant4/include/ActsPlugins/Geant4/Geant4DetectorElement.hpp +++ b/Plugins/Geant4/include/ActsPlugins/Geant4/Geant4DetectorElement.hpp @@ -13,8 +13,6 @@ #include "Acts/Surfaces/Surface.hpp" #include "Acts/Surfaces/SurfacePlacementBase.hpp" -#include - class G4VPhysicalVolume; namespace Acts { @@ -42,8 +40,7 @@ class Geant4DetectorElement : public Acts::SurfacePlacementBase { /// @param g4physVol the physical volume representing this detector element /// @param toGlobal the global transformation before the volume /// @param thickness the thickness of this detector element - Geant4DetectorElement(std::shared_ptr surface, - const G4VPhysicalVolume& g4physVol, + Geant4DetectorElement(const G4VPhysicalVolume& g4physVol, const Acts::Transform3& toGlobal, double thickness); /// Return local to global transform associated with this detector element @@ -55,11 +52,11 @@ class Geant4DetectorElement : public Acts::SurfacePlacementBase { /// Return surface associated with this detector element /// @return Const reference to the associated surface - const Acts::Surface& surface() const override; + const Acts::Surface& surface() const; /// Non-const access to surface associated with this detector element /// @return Mutable reference to the associated surface - Acts::Surface& surface() override; + Acts::Surface& surface(); /// Return the thickness of this detector element /// @return The thickness value in length units @@ -72,8 +69,6 @@ class Geant4DetectorElement : public Acts::SurfacePlacementBase { bool isSensitive() const final { return true; } private: - /// Corresponding Surface - std::shared_ptr m_surface; /// The GEant4 physical volume const G4VPhysicalVolume* m_g4physVol{nullptr}; /// The global transformation before the volume diff --git a/Plugins/Geant4/include/ActsPlugins/Geant4/Geant4DetectorSurfaceFactory.hpp b/Plugins/Geant4/include/ActsPlugins/Geant4/Geant4DetectorSurfaceFactory.hpp index 03782656b4c..742cc04c69d 100644 --- a/Plugins/Geant4/include/ActsPlugins/Geant4/Geant4DetectorSurfaceFactory.hpp +++ b/Plugins/Geant4/include/ActsPlugins/Geant4/Geant4DetectorSurfaceFactory.hpp @@ -51,8 +51,13 @@ class Geant4DetectorSurfaceFactory { [](std::shared_ptr surface, const G4VPhysicalVolume& g4physVol, const Acts::Transform3& toGlobal, double thickness) { - return std::make_shared( - std::move(surface), g4physVol, toGlobal, thickness); + auto el = std::make_shared(g4physVol, toGlobal, + thickness); + if (thickness > 0.) { + surface->assignThickness(thickness); + } + el->assignSurface(std::move(surface)); + return el; }; /// @endcond }; diff --git a/Plugins/Geant4/src/Geant4DetectorElement.cpp b/Plugins/Geant4/src/Geant4DetectorElement.cpp index ad2fc4d1e71..17fa218c828 100644 --- a/Plugins/Geant4/src/Geant4DetectorElement.cpp +++ b/Plugins/Geant4/src/Geant4DetectorElement.cpp @@ -10,34 +10,15 @@ #include "Acts/Surfaces/Surface.hpp" -#include using namespace Acts; namespace ActsPlugins { -Geant4DetectorElement::Geant4DetectorElement(std::shared_ptr surface, - const G4VPhysicalVolume& g4physVol, +Geant4DetectorElement::Geant4DetectorElement(const G4VPhysicalVolume& g4physVol, const Transform3& toGlobal, double thickness) - : m_surface(std::move(surface)), - m_g4physVol(&g4physVol), - m_toGlobal(toGlobal), - m_thickness(thickness) { - if (m_surface == nullptr) { - throw std::invalid_argument( - "Geant4DetectorElement: Surface cannot be nullptr"); - } - if (m_surface->isSensitive()) { - throw std::logic_error( - "Geant4DetectorElement: Surface already has an associated detector " - "element"); - } - if (thickness > 0.) { - m_surface->assignThickness(m_thickness); - } - m_surface->assignSurfacePlacement(*this); -} + : m_g4physVol(&g4physVol), m_toGlobal(toGlobal), m_thickness(thickness) {} const Transform3& Geant4DetectorElement::localToGlobalTransform( const GeometryContext& /*gctx*/) const { @@ -45,11 +26,11 @@ const Transform3& Geant4DetectorElement::localToGlobalTransform( } const Surface& Geant4DetectorElement::surface() const { - return *m_surface; + return *SurfacePlacementBase::surface(); } Surface& Geant4DetectorElement::surface() { - return *m_surface; + return const_cast(*SurfacePlacementBase::surface()); } double Geant4DetectorElement::thickness() const { diff --git a/Plugins/GeoModel/include/ActsPlugins/GeoModel/GeoModelDetectorElement.hpp b/Plugins/GeoModel/include/ActsPlugins/GeoModel/GeoModelDetectorElement.hpp index 8ef1d071061..4e7ff2b293d 100644 --- a/Plugins/GeoModel/include/ActsPlugins/GeoModel/GeoModelDetectorElement.hpp +++ b/Plugins/GeoModel/include/ActsPlugins/GeoModel/GeoModelDetectorElement.hpp @@ -43,27 +43,20 @@ class GeoModelDetectorElement : public Acts::SurfacePlacementBase { // Deleted default constructor GeoModelDetectorElement() = delete; - /// @brief Factory to create a planar detector element with connected surface - /// - /// @tparam SurfaceType the surface type - /// @tparam BoundsType the bounds type + /// @brief Factory to create a detector element wired to an existing surface /// /// @param geoPhysVol representing the physical volume - /// @param bounds the bounds class /// @param sfTransform the surface transform /// @param thickness the thickness of the detector element + /// @param surface the surface to associate with this element /// /// @return a shared pointer to an instance of the detector element - template static std::shared_ptr createDetectorElement( - const PVConstLink& geoPhysVol, - const std::shared_ptr& bounds, - const Acts::Transform3& sfTransform, double thickness) { - // First create the detector element with a nullptr + const PVConstLink& geoPhysVol, const Acts::Transform3& sfTransform, + double thickness, const std::shared_ptr& surface) { auto detElement = std::make_shared( - geoPhysVol, nullptr, sfTransform, thickness); - auto surface = Acts::Surface::makeShared(bounds, *detElement); - detElement->attachSurface(surface); + geoPhysVol, sfTransform, thickness); + detElement->assignSurface(surface); return detElement; } @@ -74,7 +67,6 @@ class GeoModelDetectorElement : public Acts::SurfacePlacementBase { /// @param sfTransform the surface transform /// @param thickness the thickness of the detector element GeoModelDetectorElement(PVConstLink geoPhysVol, - std::shared_ptr surface, const Acts::Transform3& sfTransform, double thickness); @@ -89,14 +81,6 @@ class GeoModelDetectorElement : public Acts::SurfacePlacementBase { /// @return The nominal transform const Acts::Transform3& nominalTransform() const; - /// Return surface associated with this detector element - /// @return The surface - const Acts::Surface& surface() const override; - - /// Non-const access to surface associated with this detector element - /// @return The surface - Acts::Surface& surface() override; - /// Return the thickness of this detector element /// @return The thickness double thickness() const; @@ -122,22 +106,10 @@ class GeoModelDetectorElement : public Acts::SurfacePlacementBase { bool isSensitive() const final { return true; } protected: - /// Attach a surface - /// - /// @param surface The surface to attach - void attachSurface(std::shared_ptr surface) { - m_surface = std::move(surface); - assert(m_surface != nullptr); - m_surface->assignThickness(thickness()); - m_surface->assignSurfacePlacement(*this); - } - private: std::string m_entryName; /// The GeoModel full physical volume PVConstLink m_geoPhysVol{nullptr}; - /// The surface - std::shared_ptr m_surface; /// The global transformation before the volume Acts::Transform3 m_surfaceTransform{Acts::Transform3::Identity()}; /// Thickness of this detector element diff --git a/Plugins/GeoModel/include/ActsPlugins/GeoModel/GeoModelDetectorElementITk.hpp b/Plugins/GeoModel/include/ActsPlugins/GeoModel/GeoModelDetectorElementITk.hpp index 6e3782d4a15..1d9429946b8 100644 --- a/Plugins/GeoModel/include/ActsPlugins/GeoModel/GeoModelDetectorElementITk.hpp +++ b/Plugins/GeoModel/include/ActsPlugins/GeoModel/GeoModelDetectorElementITk.hpp @@ -83,13 +83,11 @@ class GeoModelDetectorElementITk : public GeoModelDetectorElement { /// @param phiModule Phi module identifier /// @param side Side identifier GeoModelDetectorElementITk(const PVConstLink& geoPhysVol, - std::shared_ptr surface, const Acts::Transform3& sfTransform, double thickness, int hardware, int barrelEndcap, int layerWheel, int etaModule, int phiModule, int side) - : GeoModelDetectorElement(geoPhysVol, std::move(surface), sfTransform, - thickness), + : GeoModelDetectorElement(geoPhysVol, sfTransform, thickness), m_identifier(hardware, barrelEndcap, layerWheel, etaModule, phiModule, side) {} diff --git a/Plugins/GeoModel/src/GeoModelDetectorElement.cpp b/Plugins/GeoModel/src/GeoModelDetectorElement.cpp index 5e875d869d3..0b09eba85ef 100644 --- a/Plugins/GeoModel/src/GeoModelDetectorElement.cpp +++ b/Plugins/GeoModel/src/GeoModelDetectorElement.cpp @@ -17,16 +17,10 @@ using namespace Acts; ActsPlugins::GeoModelDetectorElement::GeoModelDetectorElement( - PVConstLink geoPhysVol, std::shared_ptr surface, - const Transform3& sfTransform, double thickness) + PVConstLink geoPhysVol, const Transform3& sfTransform, double thickness) : m_geoPhysVol(std::move(geoPhysVol)), - m_surface(std::move(surface)), m_surfaceTransform(sfTransform), - m_thickness(thickness) { - if (m_surface) { - attachSurface(std::move(m_surface)); - } -} + m_thickness(thickness) {} const Transform3& ActsPlugins::GeoModelDetectorElement::localToGlobalTransform( const GeometryContext& /*gctx*/) const { @@ -38,14 +32,6 @@ const Transform3& ActsPlugins::GeoModelDetectorElement::nominalTransform() return m_surfaceTransform; } -const Surface& ActsPlugins::GeoModelDetectorElement::surface() const { - return *m_surface; -} - -Surface& ActsPlugins::GeoModelDetectorElement::surface() { - return *m_surface; -} - double ActsPlugins::GeoModelDetectorElement::thickness() const { return m_thickness; } diff --git a/Plugins/GeoModel/src/GeoModelDetectorElementITk.cpp b/Plugins/GeoModel/src/GeoModelDetectorElementITk.cpp index 1f9a3119af6..570b9593e2b 100644 --- a/Plugins/GeoModel/src/GeoModelDetectorElementITk.cpp +++ b/Plugins/GeoModel/src/GeoModelDetectorElementITk.cpp @@ -98,12 +98,12 @@ GeoModelDetectorElementITk::convertFromGeomodel( dynamic_cast(srf->bounds())); auto itkEl = std::make_shared( - detEl->physicalVolume(), nullptr, detEl->localToGlobalTransform(gctx), + detEl->physicalVolume(), detEl->localToGlobalTransform(gctx), detEl->thickness(), hardware, barrelEndcap, layerWheel, etaModule, phiModule, side); - auto surface = Surface::makeShared(bounds, *itkEl); - - itkEl->attachSurface(surface); + auto surface = Surface::makeShared( + detEl->localToGlobalTransform(gctx), bounds); + itkEl->assignSurface(surface); itkEl->setDatabaseEntryName(detEl->databaseEntryName()); return std::pair{itkEl, surface}; }; diff --git a/Plugins/GeoModel/src/detail/GeoBoxConverter.cpp b/Plugins/GeoModel/src/detail/GeoBoxConverter.cpp index 746c9fee2d7..deb3b1d0da8 100644 --- a/Plugins/GeoModel/src/detail/GeoBoxConverter.cpp +++ b/Plugins/GeoModel/src/detail/GeoBoxConverter.cpp @@ -63,12 +63,10 @@ ActsPlugins::detail::GeoBoxConverter::operator()( Surface::makeShared(transform, rectangleBounds); return std::make_tuple(nullptr, surface); } - // Create the element and the surface - auto detectorElement = - GeoModelDetectorElement::createDetectorElement( - geoPV, rectangleBounds, transform, - 2 * unitLength * halfLengths[zIndex]); - auto surface = detectorElement->surface().getSharedPtr(); + // Create the surface and element + auto surface = Surface::makeShared(transform, rectangleBounds); + auto detectorElement = GeoModelDetectorElement::createDetectorElement( + geoPV, transform, 2 * unitLength * halfLengths[zIndex], surface); // Return the detector element and surface return std::make_tuple(detectorElement, surface); } diff --git a/Plugins/GeoModel/src/detail/GeoIntersectionAnnulusConverter.cpp b/Plugins/GeoModel/src/detail/GeoIntersectionAnnulusConverter.cpp index 205505e9331..ac217c17e56 100644 --- a/Plugins/GeoModel/src/detail/GeoIntersectionAnnulusConverter.cpp +++ b/Plugins/GeoModel/src/detail/GeoIntersectionAnnulusConverter.cpp @@ -87,11 +87,13 @@ ActsPlugins::detail::GeoIntersectionAnnulusConverter::operator()( } // Create the detector element - auto detectorElement = - GeoModelDetectorElement::createDetectorElement( - geoPV, boundFactory.insert(annulusBounds), annulusTransform, - thickness); - auto surface = detectorElement->surface().getSharedPtr(); + auto surface = Surface::makeShared( + annulusTransform, boundFactory.insert(annulusBounds)); + surface->assignThickness(thickness); + + auto detectorElement = std::make_shared( + geoPV, annulusTransform, thickness); + surface->assignSurfacePlacement(detectorElement); return std::make_tuple(detectorElement, surface); } return std::make_tuple(nullptr, nullptr); diff --git a/Plugins/GeoModel/src/detail/GeoPolygonConverter.cpp b/Plugins/GeoModel/src/detail/GeoPolygonConverter.cpp index 371cef7a030..5c469811b1a 100644 --- a/Plugins/GeoModel/src/detail/GeoPolygonConverter.cpp +++ b/Plugins/GeoModel/src/detail/GeoPolygonConverter.cpp @@ -73,11 +73,10 @@ ActsPlugins::detail::GeoPolygonConverter::operator()( auto surface = Surface::makeShared(transform, trapBounds); return std::make_tuple(nullptr, surface); } - // Create the element and the surface - auto detectorElement = - GeoModelDetectorElement::createDetectorElement( - geoPV, trapBounds, transform, unitLength * polygon.getDZ()); - auto surface = detectorElement->surface().getSharedPtr(); + // Create the surface and element + auto surface = Surface::makeShared(transform, trapBounds); + auto detectorElement = GeoModelDetectorElement::createDetectorElement( + geoPV, transform, unitLength * polygon.getDZ(), surface); // Return the detector element and surface return std::make_tuple(detectorElement, surface); } else if (nVertices == 6) { @@ -111,11 +110,10 @@ ActsPlugins::detail::GeoPolygonConverter::operator()( Surface::makeShared(transform, diamondBounds); return std::make_tuple(nullptr, surface); } - // Create the element and the surface - auto detectorElement = - GeoModelDetectorElement::createDetectorElement( - geoPV, diamondBounds, transform, unitLength * polygon.getDZ()); - auto surface = detectorElement->surface().getSharedPtr(); + // Create the surface and element + auto surface = Surface::makeShared(transform, diamondBounds); + auto detectorElement = GeoModelDetectorElement::createDetectorElement( + geoPV, transform, unitLength * polygon.getDZ(), surface); // Return the detector element and surface return std::make_tuple(detectorElement, surface); } else { diff --git a/Plugins/GeoModel/src/detail/GeoTrdConverter.cpp b/Plugins/GeoModel/src/detail/GeoTrdConverter.cpp index d1ba453d965..93a43d2a254 100644 --- a/Plugins/GeoModel/src/detail/GeoTrdConverter.cpp +++ b/Plugins/GeoModel/src/detail/GeoTrdConverter.cpp @@ -93,11 +93,9 @@ ActsPlugins::detail::GeoTrdConverter::operator()( return std::make_tuple(nullptr, surface); } - // Create the element and the surface - - auto detectorElement = - GeoModelDetectorElement::createDetectorElement( - geoPV, trapezoidBounds, transform, thickness); - auto surface = detectorElement->surface().getSharedPtr(); + // Create the surface and element + auto surface = Surface::makeShared(transform, trapezoidBounds); + auto detectorElement = GeoModelDetectorElement::createDetectorElement( + geoPV, transform, thickness, surface); return std::make_tuple(detectorElement, surface); } diff --git a/Plugins/GeoModel/src/detail/GeoTubeConverter.cpp b/Plugins/GeoModel/src/detail/GeoTubeConverter.cpp index bea4b3f01b3..6c6a00fe0b4 100644 --- a/Plugins/GeoModel/src/detail/GeoTubeConverter.cpp +++ b/Plugins/GeoModel/src/detail/GeoTubeConverter.cpp @@ -53,11 +53,10 @@ ActsPlugins::detail::GeoTubeConverter::operator()( return std::make_tuple(nullptr, surface); } - auto detectorElement = - GeoModelDetectorElement::createDetectorElement( - geoPV, lineBounds, transform, 2 * outerRadius); - auto surface = detectorElement->surface().getSharedPtr(); - return std::make_tuple(detectorElement, surface); + auto surf0 = Surface::makeShared(transform, lineBounds); + auto detEl0 = GeoModelDetectorElement::createDetectorElement( + geoPV, transform, 2 * outerRadius, surf0); + return std::make_tuple(detEl0, surf0); // Next option is translation to disc } else if (targetShape == Surface::SurfaceType::Disc) { auto radialBounds = @@ -68,12 +67,10 @@ ActsPlugins::detail::GeoTubeConverter::operator()( } // Create the element and the surface - auto detectorElement = - GeoModelDetectorElement::createDetectorElement( - geoPV, radialBounds, transform, 2 * halfZ); - auto surface = detectorElement->surface().getSharedPtr(); - return std::make_tuple(detectorElement, surface); + auto surf1 = Surface::makeShared(transform, radialBounds); + auto detEl1 = GeoModelDetectorElement::createDetectorElement( + geoPV, transform, 2 * halfZ, surf1); + return std::make_tuple(detEl1, surf1); } // Finally cylinder to cylinder auto cylinderBounds = std::make_shared(outerRadius, halfZ); @@ -83,11 +80,8 @@ ActsPlugins::detail::GeoTubeConverter::operator()( return std::make_tuple(nullptr, surface); } // Create the element and the surface - - auto detectorElement = - GeoModelDetectorElement::createDetectorElement( - geoPV, cylinderBounds, transform, outerRadius - innerRadius); - auto surface = detectorElement->surface().getSharedPtr(); - return std::make_tuple(detectorElement, surface); + auto surf2 = Surface::makeShared(transform, cylinderBounds); + auto detEl2 = GeoModelDetectorElement::createDetectorElement( + geoPV, transform, outerRadius - innerRadius, surf2); + return std::make_tuple(detEl2, surf2); } diff --git a/Plugins/GeoModel/src/detail/GeoUnionDoubleTrdConverter.cpp b/Plugins/GeoModel/src/detail/GeoUnionDoubleTrdConverter.cpp index 2bdadc24d98..e334ecba99e 100644 --- a/Plugins/GeoModel/src/detail/GeoUnionDoubleTrdConverter.cpp +++ b/Plugins/GeoModel/src/detail/GeoUnionDoubleTrdConverter.cpp @@ -160,12 +160,10 @@ Result GeoUnionDoubleTrdConverter::operator()( return std::make_tuple(nullptr, surface); } - // Create the element and the surface (we assume both have equal thickness) - auto detectorElement = - GeoModelDetectorElement::createDetectorElement( - geoPV, trapezoidBounds, transform, elA->thickness()); - auto surface = detectorElement->surface().getSharedPtr(); - + // Create the surface and element (we assume both have equal thickness) + auto surface = Surface::makeShared(transform, trapezoidBounds); + auto detectorElement = GeoModelDetectorElement::createDetectorElement( + geoPV, transform, elA->thickness(), surface); return std::make_tuple(detectorElement, surface); } diff --git a/Plugins/Json/include/ActsPlugins/Json/JsonDetectorElement.hpp b/Plugins/Json/include/ActsPlugins/Json/JsonDetectorElement.hpp index 9bf40fa2fc8..946cde43087 100644 --- a/Plugins/Json/include/ActsPlugins/Json/JsonDetectorElement.hpp +++ b/Plugins/Json/include/ActsPlugins/Json/JsonDetectorElement.hpp @@ -29,13 +29,6 @@ class JsonDetectorElement : public SurfacePlacementBase { /// @param thickness Thickness of the detector element JsonDetectorElement(const nlohmann::json &jSurface, double thickness); - /// Return mutable reference to the surface - /// @return Mutable reference to the associated surface - Surface &surface() override; - /// Return const reference to the surface - /// @return Const reference to the associated surface - const Surface &surface() const override; - /// Return the thickness of the detector element /// @return Thickness value double thickness() const; @@ -48,8 +41,10 @@ class JsonDetectorElement : public SurfacePlacementBase { bool isSensitive() const override { return true; } + std::shared_ptr createSurface(); + private: - std::shared_ptr m_surface; + const nlohmann::json m_jSurface; Transform3 m_transform{}; double m_thickness{}; }; diff --git a/Plugins/Json/src/JsonDetectorElement.cpp b/Plugins/Json/src/JsonDetectorElement.cpp index 7bbd5c85b8c..e27bb78f7ae 100644 --- a/Plugins/Json/src/JsonDetectorElement.cpp +++ b/Plugins/Json/src/JsonDetectorElement.cpp @@ -15,20 +15,7 @@ namespace Acts { JsonDetectorElement::JsonDetectorElement(const nlohmann::json &jSurface, double thickness) - : m_thickness(thickness) { - m_surface = Acts::SurfaceJsonConverter::fromJson(jSurface); - m_transform = Transform3JsonConverter::fromJson(jSurface["transform"]); - m_surface->assignSurfacePlacement(*this); - m_surface->assignThickness(thickness); -} - -const Surface &JsonDetectorElement::surface() const { - return *m_surface; -} - -Surface &JsonDetectorElement::surface() { - return *m_surface; -} + : m_jSurface(jSurface), m_thickness(thickness) {} const Transform3 &JsonDetectorElement::localToGlobalTransform( const GeometryContext & /*gctx*/) const { @@ -39,4 +26,14 @@ double JsonDetectorElement::thickness() const { return m_thickness; } +std::shared_ptr JsonDetectorElement::createSurface() { + auto surface = Acts::SurfaceJsonConverter::fromJson(m_jSurface); + m_transform = Transform3JsonConverter::fromJson(m_jSurface["transform"]); + + surface->assignSurfacePlacement(shared_from_this()); + surface->assignThickness(m_thickness); + + return surface; +} + } // namespace Acts diff --git a/Plugins/Root/include/ActsPlugins/Root/ITGeoDetectorElementSplitter.hpp b/Plugins/Root/include/ActsPlugins/Root/ITGeoDetectorElementSplitter.hpp index 839e3277892..34cde14a83b 100644 --- a/Plugins/Root/include/ActsPlugins/Root/ITGeoDetectorElementSplitter.hpp +++ b/Plugins/Root/include/ActsPlugins/Root/ITGeoDetectorElementSplitter.hpp @@ -37,9 +37,9 @@ class ITGeoDetectorElementSplitter { /// @note If no split is performed the unsplit detector element is returned /// /// @return a vector of TGeoDetectorElement objects - virtual std::vector> + virtual std::vector> split(const Acts::GeometryContext& gctx, - std::shared_ptr tgde) const = 0; + std::shared_ptr tgde) const = 0; }; /// @} diff --git a/Plugins/Root/include/ActsPlugins/Root/TGeoCylinderDiscSplitter.hpp b/Plugins/Root/include/ActsPlugins/Root/TGeoCylinderDiscSplitter.hpp index e9e2fc718a7..d9b0599c6c7 100644 --- a/Plugins/Root/include/ActsPlugins/Root/TGeoCylinderDiscSplitter.hpp +++ b/Plugins/Root/include/ActsPlugins/Root/TGeoCylinderDiscSplitter.hpp @@ -60,9 +60,9 @@ class TGeoCylinderDiscSplitter : public ITGeoDetectorElementSplitter { /// @note If no split is performed the unsplit detector element is returned /// /// @return a vector of TGeoDetectorElement objects - std::vector> split( + std::vector> split( const Acts::GeometryContext& gctx, - std::shared_ptr tgde) const override; + std::shared_ptr tgde) const override; private: Config m_cfg; diff --git a/Plugins/Root/include/ActsPlugins/Root/TGeoDetectorElement.hpp b/Plugins/Root/include/ActsPlugins/Root/TGeoDetectorElement.hpp index c3c48c836d3..bd7f5c07ede 100644 --- a/Plugins/Root/include/ActsPlugins/Root/TGeoDetectorElement.hpp +++ b/Plugins/Root/include/ActsPlugins/Root/TGeoDetectorElement.hpp @@ -133,15 +133,14 @@ class TGeoDetectorElement : public Acts::SurfacePlacementBase { /// @return Reference to the nominal transformation matrix const Acts::Transform3& nominalTransform() const; - /// Return surface associated with this detector element - /// @return Const reference to the surface - const Acts::Surface& surface() const override; - - /// Return surface associated with this detector element + /// Create and return the surface associated with this detector element. + /// + /// This method uses @c shared_from_this() so it must only be called after + /// the element is already managed by a @c std::shared_ptr. The returned + /// surface takes shared ownership of this detector element. /// - /// @note this is the non-const access - /// @return Mutable reference to the surface - Acts::Surface& surface() override; + /// @return Shared pointer to the created surface + std::shared_ptr createSurface(); /// Returns the thickness of the module /// @return Thickness of the detector element in units of length @@ -165,8 +164,8 @@ class TGeoDetectorElement : public Acts::SurfacePlacementBase { std::shared_ptr m_bounds{nullptr}; /// Thickness of this detector element double m_thickness{0.}; - /// Corresponding Surface - std::shared_ptr m_surface{nullptr}; + /// Material to be applied when createSurface() is called + std::shared_ptr m_deferredMaterial{nullptr}; }; inline TGeoDetectorElement::Identifier TGeoDetectorElement::identifier() const { @@ -178,14 +177,6 @@ inline const Acts::Transform3& TGeoDetectorElement::localToGlobalTransform( return m_transform; } -inline const Acts::Surface& TGeoDetectorElement::surface() const { - return (*m_surface); -} - -inline Acts::Surface& TGeoDetectorElement::surface() { - return (*m_surface); -} - inline double TGeoDetectorElement::thickness() const { return m_thickness; } diff --git a/Plugins/Root/src/TGeoCylinderDiscSplitter.cpp b/Plugins/Root/src/TGeoCylinderDiscSplitter.cpp index d9a8808da68..e7e2c18d1df 100644 --- a/Plugins/Root/src/TGeoCylinderDiscSplitter.cpp +++ b/Plugins/Root/src/TGeoCylinderDiscSplitter.cpp @@ -15,6 +15,7 @@ #include "Acts/Surfaces/Surface.hpp" #include "Acts/Surfaces/SurfaceBounds.hpp" #include "Acts/Surfaces/TrapezoidBounds.hpp" +#include "Acts/Utilities/Diagnostics.hpp" #include "ActsPlugins/Root/TGeoDetectorElement.hpp" #include @@ -29,11 +30,15 @@ ActsPlugins::TGeoCylinderDiscSplitter::TGeoCylinderDiscSplitter( std::unique_ptr logger) : m_cfg(cfg), m_logger(std::move(logger)) {} -std::vector> +std::vector> ActsPlugins::TGeoCylinderDiscSplitter::split( const GeometryContext& gctx, - std::shared_ptr tgde) const { - const Surface& sf = tgde->surface(); + std::shared_ptr tgde) const { + // tgde->createSurface() was called by the caller before invoking split(); + // the weak_ptr is valid so surface() returns the existing surface. + ACTS_PUSH_IGNORE_DEPRECATED() + const Surface& sf = *tgde->surface(); + ACTS_POP_IGNORE_DEPRECATED() // Thickness auto tgIdentifier = tgde->identifier(); const TGeoNode& tgNode = tgde->tgeoNode(); @@ -46,7 +51,7 @@ ActsPlugins::TGeoCylinderDiscSplitter::split( sf.bounds().type() == SurfaceBounds::eDisc) { ACTS_DEBUG("- splitting detected for a Disc shaped sensor."); - std::vector> + std::vector> tgDetectorElements = {}; tgDetectorElements.reserve(std::abs(m_cfg.discPhiSegments) * std::abs(m_cfg.discRadialSegments)); @@ -114,7 +119,7 @@ ActsPlugins::TGeoCylinderDiscSplitter::split( sf.bounds().type() == SurfaceBounds::eCylinder) { ACTS_DEBUG("- splitting detected for a Cylinder shaped sensor."); - std::vector> + std::vector> tgDetectorElements = {}; tgDetectorElements.reserve(std::abs(m_cfg.cylinderPhiSegments) * std::abs(m_cfg.cylinderLongitudinalSegments)); diff --git a/Plugins/Root/src/TGeoDetectorElement.cpp b/Plugins/Root/src/TGeoDetectorElement.cpp index 3c020a53d59..30d8914abc9 100644 --- a/Plugins/Root/src/TGeoDetectorElement.cpp +++ b/Plugins/Root/src/TGeoDetectorElement.cpp @@ -9,13 +9,16 @@ #include "ActsPlugins/Root/TGeoDetectorElement.hpp" #include "Acts/Definitions/Algebra.hpp" +#include "Acts/Surfaces/CylinderBounds.hpp" #include "Acts/Surfaces/CylinderSurface.hpp" +#include "Acts/Surfaces/DiscBounds.hpp" #include "Acts/Surfaces/DiscSurface.hpp" #include "Acts/Surfaces/PlanarBounds.hpp" #include "Acts/Surfaces/PlaneSurface.hpp" #include "Acts/Surfaces/Surface.hpp" #include "ActsPlugins/Root/TGeoSurfaceConverter.hpp" +#include #include #include "RtypesCore.h" @@ -31,9 +34,9 @@ TGeoDetectorElement::TGeoDetectorElement( const Identifier& identifier, const TGeoNode& tGeoNode, const TGeoMatrix& tGeoMatrix, TGeoAxes axes, double scalor, std::shared_ptr material) - : m_detElement(&tGeoNode), m_identifier(identifier) { - // Create temporary local non const surface (to allow setting the - // material) + : m_detElement(&tGeoNode), + m_identifier(identifier), + m_deferredMaterial(std::move(material)) { const Double_t* translation = tGeoMatrix.GetTranslation(); const Double_t* rotation = tGeoMatrix.GetRotationMatrix(); @@ -47,39 +50,25 @@ TGeoDetectorElement::TGeoDetectorElement( m_transform = cTransform; m_bounds = cBounds; m_thickness = cThickness; - m_surface = Surface::makeShared(cBounds, *this); - } - - // Check next if you do not have a surface - if (m_surface == nullptr) { - auto [dBounds, dTransform, dThickness] = - TGeoSurfaceConverter::discComponents(*tgShape, rotation, translation, - axes, scalor); - if (dBounds != nullptr) { - m_bounds = dBounds; - m_transform = dTransform; - m_thickness = dThickness; - m_surface = Surface::makeShared(dBounds, *this); - } + return; } - // Check next if you do not have a surface - if (m_surface == nullptr) { - auto [pBounds, pTransform, pThickness] = - TGeoSurfaceConverter::planeComponents(*tgShape, rotation, translation, - axes, scalor); - if (pBounds != nullptr) { - m_bounds = pBounds; - m_transform = pTransform; - m_thickness = pThickness; - m_surface = Surface::makeShared(pBounds, *this); - } + auto [dBounds, dTransform, dThickness] = TGeoSurfaceConverter::discComponents( + *tgShape, rotation, translation, axes, scalor); + if (dBounds != nullptr) { + m_bounds = dBounds; + m_transform = dTransform; + m_thickness = dThickness; + return; } - // set the asscoiated material (non const method) - if (m_surface != nullptr) { - m_surface->assignSurfaceMaterial(std::move(material)); - m_surface->assignThickness(m_thickness); + auto [pBounds, pTransform, pThickness] = + TGeoSurfaceConverter::planeComponents(*tgShape, rotation, translation, + axes, scalor); + if (pBounds != nullptr) { + m_bounds = pBounds; + m_transform = pTransform; + m_thickness = pThickness; } } @@ -91,10 +80,7 @@ TGeoDetectorElement::TGeoDetectorElement( m_transform(tgTransform), m_identifier(identifier), m_bounds(tgBounds), - m_thickness(tgThickness) { - m_surface = Surface::makeShared(tgBounds, *this); - m_surface->assignThickness(m_thickness); -} + m_thickness(tgThickness) {} TGeoDetectorElement::TGeoDetectorElement( const Identifier& identifier, const TGeoNode& tGeoNode, @@ -104,13 +90,35 @@ TGeoDetectorElement::TGeoDetectorElement( m_transform(tgTransform), m_identifier(identifier), m_bounds(tgBounds), - m_thickness(tgThickness) { - m_surface = Surface::makeShared(tgBounds, *this); - m_surface->assignThickness(m_thickness); -} + m_thickness(tgThickness) {} TGeoDetectorElement::~TGeoDetectorElement() = default; +std::shared_ptr TGeoDetectorElement::createSurface() { + std::shared_ptr surf; + + if (auto cBounds = + std::dynamic_pointer_cast(m_bounds)) { + surf = Surface::makeShared(m_transform, cBounds); + } else if (auto dBounds = + std::dynamic_pointer_cast(m_bounds)) { + surf = Surface::makeShared(m_transform, dBounds); + } else if (auto pBounds = + std::dynamic_pointer_cast(m_bounds)) { + surf = Surface::makeShared(m_transform, pBounds); + } + + if (surf != nullptr) { + assignSurface(surf); + surf->assignThickness(m_thickness); + if (m_deferredMaterial) { + surf->assignSurfaceMaterial(m_deferredMaterial); + } + } + + return surf; +} + const Transform3& TGeoDetectorElement::nominalTransform() const { return m_transform; } diff --git a/Plugins/Root/src/TGeoLayerBuilder.cpp b/Plugins/Root/src/TGeoLayerBuilder.cpp index 2fef29c02a2..9d8f182b5e0 100644 --- a/Plugins/Root/src/TGeoLayerBuilder.cpp +++ b/Plugins/Root/src/TGeoLayerBuilder.cpp @@ -261,22 +261,33 @@ void ActsPlugins::TGeoLayerBuilder::buildLayers(const GeometryContext& gctx, auto tgElement = m_cfg.detectorElementFactory( identifier, *snode.node, *snode.transform, layerCfg.localAxes, m_cfg.unit, nullptr); + // createSurface() must be called before the splitter, which inspects + // the element's surface geometry. Keep the shared_ptr alive until + // after split() returns so the weak_ptr inside tgElement stays valid. + auto tgElementSurface = tgElement->createSurface(); - std::vector> tgElements = + std::vector> tgElements = (m_cfg.detectorElementSplitter == nullptr) - ? std::vector< - std::shared_ptr>{tgElement} + ? std::vector>{tgElement} : m_cfg.detectorElementSplitter->split(gctx, tgElement); for (const auto& tge : tgElements) { m_elementStore.push_back(tge); - layerSurfaces.push_back(tge->surface().getSharedPtr()); + // For the original (unsplit) element the surface was already created + // above; for split children createSurface() is called here. + auto surf = (tge == tgElement) + ? tgElementSurface + : tge->createSurface(); + layerSurfaces.push_back(std::move(surf)); } } ACTS_DEBUG("- created TGeoDetectorElements : " << layerSurfaces.size()); if (m_cfg.protoLayerHelper != nullptr && !layerCfg.splitConfigs.empty()) { + // ProtoLayer stores raw Surface* pointers; keep the shared_ptrs alive + // until we've finished iterating over the proto-layers below. + auto allSurfaces = layerSurfaces; auto protoLayers = m_cfg.protoLayerHelper->protoLayers( gctx, unpackSmartPointers(layerSurfaces), layerCfg.splitConfigs); ACTS_DEBUG("- splitting into " << protoLayers.size() << " layers."); diff --git a/Python/Core/src/Navigation.cpp b/Python/Core/src/Navigation.cpp index b81fad4d59c..7c351f39265 100644 --- a/Python/Core/src/Navigation.cpp +++ b/Python/Core/src/Navigation.cpp @@ -44,13 +44,13 @@ class DetectorElementStub : public SurfacePlacementBase { /// Is the detector element a sensitive element bool isSensitive() const override { return true; } - /// Return surface representation - const return pattern - const Surface& surface() const override { + /// Return surface representation - const return pattern (shadows base) + const Surface& surface() const { throw std::runtime_error("Not implemented"); } - /// Non-const return pattern - Surface& surface() override { throw std::runtime_error("Not implemented"); } + /// Non-const return pattern (shadows base) + Surface& surface() { throw std::runtime_error("Not implemented"); } private: Transform3 m_transform{Transform3::Identity()}; @@ -121,11 +121,11 @@ void addNavigation(py::module_& m) { std::make_shared(30, 40, 100)); vol1->setVolumeName("TestVolume"); - auto detElem = std::make_unique(); + auto detElem = std::make_shared(); auto surface = Surface::makeShared( Transform3::Identity(), std::make_shared(30, 40)); - surface->assignSurfacePlacement(*detElem); + surface->assignSurfacePlacement(std::move(detElem)); vol1->addSurface(std::move(surface)); diff --git a/Python/Core/src/Surfaces.cpp b/Python/Core/src/Surfaces.cpp index a3b31371687..1cb37659eaa 100644 --- a/Python/Core/src/Surfaces.cpp +++ b/Python/Core/src/Surfaces.cpp @@ -345,9 +345,11 @@ void addSurfaces(py::module_& m) { }) .def_static("createDisc", [](const std::shared_ptr& bounds, - const SurfacePlacementBase& detelement) { - return Surface::makeShared(bounds, - detelement); + std::shared_ptr detelement) { + auto surf = Surface::makeShared( + Transform3::Identity(), bounds); + surf->assignSurfacePlacement(std::move(detelement)); + return surf; }) .def_static("createStraw", [](const Transform3& transform, @@ -357,9 +359,11 @@ void addSurfaces(py::module_& m) { }) .def_static("createStraw", [](const std::shared_ptr& bounds, - const SurfacePlacementBase& detelement) { - return Surface::makeShared(bounds, - detelement); + std::shared_ptr detelement) { + auto surf = Surface::makeShared( + Transform3::Identity(), bounds); + surf->assignSurfacePlacement(std::move(detelement)); + return surf; }) .def_static("createPerigee", [](const Vector3& vertex) { @@ -373,9 +377,11 @@ void addSurfaces(py::module_& m) { }) .def_static("createPlane", [](const std::shared_ptr& pbounds, - const SurfacePlacementBase& detelement) { - return Surface::makeShared(pbounds, - detelement); + std::shared_ptr detelement) { + auto surf = Surface::makeShared( + Transform3::Identity(), pbounds); + surf->assignSurfacePlacement(std::move(detelement)); + return surf; }); py::class_>( diff --git a/Python/Examples/src/plugins/GeoModel.cpp b/Python/Examples/src/plugins/GeoModel.cpp index f40e6fe56dd..665b48325ae 100644 --- a/Python/Examples/src/plugins/GeoModel.cpp +++ b/Python/Examples/src/plugins/GeoModel.cpp @@ -112,41 +112,49 @@ PYBIND11_MODULE(ActsExamplesPythonBindingsGeoModel, gm) { gm.def( "splitBarrelModule", [](const GeometryContext& gctx, - std::shared_ptr detElement, + std::shared_ptr detElement, unsigned nSegments, Logging::Level logLevel) { auto logger = getDefaultLogger("ITkSlitBarrel", logLevel); auto name = detElement->databaseEntryName(); + std::vector> splitSurfaces; auto factory = [&](const auto& trafo, const auto& bounds) { - return GeoModelDetectorElement::createDetectorElement< - PlaneSurface, RectangleBounds>(detElement->physicalVolume(), - bounds, trafo, - detElement->thickness()); + auto surf = Acts::Surface::makeShared( + trafo, bounds); + splitSurfaces.push_back(surf); + return GeoModelDetectorElement::createDetectorElement( + detElement->physicalVolume(), trafo, detElement->thickness(), + surf); }; - return ITk::splitBarrelModule(gctx, detElement, nSegments, factory, - name, *logger); + auto elements = ITk::splitBarrelModule(gctx, detElement, nSegments, + factory, name, *logger); + return std::make_pair(elements, splitSurfaces); }, "gxtx"_a, "detElement"_a, "nSegments"_a, "logLevel"_a = Logging::INFO); gm.def( "splitDiscModule", [](const GeometryContext& gctx, - std::shared_ptr detElement, + std::shared_ptr detElement, const std::vector>& patterns, Logging::Level logLevel) { auto logger = getDefaultLogger("ITkSlitBarrel", logLevel); auto name = detElement->databaseEntryName(); + std::vector> splitSurfaces; auto factory = [&](const auto& trafo, const auto& bounds) { - return GeoModelDetectorElement::createDetectorElement< - DiscSurface, AnnulusBounds>(detElement->physicalVolume(), - bounds, trafo, - detElement->thickness()); + auto surf = Acts::Surface::makeShared( + trafo, bounds); + splitSurfaces.push_back(surf); + return GeoModelDetectorElement::createDetectorElement( + detElement->physicalVolume(), trafo, detElement->thickness(), + surf); }; - return ITk::splitDiscModule(gctx, detElement, patterns, factory, name, - *logger); + auto elements = ITk::splitDiscModule(gctx, detElement, patterns, + factory, name, *logger); + return std::make_pair(elements, splitSurfaces); }, "gxtx"_a, "detElement"_a, "splitRanges"_a, "logLevel"_a = Logging::INFO); diff --git a/Python/Plugins/src/GeoModel.cpp b/Python/Plugins/src/GeoModel.cpp index 28d79e38183..c4a7640a2c4 100644 --- a/Python/Plugins/src/GeoModel.cpp +++ b/Python/Plugins/src/GeoModel.cpp @@ -59,15 +59,15 @@ PYBIND11_MODULE(ActsPluginsPythonBindingsGeoModel, gm) { gm, "GeoModelDetectorElement") .def("logVolName", &GeoModelDetectorElement::logVolName) .def("databaseEntryName", &GeoModelDetectorElement::databaseEntryName) - .def("surface", [](GeoModelDetectorElement self) { - return self.surface().getSharedPtr(); + .def("surface", [](GeoModelDetectorElement& self) { + return self.surface()->getSharedPtr(); }); py::class_>( gm, "GeoModelDetectorElementITk") .def("surface", [](GeoModelDetectorElementITk& self) { - return self.surface().getSharedPtr(); + return self.surface()->getSharedPtr(); }); gm.def("convertToItk", &GeoModelDetectorElementITk::convertFromGeomodel); } diff --git a/Python/Plugins/src/Json.cpp b/Python/Plugins/src/Json.cpp index 9e40f3a82b5..bcddda90468 100644 --- a/Python/Plugins/src/Json.cpp +++ b/Python/Plugins/src/Json.cpp @@ -68,7 +68,7 @@ PYBIND11_MODULE(ActsPluginsPythonBindingsJson, json) { std::shared_ptr>(json, "JsonDetectorElement") .def("surface", [](JsonDetectorElement& self) { - return self.surface().getSharedPtr(); + return self.surface()->getSharedPtr(); }); json.def("readDetectorElementsFromJson", diff --git a/Python/Plugins/src/TGeo.cpp b/Python/Plugins/src/TGeo.cpp index acdd8df1463..cbc5c88292f 100644 --- a/Python/Plugins/src/TGeo.cpp +++ b/Python/Plugins/src/TGeo.cpp @@ -54,7 +54,7 @@ PYBIND11_MODULE(ActsPluginsPythonBindingsTGeo, tgeo) { py::class_>( tgeo, "TGeoDetectorElement") .def("surface", [](const TGeoDetectorElement& self) { - return self.surface().getSharedPtr(); + return self.surface()->getSharedPtr(); }); } diff --git a/Tests/CommonHelpers/include/ActsTests/CommonHelpers/DetectorElementStub.hpp b/Tests/CommonHelpers/include/ActsTests/CommonHelpers/DetectorElementStub.hpp index 0cf24c913de..298bea72cf2 100644 --- a/Tests/CommonHelpers/include/ActsTests/CommonHelpers/DetectorElementStub.hpp +++ b/Tests/CommonHelpers/include/ActsTests/CommonHelpers/DetectorElementStub.hpp @@ -85,11 +85,27 @@ class DetectorElementStub : public Acts::SurfacePlacementBase { const Acts::Transform3& localToGlobalTransform( const Acts::GeometryContext& gctx) const override; + /// Create and return the surface associated with this detector element. + /// + /// Must be called only after the element is managed by a @c std::shared_ptr. + /// The returned surface takes shared ownership of this detector element. + /// + /// @return Shared pointer to the created surface + std::shared_ptr createSurface(); + /// Return surface associated with this detector element - const Acts::Surface& surface() const override; + /// @deprecated Use @c createSurface() and hold the returned shared_ptr. + [[deprecated( + "Use createSurface() to get a Surface with shared ownership of the " + "placement; surface() will be removed in a future release")]] + const Acts::Surface& surface() const; /// Non-const access to surface associated with this detector element - Acts::Surface& surface() override; + /// @deprecated Use @c createSurface() and hold the returned shared_ptr. + [[deprecated( + "Use createSurface() to get a Surface with shared ownership of the " + "placement; surface() will be removed in a future release")]] + Acts::Surface& surface(); /// The maximal thickness of the detector element wrt normal axis double thickness() const; @@ -100,10 +116,20 @@ class DetectorElementStub : public Acts::SurfacePlacementBase { private: /// the transform for positioning in 3D space Acts::Transform3 m_elementTransform; - /// the surface represented by it - std::shared_ptr m_elementSurface{nullptr}; + /// weak reference back to the surface (owned externally via shared_ptr) + mutable std::weak_ptr m_elementSurface; + /// keeps the surface alive when constructed via the deprecated raw-ptr path + /// (i.e. not via createSurface() with shared ownership); null after + /// createSurface() is called. + mutable std::shared_ptr m_legacySurface; /// the element thickness double m_elementThickness{0.}; + /// bounds (one of these will be non-null) + std::shared_ptr m_cylinderBounds; + std::shared_ptr m_planarBounds; + std::shared_ptr m_lineBounds; + /// optional deferred material + std::shared_ptr m_deferredMaterial; }; inline const Acts::Transform3& DetectorElementStub::localToGlobalTransform( @@ -111,14 +137,6 @@ inline const Acts::Transform3& DetectorElementStub::localToGlobalTransform( return m_elementTransform; } -inline const Acts::Surface& DetectorElementStub::surface() const { - return *m_elementSurface; -} - -inline Acts::Surface& DetectorElementStub::surface() { - return *m_elementSurface; -} - inline double DetectorElementStub::thickness() const { return m_elementThickness; } diff --git a/Tests/CommonHelpers/include/ActsTests/CommonHelpers/LineSurfaceStub.hpp b/Tests/CommonHelpers/include/ActsTests/CommonHelpers/LineSurfaceStub.hpp index 1eb93795bfe..08379e09a44 100644 --- a/Tests/CommonHelpers/include/ActsTests/CommonHelpers/LineSurfaceStub.hpp +++ b/Tests/CommonHelpers/include/ActsTests/CommonHelpers/LineSurfaceStub.hpp @@ -29,9 +29,10 @@ class LineSurfaceStub : public Acts::LineSurface { Acts::LineSurface(htrans, std::move(lbounds)) { /*nop */ } LineSurfaceStub(std::shared_ptr lbounds, - const Acts::SurfacePlacementBase& placement) + std::shared_ptr placement) : Acts::GeometryObject(), - Acts::LineSurface(std::move(lbounds), placement) { /* nop */ } + Acts::LineSurface(std::move(lbounds), std::move(placement)) { /* nop */ + } LineSurfaceStub(const LineSurfaceStub& ls) : Acts::GeometryObject(), Acts::LineSurface(ls) { /* nop */ } diff --git a/Tests/CommonHelpers/src/DetectorElementStub.cpp b/Tests/CommonHelpers/src/DetectorElementStub.cpp index 2ca2f3923d2..d49c55f31b8 100644 --- a/Tests/CommonHelpers/src/DetectorElementStub.cpp +++ b/Tests/CommonHelpers/src/DetectorElementStub.cpp @@ -23,35 +23,77 @@ ActsTests::DetectorElementStub::DetectorElementStub(const Transform3& transform) ActsTests::DetectorElementStub::DetectorElementStub( const Transform3& transform, std::shared_ptr cBounds, double thickness, std::shared_ptr material) - : m_elementTransform(transform), m_elementThickness(thickness) { - m_elementSurface = - Surface::makeShared(std::move(cBounds), *this); - m_elementSurface->assignThickness(thickness); - m_elementSurface->assignSurfaceMaterial(std::move(material)); - assert(m_elementSurface->surfacePlacement() == this); - assert(m_elementSurface->isSensitive() == isSensitive()); + : m_elementTransform(transform), + m_elementThickness(thickness), + m_cylinderBounds(std::move(cBounds)), + m_deferredMaterial(std::move(material)) { + auto surf = Surface::makeShared(transform, m_cylinderBounds); + surf->assignThickness(m_elementThickness); + surf->assignSurfaceMaterial(m_deferredMaterial); + m_elementSurface = surf; + m_legacySurface = std::move(surf); } ActsTests::DetectorElementStub::DetectorElementStub( const Transform3& transform, std::shared_ptr pBounds, double thickness, std::shared_ptr material) - : m_elementTransform(transform), m_elementThickness(thickness) { - m_elementSurface = - Surface::makeShared(std::move(pBounds), *this); - m_elementSurface->assignThickness(thickness); - m_elementSurface->assignSurfaceMaterial(std::move(material)); - assert(m_elementSurface->surfacePlacement() == this); - assert(m_elementSurface->isSensitive() == isSensitive()); + : m_elementTransform(transform), + m_elementThickness(thickness), + m_planarBounds(std::move(pBounds)), + m_deferredMaterial(std::move(material)) { + auto surf = Surface::makeShared(transform, m_planarBounds); + surf->assignThickness(m_elementThickness); + surf->assignSurfaceMaterial(m_deferredMaterial); + m_elementSurface = surf; + m_legacySurface = std::move(surf); } ActsTests::DetectorElementStub::DetectorElementStub( const Transform3& transform, std::shared_ptr lBounds, double thickness, std::shared_ptr material) - : m_elementTransform(transform), m_elementThickness(thickness) { - m_elementSurface = - Surface::makeShared(std::move(lBounds), *this); - m_elementSurface->assignThickness(thickness); - m_elementSurface->assignSurfaceMaterial(std::move(material)); - assert(m_elementSurface->surfacePlacement() == this); - assert(m_elementSurface->isSensitive() == isSensitive()); + : m_elementTransform(transform), + m_elementThickness(thickness), + m_lineBounds(std::move(lBounds)), + m_deferredMaterial(std::move(material)) { + auto surf = Surface::makeShared(transform, m_lineBounds); + surf->assignThickness(m_elementThickness); + surf->assignSurfaceMaterial(m_deferredMaterial); + m_elementSurface = surf; + m_legacySurface = std::move(surf); +} + +std::shared_ptr ActsTests::DetectorElementStub::createSurface() { + std::shared_ptr surf; + + if (m_cylinderBounds) { + surf = Surface::makeShared(m_elementTransform, + m_cylinderBounds); + } else if (m_planarBounds) { + surf = Surface::makeShared(m_elementTransform, m_planarBounds); + } else if (m_lineBounds) { + surf = + Surface::makeShared(m_elementTransform, m_lineBounds); + } + + if (surf != nullptr) { + assignSurface(surf); + surf->assignThickness(m_elementThickness); + surf->assignSurfaceMaterial(m_deferredMaterial); + } + + m_legacySurface = surf; + m_elementSurface = surf; + return surf; +} + +const Acts::Surface& ActsTests::DetectorElementStub::surface() const { + auto s = m_elementSurface.lock(); + assert(s && "DetectorElementStub: call createSurface() before surface()"); + return *s; +} + +Acts::Surface& ActsTests::DetectorElementStub::surface() { + auto s = m_elementSurface.lock(); + assert(s && "DetectorElementStub: call createSurface() before surface()"); + return *s; } diff --git a/Tests/UnitTests/Alignment/Kernel/AlignmentTests.cpp b/Tests/UnitTests/Alignment/Kernel/AlignmentTests.cpp index df943fae8c0..907cc2d471e 100644 --- a/Tests/UnitTests/Alignment/Kernel/AlignmentTests.cpp +++ b/Tests/UnitTests/Alignment/Kernel/AlignmentTests.cpp @@ -124,7 +124,7 @@ struct TelescopeDetector { auto detElement = std::make_shared( trafo, rBounds, 1._um, surfaceMaterial); // The surface is not right!!! - auto surface = detElement->surface().getSharedPtr(); + auto surface = detElement->createSurface(); // Add it to the event store detectorStore.push_back(std::move(detElement)); auto surArray = std::make_unique(surface); diff --git a/Tests/UnitTests/Core/Geometry/AlignmentContextTests.cpp b/Tests/UnitTests/Core/Geometry/AlignmentContextTests.cpp index 6dce524df73..d3058f35ced 100644 --- a/Tests/UnitTests/Core/Geometry/AlignmentContextTests.cpp +++ b/Tests/UnitTests/Core/Geometry/AlignmentContextTests.cpp @@ -111,14 +111,22 @@ class AlignableDetectorElement : public SurfacePlacementBase { AlignableDetectorElement(std::shared_ptr transform, const std::shared_ptr& pBounds, double thickness) - : m_elementTransform(std::move(transform)) { - m_elementSurface = Surface::makeShared(pBounds, *this); - m_elementSurface->assignThickness(thickness); - } + : m_elementTransform(std::move(transform)), + m_pBounds(pBounds), + m_thickness(thickness) {} /// Destructor ~AlignableDetectorElement() override = default; + /// Create and return the surface associated with this detector element. + /// Must be called only after the element is managed by a std::shared_ptr. + std::shared_ptr createSurface() { + auto surf = Surface::makeShared(Transform3::Identity(), m_pBounds); + surf->assignSurfacePlacement(shared_from_this()); + surf->assignThickness(m_thickness); + return surf; + } + /// Return local to global transform associated with this identifier /// /// @param gctx The current geometry context object, e.g. alignment @@ -127,20 +135,14 @@ class AlignableDetectorElement : public SurfacePlacementBase { const Transform3& localToGlobalTransform( const GeometryContext& gctx) const override; - /// Return surface associated with this detector element - const Surface& surface() const override; - - /// Non-const access to the surface associated with this detector element - Surface& surface() override; - /// Is the detector element a sensitive element bool isSensitive() const override { return true; } private: /// the transform for positioning in 3D space std::shared_ptr m_elementTransform; - /// the surface represented by it - std::shared_ptr m_elementSurface{nullptr}; + std::shared_ptr m_pBounds; + double m_thickness; }; inline const Transform3& AlignableDetectorElement::localToGlobalTransform( @@ -153,14 +155,6 @@ inline const Transform3& AlignableDetectorElement::localToGlobalTransform( return (*m_elementTransform); } -inline const Surface& AlignableDetectorElement::surface() const { - return *m_elementSurface; -} - -inline Surface& AlignableDetectorElement::surface() { - return *m_elementSurface; -} - class AlignableVolumePlacement : public VolumePlacementBase { public: /// @@ -276,11 +270,11 @@ BOOST_AUTO_TEST_CASE(AlignmentContextTests) { std::make_shared>(alignmentArray); // The detector element at nominal position - AlignableDetectorElement alignedElement( + auto alignedElementPtr = std::make_shared( std::make_shared(Transform3::Identity()), std::make_shared(100_cm, 100_cm), 1_mm); - const auto& alignedSurface = alignedElement.surface(); + auto alignedSurface = alignedElementPtr->createSurface(); // The alignment contexts AlignmentContext alignDefault{}; @@ -292,48 +286,48 @@ BOOST_AUTO_TEST_CASE(AlignmentContextTests) { GeometryContext positiveContext{alignPositive.getContext()}; // Test the transforms - BOOST_CHECK(isSame(alignedSurface.localToGlobalTransform(defaultContext), + BOOST_CHECK(isSame(alignedSurface->localToGlobalTransform(defaultContext), Transform3::Identity())); - BOOST_CHECK(isSame(alignedSurface.localToGlobalTransform(negativeContext), + BOOST_CHECK(isSame(alignedSurface->localToGlobalTransform(negativeContext), negativeTransform)); - BOOST_CHECK(isSame(alignedSurface.localToGlobalTransform(positiveContext), + BOOST_CHECK(isSame(alignedSurface->localToGlobalTransform(positiveContext), positiveTransform)); // Test the centers - BOOST_CHECK_EQUAL(alignedSurface.center(defaultContext), nominalCenter); - BOOST_CHECK_EQUAL(alignedSurface.center(negativeContext), negativeCenter); - BOOST_CHECK_EQUAL(alignedSurface.center(positiveContext), positiveCenter); + BOOST_CHECK_EQUAL(alignedSurface->center(defaultContext), nominalCenter); + BOOST_CHECK_EQUAL(alignedSurface->center(negativeContext), negativeCenter); + BOOST_CHECK_EQUAL(alignedSurface->center(positiveContext), positiveCenter); // Test OnSurface BOOST_CHECK( - alignedSurface.isOnSurface(defaultContext, onNominal, dummyMomentum)); + alignedSurface->isOnSurface(defaultContext, onNominal, dummyMomentum)); BOOST_CHECK( - alignedSurface.isOnSurface(negativeContext, onNegative, dummyMomentum)); + alignedSurface->isOnSurface(negativeContext, onNegative, dummyMomentum)); BOOST_CHECK( - alignedSurface.isOnSurface(positiveContext, onPositive, dummyMomentum)); + alignedSurface->isOnSurface(positiveContext, onPositive, dummyMomentum)); // Test local to Global and vice versa - globalPosition = alignedSurface.localToGlobal(defaultContext, localPosition, - dummyMomentum); + globalPosition = alignedSurface->localToGlobal(defaultContext, localPosition, + dummyMomentum); BOOST_CHECK_EQUAL(globalPosition, onNominal); localPosition = - alignedSurface.globalToLocal(defaultContext, onNominal, dummyMomentum) + alignedSurface->globalToLocal(defaultContext, onNominal, dummyMomentum) .value(); BOOST_CHECK_EQUAL(localPosition, Vector2(3., 3.)); - globalPosition = alignedSurface.localToGlobal(negativeContext, localPosition, - dummyMomentum); + globalPosition = alignedSurface->localToGlobal(negativeContext, localPosition, + dummyMomentum); BOOST_CHECK_EQUAL(globalPosition, onNegative); localPosition = - alignedSurface.globalToLocal(negativeContext, onNegative, dummyMomentum) + alignedSurface->globalToLocal(negativeContext, onNegative, dummyMomentum) .value(); BOOST_CHECK_EQUAL(localPosition, Vector2(3., 3.)); - globalPosition = alignedSurface.localToGlobal(positiveContext, localPosition, - dummyMomentum); + globalPosition = alignedSurface->localToGlobal(positiveContext, localPosition, + dummyMomentum); BOOST_CHECK_EQUAL(globalPosition, onPositive); localPosition = - alignedSurface.globalToLocal(positiveContext, onPositive, dummyMomentum) + alignedSurface->globalToLocal(positiveContext, onPositive, dummyMomentum) .value(); BOOST_CHECK_EQUAL(localPosition, Vector2(3., 3.)); } diff --git a/Tests/UnitTests/Core/Geometry/BlueprintApiTests.cpp b/Tests/UnitTests/Core/Geometry/BlueprintApiTests.cpp index 23aa8930692..c615c1ec7e5 100644 --- a/Tests/UnitTests/Core/Geometry/BlueprintApiTests.cpp +++ b/Tests/UnitTests/Core/Geometry/BlueprintApiTests.cpp @@ -51,7 +51,7 @@ auto gctx = GeometryContext::dangerouslyDefaultConstruct(); inline std::vector> makeFanLayer( const Transform3& base, - std::vector>& elements, + std::vector>& elements, double r = 300_mm, std::size_t nSensors = 8, double thickness = 0) { auto recBounds = std::make_shared(40_mm, 60_mm); @@ -67,19 +67,16 @@ inline std::vector> makeFanLayer( trf = trf * Translation3{Vector3::UnitZ() * 5_mm}; } - auto& element = elements.emplace_back( - std::make_unique(trf, recBounds, thickness)); - - element->surface().assignSurfacePlacement(*element); - - surfaces.push_back(element->surface().getSharedPtr()); + auto element = std::make_shared(trf, recBounds, thickness); + elements.push_back(element); + surfaces.push_back(element->createSurface()); } return surfaces; } inline std::vector> makeBarrelLayer( const Transform3& base, - std::vector>& elements, + std::vector>& elements, double r = 300_mm, std::size_t nStaves = 10, int nSensorsPerStave = 8, double thickness = 0, double hlPhi = 40_mm, double hlZ = 60_mm) { auto recBounds = std::make_shared(hlPhi, hlZ); @@ -98,10 +95,9 @@ inline std::vector> makeBarrelLayer( AngleAxis3{10_degree, Vector3::UnitZ()} * AngleAxis3{90_degree, Vector3::UnitY()} * AngleAxis3{90_degree, Vector3::UnitZ()}; - auto& element = elements.emplace_back( - std::make_unique(trf, recBounds, thickness)); - element->surface().assignSurfacePlacement(*element); - surfaces.push_back(element->surface().getSharedPtr()); + auto element = std::make_shared(trf, recBounds, thickness); + elements.push_back(element); + surfaces.push_back(element->createSurface()); } } @@ -258,7 +254,7 @@ BOOST_AUTO_TEST_CASE(NodeApiTestContainers) { // Transform3 base{AngleAxis3{30_degree, Vector3{1, 0, 0}}}; Transform3 base{Transform3::Identity()}; - std::vector> detectorElements; + std::vector> detectorElements; auto makeFan = [&](const Transform3& layerBase, auto&&..., double r, std::size_t nSensors, double thickness) { return makeFanLayer(layerBase, detectorElements, r, nSensors, thickness); diff --git a/Tests/UnitTests/Core/Geometry/BlueprintTests.cpp b/Tests/UnitTests/Core/Geometry/BlueprintTests.cpp index fe6958cc1ad..0a46d6db623 100644 --- a/Tests/UnitTests/Core/Geometry/BlueprintTests.cpp +++ b/Tests/UnitTests/Core/Geometry/BlueprintTests.cpp @@ -439,7 +439,7 @@ BOOST_AUTO_TEST_CASE(DiscLayer) { Transform3 base = Transform3::Identity() * AngleAxis3{yrot, Vector3::UnitY()}; std::vector> surfaces; - std::vector> elements; + std::vector> elements; double r = 300_mm; std::size_t nSensors = 8; double thickness = 2.5_mm; @@ -456,12 +456,9 @@ BOOST_AUTO_TEST_CASE(DiscLayer) { trf = trf * Translation3{Vector3::UnitZ() * 5_mm}; } - auto& element = elements.emplace_back( - std::make_unique(trf, recBounds, thickness)); - - element->surface().assignSurfacePlacement(*element); - - surfaces.push_back(element->surface().getSharedPtr()); + auto element = std::make_shared(trf, recBounds, thickness); + elements.push_back(element); + surfaces.push_back(element->createSurface()); } std::function withSurfaces = @@ -526,7 +523,7 @@ BOOST_AUTO_TEST_CASE(CylinderLayer) { Transform3 base = Transform3::Identity() * AngleAxis3{yrot, Vector3::UnitY()}; std::vector> surfaces; - std::vector> elements; + std::vector> elements; double r = 300_mm; std::size_t nStaves = 10; @@ -549,10 +546,9 @@ BOOST_AUTO_TEST_CASE(CylinderLayer) { AngleAxis3{10_degree, Vector3::UnitZ()} * AngleAxis3{90_degree, Vector3::UnitY()} * AngleAxis3{90_degree, Vector3::UnitZ()}; - auto& element = elements.emplace_back( - std::make_unique(trf, recBounds, thickness)); - element->surface().assignSurfacePlacement(*element); - surfaces.push_back(element->surface().getSharedPtr()); + auto element = std::make_shared(trf, recBounds, thickness); + elements.push_back(element); + surfaces.push_back(element->createSurface()); } } @@ -1069,7 +1065,7 @@ BOOST_AUTO_TEST_CASE(LayerCenterOfGravity) { Transform3::Identity() * AngleAxis3{yrot, Vector3::UnitY()}; std::vector> surfaces; - std::vector> elements; + std::vector> elements; double r = 300_mm; std::size_t nSensors = 8; double thickness = 2.5_mm; @@ -1084,11 +1080,9 @@ BOOST_AUTO_TEST_CASE(LayerCenterOfGravity) { trf = trf * Translation3{Vector3::UnitZ() * 5_mm}; } - auto& element = elements.emplace_back( - std::make_unique(trf, recBounds, thickness)); - - element->surface().assignSurfacePlacement(*element); - surfaces.push_back(element->surface().getSharedPtr()); + auto element = std::make_shared(trf, recBounds, thickness); + elements.push_back(element); + surfaces.push_back(element->createSurface()); } Blueprint root{{.envelope = ExtentEnvelope{{ @@ -1132,7 +1126,7 @@ BOOST_AUTO_TEST_CASE(LayerCenterOfGravity) { Transform3::Identity() * AngleAxis3{yrot, Vector3::UnitY()}; std::vector> surfaces; - std::vector> elements; + std::vector> elements; double r = 300_mm; std::size_t nStaves = 10; @@ -1155,10 +1149,9 @@ BOOST_AUTO_TEST_CASE(LayerCenterOfGravity) { AngleAxis3{10_degree, Vector3::UnitZ()} * AngleAxis3{90_degree, Vector3::UnitY()} * AngleAxis3{90_degree, Vector3::UnitZ()}; - auto& element = elements.emplace_back( - std::make_unique(trf, recBounds, thickness)); - element->surface().assignSurfacePlacement(*element); - surfaces.push_back(element->surface().getSharedPtr()); + auto element = std::make_shared(trf, recBounds, thickness); + elements.push_back(element); + surfaces.push_back(element->createSurface()); } } diff --git a/Tests/UnitTests/Core/Geometry/CuboidVolumeBuilderTests.cpp b/Tests/UnitTests/Core/Geometry/CuboidVolumeBuilderTests.cpp index 6dc73114253..48676965a9d 100644 --- a/Tests/UnitTests/Core/Geometry/CuboidVolumeBuilderTests.cpp +++ b/Tests/UnitTests/Core/Geometry/CuboidVolumeBuilderTests.cpp @@ -77,7 +77,7 @@ BOOST_AUTO_TEST_CASE(CuboidVolumeBuilderTest) { [](const Transform3& trans, const std::shared_ptr& bounds, double thickness) { - return new DetectorElementStub(trans, bounds, thickness); + return std::make_shared(trans, bounds, thickness); }; surfaceConfig.push_back(cfg); } diff --git a/Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp b/Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp index 6154e6ab0e9..1a2a860da0b 100644 --- a/Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp +++ b/Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp @@ -183,7 +183,7 @@ BOOST_AUTO_TEST_CASE(OrientedLayer) { auto recBounds = std::make_shared(3_mm, 6_mm); - std::vector> detectorElements; + std::vector> detectorElements; auto makeFan = [&](double yrot, double thickness = 0) { detectorElements.clear(); @@ -199,10 +199,9 @@ BOOST_AUTO_TEST_CASE(OrientedLayer) { AngleAxis3{deltaPhi * i, Vector3::UnitZ()} * Translation3(Vector3::UnitX() * r); - auto& element = detectorElements.emplace_back( - std::make_unique(trf, recBounds, thickness)); - - surfaces.push_back(element->surface().getSharedPtr()); + auto element = std::make_shared(trf, recBounds, thickness); + detectorElements.push_back(element); + surfaces.push_back(element->createSurface()); } return surfaces; }; diff --git a/Tests/UnitTests/Core/Navigation/MultiWireNavigationTests.cpp b/Tests/UnitTests/Core/Navigation/MultiWireNavigationTests.cpp index bf35c67fc57..8c03e1c08af 100644 --- a/Tests/UnitTests/Core/Navigation/MultiWireNavigationTests.cpp +++ b/Tests/UnitTests/Core/Navigation/MultiWireNavigationTests.cpp @@ -42,7 +42,7 @@ auto logger = getDefaultLogger("MultiWireNavigationTests", Logging::VERBOSE); void generateStrawSurfaces( std::size_t nStraws, std::size_t nLayers, double radius, double halfZ, std::vector>& strawSurfaces, - std::vector>& elements) { + std::vector>& elements) { // The transform of the 1st surface Vector3 ipos = {-0.5 * nStraws * 2 * radius + radius, -0.5 * nLayers * 2 * radius + radius, 0.}; @@ -56,15 +56,13 @@ void generateStrawSurfaces( for (std::size_t j = 0; j < nStraws; j++) { pos.x() = ipos.x() + 2 * j * radius; - auto& element = - elements.emplace_back(std::make_unique( - Transform3(Translation3(pos)), lineBounds, 0)); + auto element = std::make_shared( + Transform3(Translation3(pos)), lineBounds, 0); + elements.push_back(element); - element->surface().assignGeometryId(GeometryIdentifier(id++)); - - element->surface().assignSurfacePlacement(*element); - - strawSurfaces.push_back(element->surface().getSharedPtr()); + auto surf = element->createSurface(); + surf->assignGeometryId(GeometryIdentifier(id++)); + strawSurfaces.push_back(std::move(surf)); } pos.y() = ipos.y() + 2 * (i + 1) * radius; @@ -75,7 +73,7 @@ void generateStrawSurfaces( BOOST_AUTO_TEST_CASE(MultiLayer_NavigationPolicy) { // Create the surfaces std::vector> strawSurfaces = {}; - std::vector> detElements = {}; + std::vector> detElements = {}; generateStrawSurfaces(nSurfacesX, nSurfacesY, radius, halfZ, strawSurfaces, detElements); diff --git a/Tests/UnitTests/Core/Propagator/NavigatorTests.cpp b/Tests/UnitTests/Core/Propagator/NavigatorTests.cpp index fd048736ba8..c0b5fa56c49 100644 --- a/Tests/UnitTests/Core/Propagator/NavigatorTests.cpp +++ b/Tests/UnitTests/Core/Propagator/NavigatorTests.cpp @@ -688,9 +688,9 @@ BOOST_AUTO_TEST_CASE(TryAllNavigationPolicy_SurfaceInsideVolume) { std::move(planarBounds)); auto detElement = - std::make_unique(Transform3::Identity()); + std::make_shared(Transform3::Identity()); - surface->assignSurfacePlacement(*detElement); + surface->assignSurfacePlacement(detElement); parentVol->assignGeometryId(GeometryIdentifier{}.withVolume(1)); parentVol->addSurface(surface); diff --git a/Tests/UnitTests/Core/Surfaces/CylinderSurfaceTests.cpp b/Tests/UnitTests/Core/Surfaces/CylinderSurfaceTests.cpp index bc498481ab5..420030898a6 100644 --- a/Tests/UnitTests/Core/Surfaces/CylinderSurfaceTests.cpp +++ b/Tests/UnitTests/Core/Surfaces/CylinderSurfaceTests.cpp @@ -349,7 +349,7 @@ BOOST_AUTO_TEST_CASE(CylinderSurfaceBinningPosition) { BOOST_AUTO_TEST_SUITE(CylinderSurfaceMerging) BOOST_AUTO_TEST_CASE(InvalidDetectorElement) { - DetectorElementStub detElem; + auto detElem = std::make_shared(); auto bounds = std::make_shared(100_mm, 100_mm); auto cyl1 = Surface::makeShared(bounds, detElem); diff --git a/Tests/UnitTests/Core/Surfaces/DiscSurfaceTests.cpp b/Tests/UnitTests/Core/Surfaces/DiscSurfaceTests.cpp index a6334e40866..e625edacba0 100644 --- a/Tests/UnitTests/Core/Surfaces/DiscSurfaceTests.cpp +++ b/Tests/UnitTests/Core/Surfaces/DiscSurfaceTests.cpp @@ -77,7 +77,7 @@ BOOST_AUTO_TEST_CASE(DiscSurfaceConstruction) { tgContext, *anotherDiscSurface, pTransform)); /// Construct with nullptr bounds - DetectorElementStub detElem; + auto detElem = std::make_shared(); BOOST_CHECK_THROW( auto nullBounds = Surface::makeShared(nullptr, detElem), AssertionFailureException); @@ -442,7 +442,7 @@ BOOST_AUTO_TEST_CASE(IncompatibleBounds) { } BOOST_AUTO_TEST_CASE(InvalidDetectorElement) { - DetectorElementStub detElem; + auto detElem = std::make_shared(); auto bounds1 = std::make_shared(30_mm, 100_mm); auto disc1 = Surface::makeShared(bounds1, detElem); diff --git a/Tests/UnitTests/Core/Surfaces/LineSurfaceTests.cpp b/Tests/UnitTests/Core/Surfaces/LineSurfaceTests.cpp index e152a95961f..03a0e7bb30a 100644 --- a/Tests/UnitTests/Core/Surfaces/LineSurfaceTests.cpp +++ b/Tests/UnitTests/Core/Surfaces/LineSurfaceTests.cpp @@ -68,7 +68,7 @@ BOOST_AUTO_TEST_CASE(LineSurface_Constructors_test) { /// ctor with LineBounds, detector element, Identifier auto pMaterial = std::make_shared(makePercentSlab()); - DetectorElementStub detElement{pTransform, pLineBounds, 0.2, pMaterial}; + auto detElement = std::make_shared(pTransform, pLineBounds, 0.2, pMaterial); BOOST_CHECK(LineSurfaceStub(pLineBounds, detElement).constructedOk()); LineSurfaceStub lineToCopy(pTransform, 2., 20.); @@ -80,7 +80,7 @@ BOOST_AUTO_TEST_CASE(LineSurface_Constructors_test) { LineSurfaceStub(tgContext, lineToCopy, transform).constructedOk()); /// Construct with nullptr bounds - DetectorElementStub detElem; + auto detElem = std::make_shared(); BOOST_CHECK_THROW(LineSurfaceStub nullBounds(nullptr, detElem), AssertionFailureException); diff --git a/Tests/UnitTests/Core/Surfaces/PlaneSurfaceTests.cpp b/Tests/UnitTests/Core/Surfaces/PlaneSurfaceTests.cpp index e06a0d7713d..8bd0746fd0c 100644 --- a/Tests/UnitTests/Core/Surfaces/PlaneSurfaceTests.cpp +++ b/Tests/UnitTests/Core/Surfaces/PlaneSurfaceTests.cpp @@ -75,7 +75,7 @@ BOOST_AUTO_TEST_CASE(PlaneSurfaceConstruction) { BOOST_CHECK_EQUAL(copiedTransformedPlaneSurface->type(), Surface::Plane); /// Construct with nullptr bounds - DetectorElementStub detElem; + auto detElem = std::make_shared(); BOOST_CHECK_THROW( auto nullBounds = Surface::makeShared(nullptr, detElem), AssertionFailureException); diff --git a/Tests/UnitTests/Core/Surfaces/StrawSurfaceTests.cpp b/Tests/UnitTests/Core/Surfaces/StrawSurfaceTests.cpp index 3a9ae5a67d2..d3b8f6262a1 100644 --- a/Tests/UnitTests/Core/Surfaces/StrawSurfaceTests.cpp +++ b/Tests/UnitTests/Core/Surfaces/StrawSurfaceTests.cpp @@ -57,7 +57,7 @@ BOOST_AUTO_TEST_CASE(StrawSurfaceConstruction) { /// Constructor with LineBounds ptr, DetectorElement std::shared_ptr p = std::make_shared(1., 10.); - DetectorElementStub detElement{pTransform, p, 1., nullptr}; + auto detElement = std::make_shared(pTransform, p, 1., nullptr); BOOST_CHECK_EQUAL( Surface::makeShared(pLineBounds, detElement)->type(), Surface::Straw); diff --git a/Tests/UnitTests/Core/Surfaces/SurfaceStub.hpp b/Tests/UnitTests/Core/Surfaces/SurfaceStub.hpp index a619776a07d..e282eee527d 100644 --- a/Tests/UnitTests/Core/Surfaces/SurfaceStub.hpp +++ b/Tests/UnitTests/Core/Surfaces/SurfaceStub.hpp @@ -27,8 +27,9 @@ class SurfaceStub : public Acts::RegularSurface { SurfaceStub(const Acts::GeometryContext& gctx, const SurfaceStub& sf, const Acts::Transform3& transf) : Acts::GeometryObject(), Acts::RegularSurface(gctx, sf, transf) {} - explicit SurfaceStub(const Acts::SurfacePlacementBase& detelement) - : Acts::GeometryObject(), Acts::RegularSurface(detelement) {} + explicit SurfaceStub(std::shared_ptr placement) + : Acts::GeometryObject(), + Acts::RegularSurface(std::move(placement)) {} ~SurfaceStub() override = default; diff --git a/Tests/UnitTests/Core/Surfaces/SurfaceTests.cpp b/Tests/UnitTests/Core/Surfaces/SurfaceTests.cpp index 0545eb4e78c..47d0de3db88 100644 --- a/Tests/UnitTests/Core/Surfaces/SurfaceTests.cpp +++ b/Tests/UnitTests/Core/Surfaces/SurfaceTests.cpp @@ -68,7 +68,7 @@ BOOST_AUTO_TEST_CASE(SurfaceConstruction) { auto pTransform = Transform3(translation); std::shared_ptr p = std::make_shared(5., 10.); - DetectorElementStub detElement{pTransform, p, 0.2, nullptr}; + auto detElement = std::make_shared(pTransform, p, 0.2, nullptr); BOOST_CHECK_EQUAL(Surface::Other, SurfaceStub(detElement).type()); } @@ -83,11 +83,11 @@ BOOST_AUTO_TEST_CASE(SurfaceProperties) { auto pLayer = PlaneLayer::create(pTransform, pPlanarBound, nullptr); auto pMaterial = std::make_shared(makePercentSlab()); - DetectorElementStub detElement{pTransform, pPlanarBound, 0.2, pMaterial}; + auto detElement = std::make_shared(pTransform, pPlanarBound, 0.2, pMaterial); SurfaceStub surface(detElement); // associatedDetectorElement - BOOST_CHECK_EQUAL(surface.surfacePlacement(), &detElement); + BOOST_CHECK_EQUAL(surface.surfacePlacement(), detElement.get()); // test associatelayer, associatedLayer surface.associateLayer(*pLayer); @@ -159,9 +159,9 @@ BOOST_AUTO_TEST_CASE(EqualityOperators) { auto pLayer = PlaneLayer::create(pTransform1, pPlanarBound, nullptr); auto pMaterial = std::make_shared(makePercentSlab()); - DetectorElementStub detElement1{pTransform1, pPlanarBound, 0.2, pMaterial}; - DetectorElementStub detElement2{pTransform1, pPlanarBound, 0.3, pMaterial}; - DetectorElementStub detElement3{pTransform2, pPlanarBound, 0.3, pMaterial}; + auto detElement1 = std::make_shared(pTransform1, pPlanarBound, 0.2, pMaterial); + auto detElement2 = std::make_shared(pTransform1, pPlanarBound, 0.3, pMaterial); + auto detElement3 = std::make_shared(pTransform2, pPlanarBound, 0.3, pMaterial); SurfaceStub surface1(detElement1); SurfaceStub surface2(detElement1); // 1 and 2 are the same diff --git a/Tests/UnitTests/Core/TrackFitting/Gx2fTests.cpp b/Tests/UnitTests/Core/TrackFitting/Gx2fTests.cpp index 5579f51f3ef..77dcde9279c 100644 --- a/Tests/UnitTests/Core/TrackFitting/Gx2fTests.cpp +++ b/Tests/UnitTests/Core/TrackFitting/Gx2fTests.cpp @@ -163,7 +163,7 @@ std::shared_ptr makeToyDetector( [](const Transform3& trans, const std::shared_ptr& bounds, double thickness) { - return new DetectorElementStub(trans, bounds, thickness); + return std::make_shared(trans, bounds, thickness); }; surfaceConfig.push_back(cfg); } diff --git a/Tests/UnitTests/Core/Visualization/EventDataView3DBase.hpp b/Tests/UnitTests/Core/Visualization/EventDataView3DBase.hpp index 5f3eaee29c1..f4a36ee0918 100644 --- a/Tests/UnitTests/Core/Visualization/EventDataView3DBase.hpp +++ b/Tests/UnitTests/Core/Visualization/EventDataView3DBase.hpp @@ -109,7 +109,7 @@ void createDetector(GeometryContext& tgContext, [](const Transform3& trans, const std::shared_ptr& bounds, double thickness) { - return new ActsTests::DetectorElementStub(trans, bounds, thickness); + return std::make_shared(trans, bounds, thickness); }; CuboidVolumeBuilder::LayerConfig lConf; lConf.surfaceCfg = {sConf}; diff --git a/Tests/UnitTests/Plugins/DD4hep/DD4hepCylindricalDetectorGen3Tests.cpp b/Tests/UnitTests/Plugins/DD4hep/DD4hepCylindricalDetectorGen3Tests.cpp index aec69611ea1..6a3b0d157c2 100644 --- a/Tests/UnitTests/Plugins/DD4hep/DD4hepCylindricalDetectorGen3Tests.cpp +++ b/Tests/UnitTests/Plugins/DD4hep/DD4hepCylindricalDetectorGen3Tests.cpp @@ -504,7 +504,7 @@ BOOST_AUTO_TEST_CASE(DD4hepCylidricalDetectorExplicit) { auto dd4hepDetEl = std::make_shared( module, detAxis, 1_cm, nullptr); detectorElements.push_back(dd4hepDetEl); - layers[layerId].push_back(dd4hepDetEl->surface().getSharedPtr()); + layers[layerId].push_back(dd4hepDetEl->createSurface()); } layerId++; } @@ -591,8 +591,7 @@ BOOST_AUTO_TEST_CASE(DD4hepCylidricalDetectorExplicit) { auto dd4hepDetEl = std::make_shared( module, detAxis, 1_cm, nullptr); detectorElements.push_back(dd4hepDetEl); - initialLayers[layerId].push_back( - dd4hepDetEl->surface().getSharedPtr()); + initialLayers[layerId].push_back(dd4hepDetEl->createSurface()); } layerId++; } diff --git a/Tests/UnitTests/Plugins/DD4hep/DD4hepDetectorElementTests.cpp b/Tests/UnitTests/Plugins/DD4hep/DD4hepDetectorElementTests.cpp index ad09425adee..9a730b0c815 100644 --- a/Tests/UnitTests/Plugins/DD4hep/DD4hepDetectorElementTests.cpp +++ b/Tests/UnitTests/Plugins/DD4hep/DD4hepDetectorElementTests.cpp @@ -118,7 +118,8 @@ BOOST_AUTO_TEST_CASE(DD4hepPluginDetectorElementCylinder) { BOOST_REQUIRE_NE(cylindricalElement, nullptr); - const auto& surface = cylindricalElement->surface(); + auto surfacePtr = cylindricalElement->createSurface(); + const auto& surface = *surfacePtr; BOOST_CHECK_EQUAL(surface.type(), Surface::SurfaceType::Cylinder); BOOST_CHECK(surface.localToGlobalTransform(tContext).isApprox( Transform3::Identity())); @@ -152,7 +153,8 @@ BOOST_AUTO_TEST_CASE(DD4hepPluginDetectorElementSectoralCylinder) { BOOST_REQUIRE_NE(cylindricalElement, nullptr); - const auto& surface = cylindricalElement->surface(); + auto surfacePtr = cylindricalElement->createSurface(); + const auto& surface = *surfacePtr; BOOST_CHECK_EQUAL(surface.type(), Surface::SurfaceType::Cylinder); BOOST_CHECK(surface.localToGlobalTransform(tContext).isApprox( Transform3::Identity())); @@ -187,7 +189,8 @@ BOOST_AUTO_TEST_CASE(DD4hepPluginDetectorElementDisc) { BOOST_REQUIRE_NE(discElement, nullptr); - const auto& surface = discElement->surface(); + auto surfacePtr = discElement->createSurface(); + const auto& surface = *surfacePtr; BOOST_CHECK_EQUAL(surface.type(), Surface::SurfaceType::Disc); BOOST_CHECK(surface.localToGlobalTransform(tContext).isApprox( Transform3::Identity())); @@ -220,7 +223,8 @@ BOOST_AUTO_TEST_CASE(DD4hepPluginDetectorElementSectoralDisc) { BOOST_REQUIRE_NE(discElement, nullptr); - const auto& surface = discElement->surface(); + auto surfacePtr = discElement->createSurface(); + const auto& surface = *surfacePtr; BOOST_CHECK_EQUAL(surface.type(), Surface::SurfaceType::Disc); BOOST_CHECK(surface.localToGlobalTransform(tContext).isApprox( Transform3::Identity())); @@ -257,7 +261,8 @@ BOOST_AUTO_TEST_CASE(DD4hepPluginDetectorElementRectangle) { BOOST_REQUIRE_NE(rectangleElement, nullptr); - Surface& surface = rectangleElement->surface(); + auto surfacePtr = rectangleElement->createSurface(); + auto& surface = *surfacePtr; surface.assignGeometryId(GeometryIdentifier().withVolume(1).withLayer(2)); BOOST_CHECK_EQUAL(surface.type(), Surface::SurfaceType::Plane); @@ -302,7 +307,8 @@ BOOST_AUTO_TEST_CASE(DD4hepPluginDetectorElementTrapezoid) { BOOST_REQUIRE_NE(trapezoidElement, nullptr); - const auto& surface = trapezoidElement->surface(); + auto surfacePtr = trapezoidElement->createSurface(); + const auto& surface = *surfacePtr; BOOST_CHECK_EQUAL(surface.type(), Surface::SurfaceType::Plane); auto sTransform = surface.localToGlobalTransform(tContext); diff --git a/Tests/UnitTests/Plugins/Detray/DetrayPayloadConverterTests.cpp b/Tests/UnitTests/Plugins/Detray/DetrayPayloadConverterTests.cpp index 2fa25f79d59..cc8b62d533d 100644 --- a/Tests/UnitTests/Plugins/Detray/DetrayPayloadConverterTests.cpp +++ b/Tests/UnitTests/Plugins/Detray/DetrayPayloadConverterTests.cpp @@ -350,7 +350,7 @@ BOOST_AUTO_TEST_CASE(DetraySurfaceConversionTests) { // Create surface using the detector element auto sensitiveSurface = - Surface::makeShared(bounds, *detElement); + Surface::makeShared(bounds, detElement); auto payload = converter.convertSurface(gctx, *sensitiveSurface); BOOST_CHECK(payload.type == detray::surface_id::e_sensitive); diff --git a/Tests/UnitTests/Plugins/Geant4/Geant4DetectorElementTests.cpp b/Tests/UnitTests/Plugins/Geant4/Geant4DetectorElementTests.cpp index cf7f5b34905..502ddf1b356 100644 --- a/Tests/UnitTests/Plugins/Geant4/Geant4DetectorElementTests.cpp +++ b/Tests/UnitTests/Plugins/Geant4/Geant4DetectorElementTests.cpp @@ -41,12 +41,14 @@ BOOST_AUTO_TEST_CASE(Geant4DetectorElement_construction) { auto rSurface = Surface::makeShared(rTransform, std::move(rBounds)); // A detector element - Geant4DetectorElement g4DetElement(rSurface, *g4physVol, rTransform, 0.1); - - BOOST_CHECK_EQUAL(g4DetElement.thickness(), 0.1); - BOOST_CHECK_EQUAL(&g4DetElement.surface(), rSurface.get()); - BOOST_CHECK_EQUAL(&g4DetElement.g4PhysicalVolume(), g4physVol.get()); - BOOST_CHECK(g4DetElement.localToGlobalTransform(tContext).isApprox( + auto g4DetElement = + std::make_shared(*g4physVol, rTransform, 0.1); + rSurface->assignSurfacePlacement(g4DetElement); + + BOOST_CHECK_EQUAL(g4DetElement->thickness(), 0.1); + BOOST_CHECK_EQUAL(&g4DetElement->surface(), rSurface.get()); + BOOST_CHECK_EQUAL(&g4DetElement->g4PhysicalVolume(), g4physVol.get()); + BOOST_CHECK(g4DetElement->localToGlobalTransform(tContext).isApprox( rSurface->localToGlobalTransform(tContext))); } diff --git a/Tests/UnitTests/Plugins/Geant4/Geant4DetectorSurfaceFactoryTests.cpp b/Tests/UnitTests/Plugins/Geant4/Geant4DetectorSurfaceFactoryTests.cpp index d74bea4dca7..764f96dd7b0 100644 --- a/Tests/UnitTests/Plugins/Geant4/Geant4DetectorSurfaceFactoryTests.cpp +++ b/Tests/UnitTests/Plugins/Geant4/Geant4DetectorSurfaceFactoryTests.cpp @@ -225,8 +225,13 @@ BOOST_AUTO_TEST_CASE(Geant4DetecturSurfaceFactory_elemnet_overwrite) { [](std::shared_ptr surface, const G4VPhysicalVolume& g4physVol, const Transform3& toGlobal, double thickness) -> std::shared_ptr { - return std::make_shared( - std::move(surface), g4physVol, toGlobal, thickness); + auto el = std::make_shared(g4physVol, toGlobal, + thickness); + if (thickness > 0.) { + surface->assignThickness(thickness); + } + el->assignSurface(std::move(surface)); + return el; }; G4Box* worldS = new G4Box("world", 100, 100, 100); diff --git a/Tests/UnitTests/Plugins/GeoModel/GeoModelDetectorElementITkTests.cpp b/Tests/UnitTests/Plugins/GeoModel/GeoModelDetectorElementITkTests.cpp index a18f68fc165..be8a238d560 100644 --- a/Tests/UnitTests/Plugins/GeoModel/GeoModelDetectorElementITkTests.cpp +++ b/Tests/UnitTests/Plugins/GeoModel/GeoModelDetectorElementITkTests.cpp @@ -60,23 +60,21 @@ BOOST_AUTO_TEST_CASE(GeoModelDetectorElementConstruction) { auto fphys = make_intrusive(log); auto rBounds = std::make_shared(100, 200); - auto element = - GeoModelDetectorElement::createDetectorElement( - fphys, rBounds, Transform3::Identity(), 2.0); + auto geoSurface = + Surface::makeShared(Transform3::Identity(), rBounds); + auto geoElement = GeoModelDetectorElement::createDetectorElement( + fphys, Transform3::Identity(), 2.0, geoSurface); const int hardware = 0, barrelEndcap = -2, layerWheel = 100, phiModule = 200, etaModule = 300, side = 1; - auto [itkElement, _] = GeoModelDetectorElementITk::convertFromGeomodel( - element, element->surface().getSharedPtr(), gctx, hardware, barrelEndcap, + auto [itkElement, itkSurface] = GeoModelDetectorElementITk::convertFromGeomodel( + geoElement, geoSurface, gctx, hardware, barrelEndcap, layerWheel, etaModule, phiModule, side); - BOOST_CHECK_EQUAL(element->surface().type(), itkElement->surface().type()); - BOOST_CHECK_EQUAL(element->surface().bounds().type(), - itkElement->surface().bounds().type()); - BOOST_CHECK_NE(element->surface().surfacePlacement(), - itkElement->surface().surfacePlacement()); + BOOST_CHECK_EQUAL(geoSurface->type(), itkSurface->type()); + BOOST_CHECK_EQUAL(geoSurface->bounds().type(), itkSurface->bounds().type()); + BOOST_CHECK_NE(geoSurface->surfacePlacement(), itkSurface->surfacePlacement()); BOOST_CHECK_EQUAL(itkElement->identifier().barrelEndcap(), barrelEndcap); BOOST_CHECK_EQUAL(itkElement->identifier().hardware(), hardware); BOOST_CHECK_EQUAL(itkElement->identifier().layerWheel(), layerWheel); diff --git a/Tests/UnitTests/Plugins/GeoModel/GeoModelDetectorElementTests.cpp b/Tests/UnitTests/Plugins/GeoModel/GeoModelDetectorElementTests.cpp index 1956250be9d..d478ddcc076 100644 --- a/Tests/UnitTests/Plugins/GeoModel/GeoModelDetectorElementTests.cpp +++ b/Tests/UnitTests/Plugins/GeoModel/GeoModelDetectorElementTests.cpp @@ -36,13 +36,12 @@ BOOST_AUTO_TEST_CASE(GeoModelDetectorElementConstruction) { auto rBounds = std::make_shared(100, 200); PVConstLink physXY{fphysXY}; - auto elementXY = - GeoModelDetectorElement::createDetectorElement( - physXY, rBounds, Transform3::Identity(), 2.0); + auto surfaceXY = + Surface::makeShared(Transform3::Identity(), rBounds); + auto elementXY = GeoModelDetectorElement::createDetectorElement( + physXY, Transform3::Identity(), 2.0, surfaceXY); - const Surface& surface = elementXY->surface(); - BOOST_CHECK(surface.type() == Surface::SurfaceType::Plane); + BOOST_CHECK(surfaceXY->type() == Surface::SurfaceType::Plane); } BOOST_AUTO_TEST_SUITE_END() diff --git a/Tests/UnitTests/Plugins/Json/JsonSurfacesReaderTests.cpp b/Tests/UnitTests/Plugins/Json/JsonSurfacesReaderTests.cpp index 917eb5441bd..6cb7fea335d 100644 --- a/Tests/UnitTests/Plugins/Json/JsonSurfacesReaderTests.cpp +++ b/Tests/UnitTests/Plugins/Json/JsonSurfacesReaderTests.cpp @@ -55,7 +55,7 @@ struct FileFixture { FileFixture() { nlohmann::json js = nlohmann::json::array(); - for (const auto &s : surfaces) { + for (const auto& s : surfaces) { js.push_back(SurfaceJsonConverter::toJson(gctx, *s)); } @@ -95,7 +95,7 @@ BOOST_AUTO_TEST_CASE(json_detelement_reading_test) { BOOST_REQUIRE_EQUAL(surfaces.size(), readBackDetElements.size()); for (auto [refSurface, detElement] : zip(surfaces, readBackDetElements)) { - auto surface = &detElement->surface(); + const auto* surface = detElement->surface(); BOOST_CHECK(refSurface->localToGlobalTransform(gctx).isApprox( surface->localToGlobalTransform(gctx), 1.e-4)); BOOST_CHECK(refSurface->localToGlobalTransform(gctx).isApprox(