diff --git a/src/ifcgeom/AbstractKernel.cpp b/src/ifcgeom/AbstractKernel.cpp index 2031197ee3..932eda9f5e 100644 --- a/src/ifcgeom/AbstractKernel.cpp +++ b/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. + 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 shapes; + shapes.reserve(child_results.size()); + for (auto& t : child_results) { + shapes.push_back(t.Shape().get()); + } + + auto* first = shapes.front(); + std::vector rest(shapes.begin() + 1, shapes.end()); + auto* accum = first->concat_many(rest); + + r.emplace_back(IfcGeom::ConversionResult(collection->instance->as()->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) { diff --git a/src/ifcgeom/ConversionResult.h b/src/ifcgeom/ConversionResult.h index 773c296c5f..a7ee161f27 100644 --- a/src/ifcgeom/ConversionResult.h +++ b/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& 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>& from, const std::vector>& to) = 0; virtual ConversionResultShape* moved(ifcopenshell::geometry::taxonomy::matrix4::ptr) const = 0; diff --git a/src/ifcgeom/kernels/opencascade/OpenCascadeConversionResult.cpp b/src/ifcgeom/kernels/opencascade/OpenCascadeConversionResult.cpp index 9d289e483d..ccb75e47a1 100644 --- a/src/ifcgeom/kernels/opencascade/OpenCascadeConversionResult.cpp +++ b/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& 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(other)->shape_); + } + return new OpenCascadeShape(std::move(compound)); +} + std::pair, OpaqueCoordinate<3>> ifcopenshell::geometry::OpenCascadeShape::bounding_box() const { throw std::runtime_error("Not implemented"); diff --git a/src/ifcgeom/kernels/opencascade/OpenCascadeConversionResult.h b/src/ifcgeom/kernels/opencascade/OpenCascadeConversionResult.h index 0dcd33c6a8..d7b0cb4adf 100644 --- a/src/ifcgeom/kernels/opencascade/OpenCascadeConversionResult.h +++ b/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& others); virtual std::size_t map(OpaqueCoordinate<4>& from, OpaqueCoordinate<4>& to); virtual std::size_t map(const std::vector>& from, const std::vector>& to); diff --git a/src/ifcgeom/kernels/opencascade/OpenCascadeKernel.h b/src/ifcgeom/kernels/opencascade/OpenCascadeKernel.h index b5f70b70ad..61fe4987b6 100644 --- a/src/ifcgeom/kernels/opencascade/OpenCascadeKernel.h +++ b/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>& openings, const IfcGeom::ConversionResults& entity_shapes, const ifcopenshell::geometry::taxonomy::matrix4& entity_trsf, IfcGeom::ConversionResults& cut_shapes); diff --git a/src/ifcgeom/kernels/opencascade/point.cpp b/src/ifcgeom/kernels/opencascade/point.cpp index 645bf204a2..8856bc9ee6 100644 --- a/src/ifcgeom/kernels/opencascade/point.cpp +++ b/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 points; - points.reserve(collection->children.size()); - for (auto& c : collection->children) { - points.push_back(convert_xyz(*std::static_pointer_cast(c))); - } - - auto compound = make_vertex_compound(points); - - auto s = results.size(); - results.emplace_back(ConversionResult( - collection->instance->as()->id(), - new OpenCascadeShape(compound), - collection->surface_style - )); - if (collection->matrix) { - results[s].prepend(collection->matrix); - } - return true; -}