Improved CurveCurveIntersectXY #8775
Conversation
core/geometry/src/curve/internalContexts/CurveCurveIntersectXY.ts
Outdated
Show resolved
Hide resolved
- carefully handle `reverse` flag when computing additional seeds for extended curveA - `refineSpiralResultsByNewton`: instead of passing size of array tail, it is simpler to pass first index of tail, like in `CurveChainWithDistanceIndex.convertChildDetailToChainDetail` - consolidate duplicated code - add helpers for resetting geomA/B - increase maxIters for Newton to handle tangent intersections, which have sub-quadratic convergence - use appropriate tolerances for point/fraction comparison
…oser spiral point compared to using the linestring fractional arclength as spiral fraction seed.
- compute two types of Newton seeds from solving the discrete spiral intersectionXY and closeApproachXY problems. - order seeds carefully so that most accurate is processed first. - cull convergent solutions with a dual tolerance filter. Subsequent equivalent solutions are not recorded. - remove check for a solution seed; this can install a less-accurate result. Instead, check the last iterate if Newton fails early on a zero derivative. - compute the max stroke error for a spiral ~ close to the midpoint of first or last stroke segment, whichever spans higher spiral curvature. This is used to limit the results of the discrete spiral closeApproachXY problem. Previous value of 1 micron was way too small, and will fail the new test.
There was a problem hiding this comment.
Made some changes. Please take a look.
There still seems to be an issue with visualizing DirectSpiral3d curves transformed out of xy-plane (in our test environment). I'm not sure this is due to how we serialize the data in iTwin, or how we deserialize it in native code, or how we process the deserialized object to view it.
Forgot to mention in commit messages: aside from removing dead code in closeApproachXY, I also made sure its output details store the projection distance in the a field. This is so we can sort on that in the intersectXY class.
Aside: the new logic for computing spiral intersectionXY via its strokes is applicable to any CurvePrimitive for which we can compute a maximum stroke error. For spirals, this computation is simple, but for a general curve, it could be expensive.
Created an issue for it: https://github.com/iTwin/itwinjs-backlog/issues/1774
What is the benefit of sorting close approaches? If we are going to pick first few ones and drop the rest, sorting makes sense but the code is using all of the close approaches so I don't understand why we have to pay for sorting. FYI, I reviewed the rest of the code and all made sense (except sorting). I will merge this and modify my close approach PR based on your changes. |
| ); | ||
| closeApproachPairs.sort((p0: CurveLocationDetailPair, p1: CurveLocationDetailPair) => p0.detailA.a - p1.detailA.a); | ||
| this._results.push(...closeApproachPairs); | ||
| return this._results.length - i0; |
There was a problem hiding this comment.
why do you return this number? The return value is never used.
| } | ||
| this.resetGeometryA(extendA0, extendA1); | ||
| this.resetGeometryB(geomB, extendB0, extendB1); | ||
| return this._results.length - i0; |
There was a problem hiding this comment.
why do you return this number? The return value is never used.
This PR is motivated by a deficiency in the initial PR whereby a tangency intersection is not found because the spiral's linestring approximation does not intersect the other curve, resulting in a failure to compute a Newton seed that refines to the tangency.