Skip to content

Commit

Permalink
feat!: add non-cost access to Surface (#2072)
Browse files Browse the repository at this point in the history
This PR adds a non-const access to the associated `Surface` to the
detector element interface and all derivates.

```c++
virtual Surface& surface() = 0;
```

Is now required to be implemented. It will allow for full const
correctness of the new `Detector` geometry, having `non-const` object
while detector creation (which will in turn allow for loading of
material and setting of `GeometryID` and other associations, but require
strict `const correctness` after geometry construction.

It removes already some now unnecessary `const_pointer_cast` or
diversions via `MutableSurfacePtr` in the examples.
  • Loading branch information
asalzburger committed Apr 28, 2023
1 parent e702f16 commit b7afb6b
Show file tree
Hide file tree
Showing 10 changed files with 60 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,12 @@ class DetectorElementBase {
/// @param gctx The current geometry context object, e.g. alignment
virtual const Transform3& transform(const GeometryContext& gctx) const = 0;

/// Return surface representation
/// Return surface representation - const return pattern
virtual const Surface& surface() const = 0;

/// Non-const return pattern
virtual Surface& surface() = 0;

/// Returns the thickness of the module
/// @return double that indicates the thickness of the module
virtual double thickness() const = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ class GenericDetectorElement : public Acts::IdentifiedDetectorElement {
/// Return surface associated with this detector element
const Acts::Surface& surface() const override;

/// Non-cost access to surface associated with this detector element
Acts::Surface& surface() override;

/// Set the identifier after construction (sometimes needed)
void assignIdentifier(const Identifier& identifier);

Expand All @@ -100,7 +103,7 @@ class GenericDetectorElement : public Acts::IdentifiedDetectorElement {
/// the transform for positioning in 3D space
std::shared_ptr<const Acts::Transform3> m_elementTransform;
/// the surface represented by it
std::shared_ptr<const Acts::Surface> m_elementSurface;
std::shared_ptr<Acts::Surface> m_elementSurface;
/// the element thickness
double m_elementThickness;
/// store either
Expand Down Expand Up @@ -132,6 +135,10 @@ ActsExamples::Generic::GenericDetectorElement::surface() const {
return *m_elementSurface;
}

inline Acts::Surface& ActsExamples::Generic::GenericDetectorElement::surface() {
return *m_elementSurface;
}

inline double ActsExamples::Generic::GenericDetectorElement::thickness() const {
return m_elementThickness;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ ActsExamples::Generic::GenericDetectorElement::GenericDetectorElement(
m_elementPlanarBounds(std::move(pBounds)),
m_elementDiscBounds(nullptr),
m_digitizationModule(std::move(digitizationModule)) {
auto mutableSurface =
std::const_pointer_cast<Acts::Surface>(m_elementSurface);
mutableSurface->assignSurfaceMaterial(std::move(material));
m_elementSurface->assignSurfaceMaterial(std::move(material));
}

ActsExamples::Generic::GenericDetectorElement::GenericDetectorElement(
Expand All @@ -48,7 +46,5 @@ ActsExamples::Generic::GenericDetectorElement::GenericDetectorElement(
m_elementPlanarBounds(nullptr),
m_elementDiscBounds(std::move(dBounds)),
m_digitizationModule(std::move(digitizationModule)) {
auto mutableSurface =
std::const_pointer_cast<Acts::Surface>(m_elementSurface);
mutableSurface->assignSurfaceMaterial(std::move(material));
m_elementSurface->assignSurfaceMaterial(std::move(material));
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ class TelescopeDetectorElement : public Acts::DetectorElementBase {
/// 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;

/// The maximal thickness of the detector element wrt normal axis
double thickness() const final;

Expand Down Expand Up @@ -97,7 +100,7 @@ class TelescopeDetectorElement : public Acts::DetectorElementBase {
// the aligned transforms
std::vector<std::unique_ptr<Acts::Transform3>> m_alignedTransforms = {};
/// the surface represented by it
std::shared_ptr<const Acts::Surface> m_elementSurface = nullptr;
std::shared_ptr<Acts::Surface> m_elementSurface = nullptr;
/// the element thickness
double m_elementThickness = 0.;
/// the planar bounds
Expand All @@ -110,6 +113,10 @@ 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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,5 @@ ActsExamples::Telescope::TelescopeDetectorElement::TelescopeDetectorElement(
m_elementThickness(thickness),
m_elementPlanarBounds(nullptr),
m_elementDiscBounds(std::move(dBounds)) {
auto mutableSurface =
std::const_pointer_cast<Acts::Surface>(m_elementSurface);
mutableSurface->assignSurfaceMaterial(std::move(material));
m_elementSurface->assignSurfaceMaterial(std::move(material));
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ class Geant4DetectorElement : public DetectorElementBase {
/// Return surface associated with this detector element
const Surface& surface() const override;

/// Non-const access to surface associated with this detector element
Surface& surface() override;

/// Return the thickness of this detector element
ActsScalar thickness() const override;

Expand Down
4 changes: 4 additions & 0 deletions Plugins/Geant4/src/Geant4DetectorElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ const Acts::Surface& Acts::Geant4DetectorElement::surface() const {
return *m_surface;
}

Acts::Surface& Acts::Geant4DetectorElement::surface() {
return *m_surface;
}

Acts::ActsScalar Acts::Geant4DetectorElement::thickness() const {
return m_thickness;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ class TGeoDetectorElement : public IdentifiedDetectorElement {
/// Return surface associated with this detector element
const Surface& surface() const override;

/// Return surface associated with this detector element
///
/// @note this is the non-const access
Surface& surface() override;

/// Retrieve the DigitizationModule
const std::shared_ptr<const DigitizationModule> digitizationModule()
const final {
Expand Down Expand Up @@ -154,6 +159,10 @@ inline const Surface& TGeoDetectorElement::surface() const {
return (*m_surface);
}

inline Surface& TGeoDetectorElement::surface() {
return (*m_surface);
}

inline double TGeoDetectorElement::thickness() const {
return m_thickness;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,8 @@ class DetectorElementStub : public DetectorElementBase {
: DetectorElementBase(),
m_elementTransform(transform),
m_elementThickness(thickness) {
auto mutableSurface = Surface::makeShared<PlaneSurface>(pBounds, *this);
mutableSurface->assignSurfaceMaterial(std::move(material));
m_elementSurface = mutableSurface;
m_elementSurface = Surface::makeShared<PlaneSurface>(pBounds, *this);
m_elementSurface->assignSurfaceMaterial(std::move(material));
}

/// Constructor for single sided detector element
Expand All @@ -71,9 +70,8 @@ class DetectorElementStub : public DetectorElementBase {
: DetectorElementBase(),
m_elementTransform(transform),
m_elementThickness(thickness) {
auto mutableSurface = Surface::makeShared<LineSurfaceStub>(lBounds, *this);
mutableSurface->assignSurfaceMaterial(std::move(material));
m_elementSurface = mutableSurface;
m_elementSurface = Surface::makeShared<LineSurfaceStub>(lBounds, *this);
m_elementSurface->assignSurfaceMaterial(std::move(material));
}

/// Destructor
Expand All @@ -89,14 +87,17 @@ class DetectorElementStub : public DetectorElementBase {
/// Return surface associated with this detector element
const Surface& surface() const override;

/// Non-const access to surface associated with this detector element
Surface& surface() override;

/// The maximal thickness of the detector element wrt normal axis
double thickness() const override;

private:
/// the transform for positioning in 3D space
Transform3 m_elementTransform;
/// the surface represented by it
std::shared_ptr<const Surface> m_elementSurface{nullptr};
std::shared_ptr<Surface> m_elementSurface{nullptr};
/// the element thickness
double m_elementThickness{0.};
};
Expand All @@ -110,6 +111,10 @@ inline const Surface& DetectorElementStub::surface() const {
return *m_elementSurface;
}

inline Surface& DetectorElementStub::surface() {
return *m_elementSurface;
}

inline double DetectorElementStub::thickness() const {
return m_elementThickness;
}
Expand Down
12 changes: 9 additions & 3 deletions Tests/UnitTests/Core/Geometry/AlignmentContextTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ class AlignableDetectorElement : public DetectorElementBase {
: DetectorElementBase(),
m_elementTransform(std::move(transform)),
m_elementThickness(thickness) {
auto mutableSurface = Surface::makeShared<PlaneSurface>(pBounds, *this);
m_elementSurface = mutableSurface;
m_elementSurface = Surface::makeShared<PlaneSurface>(pBounds, *this);
}

/// Destructor
Expand All @@ -79,14 +78,17 @@ class AlignableDetectorElement : public DetectorElementBase {
/// 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;

/// The maximal thickness of the detector element wrt normal axis
double thickness() const override;

private:
/// the transform for positioning in 3D space
std::shared_ptr<const Transform3> m_elementTransform;
/// the surface represented by it
std::shared_ptr<const Surface> m_elementSurface{nullptr};
std::shared_ptr<Surface> m_elementSurface{nullptr};
/// the element thickness
double m_elementThickness{0.};
};
Expand All @@ -105,6 +107,10 @@ inline const Surface& AlignableDetectorElement::surface() const {
return *m_elementSurface;
}

inline Surface& AlignableDetectorElement::surface() {
return *m_elementSurface;
}

inline double AlignableDetectorElement::thickness() const {
return m_elementThickness;
}
Expand Down

0 comments on commit b7afb6b

Please sign in to comment.