Skip to content
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.

Sign up for GitHub

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

Fix earcut bug - triangulating faces with +4 vertices #32

Merged
merged 12 commits into from
Nov 30, 2020
Merged

Fix earcut bug - triangulating faces with +4 vertices #32

Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
40385f4
fix earcut bug - check triangles for correct order corresponding to o…
eyalperry88 Nov 18, 2020
3feb42d
actually check if it's 2d or 3d before turning everything to 3d
eyalperry88 Nov 18, 2020
50f689b
removed accidental newline
eyalperry88 Nov 18, 2020
6cc39c3
fixed! checking the edge orientation using the face instead of faceEdges
eyalperry88 Nov 18, 2020
2bbc16d
second earcut fix - since only the first two coordinates are used in …
eyalperry88 Nov 21, 2020
f5b8979
fix for the fix - just try all dimensions combinations until we hate …
eyalperry88 Nov 22, 2020
fc84a3d
Fix indentation to match coding style; remove unnecessary changes
edemaine Nov 28, 2020
d7fb3d1
Correct is2d and needsFlip detection and y/z flipping
edemaine Nov 28, 2020
68c9a6a
needsFlip bug fix
edemaine Nov 28, 2020
ff66566
Revert 2D -> 3D mapping instead of y/z swap
edemaine Nov 29, 2020
940b2e0
earcut sometimes returns less triangles than needed to cover the face…
eyalperry88 Nov 29, 2020
18a0313
check dimension combination starting from [2,0,1] to [1, 2, 0] to [0,…
eyalperry88 Nov 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 60 additions & 21 deletions js/pattern.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,15 +544,22 @@ function initPattern(globals){

function processFold(fold, returnCreaseParams){

rawFold = JSON.parse(JSON.stringify(fold));//save pre-triangulated for for save later
//make 3d
for (var i=0;i<rawFold.vertices_coords.length;i++){
var vertex = rawFold.vertices_coords[i];
if (vertex.length === 2) {//make vertices_coords 3d
rawFold.vertices_coords[i] = [vertex[0], 0, vertex[1]];
//add missing coordinates to make 3d, mapping (x,y) -> (x,0,z)
//This is against the FOLD spec which says that, beyond two dimensions,
//"all unspecified coordinates are implicitly zero"...
var is2d = true;
for (var i=0;i<fold.vertices_coords.length;i++){
var vertex = fold.vertices_coords[i];
if (vertex.length === 2) {
fold.vertices_coords[i] = [vertex[0], 0, vertex[1]];
} else {
is2d = false;
}
}

//save pre-triangulated faces for later saveFOLD()
rawFold = JSON.parse(JSON.stringify(fold));

var cuts = FOLD.filter.cutEdges(fold);
if (cuts.length>0) {
fold = splitCuts(fold);
Expand All @@ -562,14 +569,7 @@ function initPattern(globals){
delete fold.vertices_vertices;
delete fold.vertices_edges;

foldData = triangulatePolys(fold, true);

for (var i=0;i<foldData.vertices_coords.length;i++){
var vertex = foldData.vertices_coords[i];
if (vertex.length === 2) {//make vertices_coords 3d
foldData.vertices_coords[i] = [vertex[0], 0, vertex[1]];
}
}
foldData = triangulatePolys(fold, is2d);

mountains = FOLD.filter.mountainEdges(foldData);
valleys = FOLD.filter.valleyEdges(foldData);
Expand Down Expand Up @@ -1011,17 +1011,56 @@ function initPattern(globals){
}

var faceVert = [];
for (var j=0;j<face.length;j++){
var vertex = vertices[face[j]];
faceVert.push(vertex[0]);
faceVert.push(vertex[1]);
if (!is2d) faceVert.push(vertex[2]);
var triangles = [];
if (is2d) {
for (var j=0;j<face.length;j++){
var vertex = vertices[face[j]];
faceVert.push(vertex[0]);
faceVert.push(vertex[2]);
}
triangles = earcut(faceVert, null, 2);
} else {
// earcut only uses the two first coordinates for triangulation...
// as a fix, we try each of the dimension combinations until we get a result
for (var j=0; j<3; j++) {
faceVert = [];
for (var k=0;k<face.length;k++){
var vertex = vertices[face[k]];
faceVert.push(vertex[j]);
faceVert.push(vertex[(j + 1) % 3]);
faceVert.push(vertex[(j + 2) % 3]);
}
triangles = earcut(faceVert, null, 3);
// make sure we got *enough* triangle to cover the face
if (triangles.length >= 3 * (face.length - 2)) break;
Copy link
Contributor Author

@eyalperry88 eyalperry88 Nov 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just added this update. I found another problem, here is how to recreate:

  • Open OrigamiSimulator Map SVG (Vertex tolerance: 1px)
  • Save as FOLD (do not triangulate)
  • Load FOLD
    On my end, I see exactly two triangles missing (I see this in a previous comment too if you notice)

Here is the FOLD file for reference:
origamisimulator.fold.zip

Debugging it, it seems that again because of earcut using the first two coordinates, for some faces (very few..) we get less triangles than needed to cover the face. In this FOLD, there are two 5 vertices faces that earcut returns only two triangles (one vertex is out of the game).

My fix is checking that the number of triangles is at least the number of vertices on the face - 2. That is always right, right? Using this check, we try giving earcut a different coordinate system and it eventually returns the correct triangulation. I definitely think replacing earcut would be a better option in the future though..

We're getting close... :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, assuming no added vertices (which must be the case for earcut, because it has no mechanism to return additional vertices), the number of triangles should be exactly face - 2, so this seems like a good change.

What do you think about also changing the order that we try the dimension pairs to start with x and z, like we do in 2D? It shouldn't matter, but for files that were originally flat and imported, this seems like a better starting point, and kind of nice to try the is2d algorithm and then continue? We could just reverse the loop...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me. Pushed a commit where I simply reversed the loop and checked with the FOLD and SVG models and it all seems fine

}
}

var triangles = earcut(faceVert, null, is2d? 2:3);
// triangles from earcut() can have backwards winding relative to original face
// [https://github.com/mapbox/earcut/issues/44]
// we look for the first edge of the original face among the triangles;
// if it appears reversed in any triangle, we flip all triangles
var needsFlip = null;
for (var j=0;j<triangles.length;j+=3){
for (var k=0; k<3; k++) {
if (triangles[j + k] === 0 && triangles[j + (k+1)%3] === 1) {
needsFlip = false;
break;
} else if (triangles[j + k] === 1 && triangles[j + (k+1)%3] === 0) {
needsFlip = true;
break;
}
}
if (needsFlip != null) break;
}

for (var j=0;j<triangles.length;j+=3){
var tri = [face[triangles[j+2]], face[triangles[j+1]], face[triangles[j]]];
var tri;
if (needsFlip) {
tri = [face[triangles[j+2]], face[triangles[j+1]], face[triangles[j]]];
} else {
tri = [face[triangles[j]], face[triangles[j+1]], face[triangles[j+2]]];
}
var foundEdges = [false, false, false];//ab, bc, ca

for (var k=0;k<faceEdges.length;k++){
Expand Down