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

Add removeDuplicatePoints option from polygon-decomp 0.3.0 to Bodies.fromVertices() #639

Merged
merged 3 commits into from
Jan 12, 2021

Conversation

jmfd
Copy link
Contributor

@jmfd jmfd commented Jul 26, 2018

polygon-decomp has been updated and has a new method called removeDuplicatePoints. I found that using this can greatly reduce shapes that fail to get decomposed because of very sharp angles.

I am using the default value in the docs if none is provided, but in Hype I supply a value of 1.

I did not change the argument ordering as to help preserve backwards compatibility if anyone was using the minimumArea argument.

@liabru
Copy link
Owner

liabru commented Nov 16, 2018

Great to have the new feature, though this will change the default behaviour of Bodies.fromVertices? Thats fine if it's always an improvement I guess, but its hard to tell without trying a lot of inputs. Have you been using this is Hype? Does it always give better results?

@jmfd
Copy link
Contributor Author

jmfd commented Nov 16, 2018

I can't say too well the default value of 0.01 works out, but I can say using this method was a massive improvement to successful shape creation.

I was following the example of removeCollinear for the default value. As Hype always specifies a much lower precision value based (1) in its usage because we don't ever care about sub-pixel geometry, it wouldn't affect us if the default behavior of fromVertices did not change:

removeDuplicatePoints = typeof removeDuplicatePoints !== 'undefined' ? removeDuplicatePoints : false;

That said, I think using removeDuplicatePoints just generally makes sense.

@liabru liabru merged commit 947cb97 into liabru:master Jan 12, 2021
@liabru liabru mentioned this pull request Jan 18, 2021
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