-
Notifications
You must be signed in to change notification settings - Fork 145
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
Fix earcut bug - triangulating faces with +4 vertices #32
Conversation
Unfortunately, this file: http://robbykraft.github.io/Origami/files/fold/crane.fold is still unhappy :( |
Okay and now checked with issue #21 and we also get flipped faces... (it actually happens before you save and load as FOLD - you see the flipped faces even when you load the SVG) Is that a good thing or a bad thing? |
A somewhat more minimal example. When you load the following SVG (small excerpt from the maze) into OrigamiSimulator-master with vertex merge tolerance of 1pixel - it folds flat. When you do it on my branch, exactly one pentagon face is flipped and it cannot fold flat. To be continued |
Ok, found a problem - I was checking for the orientation using the faceEdges and edges structures but it's was not actually oriented correctly. I suspect this might lead to other problems (maybe #21 ) but for this PR I could just check the orientation using the original face structure, which is correctly oriented. Now the Dodecahedron, the Crane and the OrigamiSimulator Map are all loaded correctly into a flat folded state (!) If I try to save to FOLD and load the origami simulator map - I still get exactly three missing triangles. Which seems quite better than before, though not perfect: |
…triangualation (even when dim=3), it is important we give it a non degenrate face.. this commit checks which coordinates have the largest variance and rearanges the dimensions accordingly. this doesnt affect the model in anyway
An important update. Diving into the earcut code (both the one in the repo as well as the most updated for github) - there is something very odd, while it does support multiple dimensions, for triangulation only the first two dimensions are being used! You can see this behavior in lines 50 & 52: https://github.com/mapbox/earcut/blob/master/src/earcut.js#L50 The new node only contains X and Y (regardless of dim) This used to work fine when "everything" was 2d but when you load a FOLD format that was exported from SVG it's actually 3d with the second coordinate always zero and then you get the holes seen in issue #21 . I verified this by opening the .FOLD file in fold viewer and you can see the five-vertices face is there and correctly oriented. As a fix to this behavior, I added quite a large chunk of code that re-orders the coordinates so the first two always have the largest variance. To be honest, it might be worth considering switching to a different triangulation library, or doing it ourselves. Now the OrigamiSimulator map is loading fine, including after saving and loading from SVG. There still is some problem with the Dodecahedron though, will update soon |
Ok maybe last update :) using the variance didn't work because the dodecehadron actually had two faces that had some variance but were co-linear. Instead, in my last commit I just check all options for dimension ordering until we get a triangulation back from earcut. This now works for the Dodecahdren and fix the OrigamiSimulator map bug described in #21 |
* Correct is2d detection: 2D only if all vertices are 2D, and treat 0 the same as an absent third coordinate. * Always swap y and z coordinates when importing/exporting FOLD, independent of 2D status, for consistent behavior in all cases. (To get there, `rawFold` also has swapped coordiantes.) * To decide `needsFlip`, instead of looking for a face edge in the first triangle, which can fail if the first triangle is a fully interior triangle, look for the first edge of the face in all the triangles.
Awesome, thanks for this. Trying all three pairs of coordinates seems ugly, but it seems like a good workaround for now. I did a revision pass both for style and for correctness; see commit messages and diff for description:
I think this should be good to go now, but it would be good to test everything again, as I made significant changes to the algorithms you were using. The dodecahedron example seems to work, as does Robbie's |
Swapping y/z flips the coordinate system between left- and right-handed, effectively reversing the notions of cw/ccw. This isn't an issue in 2D, because there it would be the same as mapping (x, y, z) → (x, -z, y) (because z=0) which doesn't flip handedness, but it's a problem in 3D.
… (because of it using only the first two coordinates, and possible degeneracies there). this update makes sure there are at least enough triangles before breaking
} | ||
triangles = earcut(faceVert, null, 3); | ||
// make sure we got *enough* triangle to cover the face | ||
if (triangles.length >= 3 * (face.length - 2)) break; |
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.
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... :)
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.
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...
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.
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
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, I ran another series of tests, and everything is looking great. Thanks for working so hard on this. In hindsight, I've worked around this bug a lot in the past. This will make my and many people's lives easier!
As mentioned here in this PR as well as this earcut issue - the triangles returned by earcut do not correspond to the original face order
This fix adds a small check per face - the first triangle is verified that it contains at least one edge in the correct order from the original face. If not, flip the triangle.
The way to verify this is using this closed model of a Dodecahedron (fully folded):
dodecahedron.fold.zip
Also, I changed is2d from always true to always false. But to be honest I don't know what are the effects of this. It did look very weird that earcut gets a 2d projection of the faces for triangulation (I can try to make a model where this fails, something like a pentagon that is on the Y plane)