-
Notifications
You must be signed in to change notification settings - Fork 4
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
Support arbitrary readout orientation #85
base: master
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
TVector2 rot = coords.Rotate(-fModuleRotation * TMath::Pi() / 180.); | ||
|
||
return rot; | ||
return p - TVector2(fModuleOriginX, fModuleOriginY); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why rotation has disappeared?
|
||
return coords; | ||
inline TVector2 TransformToReadoutCoordinates(const TVector2& p) { | ||
return p + TVector2(fModuleOriginX, fModuleOriginY); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rotation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be TransformToModuleCoordinates
and TransformToPlaneCoordinates
. The module coordinates define the (0,0) at the left-bottom corner.
TVector2 GetPhysicalCoordinates(const TVector2& p) { | ||
return TransformToPhysicalCoordinates(p.X(), p.Y()); | ||
} | ||
TVector2 GetPhysicalCoordinates(const TVector2& p) { return TransformToReadoutCoordinates(p); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be GetPlaneCoordinates
now.
TVector3 fPlaneAxisX; ///< The first coordinate vector of the plane in absolute coordinates. | ||
/// < This vector is contained in the plane and corresponds to the first local | ||
/// coordinate (1,0) | ||
TVector3 fPlaneAxisY; ///< The first coordinate vector of the plane in absolute coordinates. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no need for fPlaneAxisY
, this vector is the result of fPlaneAxisX
times fPlaneVector
.
Also we said to rename them to fAxisY
fNormal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the Y vector is the second coordinate vector in the plane, it's true that it is redundant as it can be obtained via the cross product (but it's different from the normal).
return GetModuleByID(mod)->GetDistanceToModule(pos); | ||
} | ||
|
||
TVector2 GetPositionInReadoutPlane(const TVector3& position) const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must be renamed to GetPosition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see now it is not the position, but the projection. So the method could be named GetProjectedCoordinates
.
@@ -143,13 +158,11 @@ class TRestDetectorReadoutPlane : public TObject { | |||
|
|||
Int_t isZInsideDriftVolume(Double_t z); | |||
|
|||
Int_t isZInsideDriftVolume(TVector3 pos); | |||
Int_t isZInsideDriftVolume(const TVector3& pos); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method must be updated, perhaps now the method should be named IsBetweenCathodeAndReadout
I will update everything later today and fix the conflicts |
Currently the readout plane only works correctly if the orientation is in the z direction.