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.

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 bug where earcut() faces have backwards winding #20

Closed
wants to merge 2 commits into from

Conversation

mayakraft
Copy link
Contributor

No description provided.

@mayakraft
Copy link
Contributor Author

an example of a FOLD file i was using for testing http://robbykraft.github.io/Origami/files/fold/crane.fold which of course could have errors on their own but to double check winding i've been using jason's FOLD viewer

@edemaine
Copy link
Collaborator

I agree that that file is much happier with this patch. I also found the earcut bug that is causing this. But I'm a little unclear whether reversing everything is always the right thing — should we instead preserve the given orientations, as in the patches mentioned in the earcut issue? I'm especially concerned about the 3D case...

I'm also confused why triangulatePolys always gets called with is2d set to true. As a result, earcut gets only the x and y coordinate of each point, with no z coordinate. Actually, if I'm reading things correctly, it gets the x coordinate and zero, because the y coordinate is put in the 3rd position not the 2nd (yuck). Should we actually use the 3D mode of earcut?

@edemaine
Copy link
Collaborator

This PR also seems to break the built-in Maze Foldings / origami simulator example (just selected from the Examples menu): it seems to flip the same faces that are incorrect in #21 (so that bug may be related to this earcut issue too). Turning off is2d didn't seem to help...

image

@edemaine
Copy link
Collaborator

Replaced by #32.

@edemaine edemaine closed this Nov 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants