-
Notifications
You must be signed in to change notification settings - Fork 102
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
Root interface changes #428
Conversation
@mrodozov - FYI |
const Double_t* t = matrix.GetTranslation(); | ||
if ( matrix.IsRotation() ) { | ||
const Double_t* r = matrix.GetRotationMatrix(); | ||
return Transform3D(r[0],r[1],r[2],t[0]*MM_2_CM, |
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 are a lot of conversions here. It looks like the MM_2_CM and RAD_2_DEGREE assume that the input units are mm and rad. Should it be a user who defines which units to use and if they are not the DD4hep default, convert them?
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 user should always use DD4hep units, when addressing/talking to DD4hep. There some exceptions to this rule, e.g. when interfaces of underlying classes are exposed, there you should use or expect ROOT or Geant4 conventions.
Updates are fine. We requested them in ROOT, |
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.
in DDCore/src/plugins/LCDDConverter.cpp
I would simply put:
TGeoMatrix linv = lm->Inverse();
and
TGeoMatrix rinv = rm->Inverse();
Modern compilers are efficient on these statements. There should be no need to use a reference.
BEGINRELEASENOTES
TGeoMatrix::Inverse
(Matrix updates root-project/root#2365), fixes ROOT TGeoMatrix interface change #426ENDRELEASENOTES