diff --git a/pyproject.toml b/pyproject.toml index 6207f16..65a3c8d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,5 +1,5 @@ [build-system] -requires = ["scikit-build-core>=0.11", "nanobind>=2.7"] +requires = ["scikit-build-core>=0.11", "nanobind>=2.7,<3"] build-backend = "scikit_build_core.build" [project] diff --git a/scripts/_env.sh b/scripts/_env.sh index 4bfbd4e..5f3e292 100755 --- a/scripts/_env.sh +++ b/scripts/_env.sh @@ -24,7 +24,10 @@ if [ ! -x "$VENV/bin/python" ]; then "$VENV/bin/pip" install -q --upgrade pip # numpy is a test dependency only: the n3xd_ocp bulk APIs hand back # ndarrays, so their tests need it to check the results. - "$VENV/bin/pip" install -q build nanobind scikit-build-core pytest numpy + # nanobind 3 reworked the type-caster and rv_policy interfaces + # occt_handle.h builds on, so moving to it is a deliberate bump, not + # something a fresh cache picks up. + "$VENV/bin/pip" install -q build "nanobind<3" scikit-build-core pytest numpy fi PY="$VENV/bin/python" export PY VENV CACHE diff --git a/src/common/occt_ndarray.h b/src/common/occt_ndarray.h new file mode 100644 index 0000000..9f234c6 --- /dev/null +++ b/src/common/occt_ndarray.h @@ -0,0 +1,33 @@ +/* + std::vector -> numpy without a copy. + + The bulk APIs fill plain vectors while the GIL is released, then hand each + one to numpy. Moving the vector into a capsule makes it the array's + storage, so there is no second buffer to copy into, and nothing leaks if + the kernel throws halfway through filling it. +*/ + +#pragma once + +#include +#include + +#include +#include +#include + +namespace nb = nanobind; + +/// Wrap `data` as a numpy array of `shape`; the array owns the buffer. +template +nb::object ocp_ndarray(std::vector &&data, + std::initializer_list shape) { + auto *owned = new std::vector(std::move(data)); + // An empty vector may have no buffer at all; numpy wants a real pointer + // even for a zero-length array. + owned->reserve(1); + nb::capsule owner(owned, [](void *p) noexcept { + delete static_cast *>(p); + }); + return nb::cast(nb::ndarray(owned->data(), shape, owner)); +} diff --git a/src/ext/ext_measure.cpp b/src/ext/ext_measure.cpp index 7b66dc4..1decbdd 100644 --- a/src/ext/ext_measure.cpp +++ b/src/ext/ext_measure.cpp @@ -18,8 +18,7 @@ */ #include "../common/occt_module.h" - -#include +#include "../common/occt_ndarray.h" #include #include @@ -35,11 +34,6 @@ namespace { -/// Own a heap buffer for as long as the numpy array that wraps it lives. -template nb::capsule owner_of(T *data) { - return nb::capsule(data, [](void *p) noexcept { delete[] (T *) p; }); -} - void measure_range(const TopTools_IndexedMapOfShape &faces, double eps, double *areas, double *centroids, int begin, int end) { for (int i = begin; i < end; ++i) { @@ -81,27 +75,25 @@ void register_ext_measure() { TopExp::MapShapes(shape, TopAbs_FACE, faces); const int count = faces.Extent(); - double *areas = new double[count > 0 ? count : 1]; - double *centroids = new double[count > 0 ? 3 * count : 1]; + std::vector areas((size_t) count); + std::vector centroids(3 * (size_t) count); { nb::gil_scoped_release nogil; if (parallel && count > 1) { OSD_Parallel::For(0, count, [&](int i) { - measure_range(faces, eps, areas, centroids, i, i + 1); + measure_range(faces, eps, areas.data(), centroids.data(), i, + i + 1); }); } else { - measure_range(faces, eps, areas, centroids, 0, count); + measure_range(faces, eps, areas.data(), centroids.data(), 0, + count); } } - size_t area_shape[1] = {(size_t) count}; - size_t centroid_shape[2] = {(size_t) count, 3}; return nb::make_tuple( - nb::ndarray>(areas, 1, area_shape, - owner_of(areas)), - nb::ndarray>( - centroids, 2, centroid_shape, owner_of(centroids))); + ocp_ndarray(std::move(areas), {(size_t) count}), + ocp_ndarray(std::move(centroids), {(size_t) count, 3})); }, "shape"_a, nb::kw_only(), "parallel"_a = true, "eps"_a = nb::none(), R"doc(Area and centre of mass of every face, in MapShapes(FACE) order. diff --git a/src/ext/ext_sample.cpp b/src/ext/ext_sample.cpp index c31f41f..4cc9bfa 100644 --- a/src/ext/ext_sample.cpp +++ b/src/ext/ext_sample.cpp @@ -21,8 +21,7 @@ */ #include "../common/occt_module.h" - -#include +#include "../common/occt_ndarray.h" #include #include @@ -33,13 +32,10 @@ #include #include +#include namespace { -template nb::capsule owner_of(T *data) { - return nb::capsule(data, [](void *p) noexcept { delete[] (T *) p; }); -} - /// numpy's linspace endpoint handling: the last sample is the bound itself, /// not start + (n-1)*step, which can miss it by an ulp. inline double linspace_at(double lo, double hi, int i, int n) { @@ -67,8 +63,8 @@ void register_ext_sample() { face.Orientation() == TopAbs_REVERSED ? -1.0 : 1.0; const size_t count = (size_t) n * (size_t) n; - double *points = new double[3 * count]; - double *normals = new double[3 * count]; + std::vector points(3 * count); + std::vector normals(3 * count); { nb::gil_scoped_release nogil; @@ -103,12 +99,9 @@ void register_ext_sample() { } } - size_t shape[3] = {(size_t) n, (size_t) n, 3}; return nb::make_tuple( - nb::ndarray>(points, 3, shape, - owner_of(points)), - nb::ndarray>(normals, 3, shape, - owner_of(normals)), + ocp_ndarray(std::move(points), {(size_t) n, (size_t) n, 3}), + ocp_ndarray(std::move(normals), {(size_t) n, (size_t) n, 3}), nb::make_tuple(umin, umax, vmin, vmax)); }, "face"_a, "n"_a, diff --git a/src/ext/ext_tess.cpp b/src/ext/ext_tess.cpp index bf1010d..fb0dc35 100644 --- a/src/ext/ext_tess.cpp +++ b/src/ext/ext_tess.cpp @@ -26,8 +26,7 @@ */ #include "../common/occt_module.h" - -#include +#include "../common/occt_ndarray.h" #include #include @@ -48,18 +47,6 @@ namespace { -template nb::capsule owner_of(T *data) { - return nb::capsule(data, [](void *p) noexcept { delete[] (T *) p; }); -} - -/// Wrap an owned buffer as an (n, cols) numpy array. -template -nb::object as_matrix(T *data, size_t rows, size_t cols) { - size_t shape[2] = {rows, cols}; - return nb::cast( - nb::ndarray>(data, 2, shape, owner_of(data))); -} - /// One face's mesh, already flattened and oriented, ready to hand to Python. struct FaceMesh { int index = 0; @@ -140,25 +127,18 @@ void register_ext_tess() { } nb::list out; - for (const FaceMesh &mesh : meshes) { + for (FaceMesh &mesh : meshes) { const size_t n = mesh.nodes.size() / 3; const size_t t = mesh.tris.size() / 3; - double *nodes = new double[mesh.nodes.size()]; - std::copy(mesh.nodes.begin(), mesh.nodes.end(), nodes); - uint32_t *tris = new uint32_t[mesh.tris.size()]; - std::copy(mesh.tris.begin(), mesh.tris.end(), tris); - nb::dict entry; entry["face_index"] = mesh.index; - entry["nodes"] = as_matrix(nodes, n, 3); - entry["triangles"] = as_matrix(tris, t, 3); + entry["nodes"] = ocp_ndarray(std::move(mesh.nodes), {n, 3}); + entry["triangles"] = ocp_ndarray(std::move(mesh.tris), {t, 3}); if (mesh.normals.empty()) { entry["normals"] = nb::none(); } else { - double *normals = new double[mesh.normals.size()]; - std::copy(mesh.normals.begin(), mesh.normals.end(), normals); - entry["normals"] = as_matrix(normals, n, 3); + entry["normals"] = ocp_ndarray(std::move(mesh.normals), {n, 3}); } out.append(entry); } @@ -203,12 +183,11 @@ the app-wide face identity, not a position in this list.)doc"); } nb::list out; - for (const auto &entry : polylines) { - double *points = new double[entry.second.size()]; - std::copy(entry.second.begin(), entry.second.end(), points); + for (auto &entry : polylines) { + const size_t count = entry.second.size() / 3; nb::dict row; row["edge_index"] = entry.first; - row["points"] = as_matrix(points, entry.second.size() / 3, 3); + row["points"] = ocp_ndarray(std::move(entry.second), {count, 3}); out.append(row); } return out; diff --git a/src/modules/mod_STEPControl.cpp b/src/modules/mod_STEPControl.cpp index 7905f9e..e9b4769 100644 --- a/src/modules/mod_STEPControl.cpp +++ b/src/modules/mod_STEPControl.cpp @@ -13,9 +13,8 @@ The GIL stays held throughout, per the note in mod_XSControl.cpp: XSTEP is built on process-global state and the app serialises imports anyway. - This is also where the S5 question lands (design.md): whether the wheel has - to ship OCCT's share/ resources for STEP to resolve units in a container - with no CSF_* environment. tests/test_inc3_io.py answers it. + The wheel ships none of OCCT's share/ resources; STEP still resolves units + in a container with no CSF_* environment, which tests/test_inc3_io.py pins. */ #include "../common/occt_module.h" diff --git a/tests/test_inc3_io.py b/tests/test_inc3_io.py index 41f828d..77253e6 100644 --- a/tests/test_inc3_io.py +++ b/tests/test_inc3_io.py @@ -4,10 +4,9 @@ Round-trips rather than digests: STEP and IGES payloads carry timestamps and a generator banner, so byte comparison is meaningless — what has to survive is the geometry, which these check by measuring the shape that comes back. -This file also settles open question **S5** from docs/design.md: whether the -wheel has to ship OCCT's share/ resources for the XSTEP layer to resolve units -when no CSF_* variable is set. These tests run in exactly that situation — a -container with the wheel and nothing else — so a pass means the answer is no. +It also pins that the wheel needs none of OCCT's share/ resources for the +XSTEP layer to resolve units when no CSF_* variable is set. These tests run in +exactly that situation, a container with the wheel and nothing else. """ from __future__ import annotations @@ -36,13 +35,13 @@ BOX = (10.0, 20.0, 30.0) @pytest.fixture(scope="module") def initialized(): - """The app's own import bootstrap, and the S5 check in one. + """The app's own import bootstrap, and the resource-free check in one. If the controllers could not initialise without OCCT's resource files, this is where it would fail — there is no CSF_* in this environment. """ assert not any(k.startswith("CSF_") for k in os.environ), ( - "a CSF_* variable is set, so this run cannot answer S5" + "a CSF_* variable is set, so this run cannot prove the wheel needs none" ) assert STEPControl_Controller.Init_s() assert IGESControl_Controller.Init_s() @@ -112,7 +111,7 @@ def test_step_reports_its_file_units(initialized, tmp_path): TColStd_SequenceOfAsciiString(), ) reader.FileUnits(length, angle, solid_angle) - assert length.Length() >= 1, "no length unit reported — S5 would be live" + assert length.Length() >= 1, "no length unit reported — the wheel would need OCCT's share/ resources" names = [length.Value(i).ToCString() for i in range(1, length.Length() + 1)] assert any("MM" in n.upper() or "MILLI" in n.upper() for n in names), names