Skip to content
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

Adds ray Bezier/NURBS surface intersection routines #1477

Open
wants to merge 48 commits into
base: develop
Choose a base branch
from

Conversation

jcs15c
Copy link
Contributor

@jcs15c jcs15c commented Dec 9, 2024

Summary

  • This PR adds various intersection routines between rays and Bezier/NURBS curves/surfaces, along with relevant tests. NURBS intersection routines first decompose patches into Bezier objects, from which intersections are recorded through recursive subdivision. Segments are used as the base case for curves, and bilinear patches are used as the base case for surfaces.
    • intersect(Ray2D, BezierCurve)
    • intersect(Ray2D, NURBSCurve)
    • intersect(Ray3D, BezierPatch)
    • intersect(Ray3D, NURBSPatch)
  • Additionally, the following capabilities have been added to facilitate the above functions
    • A primal::Line class analogous to primal::Ray but extends in either direction from the origin. This will reduce redundant calculation in later winding number methods.
    • A primal::BezierPatch::isBilinear() method that returns if a surface is approximately bilinear according to its control points.
    • Many drop in replacements of std::vector with axom::Array.
    • A few extra methods in KnotVector to simplify iterating over knot spans.
    • A fix to intersect(Ray2D, Segment) to catch some additional edge cases
  • Additionally fixes an error in a few winding_number calls.

@jcs15c jcs15c added enhancement New feature or request Primal Issues related to Axom's 'primal component labels Dec 9, 2024
@jcs15c jcs15c requested a review from kennyweiss December 9, 2024 01:41
@jcs15c jcs15c self-assigned this Dec 9, 2024
@jcs15c jcs15c marked this pull request as ready for review December 12, 2024 23:47
Copy link

@Arlie-Capps Arlie-Capps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, Jacob!

Copy link
Member

@kennyweiss kennyweiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jcs15c for this excellent contribution with extensive testing.

Please don't forget to update the RELEASE_NOTES with this new contribution and with the bugfix for the Ray-Segment intersection.

* \param [in] L the specified line (two-sided ray)
* \param [in] bb the user-supplied axis-aligned bounding box
*
* \param [out] ip the intersection point where L intersects bb.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please improve this comment.

Presumably, in general position, a line will intersect a box along a (possibly empty) line segment.
Should this parameter be a segment?

Alternatively, would the user only be interested in a (single) point of intersection? If so, does this function make any guarantees about which intersection point this is?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method follows the analogous ray-bb intersection routine, which returns the intersection with the minimum t value as the returned point. While doing this in the line case isn't quite as conceptually straightforward, since that parameter will be negative when the origin is interior to the bounding box, it's consistent, and does serve as some kind of guarantee.

At the same time, my own usage of this method doesn't even use the point of intersection at all, so it's sort of a waste to compute it. With this in mind, I'll update the comment to reflect the behavior of this method, but also write a new one that doesn't compute the intersection in physical space.

src/axom/primal/geometry/BezierPatch.hpp Outdated Show resolved Hide resolved
src/axom/primal/geometry/BezierPatch.hpp Outdated Show resolved Hide resolved
* \class Line
*
* \brief Represents a line, \f$ L(t) \in \mathcal{R}^d \f$ , defined by an
* origin point, \f$ P \f$ and a normalized direction vector, \f$ \vec{d} \f$,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: Is normalization required for the intended use of Line? Are there cases where we might not want to normalize the direction vector?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily, but the primal::Ray class does too, and I can come up with a couple reasons why it could be useful. For example, the magntude doesn't have a useful meaning inherently, and making it a unit vector makes the math objects have a consistent parameterization, which is more useful in algortihms.

src/axom/primal/operators/detail/intersect_bezier_impl.hpp Outdated Show resolved Hide resolved
Comment on lines +85 to +105
if(shouldPrintIntersections)
{
std::stringstream sstr;

sstr << "Intersections for ray and patch: "
<< "\n\t" << ray << "\n\t" << patch;

sstr << "\ns (" << u.size() << "): ";
for(auto i = 0u; i < u.size(); ++i)
{
sstr << std::setprecision(16) << "(" << u[i] << "," << v[i] << "),";
}

sstr << "\nt (" << t.size() << "): ";
for(auto i = 0; i < t.size(); ++i)
{
sstr << std::setprecision(16) << t[i] << ",";
}

SLIC_INFO(sstr.str());
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor (preference, not requirement):
The output in this function might read a bit more clearly using fmt instead of stringstream

src/axom/primal/tests/primal_surface_intersect.cpp Outdated Show resolved Hide resolved
Comment on lines +139 to +143
BezierPatchType bilinear_patch(1, 1);
bilinear_patch(0, 0) = PointType({-1.0, 1.0, 1.0});
bilinear_patch(1, 0) = PointType({-1.0, -1.0, 2.0});
bilinear_patch(1, 1) = PointType({1.0, -1.0, 1.0});
bilinear_patch(0, 1) = PointType({1.0, 1.0, 2.0});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What intersections would be get with a line that intersects a non-planar patch, e.g.:

  BezierPatchType bilinear_patch(1, 1);
  bilinear_patch(0, 0) = PointType({0, 0, 0});
  bilinear_patch(1, 0) = PointType({0, 1, 0});
  bilinear_patch(1, 1) = PointType({1, 0, 0});
  bilinear_patch(0, 1) = PointType({1, 1, 2});

  RayType ray(PointType({0, .5, 0.}), VectorType({0,1,1}));

i.e., (if I got it right) the intersection is along the entire patch along its isocurve v==.5

(This might be captured below in the "infinitely many" case)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep! This is explicitly handled in those cases, where the entire intersection is both a v or a u intersection.

Comment on lines 163 to 164
// Ray with no intersections, but in a way that is difficult for
// the standard GARP implementation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Consider adding a note about why this case is difficult for GARP

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case has a double root in the quadratic that needs to be solved, so I've actually moved this test and others like it to the dedicated test block.

@jcs15c jcs15c force-pushed the feature/spainhour/ray_surface_intersection branch from 95517c4 to d200978 Compare December 18, 2024 06:51
@jcs15c jcs15c force-pushed the feature/spainhour/ray_surface_intersection branch from a51dddd to 5758e7a Compare December 18, 2024 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Primal Issues related to Axom's 'primal component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants