Modeling Data - Fix GCPnts arc-length sampler point counts

[gsdali]

Resubmission of #1417, which was closed by an accidental force-push (a bad rebase on our side left
the branch with no commit in common with IR, and GitHub auto-closes a PR when that happens) rather
than by any review outcome. Same fix, same branch, now including the three changes @gkv311 requested
on #1417: SquareDistance() instead of Distance() in the loop, theTol3d renamed to theTol, and
the duplicated Standard_ConstructionError_Raise_if check dropped in favor of a single unconditional
if. Full response to that review is in the first comment below.

Description

Two defects in the GCPnts arc-length samplers, both about the requested point count.

NbPoints() is not bounded by the requested count

GCPnts_UniformAbscissa::initialize sizes myParams at theNbPoints + 5 and the walk fills it
until it reaches the end parameter or runs out of room, setting myNbPoints to whatever it reached.
A caller that sizes its own buffer from the requested count rather than from NbPoints() is handed
more points than it asked for. GCPnts_QuasiUniformAbscissa inherits this for every curve that is
neither Bezier nor BSpline, since it forwards to GCPnts_UniformAbscissa for those, so the same
class returns exactly theNbPoints on a Bezier and possibly more on an ellipse.

The cause is a tolerance mismatch rather than an off-by-one. Perform terminates on

if (std::abs(aUi - aUU2) <= theEPSILON)

where theEPSILON is theC.Resolution(theTol), a parametric tolerance derived from a 3D one using
the curve's largest derivative. On an ellipse with major radius 1e6 and minor radius 1e-3 that is
about 1e-13, while the local derivative at the end of that curve is 1e-3, so the parametric tolerance
that actually corresponds to 1e-7 in 3D there is about 1e-4. The test is around nine orders of
magnitude too tight at that end of the curve. The walk stops 1.557e-08 short, does not treat that as
done, takes one more step and snaps it to the end.

The appended point is a duplicate: 1.175e-10 from its neighbour in 3D. On that curve, 22 of the
counts from 2 to 60 return one point more than requested, for both classes.

Perform now also accepts a point that coincides with the end within the caller's 3D tolerance, not
only one close in parameter. The tolerance is squared once outside the loop (aTol2) and compared
via SquareDistance() rather than calling Distance() on every candidate step; the parameter is
named theTol rather than theTol3d, since Perform also instantiates on Adaptor2d_Curve2d. The
end point is evaluated once before the walk, and the distance test is gated behind
aUU2 - aUi < aDelta so it runs on the final step rather than on every step.

Clamping myNbPoints to theNbPoints was the other option and is worse: the surplus point is the
one carrying the exact end parameter, so clamping leaves the distribution stopping short of the
curve.

A point count below 2 stores out of bounds

Both classes document theNbPoints >= 2 and enforced it with Standard_ConstructionError_Raise_if,
which compiles to nothing when No_Exception is defined, as it is for Release builds with
BUILD_RELEASE_DISABLE_EXCEPTIONS (the default). GCPnts_QuasiUniformAbscissa::initialize then
allocates NCollection_HArray1<double>(1, theNbPoints), an empty range for such a count, and the
next statement is an unconditional myParams->SetValue(1, theU1). SetValue's own bounds check is a
Raise_if too, so the store lands out of bounds.

Reproduced with theNbPoints = 0 and with a negative count, on a 4-pole Bezier and an 8-point
BSpline fit. GCPnts_UniformAbscissa has the same missing precondition without the out-of-bounds
store: it answers a request for zero points with five.

Both classes now leave the object not done for a count below 2, unconditionally: the ordinary if
replaces Standard_ConstructionError_Raise_if rather than duplicating its condition alongside it, so
the result no longer depends on whether No_Exception is defined.

No public API signature changes.

Type of change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

A harness fingerprints both classes across 17 curve types (a line, circles of radius 1e-6 to 1e7, a
1e6 x 1e-3 ellipse, a 5 x 2 ellipse, a hyperbola, a parabola, a 2-pole and a 4-pole Bezier, an
8-point and a 40-point BSpline, an offset circle, an offset BSpline, a trimmed circle, and a
half-period slice of the pathological ellipse) and counts 2 to 200, recording the point count, the
first and last parameter and a digest of the whole parameter list, so a stock run and a patched run
can be diffed result by result.

• 6766 configurations measured. 232 results change, and they are exactly the 232 that were
  returning more points than requested. Every other result is identical parameter for parameter,
  and on the changed ones the last parameter is still exactly the end.
• Requests that over-returned: 22 counts to 0.
• Degenerate counts (0, 1, negative) on every curve now return IsDone() == false for both classes,
  in every build configuration, in place of an out-of-bounds store, a five-point answer or a
  one-point answer depending on which curve and which class was called.
• clang-format clean on both touched files.

Also built into a downstream project's kernel and run against its full suite: clean.

Reproducers and the full write-up:
https://github.com/SecondMouseAU/OCCTSwift/tree/main/Scripts/repro/555-gcpnts-count-contract

Checklist:

• My code follows the code style of this project (clang-format clean on every touched file)
• My change requires a change to the documentation

[gsdali]

Thank you for the review. Pushed an update addressing all three points.

1. The No_Exception guard. Took option (b), replacing Standard_ConstructionError_Raise_if
with the plain if rather than adding an unconditional throw alongside it. Both classes now
answer a count below 2 with IsDone() == false, in every build, not only one that defines
No_Exception: the if replaces the precondition check entirely rather than duplicating it below
a macro that only sometimes compiles to anything.

We build with BUILD_RELEASE_DISABLE_EXCEPTIONS=ON, so this was not a coin flip for us: measured
before choosing, not reasoned out. Every one of our call sites into these two classes' count-based
initialize() (9 across the bridge, one of them a shared static helper with 2 further callers, so
11 total) already wraps the construction in catch (...), so a caller-visible degenerate-count
result is identical whether the kernel throws or returns not-done: both options were safe for us.
Our own tests (Issue558SamplingCountBoundsTests.swift) assert the not-done/empty-result contract
for a count of 1 across every one of those entry points, which is what a silent not-done produces
directly and what a thrown exception produces once our own catch (...) converts it, so neither
option changes an observable answer on our side.

We also checked whether anything in the tree calls the count-based initialize() with a count it
does not pre-validate: grepped src/ for both classes outside their own GCPnts package. Five
production call sites and one Draw test command construct either class with a count; every one
either hardcodes it (N = 40, NbOfPnts = 61, npt = 4/8) or clamps/rejects it first
(std::max(2, aNbPoints), std::max(3, aNbSamplePoints), the Draw command's own
if (aSrcNbPnts < 2) { ...; return 1; }). So no caller in the tree can reach the degenerate
branch at all today, and neither option changes observed behavior anywhere in OCCT, on top of not
changing it for us.

Given that, we preferred (b) over (a) for a reason external to our own build: an explicit
if (...) throw ...; makes every OCCT-internal caller of these two classes' count-based
initialize() newly exception-throwing for a degenerate count, in every build, including ones built
specifically to avoid that overhead. (b) keeps the existing not-done contract, and it no longer
depends on a build flag to have that shape. We did confirm directly (compiled both variants without
No_Exception and probed a degenerate count on each): the current PR's guard still throws
Standard_ConstructionError there today; after this update, neither class throws for this input in
either build configuration.

2. Distance() in a loop. Hoisted theTol * theTol once above the loop as aTol2 and compare
SquareDistance() against it instead of calling Distance() (and the implicit std::sqrt()) on
every candidate step.

3. theTol3d renamed to theTol. Renamed the parameter and checked both call sites: both are
inside GCPnts_UniformAbscissa::initialize() overloads that already have their own theTol
parameter, and both already pass it straight through (std::max(theTol, Precision::Confusion()))
as a plain argument expression, not through an intermediate local also named theTol. Perform() is
a free function at namespace scope, not a member of the class whose initialize() calls it, so there
is no enclosing scope for either theTol to shadow; renaming introduces no shadowing anywhere in
the file.

Re-measured the full 6766-configuration equivalence sweep (17 curve types x point counts 2-200,
both classes) after all three changes: identical to the pre-review patch, 232 changed configurations,
all of them the over-request cases, nothing else moved. Re-ran the degenerate-count sweep across
5 curve types x both classes x counts {0, 1, -3}: unchanged, IsDone() == false throughout, no
crash. Full downstream swift test suite (OCCTSwift, the consumer this was filed from) unaffected.