Own bulk-API ndarray buffers with std::vector, pin nanobind below 3

face_grid leaked both buffers when the kernel threw mid-grid, and tess copied
every mesh array a second time. Also drop the stale S5 references.
This commit is contained in:
2026-10-29 14:37:36 +01:00
parent 5a1b0fdeb1
commit 407ef262ba
8 changed files with 69 additions and 71 deletions
+1 -1
View File
@@ -1,5 +1,5 @@
[build-system] [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" build-backend = "scikit_build_core.build"
[project] [project]
+4 -1
View File
@@ -24,7 +24,10 @@ if [ ! -x "$VENV/bin/python" ]; then
"$VENV/bin/pip" install -q --upgrade pip "$VENV/bin/pip" install -q --upgrade pip
# numpy is a test dependency only: the n3xd_ocp bulk APIs hand back # numpy is a test dependency only: the n3xd_ocp bulk APIs hand back
# ndarrays, so their tests need it to check the results. # 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 fi
PY="$VENV/bin/python" PY="$VENV/bin/python"
export PY VENV CACHE export PY VENV CACHE
+33
View File
@@ -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 <nanobind/nanobind.h>
#include <nanobind/ndarray.h>
#include <initializer_list>
#include <utility>
#include <vector>
namespace nb = nanobind;
/// Wrap `data` as a numpy array of `shape`; the array owns the buffer.
template <typename T>
nb::object ocp_ndarray(std::vector<T> &&data,
std::initializer_list<size_t> shape) {
auto *owned = new std::vector<T>(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<std::vector<T> *>(p);
});
return nb::cast(nb::ndarray<nb::numpy, T>(owned->data(), shape, owner));
}
+9 -17
View File
@@ -18,8 +18,7 @@
*/ */
#include "../common/occt_module.h" #include "../common/occt_module.h"
#include "../common/occt_ndarray.h"
#include <nanobind/ndarray.h>
#include <BRepGProp.hxx> #include <BRepGProp.hxx>
#include <GProp_GProps.hxx> #include <GProp_GProps.hxx>
@@ -35,11 +34,6 @@
namespace { namespace {
/// Own a heap buffer for as long as the numpy array that wraps it lives.
template <typename T> 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, void measure_range(const TopTools_IndexedMapOfShape &faces, double eps,
double *areas, double *centroids, int begin, int end) { double *areas, double *centroids, int begin, int end) {
for (int i = begin; i < end; ++i) { for (int i = begin; i < end; ++i) {
@@ -81,27 +75,25 @@ void register_ext_measure() {
TopExp::MapShapes(shape, TopAbs_FACE, faces); TopExp::MapShapes(shape, TopAbs_FACE, faces);
const int count = faces.Extent(); const int count = faces.Extent();
double *areas = new double[count > 0 ? count : 1]; std::vector<double> areas((size_t) count);
double *centroids = new double[count > 0 ? 3 * count : 1]; std::vector<double> centroids(3 * (size_t) count);
{ {
nb::gil_scoped_release nogil; nb::gil_scoped_release nogil;
if (parallel && count > 1) { if (parallel && count > 1) {
OSD_Parallel::For(0, count, [&](int i) { 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 { } 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( return nb::make_tuple(
nb::ndarray<nb::numpy, double, nb::ndim<1>>(areas, 1, area_shape, ocp_ndarray(std::move(areas), {(size_t) count}),
owner_of(areas)), ocp_ndarray(std::move(centroids), {(size_t) count, 3}));
nb::ndarray<nb::numpy, double, nb::ndim<2>>(
centroids, 2, centroid_shape, owner_of(centroids)));
}, },
"shape"_a, nb::kw_only(), "parallel"_a = true, "eps"_a = nb::none(), "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. R"doc(Area and centre of mass of every face, in MapShapes(FACE) order.
+6 -13
View File
@@ -21,8 +21,7 @@
*/ */
#include "../common/occt_module.h" #include "../common/occt_module.h"
#include "../common/occt_ndarray.h"
#include <nanobind/ndarray.h>
#include <BRepAdaptor_Surface.hxx> #include <BRepAdaptor_Surface.hxx>
#include <BRepTools.hxx> #include <BRepTools.hxx>
@@ -33,13 +32,10 @@
#include <cmath> #include <cmath>
#include <stdexcept> #include <stdexcept>
#include <vector>
namespace { namespace {
template <typename T> 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, /// numpy's linspace endpoint handling: the last sample is the bound itself,
/// not start + (n-1)*step, which can miss it by an ulp. /// not start + (n-1)*step, which can miss it by an ulp.
inline double linspace_at(double lo, double hi, int i, int n) { 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; face.Orientation() == TopAbs_REVERSED ? -1.0 : 1.0;
const size_t count = (size_t) n * (size_t) n; const size_t count = (size_t) n * (size_t) n;
double *points = new double[3 * count]; std::vector<double> points(3 * count);
double *normals = new double[3 * count]; std::vector<double> normals(3 * count);
{ {
nb::gil_scoped_release nogil; 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( return nb::make_tuple(
nb::ndarray<nb::numpy, double, nb::ndim<3>>(points, 3, shape, ocp_ndarray(std::move(points), {(size_t) n, (size_t) n, 3}),
owner_of(points)), ocp_ndarray(std::move(normals), {(size_t) n, (size_t) n, 3}),
nb::ndarray<nb::numpy, double, nb::ndim<3>>(normals, 3, shape,
owner_of(normals)),
nb::make_tuple(umin, umax, vmin, vmax)); nb::make_tuple(umin, umax, vmin, vmax));
}, },
"face"_a, "n"_a, "face"_a, "n"_a,
+8 -29
View File
@@ -26,8 +26,7 @@
*/ */
#include "../common/occt_module.h" #include "../common/occt_module.h"
#include "../common/occt_ndarray.h"
#include <nanobind/ndarray.h>
#include <BRepAdaptor_Curve.hxx> #include <BRepAdaptor_Curve.hxx>
#include <BRep_Tool.hxx> #include <BRep_Tool.hxx>
@@ -48,18 +47,6 @@
namespace { namespace {
template <typename T> 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 <typename T>
nb::object as_matrix(T *data, size_t rows, size_t cols) {
size_t shape[2] = {rows, cols};
return nb::cast(
nb::ndarray<nb::numpy, T, nb::ndim<2>>(data, 2, shape, owner_of(data)));
}
/// One face's mesh, already flattened and oriented, ready to hand to Python. /// One face's mesh, already flattened and oriented, ready to hand to Python.
struct FaceMesh { struct FaceMesh {
int index = 0; int index = 0;
@@ -140,25 +127,18 @@ void register_ext_tess() {
} }
nb::list out; nb::list out;
for (const FaceMesh &mesh : meshes) { for (FaceMesh &mesh : meshes) {
const size_t n = mesh.nodes.size() / 3; const size_t n = mesh.nodes.size() / 3;
const size_t t = mesh.tris.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; nb::dict entry;
entry["face_index"] = mesh.index; entry["face_index"] = mesh.index;
entry["nodes"] = as_matrix(nodes, n, 3); entry["nodes"] = ocp_ndarray(std::move(mesh.nodes), {n, 3});
entry["triangles"] = as_matrix(tris, t, 3); entry["triangles"] = ocp_ndarray(std::move(mesh.tris), {t, 3});
if (mesh.normals.empty()) { if (mesh.normals.empty()) {
entry["normals"] = nb::none(); entry["normals"] = nb::none();
} else { } else {
double *normals = new double[mesh.normals.size()]; entry["normals"] = ocp_ndarray(std::move(mesh.normals), {n, 3});
std::copy(mesh.normals.begin(), mesh.normals.end(), normals);
entry["normals"] = as_matrix(normals, n, 3);
} }
out.append(entry); out.append(entry);
} }
@@ -203,12 +183,11 @@ the app-wide face identity, not a position in this list.)doc");
} }
nb::list out; nb::list out;
for (const auto &entry : polylines) { for (auto &entry : polylines) {
double *points = new double[entry.second.size()]; const size_t count = entry.second.size() / 3;
std::copy(entry.second.begin(), entry.second.end(), points);
nb::dict row; nb::dict row;
row["edge_index"] = entry.first; 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); out.append(row);
} }
return out; return out;
+2 -3
View File
@@ -13,9 +13,8 @@
The GIL stays held throughout, per the note in mod_XSControl.cpp: XSTEP is 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. 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 The wheel ships none of OCCT's share/ resources; STEP still resolves units
to ship OCCT's share/ resources for STEP to resolve units in a container in a container with no CSF_* environment, which tests/test_inc3_io.py pins.
with no CSF_* environment. tests/test_inc3_io.py answers it.
*/ */
#include "../common/occt_module.h" #include "../common/occt_module.h"
+6 -7
View File
@@ -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 generator banner, so byte comparison is meaningless — what has to survive is
the geometry, which these check by measuring the shape that comes back. 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 It also pins that the wheel needs none of OCCT's share/ resources for the
wheel has to ship OCCT's share/ resources for the XSTEP layer to resolve units XSTEP layer to resolve units when no CSF_* variable is set. These tests run in
when no CSF_* variable is set. These tests run in exactly that situation — a exactly that situation, a container with the wheel and nothing else.
container with the wheel and nothing else — so a pass means the answer is no.
""" """
from __future__ import annotations from __future__ import annotations
@@ -36,13 +35,13 @@ BOX = (10.0, 20.0, 30.0)
@pytest.fixture(scope="module") @pytest.fixture(scope="module")
def initialized(): 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 If the controllers could not initialise without OCCT's resource files, this
is where it would fail — there is no CSF_* in this environment. is where it would fail — there is no CSF_* in this environment.
""" """
assert not any(k.startswith("CSF_") for k in os.environ), ( 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 STEPControl_Controller.Init_s()
assert IGESControl_Controller.Init_s() assert IGESControl_Controller.Init_s()
@@ -112,7 +111,7 @@ def test_step_reports_its_file_units(initialized, tmp_path):
TColStd_SequenceOfAsciiString(), TColStd_SequenceOfAsciiString(),
) )
reader.FileUnits(length, angle, solid_angle) 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)] 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 assert any("MM" in n.upper() or "MILLI" in n.upper() for n in names), names