Implement Intersects for ICurve and all subtypes

[DmytroMuravskyi]

BACKGROUND:

• Intent of this PR is to add Intersects function to ICurve, so it can be called for any pair of curves.

DESCRIPTION:

• There are next curve types that used for intersections: InfiniteLine, Circle, Ellipse and Bezier.
• Line. Arc and Elliptic arc are using intersection function of their base curve but with additional domain checks for each intersection point. Each type doesn't have specific Intersects, only PointOnDomain function implemented.
• Every pair of ICurve is supported, but for simplicity, now every pair of specific curve type is represented. You can't call circle.Intersects(beizer) for example, only bezier.Intersects(circle). Some duplicating function are there, line ellipse.Intersects(circle) and cirlce.Intersects(ellipse) and they just call each other.
• Using interface is slightly slower than using direct functions since type switch must be done find exact Intersects function.
• ParameterAt is added to InfiniteLine, Circle and Ellipse to be able to get parameter at certain point if it's on the curve.
• IndexedPolycurve calls Intersects on each of its subcurves.
• Ellipse to Circle, Ellipse to Ellipse can have up to 4 intersections and can't be solved by quadratic equation. Equation.SolveIterative is used for them.
• The same is true for Bezier to Line, Bezier to Circle and Bezier to Ellipse. Since Bezier is not limited to 4 control points in Elements, they can only be solved iteratively.
• Bezier to Bezier is solved by bisecting their bounding boxes. The approach is taken for the document mentioned in Bezier code file.
• Equation. SolveIterative splits function for up to N steps and tried to find a root at each section - both when function crosses 0 or touches it, but only one per step. It's certainly possible to do further improvements for it, but it feels like overkill right now since there are many ways solving complex intersections, including using 3rd party libraries.
• There are other helper functions added: Units.AdjustRadian, Vector3Extensions.UniqueAverageWithinTolerance, Vector3.ClosestPointOn(InfiniteLine line), Plane.Intersects(Plane other, out InfiniteLine result).

FUTURE WORK:

• Some functions feel obsolete after these changes: Line.PointOnLine, old Line.Intersects(Line), etc.

TESTING:

• Tests created for each new function.

REQUIRED:

• All changes are up to date in CHANGELOG.md.

COMMENTS:

• Elements.Tests.ContentTests.InstanceContentElement test is failing. I think it's because it's timer based and should be fixed again after restart. If this is the case this test must be either updated or removed.

---

This change is 

[DmytroMuravskyi]

@anthonie-kramer Added draft of implementation for ICurve.Intersects(ICurve).
Now every function pair is presented by they can be added for sure.
BaseCurve types: InfiniteLine, Circle and Ellipse - have function for intersecting with one another but not with other types.
TrimmedCurve are the same but with domain check. All code is located in TrimmedCurve with only PointOnDomain implemented in each subclass. Line.PointOnLine function is logically collides with Line.PointOnDomain but they still little bit different as PointOnLine has includeEnds parameter.
IndexedPolycurve has no new intersection math and just about intersecting individual pieces. Since there can be duplicating intersections at joint points between sub curves - extra filtering s added.
Bezier is not yet implemented as it's most complex type.
Equation.SolveIterative needs improvements - I've added basic implementation to show the full picture here.
There are many existing Intersects functions that may overlap with new functons. Good example is Line.Intersects(Line, out Vector3) that is different by just out parameter from new Line.Intersects(Line, out List) This mean that just using
l1.Intersects(l2, out var intersections) will not compile as it's ambiguous. This can be fixed by either changing the signature of new function tree to some non colliding name or allowing specific implementations have different signature from interface function. In line to line case there will be only one out point, although all other intersections have multiple of them.

Does Intersects need tolerance parameter added?

[ikeough]

This is great! Very excited to see this being worked on. I put a couple of small comment related to documentation here. As for the questions above about the overlap of methods, I'd be interested to know how big the change would be to standardize on your new interface and obsolete the old methods. I realize this would mean a bump of the release to 3.0. We could also make this a minor bump and deprecate the old intersection methods.

Reviewable status: 0 of 1 approvals obtained (waiting on @DmytroMuravskyi)

---

Elements/src/Geometry/TrimmedCurve.cs line 20 at r1 (raw file):

        public TBasis BasisCurve { get; protected set; }

        public abstract bool PointOnDomain(Vector3 point);

Can we get a comment for this new public method, and an <inheritdoc> everywhere it is implemented?

---

Elements/src/Geometry/Vector3.cs line 854 at r1 (raw file):

        }

        public Vector3 ClosestPointOn(InfiniteLine line)

Can we get a comment here?

---

Elements/src/Geometry/Interfaces/ICurve.cs line 31 at r1 (raw file):

        double ParameterAtDistanceFromParameter(double distance, double parameter);

        bool Intersects(ICurve curve, out List<Vector3> results);

Can we a comment here? We've started to use <inheritdoc> in other places, so a good comment on this interface can then be shared in every implementation by using <inheritdoc> (https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/xmldoc/recommended-tags#inheritdoc).

[DmytroMuravskyi]

I will add the documentation for sure as a next step.

Added implementation for Bezier. For Bezier-Line, Bezier-Circle, Bezier-Ellipse I used the iterative approach where I search where function of distance from iterated point to other object is zero. For Bezier-Bezier this approach is not working, so I used approach described in document mentioned in Bezier file - intersecting subdivided bounding boxes.

Both approaches have weak points I want to minimize right now: root can be possibly missed or 2 roots can be found around an intersection point.

Other issue I'm working on is resolving ambiguity between
bool Intersects(Line l, out Vector3 result, bool infinite = false, bool includeEnds = false) old from Line
bool Intersects(TrimmedCurve curve, out List results) where T : ICurve new from TrimmedCurve
as l0.Intersects(l1, out var intersections) is always interpreted as old one and you must specify List. Maybe it's not a big problem.

[anthonie-kramer]

Looking good!

In the long term, I think we will want to find ways to supplement the iterative approaches, but it's fantastic to have this functionality now.

In general, we can improve readability in a couple of places. We can also improve performance by converting any foreach loops into for loops.

When you feel like we have enough tests, it would be awesome to see an integration test where we draw multiple beziers, lines, and other shapes in Hypar and show the intersection methods working in a Function.

Great work so far!

Reviewed 1 of 22 files at r1, 4 of 14 files at r2, 23 of 23 files at r3, all commit messages.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @DmytroMuravskyi)

---

Elements/src/Geometry/Bezier.cs line 362 at r3 (raw file):

                case Circle circle:
                    return Intersects(circle, out results);
                case Ellipse elliplse:

typo

---

Elements/src/Geometry/Bezier.cs line 394 at r3 (raw file):

            // Iteratively, find points on Bezier with 0 distance to the line.
            // It Bezier was limited to 4 points - more effective approach could be used.
            var roots = Equations.SolveIterative(Domain.Min, Domain.Max, 100,

I think if we are going to use an iterative method, we should make the steps a function of the curve length (applies to anywhere we use a step)

---

Elements/src/Geometry/Bezier.cs line 525 at r3 (raw file):

        }

        private void Intersects(Bezier other,

This method feels a little verbose / redundant

---

Elements/src/Geometry/InfiniteLine.cs line 120 at r3 (raw file):

                case Circle circle:
                    return Intersects(circle, out results);
                case Ellipse elliplse:

typo

---

Elements/src/Geometry/TrimmedCurve.cs line 42 at r3 (raw file):

                case Circle circle:
                    return Intersects(circle, out results);
                case Ellipse elliplse:

typo

---

Elements/src/Geometry/Vector3.cs line 1 at r3 (raw file):

using ClipperLib;

What are we using ClipperLib here for? I have a secret agenda to eventually remove our dependency on ClipperLib, so if there is functionality that we need from it, I would rather solve it internally than rely on ClipperLib more.

[DmytroMuravskyi]

I think foreach usage here is simple enough so it will not cause any performance downgrade.

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @anthonie-kramer and @ikeough)

---

Elements/src/Geometry/Bezier.cs line 394 at r3 (raw file):

Previously, anthonie-kramer (Anthonie Kramer) wrote…

I think if we are going to use an iterative method, we should make the steps a function of the curve length (applies to anywhere we use a step)

Done. Added Circumference function for Ellipse and used Ellipse length for steps. For Bezier number of steps is limited to 500 as it is done in other functions.

---

Elements/src/Geometry/Bezier.cs line 525 at r3 (raw file):

Previously, anthonie-kramer (Anthonie Kramer) wrote…

This method feels a little verbose / redundant

Simplified functions where it's possible. I'll be glad to do even further if you have any specific ideas in mind.

---

Elements/src/Geometry/TrimmedCurve.cs line 20 at r1 (raw file):

Previously, ikeough (Ian Keough) wrote…

Can we get a comment for this new public method, and an <inheritdoc> everywhere it is implemented?

Done.

---

Elements/src/Geometry/Vector3.cs line 854 at r1 (raw file):

Previously, ikeough (Ian Keough) wrote…

Can we get a comment here?

Done.

---

Elements/src/Geometry/Vector3.cs line 1 at r3 (raw file):

Previously, anthonie-kramer (Anthonie Kramer) wrote…

What are we using ClipperLib here for? I have a secret agenda to eventually remove our dependency on ClipperLib, so if there is functionality that we need from it, I would rather solve it internally than rely on ClipperLib more.

I had auto add for using enabled, it likes to add useless usings when wrong function name is autocompleted. Disabled it for good. I agree that Clipper lib is problematic because of use of integer math.

---

Elements/src/Geometry/Interfaces/ICurve.cs line 31 at r1 (raw file):

Previously, ikeough (Ian Keough) wrote…

Can we a comment here? We've started to use <inheritdoc> in other places, so a good comment on this interface can then be shared in every implementation by using <inheritdoc> (https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/xmldoc/recommended-tags#inheritdoc).

Done.

[DmytroMuravskyi]

Missing arc intersections are fixed in the latest push. Is there something else missing in this PR?

[anthonie-kramer]

Reviewed 5 of 7 files at r4, 4 of 4 files at r5.
Reviewable status: complete! 1 of 1 approvals obtained (waiting on @DmytroMuravskyi and @ikeough)

---

Elements/src/Geometry/Bezier.cs line 525 at r3 (raw file):

Previously, DmytroMuravskyi wrote…

Simplified functions where it's possible. I'll be glad to do even further if you have any specific ideas in mind.