-
Notifications
You must be signed in to change notification settings - Fork 280
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
Offset produces incorrect output #593
Comments
So you're negative offsetting by approaching 3 times the radius of your circle. Yes, ideally you should be getting an empty solution. |
@AngusJohnson But there are other things:
I think if you don't feel like this scenario is frequently used, maybe all these expensive checks can be activated by additional flag (turned off by default)? |
If you decide to implement this I can help you with testing, I have huge sets of inputs and outputs dumped for Clipper1, I can compare performance and results of Clipper2 and Clipper1 in offsetting. |
With negative offsets, I believe at one time I did check path bounds to ensure offsetting wasn't excessive (ie relative to the path so that path would disappear in the solution). This doesn't seem to be in the current implementation and I probably removed that check because mostly it was unnecessary, nor was it foolproof in preventing these rare offsetting artefacts. Anyhow, I could be persuaded to reintroduce this additional checking. |
One important algorithm for removing small details is:
this depends crucially on negative offset removing all small pieces. In our testing Clipper1 handles this case well. |
Another problem is that user may never guarantee thickness of his input polygons (because input may come from some human, or from another noisy algorithm), and if at least 1 polygon out of 10 is too thin and trashes result of whole operation for the rest 9 polys - it is bad. |
And actually you can even reproduce this with positive offset, if you offset polygon with small holes (holes will have to collapse correctly). |
Replace the following: Clipper2/CPP/Clipper2Lib/src/clipper.offset.cpp Lines 355 to 360 in 6222af4
with:
|
@AngusJohnson |
Second example (maybe a duplicate of previous issue) Unfortunately it is huge, so I will attach it as txt file. |
|
@AngusJohnson |
I will play more with collapsing tomorrow, but so far it looks good. |
|
|
Yep. I've amended the code ready for the next upload. |
It may be possible, but I don't know how. |
Noted, thanks. That needs fixing. |
This is very informal definition, especially in this case.
The most "Simplified" input I can give you is this one, I cannot simplify it more: |
To be fair, it seems offsetter in Clipper2 is MUCH faster than in Clipper1, but Clipper1 never generates that kind of noise. |
I will investigate further. |
There is still a bug in the new bounds check implementation, you need to check both the heigth and width of the bounding box. I can comfirm this fixes all cases of this problem I have in my unit test suite, that previously did not pass. I do check this by comparing the bounding boxes of the original poylgon and the offset one polygon to be smaller/bigger than expected with this offset whithin arcTolerance (JoinType.Round). fixed:
currently:
|
Sebastian, I don't understand what exactly you're changing and how this relates to the discussion above. Please explain. |
@AngusJohnson |
Yep, give it another go. |
@AngusJohnson |
…ish in the output), not only test the bounding box width but also the height. If either dimension is smaller than 2 times offset, the group will vanish when contracting. Suspected to fix issue AngusJohnson#593. All tests passed locally in C# and C++. Delphi is missing yet, I am not familiar with that language.
I'm sorry, I have now created a pull request for the proposed bug fix. I have tested C# and C++, don't know Delphi. |
@olologin adding to the discussion:
I have verified this in my code like this:
|
Are these artifacts still present (please check the latest commit)? |
…ish in the output), not only test the bounding box width but also the height. (#651) If either dimension is smaller than 2 times offset, the group will vanish when contracting. Suspected to fix issue #593. All tests passed locally in C# and C++. Delphi is missing yet, I am not familiar with that language.
@AngusJohnson |
Hi, could you please check the attached test, something is wrong with offset:
The input is red circle, and I dont see any obvious self-intersection there, the output are these two green things
The text was updated successfully, but these errors were encountered: