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

More infinite looping in CircularLinkedList (for v1.2.10) #55

Closed
OliverColeman opened this issue Aug 11, 2020 · 9 comments
Closed

More infinite looping in CircularLinkedList (for v1.2.10) #55

OliverColeman opened this issue Aug 11, 2020 · 9 comments
Labels

Comments

@OliverColeman
Copy link

Some examples: https://codesandbox.io/s/flatten-js-infinite-loop-181sh?file=/src/index.ts

alexbol99 added a commit that referenced this issue Aug 20, 2020
- unify is working after scaling of coordinates, subtract is not working yet
@alexbol99 alexbol99 added the bug label Aug 21, 2020
@alexbol99
Copy link
Owner

Still working on it

@jovankrajevski
Copy link

Do you have any suggestions on which types of polygons we should be avoiding so that we can avoid the infinite looping while you are working on this issue?

@OliverColeman
Copy link
Author

@alexbol99 Any chance of getting PR #56 merged? I'm currently using that branch on a webserver to avoid this bug crashing the webserver...

@jovankrajevski In case it's useful: you can use the branch for PR #56 from npm with npm i OliverColeman/flatten-js#preventinfiniteloop. Then you can catch the exception thrown when an infinite loops seems to be occurring and handle it gracefully.

@jovankrajevski
Copy link

I would prefer to see PR #56 merged tbh. imo it is better to have an error thrown, so that we could fall back on some other heuristic, than have the browser/server looping forever.

@alexbol99
Copy link
Owner

I would not fix it in this way. The reason of infinite loop is illegal merge of two polygon caused by numeric errors. I know how to prevent it in the way that it throw error before it comes to infinite loop. It just takes me some time to implement. Sorry for delay.

@alexbol99
Copy link
Owner

@jovankrajevski

Do you have any suggestions on which types of polygons we should be avoiding so that we can avoid the infinite looping while you are working on this issue?

Operation is stable when polygon has no almost zero length edges. Intersection between two edges when one of them is big and other has length closed to zero is numerically unstable. Try to filter your polygons from tiny edges that have no meaning, and the result will be fine.

@jovankrajevski
Copy link

@alexbol99 thanks. I actually solved my problem by just casting the coordinates to ints, but I will make sure that I do not have edges with length < 1. I am not sure if you are aware that this bug happens sometimes with floating point coordinates, and not just with edges that are really small, so I just wanted to point that out.

@alexbol99
Copy link
Owner

@jovankrajevski , @OliverColeman
Ok, I added infinite loop detection using infamous "turtle and rabbit" algorithm.
It throws an "Infinite loop" error if additional loop detected in the chain of edges supposed to form face.
Fix available in the version v1.2.11.
Best,
Alex

alexbol99 added a commit that referenced this issue Aug 26, 2020
@OliverColeman
Copy link
Author

Awesome, thanks for this :). I am wondering if it would be even better to put that code into CircularLinkedList.js rather than face.js? At the moment face.js is the only code that uses CircularLinkedList, but if in the future some other code uses it and causes an infinite loop, it would still be caught.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants