Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Do not allocate memory if the surface is connected with a detector el… #3069

Merged
merged 15 commits into from
Jul 3, 2024
4 changes: 2 additions & 2 deletions Core/include/Acts/Surfaces/Surface.hpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// This file is part of the Acts project.
//
// Copyright (C) 2016-2020 CERN for the benefit of the Acts project
// Copyright (C) 2016-2024 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
Expand Down Expand Up @@ -479,7 +479,7 @@ class Surface : public virtual GeometryObject,
protected:
/// Transform3 definition that positions
/// (translation, rotation) the surface in global space
Transform3 m_transform = Transform3::Identity();
std::unique_ptr<const Transform3> m_transform{};

/// Pointer to the a DetectorElementBase
const DetectorElementBase* m_associatedDetElement{nullptr};
Expand Down
3 changes: 2 additions & 1 deletion Core/src/Geometry/CylinderLayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ Acts::CylinderLayer::CylinderLayer(
auto cVolumeBounds = std::make_shared<CylinderVolumeBounds>(
*CylinderSurface::m_bounds, thickness);
// @todo rotate around x for the avePhi if you have a sector
m_representingVolume = std::make_unique<Volume>(m_transform, cVolumeBounds);
Transform3 volTrf = m_transform ? *m_transform : Transform3::Identity();
m_representingVolume = std::make_unique<Volume>(volTrf, cVolumeBounds);
junggjo9 marked this conversation as resolved.
Show resolved Hide resolved

// associate the layer to the surface
CylinderSurface::associateLayer(*this);
Expand Down
4 changes: 3 additions & 1 deletion Core/src/Geometry/DiscLayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ Acts::DiscLayer::DiscLayer(const Transform3& transform,
auto rVolumeBounds =
std::make_shared<CylinderVolumeBounds>(*rBounds, thickness);
// @todo rotate around x for the avePhi if you have a sector
m_representingVolume = std::make_unique<Volume>(m_transform, rVolumeBounds);
Transform3 volTrf = m_transform ? *m_transform : Transform3::Identity();
m_representingVolume =
std::make_unique<Volume>(std::move(volTrf), rVolumeBounds);
junggjo9 marked this conversation as resolved.
Show resolved Hide resolved
}
// associate the layer to the layer surface itself
DiscSurface::associateLayer(*this);
Expand Down
3 changes: 2 additions & 1 deletion Core/src/Surfaces/PlaneSurface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ Acts::PlaneSurface::PlaneSurface(const GeometryContext& gctx,

Acts::PlaneSurface::PlaneSurface(const Vector3& center, const Vector3& normal)
: RegularSurface(), m_bounds(nullptr) {
m_transform = CurvilinearSurface(center, normal).transform();
m_transform = std::make_unique<Transform3>(
CurvilinearSurface(center, normal).transform());
}

Acts::PlaneSurface::PlaneSurface(std::shared_ptr<const PlanarBounds> pbounds,
Expand Down
26 changes: 18 additions & 8 deletions Core/src/Surfaces/Surface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,26 @@ std::array<std::string, Acts::Surface::SurfaceType::Other>
"Cone", "Cylinder", "Disc", "Perigee", "Plane", "Straw", "Curvilinear"};

Acts::Surface::Surface(const Transform3& transform)
: GeometryObject(), m_transform(transform) {}
: GeometryObject(), m_transform(std::make_unique<Transform3>(transform)) {}

Acts::Surface::Surface(const DetectorElementBase& detelement)
: GeometryObject(), m_associatedDetElement(&detelement) {}

Acts::Surface::Surface(const Surface& other)
: GeometryObject(other),
std::enable_shared_from_this<Surface>(),
m_transform(other.m_transform),
m_surfaceMaterial(other.m_surfaceMaterial) {}

junggjo9 marked this conversation as resolved.
Show resolved Hide resolved
m_associatedDetElement(other.m_associatedDetElement),
m_surfaceMaterial(other.m_surfaceMaterial) {
if (other.m_transform) {
m_transform = std::make_unique<Transform3>(*other.m_transform);
}
andiwand marked this conversation as resolved.
Show resolved Hide resolved
}

Acts::Surface::Surface(const GeometryContext& gctx, const Surface& other,
const Transform3& shift)
: GeometryObject(),
m_transform(shift * other.transform(gctx)),
m_transform(std::make_unique<Transform3>(shift * other.transform(gctx))),
m_surfaceMaterial(other.m_surfaceMaterial) {}

Acts::Surface::~Surface() = default;
Expand Down Expand Up @@ -156,7 +161,11 @@ Acts::Surface& Acts::Surface::operator=(const Surface& other) {
if (&other != this) {
GeometryObject::operator=(other);
// detector element, identifier & layer association are unique
m_transform = other.m_transform;
if (other.m_transform) {
m_transform = std::make_unique<Transform3>(*other.m_transform);
} else {
m_transform.reset();
andiwand marked this conversation as resolved.
Show resolved Hide resolved
}
m_associatedLayer = other.m_associatedLayer;
m_surfaceMaterial = other.m_surfaceMaterial;
m_associatedDetElement = other.m_associatedDetElement;
Expand All @@ -182,7 +191,8 @@ bool Acts::Surface::operator==(const Surface& other) const {
return false;
}
// (e) compare transform values
if (!m_transform.isApprox(other.m_transform, 1e-9)) {
if (m_transform && other.m_transform &&
!m_transform->isApprox((*other.m_transform), 1e-9)) {
return false;
}
// (f) compare material
Expand Down Expand Up @@ -240,7 +250,7 @@ const Acts::Transform3& Acts::Surface::transform(
if (m_associatedDetElement != nullptr) {
return m_associatedDetElement->transform(gctx);
}
return m_transform;
return (*m_transform);
junggjo9 marked this conversation as resolved.
Show resolved Hide resolved
}

bool Acts::Surface::insideBounds(const Vector2& lposition,
Expand Down Expand Up @@ -337,7 +347,7 @@ void Acts::Surface::assignDetectorElement(
m_associatedDetElement = &detelement;
// resetting the transform as it will be handled through the detector element
// now
m_transform = Transform3::Identity();
m_transform.reset();
}

void Acts::Surface::assignSurfaceMaterial(
Expand Down
Loading