diff --git a/docs/design.md b/docs/design.md index b6ff2da..bf83067 100644 --- a/docs/design.md +++ b/docs/design.md @@ -103,8 +103,16 @@ as a class carrying the `_s` statics, and the app calls `TopoDS.Face_s(...)`, so ## GIL policy Released around calls that stay inside the kernel and cannot re-enter Python: -`Build`/`Perform`, meshing, `BRepCheck_Analyzer`, file readers and writers, and -every `n3xd_ocp` bulk API. Applied from an explicit list, never blanket. +`Build`/`Perform`, meshing, `BRepCheck_Analyzer`, `RWStl`, and every +`n3xd_ocp` bulk API. Applied from an explicit list, never blanket. + +**The XSTEP readers and writers are the exception**, amending what this section +said before Inc 3 landed. STEP and IGES read and write through the +process-global `Interface_Static` settings table, the IGES reader is documented +as not thread-safe, and the app already serialises every import behind a lock — +so holding the GIL costs nothing there and removes a whole class of question. +Upstream releases nowhere, so this also stays closer to it. `RWStl` touches no +global state and does release. `BinTools` is *also* GIL-free for the kernel half, which the original plan assumed impossible. Rather than bridging a `streambuf` that calls back into @@ -113,6 +121,19 @@ the kernel a pure C++ stream. That is correct regardless of how BinTools seeks, and costs one extra copy of the payload — which `n3xd_ocp.bintools` avoids entirely for the pool paths that care. +**An unregistered type cannot be a default argument.** nanobind converts +defaults to Python objects at *binding* time, so a `.def(..., "Algo"_a = +Extrema_ExtAlgo_Grad)` for an enum this binding does not register fails the +whole extension's import with a bare `std::bad_cast` — no file, no line. It +happened three times while writing Inc 1 and 2. Where the trailing argument is +one the app never overrides, the fix is to leave it off and let OCCT apply its +own default: `GeomAPI_ProjectPointOnSurf` (Extrema algo), +`BRepFilletAPI_MakeFillet` (`ChFi3d_FilletShape`), +`BRepOffsetAPI_MakeThickSolid.MakeThickSolidByJoin` (mode and join type) and +`BRepExtrema_DistShapeShape` (Extrema flags) all do. `tools/sigdiff.py` reports +each of them, which is the point — they are the only four places the bound +surface deliberately differs from upstream's. + **None as an argument** is rejected before the caster for simple overloads, so handle parameters that legitimately accept a null handle need an explicit `nb::arg("x").none()`. Null *returns* map to `None` unconditionally. The app @@ -185,11 +206,64 @@ def set_signal(arm_fpe: bool = False) -> None The backend adopts these after cutover, one call site at a time. +## Matching upstream, and how that is checked + +`inventory.py --check` answers "does the symbol exist". It cannot answer "does +it mean the same thing", and the gap between those two is where a binding does +real damage. `nb::init` for +`BRepPrimAPI_MakePrism` compiled cleanly and bound OCCT's *semi-infinite* +overload, because that one takes a `gp_Dir` and `gp_Dir` converts implicitly +from `gp_Vec` — so the flags shifted one position along and the result was a +valid solid of the wrong shape. The fixture digests caught it; +`tools/sigdiff.py` (`make sigdiff`) finds the class of bug directly, by diffing +every bound constructor and member against the stock wheel. + +Two further limits are worth stating, because they shaped how the increments +were gated: + +- **Static analysis cannot see instance methods.** A method called on an object + (`vec.Reverse()`) appears in no import, so `--check` is blind to it and + `--methods` only guesses. Six such gaps survived to the end of Inc 4 and the + app's suite found all six in one run — one of them, `gp_Vec.Reverse`, failing + 311 tests by itself. The suite is the only real net here. +- **The app's suite cannot gate a single increment.** `backend/tests/ + conftest.py` imports `n3xd.main`, so every test fails at collection until the + last module is bound. Increments are gated instead on reference values + `tools/gen_fixtures.py` records from the stock wheel — measurements, per-face + area and centroid in map order, mesh counts, and the `Modified`/`Generated`/ + `IsDeleted` maps compared exactly, since that is the substrate the app's + topological naming is built on. + +## OCCT 8.0 watchlist + +Recorded as each module landed, so the 10E bump is a known quantity: + +- `Standard_Failure` derives `std::exception`, and the `Raise`/`Throw`/ + `Instance` static helpers are gone. None of them is bound, so the exception + table should port unchanged. +- `GeomLProp` is superseded by the new `GeomProp`/`BRepProp` packages, whose + results come back as structs with `IsDefined` flags instead of throwing. Only + `GeomLProp_SLProps.Normal` is bound, so this is a one-line move. +- `StdPrs_BRepFont` and `StdPrs_BRepTextBuilder` become deprecated typedefs of + `Font_BRepFont`/`Font_BRepTextBuilder`. They are real classes in 7.9.3 and are + bound as such; 8.0 makes it a rename behind the same `OCP.StdPrs` names. +- Methods that returned handles through an out-parameter return them by value. + `BRep_Tool.Surface`/`Curve` and the adaptors are the ones this binding + touches. +- The global math wrappers are deprecated in favour of `std::`. Nothing here + binds them. +- The byte-identity fixtures retire at the bump: a different kernel legitimately + writes different BREP bytes. The measurement and history blocks stay. + ## Open questions -- **S5** — whether STEP needs `CSF_*` resource files shipped in the wheel. - Modern OCCT code-initialises most `Interface_Static` defaults; settle it when - Inc 3 (I/O) lands, in a container without system OCCT resources. +- ~~**S5** — whether STEP needs `CSF_*` resource files shipped in the wheel.~~ + **Settled at Inc 3: it does not.** `tests/test_inc3_io.py` asserts that no + `CSF_*` variable is set and then round-trips STEP and IGES, reading the + declared units back off both — which is exactly the resource-less container + the question was about. Both controllers initialise and the readers resolve + millimetres. The wheel ships no `share/` tree. - **Byte-identity beyond 7.9.3** — the gate compares against the stock wheel, so it necessarily retires at the OCCT 8.0 bump (roadmap 10E), where the fixtures - are re-blessed deliberately alongside assay's goldens. + are re-blessed deliberately alongside assay's goldens. See the 8.0 watchlist + above for what else moves.