From df45d262bb5fd819a845834e2f8ba10c021b9aed Mon Sep 17 00:00:00 2001 From: gsdali <51393997+gsdali@users.noreply.github.com> Date: Thu, 20 Aug 2026 06:24:52 +1000 Subject: [PATCH] refactor: one identity-table builder, and a file load that returns identity Closes #7. Phase 2 made SubShapePickResolver the one place a render-path ordinal becomes a SubShapeRef. It left the step before that, building the tables the resolver reads, duplicated three ways: CADFileLoader's private helpers, CADViewportService's statics (whose own comment said it mirrored them), and OCCTSwiftUX's ShapeIdentity. Both shapes the issue offered, because they are complementary rather than alternatives: - OCCTSwiftTools.ShapeIdentity holds the shape, its BRepGraph and all three tables, and is the only place a Shape becomes them. init(shape:graph:) keeps the existing "graph nil means no durable uids" mode; init(shape:) mints its own, which CADKit and UX each hand-rolled. The uid loop is written once, generic over [Shape], which was OCCTSwiftUX's shape rather than the older two's three copies of it. - CADLoadResult.identity, keyed by ViewportBody.id, populated inside the load behind includeIdentity: Bool = false. This is the half a public builder cannot fix: a consumer handed (bodies, shapes) still pairs them positionally, and the STL/IGES robust reload appends a shape even when that input produced no body, so every later pairing shifts. Keying by body id in the branch that creates each body removes the pairing rather than guarding it. Neither alone would do. CADViewportService.load(_:id:transform:) and loadShape(_:id:) take an in-memory Shape and never produce a CADLoadResult, so shape 2 alone leaves them needing a builder. Identity is off by default on the measured cost: BRepGraph.init serialises the whole shape to a BREP string, 5.0ms against a 14-face solid whose mesh takes 9.6ms. OCCTDesignLoop calls load and loadFromManifest from seven non-picking sites. CADKit's copy is deleted, along with the count-mismatch machinery around it, which was implemented three times for one hazard: loadFile(from:id:) pre-detected the mismatch and addIdentity re-detected it, while rebuildIdentity's wholesale wipe ran against dictionaries resetAllModelState() had emptied a line earlier. rebuildIdentity and addIdentity are replaced by one internal installIdentity(_:). One behaviour change, on a path no test could reach: the bridge's edge-polyline-only branch used to substitute an empty FaceIdentityTable and now builds the ordinary one. It was the only place any copy varied a table's content, it was asymmetric with the edge and vertex tables built in full on that same branch, and it is inert through picking because resolveFace bounds-checks against faceIndices, which is empty there. The branch is now driven from tests through an internal edgePolylineOnlyBridge seam, the same treatment bodyEntries already had. Also: shapeToBodyAndMetadata no longer builds three identity tables and discards them, which is three fewer shape-map walks per body on the path every load takes. 357 tests in 32 suites, up from 343 in 30. The pairing test asserts by geometry rather than by index and was mutation-checked. --- CLAUDE.md | 36 ++- .../OCCTSwiftCADKit/CADViewportService.swift | 275 +++++------------- Sources/OCCTSwiftTools/CADFileLoader.swift | 257 +++++++++------- Sources/OCCTSwiftTools/ShapeIdentity.swift | 131 +++++++++ Tests/OCCTSwiftCADKitTests/SmokeTests.swift | 111 ++++--- .../CADFileLoaderIdentityTests.swift | 156 ++++++++++ .../ShapeIdentityTests.swift | 274 +++++++++++++++++ docs/CHANGELOG-OCCTSwiftCADKit.md | 33 +++ docs/CHANGELOG-OCCTSwiftTools.md | 27 +- docs/index-OCCTSwiftTools.md | 2 +- docs/reference/CADFileLoader.md | 66 ++++- docs/reference/EdgeIdentityTable.md | 4 +- docs/reference/FaceIdentityTable.md | 4 +- docs/reference/README.md | 1 + docs/reference/ShapeIdentity.md | 109 +++++++ docs/reference/VertexIdentityTable.md | 4 +- docs/spec/OCCTSwiftTools.md | 22 +- okf/components/OCCTSwiftCADKit.md | 5 +- okf/components/OCCTSwiftTools.md | 14 +- 19 files changed, 1141 insertions(+), 390 deletions(-) create mode 100644 Sources/OCCTSwiftTools/ShapeIdentity.swift create mode 100644 Tests/OCCTSwiftToolsTests/CADFileLoaderIdentityTests.swift create mode 100644 Tests/OCCTSwiftToolsTests/ShapeIdentityTests.swift create mode 100644 docs/reference/ShapeIdentity.md diff --git a/CLAUDE.md b/CLAUDE.md index 340e738..5b0e45c 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: 343 tests across 30 suites, all passing.** +**Expected baseline: 357 tests across 32 suites, all passing.** ## Face identity is `IsSame`, and that decision is settled @@ -113,6 +113,40 @@ Three behaviours look like the resolver's job and are not, so they stayed where Pulling any of them down would drag presentation and viewport state into the bridge layer, which is the thing this consolidation exists to prevent. +## One identity-table builder, and identity comes back from the load + +[OCCTSwiftInteraction#7](https://github.com/SecondMouseAU/OCCTSwiftInteraction/issues/7), the layer +below phase 2. + +`OCCTSwiftTools.ShapeIdentity` is the only place a `Shape` becomes `FaceIdentityTable` / +`EdgeIdentityTable` / `VertexIdentityTable`. There were three copies before: this package's private +helpers, `CADViewportService`'s statics, and OCCTSwiftUX's. Do not write a fourth. + +- **`ShapeIdentity(shape:graph:)`** uses a graph you already hold. `graph: nil` still means + "tables without durable uids", which is the mode `shapeToBodyMetadataAndIdentities(graph:)` has + always offered. +- **`ShapeIdentity(shape:)`** mints its own graph. This is the convenience CADKit and UX each + hand-rolled. + +**After a file load, ask the loader, do not rebuild.** `CADFileLoader.load(from:format: +includeIdentity: true)` fills `CADLoadResult.identity`, keyed by `ViewportBody.id`. + +**`CADLoadResult.shapes` must never be paired positionally with `.bodies`.** The STL/IGES robust +reload appends a shape even when that input produced no body, so every later pairing shifts and a +body gets another body's geometry. Consumers used to detect the resulting count mismatch and drop +identity wholesale; the loader now keys identity by body id inside the branch that creates each +body, so there is nothing to pair and nothing to guard. `CADFileLoaderIdentityTests` holds this +down by geometry, not by index, and it was mutation-checked. + +**Identity is off by default and should stay that way.** `BRepGraph.init` serialises the whole +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. + +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. + ## One selection, held by `InteractiveContext` Phase 3 of ecosystem#43, done in diff --git a/Sources/OCCTSwiftCADKit/CADViewportService.swift b/Sources/OCCTSwiftCADKit/CADViewportService.swift index a08f77c..ae40f5d 100644 --- a/Sources/OCCTSwiftCADKit/CADViewportService.swift +++ b/Sources/OCCTSwiftCADKit/CADViewportService.swift @@ -391,7 +391,8 @@ public final class CADViewportService { default: throw CADViewportError.unsupportedFormat(ext) } - let result = try await CADFileLoader.load(from: url, format: format, progress: progress) + let result = try await CADFileLoader.load( + from: url, format: format, progress: progress, includeIdentity: true) guard let firstShape = result.shapes.first else { throw CADViewportError.emptyFile } @@ -401,7 +402,7 @@ public final class CADViewportService { self.legacyLoadedShapeEntityID = result.bodies.first?.id self.modelBodies = result.bodies self.metadata = result.metadata - rebuildIdentity(bodies: result.bodies, shapes: result.shapes) + installIdentity(result.identity) for body in result.bodies { entities[body.id] = Entity(bodyIDs: [body.id]) } @@ -427,33 +428,19 @@ public final class CADViewportService { resetAllModelState() self.legacyLoadedShape = shape self.legacyLoadedShapeEntityID = id - let graph = BRepGraph(shape: shape) - let (body, meta, faceTable, edgeTable, vertexTable) = - CADFileLoader.shapeToBodyMetadataAndIdentities( - shape, - id: id, - color: SIMD4(0.7, 0.7, 0.75, 1.0), - graph: graph - ) + let identity = ShapeIdentity(shape: shape) + let (body, meta) = CADFileLoader.shapeToBodyAndMetadata( + shape, + id: id, + color: SIMD4(0.7, 0.7, 0.75, 1.0) + ) if let body { self.modelBodies = [body] } if let meta { self.metadata[id] = meta } - self.bodyShapes[id] = shape - if let graph { - self.bodyGraphs[id] = graph - } - if let faceTable { - self.faceIdentity[id] = faceTable - } - if let edgeTable { - self.edgeIdentity[id] = edgeTable - } - if let vertexTable { - self.vertexIdentity[id] = vertexTable - } + installIdentity([id: identity]) entities[id] = Entity(bodyIDs: [id]) clearSelection() updateCapSurfaces() // picks up whatever clipping/capping is already active, also rebuilds @@ -493,117 +480,39 @@ public final class CADViewportService { } } - /// Builds a `BRepGraph` and `FaceIdentityTable` per body from a multi-body file load's - /// raw shapes, keyed by body id. - /// - /// `CADFileLoader.load(from:format:)` has no identity-table overload: it owns the - /// STL/IGES robust-reload fallback, which this package shouldn't reimplement just to - /// get identity, so the table is built directly from `shape.faces()` instead of - /// re-tessellating each body through `shapeToBodyMetadataAndIdentity`. Since OCCTSwift - /// 2.0.0 (#541/#613), `shape.faces()` is a 0-based, *deduplicated* enumeration - /// (`TopExp::MapShapes`-backed), and `Mesh.Triangle.faceIndex` (the value the mesher - /// writes into `CADBodyMetadata.faceIndices`) was converted onto that same deduplicated - /// enumeration in the same release. The two still name the identical ordinal for the - /// identical shape (see `FaceIdentityTable`'s own documentation), so ordinals still line - /// up without a second tessellation pass; only the underlying semantics moved, together, - /// from per-occurrence to per-distinct-face. - /// - /// Requires `bodies` and `shapes` to correspond positionally (`shapes[i]` is the shape - /// `bodies[i]` was tessellated from): true for `CADFileLoader.load(from:format:)`'s - /// primary bridge, where both arrays are appended together only on tessellation success. - /// Its STL/IGES robust-reload fallback (`reloadRobustAndBridge`) can violate this: it - /// appends to `shapes` on every input even when that input's body tessellation fails, so - /// a body-tessellation failure part-way through a multibody robust reload shifts `shapes` - /// out of alignment with `bodies` for every subsequent entry. Detectable from the outside - /// only via the count mismatch it produces (`shapes.count > bodies.count`): when that - /// happens, this method skips building identity entirely rather than risk pairing a body - /// with the wrong shape, matching this pack's own rule that `uid`/`shape` should be - /// absent rather than wrong. - func rebuildIdentity(bodies: [_ViewportBody], shapes: [OCCTSwift.Shape]) { - guard bodies.count == shapes.count else { - self.bodyShapes = [:] - self.bodyGraphs = [:] - self.faceIdentity = [:] - self.edgeIdentity = [:] - self.vertexIdentity = [:] - return - } - - var newShapes: [String: OCCTSwift.Shape] = [:] - var newGraphs: [String: BRepGraph] = [:] - var newFaceIdentity: [String: FaceIdentityTable] = [:] - var newEdgeIdentity: [String: EdgeIdentityTable] = [:] - var newVertexIdentity: [String: VertexIdentityTable] = [:] - - for (index, body) in bodies.enumerated() { - let shape = shapes[index] - newShapes[body.id] = shape - let graph = BRepGraph(shape: shape) - if let graph { - newGraphs[body.id] = graph - } - newFaceIdentity[body.id] = Self.makeFaceIdentityTable(shape: shape, graph: graph) - newEdgeIdentity[body.id] = Self.makeEdgeIdentityTable(shape: shape, graph: graph) - newVertexIdentity[body.id] = Self.makeVertexIdentityTable(shape: shape, graph: graph) - } - - self.bodyShapes = newShapes - self.bodyGraphs = newGraphs - self.faceIdentity = newFaceIdentity - self.edgeIdentity = newEdgeIdentity - self.vertexIdentity = newVertexIdentity - } - - /// Mirrors the private `makeFaceIdentityTable` in `OCCTSwiftTools.CADFileLoader`: map - /// every `shape.faces()` ordinal to its `Shape`, and, when a graph is available, to the - /// `GraphUID` minted via `graph.findNode(for:)` on that same face `Shape` so `IsSame` - /// semantics hold. - private static func makeFaceIdentityTable(shape: OCCTSwift.Shape, graph: BRepGraph?) - -> FaceIdentityTable - { - let faceShapes = shape.faces().compactMap { OCCTSwift.Shape.fromFace($0) } - guard let graph else { - return FaceIdentityTable(shapes: faceShapes) - } - let uids: [BRepGraph.GraphUID?] = faceShapes.map { faceShape in - guard let node = graph.findNode(for: faceShape) else { return nil } - return graph.uid(ofNodeKind: Int(node.kind.rawValue), index: node.index) - } - return FaceIdentityTable(shapes: faceShapes, uids: uids) - } - - /// Mirrors the private `makeEdgeIdentityTable` in `OCCTSwiftTools.CADFileLoader`: map - /// every `shape.edges()` ordinal (the same `TopTools_IndexedMapOfShape` traversal - /// `edgeIndices` is built from) to its `Shape` and, when available, `GraphUID`. - private static func makeEdgeIdentityTable(shape: OCCTSwift.Shape, graph: BRepGraph?) - -> EdgeIdentityTable - { - let edgeShapes = shape.edges().compactMap { OCCTSwift.Shape.fromEdge($0) } - guard let graph else { - return EdgeIdentityTable(shapes: edgeShapes) - } - let uids: [BRepGraph.GraphUID?] = edgeShapes.map { edgeShape in - guard let node = graph.findNode(for: edgeShape) else { return nil } - return graph.uid(ofNodeKind: Int(node.kind.rawValue), index: node.index) - } - return EdgeIdentityTable(shapes: edgeShapes, uids: uids) - } - - /// Mirrors the private `makeVertexIdentityTable` in `OCCTSwiftTools.CADFileLoader`: map - /// every `shape.subShapes(ofType: .vertex)` ordinal (the same `TopTools_IndexedMapOfShape` - /// traversal `vertexIndices` is built from) to its `Shape` and, when available, `GraphUID`. - private static func makeVertexIdentityTable(shape: OCCTSwift.Shape, graph: BRepGraph?) - -> VertexIdentityTable - { - let vertexShapes = shape.subShapes(ofType: .vertex) - guard let graph else { - return VertexIdentityTable(shapes: vertexShapes) - } - let uids: [BRepGraph.GraphUID?] = vertexShapes.map { vertexShape in - guard let node = graph.findNode(for: vertexShape) else { return nil } - return graph.uid(ofNodeKind: Int(node.kind.rawValue), index: node.index) + /// Installs the durable identity the loader (or `ShapeIdentity(shape:)`) already built, + /// keyed by body id. + /// + /// This service used to build the tables itself, from a hand-written copy of the private + /// helpers in `OCCTSwiftTools.CADFileLoader` whose own comment said so. Construction is + /// `ShapeIdentity`'s since OCCTSwiftInteraction#7, and a file load gets it back from + /// `CADLoadResult.identity`. + /// + /// **There is no count-mismatch guard here any more, and that is the point.** The old + /// `rebuildIdentity(bodies:shapes:)` paired `shapes[i]` with `bodies[i]` positionally, which + /// `CADFileLoader`'s STL/IGES robust reload can break: it appends a shape even when that + /// input produced no body, so every later pairing shifts and a body gets another body's + /// geometry. From out here the only visible symptom was the count mismatch, so the guard + /// dropped identity for every body, including correctly paired ones, rather than risk one + /// wrong pairing. `CADLoadResult.identity` is keyed by body id inside the loader, in the same + /// branch that creates each body, so no positional pairing happens anywhere and there is + /// nothing left to detect. + /// + /// Additive: body ids not present in `identity` keep whatever they had, which is what the + /// multi-entity `loadFile(from:id:)` needs. The deprecated single-entity loaders clear + /// everything through `resetAllModelState()` first, so they get replace-all semantics without + /// a second code path. + /// + /// Internal rather than private so tests can seed a synthetic multi-body scenario directly: + /// this package's tests ship no multi-body file on disk. + func installIdentity(_ identity: [String: ShapeIdentity]) { + for (bodyID, entry) in identity { + bodyShapes[bodyID] = entry.shape + if let graph = entry.graph { bodyGraphs[bodyID] = graph } + faceIdentity[bodyID] = entry.faces + edgeIdentity[bodyID] = entry.edges + vertexIdentity[bodyID] = entry.vertices } - return VertexIdentityTable(shapes: vertexShapes, uids: uids) } /// Convenience for callers that have file `Data` rather than a URL @@ -683,27 +592,32 @@ public final class CADViewportService { default: throw CADViewportError.unsupportedFormat(ext) } - let result = try await CADFileLoader.load(from: url, format: format, progress: progress) + let result = try await CADFileLoader.load( + from: url, format: format, progress: progress, includeIdentity: true) guard !result.bodies.isEmpty else { throw CADViewportError.emptyFile } remove(id: id) + // The loader keys identity by ITS body ids; this entity renames every body to + // "-", so identity is re-keyed alongside the rename rather than rebuilt. var bodyIDs: [String] = [] + var identity: [String: ShapeIdentity] = [:] for (index, originalBody) in result.bodies.enumerated() { let bodyID = "\(id)-\(index)" if let originalMeta = result.metadata[originalBody.id] { metadata[bodyID] = originalMeta } + if let originalIdentity = result.identity[originalBody.id] { + identity[bodyID] = originalIdentity + } var body = originalBody body.id = bodyID modelBodies.append(body) bodyIDs.append(bodyID) } - addIdentity( - bodyIDs: bodyIDs, - shapes: result.bodies.count == result.shapes.count ? result.shapes : []) + installIdentity(identity) entities[id] = Entity(bodyIDs: bodyIDs) updateCapSurfaces() // picks up whatever clipping/capping is already active, also rebuilds @@ -730,14 +644,11 @@ public final class CADViewportService { remove(id: id) - let graph = BRepGraph(shape: placedShape) - let (body, meta, faceTable, edgeTable, vertexTable) = - CADFileLoader.shapeToBodyMetadataAndIdentities( - placedShape, - id: id, - color: SIMD4(0.7, 0.7, 0.75, 1.0), - graph: graph - ) + let (body, meta) = CADFileLoader.shapeToBodyAndMetadata( + placedShape, + id: id, + color: SIMD4(0.7, 0.7, 0.75, 1.0) + ) guard let body else { entities[id] = Entity(bodyIDs: []) @@ -750,50 +661,15 @@ public final class CADViewportService { if let meta { metadata[id] = meta } - bodyShapes[id] = placedShape - if let graph { - bodyGraphs[id] = graph - } - if let faceTable { - faceIdentity[id] = faceTable - } - if let edgeTable { - edgeIdentity[id] = edgeTable - } - if let vertexTable { - vertexIdentity[id] = vertexTable - } + // Built after the body, so a shape that produced nothing renderable does not pay for a + // BRepGraph it can never be picked through. + installIdentity([id: ShapeIdentity(shape: placedShape)]) entities[id] = Entity(bodyIDs: [id]) updateCapSurfaces() // picks up whatever clipping/capping is already active, also rebuilds return id } - /// Additive counterpart to `rebuildIdentity`: builds identity for one entity's bodies - /// and merges it in with whatever other entities' identity already exists, rather than - /// wiping everything (which is what `rebuildIdentity`, used by the deprecated - /// single-entity `loadFile(from:progress:)`, does). - /// - /// Same defensive count-mismatch guard as `rebuildIdentity`: if `bodyIDs` and `shapes` - /// don't correspond positionally (or an empty `shapes` was passed because the caller - /// already detected a mismatch), identity is skipped for this batch; the bodies still - /// display, without durable-identity picks, rather than risk pairing a body with the - /// wrong shape. - private func addIdentity(bodyIDs: [String], shapes: [OCCTSwift.Shape]) { - guard bodyIDs.count == shapes.count else { return } - for (index, bodyID) in bodyIDs.enumerated() { - let shape = shapes[index] - bodyShapes[bodyID] = shape - let graph = BRepGraph(shape: shape) - if let graph { - bodyGraphs[bodyID] = graph - } - faceIdentity[bodyID] = Self.makeFaceIdentityTable(shape: shape, graph: graph) - edgeIdentity[bodyID] = Self.makeEdgeIdentityTable(shape: shape, graph: graph) - vertexIdentity[bodyID] = Self.makeVertexIdentityTable(shape: shape, graph: graph) - } - } - /// Removes a loaded entity (and its bodies) from the viewport. /// /// No-op if `id` isn't currently loaded. Clears the current selection if it referenced @@ -1649,7 +1525,7 @@ public final class CADViewportService { /// pristine `clippingSourceShapes` entry when `updateCapSurfaces` is restoring). /// /// Preserves the caller-configurable state of `original` (visibility, pickability, - /// material, transform) that `CADFileLoader.shapeToBodyMetadataAndIdentities` would + /// material, transform) that `CADFileLoader.shapeToBodyAndMetadata` would /// otherwise reset to its own defaults, and updates identity tables to match the new /// geometry (so picking the surviving faces, and the new cut face, resolves correctly, /// per the same identity contract `load(_:id:transform:)` maintains). Returns `false` @@ -1666,11 +1542,9 @@ public final class CADViewportService { bodyID: String, withCappedShape shape: OCCTSwift.Shape, preserving original: _ViewportBody ) -> Bool { guard let index = modelBodies.firstIndex(where: { $0.id == bodyID }) else { return false } - let graph = BRepGraph(shape: shape) - let (freshBody, meta, faceTable, edgeTable, vertexTable) = - CADFileLoader.shapeToBodyMetadataAndIdentities( - shape, id: bodyID, color: original.color, graph: graph - ) + let (freshBody, meta) = CADFileLoader.shapeToBodyAndMetadata( + shape, id: bodyID, color: original.color + ) guard var body = freshBody else { return false } body.isVisible = original.isVisible body.isPickable = original.isPickable @@ -1683,23 +1557,12 @@ public final class CADViewportService { modelBodies[index] = body if let meta { metadata[bodyID] = meta } else { metadata.removeValue(forKey: bodyID) } - bodyShapes[bodyID] = shape - if let graph { bodyGraphs[bodyID] = graph } else { bodyGraphs.removeValue(forKey: bodyID) } - if let faceTable { - faceIdentity[bodyID] = faceTable - } else { - faceIdentity.removeValue(forKey: bodyID) - } - if let edgeTable { - edgeIdentity[bodyID] = edgeTable - } else { - edgeIdentity.removeValue(forKey: bodyID) - } - if let vertexTable { - vertexIdentity[bodyID] = vertexTable - } else { - vertexIdentity.removeValue(forKey: bodyID) - } + let identity = ShapeIdentity(shape: shape) + installIdentity([bodyID: identity]) + // `installIdentity` merges, and a cap replaces this body's geometry outright: if the graph + // failed to build for the NEW shape, the OLD one must go rather than linger naming + // pre-cap topology. The tables above are unconditional, so only the graph needs this. + if identity.graph == nil { bodyGraphs.removeValue(forKey: bodyID) } scalarFields.removeValue(forKey: bodyID) dropLastScalarFieldBodyID(ifCurrently: bodyID) return true diff --git a/Sources/OCCTSwiftTools/CADFileLoader.swift b/Sources/OCCTSwiftTools/CADFileLoader.swift index ef4c2c8..4046131 100644 --- a/Sources/OCCTSwiftTools/CADFileLoader.swift +++ b/Sources/OCCTSwiftTools/CADFileLoader.swift @@ -19,15 +19,38 @@ import simd public struct CADLoadResult: @unchecked Sendable { public var bodies: [ViewportBody] public var metadata: [String: CADBodyMetadata] + + /// Every shape the load produced, in load order. + /// + /// **Do not pair this with `bodies` positionally.** It holds on the primary bridge, where both + /// arrays are appended together only on tessellation success, but not for the STL/IGES robust + /// reload (`reloadRobustAndBridge`), which appends a shape even when that input produced no + /// body and so shifts every later pairing: a body would be handed another body's geometry. + /// Use `identity` instead, which the loader keys by body id at the moment each body is + /// created, so the pairing is never inferred from the outside. See OCCTSwiftInteraction#7. public var shapes: [Shape] public var dimensions: [DimensionInfo] public var geomTolerances: [GeomToleranceInfo] public var datums: [DatumInfo] + /// The shape, `BRepGraph` and three ordinal-to-identity tables for each loaded body, keyed by + /// `ViewportBody.id`. + /// + /// Empty unless the load was asked for it (`includeIdentity: true`), because building it costs + /// a `BRepGraph` per body and a consumer that only renders should not pay for one. See + /// `ShapeIdentity` for the measured cost. + /// + /// This is the supported way to get identity out of a multi-body file load. Before + /// OCCTSwiftInteraction#7 there was none, and every consumer rebuilt the tables from + /// `shapes` and `bodies` itself, which is both duplicated work and the pairing hazard + /// documented on `shapes` above. + public var identity: [String: ShapeIdentity] + public init( bodies: [ViewportBody] = [], metadata: [String: CADBodyMetadata] = [:], shapes: [Shape] = [], dimensions: [DimensionInfo] = [], - geomTolerances: [GeomToleranceInfo] = [], datums: [DatumInfo] = [] + geomTolerances: [GeomToleranceInfo] = [], datums: [DatumInfo] = [], + identity: [String: ShapeIdentity] = [:] ) { self.bodies = bodies self.metadata = metadata @@ -35,6 +58,7 @@ public struct CADLoadResult: @unchecked Sendable { self.dimensions = dimensions self.geomTolerances = geomTolerances self.datums = datums + self.identity = identity } } @@ -49,6 +73,15 @@ public enum CADFileLoader { /// - progress: optional progress + cancellation observer. Honored by `.step` and /// `.iges` formats only; STL/OBJ/BREP loaders are single-call upstream and don't /// surface progress. + /// - includeIdentity: If `true`, populates `CADLoadResult.identity` with a `ShapeIdentity` + /// per loaded body: the shape it was tessellated from, a `BRepGraph` built for it, and the + /// three ordinal-to-identity tables `SubShapePickResolver` reads. Off by default because + /// each body costs a `BRepGraph`, which serialises the whole shape to a BREP string on the + /// way through (measured at roughly half the cost of meshing it), and a consumer that + /// loads geometry to render or reproject it never picks. Turn it on for anything that + /// does: it is the only supported way to get identity out of a multi-body file load, and + /// it is built inside the loader precisely so no consumer has to pair `shapes` with + /// `bodies` itself. See OCCTSwiftInteraction#7. /// - Returns: The loaded bodies with their selection metadata. /// - Throws: Whatever the underlying `OCCTSwiftIO.ShapeLoader.load` throws for the given /// `format` (a malformed or unreadable file), or `OCCTSwift.ImportError.cancelled` if @@ -56,22 +89,36 @@ public enum CADFileLoader { public static func load( from url: URL, format: CADFileFormat, - progress: ImportProgress? = nil + progress: ImportProgress? = nil, + includeIdentity: Bool = false ) async throws -> CADLoadResult { let ioResult = try await ShapeLoader.load(from: url, format: format, progress: progress) return bridgeWithFallback( ioResult: ioResult, idPrefix: format.rawValue, - url: url, format: format, progress: progress + url: url, format: format, progress: progress, + includeIdentity: includeIdentity ) } /// Loads bodies from a script manifest (manifest.json + BREP files). - public static func loadFromManifest(at url: URL) throws -> CADLoadResult { + /// + /// - Parameters: + /// - url: location of the manifest. + /// - includeIdentity: see `load(from:format:progress:includeIdentity:)`. + /// - Returns: The loaded bodies with their selection metadata, and, when asked, one + /// `ShapeIdentity` per body. + /// - Throws: Whatever `OCCTSwiftIO.ShapeLoader.loadFromManifest` throws for an unreadable or + /// malformed manifest, or for a BREP file it references that cannot be read. + public static func loadFromManifest( + at url: URL, + includeIdentity: Bool = false + ) throws -> CADLoadResult { let ioResult = try ShapeLoader.loadFromManifest(at: url) var bodies: [ViewportBody] = [] var metadata: [String: CADBodyMetadata] = [:] var shapes: [Shape] = [] + var identity: [String: ShapeIdentity] = [:] for (index, pair) in ioResult.shapesWithColors.enumerated() { let descriptor = ioResult.manifest?.bodies[index] @@ -83,10 +130,12 @@ public enum CADFileLoader { bodies.append(body) shapes.append(pair.shape) if let meta { metadata[bodyID] = meta } + if includeIdentity { identity[bodyID] = ShapeIdentity(shape: pair.shape) } } } - return CADLoadResult(bodies: bodies, metadata: metadata, shapes: shapes) + return CADLoadResult( + bodies: bodies, metadata: metadata, shapes: shapes, identity: identity) } // MARK: - Bridge with STL/IGES robust fallback @@ -101,11 +150,13 @@ public enum CADFileLoader { idPrefix: String, url: URL, format: CADFileFormat, - progress: ImportProgress? + progress: ImportProgress?, + includeIdentity: Bool ) -> CADLoadResult { var bodies: [ViewportBody] = [] var metadata: [String: CADBodyMetadata] = [:] var shapes: [Shape] = [] + var identity: [String: ShapeIdentity] = [:] var needsRobustReload = false for (index, pair) in ioResult.shapesWithColors.enumerated() { @@ -118,6 +169,7 @@ public enum CADFileLoader { bodies.append(body) shapes.append(pair.shape) if let meta { metadata[bodyID] = meta } + if includeIdentity { identity[bodyID] = ShapeIdentity(shape: pair.shape) } } else if format == .stl || format == .iges { needsRobustReload = true break @@ -130,19 +182,22 @@ public enum CADFileLoader { // splits it into one ViewportBody per body. if needsRobustReload { return reloadRobustAndBridge( - idPrefix: idPrefix, url: url, format: format, progress: progress) + idPrefix: idPrefix, url: url, format: format, progress: progress, + includeIdentity: includeIdentity) } return CADLoadResult( bodies: bodies, metadata: metadata, shapes: shapes, dimensions: ioResult.dimensions, geomTolerances: ioResult.geomTolerances, - datums: ioResult.datums + datums: ioResult.datums, + identity: identity ) } private static func reloadRobustAndBridge( - idPrefix: String, url: URL, format: CADFileFormat, progress: ImportProgress? + idPrefix: String, url: URL, format: CADFileFormat, progress: ImportProgress?, + includeIdentity: Bool ) -> CADLoadResult { // The robust reload mirrors the primary load's blocking call; we're // already on a detached task at this point (outer load() is async), @@ -163,6 +218,7 @@ public enum CADFileLoader { var bodies: [ViewportBody] = [] var metadata: [String: CADBodyMetadata] = [:] var shapes: [Shape] = [] + var identity: [String: ShapeIdentity] = [:] for (index, pair) in ioRobust.shapesWithColors.enumerated() { let bodyID = "\(idPrefix)-\(index)" let rgba = pair.color ?? SIMD4(0.7, 0.7, 0.7, 1.0) @@ -171,11 +227,18 @@ public enum CADFileLoader { bodies.append(body) shapes.append(pair.shape) if let meta { metadata[bodyID] = meta } + // Keyed by the body id this shape actually produced, in the same branch that + // produced it. This is the loop that creates the pairing hazard documented on + // `CADLoadResult.shapes`: the `else` below appends a shape with no body, so + // every later positional pairing shifts. Identity never pairs positionally, so + // there is nothing here for a consumer to get wrong (OCCTSwiftInteraction#7). + if includeIdentity { identity[bodyID] = ShapeIdentity(shape: pair.shape) } } else { shapes.append(pair.shape) } } - return CADLoadResult(bodies: bodies, metadata: metadata, shapes: shapes) + return CADLoadResult( + bodies: bodies, metadata: metadata, shapes: shapes, identity: identity) } catch { return CADLoadResult() } @@ -272,11 +335,11 @@ public enum CADFileLoader { includeMeasurements: Bool = false, directMesh useDirectMesh: Bool = false ) -> (ViewportBody?, CADBodyMetadata?) { - let (body, meta, _, _, _) = bridgeShapeToBody( + let (body, meta, _) = bridgeShapeToBody( shape, id: bodyID, color: rgba, stl: stl, deflection: customDeflection, gpuTessellation: gpuTessellation, edgeDeflection: edgeDeflection, maxPointsPerEdge: maxPointsPerEdge, includeMeasurements: includeMeasurements, - directMesh: useDirectMesh, graph: nil + directMesh: useDirectMesh, identity: false, graph: nil ) return (body, meta) } @@ -311,13 +374,13 @@ public enum CADFileLoader { directMesh useDirectMesh: Bool = false, graph: BRepGraph? = nil ) -> (ViewportBody?, CADBodyMetadata?, FaceIdentityTable?) { - let (body, meta, faceIdentity, _, _) = bridgeShapeToBody( + let (body, meta, identity) = bridgeShapeToBody( shape, id: bodyID, color: rgba, stl: stl, deflection: customDeflection, gpuTessellation: gpuTessellation, edgeDeflection: edgeDeflection, maxPointsPerEdge: maxPointsPerEdge, includeMeasurements: includeMeasurements, - directMesh: useDirectMesh, graph: graph + directMesh: useDirectMesh, identity: true, graph: graph ) - return (body, meta, faceIdentity) + return (body, meta, identity?.faces) } /// Overload of `shapeToBodyAndMetadata` that emits identity tables for all three pickable @@ -331,6 +394,11 @@ public enum CADFileLoader { /// Without a graph, only `shapes` is populated on each table. /// /// All other parameters match `shapeToBodyAndMetadata`. + /// + /// Table construction itself is `ShapeIdentity`'s since OCCTSwiftInteraction#7; this overload + /// is the one-pass convenience for a caller that wants a mesh and identity from the same call. + /// A caller that already has a `ViewportBody`, or that wants identity for a shape it is not + /// tessellating, builds a `ShapeIdentity` directly instead. public static func shapeToBodyMetadataAndIdentities( _ shape: Shape, id bodyID: String, @@ -347,14 +415,19 @@ public enum CADFileLoader { ViewportBody?, CADBodyMetadata?, FaceIdentityTable?, EdgeIdentityTable?, VertexIdentityTable? ) { - bridgeShapeToBody( + let (body, meta, identity) = bridgeShapeToBody( shape, id: bodyID, color: rgba, stl: stl, deflection: customDeflection, gpuTessellation: gpuTessellation, edgeDeflection: edgeDeflection, maxPointsPerEdge: maxPointsPerEdge, includeMeasurements: includeMeasurements, - directMesh: useDirectMesh, graph: graph + directMesh: useDirectMesh, identity: true, graph: graph ) + return (body, meta, identity?.faces, identity?.edges, identity?.vertices) } + // The `identity` flag is whether to build the `ShapeIdentity` at all, which is distinct from + // `graph == nil` (build the tables, but without durable uids). Skipping it entirely is what + // `shapeToBodyAndMetadata` wants: it discards the tables, and building three of them walks the + // shape's face, edge and vertex maps for nothing. private static func bridgeShapeToBody( _ shape: Shape, id bodyID: String, @@ -366,11 +439,9 @@ public enum CADFileLoader { maxPointsPerEdge: Int, includeMeasurements: Bool, directMesh useDirectMesh: Bool, + identity: Bool, graph: BRepGraph? - ) -> ( - ViewportBody?, CADBodyMetadata?, FaceIdentityTable?, EdgeIdentityTable?, - VertexIdentityTable? - ) { + ) -> (ViewportBody?, CADBodyMetadata?, ShapeIdentity?) { let measurements: ShapeMeasurements? = includeMeasurements ? shape.measure() : nil let mesh: Mesh? if let customDeflection { @@ -385,32 +456,11 @@ public enum CADFileLoader { mesh = shape.mesh(parameters: highQualityMeshParams) } guard let mesh else { - let edgePolylines = extractEdgePolylines( - from: shape, deflection: edgeDeflection, maxPointsPerEdge: maxPointsPerEdge + return edgePolylineOnlyBridge( + shape, id: bodyID, color: rgba, edgeDeflection: edgeDeflection, + maxPointsPerEdge: maxPointsPerEdge, measurements: measurements, + identity: identity, graph: graph ) - if !edgePolylines.isEmpty { - let edges = edgePolylines.map { $0.points } - let pickVerts = sourceShapeVertexPickData(from: shape) - let edgeIndices = flattenEdgeIndices(edgePolylines) - let body = ViewportBody( - id: bodyID, vertexData: [], indices: [], - edges: edges, - edgeIndices: edgeIndices, - vertices: pickVerts.positions, - vertexIndices: pickVerts.indices, - color: rgba - ) - let meta = CADBodyMetadata( - faceIndices: [], edgePolylines: edgePolylines, - vertices: pickVerts.positions, - measurements: measurements - ) - let faceIdentity = FaceIdentityTable(shapes: [], uids: graph != nil ? [] : nil) - let edgeIdentity = makeEdgeIdentityTable(from: shape, graph: graph) - let vertexIdentity = makeVertexIdentityTable(from: shape, graph: graph) - return (body, meta, faceIdentity, edgeIdentity, vertexIdentity) - } - return (nil, nil, nil, nil, nil) } let triangles = mesh.trianglesWithFaces() @@ -472,77 +522,60 @@ public enum CADFileLoader { vertices: pickVerts.positions, measurements: measurements ) - let faceIdentity = makeFaceIdentityTable(from: shape, graph: graph) - let edgeIdentity = makeEdgeIdentityTable(from: shape, graph: graph) - let vertexIdentity = makeVertexIdentityTable(from: shape, graph: graph) - return (body, meta, faceIdentity, edgeIdentity, vertexIdentity) + return (body, meta, identity ? ShapeIdentity(shape: shape, graph: graph) : nil) } - /// Builds a `FaceIdentityTable` from `shape.faces()`, the same enumeration - /// `OCCTShapeCreateMeshWithParams` uses to assign `Mesh.Triangle.faceIndex`, so - /// `shapes[ordinal]` always names the exact face tessellated into the triangles carrying - /// that ordinal. + /// The bridge's fallback branch: a shape whose `mesh(...)` returned nil, rendered as edge + /// polylines with vertex pick points and no triangles at all. /// - /// As of OCCTSwift v2.0.0 (#541/#613) both sides moved together onto the same deduplicated - /// enumeration: a face shared between two shells is meshed once per owning shell (each - /// triangulation wound outward for its own owner) but both triangulations now carry the one - /// index that names the shared face, matching the single entry `shape.faces()` now returns - /// for it. Before v2.0.0, `shape.faces()` and the mesher's own walk were the SAME raw, - /// non-deduplicating `TopExp_Explorer` traversal (one entry per shell a shared face belonged - /// to), which is what motivated capturing this correspondence directly in the first place - /// (issue #42) rather than trusting `shape.subShapes(ofType: .face)`'s independently - /// deduplicated enumeration to agree with it. No source change was needed here for the bump: - /// this function already reads `shape.faces()` dynamically rather than hardcoding either - /// enumeration's shape. - private static func makeFaceIdentityTable(from shape: Shape, graph: BRepGraph?) - -> FaceIdentityTable - { - let faceShapes = shape.faces().compactMap { Shape.fromFace($0) } - guard let graph else { - return FaceIdentityTable(shapes: faceShapes) - } - let uids: [BRepGraph.GraphUID?] = faceShapes.map { faceShape in - guard let node = graph.findNode(for: faceShape) else { return nil } - return graph.uid(ofNodeKind: Int(node.kind.rawValue), index: node.index) - } - return FaceIdentityTable(shapes: faceShapes, uids: uids) - } - - /// Builds an `EdgeIdentityTable` from `shape.edges()`, the same `TopTools_IndexedMapOfShape` - /// traversal `Shape.edge(at:)` and the bulk edge-polyline extractor behind - /// `ViewportBody.edgeIndices` use, so `shapes[ordinal]` always names the exact edge behind - /// the segments carrying that ordinal. - private static func makeEdgeIdentityTable(from shape: Shape, graph: BRepGraph?) - -> EdgeIdentityTable - { - let edgeShapes = shape.edges().compactMap { Shape.fromEdge($0) } - guard let graph else { - return EdgeIdentityTable(shapes: edgeShapes) - } - let uids: [BRepGraph.GraphUID?] = edgeShapes.map { edgeShape in - guard let node = graph.findNode(for: edgeShape) else { return nil } - return graph.uid(ofNodeKind: Int(node.kind.rawValue), index: node.index) - } - return EdgeIdentityTable(shapes: edgeShapes, uids: uids) - } + /// Internal rather than private so it can be unit-tested directly, the same reason + /// `bodyEntries` is. This branch fires only when meshing fails outright, which is the very + /// condition that triggers the STL/IGES robust reload, and it is not reachable from a + /// synthetic shape: a wire, an edge and a lone vertex all mesh to an empty `Mesh` rather than + /// to nil (measured), so they come back through the meshed branch with `faceIndices: []`. + /// + /// Identity here is the ordinary `ShapeIdentity`, built the same way as on the meshed branch. + /// It used to be special-cased, substituting an empty `FaceIdentityTable` on the grounds that + /// no face ordinal exists when nothing was tessellated. That was the one place any copy of + /// this logic varied a table's *content*, it was asymmetric with the edge and vertex tables + /// built in full on this same branch, and it was inert: `SubShapePickResolver.resolveFace` + /// bounds-checks against `faceIndices`, which is empty here, so the face table is never read + /// through a pick either way. Dropped in favour of one rule (OCCTSwiftInteraction#7). The + /// table now answers "which faces does this shape have, and what are their durable uids" even + /// for a shape the mesher could not handle, which is strictly more than it answered before. + static func edgePolylineOnlyBridge( + _ shape: Shape, + id bodyID: String, + color rgba: SIMD4, + edgeDeflection: Double, + maxPointsPerEdge: Int, + measurements: ShapeMeasurements?, + identity: Bool, + graph: BRepGraph? + ) -> (ViewportBody?, CADBodyMetadata?, ShapeIdentity?) { + let edgePolylines = extractEdgePolylines( + from: shape, deflection: edgeDeflection, maxPointsPerEdge: maxPointsPerEdge + ) + guard !edgePolylines.isEmpty else { return (nil, nil, nil) } - /// Builds a `VertexIdentityTable` from `shape.subShapes(ofType: .vertex)`, the same - /// `TopTools_IndexedMapOfShape` traversal `Shape.vertices()` / `Shape.vertex(at:)` behind - /// `ViewportBody.vertexIndices` use, so `shapes[ordinal]` always names the exact vertex - /// behind the pick point carrying that ordinal. - private static func makeVertexIdentityTable(from shape: Shape, graph: BRepGraph?) - -> VertexIdentityTable - { - let vertexShapes = shape.subShapes(ofType: .vertex) - guard let graph else { - return VertexIdentityTable(shapes: vertexShapes) - } - let uids: [BRepGraph.GraphUID?] = vertexShapes.map { vertexShape in - guard let node = graph.findNode(for: vertexShape) else { return nil } - return graph.uid(ofNodeKind: Int(node.kind.rawValue), index: node.index) - } - return VertexIdentityTable(shapes: vertexShapes, uids: uids) + let edges = edgePolylines.map { $0.points } + let pickVerts = sourceShapeVertexPickData(from: shape) + let edgeIndices = flattenEdgeIndices(edgePolylines) + let body = ViewportBody( + id: bodyID, vertexData: [], indices: [], + edges: edges, + edgeIndices: edgeIndices, + vertices: pickVerts.positions, + vertexIndices: pickVerts.indices, + color: rgba + ) + let meta = CADBodyMetadata( + faceIndices: [], edgePolylines: edgePolylines, + vertices: pickVerts.positions, + measurements: measurements + ) + return (body, meta, identity ? ShapeIdentity(shape: shape, graph: graph) : nil) } // MARK: - Edge / Vertex extraction helpers diff --git a/Sources/OCCTSwiftTools/ShapeIdentity.swift b/Sources/OCCTSwiftTools/ShapeIdentity.swift new file mode 100644 index 0000000..09debde --- /dev/null +++ b/Sources/OCCTSwiftTools/ShapeIdentity.swift @@ -0,0 +1,131 @@ +// ShapeIdentity.swift +// OCCTSwiftTools +// +// The one place a Shape becomes the three ordinal-to-identity tables a pick resolves through +// (OCCTSwiftInteraction#7). + +import OCCTSwift + +/// Everything needed to turn a render-path ordinal on one body back into topology: the `Shape` the +/// body was tessellated from, the `BRepGraph` its durable uids were minted from, and the three +/// per-kind identity tables `SubShapePickResolver` reads. +/// +/// ## Why this type exists +/// +/// Phase 2 of [ecosystem#43](https://github.com/SecondMouseAU/ecosystem/issues/43) made +/// `SubShapePickResolver` the one place a render-path ordinal becomes a `SubShapeRef`. It did not +/// consolidate the step before that, building the tables the resolver reads, and three copies of +/// that step accumulated: `CADFileLoader`'s private helpers, `OCCTSwiftCADKit`'s (whose own comment +/// said it mirrored them), and `OCCTSwiftUX`'s `ShapeIdentity`, added for the same reason and +/// saying so in its header. This is the merged version, and it lives in the lowest target that can +/// build a table at all. +/// +/// The bakeoff on OCCTSwiftInteraction#7 found the three agreed on every success path and differed +/// only on failure paths, which is the configuration one edit away from a real divergence. Two +/// shapes were taken from the copies being replaced: +/// +/// - The uid loop is written **once**, generic over `[Shape]`, which was `OCCTSwiftUX`'s shape +/// rather than the older two's (they each wrote it out three times, once per kind). +/// - The graph is **retained**, which was `OCCTSwiftCADKit`'s. `OCCTSwiftUX` built one per shape +/// and dropped it, so anything later needing a graph rebuilt it, and `BRepGraph.init` serialises +/// the whole shape to a BREP string on the way through. +/// +/// ## Enumerations, and why these ones +/// +/// 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: +/// +/// - **Faces**: `Shape.faces()`, the same deduplicated enumeration the mesher assigns +/// `Mesh.Triangle.faceIndex` from since OCCTSwift v2.0.0. +/// - **Edges**: `Shape.edges()`, the same `TopTools_IndexedMapOfShape` traversal the bulk +/// edge-polyline extractor behind `ViewportBody.edgeIndices` uses. +/// - **Vertices**: `Shape.subShapes(ofType: .vertex)`, the traversal behind +/// `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` for the full reasoning. +/// +/// ## Building the tables is not free +/// +/// Measured against a 14-face, 36-edge solid: meshing 9.6ms, `BRepGraph(shape:)` 5.0ms, of which +/// 3.8ms is the `toBREPString()` full-BREP serialisation inside `BRepGraph.init`. That is why +/// `CADFileLoader.load(from:format:)` builds identity only when asked (`includeIdentity`), rather +/// than for every consumer: headless callers that load geometry to render or reproject it never +/// pick, and should not pay for a graph. +public struct ShapeIdentity: Sendable { + + /// The shape every table below was enumerated from. + /// + /// Also what `SubShapePickResolver` falls back to when a table misses, so carrying it here + /// keeps the resolver's two inputs from drifting apart. + public let shape: Shape + + /// The graph the tables' uids were minted from, retained for its shape's lifetime. + /// + /// `nil` when no graph was supplied to `init(shape:graph:)`, or when `BRepGraph(shape:)` failed + /// in `init(shape:)` (a pathological shape). Either way every table's `uids` is `nil` and picks + /// resolve to a `Shape` and an ordinal without a durable handle. + public let graph: BRepGraph? + + /// Maps each `ViewportBody.faceIndices` ordinal to its `Shape` and `GraphUID`. + public let faces: FaceIdentityTable + + /// Maps each `ViewportBody.edgeIndices` ordinal to its `Shape` and `GraphUID`. + public let edges: EdgeIdentityTable + + /// Maps each `ViewportBody.vertexIndices` ordinal to its `Shape` and `GraphUID`. + public let vertices: VertexIdentityTable + + /// Build all three tables for `shape`, minting uids from a graph the caller already holds. + /// + /// Pass a `BRepGraph` built from this same `shape`. Passing `nil` is supported and means + /// "shapes but no durable handles": every table still resolves `shape(forOrdinal:)`, and every + /// `uid(forOrdinal:)` returns `nil`. That is the mode `CADFileLoader`'s per-shape bridge has + /// always offered through its `graph:` parameter, and it is preserved here. + /// + /// - Parameters: + /// - shape: the shape to enumerate. + /// - graph: a graph built from `shape`, or `nil` for shapes-only tables. + public init(shape: Shape, graph: BRepGraph?) { + self.shape = shape + self.graph = graph + let faceShapes = shape.faces().compactMap { Shape.fromFace($0) } + let edgeShapes = shape.edges().compactMap { Shape.fromEdge($0) } + let vertexShapes = shape.subShapes(ofType: .vertex) + self.faces = FaceIdentityTable( + shapes: faceShapes, uids: Self.uids(for: faceShapes, in: graph)) + self.edges = EdgeIdentityTable( + shapes: edgeShapes, uids: Self.uids(for: edgeShapes, in: graph)) + self.vertices = VertexIdentityTable( + shapes: vertexShapes, uids: Self.uids(for: vertexShapes, in: graph)) + } + + /// Build all three tables for `shape`, minting a `BRepGraph` for it first. + /// + /// The convenience `OCCTSwiftCADKit` and `OCCTSwiftUX` each hand-rolled: a consumer holding an + /// in-memory `Shape` wants durable identity and has no graph yet. Graph construction is + /// failable, and a failure degrades rather than throws: the tables still resolve every ordinal + /// to its `Shape`, and `uid(forOrdinal:)` is `nil` throughout, so a pick carries an ordinal and + /// a shape but nothing that survives a later modelling operation. + /// + /// See the type's own note on cost before calling this per body in a loop. + public init(shape: Shape) { + self.init(shape: shape, graph: BRepGraph(shape: shape)) + } + + /// The durable uid for each sub-shape, or `nil` throughout when there is no graph. + /// + /// 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?]? { + guard let graph else { return nil } + return shapes.map { sub in + guard let node = graph.findNode(for: sub) else { return nil } + return graph.uid(ofNodeKind: Int(node.kind.rawValue), index: node.index) + } + } +} diff --git a/Tests/OCCTSwiftCADKitTests/SmokeTests.swift b/Tests/OCCTSwiftCADKitTests/SmokeTests.swift index 904d6ab..01ad66b 100644 --- a/Tests/OCCTSwiftCADKitTests/SmokeTests.swift +++ b/Tests/OCCTSwiftCADKitTests/SmokeTests.swift @@ -205,23 +205,26 @@ struct SmokeTests { #expect(uidDistinct != uidA) } - /// Regression for #25 review: `rebuildIdentity` is the multi-body identity builder for - /// `loadFile`, and reimplements `FaceIdentityTable` construction locally (rather than - /// calling `shapeToBodyMetadataAndIdentity`, to avoid re-tessellating every body). + /// Regression for #25 review: `installIdentity` is how a multi-body `loadFile` gets durable + /// identity into this service. /// /// `sharedFaceBetweenShellsResolvesToSameUID` above only exercises the single-body path - /// of `loadShape` through the library's own identity builder; this test drives - /// `rebuildIdentity` directly against more than one body, including the shared-face - /// fixture, to prove the local reimplementation collapses the shared face to one - /// `GraphUID` the same way. `metadata` is seeded directly since `loadFile` needs a real - /// multi-body file on disk, which this package's tests don't ship. + /// of `loadShape`; this test drives more than one body at once, including the shared-face + /// fixture, to prove the shared face still collapses to one `GraphUID` per body and that + /// two bodies' identity does not collide. `metadata` is seeded directly since `loadFile` + /// needs a real multi-body file on disk, which this package's tests don't ship. + /// + /// Rewritten for OCCTSwiftInteraction#7: this service no longer builds the tables itself + /// (that was the second of three copies of `OCCTSwiftTools.CADFileLoader`'s private + /// helpers), so the test seeds `OCCTSwiftTools.ShapeIdentity` values keyed by body id, the + /// same shape `CADLoadResult.identity` hands it in production. No positional pairing. /// /// Updated for OCCTSwift v2.0.0 (issue #54): see `sharedFaceBetweenShellsResolvesToSameUID` /// above for why the shared face's two triangulations are now found by *value*, not by two /// different ordinals. @MainActor - @Test("rebuildIdentity resolves durable identity correctly across multiple bodies") - func rebuildIdentityMultiBodyResolvesDurableIdentity() { + @Test("installIdentity resolves durable identity correctly across multiple bodies") + func installIdentityMultiBodyResolvesDurableIdentity() { guard let plainBox = Shape.box(width: 4, height: 4, depth: 4) else { Issue.record("Shape.box returned nil") return @@ -271,12 +274,15 @@ struct SmokeTests { let service = CADViewportService() service.metadata = ["multi-0": plainMeta, "multi-1": compoundMeta] - service.rebuildIdentity(bodies: [plainBody, compoundBody], shapes: [plainBox, compound]) + service.installIdentity([ + plainBody.id: ShapeIdentity(shape: plainBox), + compoundBody.id: ShapeIdentity(shape: compound), + ]) guard let pickA = service.resolveFacePick(bodyID: "multi-1", triangleIndex: triForShellA), let pickB = service.resolveFacePick(bodyID: "multi-1", triangleIndex: triForShellB) else { - Issue.record("resolveFacePick failed against rebuildIdentity's output") + Issue.record("resolveFacePick failed against installIdentity's output") return } guard let uidA = pickA.uid, let uidB = pickB.uid else { @@ -297,26 +303,32 @@ struct SmokeTests { } } - /// Regression for #25 review: the STL/IGES robust-reload fallback of - /// `OCCTSwiftTools.CADFileLoader` (`reloadRobustAndBridge`) can append a shape for every - /// input even when that input's body tessellation fails, which shifts - /// `CADLoadResult.shapes` out of positional alignment with `.bodies` for everything after - /// the failure. + /// Regression for #25 review, re-aimed by OCCTSwiftInteraction#7. + /// + /// The original subject was `rebuildIdentity`'s count-mismatch guard: the STL/IGES + /// robust-reload fallback of `OCCTSwiftTools.CADFileLoader` (`reloadRobustAndBridge`) + /// appends a shape for every input even when that input's body tessellation fails, so + /// `CADLoadResult.shapes` shifts out of positional alignment with `.bodies` for everything + /// after the failure, and this service, pairing them positionally from the outside, had to + /// detect the resulting count mismatch and drop identity wholesale. /// - /// `rebuildIdentity` can't repair that alignment from the outside, so it must detect the - /// resulting count mismatch and refuse to build identity at all: absent rather than - /// silently wrong. + /// That guard is gone because the hazard is: `CADLoadResult.identity` is keyed by body id + /// inside the loader, in the same branch that creates each body, so no positional pairing + /// happens here at all. The unit test that a body never gets another body's geometry now + /// lives where the pairing does, in `CADFileLoaderIdentityTests`. + /// + /// What is still this service's own property, and what this test now holds down: a body + /// with no identity entry (whatever the reason) resolves to no pick rather than to a + /// wrong one. Absent rather than wrong, at the level where it can still happen. @MainActor - @Test( - "rebuildIdentity clears identity rather than risk misaligned pairing when bodies/shapes counts differ" - ) - func rebuildIdentityGuardsAgainstCountMismatch() { + @Test("a body with no identity entry resolves to no pick rather than a wrong one") + func bodyWithoutIdentityResolvesToNoPick() { guard let box = Shape.box(width: 4, height: 4, depth: 4) else { Issue.record("Shape.box returned nil") return } let (body, meta) = CADFileLoader.shapeToBodyAndMetadata( - box, id: "mismatch-0", color: SIMD4(1, 1, 1, 1) + box, id: "no-identity-0", color: SIMD4(1, 1, 1, 1) ) guard let body, let meta else { Issue.record("shapeToBodyAndMetadata returned nil") @@ -324,24 +336,27 @@ struct SmokeTests { } let service = CADViewportService() - service.metadata = ["mismatch-0": meta] - // shapes.count (2) > bodies.count (1): the exact mismatch reloadRobustAndBridge can - // produce after a partial tessellation failure mid-batch. - service.rebuildIdentity(bodies: [body], shapes: [box, box]) + service.modelBodies = [body] + service.metadata = ["no-identity-0": meta] + // Deliberately no installIdentity call: the body displays and its triangles are + // pickable, but nothing tells the service what shape they came from. + #expect(service.resolveFacePick(bodyID: "no-identity-0", triangleIndex: 0) == nil) - #expect(service.resolveFacePick(bodyID: "mismatch-0", triangleIndex: 0) == nil) + // And once identity IS installed, the same pick resolves, so the nil above is the + // missing entry rather than some other reason the pick could not land. + service.installIdentity(["no-identity-0": ShapeIdentity(shape: box)]) + #expect(service.resolveFacePick(bodyID: "no-identity-0", triangleIndex: 0) != nil) } - /// Regression for #27 review: like `rebuildIdentityMultiBodyResolvesDurableIdentity` + /// Regression for #27 review: like `installIdentityMultiBodyResolvesDurableIdentity` /// above, but for edges and vertices. /// - /// `makeEdgeIdentityTable`/`makeVertexIdentityTable` (the hand-rolled builders for the - /// multi-body `loadFile` path) had zero coverage; every other edge/vertex test only - /// drove the single-body path of `loadShape` through the library's own - /// `shapeToBodyMetadataAndIdentities`. + /// The hand-rolled `makeEdgeIdentityTable`/`makeVertexIdentityTable` this used to cover are + /// gone (OCCTSwiftInteraction#7); the multi-body edge and vertex path is still worth holding + /// down, since every other edge/vertex test drives the single-body `loadShape` path. @MainActor - @Test("rebuildIdentity resolves edge and vertex durable identity across multiple bodies") - func rebuildIdentityMultiBodyResolvesEdgeAndVertexIdentity() { + @Test("installIdentity resolves edge and vertex durable identity across multiple bodies") + func installIdentityMultiBodyResolvesEdgeAndVertexIdentity() { guard let smallBox = Shape.box(width: 2, height: 2, depth: 2) else { Issue.record("Shape.box returned nil") return @@ -366,17 +381,20 @@ struct SmokeTests { service.selectionModes = [.face, .edge, .vertex] service.metadata = ["multi-0": meta0, "multi-1": meta1] service.modelBodies = [body0, body1] - service.rebuildIdentity(bodies: [body0, body1], shapes: [smallBox, box]) + service.installIdentity([ + body0.id: ShapeIdentity(shape: smallBox), + body1.id: ShapeIdentity(shape: box), + ]) guard let edgePick = service.resolveEdgePick(bodyID: "multi-1", segmentIndex: 0) else { - Issue.record("resolveEdgePick failed against rebuildIdentity's output") + Issue.record("resolveEdgePick failed against installIdentity's output") return } #expect(edgePick.bodyID == "multi-1") #expect(edgePick.uid != nil) guard let vertexPick = service.resolveVertexPick(bodyID: "multi-1", pointIndex: 0) else { - Issue.record("resolveVertexPick failed against rebuildIdentity's output") + Issue.record("resolveVertexPick failed against installIdentity's output") return } #expect(vertexPick.bodyID == "multi-1") @@ -1736,7 +1754,7 @@ struct SmokeTests { /// entity. /// /// Constructs a synthetic two-body "candidate" entity (this package's tests don't ship - /// a multi-body file on disk) via the internal `entities`/`Entity`/`rebuildIdentity` + /// a multi-body file on disk) via the internal `entities`/`Entity`/`installIdentity` /// test seams. @MainActor @Test("applySideBySide accounts for every body of a multi-body entity, not just the first") @@ -1777,14 +1795,15 @@ struct SmokeTests { "candidate-0", "candidate-1", ]) - guard let referenceBody = service.modelBodies.first(where: { $0.id == "reference" }) else { + guard service.modelBodies.contains(where: { $0.id == "reference" }) else { Issue.record("expected reference body to already be loaded") return } - service.rebuildIdentity( - bodies: [referenceBody, candidateNearBody, candidateFarBody], - shapes: [referenceBox, candidateNear, candidateFar] - ) + service.installIdentity([ + "reference": ShapeIdentity(shape: referenceBox), + "candidate-0": ShapeIdentity(shape: candidateNear), + "candidate-1": ShapeIdentity(shape: candidateFar), + ]) service.setComparison( ComparisonView(referenceID: "reference", candidateID: "candidate", mode: .sideBySide)) diff --git a/Tests/OCCTSwiftToolsTests/CADFileLoaderIdentityTests.swift b/Tests/OCCTSwiftToolsTests/CADFileLoaderIdentityTests.swift new file mode 100644 index 0000000..2958b32 --- /dev/null +++ b/Tests/OCCTSwiftToolsTests/CADFileLoaderIdentityTests.swift @@ -0,0 +1,156 @@ +import Foundation +import OCCTSwift +import OCCTSwiftViewport +import Testing +import simd + +@testable import OCCTSwiftTools + +/// `CADLoadResult.identity` (OCCTSwiftInteraction#7): identity built inside the load, keyed by +/// body id. +/// +/// Before this, `CADFileLoader.load(from:format:)` returned `bodies`, `metadata` and `shapes` and +/// no tables, so every consumer that wanted identity after a file load paired `shapes[i]` with +/// `bodies[i]` itself. That pairing is not safe: the STL/IGES robust reload appends a shape even +/// when that input produced no body, shifting every later pairing so a body can be handed another +/// body's geometry. Two consumers found the hazard independently and guarded it two different +/// ways; a third did not guard it at all until told. Keying by body id inside the loader removes +/// the pairing rather than guarding it. +@Suite("CADFileLoader identity") +struct CADFileLoaderIdentityTests { + + /// Two solids of visibly different size, written to a BREP file. + /// + /// Lets a real multi-body load be driven end to end. The sizes differ so a shifted pairing + /// changes the bounding box, which is what the pairing assertions key on. + private static func writeTwoBodyBREP() throws -> (url: URL, dir: URL)? { + guard let small = Shape.box(width: 4, height: 4, depth: 4), + let largeUnplaced = Shape.box(width: 12, height: 12, depth: 12), + let large = largeUnplaced.translated(by: SIMD3(40, 0, 0)), + let compound = Shape.compound([small, large]) + else { return nil } + + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent("int7-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + let url = dir.appendingPathComponent("two-bodies.brep") + try compound.writeBREP(to: url) + return (url, dir) + } + + /// Off by default, because building it costs a `BRepGraph` per body and the headless + /// consumers of this API (reprojection, batch render, parts extraction) never pick. + @Test func t_identityIsEmptyUnlessAsked() async throws { + guard let fixture = try Self.writeTwoBodyBREP() else { + Issue.record("failed to build the two-body BREP fixture") + return + } + defer { try? FileManager.default.removeItem(at: fixture.dir) } + + let result = try await CADFileLoader.load(from: fixture.url, format: .brep) + #expect(result.bodies.count == 2, "fixture really is multi-body") + #expect(result.identity.isEmpty, "identity is opt-in") + } + + /// The whole point: one entry per body, keyed by that body's own id, with the shape it was + /// actually tessellated from. + /// + /// Pairing is asserted by geometry rather than by position: each body's mesh bounding box has + /// to match its identity shape's bounds. A shifted pairing would hand the 4mm body the 12mm + /// shape (and vice versa), which this catches; comparing indices would not, because a shift + /// keeps the indices perfectly plausible. + @Test func t_identityIsKeyedByBodyIDAndPairedWithTheRightGeometry() async throws { + guard let fixture = try Self.writeTwoBodyBREP() else { + Issue.record("failed to build the two-body BREP fixture") + return + } + defer { try? FileManager.default.removeItem(at: fixture.dir) } + + let result = try await CADFileLoader.load( + from: fixture.url, format: .brep, includeIdentity: true) + + #expect(result.bodies.count == 2) + #expect( + Set(result.identity.keys) == Set(result.bodies.map(\.id)), + "exactly one identity entry per body, keyed by body id") + + for body in result.bodies { + guard let identity = result.identity[body.id] else { + Issue.record("no identity for body \(body.id)") + continue + } + guard let bodyBox = body.boundingBox, let shapeBox = identity.shape.bounds else { + Issue.record("missing bounds for body \(body.id)") + continue + } + // The mesh is a linear approximation of the solid, so allow a tolerance far smaller + // than the 36mm gap between the two fixture bodies. + let dx = abs(Double(bodyBox.min.x) - shapeBox.min.x) + let dy = abs(Double(bodyBox.min.y) - shapeBox.min.y) + let dz = abs(Double(bodyBox.min.z) - shapeBox.min.z) + #expect( + dx < 0.5 && dy < 0.5 && dz < 0.5, + "body \(body.id) paired with a shape whose bounds do not match its mesh") + #expect(identity.graph != nil, "a closed solid should build a graph") + #expect(identity.faces.shapes.count == 6) + } + } + + /// A pick resolves through the identity the load returned, with no consumer-side construction. + /// + /// This is the capability the issue says was missing: `load` returned no tables, so a real + /// multi-body file load had no supported way to give a face pick a durable `GraphUID`. + @Test func t_aPickResolvesThroughTheLoadedIdentityWithNoRebuild() async throws { + guard let fixture = try Self.writeTwoBodyBREP() else { + Issue.record("failed to build the two-body BREP fixture") + return + } + defer { try? FileManager.default.removeItem(at: fixture.dir) } + + let result = try await CADFileLoader.load( + from: fixture.url, format: .brep, includeIdentity: true) + + var resolvedUIDs: Set = [] + for body in result.bodies { + guard let identity = result.identity[body.id], + let meta = result.metadata[body.id] + else { + Issue.record("no identity or metadata for body \(body.id)") + continue + } + guard + let ref = SubShapePickResolver.resolveFace( + triangleIndex: 0, faceIndices: meta.faceIndices, + identity: identity.faces, shape: identity.shape) + else { + Issue.record("triangle 0 of \(body.id) did not resolve to a face") + continue + } + guard let uid = ref.uid else { + Issue.record("face pick on \(body.id) carried no durable uid") + continue + } + resolvedUIDs.insert(uid) + } + #expect( + resolvedUIDs.count == result.bodies.count, + "each body's pick resolves to its own uid, minted from its own graph") + } + + /// `loadFromManifest` takes the same switch, so the script-manifest path is not a second + /// place a consumer has to rebuild tables from. + @Test func t_manifestLoadTakesTheSameIdentitySwitch() throws { + // Signature check rather than a full manifest fixture: this package's tests ship no + // manifest on disk, and the parameter defaulting to `false` is the part worth pinning, + // since a default of `true` would silently charge every existing caller a BRepGraph. + let missing = FileManager.default.temporaryDirectory + .appendingPathComponent("int7-absent-\(UUID().uuidString)") + .appendingPathComponent("manifest.json") + #expect(throws: (any Error).self) { + _ = try CADFileLoader.loadFromManifest(at: missing, includeIdentity: true) + } + #expect(throws: (any Error).self) { + _ = try CADFileLoader.loadFromManifest(at: missing) + } + } +} diff --git a/Tests/OCCTSwiftToolsTests/ShapeIdentityTests.swift b/Tests/OCCTSwiftToolsTests/ShapeIdentityTests.swift new file mode 100644 index 0000000..3c58851 --- /dev/null +++ b/Tests/OCCTSwiftToolsTests/ShapeIdentityTests.swift @@ -0,0 +1,274 @@ +import OCCTSwift +import OCCTSwiftViewport +import Testing +import simd + +@testable import OCCTSwiftTools + +/// Coverage for the one identity-table builder, added in OCCTSwiftInteraction#7. +/// +/// It replaced three near-copies: `CADFileLoader`'s private helpers, `OCCTSwiftCADKit`'s statics, +/// and `OCCTSwiftUX`'s `ShapeIdentity`. +/// +/// The bakeoff on #7 found the three agreed on every success path and differed only on failure +/// paths, so the failure cases carry most of the weight here: they had no shared coverage at all +/// before, which is exactly how the three drifted. +@Suite("ShapeIdentity") +struct ShapeIdentityTests { + + // MARK: - Success path + + @Test func t_boxPopulatesAllThreeTablesWithDurableUIDs() { + guard let box = Shape.box(width: 10, height: 5, depth: 3) else { + Issue.record("Shape.box returned nil") + return + } + let identity = ShapeIdentity(shape: box) + + #expect(identity.graph != nil, "a closed box builds a graph") + #expect(identity.faces.shapes.count == 6) + #expect(identity.edges.shapes.count == 12) + #expect(identity.vertices.shapes.count == 8) + + // Every ordinal resolves to a shape and to a durable uid. + for ordinal in identity.faces.shapes.indices { + #expect(identity.faces.shape(forOrdinal: ordinal) != nil) + #expect(identity.faces.uid(forOrdinal: ordinal) != nil, "face \(ordinal) has no uid") + } + for ordinal in identity.edges.shapes.indices { + #expect(identity.edges.uid(forOrdinal: ordinal) != nil, "edge \(ordinal) has no uid") + } + for ordinal in identity.vertices.shapes.indices { + #expect( + identity.vertices.uid(forOrdinal: ordinal) != nil, "vertex \(ordinal) has no uid") + } + } + + /// Each table must be built from the enumeration the render path assigns its ordinals from. + /// + /// That is the whole contract. Checked against the shape directly rather than against a + /// hardcoded count, so an upstream enumeration change surfaces here. + @Test func t_tableLengthsMatchTheShapeEnumerationsTheyIndex() { + guard let cyl = Shape.cylinder(radius: 5, height: 10) else { + Issue.record("Shape.cylinder returned nil") + return + } + let identity = ShapeIdentity(shape: cyl) + #expect(identity.faces.shapes.count == cyl.faces().count) + #expect(identity.edges.shapes.count == cyl.edges().count) + #expect(identity.vertices.shapes.count == cyl.subShapes(ofType: .vertex).count) + } + + /// `ShapeIdentity` and the bridge's own overload must produce the same tables. + /// + /// Since #7 they are the same code. If they ever diverge again, this is the tripwire. + @Test func t_matchesWhatTheBridgeOverloadReturns() { + guard let box = Shape.box(width: 8, height: 4, depth: 2) else { + Issue.record("Shape.box returned nil") + return + } + guard let graph = BRepGraph(shape: box) else { + Issue.record("BRepGraph returned nil for a box") + return + } + let direct = ShapeIdentity(shape: box, graph: graph) + let (_, _, faceTable, edgeTable, vertexTable) = + CADFileLoader.shapeToBodyMetadataAndIdentities( + box, id: "box", color: SIMD4(1, 1, 1, 1), graph: graph) + + #expect(faceTable?.shapes.count == direct.faces.shapes.count) + #expect(edgeTable?.shapes.count == direct.edges.shapes.count) + #expect(vertexTable?.shapes.count == direct.vertices.shapes.count) + for ordinal in direct.faces.shapes.indices { + #expect(faceTable?.uid(forOrdinal: ordinal) == direct.faces.uid(forOrdinal: ordinal)) + } + for ordinal in direct.edges.shapes.indices { + #expect(edgeTable?.uid(forOrdinal: ordinal) == direct.edges.uid(forOrdinal: ordinal)) + } + for ordinal in direct.vertices.shapes.indices { + #expect( + vertexTable?.uid(forOrdinal: ordinal) == direct.vertices.uid(forOrdinal: ordinal)) + } + } + + /// A face shared between two shells is one entry, not two. + /// + /// Face identity keys on `IsSame` (OCCTSwiftInteraction#1). All three copies encoded this, so + /// the survivor must too. + @Test func t_sharedFaceBetweenShellsIsOneEntryWithOneUID() { + guard let box = Shape.box(width: 10, height: 10, depth: 10) else { + Issue.record("Shape.box returned nil") + return + } + let faces = box.subShapes(ofType: .face) + guard let shellA = Shape.shellFromFaces([faces[0], faces[1], faces[2]]), + let shellB = Shape.shellFromFaces([faces[0], faces[3], faces[4], faces[5]]), + let compound = Shape.compound([shellA, shellB]) + else { + Issue.record("failed to build the shared-face compound fixture") + return + } + + let identity = ShapeIdentity(shape: compound) + // 6 distinct faces, 7 occurrences: the shared one is deduplicated by `faces()`. + #expect(identity.faces.shapes.count == compound.faces().count) + #expect(compound.orientedFaces().count > compound.faces().count, "fixture shares a face") + + let uids = identity.faces.shapes.indices.compactMap { identity.faces.uid(forOrdinal: $0) } + #expect(uids.count == identity.faces.shapes.count, "every face resolved in the graph") + #expect(Set(uids).count == uids.count, "no two ordinals share a uid") + } + + // MARK: - Failure paths + + /// The `graph: nil` mode gives tables without uids, rather than no tables. + /// + /// Every ordinal still resolves to a shape. Preserved from `CADFileLoader`'s helpers, where it + /// was the meaning of the `graph:` parameter being optional, and it had no direct coverage. + @Test func t_nilGraphGivesShapesWithoutUIDsRatherThanNoTables() { + guard let box = Shape.box(width: 4, height: 4, depth: 4) else { + Issue.record("Shape.box returned nil") + return + } + let identity = ShapeIdentity(shape: box, graph: nil) + + #expect(identity.graph == nil) + #expect(identity.faces.shapes.count == 6) + #expect(identity.edges.shapes.count == 12) + #expect(identity.vertices.shapes.count == 8) + // `uids` absent entirely, not an array of nils: a caller can tell "no graph was supplied" + // from "this ordinal did not resolve". + #expect(identity.faces.uids == nil) + #expect(identity.edges.uids == nil) + #expect(identity.vertices.uids == nil) + #expect(identity.faces.shape(forOrdinal: 0) != nil) + #expect(identity.faces.uid(forOrdinal: 0) == nil) + } + + /// A shape with no faces gets an empty face table rather than an absent one. + /// + /// With a graph its `uids` is an empty array rather than nil, so "no faces" stays + /// distinguishable from "no graph". All three copies behaved this way (measured), so this pins + /// it for the survivor. + @Test func t_shapeWithNoFacesGivesAnEmptyFaceTableNotAnAbsentOne() { + guard let box = Shape.box(width: 10, height: 10, depth: 10) else { + Issue.record("Shape.box returned nil") + return + } + guard let edgeShape = Shape.fromEdge(box.edges()[0]) else { + Issue.record("Shape.fromEdge returned nil") + return + } + + let identity = ShapeIdentity(shape: edgeShape) + #expect(edgeShape.faces().isEmpty, "fixture really has no faces") + #expect(identity.faces.shapes.isEmpty) + #expect(identity.faces.uids?.isEmpty == true, "graph present, so uids is [] not nil") + #expect(identity.faces.shape(forOrdinal: 0) == nil) + + // The kinds it does have are still populated, so an unpickable kind does not take the + // others down with it. + #expect(identity.edges.shapes.count == 1) + #expect(identity.vertices.shapes.count == 2) + #expect(identity.edges.uid(forOrdinal: 0) != nil) + } + + /// A lone vertex: only the vertex table has anything in it, and the other two are empty + /// rather than the builder failing. + @Test func t_loneVertexPopulatesOnlyTheVertexTable() { + guard let box = Shape.box(width: 10, height: 10, depth: 10) else { + Issue.record("Shape.box returned nil") + return + } + let vertexShape = box.subShapes(ofType: .vertex)[0] + let identity = ShapeIdentity(shape: vertexShape) + + #expect(identity.faces.shapes.isEmpty) + #expect(identity.edges.shapes.isEmpty) + #expect(identity.vertices.shapes.count == 1) + } + + // MARK: - The edge-polyline-only branch + + /// The one place the three copies varied a table's *content*: `CADFileLoader` substituted an + /// empty `FaceIdentityTable` on the branch taken when `mesh(...)` returns nil, while + /// `OCCTSwiftCADKit` and `OCCTSwiftUX` built the face table in full from the shape. #7 took + /// the general builder, so the face table is now populated here too. + /// + /// The branch is unreachable from a synthetic shape (a wire, an edge and a lone vertex all + /// mesh to an empty `Mesh` rather than to nil, measured), which is why it is driven through + /// the internal `edgePolylineOnlyBridge` seam directly. Same treatment, and same reason, as + /// `bodyEntries`. Using a box makes the change visible: it has 6 faces and no triangles here. + @Test func t_edgePolylineOnlyBranchBuildsTheFaceTableInFull() { + guard let box = Shape.box(width: 4, height: 4, depth: 4) else { + Issue.record("Shape.box returned nil") + return + } + let graph = BRepGraph(shape: box) + let (body, meta, identity) = CADFileLoader.edgePolylineOnlyBridge( + box, id: "edge-only", color: SIMD4(1, 1, 1, 1), + edgeDeflection: CADFileLoader.defaultEdgeDeflection, + maxPointsPerEdge: CADFileLoader.defaultMaxPointsPerEdge, + measurements: nil, identity: true, graph: graph) + + guard let body, let meta, let identity else { + Issue.record("edgePolylineOnlyBridge returned nil for a box") + return + } + // No triangles at all on this branch, which is why the face table used to be emptied. + #expect(body.indices.isEmpty) + #expect(meta.faceIndices.isEmpty) + #expect(!body.edgeIndices.isEmpty) + + // The face table is now the ordinary one. It names faces no pick can reach through this + // body, which is safe because `SubShapePickResolver.resolveFace` bounds-checks against + // `faceIndices` before ever reading it. + #expect(identity.faces.shapes.count == 6) + #expect(identity.faces.uid(forOrdinal: 0) != nil) + #expect( + SubShapePickResolver.resolveFace( + triangleIndex: 0, faceIndices: meta.faceIndices, + identity: identity.faces, shape: box) == nil, + "an empty faceIndices still means not face-pickable, table or no table") + + // Edge and vertex tables were always built in full on this branch, and still are. + #expect(identity.edges.shapes.count == 12) + #expect(identity.vertices.shapes.count == 8) + } + + /// The same branch with identity switched off builds no tables at all, so a caller that only + /// wants a body does not pay for three enumerations plus a graph walk. + @Test func t_edgePolylineOnlyBranchSkipsIdentityWhenNotAsked() { + guard let box = Shape.box(width: 4, height: 4, depth: 4) else { + Issue.record("Shape.box returned nil") + return + } + let (body, _, identity) = CADFileLoader.edgePolylineOnlyBridge( + box, id: "edge-only", color: SIMD4(1, 1, 1, 1), + edgeDeflection: CADFileLoader.defaultEdgeDeflection, + maxPointsPerEdge: CADFileLoader.defaultMaxPointsPerEdge, + measurements: nil, identity: false, graph: nil) + + #expect(body != nil) + #expect(identity == nil) + } + + /// A shape with neither a mesh nor any edge to draw produces nothing at all, rather than an + /// empty body a consumer would then have to recognise as unusable. + @Test func t_edgePolylineOnlyBranchReturnsNothingWhenThereAreNoEdges() { + guard let box = Shape.box(width: 4, height: 4, depth: 4) else { + Issue.record("Shape.box returned nil") + return + } + let vertexShape = box.subShapes(ofType: .vertex)[0] + let (body, meta, identity) = CADFileLoader.edgePolylineOnlyBridge( + vertexShape, id: "no-edges", color: SIMD4(1, 1, 1, 1), + edgeDeflection: CADFileLoader.defaultEdgeDeflection, + maxPointsPerEdge: CADFileLoader.defaultMaxPointsPerEdge, + measurements: nil, identity: true, graph: nil) + + #expect(body == nil) + #expect(meta == nil) + #expect(identity == nil) + } +} diff --git a/docs/CHANGELOG-OCCTSwiftCADKit.md b/docs/CHANGELOG-OCCTSwiftCADKit.md index d83df2e..9abfb68 100644 --- a/docs/CHANGELOG-OCCTSwiftCADKit.md +++ b/docs/CHANGELOG-OCCTSwiftCADKit.md @@ -12,6 +12,39 @@ before upgrading. Earlier history is in the pre-merge `OCCTSwiftCADKit` reposito ## Unreleased +### `CADViewportService` stops building identity tables and reads the loader's + +Closes [OCCTSwiftInteraction#7](https://github.com/SecondMouseAU/OCCTSwiftInteraction/issues/7). + +This service carried a private copy of `OCCTSwiftTools.CADFileLoader`'s three identity-table +builders, and said so: *"Mirrors the private `makeFaceIdentityTable` in +`OCCTSwiftTools.CADFileLoader`"*. It existed because `CADFileLoader.load(from:format:)` returned +no tables, so the only way to get identity after a multi-body file load was to rebuild it here. +`CADLoadResult.identity` now exists, so the copy is gone. + +#### What changes for a consumer + +Nothing in the public API. `rebuildIdentity(bodies:shapes:)` and `addIdentity(bodyIDs:shapes:)` +were both internal; they are replaced by a single internal `installIdentity(_:)` taking +`[String: OCCTSwiftTools.ShapeIdentity]`. + +One behaviour improves. Both file-loading paths used to detect a `shapes`/`bodies` count mismatch +and drop durable identity for **every** body rather than risk pairing one with the wrong shape +(the mismatch is produced by `CADFileLoader`'s STL/IGES robust reload, which appends a shape even +when that input produced no body). A file that hit that case therefore loaded with picks that +resolved to nothing at all, including for bodies that were paired correctly. The loader now keys +identity by body id in the same branch that creates each body, so there is no positional pairing +anywhere and no mismatch to detect: those bodies now pick normally. + +The guard was also implemented three times for one hazard. `loadFile(from:id:)` pre-detected the +mismatch at the call site and `addIdentity` re-detected it; `rebuildIdentity`'s wholesale wipe of +`bodyShapes` / `bodyGraphs` / all three tables ran against dictionaries `resetAllModelState()` had +emptied on the line above. + +`replaceBody` (the cap-plane re-tessellation path) keeps one thing the shared installer does not +do: it still removes a stale `BRepGraph` when the new capped shape fails to build one, since +`installIdentity` merges and would otherwise leave a graph naming pre-cap topology. + ### `CADViewportService` adopts the interactive context's selection Closes [OCCTSwiftInteraction#3](https://github.com/SecondMouseAU/OCCTSwiftInteraction/issues/3), diff --git a/docs/CHANGELOG-OCCTSwiftTools.md b/docs/CHANGELOG-OCCTSwiftTools.md index 030a76f..62ee867 100644 --- a/docs/CHANGELOG-OCCTSwiftTools.md +++ b/docs/CHANGELOG-OCCTSwiftTools.md @@ -4,7 +4,32 @@ Most recent first. Pre-1.0 was free to break; SemVer-stable from v1.0.0 per the ## Unreleased -**One canonical pick resolver, and the types that name picked topology move down here.** Closes [OCCTSwiftInteraction#2](https://github.com/SecondMouseAU/OCCTSwiftInteraction/issues/2), phase 2 of [ecosystem#43](https://github.com/SecondMouseAU/ecosystem/issues/43). +### One canonical identity-table builder, and a file load can finally return identity + +Closes [OCCTSwiftInteraction#7](https://github.com/SecondMouseAU/OCCTSwiftInteraction/issues/7). + +Phase 2 (below) made `SubShapePickResolver` the one place a render-path ordinal becomes a `SubShapeRef`. It did not consolidate the step before that, building the tables the resolver reads, and three copies of it had accumulated: this package's private helpers, `OCCTSwiftCADKit`'s statics (whose own comment said it mirrored them), and `OCCTSwiftUX`'s `ShapeIdentity`. + +New API, both additions: + +- **`ShapeIdentity`**: the `Shape`, its `BRepGraph` and all three identity tables, built once. `init(shape:graph:)` uses a graph the caller holds, with `nil` still meaning "tables without durable uids"; `init(shape:)` mints one, which is the convenience CADKit and UX each hand-rolled. The uid loop is written once, generic over `[Shape]`, rather than three times. +- **`CADLoadResult.identity: [String: ShapeIdentity]`**, keyed by `ViewportBody.id`, plus **`includeIdentity: Bool = false`** on `load(from:format:progress:)` and `loadFromManifest(at:)`. New stored property with a default and new defaulted parameters, so no source break. + +**The reason both landed rather than just the builder.** They fix different halves. A public builder removes the duplicated *construction* but leaves every consumer pairing `shapes[i]` with `bodies[i]` after a file load, which is not safe: the STL/IGES robust reload appends a shape even when that input produced no body, so every later pairing shifts and a body gets another body's geometry. `CADLoadResult.identity` removes the duplicated *pairing*, since the loader keys it by body id in the same branch that creates each body. Neither alone makes the downstream copies deletable, because `CADViewportService.load(_:id:transform:)` and `loadShape(_:id:)` take an in-memory `Shape` and never produce a `CADLoadResult` at all. + +**Why identity is off by default.** `BRepGraph.init` serialises the whole shape to a BREP string. Measured against a 14-face, 36-edge solid: meshing 9.6ms, `BRepGraph(shape:)` 5.0ms, of which 3.8ms is that serialisation. Headless consumers of `load` (reprojection, batch render, parts extraction) never pick and should not pay for it. + +**One behaviour change, on a path no test could reach.** The bridge's edge-polyline-only branch (taken when `mesh(...)` returns nil) used to substitute an empty `FaceIdentityTable`; it now builds the ordinary one. That was the only place any copy of this logic varied a table's *content*, and it was asymmetric with the edge and vertex tables built in full on the same branch. It is inert through picking either way, because `SubShapePickResolver.resolveFace` bounds-checks against `faceIndices`, which is empty there. The branch is now reachable from tests through the internal `edgePolylineOnlyBridge` seam, the same treatment `bodyEntries` already had, because it cannot be provoked from a synthetic shape: a wire, an edge and a lone vertex all mesh to an empty `Mesh` rather than to nil. + +**A small performance fix on the way through.** `shapeToBodyAndMetadata` used to build all three identity tables and discard them. It now skips construction entirely, which is three fewer shape-map walks per body, on the path every `load` body takes. + +**Tests:** 14 new across two new suites (`ShapeIdentity`, `CADFileLoader identity`), 357 total. The failure cases the three copies disagreed about (nil graph, no faces, the edge-polyline-only branch, and the shape-to-body pairing) had no shared coverage at all before. The pairing test asserts by geometry rather than by index, and was mutation-checked: deliberately shifting the pairing inside the loader fails it. + +**One thing in the issue that did not survive contact with the code.** The issue describes `CADViewportService`'s count-mismatch guard as the thing to preserve. Two of its three implementations were redundant: `loadFile(from:id:)` pre-detected the mismatch at the call site *and* `addIdentity` re-detected it, and `rebuildIdentity`'s wholesale wipe ran against dictionaries `resetAllModelState()` had emptied a line earlier. All of it is gone, because the hazard it detected cannot arise once the loader keys identity by body id. + +### One canonical pick resolver, and the types that name picked topology move down here + +Closes [OCCTSwiftInteraction#2](https://github.com/SecondMouseAU/OCCTSwiftInteraction/issues/2), phase 2 of [ecosystem#43](https://github.com/SecondMouseAU/ecosystem/issues/43). New API: diff --git a/docs/index-OCCTSwiftTools.md b/docs/index-OCCTSwiftTools.md index 040629f..1b19329 100644 --- a/docs/index-OCCTSwiftTools.md +++ b/docs/index-OCCTSwiftTools.md @@ -53,7 +53,7 @@ Task-oriented recipes, each runnable against the real API: Per-type API reference for every public symbol: -- [API Reference](reference/): `PointConverter`, `CurveConverter`, `SurfaceConverter`, `WireConverter`, `BodyUtilities`, `CADFileLoader`, `FaceIdentityTable`, `EdgeIdentityTable`, `VertexIdentityTable`, `SubShapePickResolver`, `SubShapeRef` / `SubShape` / `InteractiveObject` +- [API Reference](reference/): `PointConverter`, `CurveConverter`, `SurfaceConverter`, `WireConverter`, `BodyUtilities`, `CADFileLoader`, `FaceIdentityTable`, `EdgeIdentityTable`, `VertexIdentityTable`, `ShapeIdentity`, `SubShapePickResolver`, `SubShapeRef` / `SubShape` / `InteractiveObject` ## Project diff --git a/docs/reference/CADFileLoader.md b/docs/reference/CADFileLoader.md index 4f87f90..b683f39 100644 --- a/docs/reference/CADFileLoader.md +++ b/docs/reference/CADFileLoader.md @@ -40,21 +40,31 @@ i.e. on the robust reload. public static func load( from url: URL, format: CADFileFormat, - progress: ImportProgress? = nil + progress: ImportProgress? = nil, + includeIdentity: Bool = false ) async throws -> CADLoadResult ``` - **Parameters:** - - `url` — file URL to load. - - `format` — the `CADFileFormat` (`.step`, `.iges`, `.stl`, `.obj`, `.brep`, …). - - `progress` — optional progress + cancellation observer. Honoured by `.step` and `.iges` only — STL / OBJ / BREP are single-call upstream. If `progress.shouldCancel()` returns `true`, the import throws `OCCTSwift.ImportError.cancelled`. -- **Returns:** a `CADLoadResult` with bridged bodies, per-body metadata, raw shapes, and any PMI (dimensions / tolerances / datums). + - `url`: file URL to load. + - `format`: the `CADFileFormat` (`.step`, `.iges`, `.stl`, `.obj`, `.brep`, …). + - `progress`: optional progress + cancellation observer. Honoured by `.step` and `.iges` only (STL / OBJ / BREP are single-call upstream). If `progress.shouldCancel()` returns `true`, the import throws `OCCTSwift.ImportError.cancelled`. + - `includeIdentity`: populate [`CADLoadResult.identity`](#cadloadresult) with a [`ShapeIdentity`](ShapeIdentity) per body: the shape it was tessellated from, a `BRepGraph` for it, and the three ordinal-to-identity tables `SubShapePickResolver` reads. Off by default: each body costs a `BRepGraph`, measured at roughly half the cost of meshing that body, and a consumer that loads geometry to render or reproject it never picks. Turn it on for anything that does. This is the only supported way to get identity out of a multi-body file load. +- **Returns:** a `CADLoadResult` with bridged bodies, per-body metadata, raw shapes, any PMI (dimensions / tolerances / datums), and, when asked, per-body identity. - **Example:** ```swift let result = try await CADFileLoader.load( from: URL(fileURLWithPath: "/path/bracket.step"), format: .step ) + + // Picking as well as rendering: ask for identity, then read it by body id. + let pickable = try await CADFileLoader.load( + from: URL(fileURLWithPath: "/path/assembly.step"), + format: .step, + includeIdentity: true + ) + let uid = pickable.identity["step-0"]?.faces.uid(forOrdinal: 3) ``` --- @@ -65,11 +75,15 @@ Loads bodies from a script manifest (`manifest.json` plus its referenced BREP files), applying each body's recorded colour. Synchronous. ```swift -public static func loadFromManifest(at url: URL) throws -> CADLoadResult +public static func loadFromManifest( + at url: URL, + includeIdentity: Bool = false +) throws -> CADLoadResult ``` - **Parameters:** - - `url` — file URL of the `manifest.json`. + - `url`: file URL of the `manifest.json`. + - `includeIdentity`: as on `load(from:format:progress:includeIdentity:)`. - **Returns:** a `CADLoadResult` whose bodies use ids of the form `"script-"` and fall back to grey `(0.7, 0.7, 0.7, 1)` where no colour was recorded. - **Example:** ```swift @@ -243,6 +257,7 @@ public struct CADLoadResult: @unchecked Sendable { public var dimensions: [DimensionInfo] public var geomTolerances: [GeomToleranceInfo] public var datums: [DatumInfo] + public var identity: [String: ShapeIdentity] public init( bodies: [ViewportBody] = [], @@ -250,14 +265,41 @@ public struct CADLoadResult: @unchecked Sendable { shapes: [Shape] = [], dimensions: [DimensionInfo] = [], geomTolerances: [GeomToleranceInfo] = [], - datums: [DatumInfo] = [] + datums: [DatumInfo] = [], + identity: [String: ShapeIdentity] = [:] ) } ``` -- `bodies` — bridged, renderable bodies. -- `metadata` — per-body selection metadata, keyed by body id. -- `shapes` — the raw OCCTSwift shapes that were loaded. -- `dimensions` / `geomTolerances` / `datums` — PMI (product manufacturing information) surfaced by formats that carry it. The types come from OCCTSwiftIO. +- `bodies`: bridged, renderable bodies. +- `metadata`: per-body selection metadata, keyed by body id. +- `shapes`: the raw OCCTSwift shapes that were loaded. **Do not pair positionally with `bodies`**, see below. +- `dimensions` / `geomTolerances` / `datums`: PMI (product manufacturing information) surfaced by formats that carry it. The types come from OCCTSwiftIO. +- `identity`: a [`ShapeIdentity`](ShapeIdentity) per loaded body, keyed by `ViewportBody.id`. Empty unless the load was asked for it (`includeIdentity: true`). + +### Why `identity` is keyed by body id, and `shapes` is not safe to pair + +`shapes` and `bodies` line up positionally on the primary bridge, where both arrays are appended +together only on tessellation success. They do **not** line up after the STL/IGES robust reload +(`reloadRobustAndBridge`), which appends a shape for every input even when that input produced no +body: every later pairing shifts by one, and a body silently gets another body's geometry. + +From outside the loader the only symptom is a count mismatch (`shapes.count > bodies.count`), which +is why consumers used to guard on it and drop identity wholesale. `identity` is built inside the +loader, in the same branch that creates each body, so nothing pairs positionally and there is +nothing left to guard (OCCTSwiftInteraction#7). + +```swift +let result = try await CADFileLoader.load(from: url, format: .step, includeIdentity: true) +for body in result.bodies { + guard let identity = result.identity[body.id], + let meta = result.metadata[body.id] else { continue } + let ref = SubShapePickResolver.resolveFace( + triangleIndex: pick.triangleIndex, + faceIndices: meta.faceIndices, + identity: identity.faces, + shape: identity.shape) +} +``` --- diff --git a/docs/reference/EdgeIdentityTable.md b/docs/reference/EdgeIdentityTable.md index 9710725..e8859c8 100644 --- a/docs/reference/EdgeIdentityTable.md +++ b/docs/reference/EdgeIdentityTable.md @@ -42,7 +42,9 @@ public struct EdgeIdentityTable: Sendable { - `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. - `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 [`CADFileLoader.shapeToBodyMetadataAndIdentities`](CADFileLoader#cadfileloadershapetobodymetadataandidentities). +Obtained from [`ShapeIdentity`](ShapeIdentity), which is the one builder for all three tables, from +[`CADLoadResult.identity`](CADFileLoader#cadloadresult) after a file load, or from +[`CADFileLoader.shapeToBodyMetadataAndIdentities`](CADFileLoader#cadfileloadershapetobodymetadataandidentities) when meshing and identity are wanted from one call. ## Example diff --git a/docs/reference/FaceIdentityTable.md b/docs/reference/FaceIdentityTable.md index 15569c8..96a6a8d 100644 --- a/docs/reference/FaceIdentityTable.md +++ b/docs/reference/FaceIdentityTable.md @@ -52,7 +52,9 @@ public struct FaceIdentityTable: Sendable { - `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. - `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 [`CADFileLoader.shapeToBodyMetadataAndIdentity`](CADFileLoader#cadfileloadershapetobodymetadataandidentity) or [`shapeToBodyMetadataAndIdentities`](CADFileLoader#cadfileloadershapetobodymetadataandidentities). +Obtained from [`ShapeIdentity`](ShapeIdentity), which is the one builder for all three tables, from +[`CADLoadResult.identity`](CADFileLoader#cadloadresult) after a file load, or from +[`CADFileLoader.shapeToBodyMetadataAndIdentity`](CADFileLoader#cadfileloadershapetobodymetadataandidentity) / [`shapeToBodyMetadataAndIdentities`](CADFileLoader#cadfileloadershapetobodymetadataandidentities) when meshing and identity are wanted from one call. ## Example diff --git a/docs/reference/README.md b/docs/reference/README.md index 53eb9f2..5b1325f 100644 --- a/docs/reference/README.md +++ b/docs/reference/README.md @@ -27,4 +27,5 @@ Per-type pages: - [BodyUtilities](BodyUtilities): marker spheres and offset helpers for `ViewportBody` - [CADFileLoader](CADFileLoader): load CAD files / manifests, and the `Shape` → body bridge - [FaceIdentityTable](FaceIdentityTable) / [EdgeIdentityTable](EdgeIdentityTable) / [VertexIdentityTable](VertexIdentityTable): resolve a render-path face / edge / vertex ordinal back to its `Shape` and durable `GraphUID` +- [ShapeIdentity](ShapeIdentity): the one builder for all three of those tables, plus the `Shape` and `BRepGraph` they were built from. A file load returns one per body via `CADLoadResult.identity` - [SubShapePickResolver](SubShapePickResolver): the one canonical pick resolver, taking a GPU pick's primitive index in and giving a `SubShapeRef` out. `SubShapeRef`, `SubShape` and `InteractiveObject` live in this target too; they are documented on the [Selection](Selection) page diff --git a/docs/reference/ShapeIdentity.md b/docs/reference/ShapeIdentity.md new file mode 100644 index 0000000..4e11848 --- /dev/null +++ b/docs/reference/ShapeIdentity.md @@ -0,0 +1,109 @@ +--- +title: ShapeIdentity +parent: API Reference +--- + +# ShapeIdentity + +Everything needed to turn a render-path ordinal on one body back into topology: the `Shape` the +body was tessellated from, the `BRepGraph` its durable uids were minted from, and the three +per-kind identity tables [`SubShapePickResolver`](SubShapePickResolver) reads. + +This is the one place a `Shape` becomes identity tables, added in +[OCCTSwiftInteraction#7](https://github.com/SecondMouseAU/OCCTSwiftInteraction/issues/7). Phase 2 of +[ecosystem#43](https://github.com/SecondMouseAU/ecosystem/issues/43) made `SubShapePickResolver` the +one place a render-path ordinal becomes a `SubShapeRef`; it did not consolidate the step before +that, and three copies of table construction had accumulated (`CADFileLoader`'s private helpers, +`OCCTSwiftCADKit`'s statics, and `OCCTSwiftUX`'s own `ShapeIdentity`). This is the merged version. + +```swift +public struct ShapeIdentity: Sendable { + public let shape: Shape + public let graph: BRepGraph? + public let faces: FaceIdentityTable + public let edges: EdgeIdentityTable + public let vertices: VertexIdentityTable + + public init(shape: Shape, graph: BRepGraph?) + public init(shape: Shape) +} +``` + +## Topics + +- [Building it](#building-it) +- [What each table is enumerated from](#what-each-table-is-enumerated-from) +- [Failure behaviour](#failure-behaviour) +- [Cost](#cost) + +--- + +## Building it + +Two initialisers, differing only in who owns the graph. + +```swift +// A graph you already hold, e.g. one retained across a modelling operation. +let identity = ShapeIdentity(shape: shape, graph: myGraph) + +// No graph yet: mint one for this shape. +let identity = ShapeIdentity(shape: shape) +``` + +`init(shape:graph:)` accepts `nil` for `graph`, which means "tables but no durable handles": every +ordinal still resolves to a `Shape`, and every `uid(forOrdinal:)` returns `nil`. That is the mode +[`CADFileLoader`](CADFileLoader)'s per-shape bridge has always offered through its `graph:` +parameter. + +`init(shape:)` is the convenience `OCCTSwiftCADKit` and `OCCTSwiftUX` each hand-rolled before this +type existed. + +After a file load, do not build one per body by hand: ask the loader for them instead, with +`CADFileLoader.load(from:format:includeIdentity: true)`. That is what removes the shape-to-body +pairing hazard rather than leaving each consumer to detect it, see +[`CADLoadResult`](CADFileLoader#cadloadresult). + +## What each table is enumerated from + +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` | + +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. + +## Failure behaviour + +The three copies this replaced agreed on every success path and differed only here, which is why +these cases carry explicit test coverage. + +| Case | Behaviour | +|---|---| +| `graph: nil` passed | Every table's `uids` is `nil` (absent, not an array of nils), so a caller can tell "no graph supplied" from "this ordinal did not resolve". `shape(forOrdinal:)` still works. | +| `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. | +| 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 + +Building identity is not free, which is why `CADFileLoader.load` does not do it by default. + +Measured against a 14-face, 36-edge solid: + +| Step | Time | +|---|---| +| `shape.mesh(parameters:)` (high-quality preset) | 9.6ms | +| `BRepGraph(shape:)` | 5.0ms | +| of which `toBREPString()` inside `BRepGraph.init` | 3.8ms | + +So identity is roughly half again on top of meshing, and it scales with geometry size, because +`BRepGraph.init` serialises the whole shape to a BREP string on the way through. A headless +consumer that renders or reprojects and never picks should not pay for it. diff --git a/docs/reference/VertexIdentityTable.md b/docs/reference/VertexIdentityTable.md index e0c3e1a..1ac8d1f 100644 --- a/docs/reference/VertexIdentityTable.md +++ b/docs/reference/VertexIdentityTable.md @@ -40,7 +40,9 @@ public struct VertexIdentityTable: Sendable { - `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). -Obtained from [`CADFileLoader.shapeToBodyMetadataAndIdentities`](CADFileLoader#cadfileloadershapetobodymetadataandidentities). +Obtained from [`ShapeIdentity`](ShapeIdentity), which is the one builder for all three tables, from +[`CADLoadResult.identity`](CADFileLoader#cadloadresult) after a file load, or from +[`CADFileLoader.shapeToBodyMetadataAndIdentities`](CADFileLoader#cadfileloadershapetobodymetadataandidentities) when meshing and identity are wanted from one call. ## Example diff --git a/docs/spec/OCCTSwiftTools.md b/docs/spec/OCCTSwiftTools.md index a48d95b..3cc2751 100644 --- a/docs/spec/OCCTSwiftTools.md +++ b/docs/spec/OCCTSwiftTools.md @@ -69,10 +69,23 @@ import OCCTSwiftTools // re-exports OCCTSwiftIO transitively (@_exported) public struct CADLoadResult: @unchecked Sendable { // STAYS in Tools (has bodies) public var bodies: [ViewportBody] public var metadata: [String: CADBodyMetadata] // CADBodyMetadata is now an IO type - public var shapes: [Shape] + public var shapes: [Shape] // do NOT pair positionally with bodies public var dimensions: [DimensionInfo] public var geomTolerances: [GeomToleranceInfo] public var datums: [DatumInfo] + public var identity: [String: ShapeIdentity] // keyed by body id, opt-in (#7) +} + +/// The one builder for the three ordinal-to-identity tables (OCCTSwiftInteraction#7). +public struct ShapeIdentity: Sendable { + public let shape: Shape + public let graph: BRepGraph? + public let faces: FaceIdentityTable + public let edges: EdgeIdentityTable + public let vertices: VertexIdentityTable + + public init(shape: Shape, graph: BRepGraph?) // caller's graph, nil supported + public init(shape: Shape) // mints its own } public enum CADFileLoader { @@ -80,9 +93,12 @@ public enum CADFileLoader { /// bridges each shape to a `ViewportBody` + `CADBodyMetadata`. public static func load( from url: URL, format: CADFileFormat, - progress: ImportProgress? = nil + progress: ImportProgress? = nil, + includeIdentity: Bool = false ) async throws -> CADLoadResult - public static func loadFromManifest(at url: URL) throws -> CADLoadResult + public static func loadFromManifest( + at url: URL, includeIdentity: Bool = false + ) throws -> CADLoadResult public static func shapeToBodyAndMetadata( _ shape: Shape, id: String, color: SIMD4, stl: Bool = false, deflection: Double? = nil, gpuTessellation: Bool = false, diff --git a/okf/components/OCCTSwiftCADKit.md b/okf/components/OCCTSwiftCADKit.md index 4008470..4c7abe4 100644 --- a/okf/components/OCCTSwiftCADKit.md +++ b/okf/components/OCCTSwiftCADKit.md @@ -51,7 +51,10 @@ timestamp: 2026-06-22 reads the body's `FaceIdentityTable`/`EdgeIdentityTable`/`VertexIdentityTable`); `.faceIndex`/`.edgeIndex`/`.vertexIndex` are ephemeral render-path ordinals only. Since OCCTSwiftInteraction#3 each stores a `SubShapeRef` as `ref` and forwards `.shape`/`.uid`/the - ordinal to it, so identity is the resolver's, not a parallel copy of it. + ordinal to it, so identity is the resolver's, not a parallel copy of it. Since + OCCTSwiftInteraction#7 the tables themselves are not built here either: a file load asks for + `CADLoadResult.identity` (`includeIdentity: true`) and an in-memory shape goes through + `OCCTSwiftTools.ShapeIdentity`, both installed by the internal `installIdentity(_:)`. `PickedFaceInfo.scalarValue` is the picked face's value from the body's `ScalarField`, if any. The clip-plane pre-filter and the descriptive enrichment around each info type stay here on purpose: clip planes are this service's state, and enrichment is presentation, so neither belongs in the diff --git a/okf/components/OCCTSwiftTools.md b/okf/components/OCCTSwiftTools.md index d8b1462..30f0f88 100644 --- a/okf/components/OCCTSwiftTools.md +++ b/okf/components/OCCTSwiftTools.md @@ -15,13 +15,19 @@ and SPEC.md): - **`CADFileLoader`**: `Shape` → `ViewportBody` conversion (`shapeToBodyAndMetadata`) and STEP/STL/ OBJ/BREP loading; produces triangulated meshes plus picking metadata. - **`CADBodyMetadata`** / **`CADLoadResult`** / **`CADFileFormat`**: face/edge/vertex indices for - sub-body selection, the aggregated load result (bodies + metadata + shapes + GD&T), and the - input-format enum (`.step`, `.stl`, `.obj`, `.brep`). + sub-body selection, the aggregated load result (bodies + metadata + shapes + GD&T + per-body + identity), and the input-format enum (`.step`, `.stl`, `.obj`, `.brep`). - **`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`. Obtained from `CADFileLoader.shapeToBodyMetadataAndIdentity` (face only) or - `shapeToBodyMetadataAndIdentities` (all three). + durable `GraphUID`. +- **`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 + body via `CADLoadResult.identity` when called with `includeIdentity: true`, which is what stops + a consumer pairing `shapes` with `bodies` positionally. `shapeToBodyMetadataAndIdentity` (face + only) and `shapeToBodyMetadataAndIdentities` (all three) remain as the one-pass mesh-plus-identity + convenience. - **`SubShapePickResolver`**: the one canonical pick resolver: a GPU pick's primitive index in, a `SubShapeRef` out, handling the `faceIndices` / `edgeIndices` / `vertexIndices` indirection, the bounds checks, the empty-`vertexIndices` identity mapping, and the identity-table-over-re-derivation