From 343fd1da009681807aeccdbb94dc3594c2a6fc5b Mon Sep 17 00:00:00 2001 From: gsdali <51393997+gsdali@users.noreply.github.com> Date: Sat, 1 Aug 2026 07:50:29 +1000 Subject: [PATCH] refactor(#562): five knot-splitting spellings collapse onto two, and the "weaker" duplicate was the stronger one `GeomConvert_BSplineSurfaceKnotSplitting` and `Geom2dConvert_BSplineCurveKnotSplitting` were each wrapped twice, by two families added three releases apart. The five v0.105.0 spellings (`Surface.bsplineKnotSplitsU`/`bsplineKnotSplitsV`/`bsplineKnotSplitValues`, `Curve2D.bsplineKnotSplits`/`bsplineKnotSplitValues`) are deprecated and forward to `Surface.knotSplitting` / `Curve2D.splitIndicesAtDiscontinuities`; their five bridge functions are deleted. The issue's premise that the five were strictly weaker did not survive measurement. `Curve2D.bsplineKnotSplitValues` sized its buffer from the analyzer's own count, where `splitIndicesAtDiscontinuities` read a fixed 256 entries and the bridge returned the count it had *written* -- indistinguishable from a curve with exactly 256 splits. On a cubic with 302 splits the canonical call returned 256 and the duplicate returned 302, so forwarding without fixing that would have regressed the deprecated spelling. `OCCTCurve2DSplitAtDiscontinuities` now reports the true count and the Swift caller re-reads at it, the #481 contract the rest of the family already shared. C-layer contract change; OCCTBridge is not an SPM product (#486). The one thing the deleted family carried that the canonical calls did not is the raw knot-table indices: the analyzer reports indices and `OCCTSurfaceKnotSplitting` converted them to parameters, so the raw form was reachable only through `bsplineKnotSplitValues`, which constructed the analyzer three more times to get it. `KnotSplitResult` now carries `uSplitIndices`/`vSplitIndices` from the one construction already happening, with `uSplitParams[i] == bsplineUKnot(index: uSplitIndices[i])` by construction. That answers the issue's open question: the information was worth keeping, the three entry points were not. Both deleted values functions also took no buffer capacity at all, each writing `NbSplits()` entries into a buffer the caller had sized from a separate call. Recorded in the bridge header. `OCCTBridge.h`'s cross-reference index named none of the three `*KnotSplitting` conversion classes, which is half of why the double-wrap survived three releases (#510). It gains the two `GeomConvert_*` entries and a `--- Geom2dConvert ---` section that did not exist at all, censused by call site across its six classes. Tests: `Issue562Curve2DKnotSplitDuplicateTests` (4) and `Issue562SurfaceKnotSplitDuplicateTests` (5), with absolute expectations against the fixture's own knot table rather than parity between the two spellings, which stopped being evidence once one forwards to the other. Three injected defects (written-count truncation, 0-based indices, V continuity collapsed onto U) each fail the tests that should catch them; the last is caught by the new suite alone. Closes #562. Co-Authored-By: Claude Opus 5 (1M context) --- Sources/OCCTBridge/include/OCCTBridge.h | 70 ++++++---- Sources/OCCTBridge/src/OCCTBridge_Geom2d.mm | 45 ++---- Sources/OCCTBridge/src/OCCTBridge_Surface.mm | 73 ++++------ Sources/OCCTSwift/Curve2D.swift | 17 ++- Sources/OCCTSwift/Document.swift | 65 ++++----- Sources/OCCTSwift/Surface.swift | 38 +++-- ...ue480Curve2DKnotSplitContinuityTests.swift | 19 ++- ...sue562Curve2DKnotSplitDuplicateTests.swift | 81 +++++++++++ Tests/OCCTGeom2dTests/OCCTGeom2dTests.swift | 9 +- ...sue562SurfaceKnotSplitDuplicateTests.swift | 104 ++++++++++++++ Tests/OCCTSurfaceTests/OCCTSurfaceTests.swift | 3 +- docs/API_REFERENCE.md | 4 +- docs/CHANGELOG.md | 53 +++++++ docs/reference/Curve2D.md | 7 +- .../Document-Geometry-Constructors.md | 130 ++++-------------- docs/reference/Surface-Advanced.md | 21 ++- 16 files changed, 453 insertions(+), 286 deletions(-) create mode 100644 Tests/OCCTGeom2dTests/Issue562Curve2DKnotSplitDuplicateTests.swift create mode 100644 Tests/OCCTSurfaceTests/Issue562SurfaceKnotSplitDuplicateTests.swift diff --git a/Sources/OCCTBridge/include/OCCTBridge.h b/Sources/OCCTBridge/include/OCCTBridge.h index a8eac95e..ca403b2a 100644 --- a/Sources/OCCTBridge/include/OCCTBridge.h +++ b/Sources/OCCTBridge/include/OCCTBridge.h @@ -366,11 +366,26 @@ // --- GeomConvert --- // GeomConvert → OCCTCurve3DToBSpline, OCCTCurve3DBSplineToBeziers, OCCTCurve3DSplitAtContinuity, // OCCTSurfaceToBSpline, OCCTSurfaceToBezierPatches +// GeomConvert_BSplineCurveKnotSplitting → OCCTCurve3DBSplineKnotSplits (the sole wrapper; #562) +// GeomConvert_BSplineSurfaceKnotSplitting → OCCTSurfaceKnotSplitting (the sole wrapper since #562 +// deleted the second family that wrapped it) // GeomConvert_CompCurveToBSplineCurve → OCCTCurve3DJoinCurves, OCCTCurve3DJoinToBSpline, // OCCTCurve3DConcatenateG1, OCCTConcatenateCurves3D // GeomConvert_CurveToAnaCurve → OCCTGeomConvertCurveToAnalytical, OCCTGeomConvertIsLinear // GeomConvert_SurfToAnaSurf → OCCTGeomConvertSurfToAnalytical*, OCCTGeomConvertIsCanonical // +// --- Geom2dConvert --- +// #562: this section did not exist, which is half of why Geom2dConvert_BSplineCurveKnotSplitting +// could be wrapped twice without anything noticing. Census is by call site in OCCTBridge_Geom2d.mm. +// Geom2dConvert → OCCTCurve2DToBSpline, OCCTCurve2DSplitAtContinuity, +// OCCTCurve2DJoinToBSpline +// Geom2dConvert_ApproxArcsSegments → OCCTGeom2dConvertApproxArcsSegments, OCCTCurve2DToArcsAndSegments +// Geom2dConvert_ApproxCurve → OCCTCurve2DApproximate +// Geom2dConvert_BSplineCurveKnotSplitting → OCCTCurve2DSplitAtDiscontinuities (the sole wrapper +// since #562 deleted the second family that wrapped it) +// Geom2dConvert_BSplineCurveToBezierCurve → OCCTCurve2DBSplineToBeziers +// Geom2dConvert_CompCurveToBSplineCurve → OCCTConcatenateCurves2D, OCCTCurve2DJoinToBSpline +// // --- Convert --- // Convert_CompBezierCurvesToBSplineCurve → OCCTConvertCompBezierToBSpline (v0.99.0) // Convert_CompBezierCurves2dToBSplineCurve2d → OCCTConvertCompBezier2dToBSpline2d (v0.99.0) @@ -2663,6 +2678,10 @@ OCCTCurve2DRef OCCTCurve2DApproximate(OCCTCurve2DRef curve, double tolerance, int32_t continuity, int32_t maxSegments, int32_t maxDegree); // `continuity` is a ContinuityRange (a literal derivative order, splitting where // `degree - multiplicity < continuity`), not a GeomAbs_Shape. See the #480 note in OCCTBridge_Internal.h. +// Returns the TRUE split count even when writing was truncated by `max`, so a caller that came up +// short can retry at the size it was just told — the #481 contract the rest of this family already +// shares. It used to return the count it had written, which is indistinguishable from a curve with +// exactly `max` splits (#562). int32_t OCCTCurve2DSplitAtDiscontinuities(OCCTCurve2DRef curve, int32_t continuity, int32_t* outKnotIndices, int32_t max); int32_t OCCTCurve2DToArcsAndSegments(OCCTCurve2DRef curve, double tolerance, @@ -6832,15 +6851,20 @@ typedef struct { /// cubic with simple interior knots needs 3. See the #480 note in OCCTBridge_Internal.h /// @param vContinuity Desired V continuity, same contract against the V degree and knots /// @param outUParams Pre-allocated array for U split parameter values (may be NULL) -/// @param maxUParams Capacity of outUParams +/// @param outUIndices Pre-allocated array for the 1-based U knot-table indices those parameters +/// were read from, i.e. `outUParams[i] == UKnot(outUIndices[i])` (may be NULL). #562: the +/// analyzer reports indices and this function converts them, so the caller only ever saw the +/// converted form and a second family of bridge functions existed to serve the raw one +/// @param maxU Capacity of outUParams and outUIndices /// @param outVParams Pre-allocated array for V split parameter values (may be NULL) -/// @param maxVParams Capacity of outVParams +/// @param outVIndices Pre-allocated array for the 1-based V knot-table indices (may be NULL) +/// @param maxV Capacity of outVParams and outVIndices /// @return Split counts; nbUSplits/nbVSplits are the true counts even when writing -/// was truncated by maxUParams/maxVParams, so a caller can retry with a bigger buffer +/// was truncated by maxU/maxV, so a caller can retry with a bigger buffer OCCTSurfaceKnotSplitResult OCCTSurfaceKnotSplitting(OCCTSurfaceRef surface, int32_t uContinuity, int32_t vContinuity, - double* outUParams, int32_t maxUParams, - double* outVParams, int32_t maxVParams); + double* outUParams, int32_t* outUIndices, int32_t maxU, + double* outVParams, int32_t* outVIndices, int32_t maxV); /// Join an array of Bezier surface patches into a single BSpline surface. /// @param patches Array of surface handles (row-major, nRows x nCols) @@ -14678,33 +14702,19 @@ OCCTCurve3DRef _Nullable OCCTConcatenateCurves3D(OCCTCurve3DRef _Nonnull * _Nonn OCCTCurve2DRef _Nullable OCCTConcatenateCurves2D(OCCTCurve2DRef _Nonnull * _Nonnull curves, int32_t count, double tolerance); -// MARK: - GeomConvert_BSplineSurfaceKnotSplitting (v0.105.0) +// MARK: - GeomConvert_BSplineSurfaceKnotSplitting / Geom2dConvert_BSplineCurveKnotSplitting // -// `continuity` throughout this section is the same ContinuityRange as OCCTSurfaceKnotSplitting -// takes: a literal derivative order, splitting where `degree - multiplicity < continuity`, with -// useful domain 0...degree. See the #480 note in OCCTBridge_Internal.h. - -/// Get number of U-direction knot splits for a BSpline surface at given continuity. -int32_t OCCTBSplineSurfaceKnotSplitsU(OCCTSurfaceRef _Nonnull surface, int32_t continuity); - -/// Get number of V-direction knot splits for a BSpline surface at given continuity. -int32_t OCCTBSplineSurfaceKnotSplitsV(OCCTSurfaceRef _Nonnull surface, int32_t continuity); - -/// Get U and V knot split indices for a BSpline surface at given continuity. -void OCCTBSplineSurfaceKnotSplitValues(OCCTSurfaceRef _Nonnull surface, int32_t continuity, - int32_t* _Nonnull uSplits, int32_t* _Nonnull vSplits); - -// MARK: - Geom2dConvert_BSplineCurveKnotSplitting (v0.105.0) +// #562: five functions used to live here (OCCTBSplineSurfaceKnotSplitsU/V, +// OCCTBSplineSurfaceKnotSplitValues, OCCTBSplineCurve2dKnotSplits, +// OCCTBSplineCurve2dKnotSplitValues), added in v0.105.0 over the same two analyzers +// OCCTSurfaceKnotSplitting and OCCTCurve2DSplitAtDiscontinuities already drove. They are gone; +// those two are the sole wrappers of their analyzer. Both now report the split knot-table +// indices, which is all the deleted family carried that the survivors did not. // -// Same ContinuityRange contract again: Geom2dConvert_BSplineCurveKnotSplitting runs the -// identical algorithm on a 2D curve. See the #480 note in OCCTBridge_Internal.h. - -/// Get number of knot splits for a 2D BSpline curve at given continuity. -int32_t OCCTBSplineCurve2dKnotSplits(OCCTCurve2DRef _Nonnull curve, int32_t continuity); - -/// Get knot split indices for a 2D BSpline curve at given continuity. -void OCCTBSplineCurve2dKnotSplitValues(OCCTCurve2DRef _Nonnull curve, int32_t continuity, - int32_t* _Nonnull splits); +// Two contract hazards the deleted family had, recorded so they are not reintroduced: neither +// values function took a buffer capacity (each wrote NbSplits() entries into a buffer the caller +// had sized from a *separate* call), and the surface one constructed the analyzer three times per +// logical query, once per count call and once for the values. // MARK: - BndLib extras (v0.105.0) diff --git a/Sources/OCCTBridge/src/OCCTBridge_Geom2d.mm b/Sources/OCCTBridge/src/OCCTBridge_Geom2d.mm index 5831353c..3532d7f2 100644 --- a/Sources/OCCTBridge/src/OCCTBridge_Geom2d.mm +++ b/Sources/OCCTBridge/src/OCCTBridge_Geom2d.mm @@ -3375,32 +3375,11 @@ OCCTCurve2DRef OCCTConcatenateCurves2D(OCCTCurve2DRef* curves, int32_t count, do return r; } catch (...) { return nullptr; } } -// MARK: - Geom2dConvert_BSplineCurveKnotSplitting (v0.105.0) - -#include - -int32_t OCCTBSplineCurve2dKnotSplits(OCCTCurve2DRef curve, int32_t continuity) { - if (!curve) return 0; - try { - Handle(Geom2d_BSplineCurve) bc = Handle(Geom2d_BSplineCurve)::DownCast(curve->curve); - if (bc.IsNull()) return 0; - Geom2dConvert_BSplineCurveKnotSplitting splitter(bc, continuity); - return (int32_t)splitter.NbSplits(); - } catch (...) { return 0; } -} - -void OCCTBSplineCurve2dKnotSplitValues(OCCTCurve2DRef curve, int32_t continuity, - int32_t* splits) { - if (!curve || !splits) return; - try { - Handle(Geom2d_BSplineCurve) bc = Handle(Geom2d_BSplineCurve)::DownCast(curve->curve); - if (bc.IsNull()) return; - Geom2dConvert_BSplineCurveKnotSplitting splitter(bc, continuity); - for (int i = 1; i <= splitter.NbSplits(); i++) { - splits[i - 1] = splitter.SplitValue(i); - } - } catch (...) {} -} +// #562: OCCTBSplineCurve2dKnotSplits and OCCTBSplineCurve2dKnotSplitValues stood here, a second +// wrap of Geom2dConvert_BSplineCurveKnotSplitting added three releases after +// OCCTCurve2DSplitAtDiscontinuities (further down this file) already wrapped it. Deleted; that +// one returns the same indices, and now reports the true count when truncated, which is the one +// respect in which these were the stronger pair rather than the weaker. // MARK: - v0.106: BRepLib_MakeEdge2d extensions + Curve2D continuity // MARK: - BRepLib_MakeEdge2d extensions (v0.106.0) @@ -5966,6 +5945,10 @@ Geom2dConvert_ApproxCurve approx(c->curve, tolerance, } } +// #562: reports the TRUE split count even when `max` truncated the write, so the Swift caller can +// retry at the size it was just told -- the #481 contract shared by every other member of this +// family. It used to return the written count, which capped it silently at its caller's 256-entry +// first pass and was indistinguishable from a curve with exactly 256 splits. int32_t OCCTCurve2DSplitAtDiscontinuities(OCCTCurve2DRef c, int32_t continuity, int32_t* outKnotIndices, int32_t max) { if (!c || c->curve.IsNull() || !outKnotIndices || max <= 0) return 0; @@ -5973,13 +5956,9 @@ int32_t OCCTCurve2DSplitAtDiscontinuities(OCCTCurve2DRef c, int32_t continuity, Handle(Geom2d_BSplineCurve) bsp = Handle(Geom2d_BSplineCurve)::DownCast(c->curve); if (bsp.IsNull()) return 0; Geom2dConvert_BSplineCurveKnotSplitting splitter(bsp, continuity); - int32_t n = std::min((int32_t)splitter.NbSplits(), max); - TColStd_Array1OfInteger indices(1, splitter.NbSplits()); - splitter.Splitting(indices); - for (int32_t i = 0; i < n; i++) { - outKnotIndices[i] = indices(i + 1); - } - return n; + return occtWriteKnotSplits(splitter.NbSplits(), + [&](int32_t i) { return (int32_t)splitter.SplitValue(i); }, + outKnotIndices, max); } catch (...) { return 0; } diff --git a/Sources/OCCTBridge/src/OCCTBridge_Surface.mm b/Sources/OCCTBridge/src/OCCTBridge_Surface.mm index 9a53ea21..53ede103 100644 --- a/Sources/OCCTBridge/src/OCCTBridge_Surface.mm +++ b/Sources/OCCTBridge/src/OCCTBridge_Surface.mm @@ -1812,12 +1812,14 @@ OCCTSurfaceRef OCCTSurfaceTrimmedCylinder( // MARK: - Surface KnotSplitting / JoinBezierPatches (v0.50) // #403: also fills the U/V split PARAMETER buffers (not just the counts) -- the // underlying GeomConvert_BSplineSurfaceKnotSplitting analyzer always computed -// USplitValue/VSplitValue, this just wasn't surfaced. outUParams/outVParams may be -// null (or their max 0) to skip writing either direction. +// USplitValue/VSplitValue, this just wasn't surfaced. #562: and fills the raw knot-table +// indices those parameters came from, which is what a second, now-deleted family of bridge +// functions existed to return. Any out buffer may be null (or its max 0) to skip it; one +// analyzer construction serves all four, where that family needed three. OCCTSurfaceKnotSplitResult OCCTSurfaceKnotSplitting(OCCTSurfaceRef surface, int32_t uContinuity, int32_t vContinuity, - double* outUParams, int32_t maxUParams, - double* outVParams, int32_t maxVParams) { + double* outUParams, int32_t* outUIndices, int32_t maxU, + double* outVParams, int32_t* outVIndices, int32_t maxV) { OCCTSurfaceKnotSplitResult result = {}; if (!surface) return result; try { @@ -1826,17 +1828,27 @@ OCCTSurfaceKnotSplitResult OCCTSurfaceKnotSplitting(OCCTSurfaceRef surface, GeomConvert_BSplineSurfaceKnotSplitting splitter(bsurf, uContinuity, vContinuity); result.nbUSplits = splitter.NbUSplits(); result.nbVSplits = splitter.NbVSplits(); - if (outUParams && maxUParams > 0) { + if (outUParams && maxU > 0) { occtWriteKnotSplitParams(result.nbUSplits, [&](int32_t i) { return splitter.USplitValue(i); }, [&](int32_t idx) { return bsurf->UKnot(idx); }, - outUParams, maxUParams); + outUParams, maxU); } - if (outVParams && maxVParams > 0) { + if (outUIndices && maxU > 0) { + occtWriteKnotSplits(result.nbUSplits, + [&](int32_t i) { return (int32_t)splitter.USplitValue(i); }, + outUIndices, maxU); + } + if (outVParams && maxV > 0) { occtWriteKnotSplitParams(result.nbVSplits, [&](int32_t i) { return splitter.VSplitValue(i); }, [&](int32_t idx) { return bsurf->VKnot(idx); }, - outVParams, maxVParams); + outVParams, maxV); + } + if (outVIndices && maxV > 0) { + occtWriteKnotSplits(result.nbVSplits, + [&](int32_t i) { return (int32_t)splitter.VSplitValue(i); }, + outVIndices, maxV); } } catch (...) {} return result; @@ -3805,47 +3817,10 @@ bool OCCTBRepGPropFaceBoundaryIntegration(OCCTShapeRef face, int32_t edgeIndex, } catch (...) { return false; } } -// MARK: - v0.105: GeomConvert_BSplineSurfaceKnotSplitting -// MARK: - GeomConvert_BSplineSurfaceKnotSplitting (v0.105.0) - -#include -#include - -int32_t OCCTBSplineSurfaceKnotSplitsU(OCCTSurfaceRef surface, int32_t continuity) { - if (!surface) return 0; - try { - Handle(Geom_BSplineSurface) bsurf = Handle(Geom_BSplineSurface)::DownCast(surface->surface); - if (bsurf.IsNull()) return 0; - GeomConvert_BSplineSurfaceKnotSplitting splitter(bsurf, continuity, continuity); - return (int32_t)splitter.NbUSplits(); - } catch (...) { return 0; } -} - -int32_t OCCTBSplineSurfaceKnotSplitsV(OCCTSurfaceRef surface, int32_t continuity) { - if (!surface) return 0; - try { - Handle(Geom_BSplineSurface) bsurf = Handle(Geom_BSplineSurface)::DownCast(surface->surface); - if (bsurf.IsNull()) return 0; - GeomConvert_BSplineSurfaceKnotSplitting splitter(bsurf, continuity, continuity); - return (int32_t)splitter.NbVSplits(); - } catch (...) { return 0; } -} - -void OCCTBSplineSurfaceKnotSplitValues(OCCTSurfaceRef surface, int32_t continuity, - int32_t* uSplits, int32_t* vSplits) { - if (!surface || !uSplits || !vSplits) return; - try { - Handle(Geom_BSplineSurface) bsurf = Handle(Geom_BSplineSurface)::DownCast(surface->surface); - if (bsurf.IsNull()) return; - GeomConvert_BSplineSurfaceKnotSplitting splitter(bsurf, continuity, continuity); - for (int i = 1; i <= splitter.NbUSplits(); i++) { - uSplits[i - 1] = splitter.USplitValue(i); - } - for (int i = 1; i <= splitter.NbVSplits(); i++) { - vSplits[i - 1] = splitter.VSplitValue(i); - } - } catch (...) {} -} +// #562: OCCTBSplineSurfaceKnotSplitsU/V and OCCTBSplineSurfaceKnotSplitValues stood here, +// a second wrap of GeomConvert_BSplineSurfaceKnotSplitting added three releases after +// OCCTSurfaceKnotSplitting (line 1817 of this file) already wrapped it. Deleted; that one now +// reports the split knot-table indices too, which is the only thing these carried that it did not. // MARK: - v0.106: GC_MakeConical/Cylindrical/TrimmedCone/TrimmedCylinder + Surface continuity // MARK: - GC_MakeConicalSurface (v0.106.0) diff --git a/Sources/OCCTSwift/Curve2D.swift b/Sources/OCCTSwift/Curve2D.swift index 905dbf32..131628ec 100644 --- a/Sources/OCCTSwift/Curve2D.swift +++ b/Sources/OCCTSwift/Curve2D.swift @@ -687,10 +687,21 @@ public final class Curve2D: @unchecked Sendable { /// saturates there (#480). /// - Returns: Array of knot indices where the curve drops below the requested continuity, or nil if not a B-spline. public func splitIndicesAtDiscontinuities(continuity: ParametricContinuity = .c1) -> [Int]? { - var buffer = [Int32](repeating: 0, count: 256) - let n = Int(OCCTCurve2DSplitAtDiscontinuities(handle, continuity.rawValue, &buffer, 256)) + // Read-then-retry, the #481 pattern the rest of this family shares: the bridge reports the + // true split count even when it wrote fewer, so one retry sized to it is always enough. + // Before #562 this read a fixed 256 entries and took whatever came back, so a curve with + // more splits than that was silently cut off at 256 with nothing to notice it by. + func read(capacity: Int) -> (count: Int, buffer: [Int32]) { + var buffer = [Int32](repeating: 0, count: capacity) + let n = Int(OCCTCurve2DSplitAtDiscontinuities(handle, continuity.rawValue, + &buffer, Int32(capacity))) + return (n, buffer) + } + + var (n, buffer) = read(capacity: 256) guard n > 0 else { return nil } - return (0.. 256 { (n, buffer) = read(capacity: n) } + return buffer.prefix(n).map(Int.init) } /// Approximate this curve as a sequence of arcs and line segments. diff --git a/Sources/OCCTSwift/Document.swift b/Sources/OCCTSwift/Document.swift index 3c19f656..a8a4b33f 100644 --- a/Sources/OCCTSwift/Document.swift +++ b/Sources/OCCTSwift/Document.swift @@ -8108,64 +8108,57 @@ extension Curve2D { } } -// MARK: - GeomConvert_BSplineSurfaceKnotSplitting (v0.105.0) - -// `continuity` throughout both sections below is the same derivative-order ContinuityRange -// `Surface.knotSplitting` documents: a knot splits only when `degree - multiplicity < -// continuity`, so the meaningful range is 0...degree and it saturates there. These five -// entry points drive the same two OCCT analyzers as `Surface.knotSplitting` and -// `Curve2D.splitIndicesAtDiscontinuities` and differ only in what they return. #480. +// MARK: - Knot splitting, the v0.105.0 spellings (deprecated, #562) + +// These five entry points were added over `GeomConvert_BSplineSurfaceKnotSplitting` and +// `Geom2dConvert_BSplineCurveKnotSplitting` three releases after `Surface.knotSplitting` and +// `Curve2D.splitIndicesAtDiscontinuities` already wrapped those same two analyzers. Nothing +// reconciled them, so the same question had two spellings that could not be asked to differ: +// both of these take one continuity for both parametric directions where the surface's canonical +// call takes one per direction, and neither reaches a knot the canonical call cannot. +// +// Each now forwards to its canonical sibling. Their own five bridge functions are gone. +// `continuity` is the same derivative-order ContinuityRange throughout: a knot splits only when +// `degree - multiplicity < continuity`, so the meaningful range is 0...degree and it saturates +// there (#480). extension Surface { /// Get number of U-direction knot splits for a BSpline surface at given continuity. - /// - /// Same count as `knotSplitting(uContinuity:vContinuity:)`'s `uSplitCount`. + @available(*, deprecated, + message: "Use knotSplitting(uContinuity:vContinuity:).uSplitCount, which asks the same analyzer once instead of three times and can ask U and V different questions (#562)") public func bsplineKnotSplitsU(continuity: ParametricContinuity) -> Int { - Int(OCCTBSplineSurfaceKnotSplitsU(handle, continuity.rawValue)) + knotSplitting(uContinuity: continuity, vContinuity: continuity).uSplitCount } /// Get number of V-direction knot splits for a BSpline surface at given continuity. - /// - /// Same count as `knotSplitting(uContinuity:vContinuity:)`'s `vSplitCount`. + @available(*, deprecated, + message: "Use knotSplitting(uContinuity:vContinuity:).vSplitCount, which asks the same analyzer once instead of three times and can ask U and V different questions (#562)") public func bsplineKnotSplitsV(continuity: ParametricContinuity) -> Int { - Int(OCCTBSplineSurfaceKnotSplitsV(handle, continuity.rawValue)) + knotSplitting(uContinuity: continuity, vContinuity: continuity).vSplitCount } /// Get U and V knot split index arrays. - /// - /// Indices are 1-based into the surface's own U/V knot tables; `bsplineUKnot(index:)` and - /// `bsplineVKnot(index:)` turn them into parameters, which is what - /// `knotSplitting(uContinuity:vContinuity:)` returns directly. + @available(*, deprecated, + message: "Use knotSplitting(uContinuity:vContinuity:), whose uSplitIndices/vSplitIndices are these same indices and which also gives you the parameters they resolve to (#562)") public func bsplineKnotSplitValues(continuity: ParametricContinuity) -> (uSplits: [Int32], vSplits: [Int32]) { - let nu = bsplineKnotSplitsU(continuity: continuity) - let nv = bsplineKnotSplitsV(continuity: continuity) - var uSplits = [Int32](repeating: 0, count: max(nu, 1)) - var vSplits = [Int32](repeating: 0, count: max(nv, 1)) - OCCTBSplineSurfaceKnotSplitValues(handle, continuity.rawValue, &uSplits, &vSplits) - return (Array(uSplits.prefix(nu)), Array(vSplits.prefix(nv))) + let result = knotSplitting(uContinuity: continuity, vContinuity: continuity) + return (result.uSplitIndices.map(Int32.init), result.vSplitIndices.map(Int32.init)) } } -// MARK: - Geom2dConvert_BSplineCurveKnotSplitting (v0.105.0) - extension Curve2D { /// Get number of knot splits for a 2D BSpline curve at given continuity. - /// - /// Same count as `splitIndicesAtDiscontinuities(continuity:)`'s array length. + @available(*, deprecated, + message: "Use splitIndicesAtDiscontinuities(continuity:)?.count, the same analyzer under one spelling (#562)") public func bsplineKnotSplits(continuity: ParametricContinuity) -> Int { - Int(OCCTBSplineCurve2dKnotSplits(handle, continuity.rawValue)) + splitIndicesAtDiscontinuities(continuity: continuity)?.count ?? 0 } /// Get knot split indices for a 2D BSpline curve at given continuity. - /// - /// The same indices `splitIndicesAtDiscontinuities(continuity:)` returns, as `[Int32]` - /// and empty rather than `nil` for a non-BSpline curve. + @available(*, deprecated, + message: "Use splitIndicesAtDiscontinuities(continuity:), which returns these same indices as [Int] and nil rather than [] for a non-BSpline curve (#562)") public func bsplineKnotSplitValues(continuity: ParametricContinuity) -> [Int32] { - let n = bsplineKnotSplits(continuity: continuity) - guard n > 0 else { return [] } - var splits = [Int32](repeating: 0, count: n) - OCCTBSplineCurve2dKnotSplitValues(handle, continuity.rawValue, &splits) - return splits + splitIndicesAtDiscontinuities(continuity: continuity)?.map(Int32.init) ?? [] } } diff --git a/Sources/OCCTSwift/Surface.swift b/Sources/OCCTSwift/Surface.swift index 66dfd888..441776e4 100644 --- a/Sources/OCCTSwift/Surface.swift +++ b/Sources/OCCTSwift/Surface.swift @@ -1710,6 +1710,16 @@ extension Surface { public let uSplitParams: [Double] /// V parameter values (ascending, bounded by the surface's own V range) at each split public let vSplitParams: [Double] + /// 1-based indices into the surface's own U knot table, one per split: + /// `uSplitParams[i] == bsplineUKnot(index: uSplitIndices[i])`. + /// + /// The analyzer reports indices and this call converts them, so before #562 the raw form + /// was reachable only through a separate `bsplineKnotSplitValues(continuity:)`, which + /// constructed the analyzer three more times to get it. + public let uSplitIndices: [Int] + /// 1-based indices into the surface's own V knot table, one per split: + /// `vSplitParams[i] == bsplineVKnot(index: vSplitIndices[i])`. + public let vSplitIndices: [Int] } /// Analyze where a BSpline surface would need to be split to achieve @@ -1732,6 +1742,10 @@ extension Surface { /// let all = bsplineSurface.knotSplitting(uContinuity: .c3, vContinuity: .c3) /// // all.uSplitParams / all.vSplitParams are real U/V parameters, usable to split the /// // surface into same-continuity patches. + /// + /// // The raw knot-table indices are here too, and agree with the parameters by construction. + /// let sameThing = all.uSplitIndices.map { bsplineSurface.bsplineUKnot(index: $0) } + /// // sameThing == all.uSplitParams /// ``` /// /// - Parameters: @@ -1743,31 +1757,37 @@ extension Surface { /// API for the every-knot split at the far end of that ladder (#480). /// - vContinuity: Minimum continuity to require of each V patch, against the V degree /// and V knots - /// - Returns: Split counts plus the actual U/V parameter values, or all-empty/zero for - /// a non-BSpline surface + /// - Returns: Split counts, the actual U/V parameter values, and the knot-table indices those + /// parameters were read from, or all-empty/zero for a non-BSpline surface public func knotSplitting(uContinuity: ParametricContinuity = .c1, vContinuity: ParametricContinuity = .c1) -> KnotSplitResult { // Same retry-on-truncation pattern as Curve3D.continuityBreaks: the bridge always // reports the true split counts even when it writes fewer, so one retry sized to // those counts is always enough. - func read(uCapacity: Int32, vCapacity: Int32) -> (OCCTSurfaceKnotSplitResult, [Double], [Double]) { + typealias Read = (OCCTSurfaceKnotSplitResult, [Double], [Double], [Int32], [Int32]) + func read(uCapacity: Int32, vCapacity: Int32) -> Read { var uParams = [Double](repeating: 0, count: Int(uCapacity)) var vParams = [Double](repeating: 0, count: Int(vCapacity)) + var uIndices = [Int32](repeating: 0, count: Int(uCapacity)) + var vIndices = [Int32](repeating: 0, count: Int(vCapacity)) let result = OCCTSurfaceKnotSplitting(handle, uContinuity.rawValue, vContinuity.rawValue, - &uParams, uCapacity, &vParams, vCapacity) - return (result, uParams, vParams) + &uParams, &uIndices, uCapacity, + &vParams, &vIndices, vCapacity) + return (result, uParams, vParams, uIndices, vIndices) } - var (result, uParams, vParams) = read(uCapacity: 64, vCapacity: 64) + var (result, uParams, vParams, uIndices, vIndices) = read(uCapacity: 64, vCapacity: 64) if result.nbUSplits > 64 || result.nbVSplits > 64 { - (result, uParams, vParams) = read(uCapacity: max(result.nbUSplits, 1), - vCapacity: max(result.nbVSplits, 1)) + (result, uParams, vParams, uIndices, vIndices) = read(uCapacity: max(result.nbUSplits, 1), + vCapacity: max(result.nbVSplits, 1)) } return KnotSplitResult( uSplitCount: Int(result.nbUSplits), vSplitCount: Int(result.nbVSplits), uSplitParams: Array(uParams.prefix(Int(result.nbUSplits))), - vSplitParams: Array(vParams.prefix(Int(result.nbVSplits))) + vSplitParams: Array(vParams.prefix(Int(result.nbVSplits))), + uSplitIndices: uIndices.prefix(Int(result.nbUSplits)).map(Int.init), + vSplitIndices: vIndices.prefix(Int(result.nbVSplits)).map(Int.init) ) } diff --git a/Tests/OCCTGeom2dTests/Issue480Curve2DKnotSplitContinuityTests.swift b/Tests/OCCTGeom2dTests/Issue480Curve2DKnotSplitContinuityTests.swift index 43b9eb97..f86603ac 100644 --- a/Tests/OCCTGeom2dTests/Issue480Curve2DKnotSplitContinuityTests.swift +++ b/Tests/OCCTGeom2dTests/Issue480Curve2DKnotSplitContinuityTests.swift @@ -52,15 +52,22 @@ struct Issue480Curve2DKnotSplitContinuityTests { #expect(curve.splitIndicesAtDiscontinuities(continuity: .c0) == [1, 6]) } - @Test("The v0.105 count/values spellings agree with the canonical one") + /// #562 forwarded the v0.105 spellings onto `splitIndicesAtDiscontinuities`, which is exactly + /// what makes "the two agree" stop being evidence: one implementation cannot disagree with + /// itself. The expectations here are the literal knot indices of the fixture instead, so a + /// defect that moved both spellings together would still be caught. + @Test("The deprecated v0.105 count/values spellings still report the fixture's own knots") + @available(*, deprecated, message: "exercises the deprecated v0.105 spellings on purpose") func alternateSpellingsAgree() throws { let curve = try #require(bsplineCurve(degree: 3, interiorKnots: 4)) - for continuity in ParametricContinuity.allCases { - let canonical = curve.splitIndicesAtDiscontinuities(continuity: continuity) ?? [] - #expect(curve.bsplineKnotSplits(continuity: continuity) == canonical.count, - "count at \(continuity)") - #expect(curve.bsplineKnotSplitValues(continuity: continuity) == canonical.map(Int32.init), + // Knot table is 0...5, so 1-based indices run 1...6, and a cubic with simple interior + // knots is already C2 there: only .c3 reaches past the two bracketing knots. + for continuity: ParametricContinuity in [.c0, .c1, .c2] { + #expect(curve.bsplineKnotSplits(continuity: continuity) == 2, "count at \(continuity)") + #expect(curve.bsplineKnotSplitValues(continuity: continuity) == [1, 6], "values at \(continuity)") } + #expect(curve.bsplineKnotSplits(continuity: .c3) == 6) + #expect(curve.bsplineKnotSplitValues(continuity: .c3) == [1, 2, 3, 4, 5, 6]) } } diff --git a/Tests/OCCTGeom2dTests/Issue562Curve2DKnotSplitDuplicateTests.swift b/Tests/OCCTGeom2dTests/Issue562Curve2DKnotSplitDuplicateTests.swift new file mode 100644 index 00000000..5ceea98f --- /dev/null +++ b/Tests/OCCTGeom2dTests/Issue562Curve2DKnotSplitDuplicateTests.swift @@ -0,0 +1,81 @@ +import Testing +import simd +@testable import OCCTSwift + +/// #562: `Geom2dConvert_BSplineCurveKnotSplitting` was wrapped twice, by +/// `Curve2D.splitIndicesAtDiscontinuities` and by the v0.105.0 +/// `bsplineKnotSplits`/`bsplineKnotSplitValues` pair. The issue called the second pair strictly +/// weaker; it was not. It sized its buffer from the analyzer's own count, where the canonical +/// call read a fixed 256 entries and took whatever came back, so on a curve with more splits than +/// that the *duplicate* was the only one telling the truth. Collapsing onto the canonical spelling +/// without fixing that would have regressed it, so the truncation is fixed here and guarded below. +@Suite("Curve2D knot-splitting duplicates (#562)") +struct Issue562Curve2DKnotSplitDuplicateTests { + + /// A degree-`d` 2D BSpline whose every interior knot has multiplicity `d`: continuity there is + /// `d - d = 0`, so every knot is a split at C1 or above. `interiorKnots` knots in gives + /// `interiorKnots + 2` splits out (#481's fixture, in 2D). + private func denselyKinkedCurve(degree: Int, interiorKnots: Int) -> Curve2D? { + var knots: [Double] = [0] + var mults: [Int32] = [Int32(degree + 1)] + for i in 1...interiorKnots { + knots.append(Double(i)) + mults.append(Int32(degree)) + } + knots.append(Double(interiorKnots + 1)) + mults.append(Int32(degree + 1)) + + let nPoles = Int(mults.reduce(0, +)) - degree - 1 + let poles = (0..(Double($0), Double($0 % 3)) } + return Curve2D.bspline(poles: poles, knots: knots, multiplicities: mults, degree: degree) + } + + /// The regression the deduplication had to clear first. 300 interior knots at multiplicity 3 + /// on a cubic is 302 splits, against a 256-entry first pass. + /// + /// `last == 302` is the assertion that proves the retry re-read the full set: under truncation + /// the indices are `1...256`, so `last == count` would hold either way (#481). + @Test("A curve with more splits than the first-pass buffer reports all of them") + func moreSplitsThanTheFirstPassBuffer() throws { + let curve = try #require(denselyKinkedCurve(degree: 3, interiorKnots: 300)) + let indices = try #require(curve.splitIndicesAtDiscontinuities(continuity: .c1)) + #expect(indices.count == 302) + #expect(indices.last == 302) + #expect(indices.first == 1) + } + + /// The count is exact at the boundary too, where an off-by-one in the retry would show up. + @Test("The retry boundary is exact at 255, 256 and 257 splits") + func retryBoundaryIsExact() throws { + for splits in [255, 256, 257] { + let curve = try #require(denselyKinkedCurve(degree: 3, interiorKnots: splits - 2)) + let indices = curve.splitIndicesAtDiscontinuities(continuity: .c1) + #expect(indices?.count == splits, "at \(splits) splits") + #expect(indices?.last == splits, "last index at \(splits) splits") + } + } + + /// The deprecated pair now forwards, so it inherits the fix rather than being the only spelling + /// that had it. Absolute expectations, not agreement with the canonical call, which cannot + /// disagree with itself any more. + @Test("The deprecated pair forwards and inherits the untruncated count") + @available(*, deprecated, message: "exercises the deprecated v0.105 spellings on purpose") + func deprecatedPairForwards() throws { + let curve = try #require(denselyKinkedCurve(degree: 3, interiorKnots: 300)) + #expect(curve.bsplineKnotSplits(continuity: .c1) == 302) + let values = curve.bsplineKnotSplitValues(continuity: .c1) + #expect(values.count == 302) + #expect(values.last == 302) + } + + /// Both spellings' non-BSpline answers are preserved by the forwarding: `nil` from the + /// canonical call, `0`/`[]` from the deprecated pair. A line is not a BSpline. + @Test("A non-BSpline curve keeps each spelling's own empty answer") + @available(*, deprecated, message: "exercises the deprecated v0.105 spellings on purpose") + func nonBSplineAnswers() throws { + let line = try #require(Curve2D.line(through: .zero, direction: SIMD2(1, 0))) + #expect(line.splitIndicesAtDiscontinuities(continuity: .c1) == nil) + #expect(line.bsplineKnotSplits(continuity: .c1) == 0) + #expect(line.bsplineKnotSplitValues(continuity: .c1).isEmpty) + } +} diff --git a/Tests/OCCTGeom2dTests/OCCTGeom2dTests.swift b/Tests/OCCTGeom2dTests/OCCTGeom2dTests.swift index 63d72e2b..44abe00f 100644 --- a/Tests/OCCTGeom2dTests/OCCTGeom2dTests.swift +++ b/Tests/OCCTGeom2dTests/OCCTGeom2dTests.swift @@ -2996,12 +2996,9 @@ struct BSplineCurve2dKnotSplitTests { @Test func knotSplits() { // Create a 2D BSpline curve from interpolation if let c = Curve2D.interpolate(through: [SIMD2(0, 0), SIMD2(1, 1), SIMD2(2, 0), SIMD2(3, 1)]) { - let n = c.bsplineKnotSplits(continuity: .c0) - #expect(n >= 0) - if n > 0 { - let vals = c.bsplineKnotSplitValues(continuity: .c0) - #expect(vals.count == n) - } + // #562: was bsplineKnotSplits/bsplineKnotSplitValues, both now deprecated onto this one. + let indices = c.splitIndicesAtDiscontinuities(continuity: .c0) + #expect((indices?.count ?? 0) >= 0) } } } diff --git a/Tests/OCCTSurfaceTests/Issue562SurfaceKnotSplitDuplicateTests.swift b/Tests/OCCTSurfaceTests/Issue562SurfaceKnotSplitDuplicateTests.swift new file mode 100644 index 00000000..86af0464 --- /dev/null +++ b/Tests/OCCTSurfaceTests/Issue562SurfaceKnotSplitDuplicateTests.swift @@ -0,0 +1,104 @@ +import Testing +import simd +@testable import OCCTSwift + +/// #562: `GeomConvert_BSplineSurfaceKnotSplitting` was wrapped twice, by `Surface.knotSplitting` +/// and by the v0.105.0 `bsplineKnotSplitsU`/`bsplineKnotSplitsV`/`bsplineKnotSplitValues` trio. +/// The trio carried one thing the canonical call did not: the raw knot-table indices, where the +/// canonical call reported only the parameters it had converted them into. So the canonical result +/// now carries the indices too, and the trio forwards to it — one analyzer construction per query +/// instead of the three `bsplineKnotSplitValues` needed. +@Suite("Surface knot-splitting duplicates (#562)") +struct Issue562SurfaceKnotSplitDuplicateTests { + + private func bsplineSphere() -> Surface? { + Surface.sphere(center: .zero, radius: 5)?.toBSpline() + } + + /// The indices are the analyzer's own output and the parameters are `UKnot`/`VKnot` of them, + /// so this identity is what makes the deprecated trio's index form redundant rather than lost. + @Test("Split indices resolve through the knot table to exactly the split parameters") + func indicesResolveToParams() throws { + let surface = try #require(bsplineSphere()) + for continuity: ParametricContinuity in ParametricContinuity.allCases { + let result = surface.knotSplitting(uContinuity: continuity, vContinuity: continuity) + #expect(result.uSplitIndices.count == result.uSplitCount, "u count at \(continuity)") + #expect(result.vSplitIndices.count == result.vSplitCount, "v count at \(continuity)") + #expect(result.uSplitIndices.map { surface.bsplineUKnot(index: $0) } == result.uSplitParams, + "u indices at \(continuity)") + #expect(result.vSplitIndices.map { surface.bsplineVKnot(index: $0) } == result.vSplitParams, + "v indices at \(continuity)") + } + } + + /// Indices are 1-based into the surface's own knot table, and the first and last knots are + /// always split points, so the set always brackets the whole table. Absolute bounds, checked + /// against the knot table rather than against the other spelling. + @Test("Indices are 1-based and bracket the surface's own knot table") + func indicesAreOneBasedAndBracketing() throws { + let surface = try #require(bsplineSphere()) + let uKnots = surface.bsplineUKnots() + let vKnots = surface.bsplineVKnots() + let result = surface.knotSplitting(uContinuity: .c1, vContinuity: .c1) + + #expect(result.uSplitIndices.first == 1) + #expect(result.uSplitIndices.last == uKnots.count) + #expect(result.vSplitIndices.first == 1) + #expect(result.vSplitIndices.last == vKnots.count) + #expect(result.uSplitIndices == result.uSplitIndices.sorted()) + #expect(result.vSplitIndices == result.vSplitIndices.sorted()) + } + + /// The question the deprecated trio structurally could not ask: one continuity per parametric + /// direction. Each direction's answer must depend on its own continuity and not the other's. + @Test("U and V answer their own continuity independently") + func directionsAreIndependent() throws { + let surface = try #require(bsplineSphere()) + let both = surface.knotSplitting(uContinuity: .c3, vContinuity: .c3) + let uOnly = surface.knotSplitting(uContinuity: .c3, vContinuity: .c0) + let vOnly = surface.knotSplitting(uContinuity: .c0, vContinuity: .c3) + let neither = surface.knotSplitting(uContinuity: .c0, vContinuity: .c0) + + #expect(uOnly.uSplitCount == both.uSplitCount) + #expect(uOnly.vSplitCount == neither.vSplitCount) + #expect(vOnly.vSplitCount == both.vSplitCount) + #expect(vOnly.uSplitCount == neither.uSplitCount) + // .c0 short-circuits to the two bracketing knots, so this is a real difference to detect. + #expect(neither.uSplitCount == 2) + #expect(neither.vSplitCount == 2) + #expect(both.uSplitCount > 2) + } + + /// The deprecated trio forwards. Absolute expectations against the knot table, since agreement + /// with the canonical call is no longer independent evidence. + @Test("The deprecated trio forwards to the canonical call") + @available(*, deprecated, message: "exercises the deprecated v0.105 spellings on purpose") + func deprecatedTrioForwards() throws { + let surface = try #require(bsplineSphere()) + let uKnots = surface.bsplineUKnots() + let vKnots = surface.bsplineVKnots() + + #expect(surface.bsplineKnotSplitsU(continuity: .c0) == 2) + #expect(surface.bsplineKnotSplitsV(continuity: .c0) == 2) + + let (u, v) = surface.bsplineKnotSplitValues(continuity: .c3) + #expect(u == Array(1...Int32(uKnots.count))) + #expect(v == Array(1...Int32(vKnots.count))) + } + + /// A plane is not a BSpline surface, so every spelling reports nothing rather than crashing on + /// the buffers it was about to size from a count it never got. + @Test("A non-BSpline surface reports no splits at all") + @available(*, deprecated, message: "exercises the deprecated v0.105 spellings on purpose") + func nonBSplineSurface() throws { + let plane = try #require(Surface.plane(origin: .zero, normal: SIMD3(0, 0, 1))) + let result = plane.knotSplitting() + #expect(result.uSplitCount == 0) + #expect(result.uSplitIndices.isEmpty) + #expect(result.vSplitIndices.isEmpty) + #expect(plane.bsplineKnotSplitsU(continuity: .c1) == 0) + let (u, v) = plane.bsplineKnotSplitValues(continuity: .c1) + #expect(u.isEmpty) + #expect(v.isEmpty) + } +} diff --git a/Tests/OCCTSurfaceTests/OCCTSurfaceTests.swift b/Tests/OCCTSurfaceTests/OCCTSurfaceTests.swift index 1ee76c13..cbdac3f9 100644 --- a/Tests/OCCTSurfaceTests/OCCTSurfaceTests.swift +++ b/Tests/OCCTSurfaceTests/OCCTSurfaceTests.swift @@ -3360,7 +3360,8 @@ struct BSplineSurfaceKnotSplitTests { // Create a sphere surface and convert to BSpline if let sphere = Surface.sphere(center: .zero, radius: 5) { if let bsp = sphere.toBSpline() { - let n = bsp.bsplineKnotSplitsU(continuity: .c0) + // #562: was bsplineKnotSplitsU, now deprecated onto this one analyzer call. + let n = bsp.knotSplitting(uContinuity: .c0, vContinuity: .c0).uSplitCount #expect(n >= 0) } } diff --git a/docs/API_REFERENCE.md b/docs/API_REFERENCE.md index 5bf429df..ceb951b1 100644 --- a/docs/API_REFERENCE.md +++ b/docs/API_REFERENCE.md @@ -360,8 +360,8 @@ a map of the major areas, and the `Total` as the count. | **GCPnts_UniformAbscissa** | 4 | uniform arc-length points by count/distance, full/subrange | | **GeomConvert_CompCurveToBSpline** | 1 | concatenate bounded 3D curves into BSpline | | **Geom2dConvert_CompCurveToBSpline** | 1 | concatenate bounded 2D curves into BSpline | -| **GeomConvert_BSplineSurfaceKnotSplitting** | 3 | surface knot splits U/V count and values | -| **Geom2dConvert_BSplineCurveKnotSplitting** | 2 | 2D curve knot split count and values | +| **GeomConvert_BSplineSurfaceKnotSplitting** | 3 | *deprecated (#562)* — surface knot splits U/V count and values; forwards to `Surface.knotSplitting`, which wraps the same analyzer | +| **Geom2dConvert_BSplineCurveKnotSplitting** | 2 | *deprecated (#562)* — 2D curve knot split count and values; forwards to `Curve2D.splitIndicesAtDiscontinuities`, which wraps the same analyzer | | **BndLib Extras** | 6 | ellipse, cone, circleArc, ellipseArc, parabolaArc, hyperbolaArc bounds | | **GProp Torus** | 2 | torus surface area, torus volume | | **BRepTools_ReShape** | 8 | create, release, clear, remove, replace, isRecorded, apply, value | diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index f3b9aaa5..68fb5df5 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -75,6 +75,59 @@ implementation fails 7 of the 10, and the 3 that pass are exactly the deliberate (a point with a perpendicular foot, an out-of-range index, and the pre-existing in-range case). Bridge-only — no kernel patch, no `OCCT.xcframework` rebuild. +#### Five knot-splitting spellings collapse onto two, and the "strictly weaker" duplicate turned out to be the stronger one (#562) + +`GeomConvert_BSplineSurfaceKnotSplitting` and `Geom2dConvert_BSplineCurveKnotSplitting` were each +wrapped twice, by two families added three releases apart: `Surface.knotSplitting` and +`Curve2D.splitIndicesAtDiscontinuities` (canonical), and a v0.105.0 set of five +(`Surface.bsplineKnotSplitsU`/`bsplineKnotSplitsV`/`bsplineKnotSplitValues`, +`Curve2D.bsplineKnotSplits`/`bsplineKnotSplitValues`). All five are now deprecated and forward to +their canonical sibling; their five bridge functions are deleted. + +**The premise that the five were strictly weaker did not survive measurement.** +`Curve2D.bsplineKnotSplitValues` sized its buffer from the analyzer's own count, where +`splitIndicesAtDiscontinuities` read a fixed 256 entries and took whatever came back — and the +bridge returned the count it had *written*, so truncation was indistinguishable from a curve with +exactly 256 splits. On a cubic with 300 interior knots at multiplicity 3 (302 splits): + +| call | before | now | +|---|---|---| +| `splitIndicesAtDiscontinuities(continuity: .c1)` | 256 indices, last `256` | 302 indices, last `302` | +| `bsplineKnotSplitValues(continuity: .c1)` | 302 | 302 | + +Forwarding without fixing that would have regressed the deprecated spelling, so +`OCCTCurve2DSplitAtDiscontinuities` now reports the true count and the Swift caller re-reads at it — +the #481 contract every other member of this family already shared. That is a **C-layer contract +change**: a direct bridge caller that treated the return as "how many were written" must now clamp +it. `OCCTBridge` is not an SPM product, so no Swift package is affected. + +**What the deleted family carried that the canonical calls did not: the raw knot-table indices.** +The analyzer reports indices and `OCCTSurfaceKnotSplitting` converted them to parameters, so the raw +form was reachable only through `bsplineKnotSplitValues` — which constructed the analyzer three more +times to get it, once per count call and once for the values. `KnotSplitResult` now carries +`uSplitIndices`/`vSplitIndices` alongside the parameters, from the one construction that was already +happening, with `uSplitParams[i] == bsplineUKnot(index: uSplitIndices[i])` by construction. That +answers the issue's open question about whether the index-returning form was worth keeping: the +information was, the three extra entry points were not. + +Both deleted values functions also took no buffer capacity at all — each wrote `NbSplits()` entries +into a buffer the caller had sized from a *separate* call, which was only safe because the analyzer +is deterministic. Recorded in the bridge header so it is not reintroduced. + +`OCCTBridge.h`'s cross-reference index named none of the three `*KnotSplitting` conversion classes, +which is half of why the double-wrap went unnoticed for three releases — the index is the map used +to find every call site of a class (#510). It gains `GeomConvert_BSplineCurveKnotSplitting`, +`GeomConvert_BSplineSurfaceKnotSplitting`, and a `--- Geom2dConvert ---` section that did not exist +at all, censused by call site across its six classes. + +Tests: `Issue562Curve2DKnotSplitDuplicateTests` (`OCCTGeom2dTests`, 4 tests) and +`Issue562SurfaceKnotSplitDuplicateTests` (`OCCTSurfaceTests`, 5 tests). Expectations are absolute — +the fixture's own knot indices and knot table — rather than agreement between the two spellings, +which stopped being evidence the moment one started forwarding to the other. Three injected defects +(written-count truncation, 0-based indices, V continuity collapsed onto U) each fail the tests that +should catch them; the last is caught by the new suite alone and by none of the existing #403 or +#480 coverage. + #### The cross-reference index stops naming 135 symbols that never existed (#510) `OCCTBridge.h` opens with a hand-maintained index mapping each wrapped OCCT class to the bridge diff --git a/docs/reference/Curve2D.md b/docs/reference/Curve2D.md index 041e9027..123059ea 100644 --- a/docs/reference/Curve2D.md +++ b/docs/reference/Curve2D.md @@ -964,11 +964,16 @@ a parameter. The first and last knots are always included, so a curve that never - **Parameters:** `continuity`: minimum continuity to require of each arc. - **Returns:** Array of knot indices where continuity drops below the requested level, or `nil` if not a BSpline or no discontinuities found. -- **OCCT:** `Geom2dConvert_BSplineCurveKnotSplitting` (via `OCCTCurve2DSplitAtDiscontinuities`). +- **OCCT:** `Geom2dConvert_BSplineCurveKnotSplitting` (via `OCCTCurve2DSplitAtDiscontinuities`) — + the sole wrapper of it, since #562 deleted the second pair (`bsplineKnotSplits`, + `bsplineKnotSplitValues`) that also drove it. - **Continuity range (#480):** the continuity is a *derivative order*, and a knot splits only when `degree - multiplicity < continuity`, so the meaningful range is `0...degree` and it saturates there. A cubic with simple interior knots is already C2 there, which means `.c0`, `.c1` and `.c2` all report just the two end knots and `.c3` is the order that reports the interior ones. +- **All splits, however many (#562):** this used to read a fixed 256 entries and take whatever came + back, so a curve with more splits than that was cut off at 256 with nothing to notice it by. It + now re-reads at the true count when the first pass came up short. - **Example:** ```swift if let bsp = Curve2D.bspline(poles: [...], knots: [...], multiplicities: [...], degree: 3) { diff --git a/docs/reference/Document-Geometry-Constructors.md b/docs/reference/Document-Geometry-Constructors.md index dba132ef..f6cee402 100644 --- a/docs/reference/Document-Geometry-Constructors.md +++ b/docs/reference/Document-Geometry-Constructors.md @@ -9,7 +9,7 @@ This page covers geometry construction and analysis utilities added across v0.10 ## Topics -- [GC_MakeParabola2d](#gc_makeparabola2d) · [GCPnts_UniformAbscissa](#gcpnts_uniformabscissa) · [GeomConvert CompCurveToBSplineCurve](#geomconvert-compculvetobsplinecurve) · [Geom2dConvert CompCurveToBSplineCurve](#geom2dconvert-compcurvetobsplinecurve) · [GeomConvert BSplineSurfaceKnotSplitting](#geomconvert-bsplinesurfaceknotsplitting) · [Geom2dConvert BSplineCurveKnotSplitting](#geom2dconvert-bsplinecurveknotsplitting) · [BndLib extras](#bndlib-extras) · [GProp Torus](#gprop-torus) · [BRepTools_ReShape](#breptools_reshape) · [BRepTools_Substitution](#breptools_substitution) · [BRepLib_MakeVertex](#breplib_makevertex) · [BRepFill_PipeShell](#brepfill_pipeshell) · [OSD_Directory](#osd_directory) · [IntAna Cone-Sphere extensions](#intana-cone-sphere-extensions) · [XCAFPrs_DocumentExplorer extensions](#xcafprs_documentexplorer-extensions) · [Resource_Unicode](#resource_unicode) · [GProp weighted point sets](#gprop-weighted-point-sets) · [Draft info types](#draft-info-types) · [GeomLib_LogSample](#geomlib_logsample) · [GC_MakeConicalSurface](#gc_makeconicalsurface) · [GC_MakeCylindricalSurface](#gc_makecylindricalsurface) · [GC_MakeTrimmedCone](#gc_maketrimmedcone) · [GC_MakeTrimmedCylinder](#gc_maketrimmedcylinder) · [BRepLib_MakeEdge2d extensions](#breplib_makeedge2d-extensions) · [ShapeAnalysis_Wire](#shapeanalysis_wire) · [ShapeAnalysis_Edge](#shapeanalysis_edge) · [OSD_DirectoryIterator](#osd_directoryiterator) · [OSD_FileIterator](#osd_fileiterator) · [BRepFill_PipeShell extensions](#brepfill_pipeshell-extensions) +- [GC_MakeParabola2d](#gc_makeparabola2d) · [GCPnts_UniformAbscissa](#gcpnts_uniformabscissa) · [GeomConvert CompCurveToBSplineCurve](#geomconvert-compculvetobsplinecurve) · [Geom2dConvert CompCurveToBSplineCurve](#geom2dconvert-compcurvetobsplinecurve) · [Knot splitting, the deprecated v0.105.0 spellings](#geomconvert-bsplinesurfaceknotsplitting--geom2dconvert-bsplinecurveknotsplitting) · [BndLib extras](#bndlib-extras) · [GProp Torus](#gprop-torus) · [BRepTools_ReShape](#breptools_reshape) · [BRepTools_Substitution](#breptools_substitution) · [BRepLib_MakeVertex](#breplib_makevertex) · [BRepFill_PipeShell](#brepfill_pipeshell) · [OSD_Directory](#osd_directory) · [IntAna Cone-Sphere extensions](#intana-cone-sphere-extensions) · [XCAFPrs_DocumentExplorer extensions](#xcafprs_documentexplorer-extensions) · [Resource_Unicode](#resource_unicode) · [GProp weighted point sets](#gprop-weighted-point-sets) · [Draft info types](#draft-info-types) · [GeomLib_LogSample](#geomlib_logsample) · [GC_MakeConicalSurface](#gc_makeconicalsurface) · [GC_MakeCylindricalSurface](#gc_makecylindricalsurface) · [GC_MakeTrimmedCone](#gc_maketrimmedcone) · [GC_MakeTrimmedCylinder](#gc_maketrimmedcylinder) · [BRepLib_MakeEdge2d extensions](#breplib_makeedge2d-extensions) · [ShapeAnalysis_Wire](#shapeanalysis_wire) · [ShapeAnalysis_Edge](#shapeanalysis_edge) · [OSD_DirectoryIterator](#osd_directoryiterator) · [OSD_FileIterator](#osd_fileiterator) · [BRepFill_PipeShell extensions](#brepfill_pipeshell-extensions) --- @@ -192,116 +192,40 @@ public static func concatenate(_ curves: [Curve2D], tolerance: Double = 1e-4) -> --- -## GeomConvert BSplineSurfaceKnotSplitting +## GeomConvert BSplineSurfaceKnotSplitting / Geom2dConvert BSplineCurveKnotSplitting -Extensions on `Surface` wrapping `GeomConvert_BSplineSurfaceKnotSplitting`. +Five entry points documented here — `Surface.bsplineKnotSplitsU(continuity:)`, +`Surface.bsplineKnotSplitsV(continuity:)`, `Surface.bsplineKnotSplitValues(continuity:)`, +`Curve2D.bsplineKnotSplits(continuity:)` and `Curve2D.bsplineKnotSplitValues(continuity:)` — are +**deprecated as of #562**. They were added in v0.105.0 over the same two analyzers that +[`Surface.knotSplitting(uContinuity:vContinuity:)`](Surface-Advanced.md#knotsplittingucontinuityvcontinuity) +and [`Curve2D.splitIndicesAtDiscontinuities(continuity:)`](Curve2D.md) had already been wrapping for +three releases, and each took one continuity for both parametric directions where the surface's +canonical call takes one per direction — so they could not ask a question the canonical call could +not, only fewer of them. -`continuity` throughout this section and the next is a `ParametricContinuity` read as a *derivative -order*: a knot splits only when `degree - multiplicity < continuity`, so the meaningful range is -`0...degree` and it saturates there. A cubic with simple interior knots is already C2 at every -interior knot, which means `.c0`, `.c1` and `.c2` all report just the two bracketing knots and -`.c3` is the order that reports the interior ones (#480). These five entry points drive the same -two analyzers as [`Surface.knotSplitting`](Surface-Advanced.md) and -[`Curve2D.splitIndicesAtDiscontinuities`](Curve2D.md), differing only in what they return. +Each now forwards to its canonical sibling; their own bridge functions are gone. The one thing they +carried that the canonical calls did not — the raw 1-based knot-table indices, rather than the +parameters those indices resolve to — is now `KnotSplitResult.uSplitIndices` / `.vSplitIndices`. -### `Surface.bsplineKnotSplitsU(continuity:)` - -Number of U-direction knot split points required to achieve the specified continuity. Same count -as `knotSplitting(uContinuity:vContinuity:)`'s `uSplitCount`. - -```swift -public func bsplineKnotSplitsU(continuity: ParametricContinuity) -> Int -``` - -- **Parameters:** `continuity`: minimum continuity to require of each U patch. -- **Returns:** Count of U split indices. -- **OCCT:** `GeomConvert_BSplineSurfaceKnotSplitting::NbUSplits` -- **Example:** - ```swift - let n = bsplineSurf.bsplineKnotSplitsU(continuity: .c3) - ``` - ---- - -### `Surface.bsplineKnotSplitsV(continuity:)` - -Number of V-direction knot split points required to achieve the specified continuity. Same count -as `knotSplitting(uContinuity:vContinuity:)`'s `vSplitCount`. - -```swift -public func bsplineKnotSplitsV(continuity: ParametricContinuity) -> Int -``` - -- **Parameters:** `continuity`: minimum continuity to require of each V patch. -- **Returns:** Count of V split indices. -- **OCCT:** `GeomConvert_BSplineSurfaceKnotSplitting::NbVSplits` -- **Example:** - ```swift - let n = bsplineSurf.bsplineKnotSplitsV(continuity: .c3) - ``` - ---- - -### `Surface.bsplineKnotSplitValues(continuity:)` - -Retrieve both U and V knot-split index arrays. `bsplineUKnot(index:)` and `bsplineVKnot(index:)` -turn them into parameters, which is what `knotSplitting(uContinuity:vContinuity:)` returns directly. - -```swift -public func bsplineKnotSplitValues(continuity: ParametricContinuity) -> (uSplits: [Int32], vSplits: [Int32]) -``` - -- **Parameters:** `continuity`: minimum continuity to require of each patch. -- **Returns:** Tuple of U-split and V-split knot index arrays (1-based OCCT knot indices). -- **OCCT:** `GeomConvert_BSplineSurfaceKnotSplitting::Splitting` -- **Example:** - ```swift - let (uIdx, vIdx) = bsplineSurf.bsplineKnotSplitValues(continuity: .c3) - ``` - ---- - -## Geom2dConvert BSplineCurveKnotSplitting - -Extensions on `Curve2D` wrapping `Geom2dConvert_BSplineCurveKnotSplitting`. - -### `Curve2D.bsplineKnotSplits(continuity:)` - -Number of knot split points required to achieve the specified continuity for a 2D BSpline curve. -Same count as `splitIndicesAtDiscontinuities(continuity:)`'s array length. +| deprecated | use | +|---|---| +| `Surface.bsplineKnotSplitsU(continuity:)` | `knotSplitting(uContinuity:vContinuity:).uSplitCount` | +| `Surface.bsplineKnotSplitsV(continuity:)` | `knotSplitting(uContinuity:vContinuity:).vSplitCount` | +| `Surface.bsplineKnotSplitValues(continuity:)` | `knotSplitting(uContinuity:vContinuity:).uSplitIndices` / `.vSplitIndices` | +| `Curve2D.bsplineKnotSplits(continuity:)` | `splitIndicesAtDiscontinuities(continuity:)?.count` | +| `Curve2D.bsplineKnotSplitValues(continuity:)` | `splitIndicesAtDiscontinuities(continuity:)` | ```swift -public func bsplineKnotSplits(continuity: ParametricContinuity) -> Int -``` +// Was: three analyzer constructions, one continuity for both directions. +let (uIdx, vIdx) = bsplineSurf.bsplineKnotSplitValues(continuity: .c3) -- **Parameters:** `continuity`: minimum continuity to require of each arc. -- **Returns:** Count of split indices. -- **OCCT:** `Geom2dConvert_BSplineCurveKnotSplitting::NbSplits` -- **Example:** - ```swift - let n = curve2d.bsplineKnotSplits(continuity: .c3) - ``` - ---- - -### `Curve2D.bsplineKnotSplitValues(continuity:)` - -Retrieve the knot-split index array for a 2D BSpline curve. The same indices -`splitIndicesAtDiscontinuities(continuity:)` returns, as `[Int32]` and empty rather than `nil` -for a non-BSpline curve. - -```swift -public func bsplineKnotSplitValues(continuity: ParametricContinuity) -> [Int32] +// Now: one construction, and U and V can be asked different questions. +let splits = bsplineSurf.knotSplitting(uContinuity: .c3, vContinuity: .c1) +let uIndices = splits.uSplitIndices // the same 1-based knot indices +let uParams = splits.uSplitParams // and what they resolve to ``` -- **Parameters:** `continuity`: minimum continuity to require of each arc. -- **Returns:** Array of knot split indices, or empty if none. -- **OCCT:** `Geom2dConvert_BSplineCurveKnotSplitting::Splitting` -- **Example:** - ```swift - let splits = curve2d.bsplineKnotSplitValues(continuity: .c3) - ``` - --- ## BndLib extras diff --git a/docs/reference/Surface-Advanced.md b/docs/reference/Surface-Advanced.md index 4c2d6411..9730648e 100644 --- a/docs/reference/Surface-Advanced.md +++ b/docs/reference/Surface-Advanced.md @@ -584,16 +584,20 @@ public func knotSplitting( ) -> KnotSplitResult ``` -Returns the number of U and V splits needed, plus the actual U/V parameter values at each -split; does not modify the surface. Each direction's own first and last knots are always -included, so a direction that never drops below the requested continuity reports exactly those -two rather than nothing. +Returns the number of U and V splits needed, the actual U/V parameter values at each split, and +the knot-table indices those parameters were read from; does not modify the surface. Each +direction's own first and last knots are always included, so a direction that never drops below +the requested continuity reports exactly those two rather than nothing. - **Parameters:** `uContinuity`: minimum continuity to require of each U patch; `vContinuity`: the same against the V degree and V knots. -- **Returns:** `KnotSplitResult` with `uSplitCount`/`vSplitCount` and `uSplitParams`/`vSplitParams` - (ascending, bounded by the surface's own U/V domain). -- **OCCT:** `GeomConvert_BSplineSurfaceKnotSplitting`. +- **Returns:** `KnotSplitResult` with `uSplitCount`/`vSplitCount`, `uSplitParams`/`vSplitParams` + (ascending, bounded by the surface's own U/V domain) and `uSplitIndices`/`vSplitIndices` + (1-based into the surface's own knot tables, so + `uSplitParams[i] == bsplineUKnot(index: uSplitIndices[i])`). +- **OCCT:** `GeomConvert_BSplineSurfaceKnotSplitting` — the sole wrapper of it, since #562 deleted + the second family (`bsplineKnotSplitsU`/`bsplineKnotSplitsV`/`bsplineKnotSplitValues`) that also + drove it. The indices are what that family carried and this call previously discarded. - **Continuity range (#480):** the continuity is a *derivative order*, and a knot splits only when `degree - multiplicity < continuity`. So the meaningful range is `0...degree` and it saturates there. A bicubic surface with simple interior knots is already C2 at every interior knot, which @@ -610,6 +614,9 @@ two rather than nothing. let result = surf.knotSplitting(uContinuity: .c3, vContinuity: .c3) print("U splits needed:", result.uSplitCount, result.uSplitParams) print("V splits needed:", result.vSplitCount, result.vSplitParams) + + // The raw knot indices, and the identity that ties them to the parameters. + print(result.uSplitIndices.map { surf.bsplineUKnot(index: $0) } == result.uSplitParams) // true ``` ---