-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 fatal bug of the convolution calculation part of the Minkowski sum #8573
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, good job on spotting and fixing.
A more simple (and I guess efficient) fix is simply replacing Line 614:
- cycle.insert(curr,
+ after_next = cycle.insert(curr,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the code. We agree on the fix now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @efifogel
Minkowski_sum_2/include/CGAL/Minkowski_sum_2/Minkowski_sum_conv_2.h
Outdated
Show resolved
Hide resolved
Minkowski_sum_2/include/CGAL/Minkowski_sum_2/Minkowski_sum_conv_2.h
Outdated
Show resolved
Hide resolved
Minkowski_sum_2/include/CGAL/Minkowski_sum_2/Minkowski_sum_conv_2.h
Outdated
Show resolved
Hide resolved
BW, consider the Minkowski sum of two 2 segments, that is, two degenerate polygons. |
Successfully tested in CGAL 6.1-Ic-15 |
This pull-request was previously marked with the label |
Summary of Changes
Modify the convolution calculation part of the Minkowski sum. In the 'elimination of antennas' phase, before I fix it, only the antennas of the first segment were eliminated, skipping the possible subsequent antennas. For more details, please refer to the explanation in the issue. I modified it so that if a new antenna is merged, the newly merged antenna is used as
curr
for the next iteration; otherwise,after_next
is used ascurr
.Release Management