-
Notifications
You must be signed in to change notification settings - Fork 35
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 UnitProto after building a CsgUnit #1415
base: develop
Are you sure you want to change the base?
Conversation
8e23c9f
to
b2a0c4c
Compare
/*! | ||
* Result of a DeMorgan simplification. | ||
*/ | ||
struct DeMorganSimplifierResult |
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 could be a general "simplification" mapping. Perhaps it should be moved to CsgTree itself (imagine a function that runs "simplify" on the tree itself)?
struct DeMorganSimplifierResult | ||
{ | ||
CsgTree tree; | ||
std::vector<NodeId> equivalent_nodes; |
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.
Old node ID is the index, new node ID is the value? (Maybe new_nodes
?)
@@ -391,7 +391,7 @@ TEST_F(CsgTreeUtilsTest, transform_negated_joins) | |||
auto s1 = this->insert(Surface{S{1}}); | |||
auto n0 = this->insert(Negated{s1}); | |||
auto j0 = this->insert(Joined{op_and, {s0, n0}}); | |||
auto simplified = transform_negated_joins(tree_); | |||
auto simplified = transform_negated_joins(tree_).tree; |
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.
You should test the mapping result too...
Test summary 3 279 files 5 076 suites 3m 31s ⏱️ Results for commit 4f8f07f. ♻️ This comment has been updated with latest results. |
This is mostly for validation purposes, eventually I think we should only use the transformed
CsgTree
on GPU.Requires #1418