-
Notifications
You must be signed in to change notification settings - Fork 105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Simplify the CsgOpNodes as we build them, rather than in GetChildren #368
Changes from 13 commits
a43e337
f091479
210bb0e
7b488a5
166c603
5f91aa9
17a13d6
3bf059c
ce85a44
147017f
2ef7d4e
7cb439c
409dd3f
1adafe8
6fcf61b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
// 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 <chrono> | ||
#include <iostream> | ||
|
||
#include "manifold.h" | ||
|
||
using namespace manifold; | ||
|
||
/* | ||
Build & execute with the following command: | ||
|
||
( mkdir -p build && cd build && \ | ||
cmake -DCMAKE_BUILD_TYPE=Release -DMANIFOLD_PAR=TBB .. && \ | ||
make -j && \ | ||
time ./extras/largeSceneTest 50 ) | ||
*/ | ||
int main(int argc, char **argv) { | ||
int n = 20; | ||
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<double> elapsed = end - start; | ||
std::cout << "nTri = " << scene.NumTri() << ", time = " << elapsed.count() | ||
<< " sec" << std::endl; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,6 +73,19 @@ struct CheckOverlap { | |
} // namespace | ||
namespace manifold { | ||
|
||
std::shared_ptr<CsgNode> CsgNode::Boolean( | ||
const std::shared_ptr<CsgNode> &second, OpType op) { | ||
if (auto opNode = std::dynamic_pointer_cast<CsgOpNode>(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<std::shared_ptr<CsgNode>> children({shared_from_this(), second}); | ||
return std::make_shared<CsgOpNode>(children, op); | ||
} | ||
|
||
std::shared_ptr<CsgNode> CsgNode::Translate(const glm::vec3 &t) const { | ||
glm::mat4x3 transform(1.0f); | ||
transform[3] += t; | ||
|
@@ -227,8 +240,6 @@ CsgOpNode::CsgOpNode(const std::vector<std::shared_ptr<CsgNode>> &children, | |
auto impl = impl_.GetGuard(); | ||
impl->children_ = children; | ||
SetOp(op); | ||
// opportunistically flatten the tree without costly evaluation | ||
GetChildren(false); | ||
} | ||
|
||
CsgOpNode::CsgOpNode(std::vector<std::shared_ptr<CsgNode>> &&children, | ||
|
@@ -237,8 +248,42 @@ CsgOpNode::CsgOpNode(std::vector<std::shared_ptr<CsgNode>> &&children, | |
auto impl = impl_.GetGuard(); | ||
impl->children_ = children; | ||
SetOp(op); | ||
// opportunistically flatten the tree without costly evaluation | ||
GetChildren(false); | ||
} | ||
|
||
std::shared_ptr<CsgNode> CsgOpNode::Boolean( | ||
const std::shared_ptr<CsgNode> &second, OpType op) { | ||
std::vector<std::shared_ptr<CsgNode>> children; | ||
|
||
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<CsgOpNode>(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; | ||
} | ||
}; | ||
|
||
if (secondOp && (secondOp->impl_.UseCount() == 1) && canInlineSecondOp()) { | ||
auto secondImpl = secondOp->impl_.GetGuard(); | ||
std::copy(secondImpl->children_.begin(), secondImpl->children_.end(), | ||
geoffder marked this conversation as resolved.
Show resolved
Hide resolved
|
||
std::back_inserter(children)); | ||
} else { | ||
children.push_back(second); | ||
} | ||
|
||
return std::make_shared<CsgOpNode>(children, op); | ||
} | ||
|
||
std::shared_ptr<CsgNode> CsgOpNode::Transform(const glm::mat4x3 &m) const { | ||
|
@@ -410,44 +455,24 @@ 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<std::shared_ptr<CsgNode>> &CsgOpNode::GetChildren( | ||
bool finalize) const { | ||
bool forceToLeafNodes) const { | ||
auto impl = impl_.GetGuard(); | ||
auto &children_ = impl->children_; | ||
if (children_.empty() || (impl->simplified_ && !finalize) || impl->flattened_) | ||
return children_; | ||
impl->simplified_ = true; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Happy to see this simplification, but what was the difference between There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Flattening (also aliased here to finalization) was forcing leaf nodes, while simplification was flattening 1 level of op nodes nesting (adopting the children of same op children, and in the case of difference, of union op children past the first child). Have now renamed finalize & flattened_ to force[d]ToLeafNodes[_] to disambiguate, and simplified this method further. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perfect, thank you! |
||
impl->flattened_ = finalize; | ||
std::vector<std::shared_ptr<CsgNode>> newChildren; | ||
|
||
CsgNodeType op = op_; | ||
for (auto &child : children_) { | ||
if (child->GetNodeType() == op && child.use_count() == 1 && | ||
std::dynamic_pointer_cast<CsgOpNode>(child)->impl_.UseCount() == 1) { | ||
auto grandchildren = | ||
std::dynamic_pointer_cast<CsgOpNode>(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 (forceToLeafNodes && !impl->forcedToLeafNodes_) { | ||
impl->forcedToLeafNodes_ = true; | ||
for (auto &child : impl->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_; | ||
return impl->children_; | ||
} | ||
|
||
void CsgOpNode::SetOp(OpType op) { | ||
|
@@ -464,6 +489,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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,13 +22,16 @@ enum class CsgNodeType { Union, Intersection, Difference, Leaf }; | |
|
||
class CsgLeafNode; | ||
|
||
class CsgNode { | ||
class CsgNode : public std::enable_shared_from_this<CsgNode> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! I never know such a thing exists, I wanted to do something similar and have to do it the dumb way. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah it's a nice get out of jail free card 🤣 |
||
public: | ||
virtual std::shared_ptr<CsgLeafNode> ToLeafNode() const = 0; | ||
virtual std::shared_ptr<CsgNode> Transform(const glm::mat4x3 &m) const = 0; | ||
virtual CsgNodeType GetNodeType() const = 0; | ||
virtual glm::mat4x3 GetTransform() const = 0; | ||
|
||
virtual std::shared_ptr<CsgNode> Boolean( | ||
const std::shared_ptr<CsgNode> &second, OpType op); | ||
|
||
std::shared_ptr<CsgNode> Translate(const glm::vec3 &t) const; | ||
std::shared_ptr<CsgNode> Scale(const glm::vec3 &s) const; | ||
std::shared_ptr<CsgNode> Rotate(float xDegrees = 0, float yDegrees = 0, | ||
|
@@ -68,6 +71,9 @@ class CsgOpNode final : public CsgNode { | |
|
||
CsgOpNode(std::vector<std::shared_ptr<CsgNode>> &&children, OpType op); | ||
|
||
std::shared_ptr<CsgNode> Boolean(const std::shared_ptr<CsgNode> &second, | ||
OpType op) override; | ||
|
||
std::shared_ptr<CsgNode> Transform(const glm::mat4x3 &m) const override; | ||
|
||
std::shared_ptr<CsgLeafNode> ToLeafNode() const override; | ||
|
@@ -79,8 +85,7 @@ class CsgOpNode final : public CsgNode { | |
private: | ||
struct Impl { | ||
std::vector<std::shared_ptr<CsgNode>> children_; | ||
bool simplified_ = false; | ||
bool flattened_ = false; | ||
bool forcedToLeafNodes_ = false; | ||
}; | ||
mutable ConcurrentSharedPtr<Impl> impl_ = ConcurrentSharedPtr<Impl>(Impl{}); | ||
CsgNodeType op_; | ||
|
@@ -89,6 +94,7 @@ class CsgOpNode final : public CsgNode { | |
mutable std::shared_ptr<CsgLeafNode> cache_ = nullptr; | ||
|
||
void SetOp(OpType); | ||
bool IsOp(OpType op); | ||
|
||
static std::shared_ptr<Manifold::Impl> BatchBoolean( | ||
OpType operation, | ||
|
@@ -97,7 +103,7 @@ class CsgOpNode final : public CsgNode { | |
void BatchUnion() const; | ||
|
||
std::vector<std::shared_ptr<CsgNode>> &GetChildren( | ||
bool finalize = true) const; | ||
bool forceToLeafNodes = true) const; | ||
}; | ||
|
||
} // namespace manifold |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Excellent tests, thank you! Technically these should go in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks!! Moved them 👌 |
||
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); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This copy doesn't apply transform from the parent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hadn't realized the transform was around, thanks!