diff --git a/CLAUDE.md b/CLAUDE.md index 5b0e45c..fbdb715 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -70,7 +70,7 @@ OCCTSwiftIO built clean with zero errors while carrying three real breaks in its Dependencies resolve against local siblings when present (`../OCCTSwift` and friends), else the published URLs. No binary lives in this repo. -**Expected baseline: 357 tests across 32 suites, all passing.** +**Expected baseline: 360 tests across 32 suites, all passing.** ## Face identity is `IsSame`, and that decision is settled @@ -142,10 +142,19 @@ down by geometry, not by index, and it was mutation-checked. shape to a BREP string: measured at 5.0ms against a 14-face solid whose mesh takes 9.6ms. Headless consumers of `load` (OCCTDesignLoop's reprojection, batch render and parts extraction) never pick. +**A table's index space is the ordinal space, and that is load-bearing** +([#9](https://github.com/SecondMouseAU/OCCTSwiftInteraction/issues/9)). Ordinals index the *full* +enumeration, because that is what the mesher walks, so the face and edge tables are built with +`map`, never `compactMap`: a dropped element moves every later ordinal down one and the table then +names the sub-shape after the one the pick hit, silently. That is why `FaceIdentityTable.shapes` and +`EdgeIdentityTable.shapes` are `[Shape?]`. `VertexIdentityTable.shapes` is `[Shape]` because +`subShapes(ofType: .vertex)` needs no failable conversion; keep that asymmetry, in both directions. + The bridge's edge-polyline-only branch (`mesh(...)` returned nil) used to substitute an empty `FaceIdentityTable` and now builds the ordinary one. It is reachable from tests only through the internal `edgePolylineOnlyBridge` seam, because a wire, an edge and a lone vertex all mesh to an -empty `Mesh` rather than to nil. +empty `Mesh` rather than to nil. `ShapeIdentity.init` has an internal seam of the same kind, taking +the two sub-shape conversions as parameters, because no public API can make one of them fail. ## One selection, held by `InteractiveContext` @@ -218,11 +227,22 @@ Per-type reference for all three targets is in [docs/reference/](docs/reference/ Covenant text. - **Release pattern**: commit, push, tag, and create a GitHub release with notes. +## Platforms are iOS and macOS, and that is settled + +`Package.swift` declares `.iOS(.v18)` and `.macOS(.v15)`. Nothing else, and do not add anything +else: `OCCT.xcframework`'s `Info.plist` carries exactly three slices, `ios-arm64`, +`ios-arm64-simulator` and `macos-arm64`, supporting two platforms, and OCCTSwift's own v3.0.0 +release notes open with "macOS / iOS (device + simulator)". Anything linking the kernel on visionOS +or tvOS cannot link at all. + +The manifest declared `.visionOS(.v1)` and `.tvOS(.v18)` until the 1.0.0 sweep, inherited from the +union of what the three pre-merge manifests declared. The claim was never true for any of the three. +Root cause is filed upstream as +[OCCTSwift#978](https://github.com/SecondMouseAU/OCCTSwift/issues/978). + ## Still to finish after the merge - `okf/index.md` describes the OCCTSwiftTools half in more detail than the other two. -- The CADKit target's `visionOS`/`tvOS` build is unverified. `Package.swift` keeps the union of what - the three declared, and CADKit declared only iOS and macOS. Confirm or narrow before 1.0.0. ## Ecosystem context worth reading before non-trivial changes diff --git a/Package.swift b/Package.swift index c067fea..455e67b 100644 --- a/Package.swift +++ b/Package.swift @@ -62,15 +62,20 @@ func occtDep(_ name: String, from version: String) -> Package.Dependency { // Modeled on OCCTSwiftUX, which has vended six targets from one package since well before this. let package = Package( name: "OCCTSwiftInteraction", - // The union of what the three carried. OCCTSwiftTools and OCCTSwiftAIS both declared - // visionOS/tvOS; OCCTSwiftCADKit declared only iOS/macOS. Keeping the union rather than the - // intersection avoids regressing the two targets with the most dependents, but it does mean the - // CADKit target's visionOS/tvOS build is unverified. Confirm or narrow before 1.0.0. + // iOS and macOS, and only those two. `OCCT.xcframework`'s `Info.plist` carries exactly three + // slices, `ios-arm64`, `ios-arm64-simulator` and `macos-arm64`, supporting two platforms, and + // OCCTSwift's own v3.0.0 release notes open with "macOS / iOS (device + simulator)". Anything + // linking the kernel on visionOS or tvOS cannot link at all. + // + // This manifest declared `.visionOS(.v1)` and `.tvOS(.v18)` until the 1.0.0 sweep, taking the + // union of what OCCTSwiftTools, OCCTSwiftAIS and OCCTSwiftCADKit declared during the merge so + // as not to regress the two targets with the most dependents. That reasoning was wrong in a way + // invisible from the manifests: the wider claim was never true for any of the three, so there + // was nothing to regress. Root cause is filed upstream as + // https://github.com/SecondMouseAU/OCCTSwift/issues/978. platforms: [ .iOS(.v18), .macOS(.v15), - .visionOS(.v1), - .tvOS(.v18), ], products: [ .library(name: "OCCTSwiftTools", targets: ["OCCTSwiftTools"]), diff --git a/Sources/OCCTSwiftTools/EdgeIdentityTable.swift b/Sources/OCCTSwiftTools/EdgeIdentityTable.swift index 7d88fa5..7f9e1bb 100644 --- a/Sources/OCCTSwiftTools/EdgeIdentityTable.swift +++ b/Sources/OCCTSwiftTools/EdgeIdentityTable.swift @@ -28,20 +28,25 @@ public struct EdgeIdentityTable: Sendable { /// Built from `Shape.edges()`, the same `TopTools_IndexedMapOfShape` traversal /// `Shape.edge(at:)` and the bulk edge-polyline extractor use, so `shapes[ordinal]` is always /// the exact edge behind the segments carrying that ordinal. - public let shapes: [Shape] + /// + /// An element is `nil` when that edge's `Edge` to `Shape` conversion failed. Optional for the + /// same reason `FaceIdentityTable.shapes` is: a shorter array would move every later ordinal + /// down one and name the wrong edge (OCCTSwiftInteraction#9). + public let shapes: [Shape?] /// Durable per-ordinal handle, minted from the `BRepGraph` supplied when the table was built. /// - /// `nil` when no graph was supplied. When present, an individual element is `nil` only if - /// that ordinal's edge could not be resolved in the graph. + /// `nil` when no graph was supplied. When present, an individual element is `nil` if that + /// ordinal's edge could not be resolved in the graph, or has no entry in `shapes`. public let uids: [BRepGraph.GraphUID?]? - public init(shapes: [Shape], uids: [BRepGraph.GraphUID?]? = nil) { + public init(shapes: [Shape?], uids: [BRepGraph.GraphUID?]? = nil) { self.shapes = shapes self.uids = uids } - /// The `Shape` a render-path edge ordinal was extracted from. + /// The `Shape` a render-path edge ordinal was extracted from, or `nil` if the ordinal is out + /// of range or its conversion failed when the table was built. public func shape(forOrdinal ordinal: Int) -> Shape? { shapes.indices.contains(ordinal) ? shapes[ordinal] : nil } diff --git a/Sources/OCCTSwiftTools/FaceIdentityTable.swift b/Sources/OCCTSwiftTools/FaceIdentityTable.swift index de93c41..810fb70 100644 --- a/Sources/OCCTSwiftTools/FaceIdentityTable.swift +++ b/Sources/OCCTSwiftTools/FaceIdentityTable.swift @@ -59,20 +59,26 @@ public struct FaceIdentityTable: Sendable { /// Built from `Shape.faces()`, the same enumeration the mesher uses to assign that ordinal, /// so `shapes[ordinal]` is always the exact face tessellated into the triangles carrying that /// ordinal. - public let shapes: [Shape] + /// + /// An element is `nil` when that face's `Face` to `Shape` conversion failed. Optional so that + /// the index space is the ordinal space by construction: a shorter array would silently move + /// every later face down one and name the wrong one (OCCTSwiftInteraction#9). A `nil` entry + /// costs that one ordinal its captured shape and its uid, and no other. + public let shapes: [Shape?] /// Durable per-ordinal handle, minted from the `BRepGraph` supplied when the table was built. /// - /// `nil` when no graph was supplied. When present, an individual element is `nil` only if - /// that ordinal's face could not be resolved in the graph. + /// `nil` when no graph was supplied. When present, an individual element is `nil` if that + /// ordinal's face could not be resolved in the graph, or has no entry in `shapes`. public let uids: [BRepGraph.GraphUID?]? - public init(shapes: [Shape], uids: [BRepGraph.GraphUID?]? = nil) { + public init(shapes: [Shape?], uids: [BRepGraph.GraphUID?]? = nil) { self.shapes = shapes self.uids = uids } - /// The `Shape` a render-path face ordinal was tessellated from. + /// The `Shape` a render-path face ordinal was tessellated from, or `nil` if the ordinal is out + /// of range or its conversion failed when the table was built. public func shape(forOrdinal ordinal: Int) -> Shape? { shapes.indices.contains(ordinal) ? shapes[ordinal] : nil } diff --git a/Sources/OCCTSwiftTools/ShapeIdentity.swift b/Sources/OCCTSwiftTools/ShapeIdentity.swift index 09debde..eeb900e 100644 --- a/Sources/OCCTSwiftTools/ShapeIdentity.swift +++ b/Sources/OCCTSwiftTools/ShapeIdentity.swift @@ -42,6 +42,12 @@ import OCCTSwift /// - **Vertices**: `Shape.subShapes(ofType: .vertex)`, the traversal behind /// `ViewportBody.vertexIndices`. /// +/// The face and edge enumerations need a `Face`/`Edge` to `Shape` conversion on the way, and both +/// conversions are failable, so those two tables hold `[Shape?]` and a failed conversion becomes a +/// hole at its own ordinal rather than a missing element (OCCTSwiftInteraction#9). The vertex +/// enumeration returns `Shape` values already and needs no conversion, which is why it alone holds +/// `[Shape]`. +/// /// Face identity keys on OCCT's `TopoDS_Shape::IsSame` (settled in /// [OCCTSwiftInteraction#1](https://github.com/SecondMouseAU/OCCTSwiftInteraction/issues/1)), so a /// face shared between two shells is one entry rather than two, and `graph.findNode(for:)` matches @@ -89,10 +95,43 @@ public struct ShapeIdentity: Sendable { /// - shape: the shape to enumerate. /// - graph: a graph built from `shape`, or `nil` for shapes-only tables. public init(shape: Shape, graph: BRepGraph?) { + self.init( + shape: shape, graph: graph, faceToShape: Shape.fromFace, edgeToShape: Shape.fromEdge) + } + + /// The construction itself, with the two failable sub-shape conversions supplied by the caller. + /// + /// Internal rather than private so a test can drive a conversion failure, which no public API + /// can provoke: every `Face` and `Edge` the enumerations hand back already holds a live OCCT + /// handle, so the bridge calls behind `Shape.fromFace` / `Shape.fromEdge` do not fail for one. + /// Same treatment, and same reason, as `CADFileLoader.edgePolylineOnlyBridge`. + /// + /// - Parameters: + /// - shape: the shape to enumerate. + /// - graph: a graph built from `shape`, or `nil` for shapes-only tables. + /// - faceToShape: how a `Face` becomes a `Shape`, `Shape.fromFace` in production. + /// - edgeToShape: how an `Edge` becomes a `Shape`, `Shape.fromEdge` in production. + init( + shape: Shape, + graph: BRepGraph?, + faceToShape: (Face) -> Shape?, + edgeToShape: (Edge) -> Shape? + ) { self.shape = shape self.graph = graph - let faceShapes = shape.faces().compactMap { Shape.fromFace($0) } - let edgeShapes = shape.edges().compactMap { Shape.fromEdge($0) } + // `map`, never `compactMap`. These arrays are indexed by the render-path ordinal, which + // the mesher assigns from the unfiltered enumeration, so dropping a failed conversion + // would move every later ordinal down one and the table would then name the sub-shape + // after the one the pick hit (OCCTSwiftInteraction#9). A `nil` element means "no identity + // at this ordinal", which costs one face its durable handle instead of corrupting its + // neighbours, and `SubShapePickResolver` already reads that as a miss and re-derives from + // the shape. + let faceShapes = shape.faces().map(faceToShape) + let edgeShapes = shape.edges().map(edgeToShape) + // The vertex path takes no conversion at all: `subShapes(ofType:)` returns `Shape` values, + // so its alignment holds for free and its table stays non-optional. That asymmetry is the + // point, so do not add a conversion here to match the other two, and do not take the other + // two back to `compactMap` to match this one. let vertexShapes = shape.subShapes(ofType: .vertex) self.faces = FaceIdentityTable( shapes: faceShapes, uids: Self.uids(for: faceShapes, in: graph)) @@ -119,12 +158,15 @@ public struct ShapeIdentity: Sendable { /// /// Written once and shared by all three kinds. `findNode(for:)` matches on OCCT's `IsSame`, /// which is the identity these tables are enumerated by, so a face shared between two shells - /// resolves to the one node naming it. An individual element is `nil` only when that - /// sub-shape has no node in the graph. - private static func uids(for shapes: [Shape], in graph: BRepGraph?) -> [BRepGraph.GraphUID?]? { + /// resolves to the one node naming it. An individual element is `nil` when that sub-shape has + /// no node in the graph, and also when there is no sub-shape at that ordinal at all, which + /// keeps `uids` the same length and the same index space as `shapes`. + /// + /// Takes `[Shape?]` so the vertex table's `[Shape]` promotes on the way in. + private static func uids(for shapes: [Shape?], in graph: BRepGraph?) -> [BRepGraph.GraphUID?]? { guard let graph else { return nil } return shapes.map { sub in - guard let node = graph.findNode(for: sub) else { return nil } + guard let sub, let node = graph.findNode(for: sub) else { return nil } return graph.uid(ofNodeKind: Int(node.kind.rawValue), index: node.index) } } diff --git a/Sources/OCCTSwiftTools/VertexIdentityTable.swift b/Sources/OCCTSwiftTools/VertexIdentityTable.swift index 555000f..5250713 100644 --- a/Sources/OCCTSwiftTools/VertexIdentityTable.swift +++ b/Sources/OCCTSwiftTools/VertexIdentityTable.swift @@ -25,6 +25,12 @@ public struct VertexIdentityTable: Sendable { /// Built from `Shape.subShapes(ofType: .vertex)`, the same `TopTools_IndexedMapOfShape` /// traversal `Shape.vertices()` / `Shape.vertex(at:)` use, so `shapes[ordinal]` is always the /// exact vertex behind the pick point carrying that ordinal. + /// + /// `[Shape]` rather than the `[Shape?]` its two siblings hold, and deliberately so: that + /// traversal returns `Shape` values directly, with no failable `Face`/`Edge` conversion in the + /// way, so there is no step here that could drop an element and shift every later ordinal + /// (OCCTSwiftInteraction#9). The asymmetry records that the hazard is the conversion, not the + /// enumeration. public let shapes: [Shape] /// Durable per-ordinal handle, minted from the `BRepGraph` supplied when the table was built. diff --git a/Tests/OCCTSwiftAISTests/InteractiveContextMutationTests.swift b/Tests/OCCTSwiftAISTests/InteractiveContextMutationTests.swift index 345bf44..b2893fd 100644 --- a/Tests/OCCTSwiftAISTests/InteractiveContextMutationTests.swift +++ b/Tests/OCCTSwiftAISTests/InteractiveContextMutationTests.swift @@ -130,7 +130,8 @@ struct InteractiveContextMutationTests { // Locate the shared face by identity, not by position: an ordinal is an // artifact of enumeration order, `isSame` is the decision's own primitive // and is what makes this assertion survive a reordering of the fixture. - let sharedOrdinal = try #require(identity.shapes.firstIndex { $0.isSame(as: sharedFace) }) + let sharedOrdinal = try #require( + identity.shapes.firstIndex { $0?.isSame(as: sharedFace) == true }) let sharedUID = try #require(identity.uid(forOrdinal: sharedOrdinal)) // Sanity: everything below is non-vacuous, no OTHER face carries that uid. for ordinal in identity.shapes.indices where ordinal != sharedOrdinal { diff --git a/Tests/OCCTSwiftToolsTests/EdgeIdentityTableTests.swift b/Tests/OCCTSwiftToolsTests/EdgeIdentityTableTests.swift index bd5847a..93b3acd 100644 --- a/Tests/OCCTSwiftToolsTests/EdgeIdentityTableTests.swift +++ b/Tests/OCCTSwiftToolsTests/EdgeIdentityTableTests.swift @@ -166,10 +166,11 @@ struct EdgeIdentityTableTests { } let expectedUID = graph.uid(ofNodeKind: Int(kind.rawValue), index: index) guard - let ordinal = (0.. ``` -Toolchain floor: **swift-tools-version 6.1**, Swift language mode `.v6`. Platforms: iOS 18 / macOS 15 / visionOS 1 / tvOS 18 (matches the higher of OCCTSwift / OCCTSwiftViewport). +Toolchain floor: **swift-tools-version 6.1**, Swift language mode `.v6`. Platforms: iOS 18 / macOS 15, and nothing else (the higher of OCCTSwift / OCCTSwiftViewport, and the only two platforms `OCCT.xcframework` ships a slice for). This line read "iOS 18 / macOS 15 / visionOS 1 / tvOS 18" pre-merge and was wrong then too: [OCCTSwift#978](https://github.com/SecondMouseAU/OCCTSwift/issues/978). ## Conventions inherited from OCCTSwift diff --git a/docs/reference/EdgeIdentityTable.md b/docs/reference/EdgeIdentityTable.md index e8859c8..bd30230 100644 --- a/docs/reference/EdgeIdentityTable.md +++ b/docs/reference/EdgeIdentityTable.md @@ -29,17 +29,18 @@ deduplicating the way it does today. ```swift public struct EdgeIdentityTable: Sendable { - public let shapes: [Shape] + public let shapes: [Shape?] public let uids: [BRepGraph.GraphUID?]? - public init(shapes: [Shape], uids: [BRepGraph.GraphUID?]? = nil) + public init(shapes: [Shape?], uids: [BRepGraph.GraphUID?]? = nil) public func shape(forOrdinal ordinal: Int) -> Shape? public func uid(forOrdinal ordinal: Int) -> BRepGraph.GraphUID? } ``` - `shapes` is indexed by the ordinal stored in `ViewportBody.edgeIndices`, built from `Shape.edges()`. -- `uids` is populated only when a `BRepGraph` was supplied to the entry point that produced this table. Each element is `nil` if that ordinal's edge could not be resolved in the graph. +- An element is `nil` when that edge's `Edge` to `Shape` conversion failed. See [ordinal alignment](ShapeIdentity#ordinal-alignment-is-the-invariant) for why the array is optional rather than short. +- `uids` is populated only when a `BRepGraph` was supplied to the entry point that produced this table. Each element is `nil` if that ordinal's edge could not be resolved in the graph, or has no entry in `shapes`. - `shape(forOrdinal:)` / `uid(forOrdinal:)` return `nil` for an out-of-range ordinal (or, for `uid(forOrdinal:)`, when no graph was supplied at all). Obtained from [`ShapeIdentity`](ShapeIdentity), which is the one builder for all three tables, from diff --git a/docs/reference/FaceIdentityTable.md b/docs/reference/FaceIdentityTable.md index 96a6a8d..7cb9edd 100644 --- a/docs/reference/FaceIdentityTable.md +++ b/docs/reference/FaceIdentityTable.md @@ -39,17 +39,18 @@ which mirror this table for `ViewportBody.edgeIndices` / `vertexIndices`. ```swift public struct FaceIdentityTable: Sendable { - public let shapes: [Shape] + public let shapes: [Shape?] public let uids: [BRepGraph.GraphUID?]? - public init(shapes: [Shape], uids: [BRepGraph.GraphUID?]? = nil) + public init(shapes: [Shape?], uids: [BRepGraph.GraphUID?]? = nil) public func shape(forOrdinal ordinal: Int) -> Shape? public func uid(forOrdinal ordinal: Int) -> BRepGraph.GraphUID? } ``` - `shapes` is indexed by the ordinal stored in `ViewportBody.faceIndices` / `CADBodyMetadata.faceIndices`, built from `Shape.faces()` so it always names the exact face tessellated into the triangles carrying that ordinal. -- `uids` is populated only when a `BRepGraph` was supplied to the entry point that produced this table. Each element is `nil` if that ordinal's face could not be resolved in the graph. +- An element is `nil` when that face's `Face` to `Shape` conversion failed. See [ordinal alignment](ShapeIdentity#ordinal-alignment-is-the-invariant) for why the array is optional rather than short. +- `uids` is populated only when a `BRepGraph` was supplied to the entry point that produced this table. Each element is `nil` if that ordinal's face could not be resolved in the graph, or has no entry in `shapes`. - `shape(forOrdinal:)` / `uid(forOrdinal:)` return `nil` for an out-of-range ordinal (or, for `uid(forOrdinal:)`, when no graph was supplied at all). Obtained from [`ShapeIdentity`](ShapeIdentity), which is the one builder for all three tables, from diff --git a/docs/reference/ShapeIdentity.md b/docs/reference/ShapeIdentity.md index 4e11848..a277ad2 100644 --- a/docs/reference/ShapeIdentity.md +++ b/docs/reference/ShapeIdentity.md @@ -33,6 +33,7 @@ public struct ShapeIdentity: Sendable { - [Building it](#building-it) - [What each table is enumerated from](#what-each-table-is-enumerated-from) +- [Ordinal alignment is the invariant](#ordinal-alignment-is-the-invariant) - [Failure behaviour](#failure-behaviour) - [Cost](#cost) @@ -68,17 +69,55 @@ pairing hazard rather than leaving each consumer to detect it, see Each table is built from the enumeration the matching render-path ordinal is assigned by, so `shapes[ordinal]` always names the exact sub-shape behind the primitives carrying that ordinal. -| Table | Enumeration | Ordinal source | -|---|---|---| -| `faces` | `Shape.faces()` | `ViewportBody.faceIndices` / `CADBodyMetadata.faceIndices` | -| `edges` | `Shape.edges()` | `ViewportBody.edgeIndices` | -| `vertices` | `Shape.subShapes(ofType: .vertex)` | `ViewportBody.vertexIndices` | +| Table | Enumeration | Element type | Ordinal source | +|---|---|---|---| +| `faces` | `Shape.faces()` then `Shape.fromFace` | `Shape?` | `ViewportBody.faceIndices` / `CADBodyMetadata.faceIndices` | +| `edges` | `Shape.edges()` then `Shape.fromEdge` | `Shape?` | `ViewportBody.edgeIndices` | +| `vertices` | `Shape.subShapes(ofType: .vertex)` | `Shape` | `ViewportBody.vertexIndices` | Face identity keys on OCCT's `TopoDS_Shape::IsSame`, settled in [OCCTSwiftInteraction#1](https://github.com/SecondMouseAU/OCCTSwiftInteraction/issues/1), so a face shared between two shells is **one** entry rather than two, and `graph.findNode(for:)` matches on that same semantic. See [`FaceIdentityTable`](FaceIdentityTable) for the full reasoning. +## Ordinal alignment is the invariant + +Fixed in +[OCCTSwiftInteraction#9](https://github.com/SecondMouseAU/OCCTSwiftInteraction/issues/9). + +The ordinals stored in `ViewportBody.faceIndices` index the **full** `faces()` enumeration, because +that is what the mesher walks. Two of the three tables need a conversion on the way from that +enumeration to a `Shape`, and both conversions are failable: `Shape.fromFace` and `Shape.fromEdge` +return `nil` whenever the bridge call behind them yields no handle. + +The tables used to be built with `compactMap`, which dropped a failed conversion instead of holding +its place. One failure at ordinal `k` moved every later face down one, so `shapes[ordinal]` returned +the face **after** the one the pick hit and `uid(forOrdinal:)` minted a durable identity for it. It +failed silently: no error, no assertion, and the pick resolved and highlighted confidently on a +neighbour. + +So `faces.shapes` and `edges.shapes` are `[Shape?]`, built with `map`, and a failed conversion is a +`nil` at its own ordinal: + +- The index space **is** the ordinal space, by construction rather than by every conversion + happening to succeed. `shapes.count` is the size of the enumeration, and `shapes.indices` is the + range of valid ordinals. +- The damage from a failure is bounded to the one ordinal that failed. It loses its captured shape + and its uid; no other ordinal moves. +- A pick landing on that ordinal still resolves to the right sub-shape. + [`SubShapePickResolver`](SubShapePickResolver) reads a `nil` entry as a table miss and re-derives + from the shape via `subShape(type:index:)`, which walks the same `TopTools_IndexedMapOfShape`. + What the pick loses is the durable `GraphUID`, not the identity of what was hit. + +`vertices.shapes` is `[Shape]` and stays that way: `subShapes(ofType: .vertex)` returns `Shape` +values directly, so there is no conversion in front of it that could fail. The asymmetry records +where the hazard actually is. Do not add a conversion to the vertex path to match the other two, and +do not take the other two back to `compactMap` to match the vertex path. + +Two rejected alternatives, both from the issue. **Refusing the table** when any conversion fails is +safe but throws away identity for every face because one failed. **Asserting** on a count mismatch +turns a silent wrong answer into a crash, which is better, but only in a debug build. + ## Failure behaviour The three copies this replaced agreed on every success path and differed only here, which is why @@ -90,6 +129,7 @@ these cases carry explicit test coverage. | `BRepGraph(shape:)` fails in `init(shape:)` | Same as above. `graph` is `nil`; the shape is pathological but still resolvable by ordinal. | | Shape has no faces (a wire, an edge, a lone vertex) | `faces.shapes` is empty and, with a graph, `faces.uids` is `[]`. Empty, not absent. The kinds the shape does have are unaffected. | | An individual sub-shape has no node in the graph | That one `uids` element is `nil`; the rest are unaffected. | +| A `Face`/`Edge` to `Shape` conversion fails | That one ordinal's `shapes` element and `uids` element are both `nil`; every other ordinal keeps the shape and uid it had. See [ordinal alignment](#ordinal-alignment-is-the-invariant). | | The shape produced no mesh (edge-polyline-only bridge) | All three tables are built in full, including `faces`. The face table then names faces no pick can reach, which is safe because `SubShapePickResolver.resolveFace` bounds-checks against `faceIndices` first. `CADFileLoader` used to substitute an empty face table here; #7 dropped the special case, since it was the only place any copy varied a table's content and it was asymmetric with the edge and vertex tables built in full on the same branch. | ## Cost diff --git a/docs/reference/VertexIdentityTable.md b/docs/reference/VertexIdentityTable.md index 1ac8d1f..fe9b465 100644 --- a/docs/reference/VertexIdentityTable.md +++ b/docs/reference/VertexIdentityTable.md @@ -37,6 +37,7 @@ public struct VertexIdentityTable: Sendable { ``` - `shapes` is indexed by the ordinal stored in `ViewportBody.vertexIndices`, built from `Shape.subShapes(ofType: .vertex)`. +- `[Shape]`, not the `[Shape?]` its two siblings hold, and deliberately so: that traversal returns `Shape` values directly, with no failable `Face`/`Edge` conversion in the way, so nothing here can drop an element and shift every later ordinal. See [ordinal alignment](ShapeIdentity#ordinal-alignment-is-the-invariant). - `uids` is populated only when a `BRepGraph` was supplied to the entry point that produced this table. Each element is `nil` if that ordinal's vertex could not be resolved in the graph. - `shape(forOrdinal:)` / `uid(forOrdinal:)` return `nil` for an out-of-range ordinal (or, for `uid(forOrdinal:)`, when no graph was supplied at all). diff --git a/docs/spec/OCCTSwiftAIS.md b/docs/spec/OCCTSwiftAIS.md index 59c8740..096b4a9 100644 --- a/docs/spec/OCCTSwiftAIS.md +++ b/docs/spec/OCCTSwiftAIS.md @@ -366,7 +366,7 @@ Match OCCTSwift's conventions exactly. Cribbed verbatim: - **License**: LGPL 2.1 (matching OCCT). Copy from OCCTSwift. - **swift-tools-version**: 6.1. Language mode: `.v6`. -- **Platforms**: `.iOS(.v18)`, `.macOS(.v15)`, `.visionOS(.v1)`, `.tvOS(.v18)`. (Higher of OCCTSwift / OCCTSwiftViewport floors.) +- **Platforms**: `.iOS(.v18)`, `.macOS(.v15)`, and nothing else. (Higher of OCCTSwift / OCCTSwiftViewport floors, and the only two platforms `OCCT.xcframework` ships a slice for. The `.visionOS(.v1)` / `.tvOS(.v18)` this line used to carry was never true: [OCCTSwift#978](https://github.com/SecondMouseAU/OCCTSwift/issues/978).) - **Tests**: Swift Testing (`@Suite` / `@Test` / `#expect`). Never `#expect(x != nil); #expect(x!.field)` — Swift Testing doesn't short-circuit. Always `if let x { #expect(x.field) }`. - **Test naming**: `@Test func` names must NOT shadow API method names used inside the test body. Prefix with `t_` or use descriptive English. - **OCCT race**: tests that exercise OCCT geometry need `OCCT_SERIAL=1 swift test --parallel --num-workers 1` (NCollection container-overflow race on arm64 macOS). diff --git a/docs/spec/OCCTSwiftTools.md b/docs/spec/OCCTSwiftTools.md index 3cc2751..e1e00c1 100644 --- a/docs/spec/OCCTSwiftTools.md +++ b/docs/spec/OCCTSwiftTools.md @@ -6,8 +6,8 @@ This document is a brief for the next agent (Claude or human) picking up impleme The bridge product between two intentionally-decoupled siblings: -- **[OCCTSwift](https://github.com/gsdali/OCCTSwift)** — Swift wrapper around OpenCASCADE's modeling kernel. No Metal, no rendering. Currently shipping `v0.168.0` against OCCT 8.0.0-beta1 with macOS / iOS / visionOS / tvOS slices. -- **[OCCTSwiftViewport](https://github.com/gsdali/OCCTSwiftViewport)** — Pure-Metal viewport renderer. No OCCT dependency. Renders abstract `ViewportBody` objects. +- **[OCCTSwift](https://github.com/gsdali/OCCTSwift)**: Swift wrapper around OpenCASCADE's modeling kernel. No Metal, no rendering. Currently shipping `v0.168.0` against OCCT 8.0.0-beta1 with macOS / iOS slices (`ios-arm64`, `ios-arm64-simulator`, `macos-arm64`, and no others). +- **[OCCTSwiftViewport](https://github.com/gsdali/OCCTSwiftViewport)**: Pure-Metal viewport renderer. No OCCT dependency. Renders abstract `ViewportBody` objects. `OCCTSwiftTools` is the only library that depends on **both**. It converts `OCCTSwift.Shape` (B-Rep topology + meshable surfaces) into `OCCTSwiftViewport.ViewportBody` (vertex / index / face-id buffers consumable by the Metal renderer), plus CAD file I/O wrappers that need both kernels working together. @@ -164,7 +164,7 @@ Match OCCTSwift's conventions exactly. Cribbed verbatim from that repo's CLAUDE. - **License**: LGPL 2.1 (same as OCCT itself, with the OCCT_LGPL_EXCEPTION). Copy from OCCTSwift. - **swift-tools-version**: 6.1. Language mode: `.v6`. -- **Platforms**: `.iOS(.v15)`, `.macOS(.v12)`, `.visionOS(.v1)`, `.tvOS(.v15)`. (OCCTSwiftViewport requires iOS 18 / macOS 15 — when it does, OCCTSwiftTools' platform floor is the higher of the two: `.iOS(.v18)`, `.macOS(.v15)`. Use the higher pair.) +- **Platforms**: `.iOS(.v18)`, `.macOS(.v15)`, and nothing else. (The higher of OCCTSwift's and OCCTSwiftViewport's floors, and the only two platforms `OCCT.xcframework` ships a slice for. This line used to add `.visionOS(.v1)` / `.tvOS(.v15)`, which was never true: [OCCTSwift#978](https://github.com/SecondMouseAU/OCCTSwift/issues/978).) - **Tests**: Swift Testing (`@Suite` / `@Test` / `#expect`). Never `#expect(x != nil); #expect(x!.isValid)` — Swift Testing does not short-circuit. Always `if let x { #expect(x.isValid) }`. - **Test naming**: `@Test func` names must NOT shadow API method names used inside the test body (test runner gets confused). Prefix with `t_` or use descriptive English. - **OCCT race**: when running tests, set `OCCT_SERIAL=1 swift test --parallel --num-workers 1`. There's a known NCollection container-overflow race in OCCT on arm64 macOS that segfaults parallel runs. diff --git a/okf/components/OCCTSwiftTools.md b/okf/components/OCCTSwiftTools.md index 30f0f88..bb80db6 100644 --- a/okf/components/OCCTSwiftTools.md +++ b/okf/components/OCCTSwiftTools.md @@ -20,7 +20,9 @@ and SPEC.md): - **`FaceIdentityTable`** / **`EdgeIdentityTable`** / **`VertexIdentityTable`**: resolve a render-path face/edge/vertex ordinal (as stored in `ViewportBody.faceIndices` / `edgeIndices` / `vertexIndices`) back to its `Shape` and, when a `BRepGraph` is supplied, its - durable `GraphUID`. + durable `GraphUID`. Each table's index space is the ordinal space, which is why the two built + through a failable conversion hold `[Shape?]` and the vertex table holds `[Shape]` + (OCCTSwiftInteraction#9). - **`ShapeIdentity`**: the one place a `Shape` becomes those three tables (OCCTSwiftInteraction#7), holding the shape, its `BRepGraph` and all three. `init(shape:graph:)` takes a graph the caller holds (`nil` gives shapes without uids); `init(shape:)` mints one. A file load returns one per diff --git a/okf/index.md b/okf/index.md index df7d726..669af29 100644 --- a/okf/index.md +++ b/okf/index.md @@ -55,9 +55,13 @@ See [`references/`](references/index.md). - Per-target specs: [OCCTSwiftTools](../docs/spec/OCCTSwiftTools.md), [OCCTSwiftAIS](../docs/spec/OCCTSwiftAIS.md). OCCTSwiftCADKit has no spec. - Migration from the three old packages: [docs/MIGRATION.md](../docs/MIGRATION.md). -- Platform floor is the higher of OCCTSwift's and OCCTSwiftViewport's (macOS 15 / iOS 18). The - `visionOS`/`tvOS` claim is inherited from the Tools and AIS manifests and is **unverified for the - CADKit target**; confirm or narrow before 1.0.0. +- Platforms are **macOS 15+ and iOS 18+, and only those two**: the floor is the higher of + OCCTSwift's and OCCTSwiftViewport's, and the set is what the kernel actually ships. + `OCCT.xcframework` carries exactly three slices (`ios-arm64`, `ios-arm64-simulator`, + `macos-arm64`), so nothing links on visionOS or tvOS. The manifest inherited a `visionOS`/`tvOS` + claim from the pre-merge Tools and AIS manifests, where it was never true either; dropped before + 1.0.0, root cause filed as + [OCCTSwift#978](https://github.com/SecondMouseAU/OCCTSwift/issues/978). - Version line starts at `0.x`, reaching `1.0.0` once the picking consolidation (ecosystem#43) has landed. The three old version lines do not continue here. - LGPL-2.1, matching OCCT.