Skip to content

ACTS-266 rewrite BoundaryChecks

Moritz Kiehn requested to merge msmk-boundaries into master

This cleans up the boundary checks classes. The general idea is to move most of the check logic into the BoundaryCheck class. The concrete bounds only need to define the boundary shape, i.e. a box or polygon, and everything else is done automatically by the BoundaryCheck class. This ended up being a major modification and should probably be reviewed as a rewrite.

The following changes are implemented:

  • Add generic implementations of inside check and distance calculation for box-like and polygonal boundaries to BoundaryCheck and remove low-level helper methods for KDOP-based checks.
  • Automatically use either tolerance or covariance based methods depending on the BoundaryCheck configuration.
  • Replace concrete bounds-checking implementations w/ calls to the generic implementations in BoundaryCheck wherever possible. This enables support for the different boundary check methods, e.g. tolerance or covariance, in almost all concrete bounds classes.
  • Removal of .insideLoc0(...) and .insideLoc1(...) interfaces in favor of e.g. using the BoundaryCheck w/ configurable tolerances in either coordinate. Depending on the boundary, having separate checks for just one of two coordinates can be ill-defined and should therefore not be part of the common interface.
  • Convert the SurfaceBounds, PlanarBounds, and DiscBounds classes to pure interface classes w/o any implementation or data, and simplify there signatures. Common constructors and assignment operators have been removed.
  • Move all virtual method implementations to the corresponding cpp files. They were previously implemented as inline methods.
  • Replace custom approximate sin/cos implementations w/ std::sin, std::cos.

There are a few open questions that have not been addressed:

  • DiamondBounds can only be created when the shape is convex. Support for concave shapes was removed. It is not used anywhere, but I'm not sure if that is ok.
  • DiscTrapezoidalBounds takes a stereo angle as parameter. However, this angle is never used. Is this is a bug or can we remove this parameter.
  • EllipseBounds is the only non-polygonal boundary and requires a custom implementation. This is not yet done and the current version only works for tolerance based checks.
  • The .clone() method seems to be unused. With the switch to a pure SurfaceBounds interface it probably is not required since a concrete bounds type should always be used during construction time.

Fixes ACTS-266.

Merge request reports

Loading