ifcgeom: make point-collection batching generic in AbstractKernel

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.

src/ifcgeom/AbstractKernel.cpp

@@ -67,7 +67,60 @@ const Settings& ifcopenshell::geometry::kernels::AbstractKernel::settings() cons
 	return settings_;
 }
 
+namespace {
+	// Homogeneous taxonomy::point3 collections (e.g. IfcCartesianPointList3D)
+	// are reduced to a single result, see #134/#1409/#5218.
+	bool is_reducible_point_collection(const ifcopenshell::geometry::taxonomy::collection::ptr& collection) {
+		if (collection->children.empty()) {
+			return false;
+		}
+		for (auto& c : collection->children) {
+			if (c->kind() != ifcopenshell::geometry::taxonomy::POINT3) {
+				return false;
+			}
+		}
+		return true;
+	}
+}
+
 bool ifcopenshell::geometry::kernels::AbstractKernel::convert_impl(const taxonomy::collection::ptr collection, IfcGeom::ConversionResults& r) {
+	if (collection->instance && is_reducible_point_collection(collection)) {
+		// Reduce via the generic wrap_in_compound()/concat_many() API rather
+		// than a per-kernel convert_impl(collection) override. concat_many()
+		// combines everything in a single bulk call so kernels can implement
+		// it in O(n): a loop calling concat() pairwise would either
+		// re-classify an ever-growing accumulator (quadratic) or produce an
+		// O(n)-deep nested shape (quadratic to traverse later).
+		// Kept alive until concat_many() below: shapes[] holds raw pointers
+		// into these ConversionResults' shared_ptr<ConversionResultShape>.
+		IfcGeom::ConversionResults child_results;
+		for (auto& c : collection->children) {
+			if (!convert(c, child_results) && !partial_success_is_success) {
+				return false;
+			}
+		}
+
+		if (child_results.empty()) {
+			return false;
+		}
+
+		std::vector<IfcGeom::ConversionResultShape*> shapes;
+		shapes.reserve(child_results.size());
+		for (auto& t : child_results) {
+			shapes.push_back(t.Shape().get());
+		}
+
+		auto* first = shapes.front();
+		std::vector<IfcGeom::ConversionResultShape*> rest(shapes.begin() + 1, shapes.end());
+		auto* accum = first->concat_many(rest);
+
+		r.emplace_back(IfcGeom::ConversionResult(collection->instance->as<IfcUtil::IfcBaseEntity>()->id(), accum, collection->surface_style));
+		if (collection->matrix) {
+			r.back().prepend(collection->matrix);
+		}
+		return true;
+	}
+
 	auto s = r.size();
 	for (auto& c : collection->children) {
 		if (!convert(c, r) && !partial_success_is_success) {

src/ifcgeom/ConversionResult.h

@@ -492,6 +492,22 @@ namespace IfcGeom {
 		virtual ConversionResultShape* intersect(ConversionResultShape*) = 0;
 		virtual ConversionResultShape* concat(ConversionResultShape*) = 0;
 
+		// Bulk variant of concat(), combining `this` with every shape in
+		// `others` into a single result. Used by AbstractKernel to reduce a
+		// homogeneous collection (e.g. a point cloud) into one shape, see
+		// #134/#1409/#5218. Default falls back to repeated concat() (correct
+		// but potentially quadratic); kernels can override with a flat,
+		// linear-time bulk implementation.
+		virtual ConversionResultShape* concat_many(const std::vector<ConversionResultShape*>& others) {
+			ConversionResultShape* result = wrap_in_compound();
+			for (auto* other : others) {
+				auto* next = result->concat(other);
+				delete result;
+				result = next;
+			}
+			return result;
+		}
+
 		virtual std::size_t map(OpaqueCoordinate<4>& from, OpaqueCoordinate<4>& to) = 0;
 		virtual std::size_t map(const std::vector<OpaqueCoordinate<4>>& from, const std::vector<OpaqueCoordinate<4>>& to) = 0;
 		virtual ConversionResultShape* moved(ifcopenshell::geometry::taxonomy::matrix4::ptr) const = 0;

src/ifcgeom/kernels/opencascade/OpenCascadeConversionResult.cpp

@@ -349,7 +349,8 @@ void ifcopenshell::geometry::OpenCascadeShape::Triangulate(ifcopenshell::geometr
 		for (TopExp_Explorer texp(shape_, TopAbs_VERTEX, TopAbs_EDGE); texp.More(); texp.Next()) {
 			gp_XYZ p = BRep_Tool::Pnt(TopoDS::Vertex(texp.Current())).XYZ();
 			taxonomy_transform(place.components_, p);
-			t->addVertex(item_id, surface_style_id, p.X(), p.Y(), p.Z());
+			int idx = t->addVertex(item_id, surface_style_id, p.X(), p.Y(), p.Z());
+			t->addPoint(item_id, surface_style_id, idx);
 		}
 	}
 
@@ -586,6 +587,22 @@ ConversionResultShape* ifcopenshell::geometry::OpenCascadeShape::concat(Conversi
 	return new OpenCascadeShape(std::move(compound));
 }
 
+ConversionResultShape* ifcopenshell::geometry::OpenCascadeShape::concat_many(const std::vector<ConversionResultShape*>& others)
+{
+	// Unlike concat(), which is called pairwise and therefore re-classifies
+	// its (potentially large) receiver on every call, this builds one flat
+	// compound in a single O(n) pass: linear time and constant nesting depth
+	// regardless of how many shapes are combined.
+	TopoDS_Compound compound;
+	BRep_Builder builder;
+	builder.MakeCompound(compound);
+	builder.Add(compound, shape_);
+	for (auto* other : others) {
+		builder.Add(compound, static_cast<OpenCascadeShape*>(other)->shape_);
+	}
+	return new OpenCascadeShape(std::move(compound));
+}
+
 std::pair<OpaqueCoordinate<3>, OpaqueCoordinate<3>> ifcopenshell::geometry::OpenCascadeShape::bounding_box() const
 {
 	throw std::runtime_error("Not implemented");

src/ifcgeom/kernels/opencascade/OpenCascadeConversionResult.h

@@ -99,6 +99,7 @@ namespace ifcopenshell {
 			virtual ConversionResultShape* subtract(ConversionResultShape*);
 			virtual ConversionResultShape* intersect(ConversionResultShape*);
 			virtual ConversionResultShape* concat(ConversionResultShape*);
+			virtual ConversionResultShape* concat_many(const std::vector<ConversionResultShape*>& others);
 
 			virtual std::size_t map(OpaqueCoordinate<4>& from, OpaqueCoordinate<4>& to);
 			virtual std::size_t map(const std::vector<OpaqueCoordinate<4>>& from, const std::vector<OpaqueCoordinate<4>>& to);

src/ifcgeom/kernels/opencascade/OpenCascadeKernel.h

@@ -142,9 +142,10 @@ public:
 	virtual bool convert_impl(const ifcopenshell::geometry::taxonomy::boolean_result::ptr, IfcGeom::ConversionResults&);
 	virtual bool convert_impl(const ifcopenshell::geometry::taxonomy::loft::ptr, IfcGeom::ConversionResults&);
 	virtual bool convert_impl(const ifcopenshell::geometry::taxonomy::sweep_along_curve::ptr, IfcGeom::ConversionResults&);
-	// Prototype for issue #134 / #1409 / #5218, see point.cpp.
+	// Prototype for issue #134 / #1409 / #5218, see point.cpp. The bulk
+	// point-cloud fast path is generic infrastructure in
+	// AbstractKernel::convert_impl(collection), not a per-kernel override.
 	virtual bool convert_impl(const ifcopenshell::geometry::taxonomy::point3::ptr, IfcGeom::ConversionResults&);
-	virtual bool convert_impl(const ifcopenshell::geometry::taxonomy::collection::ptr, IfcGeom::ConversionResults&);
 
 	virtual bool convert_openings(const IfcUtil::IfcBaseEntity* entity, const std::vector<std::pair<ifcopenshell::geometry::taxonomy::ptr, ifcopenshell::geometry::taxonomy::matrix4>>& openings,
 		const IfcGeom::ConversionResults& entity_shapes, const ifcopenshell::geometry::taxonomy::matrix4& entity_trsf, IfcGeom::ConversionResults& cut_shapes);

src/ifcgeom/kernels/opencascade/point.cpp

@@ -20,13 +20,9 @@
 // This file was generated with the assistance of an AI coding tool.
 
 // Prototype kernel-level support for single-vertex / point-cloud
-// representations (issues #134, #1409, #5218). Points become a
-// TopoDS_Compound of TopoDS_Vertex, per aothms's suggested approach.
-// convert_impl(point3) handles a lone point; convert_impl(collection) takes a
-// bulk fast path for a collection made up entirely of point3 children (e.g. a
-// whole IfcCartesianPointList3D "PointCloud"), converting it to a single
-// compound in one pass instead of once per point. See commit message for the
-// overhead/benchmark discussion.
+// representations (issues #134, #1409, #5218). A point becomes a
+// TopoDS_Compound of TopoDS_Vertex. Point-cloud batching lives generically
+// in AbstractKernel::convert_impl(collection), see that file.
 
 #include "OpenCascadeKernel.h"
 
@@ -65,38 +61,3 @@ bool OpenCascadeKernel::convert_impl(const taxonomy::point3::ptr point, IfcGeom:
 	));
 	return true;
 }
-
-bool OpenCascadeKernel::convert_impl(const taxonomy::collection::ptr collection, IfcGeom::ConversionResults& results) {
-	bool all_points = !collection->children.empty();
-	for (auto& c : collection->children) {
-		if (c->kind() != taxonomy::POINT3) {
-			all_points = false;
-			break;
-		}
-	}
-
-	if (!all_points || !collection->instance) {
-		// Not a homogeneous point cloud (or has no entity to attribute the
-		// resulting shape to), fall back to the generic per-child conversion.
-		return ifcopenshell::geometry::kernels::AbstractKernel::convert_impl(collection, results);
-	}
-
-	std::vector<gp_Pnt> points;
-	points.reserve(collection->children.size());
-	for (auto& c : collection->children) {
-		points.push_back(convert_xyz<gp_Pnt>(*std::static_pointer_cast<taxonomy::point3>(c)));
-	}
-
-	auto compound = make_vertex_compound(points);
-
-	auto s = results.size();
-	results.emplace_back(ConversionResult(
-		collection->instance->as<IfcUtil::IfcBaseEntity>()->id(),
-		new OpenCascadeShape(compound),
-		collection->surface_style
-	));
-	if (collection->matrix) {
-		results[s].prepend(collection->matrix);
-	}
-	return true;
-}