Addresses aothms's review comment on PR #8759: "I don't understand
(or like...) why a convert_impl(const taxonomy::collection::ptr
collection, ...) overload is necessary... this doesn't sound like
something that every geometry kernel impl should handle by itself.
Rather something like a reduce on Result in the AbstractKernel
generic implementation that Concatenates the items internally
instead of aggregating them into a vector."
Removes OpenCascadeKernel::convert_impl(collection), the per-kernel
override that special-cased a homogeneous taxonomy::point3 collection
(e.g. a whole IfcCartesianPointList3D "point cloud") into a bulk fast
path. The batching now lives once, generically, in
AbstractKernel::convert_impl(collection): it still converts each
child individually through the kernel's own convert_impl(point3), but
folds the resulting shapes into a single result via a new generic
ConversionResultShape::concat_many() instead of aggregating one
ConversionResult per point into the vector. Any kernel that
implements convert_impl(point3) benefits automatically, with no
collection-level override of its own.
concat_many() is a bulk sibling of the existing pairwise concat():
combining N shapes via repeated pairwise concat() is either quadratic
in one direction (concat() classifies its receiver via
is_compound_of_faces(), which does a full sub-tree scan; calling it
on an ever-growing accumulator is O(n) per call) or produces an O(n)
deep nested shape in the other direction (still O(n) receiver
classification per call is avoided, but the resulting compound is
n levels deep, making the *later* TopExp_Explorer traversal during
triangulation O(n) per vertex on average, i.e. O(n^2) overall).
concat_many() gives kernels a way to combine everything in one O(n)
bulk operation instead. The default implementation (repeated
concat(), for kernels that never exercise this path) preserves
correctness; OpenCascadeShape::concat_many() overrides it with a
single BRep_Builder pass building one flat compound, matching the
original per-kernel fast path's performance exactly (verified by
benchmark: ~0.7-0.8 us/point from 10k-100k points, flat, matching
the original PR's own ~0.5-0.9 us/point claim).
Ported onto a clean v0.8.0 base (the original prototype was built on
top of Dion Moult's experimental ifcviewer-wgpu branch, PR #8759).
Fixes an id() access bug introduced while rewriting point.cpp for the
generic path: taxonomy::item::instance is a raw pointer on this base,
so both the new collection reduction here and the point3 conversion
in point.cpp need instance->as<IfcUtil::IfcBaseEntity>()->id(), the
same pattern already used throughout the rest of this file and every
other kernels/opencascade/*.cpp, not a direct instance->id()/.id()
call.
This contribution was produced with the assistance of an AI coding tool.
Exploratory proof of concept for #134, #1409 and #5218: IfcVertexPoint,
IfcCartesianPoint and IfcCartesianPointList3D used as top-level
representation items ("Vertex"/"Point"/"PointCloud") currently raise
"Failed to process shape" because AbstractKernel::convert_impl for
taxonomy::point3 is never implemented and point3 cannot appear as a
child of the generic items collection.
This makes point3/direction3 derive from geom_item instead of plain
item, so a lone point3 can stand in as a representation item, and adds:
- OpenCascadeKernel::convert_impl(point3): a single point becomes a
TopoDS_Vertex wrapped in a TopoDS_Compound, as aothms suggested in
#5218.
- OpenCascadeKernel::convert_impl(collection): a bulk fast path for a
collection made up entirely of point3 children (e.g. a whole
IfcCartesianPointList3D) that builds ONE compound with all vertices
in a single pass, instead of paying the generic per-item conversion
overhead (cache lookup, heap allocation, a separate Triangulate()
call) once per point.
- mapping for IfcVertexPoint (as a top-level item) and
IfcCartesianPointList3D.
- loose-vertex emission in OpenCascadeShape::Triangulate, since a
vertex-only shape previously triangulated to nothing.
This directly tests aothms's "the overhead is enormous" concern from
#5218. Benchmarked on this machine (Apple M-series, Release build):
- Normal (non-point) geometry is unaffected: a 5178-shape real model
processes in 4.48s before this change and 4.49s after (~0.3%, noise).
- The bulk fast path scales linearly and cheaply: ~0.5-0.9 us/point for
an IfcCartesianPointList3D from 1k to 100k points (100k points in
~93ms total).
- With the fast path disabled (pure per-item conversion, i.e. the naive
reading of "a TopoDS_Compound of TopoDS_Vertex" with no batching),
scaling is still linear, not quadratic, but ~4-7x slower per point
(~3.5-4 us/point at the same scale, 100k points in ~380ms).
So aothms's concern is real as a constant-factor tax from going through
full OCCT BRep objects (TopoDS_Vertex/Compound, shared_ptr taxonomy
nodes, per-item caching) rather than flat coordinate arrays, but it is
not the asymptotic blowup "enormous overhead" might suggest, and a
reasonably-scoped batching fast path narrows the gap substantially.
Given Bonsai already has a working, accepted Python-side bypass for
this (create_point_cloud_mesh / create_structural_point_connection_mesh
in bonsai/tool/loader.py and geometry.py), this is offered as a
proof of concept for evaluation, not a claim that it should override
the prior "something for 0.9" call.
IfcCartesianPointList2D ("PointCloud" in 2D) is intentionally out of
scope for this prototype.
Adds pytest coverage (no Catch2/C++ test harness exists in this
codebase) for all three representation types plus a 1000-point
round-trip/timing sanity check.
Generated with the assistance of an AI coding tool.
Petru Conduraru