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.

Sign up for GitHub

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

feat(geometry): Allow operator<< on surfaces without detector elements #3467

Closed
wants to merge 1 commit into from
Closed

feat(geometry): Allow operator<< on surfaces without detector elements #3467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Core/include/Acts/Surfaces/Surface.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,14 @@ class Surface : public virtual GeometryObject,
virtual ActsMatrix<2, 3> localCartesianToBoundLocalDerivative(
const GeometryContext& gctx, const Vector3& position) const = 0;

/// Outstream operator for surfaces without an associated detector element
/// @note This operator checks if an associated detector element is set
/// and will throws an exception if that's the case.
/// @param os The output stream
/// @param srf The surface to print
/// @return The output stream
friend std::ostream& operator<<(std::ostream& os, const Surface& srf);

protected:
/// Output Method for std::ostream, to be overloaded by child classes
///
Expand Down
13 changes: 13 additions & 0 deletions Core/src/Surfaces/Surface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,3 +357,16 @@ void Acts::Surface::assignSurfaceMaterial(
void Acts::Surface::associateLayer(const Acts::Layer& lay) {
m_associatedLayer = (&lay);
}

std::ostream& Acts::operator<<(std::ostream& os, const Acts::Surface& srf) {
if (srf.associatedDetectorElement() != nullptr) {
throw std::runtime_error(
"Cannot print Surface with associated DetectorElement without geometry "
"context");
}
// Using a default context is ONLY safe, if the surface does not have an
// associated detector element, which we ensure right above.
Acts::GeometryContext defaultContext;
os << srf.toStream(defaultContext);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there should also be a toStream overload without context which then drives the exception also allowing for overrides later

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requires implementing it on all derived surface types. I'm not sure that's needed, and they would likely also call each other internally to avoid copying code. Does that make sense?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we could default it to your behavior here and give the surface the opportunity to specialize but maybe this does not have any use case anyways

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point. I guess the only reason using toStream over operator<< is that with toStream you can in fact pass a geometry context.

return os;
}
Loading