feat(geometry): Allow operator<< on surfaces without detector elements #3467
Conversation
We previously disallowed this, but this is only necessary if we have a detector element. Arguably, specifically for printing it can sometimes be inconvenient to have to pass around a geometry context just for this.
|
This is convenient, but might not be 100% clean in all cases. Thoughts @andiwand, @benjaminhuth, @asalzburger ? |
| // 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); |
There was a problem hiding this comment.
I wonder if there should also be a toStream overload without context which then drives the exception also allowing for overrides later
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
|
I guess this could be surprising using the |
|
@andiwand That's exactly correct. I'm debating myself if the trade-off of confusion is worth it. |
We previously disallowed this, but this is only necessary if we have a detector element. Arguably, specifically for printing it can sometimes be inconvenient to have to pass around a geometry context just for this.