From e87f39bc7c1faecf2ecd94d11196163dddbe102a Mon Sep 17 00:00:00 2001 From: gsdali <51393997+gsdali@users.noreply.github.com> Date: Wed, 5 Aug 2026 21:10:04 +1000 Subject: [PATCH 1/2] docs: record that the concaveEdges fix will not be backported to 1.x Upstream have confirmed no backport for OCCTSwift#695. The fix exists only on the 2.0.0 refactor branch, and the only way to get it before 2.0.0 ships is to pin to that branch. Pinning to a branch is not viable here: this package is released under a semver floor and a revision pin would propagate an unstable dependency to every downstream consumer. CLAUDE.md already records being bitten by revision-pinning on OCCTSwiftViewport. The OKF entry and recipe 01's header both described the geometric selection as "a 1.x workaround with a known end date", which reads as though a patch release might arrive. It will not. Both now say the workaround is permanent while this repo is on 1.x, and becomes removable only at a 2.0.0 migration. No code change. Co-Authored-By: Claude Opus 5 (1M context) --- ...-edge-classifier-can-select-wrong-edges.md | 22 +++++++++++++------ recipes/01-mounting-bracket/main.swift | 9 ++++---- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/okf/decisions/concave-edge-classifier-can-select-wrong-edges.md b/okf/decisions/concave-edge-classifier-can-select-wrong-edges.md index 004ef7b..fc6a78a 100644 --- a/okf/decisions/concave-edge-classifier-can-select-wrong-edges.md +++ b/okf/decisions/concave-edge-classifier-can-select-wrong-edges.md @@ -28,13 +28,21 @@ published `v2.0.0-kernel.1` prerelease with the same repro: A T-prism with two reentrant edges reports 3 on 1.17.0 and 2 on 2.0.0-kernel.1. A box, having no reentrant edges, is correct on both. -The fix was too involved to backport to the 1.x line, so it is carried by the 2.0.0 refactor. -Raised upstream as [OCCTSwift#695](https://github.com/SecondMouseAU/OCCTSwift/issues/695). - -**So the geometric selection below is a 1.x workaround with a known end date.** When this repo -moves to the 2.0.0 line, `concaveEdges()` becomes usable for this case again, and recipe 01 could -return to it. That would be a legitimate simplification rather than a regression. Re-run the -repro above before relying on it, rather than assuming the migration carried the fix. +**No 1.x fix will land.** The change was too involved to backport, and upstream have confirmed a +backport will not happen: the fix exists only on the 2.0.0 refactor branch, and the only way to get +it before 2.0.0 ships is to pin to that branch. Raised upstream as +[OCCTSwift#695](https://github.com/SecondMouseAU/OCCTSwift/issues/695). + +Pinning to a branch is not an option for this package. It is released under a semver floor +(`from: "1.17.0"`), and a revision pin would propagate an unresolvable or unstable dependency to +every downstream consumer. The repo has been bitten by revision-pinning before, see the +OCCTSwiftViewport note in `CLAUDE.md`. + +**So the geometric selection below is permanent for as long as this repo is on the 1.x line.** Do +not wait for a patch release; there will not be one. The workaround becomes removable only at a +2.0.0 migration, at which point `concaveEdges()` is usable for this case again and recipe 01 could +return to it as a legitimate simplification. Re-run the repro above at that point rather than +assuming the migration carried the fix. # Why diff --git a/recipes/01-mounting-bracket/main.swift b/recipes/01-mounting-bracket/main.swift index ebf3cbc..5f190bd 100644 --- a/recipes/01-mounting-bracket/main.swift +++ b/recipes/01-mounting-bracket/main.swift @@ -53,10 +53,11 @@ let prism = Shape.extrude(profile: lProfile, direction: SIMD3(0, 0, 1), length: // The inside corner is the one straight edge parallel to the extrusion axis (Z) that // sits at (thickness, thickness): select it geometrically rather than trusting // concaveEdges(), which picks the wrong edges on this shape (see the header note). -// That classifier defect is OCCTSwift 1.x only: it is fixed in the 2.0.0 line -// (verified on v2.0.0-kernel.1, upstream OCCTSwift#695). This geometric selection is -// therefore a 1.x workaround, and this recipe could return to concaveEdges() once the -// package moves to 2.0.0. Re-run the check in the OKF entry before doing so. +// That classifier defect is OCCTSwift 1.x only and will not be fixed there: the fix +// lives on the 2.0.0 refactor branch and upstream have confirmed no backport +// (OCCTSwift#695). So this geometric selection is permanent while the package is on +// 1.x, not a stopgap awaiting a patch release. It becomes removable at a 2.0.0 +// migration; re-run the check in the OKF entry then rather than assuming. let insideCornerEdges = prism.edges { edge in guard edge.isLine else { return false } let b = edge.bounds From 76749be836611a9fcf60036af46af8fd4d915787 Mon Sep 17 00:00:00 2001 From: gsdali <51393997+gsdali@users.noreply.github.com> Date: Wed, 5 Aug 2026 22:19:46 +1000 Subject: [PATCH 2/2] docs: correct the backport account, add the missing log line and cross-references Review of #107 pointed out that "too involved to backport" contradicts the upstream record, and it does. Checking OCCTSwift#695's closing comment: a backport was built, merged and measured (PR #700, all 4646 tests green on both CI jobs), then reverted, not for any defect in it but for what it costs the release branch. Merging main into refactor/381-pass1b conflicted in 8 files across 18 regions, and those are not all mechanical: main and the refactor branch made genuinely different, individually correct decisions on the same lines. Corrected, and captured the escape hatch the upstream comment preserves: re-landing means reverting the revert, so this can be revisited if 1.x becomes untenable before v2.0.0 ships. Also recorded the root cause, because it generalises beyond this call. An edge or vertex index crossing the bridge addressed a topology occurrence rather than a position in the deduplicated enumeration that edges() / edgeCount / edge(at:) return. A 20mm box has 12 distinct edges but 24 occurrences, so the two diverge from the first repeat onwards on any ordinary solid, and edgeConcavities() zips one against the other. Upstream root cause is OCCTSwift#613, fix #650. Any bridge-crossing index deserves the same suspicion until 2.0.0. Corrected the pinning paragraph too: the fix is reachable from the published v2.0.0-kernel.1 tag as well as the branch, but neither is open to this package, because from: "1.17.0" means >=1.17.0 <2.0.0 and cannot resolve a 2.x version at all without a major bump. Remaining review items: * Added the okf/log.md line the standing rule asks for alongside an OKF entry. * CLAUDE.md's OCCTSwiftViewport paragraph still said that dependency "is currently revision-pinned to a main commit", but Package.swift:88 has carried from: "1.0.4" for some time. Put into past tense and kept as the standing reason this repo avoids revision pins, which is what the new cross-reference points at. * recipes/01-mounting-bracket/README.md's Gotchas described the workaround with no version scope; added it, so the cookbook's own three files carry the whole record. * Fixed the dangling "rather than assuming" in the recipe comment. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 2 +- ...-edge-classifier-can-select-wrong-edges.md | 30 ++++++++++++++----- okf/log.md | 5 ++++ recipes/01-mounting-bracket/README.md | 6 ++++ recipes/01-mounting-bracket/main.swift | 3 +- 5 files changed, 37 insertions(+), 9 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5bd8768..881bc8c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -80,7 +80,7 @@ No tests exist. No linter is configured. **`render-preview` AIS overlays (Phase 2)**: `--show-axes` overlays an `OCCTSwiftAIS.Trihedron` (sized to half the union bbox diagonal). Position via `--axes-position origin|center|outside|x,y,z`; default is **`outside`** (anchored 20% of the diagonal beyond the bbox-min corner so all three arrows extend into the part region and stay visible regardless of where the part sits in world space, which fixes the original Phase 2 quirk where arrows hid inside parts spanning the origin). `--show-workplane xy|yz|xz` overlays an `OCCTSwiftAIS.WorkPlane` (sized 1.2× the diagonal, semi-transparent blue by default). `--highlight face[N],edge[M],vertex[K]` extracts sub-shapes from the **first** input BREP via `Shape.subShape(type:index:)` and renders them as separate ViewportBodies with an orange highlight colour (override via `--highlight-color #hex`). The face/edge/vertex IDs use the same `face[N]` / `edge[N]` / `vertex[N]` scheme that `query-topology` emits, so the two verbs cross-reference cleanly. `--highlight` is scoped to the first input only (multi-BREP scenes that want to highlight in body N should render that body solo). `--annotate-dimensions` (the fourth Phase 2 candidate) is **not implemented**. `OCCTSwiftAIS.LinearDimension` / `AngularDimension` / `RadialDimension` render via `MeasurementOverlay`, a SwiftUI Canvas inside `MetalViewportView` that isn't reachable from `OffscreenRenderer`. Filed upstream as **OCCTSwiftViewport#26** (request: `OffscreenRenderOptions.measurements` + a CGContext post-pass on the rendered image). The verb-side wiring is one parse step + array-passthrough once that lands. -**`render-preview`** is the OCCTMCP-driven headless PNG render verb (closes #24). Wraps OCCTSwiftViewport's `OffscreenRenderer` (added in OCCTSwiftViewport#18). Input: one or more BREP paths or a `--manifest`; output: a PNG at `--output`. Camera presets `iso|front|back|top|bottom|left|right` (compute view from union bounding box) or explicit `--camera-position` / `--camera-target` / `--camera-up`. `--display-mode shaded|wireframe|shaded-with-edges|flat|xray|rendered`, `--background light|dark|transparent|#hex`, `--width` / `--height` (default 800×600). The verb hops to the main actor via `MainActor.assumeIsolated` to drive the `OffscreenRenderer` because the umbrella dispatcher is `@MainActor` but the `Subcommand.run(args:)` requirement is nonisolated. The OCCTSwiftViewport dep is currently revision-pinned to a `main` commit because OffscreenRenderer hadn't been cut as a release tag yet (latest tag was v0.49.0 from 2026-03-16, pre-OffscreenRenderer); bump to a `from: ""` pin once a Viewport release ships. +**`render-preview`** is the OCCTMCP-driven headless PNG render verb (closes #24). Wraps OCCTSwiftViewport's `OffscreenRenderer` (added in OCCTSwiftViewport#18). Input: one or more BREP paths or a `--manifest`; output: a PNG at `--output`. Camera presets `iso|front|back|top|bottom|left|right` (compute view from union bounding box) or explicit `--camera-position` / `--camera-target` / `--camera-up`. `--display-mode shaded|wireframe|shaded-with-edges|flat|xray|rendered`, `--background light|dark|transparent|#hex`, `--width` / `--height` (default 800×600). The verb hops to the main actor via `MainActor.assumeIsolated` to drive the `OffscreenRenderer` because the umbrella dispatcher is `@MainActor` but the `Subcommand.run(args:)` requirement is nonisolated. The OCCTSwiftViewport dep **was** revision-pinned to a `main` commit while OffscreenRenderer had no release tag (the latest was v0.49.0 from 2026-03-16, pre-OffscreenRenderer). That is resolved: `Package.swift` now pins `from: "1.0.4"`. Kept here as the standing reason this repo avoids revision pins, since one propagates an unstable dependency to every downstream consumer. **`mesh` / `simplify-mesh`** are the OCCTMCP-driven mesh verbs (closes #22). `mesh` wraps `Shape.mesh(parameters: MeshParameters)` (which drives `BRepMesh_IncrementalMesh`); reports `triangleCount` / `vertexCount` plus quality metrics (`minAspectRatio` / `meanAspectRatio` / `degenerateTriangles` / `nonManifoldEdges`). Returns triangle data inline (`{ "geometry": { "vertices": [...], "indices": [...] } }`) by default; if `--output ` is supplied OR triangle count exceeds `inlineTriangleThreshold` (100K), writes the mesh to disk via `Exporter.writeSTL` / `writeOBJ` and sets `geometry: null` + `outputPath: "..."`. `simplify-mesh` wraps OCCTSwiftMesh's `Mesh.simplified(_:)` (QEM decimation backed by vendored meshoptimizer); requires exactly one of `--target-triangle-count` / `--target-reduction`, supports `--max-hausdorff-distance` cap and `--preserve-boundary` / `--preserve-topology` flags. Writes the decimated mesh to `--output` (.stl or .obj) via a small Mesh-direct ASCII writer (OCCTSwift's `writeSTL` / `writeOBJ` take `Shape`, not `Mesh`). Reports `qualityDelta.meanAspectRatioDelta` + `qualityDelta.hausdorffDistance` (in input mesh units). Mesh-domain algorithms were ruled out-of-scope for OCCTSwift core in OCCTSwift#92 and moved to OCCTSwiftMesh: same LGPL boundary, vendored meshoptimizer license chain documented in the verb header. diff --git a/okf/decisions/concave-edge-classifier-can-select-wrong-edges.md b/okf/decisions/concave-edge-classifier-can-select-wrong-edges.md index fc6a78a..0b715d2 100644 --- a/okf/decisions/concave-edge-classifier-can-select-wrong-edges.md +++ b/okf/decisions/concave-edge-classifier-can-select-wrong-edges.md @@ -28,15 +28,31 @@ published `v2.0.0-kernel.1` prerelease with the same repro: A T-prism with two reentrant edges reports 3 on 1.17.0 and 2 on 2.0.0-kernel.1. A box, having no reentrant edges, is correct on both. -**No 1.x fix will land.** The change was too involved to backport, and upstream have confirmed a -backport will not happen: the fix exists only on the 2.0.0 refactor branch, and the only way to get -it before 2.0.0 ships is to pin to that branch. Raised upstream as +**No 1.x fix will land**, but not because a backport was infeasible. Upstream built, merged and +measured one (OCCTSwift PR #700, all 4646 tests green on both CI jobs) and then **reverted it**, +for what it costs the release branch rather than for any defect in it: merging main into +`refactor/381-pass1b`, which carries the whole of v2.0.0, conflicted in 8 files across 18 regions, +and those are not all mechanical. Main and the refactor branch made genuinely different, individually +correct decisions on the same lines, because the sub-shape indexing work landed only on the refactor +branch. + +The backport is not lost: re-landing means reverting the revert, so if 1.x becomes untenable before +v2.0.0 ships this can be revisited. Tracked at [OCCTSwift#695](https://github.com/SecondMouseAU/OCCTSwift/issues/695). -Pinning to a branch is not an option for this package. It is released under a semver floor -(`from: "1.17.0"`), and a revision pin would propagate an unresolvable or unstable dependency to -every downstream consumer. The repo has been bitten by revision-pinning before, see the -OCCTSwiftViewport note in `CLAUDE.md`. +**Root cause, worth knowing because it generalises.** An edge or vertex index crossing the bridge +addressed a topology *occurrence* rather than a position in the deduplicated enumeration that +`edges()` / `edgeCount` / `edge(at:)` return. A 20 mm box has 12 distinct edges but 24 edge +occurrences, so the two enumerations diverge from the first repeat onwards on any ordinary solid, +and `edgeConcavities()` zips one against the other. Upstream root-cause issue is OCCTSwift#613; the +fix is #650. Treat any bridge-crossing index with the same suspicion until 2.0.0. + +Neither route to the fix is open to this package. It is reachable both from the refactor branch +and from the published `v2.0.0-kernel.1` prerelease tag, but this package is released under a semver +floor of `from: "1.17.0"`, meaning `>=1.17.0 <2.0.0`, so it cannot resolve a 2.x version at all +without a major bump, and a revision pin would propagate an unstable dependency to every downstream +consumer. This repo has carried a revision pin before (OCCTSwiftViewport, before `OffscreenRenderer` +had a release tag) and the note in `CLAUDE.md` records why it was replaced with a version pin. **So the geometric selection below is permanent for as long as this repo is on the 1.x line.** Do not wait for a patch release; there will not be one. The workaround becomes removable only at a diff --git a/okf/log.md b/okf/log.md index 0624f5c..b3e9f0d 100644 --- a/okf/log.md +++ b/okf/log.md @@ -28,6 +28,11 @@ edges, so the call returned nil and a `??` fallback hid it. Now selects the OD and raised-face rim explicitly. * **Creation**: Recorded the revolve-seams-cannot-be-chamfered decision. +* **Update**: Recorded that OCCTSwift#695, the concaveEdges/convexEdges reentrant-edge + misclassification, will not be backported to 1.x. A backport was built and merged upstream + (PR #700, tests green) then reverted for what it costs the 2.0.0 release branch. The geometric + edge selection in recipe 01 is therefore permanent while this repo is on the 1.x line, not a + stopgap awaiting a patch release. ## 2026-08-04 diff --git a/recipes/01-mounting-bracket/README.md b/recipes/01-mounting-bracket/README.md index abe2dec..7d524bc 100644 --- a/recipes/01-mounting-bracket/README.md +++ b/recipes/01-mounting-bracket/README.md @@ -50,6 +50,12 @@ by 1 mm so the resulting cut faces are clean and coincident-face artifacts are a (OCCTSwiftScripts #105). The true inside-corner edge has no such limit (its bound is `legLength − thickness`, 45 mm here), which is why the same `filletRadius = 8` works fine once the correct edge is selected. +- **This is an OCCTSwift 1.x defect and no 1.x fix is coming.** It is fixed in the 2.0.0 line + (upstream OCCTSwift#695, root cause #613). A backport was built and merged upstream, then + reverted for what it costs the 2.0.0 release branch, so the geometric selection above is + permanent while this package is on 1.x rather than a stopgap. It becomes removable at a 2.0.0 + migration. See `okf/decisions/concave-edge-classifier-can-select-wrong-edges.md`, which carries + the repro to re-run at that point. - **A concave fillet adds material, it does not remove it.** Rounding the inside corner fills part of the sharp reentrant point with a blend, so `bracket.volume` after the fillet is *larger* than the prism's, by `filletRadius² · (1 − π/4) · width`. Do not expect diff --git a/recipes/01-mounting-bracket/main.swift b/recipes/01-mounting-bracket/main.swift index 5f190bd..865b12f 100644 --- a/recipes/01-mounting-bracket/main.swift +++ b/recipes/01-mounting-bracket/main.swift @@ -57,7 +57,8 @@ let prism = Shape.extrude(profile: lProfile, direction: SIMD3(0, 0, 1), length: // lives on the 2.0.0 refactor branch and upstream have confirmed no backport // (OCCTSwift#695). So this geometric selection is permanent while the package is on // 1.x, not a stopgap awaiting a patch release. It becomes removable at a 2.0.0 -// migration; re-run the check in the OKF entry then rather than assuming. +// migration; re-run the check in the OKF entry at that point rather than assuming +// the migration carried the fix. let insideCornerEdges = prism.edges { edge in guard edge.isLine else { return false } let b = edge.bounds