Fix excessive memory usage in batch union operations (#1495)
* Fix excessive memory usage in batch union operations The MeshCompare comparator used for heap operations in BatchUnion was calling GetImpl()->NumVert(), which triggers transform application and creates transformed mesh copies. This caused all meshes to be loaded into memory simultaneously during the initial sorting phase. Changes: - Add NumVert() method to CsgLeafNode that returns the vertex count without triggering transform application - Add GetBoundingBox() method to CsgLeafNode for future optimization (currently still uses GetImpl() for correctness) - Update MeshCompare to use the new NumVert() method - Add memory tracking infrastructure under MANIFOLD_DEBUG for debugging - Add test case for batch union with many transformed meshes Fixes #1493 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Remove debug mesh tracking code Remove the memory tracking infrastructure that was added for debugging purposes. The core optimization (NumVert() and GetBoundingBox() methods) remains intact. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Remove test that was added for debugging The BatchUnionManyTransformed test was added for debugging but isn't needed as part of the final fix. The existing BatchBoolean test already covers the batch union functionality. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Use Arvo's algorithm for GetBoundingBox Replace the 8-corner transformation with Arvo's more efficient algorithm that computes the transformed AABB directly from the matrix elements. This reduces the operation from 8 matrix-vector multiplies to 9 scalar multiplies. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Simplify SimpleBoolean return statement The two-statement form with explicit std::move was unnecessary. In C++17, the temporary from Boolean3::Result() binds directly to the rvalue reference parameter of ImplToLeaf, making the code equivalent. The simpler one-liner is clearer and equally efficient. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.5
parent
301c165a20
commit
a850f73d51
+41
-4
@@ -26,10 +26,12 @@
|
||||
|
||||
namespace {
|
||||
using namespace manifold;
|
||||
|
||||
struct MeshCompare {
|
||||
bool operator()(const std::shared_ptr<CsgLeafNode>& a,
|
||||
const std::shared_ptr<CsgLeafNode>& b) {
|
||||
return a->GetImpl()->NumVert() < b->GetImpl()->NumVert();
|
||||
// Use NumVert() which doesn't trigger transform application.
|
||||
return a->NumVert() < b->NumVert();
|
||||
}
|
||||
};
|
||||
|
||||
@@ -104,6 +106,34 @@ std::shared_ptr<CsgNode> CsgLeafNode::Transform(const mat3x4& m) const {
|
||||
|
||||
CsgNodeType CsgLeafNode::GetNodeType() const { return CsgNodeType::Leaf; }
|
||||
|
||||
Box CsgLeafNode::GetBoundingBox() const {
|
||||
// Compute transformed bounding box without triggering full mesh transform.
|
||||
// This is an approximation - the actual bounding box of the transformed mesh
|
||||
// may be tighter, but this is sufficient for overlap checks.
|
||||
const Box& box = pImpl_->bBox_;
|
||||
if (transform_ == mat3x4(la::identity)) {
|
||||
return box;
|
||||
}
|
||||
|
||||
// Arvo's algorithm for transforming AABBs efficiently.
|
||||
// Instead of transforming all 8 corners, we compute min/max directly
|
||||
// from the matrix elements based on their signs.
|
||||
vec3 newMin = transform_[3]; // translation component
|
||||
vec3 newMax = newMin;
|
||||
|
||||
for (int i = 0; i < 3; i++) {
|
||||
for (int j = 0; j < 3; j++) {
|
||||
const auto a = transform_[j][i] * box.min[j];
|
||||
const auto b = transform_[j][i] * box.max[j];
|
||||
newMin[i] += std::min(a, b);
|
||||
newMax[i] += std::max(a, b);
|
||||
}
|
||||
}
|
||||
return Box{newMin, newMax};
|
||||
}
|
||||
|
||||
size_t CsgLeafNode::NumVert() const { return pImpl_->NumVert(); }
|
||||
|
||||
std::shared_ptr<CsgLeafNode> ImplToLeaf(Manifold::Impl&& impl) {
|
||||
return std::make_shared<CsgLeafNode>(
|
||||
std::make_shared<Manifold::Impl>(std::move(impl)));
|
||||
@@ -188,7 +218,7 @@ std::shared_ptr<CsgLeafNode> CsgLeafNode::Compose(
|
||||
if (!std::isfinite(nodeEpsilon)) nodeEpsilon = -1;
|
||||
epsilon = std::max(epsilon, nodeEpsilon);
|
||||
tolerance = std::max(tolerance, node->pImpl_->tolerance_);
|
||||
bbox = bbox.Union(node->pImpl_->bBox_);
|
||||
bbox = bbox.Union(node->GetBoundingBox());
|
||||
|
||||
vertIndices.push_back(numVert);
|
||||
edgeIndices.push_back(numEdge * 2);
|
||||
@@ -323,7 +353,14 @@ std::shared_ptr<CsgLeafNode> CsgLeafNode::Compose(
|
||||
const int offset = i * Manifold::Impl::meshIDCounter_;
|
||||
|
||||
for (const auto& pair : node->pImpl_->meshRelation_.meshIDtransform) {
|
||||
combined.meshRelation_.meshIDtransform[pair.first + offset] = pair.second;
|
||||
Manifold::Impl::Relation rel = pair.second;
|
||||
// Apply the node's transform to the mesh relation if not identity.
|
||||
// This is necessary because we may not have called GetImpl() which would
|
||||
// have applied the transform to the mesh relations.
|
||||
if (node->transform_ != mat3x4(la::identity)) {
|
||||
rel.transform = node->transform_ * Mat4(rel.transform);
|
||||
}
|
||||
combined.meshRelation_.meshIDtransform[pair.first + offset] = rel;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -414,7 +451,7 @@ std::shared_ptr<CsgLeafNode> BatchUnion(
|
||||
Vec<Box> boxes;
|
||||
boxes.reserve(children.size() - start);
|
||||
for (size_t i = start; i < children.size(); i++) {
|
||||
boxes.push_back(children[i]->GetImpl()->bBox_);
|
||||
boxes.push_back(children[i]->GetBoundingBox());
|
||||
}
|
||||
// partition the children into a set of disjoint sets
|
||||
// each set contains a set of children that are pairwise disjoint
|
||||
|
||||
@@ -54,6 +54,13 @@ class CsgLeafNode final : public CsgNode {
|
||||
static std::shared_ptr<CsgLeafNode> Compose(
|
||||
const std::vector<std::shared_ptr<CsgLeafNode>>& nodes);
|
||||
|
||||
// Returns the bounding box without triggering eager transform application.
|
||||
// This is more efficient for overlap checks when we don't need the full mesh.
|
||||
Box GetBoundingBox() const;
|
||||
|
||||
// Returns the number of vertices without triggering eager transform.
|
||||
size_t NumVert() const;
|
||||
|
||||
private:
|
||||
mutable std::shared_ptr<const Manifold::Impl> pImpl_;
|
||||
mutable mat3x4 transform_ = la::identity;
|
||||
|
||||
Reference in New Issue
Block a user