Skip to content
Snippets Groups Projects

TrkSurfaces: Fix bugs in CylinderSurface::straightLineIntersection.

Merged TrkSurfaces: Fix bugs in CylinderSurface::straightLineIntersection.
ssnyder/athena:cylIntersect.TrkSurfaces-20190129 into master
Merged Scott Snyder requested to merge ssnyder/athena:cylIntersect.TrkSurfaces-20190129 into master

CylinderSurface::straightLineIntersection has a couple serious bugs that appear to have been there since it was first written.

We had

    if (needsTransform){
        Amg::Transform3D invTrans = transform().inverse();
        point1    = invTrans*pos;
        direction = invTrans.linear()*dir;
    }
    Amg::Vector3D point2 = point1 + dir;

where point1 and point2 are meant to be points along the supplied line, after the reference frame transform. But the last line here uses the direction vector before the transform, rather than after. It should use direction', rather than dir'.

But, in fact, the expression

      double d = (point2.x()*point1.y() - point1.x()*point2.y())/(point2.x()-point1.x());

can be written more efficiently as

      double d = point1.y() - point1.x()*k;

in which case, we no longer need the point2 variable. Writing it this way probably also has better precision, since we avoid subtracting large numbers that are close together.

Further, in the section handling the case where direction.x()==0, we have

        t1 = y-point1.y();
        t2 = -y-point1.y();

which is not correct, since t1 and t2 are meant to be the 3D distances to the intersection; what we have here is the 2D projection. We instead need to do

        t1 = (y-point1.y())/direction.y();
        t2 = (-y-point1.y())/direction.y();

The code also does not handle the case when both direction.x() and direction.y() are 0.

Finally, we can change some of the repeated divisions by the same value to multiplication by a reciprocal.

Also add a rudimentary unit test for this functionality.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
Please register or sign in to reply