-
Notifications
You must be signed in to change notification settings - Fork 20
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
Union bug #3
Comments
Indeed, interesting, thanks! I'll have to dig back through the algorithm to try debugging it, haven't looked at the stuff for quite long. |
Reopening this, as my attempt at fixing it was too optimistic and actually has broken more than it fixed, unfortunately (see #4, especially some discussion, and #5). Also, from some further discussion @mourner had with F. Martínez, it seems that the original algorithm as described in the paper wasn't supporting polygons with overlapping edges, such as those above. So, the hunt for a solution is still open. To give some visual aid, the input is as below: |
This does not solve the problem, but here is a potential workaround for this issue that may help some people. Instead of trying to do a union of many contours at the same time, treat each contour as its own polygon and then do the union of those polygons. So the described example could be solved with the following code:
Which produces the following output:
Here is an SVG of the output (scaled up by 10x) to illustrate graphically that it does work as a workaround. I hope this helps someone... |
I am using this lib in production and I am getting WAY too many errors because of this. About 25% of my unions fail (which sucks). Here is an example of a union which failed (and shouldn't have). The format looks a little different because I am using my own point system, but you get the idea.
and
produces
I have hundreds of similar examples. This is such a problem I am considering just writing a Cgo wrapper around GPC to try to solve this. :( Any ideas for solving this? |
Sorry, still can't help you :( a serious analysis of the underlying algorithm & paper is required to find a correct solution. That said, thanks for the new testcases! They might hopefully become useful at some point in future. |
Ya no worries. This is what I was expecting. I was reading some other stuff on this algorithm (sorry I can't find the link now), but it seemed to imply that this algorithm has problems when two polygons share an edge. Unfortunately this is often the case for my polygons. I may actually see if I can modify my software to make the lines 0.001mm apart and see if that reduces the chance of union failing. If I can reduce my error frequency by doing this I will report back. My software generates CAD, so I have to be careful about fudging the user's numbers too much, but I think I can get away with 0.001mm. Thanks for the hard work with this code... :) |
My first round of testing seems to be showing that my workaround is working. Here is the problem that I think exists and how I am currently handling it. I think the problem is when the two objects being unioned share an edge. My failure rate when the two objects shared at least one edge was very large (like ~25-35%). I have modified my code to make sure that the two objects don't share edges. I basically changed the way one of my objects draws so it is 0.001mm smaller than it used to be. Since this is well below the tolerances of the cutting tool, this should not affect the resulting part (actually, in this case I know that the union will result in the correct size part regardless). By doing this I have made sure that the edges that would have been shared are actually 0.001mm apart, which seems to be enough for this library to correctly determine the union. Hope this helps others who are having similar problem... Good Luck... :) |
I've looked into it some, and I'm pretty sure the issue is in the connector.add() and pointchain.linkSegment() functions. My understanding is at the point in the algorithm that these functions are called, the polygons are in the form of a bunch of segments without any information about the original relationships among the segments. As each segment is added, these functions check whether the segment lines up with the beginning or the end of an existing pointchain. If it lines up, the segment is added to the pointchain, otherwise it is used to start a new pointchain. The problem arises when you have more than two segments that converge on a single point. In this case, the first segment to be added extends the point chain and any subsequent ones get left out as unclosed polygons and don't form part of the final result. My guess is that within the context of this algorithm, the solution would lie in making sure that the segments get added to the pointchain in the correct order, although I'm not sure whether that's possible without messing up the order for the rest of the algorithm. Another way to go about it could be to redesign the result constructor to consist of a graph structure with nodes and edges---where each node can have more than two edges attached to it---instead of point chains and then have some way to construct the appropriate polygon after all the segments have been added. |
I have quite a bit of experience with this problem and I just have not gotten around to implementing a better solution or changing algorithms. If you want a small program which can consistently output the problem for testing, you can check my test program here: https://github.com/swill/polycliptest I actually wrote a small monitoring software (https://github.com/swill/slackd) for my production application (http://builder.swillkb.com) in order to better understand how often this problem arises and I have collected hundreds of failing cases which I plan to validate are working when I rewrite this aspect of my code. If anyone spends time on this and wants some data to validate their solution, let me know and I can help provide some cases or you can just use my |
There is now a potential fix for this in #9 . Basically, I added a preprocessor that adds all of the segments to a graph structure before adding them to the event queue. If, when adding a segment to the graph:
With this fix, however, there is still one failing test. This test is basically a triangle that loops around itself twice. The text next to the test states that the result of this should be a nil polygon, because the second loop cancels out the first loop. The way I've designed the preprocessor this situation is just considered a single triangle, which makes sense to me, but if it's decided that the test is correct I should be able to fix the situation by changing condition 1 to also delete the existing segment. This probably adds some processing time to the algorithm, but probably not a lot because most of the time appears to be spent in the meat part of the algorithm rather than in this setup part. |
) The sweep line algorithm computes the [inside] state of an endpoint and the [inout] state of its corresponding segment based on the previous endpoint in the sweep line. When an endpoint divides its predecessor, the context for the endpoint thus changes, and the endpoint must be re-swept to compute the (potentially changed) [inside] and [inout] states of that endpoint.
prints:
which looks incorrect...
(4 touching rectangles, the upper right one is lost after union)
The text was updated successfully, but these errors were encountered: