From a43e337a213cd052528bbc7fe8fa18d1333cc9d0 Mon Sep 17 00:00:00 2001 From: ochafik Date: Tue, 14 Mar 2023 02:42:28 +0000 Subject: [PATCH 01/15] Flatten the CsgOpNodes as we build them, rather than in GetChildren --- extras/CMakeLists.txt | 8 ++-- extras/large_scene_test.cpp | 58 ++++++++++++++++++++++++++ src/manifold/src/csg_tree.cpp | 76 ++++++++++++++++++++++++----------- src/manifold/src/csg_tree.h | 9 ++++- src/manifold/src/manifold.cpp | 3 +- 5 files changed, 125 insertions(+), 29 deletions(-) create mode 100644 extras/large_scene_test.cpp diff --git a/extras/CMakeLists.txt b/extras/CMakeLists.txt index e03c7403b..5944b0a96 100644 --- a/extras/CMakeLists.txt +++ b/extras/CMakeLists.txt @@ -16,18 +16,20 @@ project(extras) add_executable(perfTest perf_test.cpp) target_link_libraries(perfTest manifold) - target_compile_options(perfTest PRIVATE ${MANIFOLD_FLAGS}) target_compile_features(perfTest PUBLIC cxx_std_17) +add_executable(largeSceneTest large_scene_test.cpp) +target_link_libraries(largeSceneTest manifold) +target_compile_options(largeSceneTest PRIVATE ${MANIFOLD_FLAGS}) +target_compile_features(largeSceneTest PUBLIC cxx_std_17) + if(BUILD_TEST_CGAL) add_executable(perfTestCGAL perf_test_cgal.cpp) find_package(CGAL REQUIRED COMPONENTS Core) target_compile_definitions(perfTestCGAL PRIVATE CGAL_USE_GMPXX) - # target_compile_definitions(perfTestCGAL PRIVATE CGAL_DEBUG) target_link_libraries(perfTestCGAL manifold CGAL::CGAL CGAL::CGAL_Core) - target_compile_options(perfTestCGAL PRIVATE ${MANIFOLD_FLAGS}) target_compile_features(perfTestCGAL PUBLIC cxx_std_17) endif() diff --git a/extras/large_scene_test.cpp b/extras/large_scene_test.cpp new file mode 100644 index 000000000..7d1425840 --- /dev/null +++ b/extras/large_scene_test.cpp @@ -0,0 +1,58 @@ +// Copyright 2020 The Manifold Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include + +#include "manifold.h" + +using namespace manifold; + +/* + Build & execute with the following command: + + ( mkdir -p build && cd build && \ + cmake -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=ON \ + -DMANIFOLD_PAR=TBB \ + -DTHRUST_MULTICONFIG_ENABLE_SYSTEM_TBB=1 \ + -DTHRUST_HOST_SYSTEM=THRUST_HOST_SYSTEM_TBB \ + .. && \ + make -j && \ + time ./extras/largeSceneTest 50 ) +*/ +int main(int argc, char **argv) { + int n = 20; // Crashes at n=50 + if (argc == 2) n = atoi(argv[1]); + + std::cout << "n = " << n << std::endl; + + auto start = std::chrono::high_resolution_clock::now(); + Manifold scene; + + for (int i = 0; i < n; ++i) { + for (int j = 0; j < n; ++j) { + for (int k = 0; k < n; ++k) { + if (i == 0 && j == 0 && k == 0) continue; + + Manifold sphere = Manifold::Sphere(1).Translate(glm::vec3(i, j, k)); + scene = scene.Boolean(sphere, OpType::Add); + } + } + } + scene.NumTri(); + auto end = std::chrono::high_resolution_clock::now(); + std::chrono::duration elapsed = end - start; + std::cout << "nTri = " << scene.NumTri() << ", time = " << elapsed.count() + << " sec" << std::endl; +} diff --git a/src/manifold/src/csg_tree.cpp b/src/manifold/src/csg_tree.cpp index 8884189e8..f3663efe5 100644 --- a/src/manifold/src/csg_tree.cpp +++ b/src/manifold/src/csg_tree.cpp @@ -73,6 +73,20 @@ struct CheckOverlap { } // namespace namespace manifold { +std::shared_ptr CsgNode::Boolean(std::shared_ptr second, + OpType op) { + if (auto opNode = std::dynamic_pointer_cast(second)) { + // "this" is not a CsgOpNode (which overrides Boolean), but if "second" is + // and the operation is commutative, we let it built the tree. + if ((op == OpType::Add || op == OpType::Intersect)) { + return opNode->Boolean(shared_from_this(), op); + } + } + std::vector> children( + {shared_from_this(), second->shared_from_this()}); + return std::make_shared(children, op); +} + std::shared_ptr CsgNode::Translate(const glm::vec3 &t) const { glm::mat4x3 transform(1.0f); transform[3] += t; @@ -237,8 +251,27 @@ CsgOpNode::CsgOpNode(std::vector> &&children, auto impl = impl_.GetGuard(); impl->children_ = children; SetOp(op); - // opportunistically flatten the tree without costly evaluation - GetChildren(false); +} + +std::shared_ptr CsgOpNode::Boolean(std::shared_ptr second, + OpType op) { + std::vector> children; + + auto handleOperand = [&](const std::shared_ptr &operand) { + if (auto opNode = std::dynamic_pointer_cast(operand)) { + if (opNode->IsOp(op)) { + for (auto &child : opNode->GetChildren(/* finalize= */ false)) { + children.push_back(child); + } + return; + } + } + children.push_back(operand); + }; + handleOperand(shared_from_this()); + handleOperand(second); + + return std::make_shared(children, op); } std::shared_ptr CsgOpNode::Transform(const glm::mat4x3 &m) const { @@ -423,30 +456,14 @@ std::vector> &CsgOpNode::GetChildren( return children_; impl->simplified_ = true; impl->flattened_ = finalize; - std::vector> newChildren; - - CsgNodeType op = op_; - for (auto &child : children_) { - if (child->GetNodeType() == op && child.use_count() == 1 && - std::dynamic_pointer_cast(child)->impl_.UseCount() == 1) { - auto grandchildren = - std::dynamic_pointer_cast(child)->GetChildren(finalize); - int start = children_.size(); - for (auto &grandchild : grandchildren) { - newChildren.push_back(grandchild->Transform(child->GetTransform())); - } - } else { - if (!finalize || child->GetNodeType() == CsgNodeType::Leaf) { - newChildren.push_back(child); - } else { - newChildren.push_back(child->ToLeafNode()); + + if (finalize) { + for (auto &child : children_) { + if (child->GetNodeType() != CsgNodeType::Leaf) { + child = child->ToLeafNode(); } } - // special handling for difference: we treat it as first - (second + third + - // ...) so op = Union after the first node - if (op == CsgNodeType::Difference) op = CsgNodeType::Union; } - children_ = newChildren; return children_; } @@ -464,6 +481,19 @@ void CsgOpNode::SetOp(OpType op) { } } +bool CsgOpNode::IsOp(OpType op) { + switch (op) { + case OpType::Add: + return op_ == CsgNodeType::Union; + case OpType::Subtract: + return op_ == CsgNodeType::Difference; + case OpType::Intersect: + return op_ == CsgNodeType::Intersection; + default: + return false; + } +} + glm::mat4x3 CsgOpNode::GetTransform() const { return transform_; } } // namespace manifold diff --git a/src/manifold/src/csg_tree.h b/src/manifold/src/csg_tree.h index bc4364117..040746fd6 100644 --- a/src/manifold/src/csg_tree.h +++ b/src/manifold/src/csg_tree.h @@ -22,13 +22,16 @@ enum class CsgNodeType { Union, Intersection, Difference, Leaf }; class CsgLeafNode; -class CsgNode { +class CsgNode : public std::enable_shared_from_this { public: virtual std::shared_ptr ToLeafNode() const = 0; virtual std::shared_ptr Transform(const glm::mat4x3 &m) const = 0; virtual CsgNodeType GetNodeType() const = 0; virtual glm::mat4x3 GetTransform() const = 0; + virtual std::shared_ptr Boolean(std::shared_ptr second, + OpType op); + std::shared_ptr Translate(const glm::vec3 &t) const; std::shared_ptr Scale(const glm::vec3 &s) const; std::shared_ptr Rotate(float xDegrees = 0, float yDegrees = 0, @@ -68,6 +71,9 @@ class CsgOpNode final : public CsgNode { CsgOpNode(std::vector> &&children, OpType op); + std::shared_ptr Boolean(std::shared_ptr second, + OpType op) override; + std::shared_ptr Transform(const glm::mat4x3 &m) const override; std::shared_ptr ToLeafNode() const override; @@ -89,6 +95,7 @@ class CsgOpNode final : public CsgNode { mutable std::shared_ptr cache_ = nullptr; void SetOp(OpType); + bool IsOp(OpType op); static std::shared_ptr BatchBoolean( OpType operation, diff --git a/src/manifold/src/manifold.cpp b/src/manifold/src/manifold.cpp index f80c94c5c..6461cae09 100644 --- a/src/manifold/src/manifold.cpp +++ b/src/manifold/src/manifold.cpp @@ -585,8 +585,7 @@ Manifold Manifold::Refine(int n) const { * @param op The type of operation to perform. */ Manifold Manifold::Boolean(const Manifold& second, OpType op) const { - std::vector> children({pNode_, second.pNode_}); - return Manifold(std::make_shared(children, op)); + return Manifold(pNode_->Boolean(second.pNode_, op)); } Manifold Manifold::BatchBoolean(const std::vector& manifolds, From f0914797f4536f71a100cc72aac8e0c89ef4f78a Mon Sep 17 00:00:00 2001 From: ochafik Date: Tue, 14 Mar 2023 03:20:22 +0000 Subject: [PATCH 02/15] Don't inline reused nodes in CsgNode::Boolean --- src/manifold/src/csg_tree.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/manifold/src/csg_tree.cpp b/src/manifold/src/csg_tree.cpp index f3663efe5..efb90be13 100644 --- a/src/manifold/src/csg_tree.cpp +++ b/src/manifold/src/csg_tree.cpp @@ -259,7 +259,8 @@ std::shared_ptr CsgOpNode::Boolean(std::shared_ptr second, auto handleOperand = [&](const std::shared_ptr &operand) { if (auto opNode = std::dynamic_pointer_cast(operand)) { - if (opNode->IsOp(op)) { + if (opNode->IsOp(op) && opNode.use_count() == 1 && + opNode->impl_.UseCount() == 1) { for (auto &child : opNode->GetChildren(/* finalize= */ false)) { children.push_back(child); } From 210bb0ee3f57690b5ea97f22dfc1b5ad2b95d269 Mon Sep 17 00:00:00 2001 From: ochafik Date: Tue, 14 Mar 2023 03:20:31 +0000 Subject: [PATCH 03/15] Reinstate difference -> union rewrite --- src/manifold/src/csg_tree.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/manifold/src/csg_tree.cpp b/src/manifold/src/csg_tree.cpp index efb90be13..a189ca204 100644 --- a/src/manifold/src/csg_tree.cpp +++ b/src/manifold/src/csg_tree.cpp @@ -272,6 +272,14 @@ std::shared_ptr CsgOpNode::Boolean(std::shared_ptr second, handleOperand(shared_from_this()); handleOperand(second); + if (op == OpType::Subtract && children.size() > 2) { + // special handling for difference: we treat it as first - (second + third + + // ...) so op = Union after the first node + std::vector> unionChildren(children.begin() + 1, + children.end()); + children.resize(1); + children.push_back(std::make_shared(unionChildren, OpType::Add)); + } return std::make_shared(children, op); } From 7b488a55f25f1d6d4335c17154f8d034c07453de Mon Sep 17 00:00:00 2001 From: ochafik Date: Tue, 14 Mar 2023 03:20:50 +0000 Subject: [PATCH 04/15] No need for opportunistic flattening anymore --- src/manifold/src/csg_tree.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/manifold/src/csg_tree.cpp b/src/manifold/src/csg_tree.cpp index a189ca204..a048958d6 100644 --- a/src/manifold/src/csg_tree.cpp +++ b/src/manifold/src/csg_tree.cpp @@ -241,8 +241,6 @@ CsgOpNode::CsgOpNode(const std::vector> &children, auto impl = impl_.GetGuard(); impl->children_ = children; SetOp(op); - // opportunistically flatten the tree without costly evaluation - GetChildren(false); } CsgOpNode::CsgOpNode(std::vector> &&children, From 166c60353ba34d977420ca1472c0b7014a3c009e Mon Sep 17 00:00:00 2001 From: ochafik Date: Tue, 14 Mar 2023 03:25:08 +0000 Subject: [PATCH 05/15] Fix typo --- src/manifold/src/csg_tree.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/manifold/src/csg_tree.cpp b/src/manifold/src/csg_tree.cpp index a048958d6..338d2a77c 100644 --- a/src/manifold/src/csg_tree.cpp +++ b/src/manifold/src/csg_tree.cpp @@ -82,8 +82,7 @@ std::shared_ptr CsgNode::Boolean(std::shared_ptr second, return opNode->Boolean(shared_from_this(), op); } } - std::vector> children( - {shared_from_this(), second->shared_from_this()}); + std::vector> children({shared_from_this(), second}); return std::make_shared(children, op); } From 5f91aa946893444051b6c4a48e2e73b4e8c5fc89 Mon Sep 17 00:00:00 2001 From: ochafik Date: Tue, 14 Mar 2023 03:28:37 +0000 Subject: [PATCH 06/15] Remove CsgOpNode::Impl::simplified_ --- src/manifold/src/csg_tree.cpp | 4 +--- src/manifold/src/csg_tree.h | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/manifold/src/csg_tree.cpp b/src/manifold/src/csg_tree.cpp index 338d2a77c..750ac32d2 100644 --- a/src/manifold/src/csg_tree.cpp +++ b/src/manifold/src/csg_tree.cpp @@ -458,9 +458,7 @@ std::vector> &CsgOpNode::GetChildren( bool finalize) const { auto impl = impl_.GetGuard(); auto &children_ = impl->children_; - if (children_.empty() || (impl->simplified_ && !finalize) || impl->flattened_) - return children_; - impl->simplified_ = true; + if (children_.empty() || impl->flattened_) return children_; impl->flattened_ = finalize; if (finalize) { diff --git a/src/manifold/src/csg_tree.h b/src/manifold/src/csg_tree.h index 040746fd6..e397ca802 100644 --- a/src/manifold/src/csg_tree.h +++ b/src/manifold/src/csg_tree.h @@ -85,7 +85,6 @@ class CsgOpNode final : public CsgNode { private: struct Impl { std::vector> children_; - bool simplified_ = false; bool flattened_ = false; }; mutable ConcurrentSharedPtr impl_ = ConcurrentSharedPtr(Impl{}); From 17a13d687dc6b1758dc9b45cc09973c20469a394 Mon Sep 17 00:00:00 2001 From: ochafik Date: Tue, 14 Mar 2023 04:31:58 +0000 Subject: [PATCH 07/15] Don't overskip referenced nodes in simplification --- src/manifold/src/csg_tree.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/manifold/src/csg_tree.cpp b/src/manifold/src/csg_tree.cpp index 750ac32d2..4b9422c77 100644 --- a/src/manifold/src/csg_tree.cpp +++ b/src/manifold/src/csg_tree.cpp @@ -256,8 +256,7 @@ std::shared_ptr CsgOpNode::Boolean(std::shared_ptr second, auto handleOperand = [&](const std::shared_ptr &operand) { if (auto opNode = std::dynamic_pointer_cast(operand)) { - if (opNode->IsOp(op) && opNode.use_count() == 1 && - opNode->impl_.UseCount() == 1) { + if (opNode->IsOp(op) && opNode->impl_.UseCount() == 1) { for (auto &child : opNode->GetChildren(/* finalize= */ false)) { children.push_back(child); } From 3bf059c0c48bbbfd1d3c6cd323aa3e0828258ebb Mon Sep 17 00:00:00 2001 From: ochafik Date: Tue, 14 Mar 2023 04:49:42 +0000 Subject: [PATCH 08/15] Avoid needless copy of shared pointers --- src/manifold/src/csg_tree.cpp | 8 ++++---- src/manifold/src/csg_tree.h | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/manifold/src/csg_tree.cpp b/src/manifold/src/csg_tree.cpp index 4b9422c77..b66aebe37 100644 --- a/src/manifold/src/csg_tree.cpp +++ b/src/manifold/src/csg_tree.cpp @@ -73,8 +73,8 @@ struct CheckOverlap { } // namespace namespace manifold { -std::shared_ptr CsgNode::Boolean(std::shared_ptr second, - OpType op) { +std::shared_ptr CsgNode::Boolean( + const std::shared_ptr &second, OpType op) { if (auto opNode = std::dynamic_pointer_cast(second)) { // "this" is not a CsgOpNode (which overrides Boolean), but if "second" is // and the operation is commutative, we let it built the tree. @@ -250,8 +250,8 @@ CsgOpNode::CsgOpNode(std::vector> &&children, SetOp(op); } -std::shared_ptr CsgOpNode::Boolean(std::shared_ptr second, - OpType op) { +std::shared_ptr CsgOpNode::Boolean( + const std::shared_ptr &second, OpType op) { std::vector> children; auto handleOperand = [&](const std::shared_ptr &operand) { diff --git a/src/manifold/src/csg_tree.h b/src/manifold/src/csg_tree.h index e397ca802..e6a913492 100644 --- a/src/manifold/src/csg_tree.h +++ b/src/manifold/src/csg_tree.h @@ -29,8 +29,8 @@ class CsgNode : public std::enable_shared_from_this { virtual CsgNodeType GetNodeType() const = 0; virtual glm::mat4x3 GetTransform() const = 0; - virtual std::shared_ptr Boolean(std::shared_ptr second, - OpType op); + virtual std::shared_ptr Boolean( + const std::shared_ptr &second, OpType op); std::shared_ptr Translate(const glm::vec3 &t) const; std::shared_ptr Scale(const glm::vec3 &s) const; @@ -71,7 +71,7 @@ class CsgOpNode final : public CsgNode { CsgOpNode(std::vector> &&children, OpType op); - std::shared_ptr Boolean(std::shared_ptr second, + std::shared_ptr Boolean(const std::shared_ptr &second, OpType op) override; std::shared_ptr Transform(const glm::mat4x3 &m) const override; From ce85a44e387c4fc495c2889ad1c95588ba217daf Mon Sep 17 00:00:00 2001 From: ochafik Date: Tue, 14 Mar 2023 12:45:18 +0000 Subject: [PATCH 09/15] Disambiguate CsgOpNode::Impl::flatten_ -> forcedToLeafNodes_ and align GetChildren arg naming --- extras/large_scene_test.cpp | 2 +- src/manifold/src/csg_tree.cpp | 18 +++++++++--------- src/manifold/src/csg_tree.h | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/extras/large_scene_test.cpp b/extras/large_scene_test.cpp index 7d1425840..8557cee47 100644 --- a/extras/large_scene_test.cpp +++ b/extras/large_scene_test.cpp @@ -32,7 +32,7 @@ using namespace manifold; time ./extras/largeSceneTest 50 ) */ int main(int argc, char **argv) { - int n = 20; // Crashes at n=50 + int n = 20; if (argc == 2) n = atoi(argv[1]); std::cout << "n = " << n << std::endl; diff --git a/src/manifold/src/csg_tree.cpp b/src/manifold/src/csg_tree.cpp index b66aebe37..d057ce180 100644 --- a/src/manifold/src/csg_tree.cpp +++ b/src/manifold/src/csg_tree.cpp @@ -257,7 +257,7 @@ std::shared_ptr CsgOpNode::Boolean( auto handleOperand = [&](const std::shared_ptr &operand) { if (auto opNode = std::dynamic_pointer_cast(operand)) { if (opNode->IsOp(op) && opNode->impl_.UseCount() == 1) { - for (auto &child : opNode->GetChildren(/* finalize= */ false)) { + for (auto &child : opNode->GetChildren(/* forceToLeafNodes= */ false)) { children.push_back(child); } return; @@ -448,19 +448,19 @@ void CsgOpNode::BatchUnion() const { /** * Flatten the children to a list of leaf nodes and return them. - * If finalize is true, the list will be guaranteed to be a list of leaf nodes - * (i.e. no ops). Otherwise, the list may contain ops. - * Note that this function will not apply the transform to children, as they may - * be shared with other nodes. + * If forceToLeafNodes is true, the list will be guaranteed to be a list of leaf + * nodes (i.e. no ops). Otherwise, the list may contain ops. Note that this + * function will not apply the transform to children, as they may be shared with + * other nodes. */ std::vector> &CsgOpNode::GetChildren( - bool finalize) const { + bool forceToLeafNodes) const { auto impl = impl_.GetGuard(); auto &children_ = impl->children_; - if (children_.empty() || impl->flattened_) return children_; - impl->flattened_ = finalize; + if (children_.empty() || impl->forcedToLeafNodes_) return children_; + impl->forcedToLeafNodes_ = forceToLeafNodes; - if (finalize) { + if (forceToLeafNodes) { for (auto &child : children_) { if (child->GetNodeType() != CsgNodeType::Leaf) { child = child->ToLeafNode(); diff --git a/src/manifold/src/csg_tree.h b/src/manifold/src/csg_tree.h index e6a913492..5acbd25fa 100644 --- a/src/manifold/src/csg_tree.h +++ b/src/manifold/src/csg_tree.h @@ -85,7 +85,7 @@ class CsgOpNode final : public CsgNode { private: struct Impl { std::vector> children_; - bool flattened_ = false; + bool forcedToLeafNodes_ = false; }; mutable ConcurrentSharedPtr impl_ = ConcurrentSharedPtr(Impl{}); CsgNodeType op_; @@ -103,7 +103,7 @@ class CsgOpNode final : public CsgNode { void BatchUnion() const; std::vector> &GetChildren( - bool finalize = true) const; + bool forceToLeafNodes = true) const; }; } // namespace manifold From 147017f2fa96749bb1ce3975d755cb6eca2bbb9e Mon Sep 17 00:00:00 2001 From: ochafik Date: Tue, 14 Mar 2023 12:53:06 +0000 Subject: [PATCH 10/15] Simplified CsgOpNode::GetChildren --- src/manifold/src/csg_tree.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/manifold/src/csg_tree.cpp b/src/manifold/src/csg_tree.cpp index d057ce180..eda9c4151 100644 --- a/src/manifold/src/csg_tree.cpp +++ b/src/manifold/src/csg_tree.cpp @@ -456,18 +456,16 @@ void CsgOpNode::BatchUnion() const { std::vector> &CsgOpNode::GetChildren( bool forceToLeafNodes) const { auto impl = impl_.GetGuard(); - auto &children_ = impl->children_; - if (children_.empty() || impl->forcedToLeafNodes_) return children_; - impl->forcedToLeafNodes_ = forceToLeafNodes; - if (forceToLeafNodes) { - for (auto &child : children_) { + if (forceToLeafNodes && !impl->forcedToLeafNodes_) { + impl->forcedToLeafNodes_ = true; + for (auto &child : impl->children_) { if (child->GetNodeType() != CsgNodeType::Leaf) { child = child->ToLeafNode(); } } } - return children_; + return impl->children_; } void CsgOpNode::SetOp(OpType op) { From 2ef7d4ea9eda8721eb0caf91d0c47d49deec62bc Mon Sep 17 00:00:00 2001 From: ochafik Date: Tue, 14 Mar 2023 13:14:38 +0000 Subject: [PATCH 11/15] Simpler (a-(b+c)) -> (a-b-c) transform in CsgOpNode::Boolean to work better w/ CsgOpNode::ToLeafNode --- src/manifold/src/csg_tree.cpp | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/manifold/src/csg_tree.cpp b/src/manifold/src/csg_tree.cpp index eda9c4151..cf94ddb60 100644 --- a/src/manifold/src/csg_tree.cpp +++ b/src/manifold/src/csg_tree.cpp @@ -256,7 +256,9 @@ std::shared_ptr CsgOpNode::Boolean( auto handleOperand = [&](const std::shared_ptr &operand) { if (auto opNode = std::dynamic_pointer_cast(operand)) { - if (opNode->IsOp(op) && opNode->impl_.UseCount() == 1) { + if ((opNode->IsOp(op) || + (op == OpType::Subtract && opNode->IsOp(OpType::Add))) && + opNode->impl_.UseCount() == 1) { for (auto &child : opNode->GetChildren(/* forceToLeafNodes= */ false)) { children.push_back(child); } @@ -268,14 +270,6 @@ std::shared_ptr CsgOpNode::Boolean( handleOperand(shared_from_this()); handleOperand(second); - if (op == OpType::Subtract && children.size() > 2) { - // special handling for difference: we treat it as first - (second + third + - // ...) so op = Union after the first node - std::vector> unionChildren(children.begin() + 1, - children.end()); - children.resize(1); - children.push_back(std::make_shared(unionChildren, OpType::Add)); - } return std::make_shared(children, op); } From 7cb439c8111401bc67b8a7a5925979f3f12b64c3 Mon Sep 17 00:00:00 2001 From: ochafik Date: Tue, 14 Mar 2023 21:20:45 +0000 Subject: [PATCH 12/15] Simpler build hints for large_scene_test.cpp --- extras/large_scene_test.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/extras/large_scene_test.cpp b/extras/large_scene_test.cpp index 8557cee47..98ff26cbe 100644 --- a/extras/large_scene_test.cpp +++ b/extras/large_scene_test.cpp @@ -23,11 +23,7 @@ using namespace manifold; Build & execute with the following command: ( mkdir -p build && cd build && \ - cmake -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=ON \ - -DMANIFOLD_PAR=TBB \ - -DTHRUST_MULTICONFIG_ENABLE_SYSTEM_TBB=1 \ - -DTHRUST_HOST_SYSTEM=THRUST_HOST_SYSTEM_TBB \ - .. && \ + cmake -DCMAKE_BUILD_TYPE=Release -DMANIFOLD_PAR=TBB .. && \ make -j && \ time ./extras/largeSceneTest 50 ) */ From 409dd3fe9d2fec0185e123edef568cfb3f9f947d Mon Sep 17 00:00:00 2001 From: ochafik Date: Thu, 16 Mar 2023 03:41:09 +0000 Subject: [PATCH 13/15] [flatten] Refine rules about tree flattening + added test case --- src/manifold/src/csg_tree.cpp | 39 +++++++++++++++++++++++------------ test/manifold_test.cpp | 28 +++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 13 deletions(-) diff --git a/src/manifold/src/csg_tree.cpp b/src/manifold/src/csg_tree.cpp index cf94ddb60..07796ce83 100644 --- a/src/manifold/src/csg_tree.cpp +++ b/src/manifold/src/csg_tree.cpp @@ -254,21 +254,34 @@ std::shared_ptr CsgOpNode::Boolean( const std::shared_ptr &second, OpType op) { std::vector> children; - auto handleOperand = [&](const std::shared_ptr &operand) { - if (auto opNode = std::dynamic_pointer_cast(operand)) { - if ((opNode->IsOp(op) || - (op == OpType::Subtract && opNode->IsOp(OpType::Add))) && - opNode->impl_.UseCount() == 1) { - for (auto &child : opNode->GetChildren(/* forceToLeafNodes= */ false)) { - children.push_back(child); - } - return; - } + if (IsOp(op) && (impl_.UseCount() == 1)) { + auto impl = impl_.GetGuard(); + std::copy(impl->children_.begin(), impl->children_.end(), + std::back_inserter(children)); + } else { + children.push_back(shared_from_this()); + } + + auto secondOp = std::dynamic_pointer_cast(second); + auto canInlineSecondOp = [&]() { + switch (op) { + case OpType::Add: + case OpType::Intersect: + return secondOp->IsOp(op); + case OpType::Subtract: + return secondOp->IsOp(OpType::Add); + default: + return false; } - children.push_back(operand); }; - handleOperand(shared_from_this()); - handleOperand(second); + + if (secondOp && (secondOp->impl_.UseCount() == 1) && canInlineSecondOp()) { + auto secondImpl = secondOp->impl_.GetGuard(); + std::copy(secondImpl->children_.begin(), secondImpl->children_.end(), + std::back_inserter(children)); + } else { + children.push_back(second); + } return std::make_shared(children, op); } diff --git a/test/manifold_test.cpp b/test/manifold_test.cpp index 52a8135bb..f00f3a8e5 100644 --- a/test/manifold_test.cpp +++ b/test/manifold_test.cpp @@ -505,3 +505,31 @@ TEST(Manifold, MirrorUnion) { EXPECT_FLOAT_EQ(vol_a * 2.75, result.GetProperties().volume); EXPECT_TRUE(a.Mirror(glm::vec3(0)).IsEmpty()); } + +TEST(Manifold, BooleanVolumes) { + glm::mat4 m = glm::translate(glm::mat4(1.0f), glm::vec3(1.0f)); + + // Define solids which volumes are easy to compute w/ bit arithmetics: + // m1, m2, m4 are unique, non intersecting "bits" (of volume 1, 2, 4) + // m3 = m1 + m2 + // m7 = m1 + m2 + m3 + auto m1 = Manifold::Cube({1, 1, 1}); + auto m2 = Manifold::Cube({2, 1, 1}).Transform( + glm::translate(glm::mat4(1.0f), glm::vec3(1.0f, 0, 0))); + auto m4 = Manifold::Cube({4, 1, 1}).Transform( + glm::translate(glm::mat4(1.0f), glm::vec3(3.0f, 0, 0))); + auto m3 = Manifold::Cube({3, 1, 1}); + auto m7 = Manifold::Cube({7, 1, 1}); + + EXPECT_FLOAT_EQ((m1 ^ m2).GetProperties().volume, 0); + EXPECT_FLOAT_EQ((m1 + m2 + m4).GetProperties().volume, 7); + EXPECT_FLOAT_EQ((m1 + m2 - m4).GetProperties().volume, 3); + EXPECT_FLOAT_EQ((m1 + (m2 ^ m4)).GetProperties().volume, 1); + EXPECT_FLOAT_EQ((m7 ^ m4).GetProperties().volume, 4); + EXPECT_FLOAT_EQ((m7 ^ m3 ^ m1).GetProperties().volume, 1); + EXPECT_FLOAT_EQ((m7 ^ (m1 + m2)).GetProperties().volume, 3); + EXPECT_FLOAT_EQ((m7 - m4).GetProperties().volume, 3); + EXPECT_FLOAT_EQ((m7 - m4 - m2).GetProperties().volume, 1); + EXPECT_FLOAT_EQ((m7 - (m7 - m1)).GetProperties().volume, 1); + EXPECT_FLOAT_EQ((m7 - (m1 + m2)).GetProperties().volume, 4); +} From 1adafe894908e14e597228f37dd33f0ea8eb94c7 Mon Sep 17 00:00:00 2001 From: ochafik Date: Thu, 16 Mar 2023 15:23:56 +0000 Subject: [PATCH 14/15] [manifold] Propagate transforms when flattening op trees --- src/manifold/src/csg_tree.cpp | 22 +++++++++++++++------- test/manifold_test.cpp | 8 ++++++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/manifold/src/csg_tree.cpp b/src/manifold/src/csg_tree.cpp index 07796ce83..01441e78d 100644 --- a/src/manifold/src/csg_tree.cpp +++ b/src/manifold/src/csg_tree.cpp @@ -254,12 +254,21 @@ std::shared_ptr CsgOpNode::Boolean( const std::shared_ptr &second, OpType op) { std::vector> children; - if (IsOp(op) && (impl_.UseCount() == 1)) { + auto isReused = [](const auto &node) { return node->impl_.UseCount() > 1; }; + + auto copyChildren = [&](const auto &list, const glm::mat4x3 &transform) { + for (const auto &child : list) { + children.push_back(child->Transform(transform)); + } + }; + + auto self = std::dynamic_pointer_cast(shared_from_this()); + assert(self); + if (IsOp(op) && !isReused(self)) { auto impl = impl_.GetGuard(); - std::copy(impl->children_.begin(), impl->children_.end(), - std::back_inserter(children)); + copyChildren(impl->children_, transform_); } else { - children.push_back(shared_from_this()); + children.push_back(self); } auto secondOp = std::dynamic_pointer_cast(second); @@ -275,10 +284,9 @@ std::shared_ptr CsgOpNode::Boolean( } }; - if (secondOp && (secondOp->impl_.UseCount() == 1) && canInlineSecondOp()) { + if (secondOp && canInlineSecondOp() && !isReused(secondOp)) { auto secondImpl = secondOp->impl_.GetGuard(); - std::copy(secondImpl->children_.begin(), secondImpl->children_.end(), - std::back_inserter(children)); + copyChildren(secondImpl->children_, secondOp->transform_); } else { children.push_back(second); } diff --git a/test/manifold_test.cpp b/test/manifold_test.cpp index f00f3a8e5..f7ac18dbd 100644 --- a/test/manifold_test.cpp +++ b/test/manifold_test.cpp @@ -533,3 +533,11 @@ TEST(Manifold, BooleanVolumes) { EXPECT_FLOAT_EQ((m7 - (m7 - m1)).GetProperties().volume, 1); EXPECT_FLOAT_EQ((m7 - (m1 + m2)).GetProperties().volume, 4); } + +TEST(Manifold, TreeTransforms) { + auto a = (Manifold::Cube({1, 1, 1}) + Manifold::Cube({1, 1, 1})) + .Translate({1, 0, 0}); + auto b = (Manifold::Cube({1, 1, 1}) + Manifold::Cube({1, 1, 1})); + + EXPECT_FLOAT_EQ((a + b).GetProperties().volume, 2); +} From 6fcf61b626d0972255ee002aa98856ae921f0a1d Mon Sep 17 00:00:00 2001 From: ochafik Date: Thu, 16 Mar 2023 16:39:48 +0000 Subject: [PATCH 15/15] [flatten] move tests from manifold_test to boolean_test --- test/boolean_test.cpp | 36 ++++++++++++++++++++++++++++++++++++ test/manifold_test.cpp | 36 ------------------------------------ 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/test/boolean_test.cpp b/test/boolean_test.cpp index 29f30e32a..4cea24ce9 100644 --- a/test/boolean_test.cpp +++ b/test/boolean_test.cpp @@ -589,3 +589,39 @@ TEST(Boolean, UnionDifference) { float blocksize = block.GetProperties().volume; EXPECT_NEAR(resultsize, blocksize * 2, 0.0001); } + +TEST(Boolean, BooleanVolumes) { + glm::mat4 m = glm::translate(glm::mat4(1.0f), glm::vec3(1.0f)); + + // Define solids which volumes are easy to compute w/ bit arithmetics: + // m1, m2, m4 are unique, non intersecting "bits" (of volume 1, 2, 4) + // m3 = m1 + m2 + // m7 = m1 + m2 + m3 + auto m1 = Manifold::Cube({1, 1, 1}); + auto m2 = Manifold::Cube({2, 1, 1}).Transform( + glm::translate(glm::mat4(1.0f), glm::vec3(1.0f, 0, 0))); + auto m4 = Manifold::Cube({4, 1, 1}).Transform( + glm::translate(glm::mat4(1.0f), glm::vec3(3.0f, 0, 0))); + auto m3 = Manifold::Cube({3, 1, 1}); + auto m7 = Manifold::Cube({7, 1, 1}); + + EXPECT_FLOAT_EQ((m1 ^ m2).GetProperties().volume, 0); + EXPECT_FLOAT_EQ((m1 + m2 + m4).GetProperties().volume, 7); + EXPECT_FLOAT_EQ((m1 + m2 - m4).GetProperties().volume, 3); + EXPECT_FLOAT_EQ((m1 + (m2 ^ m4)).GetProperties().volume, 1); + EXPECT_FLOAT_EQ((m7 ^ m4).GetProperties().volume, 4); + EXPECT_FLOAT_EQ((m7 ^ m3 ^ m1).GetProperties().volume, 1); + EXPECT_FLOAT_EQ((m7 ^ (m1 + m2)).GetProperties().volume, 3); + EXPECT_FLOAT_EQ((m7 - m4).GetProperties().volume, 3); + EXPECT_FLOAT_EQ((m7 - m4 - m2).GetProperties().volume, 1); + EXPECT_FLOAT_EQ((m7 - (m7 - m1)).GetProperties().volume, 1); + EXPECT_FLOAT_EQ((m7 - (m1 + m2)).GetProperties().volume, 4); +} + +TEST(Boolean, TreeTransforms) { + auto a = (Manifold::Cube({1, 1, 1}) + Manifold::Cube({1, 1, 1})) + .Translate({1, 0, 0}); + auto b = (Manifold::Cube({1, 1, 1}) + Manifold::Cube({1, 1, 1})); + + EXPECT_FLOAT_EQ((a + b).GetProperties().volume, 2); +} diff --git a/test/manifold_test.cpp b/test/manifold_test.cpp index f7ac18dbd..52a8135bb 100644 --- a/test/manifold_test.cpp +++ b/test/manifold_test.cpp @@ -505,39 +505,3 @@ TEST(Manifold, MirrorUnion) { EXPECT_FLOAT_EQ(vol_a * 2.75, result.GetProperties().volume); EXPECT_TRUE(a.Mirror(glm::vec3(0)).IsEmpty()); } - -TEST(Manifold, BooleanVolumes) { - glm::mat4 m = glm::translate(glm::mat4(1.0f), glm::vec3(1.0f)); - - // Define solids which volumes are easy to compute w/ bit arithmetics: - // m1, m2, m4 are unique, non intersecting "bits" (of volume 1, 2, 4) - // m3 = m1 + m2 - // m7 = m1 + m2 + m3 - auto m1 = Manifold::Cube({1, 1, 1}); - auto m2 = Manifold::Cube({2, 1, 1}).Transform( - glm::translate(glm::mat4(1.0f), glm::vec3(1.0f, 0, 0))); - auto m4 = Manifold::Cube({4, 1, 1}).Transform( - glm::translate(glm::mat4(1.0f), glm::vec3(3.0f, 0, 0))); - auto m3 = Manifold::Cube({3, 1, 1}); - auto m7 = Manifold::Cube({7, 1, 1}); - - EXPECT_FLOAT_EQ((m1 ^ m2).GetProperties().volume, 0); - EXPECT_FLOAT_EQ((m1 + m2 + m4).GetProperties().volume, 7); - EXPECT_FLOAT_EQ((m1 + m2 - m4).GetProperties().volume, 3); - EXPECT_FLOAT_EQ((m1 + (m2 ^ m4)).GetProperties().volume, 1); - EXPECT_FLOAT_EQ((m7 ^ m4).GetProperties().volume, 4); - EXPECT_FLOAT_EQ((m7 ^ m3 ^ m1).GetProperties().volume, 1); - EXPECT_FLOAT_EQ((m7 ^ (m1 + m2)).GetProperties().volume, 3); - EXPECT_FLOAT_EQ((m7 - m4).GetProperties().volume, 3); - EXPECT_FLOAT_EQ((m7 - m4 - m2).GetProperties().volume, 1); - EXPECT_FLOAT_EQ((m7 - (m7 - m1)).GetProperties().volume, 1); - EXPECT_FLOAT_EQ((m7 - (m1 + m2)).GetProperties().volume, 4); -} - -TEST(Manifold, TreeTransforms) { - auto a = (Manifold::Cube({1, 1, 1}) + Manifold::Cube({1, 1, 1})) - .Translate({1, 0, 0}); - auto b = (Manifold::Cube({1, 1, 1}) + Manifold::Cube({1, 1, 1})); - - EXPECT_FLOAT_EQ((a + b).GetProperties().volume, 2); -}