From 837874265d7c79bc7668add3b141a32c7c7a43a7 Mon Sep 17 00:00:00 2001 From: gsdali <51393997+gsdali@users.noreply.github.com> Date: Wed, 19 Aug 2026 18:03:29 +1000 Subject: [PATCH] chore(#118): bump OCCTSwift floor to 3.0.0 Two breaking changes, both compile errors: - Selector.SubShapeType.compsolid renamed .compSolid, consolidating four drifted Swift mirrors of TopAbs_ShapeEnum onto ShapeType. Zero source changes needed: this repo already spelled it ShapeType.compSolid. - Shape.bounds/.size/.center, Wire.bounds, Edge.bounds, Face.bounds become Optional instead of fabricating (0,0,0)-(0,0,0) for a shape with no bounding box. Every call site now unwraps: QueryTopology/LoadBrep/ MeasureDeviation/RenderPreview/Metrics throw a named ScriptError on a nil bounding box (a real error on a loaded BREP, not a state to paper over with ?? .zero); the two recipe edge-selector predicates return false on a nil bounds rather than fabricate a match. Added Tests/OcctkitCommandTests/OptionalBoundsTests.swift, a regression suite that constructs a genuinely void shape (the intersection of two disjoint boxes) and asserts LoadBrepCommand.buildResponse and MeasureDeviationCommand.defaultDeflection throw rather than fabricate a zero-size box. swift build, swift test (12/12), and the recipe smoke suite (all 7 recipes, matching their reference output.brep exactly) all verified clean locally against the real v3.0.0 sibling checkout. The rest of the cohort (OCCTSwiftTools/Mesh/IO/AIS) has not yet released an OCCTSwift-3.0.0-compatible version, and OCCTSwiftAIS has 3 of its own unfixed .bounds call sites, so neither a fresh clone/CI run nor an as-is local sibling build can resolve the full graph yet. Package.resolved is left untouched, matching the #111 precedent. See the new OKF decision entry for the full detail; this repo's release should wait for the cohort, same situation as the 2.0.0 bump. Co-Authored-By: Claude Sonnet 5 --- CLAUDE.md | 2 +- Package.swift | 26 ++++++- Sources/occtkit/Commands/LoadBrep.swift | 6 +- .../occtkit/Commands/MeasureDeviation.swift | 8 ++- Sources/occtkit/Commands/Metrics.swift | 6 +- Sources/occtkit/Commands/QueryTopology.swift | 10 ++- Sources/occtkit/Commands/RenderPreview.swift | 5 +- .../OptionalBoundsTests.swift | 55 +++++++++++++++ ...-edge-classifier-can-select-wrong-edges.md | 7 +- okf/decisions/index.md | 4 ++ ...0-floor-bump-blocked-on-cohort-releases.md | 70 +++++++++++++++++++ okf/log.md | 21 ++++++ recipes/01-mounting-bracket/main.swift | 3 +- recipes/03-pipe-flange/main.swift | 6 +- 14 files changed, 211 insertions(+), 18 deletions(-) create mode 100644 Tests/OcctkitCommandTests/OptionalBoundsTests.swift create mode 100644 okf/decisions/occtswift-3.0.0-floor-bump-blocked-on-cohort-releases.md diff --git a/CLAUDE.md b/CLAUDE.md index fa38753..0e4f6dc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -121,7 +121,7 @@ No tests exist. No linter is configured. The full cohort graduated to v1.0 on 2026-05-07 alongside OCCT 8.0.0 GA. SemVer-stable from these floors; only bump on documented breaking changes. Pre-1.0 dep history (which API landed in which 0.x tag) lives in git log; consult it when you actually need to support an older floor, otherwise treat the v1.0 surface as the contract. -- **OCCTSwift**: `https://github.com/SecondMouseAU/OCCTSwift.git` (>= 2.0.0; xcframework built against **OCCT 8.0.1**). The B-Rep kernel: ~400+ methods for parametric CAD, the full ISO drawings stack (Sheet/TitleBlock/ProjectionSymbol/Section2D/Hatch/AutoCentermarks/CuttingPlaneLine/CosmeticThread/SurfaceFinish/GDT/DetailView/DrawingScale), `FeatureReconstructor` for `reconstruct`, the `SheetMetal` namespace for `compose-sheet-metal`, and the XCAF surfaces (`AssemblyNode.labelId`, `Document.node(at:)`) for `inspect-assembly` / `set-metadata`. **Floored at v2.0.0** (OCCTSwiftScripts#111): a correctness major (17 breaking changes to the public Swift API; see `docs/SEMVER.md#v200` in the OCCTSwift repo), not a wrapping one. Two fixes landed in this repo alongside the bump: (1) `ShapeAnalysisResult.selfIntersectionCount` was removed (#763; always `0`, never computed), so `Heal.swift`/`GraphValidate.swift` now report `hasSelfIntersection`/`selfIntersecting` as `Bool?` via the real, opt-in `Shape.analyze(selfIntersectionTimeout:)` check (`nil` = "not checked" by default, since the check is ~3000x an ordinary scan on pathological input and both verbs would run it twice), rather than the fabricated always-`0`/always-`false` the removed field silently produced. (2) AAG builds nodes from face **occurrences** (#642): `AAGNode.faceIndex` / `PocketFeature.floorFaceIndex`/`wallFaceIndices` / `detectHoles()`'s `faceIndex` / `AAGEdge.face1Index`/`face2Index` now index `Shape.orientedFaces()`, not the `Shape.faces()` `face[N]` scheme `query-topology` emits (the two agreed automatically pre-2.0.0, since `faces()` was itself occurrence-based then). `FeatureRecognize.swift` (both the `occtkit` command and the legacy standalone target), `GraphSelect.swift`, and `GraphML.swift` all cross-reference AAG output against that `face[N]` scheme and now resolve through the new `AAGNode.distinctFaceIndex` bridge; a no-op on any shape that shares no face (every single-solid part, the only kind this repo's pre-#111 tests exercised), so it only bites a multi-solid compound with a shared face, exactly the shape a caller runs `feature-recognize`/`graph-select`/`graph-ml` against to look for cross-solid structure. `Tests/OcctkitCommandTests/AAGFaceIndexTests.swift` regression-tests the fix directly against `graph-select`/`graph-ml`'s JSON output on a split-box-compound fixture. Previously **floored at v1.17.0** (raised in d5d31e8 for the OCCTSwift#377/#380 Pass 1a duplication and bug-fix audit; also carries the `Shape.drilled` direction fix, OCCTSwift#272, which lands between 1.12.0 and 1.12.9 and corrected recipe 01's through-holes). Before that, **floored at v1.15.0**: v1.15.0 renamed the Swift wrapper class `TopologyGraph` → `BRepGraph` (OCCTSwift#335) to match the C++ package it wraps; this repo has migrated off the deprecated `TopologyGraph` typealias onto `BRepGraph` directly (OCCTSwiftScripts#78), so the floor must guarantee the `BRepGraph` symbol exists. Earlier, v1.7.0 realigned the BRepGraph wrapper to OCCT's redesigned graph model (definitions vs references/usages, persistent UIDs, controlled layers) and v1.7.1 made the derived graph reads real again: `adjacentFaces`/`faces(of:)`/`edges(of:)`/`sharedEdges`, `faceSameDomain`, `faceIsNaturalRestriction`, plus durable `UID`/`RefUID`/`ItemUID` identity. Our graph verbs (graph-validate/compact/dedup/ml, query-topology) build and run **unchanged** against it. Behaviour changes are **confined to the BRepGraph domain**: `edgeMaxContinuity`/`setEdgeRegularity` are now no-ops (use `Shape.maxContinuity` for continuity); `degenerated`/`closed`/`sameParameter`/`sameRange` setters no-op while their getters return the live derived value. The cookbook ergonomics relied on since v1.3.1, namely `Shape.circularPatternCut` (#169), orientation-normalised `Shape.sweep` + `orientedForward`/`signedVolume` (#170), `concaveEdges`/`convexEdges`/`edges(where:)` selectors (#171), are unchanged. **The rest of the cohort has not yet released a version compatible with OCCTSwift 2.0.0**: OCCTSwiftIO's latest release (v1.7.6) still requires `occtswift 1.17.0..<2.0.0` transitively (confirmed by a real remote `swift package resolve` against a fresh checkout with no local sibling), and OCCTSwiftTools/AIS/Mesh's own latest releases are all still on their "repin to 1.17.0" floors too. This repo's own bump is complete and builds clean locally via the sibling-checkout trick (every sibling repo's *own*, not-yet-released local checkout has already moved its floor to 2.0.0 too), but a fresh clone / CI run cannot resolve the dependency graph from remote until at least OCCTSwiftIO ships a 2.0.0-compatible release. Per OCCTSwiftScripts#111's own text, this repo's release should wait for that (and for OCCTSwiftTools's), same as the rest of the cohort. +- **OCCTSwift**: `https://github.com/SecondMouseAU/OCCTSwift.git` (>= 3.0.0; xcframework built against **OCCT 8.0.1**). The B-Rep kernel: ~400+ methods for parametric CAD, the full ISO drawings stack (Sheet/TitleBlock/ProjectionSymbol/Section2D/Hatch/AutoCentermarks/CuttingPlaneLine/CosmeticThread/SurfaceFinish/GDT/DetailView/DrawingScale), `FeatureReconstructor` for `reconstruct`, the `SheetMetal` namespace for `compose-sheet-metal`, and the XCAF surfaces (`AssemblyNode.labelId`, `Document.node(at:)`) for `inspect-assembly` / `set-metadata`. **Floored at v3.0.0** (OCCTSwiftScripts#118): a correctness/consolidation major (OCCT itself stays at 8.0.1; the kernel was only rebuilt to carry two patches the v2.0.0 asset was missing), not a wrapping one. Two breaking changes, both compile errors (see `docs/SEMVER.md#v300` in the OCCTSwift repo): (1) `Selector.SubShapeType.compsolid` renamed `.compSolid`, consolidating four drifted Swift mirrors of `TopAbs_ShapeEnum` onto `ShapeType` — zero source changes needed here, since this repo already spelled it `ShapeType.compSolid` (`LoadBrep.swift`, `Pattern.swift`, `RenderPreview.swift`), the surviving spelling. (2) `Shape.bounds`/`.size`/`.center`, `Wire.bounds`, `Edge.bounds`, `Face.bounds` (and `.exactBounds`, unused here) become `Optional`: they used to fabricate `(0,0,0)-(0,0,0)` for a shape with no bounding box, indistinguishable from a genuine zero-size shape at the world origin (`Shape.boundingBox`/`boundingBoxOptimal()` already behaved correctly and are unchanged). Every `.bounds` call site in this repo now unwraps: `QueryTopology.swift`/`LoadBrep.swift`/`MeasureDeviation.swift`/`RenderPreview.swift`/`Metrics.swift` throw a named `ScriptError` on a `nil` bounding box (a real error on a loaded BREP, not a state worth papering over with `?? .zero`), and the two recipe edge-selector predicates (`recipes/01-mounting-bracket`, `recipes/03-pipe-flange`) return `false` on a `nil` bounds rather than fabricate a match. `Tests/OcctkitCommandTests/OptionalBoundsTests.swift` regression-tests the throw path directly: it constructs a genuinely void shape (the intersection of two disjoint boxes) and asserts `LoadBrepCommand.buildResponse`/`MeasureDeviationCommand.defaultDeflection` throw rather than fabricate a zero-size box. **The rest of the cohort has not yet released an OCCTSwift-3.0.0-compatible version**: OCCTSwiftTools/Mesh's latest releases (and OCCTSwiftIO's, which has no direct OCCTSwift dependency but gates via Tools) all still cap `from: "2.0.0"` (`.upToNextMajor` excludes 3.0.0), and OCCTSwiftAIS inherits the cap transitively through Tools plus has 3 of its own unfixed `.bounds` call sites (`Dimension.swift`, `AreaSelection.swift`) — confirmed by attempting a real local build against the sibling checkouts, which fails inside OCCTSwiftAIS. This repo's own fix is complete and verified (`swift build`, `swift test`, and the recipe smoke suite all pass locally against a temporarily-patched AIS sibling, reverted after verification), but neither a fresh clone/CI run nor an as-is local sibling build can resolve the graph until Tools/Mesh/IO/AIS ship their own 3.0.0 repins. Per OCCTSwiftScripts#118, this repo's release should wait for that, same situation as the 2.0.0 bump. Previously **floored at v2.0.0** (OCCTSwiftScripts#111): a correctness major (17 breaking changes to the public Swift API; see `docs/SEMVER.md#v200` in the OCCTSwift repo). Two fixes landed in this repo alongside that bump: (1) `ShapeAnalysisResult.selfIntersectionCount` was removed (#763; always `0`, never computed), so `Heal.swift`/`GraphValidate.swift` now report `hasSelfIntersection`/`selfIntersecting` as `Bool?` via the real, opt-in `Shape.analyze(selfIntersectionTimeout:)` check (`nil` = "not checked" by default, since the check is ~3000x an ordinary scan on pathological input and both verbs would run it twice), rather than the fabricated always-`0`/always-`false` the removed field silently produced. (2) AAG builds nodes from face **occurrences** (#642): `AAGNode.faceIndex` / `PocketFeature.floorFaceIndex`/`wallFaceIndices` / `detectHoles()`'s `faceIndex` / `AAGEdge.face1Index`/`face2Index` now index `Shape.orientedFaces()`, not the `Shape.faces()` `face[N]` scheme `query-topology` emits (the two agreed automatically pre-2.0.0, since `faces()` was itself occurrence-based then). `FeatureRecognize.swift` (both the `occtkit` command and the legacy standalone target), `GraphSelect.swift`, and `GraphML.swift` all cross-reference AAG output against that `face[N]` scheme and now resolve through the new `AAGNode.distinctFaceIndex` bridge; a no-op on any shape that shares no face (every single-solid part, the only kind this repo's pre-#111 tests exercised), so it only bites a multi-solid compound with a shared face, exactly the shape a caller runs `feature-recognize`/`graph-select`/`graph-ml` against to look for cross-solid structure. `Tests/OcctkitCommandTests/AAGFaceIndexTests.swift` regression-tests the fix directly against `graph-select`/`graph-ml`'s JSON output on a split-box-compound fixture. Before that, **floored at v1.17.0** (raised in d5d31e8 for the OCCTSwift#377/#380 Pass 1a duplication and bug-fix audit; also carries the `Shape.drilled` direction fix, OCCTSwift#272, which lands between 1.12.0 and 1.12.9 and corrected recipe 01's through-holes). Before that, **floored at v1.15.0**: v1.15.0 renamed the Swift wrapper class `TopologyGraph` → `BRepGraph` (OCCTSwift#335) to match the C++ package it wraps; this repo has migrated off the deprecated `TopologyGraph` typealias onto `BRepGraph` directly (OCCTSwiftScripts#78), so the floor must guarantee the `BRepGraph` symbol exists. Earlier, v1.7.0 realigned the BRepGraph wrapper to OCCT's redesigned graph model (definitions vs references/usages, persistent UIDs, controlled layers) and v1.7.1 made the derived graph reads real again: `adjacentFaces`/`faces(of:)`/`edges(of:)`/`sharedEdges`, `faceSameDomain`, `faceIsNaturalRestriction`, plus durable `UID`/`RefUID`/`ItemUID` identity. Our graph verbs (graph-validate/compact/dedup/ml, query-topology) build and run **unchanged** against it. Behaviour changes are **confined to the BRepGraph domain**: `edgeMaxContinuity`/`setEdgeRegularity` are now no-ops (use `Shape.maxContinuity` for continuity); `degenerated`/`closed`/`sameParameter`/`sameRange` setters no-op while their getters return the live derived value. The cookbook ergonomics relied on since v1.3.1, namely `Shape.circularPatternCut` (#169), orientation-normalised `Shape.sweep` + `orientedForward`/`signedVolume` (#170), `concaveEdges`/`convexEdges`/`edges(where:)` selectors (#171), are unchanged. The 2.0.0-bump-era cohort gap (OCCTSwiftIO/Tools/AIS/Mesh all still on their "repin to 1.17.0" floors, blocking remote resolution) resolved by 2026-08-10, when OCCTSwiftIO v1.7.7, OCCTSwiftTools v1.6.3, and OCCTSwiftMesh v1.7.3 each shipped their own "repin OCCTSwift to 2.0.0" release; see the v3.0.0 cohort-gap note above this paragraph for the current (unresolved as of this writing) equivalent. - **OCCTSwiftViewport**: `https://github.com/gsdali/OCCTSwiftViewport.git` (>= 1.0.0). Provides `OffscreenRenderer`, `CameraState`, `DisplayMode`, `ViewportBody` for `render-preview`. Graduated to v1.0.0 on 2026-05-08, one day after the rest of the cohort; floor unblocked by Tools v1.0.2 (closes #45). - **OCCTSwiftTools**: `https://github.com/gsdali/OCCTSwiftTools.git` (>= 1.0.0; resolves to v1.0.2+ for the widened Viewport constraint). Bridge layer between the B-Rep kernel and the Metal viewport. We use `CADFileLoader.shapeToBodyAndMetadata` in `render-preview` for Shape → `ViewportBody` conversion (both input bodies and highlight sub-shapes). Lives in its own repo since Viewport v0.55.0 split. - **OCCTSwiftAIS**: `https://github.com/gsdali/OCCTSwiftAIS.git` (>= 1.0.0). Headless-friendly subset only: `Trihedron` / `WorkPlane` / `Axis` / `PointCloud` scene objects (each emits `[ViewportBody]` via `makeBodies()`) for `render-preview`'s `--show-axes` / `--show-workplane` overlays, plus the SubShape selection vocabulary for `--highlight face[N]/edge[M]/vertex[K]`. Selection / Manipulator / SwiftUI surfaces aren't relevant to a CLI. `Dimension` overlays render via a SwiftUI Canvas inside `MetalViewportView` and so can't reach `OffscreenRenderer`, so `--annotate-dimensions` is deferred (filed as OCCTSwiftViewport#26). Note: `OCCTSwiftAIS` re-exports a `DisplayMode` enum (3 cases) that collides with `OCCTSwiftViewport.DisplayMode` (6 cases); fully-qualify in `RenderPreview.swift` as `OCCTSwiftViewport.DisplayMode`. diff --git a/Package.swift b/Package.swift index bec8e2e..a3cc9f0 100644 --- a/Package.swift +++ b/Package.swift @@ -101,7 +101,31 @@ let package = Package( // Tests/OcctkitCommandTests/AAGFaceIndexTests.swift (added alongside this bump) // started exercising a real shared-face compound. // See docs/SEMVER.md#v200 in the OCCTSwift repo for the full break table. - occtDep("OCCTSwift", from: "2.0.0"), + // Bumped 2.0.0 -> 3.0.0 (OCCTSwiftScripts#118): a correctness/consolidation major, OCCT + // stays at 8.0.1 (kernel rebuilt to carry two patches the v2.0.0 asset was missing). + // Two breaks, both compile errors: + // - `Selector.SubShapeType.compsolid` renamed `.compSolid`, consolidating four drifted + // Swift mirrors of TopAbs_ShapeEnum onto `ShapeType`. Zero hits here: this repo + // already spelled it `ShapeType.compSolid` (LoadBrep.swift, Pattern.swift, + // RenderPreview.swift), which was already the surviving spelling. + // - `Shape.bounds`/`.size`/`.center`, `Wire.bounds`, `Edge.bounds`, `Face.bounds` (and + // `.exactBounds`, unused here) become Optional: they used to fabricate + // `(0,0,0)-(0,0,0)` for a shape with no bounding box, indistinguishable from a + // genuine zero-size shape at the origin. Every `.bounds` call site in this repo now + // unwraps: query-topology/load-brep/measure-distance/render-preview/metrics throw a + // named ScriptError on a nil bounding box (a real error for a loaded BREP, not a + // state to paper over); the two recipe edge-selector predicates return `false` (guard + // against a mid-selection nil rather than fabricate a match). + // See docs/SEMVER.md#v300 in the OCCTSwift repo for the full break table. + // + // The rest of the cohort has not yet released an OCCTSwift-3.0.0-compatible version as of + // this bump (OCCTSwiftTools/Mesh/IO's latest releases all still cap `from: "2.0.0"`, + // i.e. `.upToNextMajor` excludes 3.0.0; OCCTSwiftAIS transitively via Tools). `from:` pins + // below are unaffected by this bump directly, but a fresh clone / CI cannot resolve the + // full graph until they catch up, same situation as the 2.0.0 bump (see git history on + // this comment block). Local builds against sibling checkouts work today because path + // dependencies bypass semver ranges entirely. + occtDep("OCCTSwift", from: "3.0.0"), // RenderPreview rasterizes through Viewport's OffscreenRenderer. // Floored at v1.0.4: v1.0.3 fixes an uncatchable quantize() crash on // body load (Viewport #30) and v1.0.4 makes the published Viewport diff --git a/Sources/occtkit/Commands/LoadBrep.swift b/Sources/occtkit/Commands/LoadBrep.swift index 458fea7..8dc965d 100644 --- a/Sources/occtkit/Commands/LoadBrep.swift +++ b/Sources/occtkit/Commands/LoadBrep.swift @@ -184,8 +184,10 @@ enum LoadBrepCommand: Subcommand { } } - static func buildResponse(bodyId: String, shape: Shape) -> Response { - let bb = shape.bounds + static func buildResponse(bodyId: String, shape: Shape) throws -> Response { + guard let bb = shape.bounds else { + throw ScriptError.message("'\(bodyId)' has no bounding box (void or degenerate shape)") + } return Response( bodyId: bodyId, isValid: shape.isValid, diff --git a/Sources/occtkit/Commands/MeasureDeviation.swift b/Sources/occtkit/Commands/MeasureDeviation.swift index 53f2990..c667bd9 100644 --- a/Sources/occtkit/Commands/MeasureDeviation.swift +++ b/Sources/occtkit/Commands/MeasureDeviation.swift @@ -73,7 +73,7 @@ enum MeasureDeviationCommand: Subcommand { let aShape = try GraphIO.loadBREP(at: req.a) let bShape = try GraphIO.loadBREP(at: req.b) - let defl = req.deflection ?? defaultDeflection(for: aShape) + let defl = try req.deflection ?? defaultDeflection(for: aShape) guard defl > 0 else { throw ScriptError.message("deflection must be positive") } guard let aTris = TriMesh(shape: aShape, deflection: defl) else { @@ -204,8 +204,10 @@ enum MeasureDeviationCommand: Subcommand { // ── geometry helpers ──────────────────────────────────────────────── - static func defaultDeflection(for shape: Shape) -> Double { - let b = shape.bounds + static func defaultDeflection(for shape: Shape) throws -> Double { + guard let b = shape.bounds else { + throw ScriptError.message("shape has no bounding box (void or degenerate shape)") + } let diag = simd_length(b.max - b.min) return Swift.max(diag * 0.005, 1e-6) } diff --git a/Sources/occtkit/Commands/Metrics.swift b/Sources/occtkit/Commands/Metrics.swift index 300fc03..a692e35 100644 --- a/Sources/occtkit/Commands/Metrics.swift +++ b/Sources/occtkit/Commands/Metrics.swift @@ -76,9 +76,11 @@ enum MetricsCommand: Subcommand { wants("volume") || wants("centerOfMass") || wants("principalAxes") ? shape.volumeInertia : nil - let bb: Response.BoundingBox? = { + let bb: Response.BoundingBox? = try { guard wants("boundingBox") else { return nil } - let b = shape.bounds + guard let b = shape.bounds else { + throw ScriptError.message("shape has no bounding box (void or degenerate shape)") + } return .init( min: [b.min.x, b.min.y, b.min.z], max: [b.max.x, b.max.y, b.max.z] diff --git a/Sources/occtkit/Commands/QueryTopology.swift b/Sources/occtkit/Commands/QueryTopology.swift index 6c699a0..73991a2 100644 --- a/Sources/occtkit/Commands/QueryTopology.swift +++ b/Sources/occtkit/Commands/QueryTopology.swift @@ -135,7 +135,10 @@ enum QueryTopologyCommand: Subcommand { let area = face.area() if let m = filter.minArea, area < m { continue } if let m = filter.maxArea, area > m { continue } - let bb = face.bounds + guard let bb = face.bounds else { + throw ScriptError.message( + "face[\(i)] has no bounding box (void or degenerate face)") + } let center = SIMD3( (bb.min.x + bb.max.x) * 0.5, (bb.min.y + bb.max.y) * 0.5, @@ -181,7 +184,10 @@ enum QueryTopologyCommand: Subcommand { let length = edge.length if let m = filter.minLength, length < m { continue } if let m = filter.maxLength, length > m { continue } - let bb = edge.bounds + guard let bb = edge.bounds else { + throw ScriptError.message( + "edge[\(i)] has no bounding box (void or degenerate edge)") + } let center = SIMD3( (bb.min.x + bb.max.x) * 0.5, (bb.min.y + bb.max.y) * 0.5, diff --git a/Sources/occtkit/Commands/RenderPreview.swift b/Sources/occtkit/Commands/RenderPreview.swift index 55cf3e7..2918057 100644 --- a/Sources/occtkit/Commands/RenderPreview.swift +++ b/Sources/occtkit/Commands/RenderPreview.swift @@ -158,7 +158,10 @@ enum RenderPreviewCommand: Subcommand { throw ScriptError.message("Failed to convert '\(path)' to a renderable body") } bodies.append(body) - let bb = shape.bounds + guard let bb = shape.bounds else { + throw ScriptError.message( + "'\(path)' has no bounding box (void or degenerate shape)") + } unionMin = simd_min(unionMin, SIMD3(Float(bb.min.x), Float(bb.min.y), Float(bb.min.z))) unionMax = simd_max(unionMax, SIMD3(Float(bb.max.x), Float(bb.max.y), Float(bb.max.z))) } diff --git a/Tests/OcctkitCommandTests/OptionalBoundsTests.swift b/Tests/OcctkitCommandTests/OptionalBoundsTests.swift new file mode 100644 index 0000000..6dd8de1 --- /dev/null +++ b/Tests/OcctkitCommandTests/OptionalBoundsTests.swift @@ -0,0 +1,55 @@ +// OptionalBoundsTests.swift +// OcctkitCommandTests +// +// Regression coverage for OCCTSwiftScripts#118 / OCCTSwift#943 (v3.0.0): `Shape.bounds` (and +// the other five affected accessors) went from fabricating `(0,0,0)-(0,0,0)` on a shape with +// no bounding box to returning `nil`. Every call site in this repo that reads a bounding box +// off a loaded shape now has to unwrap that `Optional` instead of silently reporting a bogus +// zero-size box. This locks in the unwrap-and-throw behaviour directly, rather than relying on +// the recipe smoke suite (which only exercises the "has real bounds" path). + +import Foundation +import OCCTSwift +import ScriptHarness +import Testing + +@testable import occtkit + +@Suite("occtkit bounds-Optional handling (#118)") +struct OptionalBoundsTests { + + /// The intersection of two boxes with no overlap: a valid, well-formed empty `Shape` + /// (`BRepAlgoAPI_Common` on disjoint inputs succeeds with an empty result rather than + /// failing) whose `Bnd_Box` is void, so every affected accessor reports `nil` per OCCTSwift + /// v3.0.0's own migration note. + private func voidShape() throws -> Shape { + let a = try #require(Shape.box(width: 1, height: 1, depth: 1)) + let bOrigin = try #require(Shape.box(width: 1, height: 1, depth: 1)) + let b = try #require(bOrigin.translated(by: SIMD3(1000, 1000, 1000))) + return try #require(a.intersection(b)) + } + + @Test( + "Two disjoint boxes' intersection has no bounding box (sanity check for the fixture itself)" + ) + func voidShapeHasNilBounds() throws { + let shape = try voidShape() + #expect(shape.bounds == nil) + } + + @Test("LoadBrepCommand.buildResponse throws, not fabricates, on a shape with no bounding box") + func loadBrepThrowsOnVoidBounds() throws { + let shape = try voidShape() + #expect(throws: ScriptError.self) { + _ = try LoadBrepCommand.buildResponse(bodyId: "body_0", shape: shape) + } + } + + @Test("MeasureDeviationCommand.defaultDeflection throws, not fabricates, on a void shape") + func defaultDeflectionThrowsOnVoidBounds() throws { + let shape = try voidShape() + #expect(throws: ScriptError.self) { + _ = try MeasureDeviationCommand.defaultDeflection(for: shape) + } + } +} diff --git a/okf/decisions/concave-edge-classifier-can-select-wrong-edges.md b/okf/decisions/concave-edge-classifier-can-select-wrong-edges.md index 0b715d2..c797145 100644 --- a/okf/decisions/concave-edge-classifier-can-select-wrong-edges.md +++ b/okf/decisions/concave-edge-classifier-can-select-wrong-edges.md @@ -117,8 +117,7 @@ classifier's output: ```swift let insideCorner = prism.edges { edge in - guard edge.isLine else { return false } - let b = edge.bounds + guard edge.isLine, let b = edge.bounds else { return false } let runsFullWidth = abs((b.max.z - b.min.z) - width) < 1e-6 && abs(b.max.x - b.min.x) < 1e-6 && abs(b.max.y - b.min.y) < 1e-6 guard runsFullWidth else { return false } @@ -126,6 +125,10 @@ let insideCorner = prism.edges { edge in } ``` +(`edge.bounds` became `Optional` at OCCTSwift 3.0.0, unrelated to this entry's own finding; the +`let b = edge.bounds` unwrap above was added then, see +[OCCTSwift 3.0.0 floor bump](occtswift-3.0.0-floor-bump-blocked-on-cohort-releases.md).) + # Related [Revolve seams cannot be chamfered](revolve-seams-cannot-be-chamfered.md) already noted, on diff --git a/okf/decisions/index.md b/okf/decisions/index.md index f858e04..505fb87 100644 --- a/okf/decisions/index.md +++ b/okf/decisions/index.md @@ -24,3 +24,7 @@ that need standalone rationale. `Package.swift` floors OCCTSwift at 2.0.0 and this repo's own code is fixed, but OCCTSwiftIO's latest release still caps `occtswift` below 2.0.0 transitively, so a fresh clone cannot resolve until the cohort ships. The PR is not blocked; the release is. + Resolved 2026-08-10. +* [OCCTSwift 3.0.0 floor bump blocked on cohort releases](occtswift-3.0.0-floor-bump-blocked-on-cohort-releases.md): + same shape of blocker, but this time OCCTSwiftAIS has 3 of its own unfixed `.bounds` call + sites, so even a local sibling-checkout build fails, not just remote/CI resolution. diff --git a/okf/decisions/occtswift-3.0.0-floor-bump-blocked-on-cohort-releases.md b/okf/decisions/occtswift-3.0.0-floor-bump-blocked-on-cohort-releases.md new file mode 100644 index 0000000..8bd121b --- /dev/null +++ b/okf/decisions/occtswift-3.0.0-floor-bump-blocked-on-cohort-releases.md @@ -0,0 +1,70 @@ +--- +type: decision +title: OCCTSwift floor bumped to 3.0.0 in source, but the graph cannot resolve until the cohort releases +description: Package.swift now floors OCCTSwift at 3.0.0 and this repo's own code is fixed against both breaks, but OCCTSwiftTools/Mesh's latest releases still cap occtswift below 3.0.0, OCCTSwiftIO gates via Tools, and OCCTSwiftAIS has 3 of its own unfixed .bounds call sites. Neither a fresh clone/CI run nor an as-is local sibling build can resolve until the cohort ships. +resource: https://github.com/SecondMouseAU/OCCTSwiftScripts/issues/118 +tags: [decision, occtswift, dependency-resolution, release-sequencing, semver] +timestamp: 2026-08-19 +--- + +# Decision + +`Package.swift`'s `OCCTSwift` dependency is `from: "3.0.0"` as of OCCTSwiftScripts#118. This +repo's own source is fixed against both v3.0.0 breaks (see `CLAUDE.md`'s dependency entry): the +`Selector.SubShapeType.compsolid` -> `.compSolid` rename needed no change here (already spelled +correctly), and every `.bounds` call site that became `Optional` (`Shape`/`Wire`/`Edge`/`Face`) +now unwraps, throwing a named `ScriptError` in the five `occtkit` verbs that read a bounding box +off a loaded BREP, and returning `false` from the two recipe edge-selector predicates that filter +on one. + +**This is a smaller version of the same blocker as the 2.0.0 bump** +([prior decision](occtswift-2.0.0-floor-bump-blocked-on-cohort-releases.md)), but this time it +also blocks the *local sibling* build, not just remote/CI resolution: + +* OCCTSwiftTools (latest: v1.6.3) and OCCTSwiftMesh (latest: v1.7.4) both still declare + `occtDep("OCCTSwift", from: "2.0.0")`, which SwiftPM's `from:` resolves as `.upToNextMajor`, + i.e. `>=2.0.0, <3.0.0` — excluding 3.0.0. +* OCCTSwiftIO has no direct `OCCTSwift` dependency but gates transitively through Tools. +* OCCTSwiftAIS (latest: v1.3.1) depends on `OCCTSwiftTools >= 1.6.1` and additionally has 3 of its + own `.bounds` call sites that do not yet unwrap: `Dimension.swift:238` (`resolveBody`), + `Dimension.swift:249` (`resolveFace`), `AreaSelection.swift:109` (body-mode hit testing). +* OCCTSwiftScripts depends on all four locally as real sibling checkouts (`../OCCTSwiftTools` etc. + all exist under `/Users/elb/Projects/`), so `occtDep`'s local-path-override trick applies + uniformly across the whole graph, not just to `OCCTSwift` itself. Since path dependencies + bypass semver ranges entirely, Tools/Mesh/IO's version caps do not block a local build by + themselves — but AIS's own unfixed `.bounds` sites are a real compile error in the local graph, + confirmed directly: `swift build` in this repo fails inside + `OCCTSwiftAIS/Sources/OCCTSwiftAIS/AreaSelection.swift:109` and `Dimension.swift:238,249` with + "value of optional type ... must be unwrapped". + +Verified past that point by patching those 3 AIS call sites *locally only* (not committed; AIS's +own repin is out of scope for this repo and reportedly already in progress elsewhere), then running +`swift build`, `swift build --build-tests`, `swift test` (12/12 passing, including the new +`Tests/OcctkitCommandTests/OptionalBoundsTests.swift` regression suite that constructs a +genuinely void shape via the intersection of two disjoint boxes and asserts the new throw paths +fire instead of fabricating a zero-size box), and `Scripts/recipe-check.sh` against a release +build (all 7 recipes match their reference `output.brep`, `Δvol 0.00e+00`). The AIS patch was +reverted immediately after verification, leaving that repo untouched. + +Note for anyone resuming this: `swift test` (unlike `swift build --build-tests`) sometimes pulls +OCCTSwiftAIS's own `Tests/` target into the build plan too, which has 2 more unrelated `.bounds` +sites of its own (`RemapTests.swift`) plus an unrelated pre-existing `SubShape` inference error. +This looked nondeterministic across otherwise-identical runs (plausibly incremental-plan-cache +dependent) and is unrelated to this repo's own correctness; `swift test --skip-build` after a +successful `swift build --build-tests` sidesteps it without touching AIS's test files. + +# What this means for sequencing + +Same conclusion as the 2.0.0 precedent: the **PR** for this repo's own fix is not blocked (review +and merge do not require a resolvable graph from either remote or an unpatched local sibling set). +The **release** (a git tag consumers resolve against) and, this time, **CI on the PR itself** are +both blocked until OCCTSwiftTools and OCCTSwiftMesh ship their own OCCTSwift-3.0.0-compatible +releases and OCCTSwiftAIS ships both its repin and its 3 call-site fixes. `.github/workflows/*.yml` +in this repo use a plain `actions/checkout@v4` with no sibling checkouts, so CI resolves purely +from remote and will fail red on this PR until then, unlike a local sibling build (which fails for +the different, AIS-specific reason above). + +`Package.resolved` is left untouched by the OCCTSwiftScripts#118 change, same reasoning as #111: +regenerating it now would either commit path-relative local-machine state or fail outright against +remote. It refreshes naturally once `swift package resolve`/`swift build` runs after the cohort +catches up. diff --git a/okf/log.md b/okf/log.md index e71ac3a..fe3c4e9 100644 --- a/okf/log.md +++ b/okf/log.md @@ -1,5 +1,26 @@ # Knowledge Log +## 2026-08-19 (chore/118-occtswift-3.0.0) + +* **Update**: Bumped the OCCTSwift floor to 3.0.0 (#118), a correctness/consolidation major (OCCT + itself stays at 8.0.1). Two breaks: `Selector.SubShapeType.compsolid` -> `.compSolid` needed no + source change (this repo already used the surviving `ShapeType.compSolid` spelling); six + bounding-box accessors (`Shape.bounds`/`.size`/`.center`, `Wire.bounds`, `Edge.bounds`, + `Face.bounds`) became `Optional` instead of fabricating `(0,0,0)-(0,0,0)` for a shape with no + bounding box. Fixed every call site: `QueryTopology.swift`, `LoadBrep.swift`, + `MeasureDeviation.swift`, `RenderPreview.swift`, and `Metrics.swift` now throw a named + `ScriptError` on a `nil` bounding box; the two recipe edge-selector predicates + (`01-mounting-bracket`, `03-pipe-flange`) return `false` on `nil` rather than fabricate a match. + Added `Tests/OcctkitCommandTests/OptionalBoundsTests.swift`, constructing a genuinely void + shape (two disjoint boxes' intersection) to regression-test the throw path directly. + `swift build`/`swift test` (12/12) clean and all 7 recipe smoke tests match their reference + `output.brep` exactly (`Δvol 0.00e+00`). +* **Creation**: Recorded the occtswift-3.0.0-floor-bump-blocked-on-cohort-releases decision: this + repo's own bump is complete and verified, but OCCTSwiftTools/Mesh still cap `occtswift` below + 3.0.0 and OCCTSwiftAIS has 3 of its own unfixed `.bounds` sites, so even a local sibling build + fails past that point (verified by a temporary, reverted local-only patch to AIS) — a stricter + blocker than the 2.0.0 bump, which only blocked remote/CI resolution. + ## 2026-08-10 (chore/bump-occtswift-2.0.0) * **Update**: Bumped the OCCTSwift floor to 2.0.0 (#111), a correctness major with 17 breaking diff --git a/recipes/01-mounting-bracket/main.swift b/recipes/01-mounting-bracket/main.swift index 865b12f..5cebecd 100644 --- a/recipes/01-mounting-bracket/main.swift +++ b/recipes/01-mounting-bracket/main.swift @@ -60,8 +60,7 @@ let prism = Shape.extrude(profile: lProfile, direction: SIMD3(0, 0, 1), length: // migration; re-run the check in the OKF entry at that point rather than assuming // the migration carried the fix. let insideCornerEdges = prism.edges { edge in - guard edge.isLine else { return false } - let b = edge.bounds + guard edge.isLine, let b = edge.bounds else { return false } let runsFullWidth = abs((b.max.z - b.min.z) - width) < 1e-6 && abs(b.max.x - b.min.x) < 1e-6 && abs(b.max.y - b.min.y) < 1e-6 guard runsFullWidth else { return false } diff --git a/recipes/03-pipe-flange/main.swift b/recipes/03-pipe-flange/main.swift index 49b8fa6..5246e6f 100644 --- a/recipes/03-pipe-flange/main.swift +++ b/recipes/03-pipe-flange/main.swift @@ -100,8 +100,10 @@ func axisCircleRadius(_ edge: Edge) -> Double? { } let chamferTargets = flange.edges { edge in guard let r = axisCircleRadius(edge) else { return false } - if abs(r - outerRadius) < 1e-3 { return true } // OD, front + back - if abs(r - raisedRadius) < 1e-3 && edge.bounds.min.y > thickness + 1e-3 { return true } // raised-face rim + if abs(r - outerRadius) < 1e-3 { return true } // OD, front + back + if abs(r - raisedRadius) < 1e-3, let b = edge.bounds, b.min.y > thickness + 1e-3 { + return true // raised-face rim + } return false } flange = flange.chamferedWithFullHistory(distance: chamferDistance,