Modeling - Guard silently-uncapped solid in BRepOffsetAPI_ThruSections

[gsdali]

Pre-Submission Checks

• I checked existing issues, pull requests, discussions, and forum topics for related work.
• I followed the contribution guidance in .github/CONTRIBUTING.md.
• I used a PR title in the Group - Summary format.

Problem / Motivation

BRepOffsetAPI_ThruSections(isSolid: true) silently omits both end-cap faces when a closed
section wire has two or more periods of out-of-plane variation around the loop, a genuinely
non-planar closed curve. Build() still calls Done() and reports IsDone() == true, and
Shape() returns a TopoDS_Solid that is missing two faces and is not actually closed.
BRepCheck_Analyzer catches this later, but with no localized error (errorCount == 0, no
detailed check statuses), so a caller has already trusted IsDone() by the time anything flags
it.

Root cause: MakeSolid() (the static helper both CreateRuled() and CreateSmoothed() call to
close a loft's two open ends) caps each end via PerformPlan(), which tries
BRepBuilderAPI_FindPlane and then a BRepLib_FindSurface-backed MakeFace(wire) fallback.
Neither finds anything but a plane or a surface already attached to the wire's edges. A section
wire with k >= 2 periods of out-of-plane variation around the loop has no such surface, so
PerformPlan() fails for it. MakeSolid() already tracks this in its own local B, threaded
through both PerformPlan() calls — and discards it: it force-marks the shell and solid
Closed(true) unconditionally before returning, and the caller then calls Done()
unconditionally too.

A single period of out-of-plane variation (e.g. z = amp * cos(theta) at constant radius) caps
fine, because that curve is secretly planar: it is exactly the intersection of the cylinder
r = const with a tilted plane, so BRepBuilderAPI_FindPlane finds it. k >= 2 has no such
plane and no other analytic surface either.

Found downstream of a bevel gear ThruSections loft in the OCCTSwift ecosystem, where each
section is a toothed profile whose out-of-plane period equals the tooth count, always >= 2
for a real gear. Bisected in
SecondMouseAU/OCCTSwiftScripts#108
away from every property of the gear down to this single geometric trigger. The resulting
healed()/fixSolid()-demotes-solid-to-shell symptom was filed and fixed downstream as
SecondMouseAU/OCCTSwift#702; this
capping omission is the root cause Open-Cascade-SAS#702 never investigated, filed as its own issue at
SecondMouseAU/OCCTSwift#905.

Proposed Solution

This PR previously guarded both call sites on myFirst.IsNull() || myLast.IsNull(). That
attempt is superseded by this version — CI caught a real regression in it (see "First attempt"
below), and the corrected fix lives entirely inside MakeSolid() instead.

MakeSolid()'s local B already distinguishes exactly the two cases that matter: it stays
true when capping either succeeded or wasn't needed (PerformPlan()'s own degenerate-wire
shortcut — every edge of the wire is degenerate, a single-vertex "point" section built via
AddVertex(), e.g. a cone's apex, which needs no cap face and correctly reports success with a
null face), and ends false only when capping was genuinely attempted and failed. After the
capping block, if B is still false, throw StdFail_NotDone, matching the exception this same
function already throws for a null shell two lines above:

if (!B)
{
  throw StdFail_NotDone("BRepOffsetAPI_ThruSections: could not close a non-planar extremity");
}

No signature change and no call-site change: both call sites already run inside Build()'s only
try/catch (catch (Standard_Failure const&) { NotDone(); return; }), so the throw propagates
there and IsDone() correctly reads false.

First attempt, and why it's replaced. The original version of this PR guarded on
myFirst.IsNull() || myLast.IsNull() at both call sites instead. That looks equivalent and is
not: PerformPlan() also leaves the output face null, and returns true (no failure), when the
wire is degenerate — so a null-face check cannot tell "no cap needed" apart from "cap needed and
failed." CI caught this directly: BOPAlgo_PaveFillerTest.FuseConeLoftWithBox_DegeneratedEdge (a
circle-to-vertex loft, always legitimately capped) regressed to IsDone() == false. This version
fixes that: verified by compiling and running that exact upstream GTest, unmodified, against the
corrected fix (see Validation).

This does not make BRepOffsetAPI_ThruSections able to cap a genuinely non-planar wire (that
would need a real capping algorithm, e.g. an N-sided patch, out of scope here). It converts a
silent wrong success into a correctly reported failure, matching every other failure path in this
file.

Limitation, unchanged from before: GetStatus() is not updated by this fix and stays
BRepFill_ThruSectionErrorStatus_Done on this path. Build()'s catch block doesn't touch
myStatus for any exception source, not one this fix introduces — IsDone() is the correct,
fixed signal.

Validation

Checks performed:

• clang-format --dry-run --Werror against this tree's own .clang-format: zero violations
  on both changed files.
• Compiled both changed files with clang++ -std=c++17 -fsyntax-only against the public
  headers bundled in the downstream OCCTSwift repo's OCCT.xcframework (V8_0_1 baseline):
  clean.
• Override-link tested (compile the changed .cxx standalone, link it ahead of the
  prebuilt libOCCT-macos.a, no full OCCT rebuild): built two versions of
  BRepOffsetAPI_ThruSections.cxx, unpatched and patched, and linked each against the new
  BRepOffsetAPI_ThruSections_Test.cxx.
  • Against the unpatched override, the new NonPlanarClosedWireCappingFails test fails
    exactly as described above: IsDone() reads true for a two-wire k=2 loft.
  • Against the patched override, it passes, along with the new
    DegenerateVertexEndStillSucceeds test and all three pre-existing tests in the file
    (OCC10006_LoftAndFusion, BSplineProfilesWithDifferentPoleCount,
    OCC895_TwoCircularArcWires_NoTwist).
  • Also compiled and ran the actual BOPAlgo_PaveFiller_Test.cxx
FuseConeLoftWithBox_DegeneratedEdge test, unmodified, against the patched override: passes
    — direct confirmation this fix does not reintroduce the first attempt's regression, not just
    an equivalent local test standing in for it. Confirmed (with the same setup) that it does
    fail, with the identical failure message CI reported, under the first attempt's null-face
    guard.
• Not built against a full OCCT tree/GTest suite via CMake: no local OCCT build toolchain was
  available in this environment; the override-link technique above compiles and links the
  real changed sources against the real prebuilt OCCT archive, which is the closest available
  substitute. This PR's own CI (GTest jobs on macOS/Linux/Windows) is the authoritative check.

Review Notes

Reproduced against the OCCTSwift 2.0.0 xcframework before writing this patch:
raw.shapeType == .Solid, subShapeCount(ofType: .face) == 180 (wall-only; a working k <= 1
loft produces 182, wall + 2 caps), checkResult.isValid == false, checkResult.errorCount == 0,
detailedCheckStatuses == [].

🤖 Generated with Claude Code

[gsdali]

Pushed a corrected commit fixing the regression the previous run found.

Root cause of the CI failure: the guard checked myFirst.IsNull() || myLast.IsNull(), but a null
cap face means two different things MakeSolid() can't distinguish that way — capping genuinely
failed, or capping wasn't needed because the wire is degenerate (a point section from
AddVertex(), e.g. the cone's apex in FuseConeLoftWithBox_DegeneratedEdge). PerformPlan()
already makes this distinction internally (its isDegen shortcut returns success with a null
face), and threads it through a local bool B that MakeSolid() was discarding. The fix now
checks B after the capping block instead of face-nullness, and throws StdFail_NotDone (the
same exception this function already throws for a null shell two lines above) rather than
threading a new status flag through both call sites.

Before pushing, compiled and ran the actual BOPAlgo_PaveFillerTest.FuseConeLoftWithBox_DegeneratedEdge
test (unmodified) against an override-link of the corrected .cxx: passes. Also confirmed it
reproduces the exact CI failure (Value of: aLoftMaker.IsDone() / Actual: false / Expected: true)
against an override-link of the original null-face guard, to be sure the regression's cause was
correctly identified before changing the fix. Added two new GTest cases in
BRepOffsetAPI_ThruSections_Test.cxx (NonPlanarClosedWireCappingFails,
DegenerateVertexEndStillSucceeds) covering both sides of that distinction. Full details in the
updated PR description.

[gsdali]

Closing this. Opening a PR on this fork itself, to stage and CI-validate a patch before
submitting upstream, isn't how this project's carried patches have ever been done — every prior
one went straight from local override-link validation (compile the changed source standalone,
link it ahead of the prebuilt OCCT archive, no full rebuild) to a PR against
Open-Cascade-SAS/OCCT directly. This PR (this fork's first and only PR to date) was an
unprecedented, erroneous extra step, and its content — both the original null-face guard and the
corrected bool B-based fix pushed here afterward — has been superseded by
Open-Cascade-SAS/OCCT#1462, filed the normal
way. Its branch stays as-is for history; no further work happens here.