design.md: record what the increments settled
- GIL policy gains the XSTEP exception (STEP/IGES hold it; RWStl releases). - A new rule, learned three times the hard way: an unregistered type cannot be a default argument, because nanobind converts defaults at binding time and the failure is a bare std::bad_cast at import with no file or line. - A section on matching upstream: what sigdiff exists for, why static analysis cannot see instance methods, and why the app's suite cannot gate a single increment. - An OCCT 8.0 watchlist, so the 10E bump is a known quantity. - S5 marked settled: no CSF_* resources needed in the wheel. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DfriM8XUkn7uYf5Dwe2xo6
This commit is contained in:
@@ -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<TopoDS_Shape, gp_Vec, bool, bool, bool>` 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.
|
||||
|
||||
Reference in New Issue
Block a user