-
Notifications
You must be signed in to change notification settings - Fork 98
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
more triangulator check #516
Conversation
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## master #516 +/- ##
=======================================
Coverage 90.36% 90.36%
=======================================
Files 35 35
Lines 4433 4433
=======================================
Hits 4006 4006
Misses 427 427
☔ View full report in Codecov by Sentry. |
Here is an optimization that runs triangulation in parallel, so test cases like diff --git a/src/manifold/src/face_op.cpp b/src/manifold/src/face_op.cpp
index 0437ad3..680ff0a 100644
--- a/src/manifold/src/face_op.cpp
+++ b/src/manifold/src/face_op.cpp
@@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+#include <future>
#include <map>
#include "impl.h"
@@ -39,6 +40,10 @@ void Manifold::Impl::Face2Tri(const VecDH<int>& faceEdge,
triNormal.reserve(halfedge_.size());
triRef.reserve(halfedge_.size());
+ std::vector<
+ std::pair<int, std::future<std::pair<int, std::vector<glm::ivec3>>>>>
+ slowTasks;
+
for (int face = 0; face < faceEdge.size() - 1; ++face) {
const int firstEdge = faceEdge[face];
const int lastEdge = faceEdge[face + 1];
@@ -116,11 +121,24 @@ void Manifold::Impl::Face2Tri(const VecDH<int>& faceEdge,
triNormal.push_back(normal);
triRef.push_back(halfedgeRef[firstEdge]);
}
+ } else if (numEdge > 512) {
+ slowTasks.push_back(std::make_pair(
+ numEdge, std::async(
+ std::launch::async,
+ [&](int face) {
+ const glm::vec3 normal = faceNormal_[face];
+ const glm::mat3x2 projection =
+ GetAxisAlignedProjection(normal);
+ const PolygonsIdx polys =
+ Face2Polygons(face, projection, faceEdge);
+ std::vector<glm::ivec3> newTris =
+ TriangulateIdx(polys, precision_);
+ return std::make_pair(face, std::move(newTris));
+ },
+ face)));
} else { // General triangulation
const glm::mat3x2 projection = GetAxisAlignedProjection(normal);
-
const PolygonsIdx polys = Face2Polygons(face, projection, faceEdge);
-
std::vector<glm::ivec3> newTris = TriangulateIdx(polys, precision_);
for (auto tri : newTris) {
@@ -130,6 +148,22 @@ void Manifold::Impl::Face2Tri(const VecDH<int>& faceEdge,
}
}
}
+
+ std::sort(slowTasks.begin(), slowTasks.end(),
+ [](const auto& p1, const auto& p2) { return p1.first < p2.first; });
+ for (auto& task : slowTasks) {
+ task.second.wait();
+ const auto pair = task.second.get();
+ const int face = pair.first;
+ const int firstEdge = faceEdge[face];
+ const glm::vec3 normal = faceNormal_[face];
+ for (auto tri : pair.second) {
+ triVerts.push_back(tri);
+ triNormal.push_back(normal);
+ triRef.push_back(halfedgeRef[firstEdge]);
+ }
+ }
+
faceNormal_ = std::move(triNormal);
CreateHalfedges(triVerts);
}
@@ -143,7 +177,7 @@ PolygonsIdx Manifold::Impl::Face2Polygons(int face, glm::mat3x2 projection,
const int firstEdge = faceEdge[face];
const int lastEdge = faceEdge[face + 1];
- std::map<int, int> vert_edge;
+ std::unordered_map<int, int> vert_edge;
for (int edge = firstEdge; edge < lastEdge; ++edge) {
const bool inserted =
vert_edge.emplace(std::make_pair(halfedge_[edge].startVert, edge)) The main problem is that I am not sure if we can reorder faces like this. |
I think it's okay to make the faces in a different order - they're independent calculations and the triangles get resorted again anyway in |
For winding directions, I think it's probably adequate to check if the overall area is positive. It might miss an inverted hole or something, but that should become pretty obvious from the reduced example. The right way is to find each polygon's winding number by testing a non-degenerate point, then testing the sign of the area against that, but that's likely more trouble than it's worth. |
It increases the number of degenerate triangles in
Yeah sounds quite complicated, I think the user can visualize the polygon and look at winding direction by themselves. |
The extra 1000 degenerates sounds like it's in the noise to me - no surprise there's some order-dependence there. I need to come back and see if I can get that down again sometime. It should be possible to remove all of them, honestly. |
ah I have no idea why the wasm build failed. it seems that the code runs fine, but the js part is now wrong? |
I moved the face reordering out because it breaks the mengerSponge js example. I will open a separate issue for it. |
@@ -143,7 +144,7 @@ void CheckTopology(const std::vector<glm::ivec3> &triangles, | |||
|
|||
void CheckGeometry(const std::vector<glm::ivec3> &triangles, | |||
const PolygonsIdx &polys, float precision) { | |||
std::map<int, glm::vec2> vertPos; | |||
std::unordered_map<int, glm::vec2> vertPos; |
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.
ok, this seemingly innocent change is causing bug...
causing changes in triangulation because the initial vertex is now non-determinsitic
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.
Interesting, thanks! I have a plan on the triangulator, but I need to find a couple days to put it into action.
* add minimize_testcase helper * added testcases from elalish#502 * minor cleanup * Update polygon.cpp * Update face_op.cpp
This enables triangle overlap check when
MANIFOLD_DEBUG
is enabled. And I also added a tool calledminimizeTestcase
for minimizing triangulator testcases.The added checks should not slow the triangulator down too much whenMANIFOLD_DEBUG=on
. On my computer with TBB enabled,Samples.Sponge4
runs in 6838ms comparing with > 7000ms previously.minimizeTestcase
does the following:Usage: ./minimizeTestcase <input file>
where input file is a path to a text file containing the error output from the triangulator whenMANIFOLD_DEBUG
is on. For example:Polygons
in the input file, and try to remove vertices and simple polygons one by one while keeping the resulting polygon correct (does not introduce overlapping edges, and not inverting winding directions), while keeping the same error output.^ not CCW
in the line below. (should be relatively efficient, I can minimize a 50k vertices test case into 10 vertices in 32s)For now, the tool will not check if the input polygon is valid, but doing this should not be hard.The tool will check for self-intersection but not check if the polygon winding directions are correct. (not sure what is the best way for checking that when we can have subtle self-intersections that are epsilon-valid)I also added two test cases that I got from the woodgrain stl file.