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 the stackOverflow issue in #54 #64

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

madhephaestus
Copy link
Contributor

This is a fix to the stack overflow issue in the plane split function. If a plane that comes in that is not as flat as the fixed Plane.EPSILON value, then the algorithm would recourse until a crash.

This change measures the epsilon off of the incoming polygon to set the epsilon values accurately for each incoming polygon.

@CreamyCookie
Copy link

@madhephaestus When I enable this the icosahedron.intersect assert of the IcosahedronTest fails

@madhephaestus
Copy link
Contributor Author

This feature should be implemented in neuronrobotics/jcsg along with lots of additional features. I made this pr in hopes the upstream project was not abandoned, but alas, it seems the neuronrobotics fork seems to the the currently maintained fork.

@CreamyCookie
Copy link

CreamyCookie commented Mar 22, 2023

@madhephaestus Oh, maybe I should have mentioned.. I applied the changes to your fork, and there the test failed.

@madhephaestus
Copy link
Contributor Author

My fork already has this feature. This PR is intended to back-feed solutions to begs to the ubstream project, in case someone is stock on this fork for some reason. There is no need to apply this to my fork.

@CreamyCookie
Copy link

@madhephaestus Ah, I see, my bad

Thank you for your fork 👍

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.

None yet

2 participants