Skip to content

Commit

Permalink
adressing first set of PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@asalzburger
asalzburger committed Aug 28, 2024
1 parent cc4ae17 commit d0037f0
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 49 deletions.
14 changes: 13 additions & 1 deletion Core/include/Acts/Navigation/NavigationStream.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,18 @@ struct NavigationStream {
const Surface& surface() const { return *intersection.object(); }
/// Cinvencience access to the path length
ActsScalar pathLength() const { return intersection.pathLength(); }

/// Order along the path length
///
/// @param aCandidate is the first candidate
/// @param bCandidate is the second candidate
///
/// @return true if aCandidate is closer to the origin
constexpr static bool pathLengthOrder(const Candidate& aCandidate,
const Candidate& bCandidate) {
return ObjectIntersection<Surface>::pathLengthOrder(
aCandidate.intersection, bCandidate.intersection);
}
};

/// The candidates of this navigation stream
Expand Down Expand Up @@ -90,7 +102,7 @@ struct NavigationStream {
Candidate& currentCandidate() { return candidates.at(currentIndex); }

/// The number of active candidates
std::size_t activeCandidates() const {
std::size_t remainingCandidates() const {
return (candidates.size() - currentIndex);
}
};
Expand Down
20 changes: 13 additions & 7 deletions Core/include/Acts/Navigation/NavigationStreamHelper.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@ inline static void fillSurface(NavigationStream& nStream,
const Surface* surface,
BoundaryTolerance bTolerance,
bool abortTarget = false) {
nStream.candidates.push_back(NavigationStream::Candidate{
ObjectIntersection<Surface>(surface), nullptr, bTolerance, abortTarget});
nStream.candidates.push_back(
NavigationStream::Candidate{ObjectIntersection<Surface>::invalid(surface),
nullptr, bTolerance, abortTarget});
}

/// Helper struct that allows to fill surfaces into the candidate vector
Expand All @@ -50,8 +51,9 @@ inline static void fillSurfaces(NavigationStream& nStream,
BoundaryTolerance bTolerance,
bool abortTarget = false) {
std::for_each(surfaces.begin(), surfaces.end(), [&](const auto& s) {
nStream.candidates.push_back(NavigationStream::Candidate{
ObjectIntersection<Surface>(s), nullptr, bTolerance, abortTarget});
nStream.candidates.push_back(
NavigationStream::Candidate{ObjectIntersection<Surface>::invalid(s),
nullptr, bTolerance, abortTarget});
});
}

Expand All @@ -63,7 +65,7 @@ inline static void fillPortals(NavigationStream& nStream,
const std::vector<const Portal*>& portals) {
std::for_each(portals.begin(), portals.end(), [&](const auto& p) {
nStream.candidates.push_back(NavigationStream::Candidate{
ObjectIntersection<Surface>(&(p->surface())), p,
ObjectIntersection<Surface>::invalid(&(p->surface())), p,
BoundaryTolerance::None()});
});
}
Expand All @@ -74,6 +76,7 @@ inline static void fillPortals(NavigationStream& nStream,
/// @param gctx is the geometry context
/// @param queryPoint holds current position, direction, etc.
/// @param cTolerance is the candidate search tolerance
/// @param onSurfaceTolerance is the tolerance for on-surface intersections
///
/// This method will first de-duplicate the candidates on basis of the surface
/// pointer to make sure that the multi-intersections are handled correctly.
Expand All @@ -83,7 +86,8 @@ inline static void fillPortals(NavigationStream& nStream,
/// @return true if the stream is active, false indicates that there are no valid candidates
bool initializeStream(NavigationStream& stream, const GeometryContext& gctx,
const NavigationStream::QueryPoint& queryPoint,
BoundaryTolerance cTolerance);
BoundaryTolerance cTolerance,
ActsScalar onSurfaceTolerance = s_onSurfaceTolerance);

/// Convenience method to update a stream from a new query point,
/// this could be called from navigation delegates that do not require
Expand All @@ -92,10 +96,12 @@ bool initializeStream(NavigationStream& stream, const GeometryContext& gctx,
/// @param stream [in, out] is the navigation stream to be updated
/// @param gctx is the geometry context
/// @param queryPoint holds current position, direction, etc.
/// @param onSurfaceTolerance is the tolerance for on-surface intersections
///
/// @return true if the stream is active, false indicate no valid candidates left
bool updateStream(NavigationStream& stream, const GeometryContext& gctx,
const NavigationStream::QueryPoint& queryPoint);
const NavigationStream::QueryPoint& queryPoint,
ActsScalar onSurfaceTolerance = s_onSurfaceTolerance);

} // namespace NavigationStreamHelper

Expand Down
13 changes: 4 additions & 9 deletions Core/include/Acts/Utilities/Intersection.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,6 @@ class ObjectIntersection {
const object_t* object, std::uint8_t index = 0)
: m_intersection(intersection), m_object(object), m_index(index) {}

/// Invalid object intersection - only holding the object itself
///
/// @param object is the object to be instersected
constexpr ObjectIntersection(const object_t* object)
: m_intersection(Intersection3D::invalid()),
m_object(object),
m_index(0) {}

/// Returns whether the intersection was successful or not
/// @deprecated
[[deprecated("Use isValid() instead")]] constexpr explicit operator bool()
Expand Down Expand Up @@ -192,7 +184,10 @@ class ObjectIntersection {

constexpr std::uint8_t index() const { return m_index; }

constexpr static ObjectIntersection invalid() { return ObjectIntersection(); }
constexpr static ObjectIntersection invalid(
const object_t* object = nullptr) {
return ObjectIntersection(Intersection3D::invalid(), object);
}

constexpr static bool pathLengthOrder(
const ObjectIntersection& aIntersection,
Expand Down
43 changes: 22 additions & 21 deletions Core/src/Navigation/NavigationStreamHelper.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
bool Acts::NavigationStreamHelper::initializeStream(
NavigationStream& stream, const GeometryContext& gctx,
const NavigationStream::QueryPoint& queryPoint,
BoundaryTolerance cTolerance) {
BoundaryTolerance cTolerance, ActsScalar onSurfaceTolerance) {
// Position and direction from the query point
const Vector3& position = queryPoint.position;
const Vector3& direction = queryPoint.direction;
Expand All @@ -34,46 +34,46 @@ bool Acts::NavigationStreamHelper::initializeStream(
}),
stream.candidates.end());

// A container collecting additional valid ones from multi intersections
std::vector<NavigationStream::Candidate> miCandidates = {};
// A container collecting additional candidates from multiple
// valid interseciton
std::vector<NavigationStream::Candidate> additionalCandidates = {};
for (auto& [sIntersection, portal, bTolerance, abortTarget] :
stream.candidates) {
// Get the surface from the object intersection
const Surface* surface = sIntersection.object();
// Intersect the surface
auto multiIntersection = surface->intersect(
gctx, position, direction, cTolerance, s_onSurfaceTolerance);
auto multiIntersection = surface->intersect(gctx, position, direction,
cTolerance, onSurfaceTolerance);

// Split them into valid intersections
bool miCandidate = false;
// Split them into valid intersections, keep track of potentially
// additional candidates
bool originalCandidateUpdated = false;
for (auto& rsIntersection : multiIntersection.split()) {
// Skip negative solutions
if (rsIntersection.pathLength() < 0.) {
// Skip negative solutions, respecting the on surface tolerance
if (rsIntersection.pathLength() < -onSurfaceTolerance) {
continue;
}
// Valid solution is either on surface or updates the distance
if (rsIntersection.isValid()) {
if (!miCandidate) {
if (!originalCandidateUpdated) {
sIntersection = rsIntersection;
miCandidate = true;
originalCandidateUpdated = true;
} else {
miCandidates.push_back(
additionalCandidates.push_back(
NavigationStream::Candidate{rsIntersection, portal, bTolerance});
}
}
}
}

// Append the multi intersection candidates
stream.candidates.insert(stream.candidates.end(), miCandidates.begin(),
miCandidates.end());
stream.candidates.insert(stream.candidates.end(),
additionalCandidates.begin(),
additionalCandidates.end());

// Sort the candidates by path length
std::sort(stream.candidates.begin(), stream.candidates.end(),
[](const NavigationStream::Candidate& a,
const NavigationStream::Candidate& b) {
return a.intersection.pathLength() < b.intersection.pathLength();
});
NavigationStream::Candidate::pathLengthOrder);

// The we find the first invalid candidate
auto firstInvalid = std::find_if(
Expand All @@ -96,7 +96,8 @@ bool Acts::NavigationStreamHelper::initializeStream(

bool Acts::NavigationStreamHelper::updateStream(
NavigationStream& stream, const GeometryContext& gctx,
const NavigationStream::QueryPoint& queryPoint) {
const NavigationStream::QueryPoint& queryPoint,
ActsScalar onSurfaceTolerance) {
// Position and direction from the query point
const Vector3& position = queryPoint.position;
const Vector3& direction = queryPoint.direction;
Expand All @@ -110,8 +111,8 @@ bool Acts::NavigationStreamHelper::updateStream(
// Get the surface from the object intersection
const Surface* surface = sIntersection.object();
// (re-)Intersect the surface
auto multiIntersection = surface->intersect(
gctx, position, direction, bTolerance, s_onSurfaceTolerance);
auto multiIntersection = surface->intersect(gctx, position, direction,
bTolerance, onSurfaceTolerance);
// Split them into valid intersections
for (auto& rsIntersection : multiIntersection.split()) {
// Skip wrong index solution
Expand Down
22 changes: 11 additions & 11 deletions Tests/UnitTests/Core/Navigation/NavigationStreamTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ BOOST_AUTO_TEST_CASE(NavigationStream_InitializePlanes) {
nStream, gContext, {Vector3(0., 0., -30.), Vector3(0., 0., 1.)},
BoundaryTolerance::Infinite()));

BOOST_CHECK_EQUAL(nStream.activeCandidates(), 4u);
BOOST_CHECK_EQUAL(nStream.remainingCandidates(), 4u);
BOOST_CHECK_EQUAL(&nStream.currentCandidate().surface(), surfaces[1u].get());

// (2) Run an initial update
Expand All @@ -118,7 +118,7 @@ BOOST_AUTO_TEST_CASE(NavigationStream_InitializePlanes) {
BOOST_CHECK(NavigationStreamHelper::initializeStream(
nStream, gContext, {Vector3(0., 0., 0.), Vector3(0., 0., 1.)},
BoundaryTolerance::Infinite()));
BOOST_CHECK_EQUAL(nStream.activeCandidates(), 3u);
BOOST_CHECK_EQUAL(nStream.remainingCandidates(), 3u);
BOOST_CHECK_EQUAL(&nStream.currentCandidate().surface(), surfaces[3u].get());

// (3) Run an initial update
Expand All @@ -128,15 +128,15 @@ BOOST_AUTO_TEST_CASE(NavigationStream_InitializePlanes) {
BOOST_CHECK(NavigationStreamHelper::initializeStream(
nStream, gContext, {Vector3(0., 0., -100.), Vector3(0., 0., 1.)},
BoundaryTolerance::None()));
BOOST_CHECK_EQUAL(nStream.activeCandidates(), 3u);
BOOST_CHECK_EQUAL(nStream.remainingCandidates(), 3u);

// (4) Run an initial update
// - none of the surfaces should be reachable
nStream = nStreamTemplate;
BOOST_CHECK(!NavigationStreamHelper::initializeStream(
nStream, gContext, {Vector3(0., 0., 0.), Vector3(1., 0., 0.)},
BoundaryTolerance::Infinite()));
BOOST_CHECK_EQUAL(nStream.activeCandidates(), 0u);
BOOST_CHECK_EQUAL(nStream.remainingCandidates(), 0u);
BOOST_CHECK_THROW(nStream.currentCandidate(), std::out_of_range);

// (5) Test de-duplication
Expand All @@ -149,7 +149,7 @@ BOOST_AUTO_TEST_CASE(NavigationStream_InitializePlanes) {
BOOST_CHECK(NavigationStreamHelper::initializeStream(
nStream, gContext, {Vector3(0., 0., -100.), Vector3(0., 0., 1.)},
BoundaryTolerance::Infinite()));
BOOST_CHECK_EQUAL(nStream.activeCandidates(), 4u);
BOOST_CHECK_EQUAL(nStream.remainingCandidates(), 4u);
}

BOOST_AUTO_TEST_CASE(NavigationStream_UpdatePlanes) {
Expand All @@ -174,7 +174,7 @@ BOOST_AUTO_TEST_CASE(NavigationStream_UpdatePlanes) {
NavigationStream nStream = nStreamTemplate;
BOOST_CHECK(NavigationStreamHelper::initializeStream(
nStream, gContext, qPoint, BoundaryTolerance::Infinite()));
BOOST_CHECK_EQUAL(nStream.activeCandidates(), 4u);
BOOST_CHECK_EQUAL(nStream.remainingCandidates(), 4u);
BOOST_CHECK_EQUAL(&nStream.currentCandidate().surface(), surfaces[1u].get());
CHECK_CLOSE_ABS(nStream.currentCandidate().pathLength(), 10.,
std::numeric_limits<ActsScalar>::epsilon());
Expand Down Expand Up @@ -235,7 +235,7 @@ BOOST_AUTO_TEST_CASE(NavigationStream_InitializeCylinders) {
NavigationStreamHelper::fillSurfaces(nStreamTemplate, {surface.get()},
Acts::BoundaryTolerance::None());
}
BOOST_CHECK_EQUAL(nStreamTemplate.activeCandidates(), 4u);
BOOST_CHECK_EQUAL(nStreamTemplate.remainingCandidates(), 4u);

// (1) Run an initial update - from a position/direction where all are
// reachable
Expand All @@ -246,7 +246,7 @@ BOOST_AUTO_TEST_CASE(NavigationStream_InitializeCylinders) {
{Vector3(0., 0., 0.), Vector3(1., 1., 0.).normalized()},
BoundaryTolerance::Infinite()));
// We should have 5 candidates, as one cylinder is reachable twice
BOOST_CHECK_EQUAL(nStream.activeCandidates(), 5u);
BOOST_CHECK_EQUAL(nStream.remainingCandidates(), 5u);
// First one is inner candidate
BOOST_CHECK_EQUAL(&nStream.currentCandidate().surface(), surfaces[2].get());
// Surface of 2nd and 3rd candidate should be the same
Expand All @@ -261,7 +261,7 @@ BOOST_AUTO_TEST_CASE(NavigationStream_InitializeCylinders) {
nStream, gContext, {Vector3(0., 0., 0.), Vector3(1., 0., 0.)},
BoundaryTolerance::Infinite()));
// We should have 3 candidates
BOOST_CHECK_EQUAL(nStream.activeCandidates(), 3u);
BOOST_CHECK_EQUAL(nStream.remainingCandidates(), 3u);

// (3) Run an initial update - from a position/direction where only the
// concentric ones within bounds are reachable
Expand All @@ -270,7 +270,7 @@ BOOST_AUTO_TEST_CASE(NavigationStream_InitializeCylinders) {
nStream, gContext, {Vector3(0., 0., 0.), Vector3(1., 0., 0.)},
BoundaryTolerance::None()));
// We should have 2 candidates
BOOST_CHECK_EQUAL(nStream.activeCandidates(), 2u);
BOOST_CHECK_EQUAL(nStream.remainingCandidates(), 2u);

// (4) Run an initial update - from a position/direction where none are
// reachable
Expand All @@ -280,7 +280,7 @@ BOOST_AUTO_TEST_CASE(NavigationStream_InitializeCylinders) {
nStream, gContext, {Vector3(0., 0., 0.), Vector3(0., 0., 1.)},
BoundaryTolerance::None()));
// We should have 0 candidates
BOOST_CHECK_EQUAL(nStream.activeCandidates(), 0u);
BOOST_CHECK_EQUAL(nStream.remainingCandidates(), 0u);
BOOST_CHECK_THROW(nStream.currentCandidate(), std::out_of_range);
}

Expand Down

0 comments on commit d0037f0

Please sign in to comment.