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

Regression when removing type casting in IsCollinear() #831

Closed
here-abarany opened this issue May 8, 2024 · 4 comments
Closed

Regression when removing type casting in IsCollinear() #831

here-abarany opened this issue May 8, 2024 · 4 comments

Comments

@here-abarany
Copy link

When updating to the latest revision of Clipper2, we encountered a regression due to #825 for removing the type casting in IsCollinear().

In this test case, we have four points that cross over in an hourglass shape. We call Execute(Union, EvenOdd) to "repair" this invalid polygon into a pair of triangles that touch at a point.

These values, which are intended for single-precision floats, work as intended:

Input (with AddSubject()):

-536870912, -536870912
536870912, -536870912
-536870912, 536870912
536870912, 536870912

Output (2 paths):

0, 0
-536870912, 536870912
536870912, 536870912

536870912, -536870912
-536870912, -536870912
0, 0

However, these values, which are intended for doubles, break with #825:

-1125899906842624, -1125899906842624
1125899906842624, -1125899906842624
-1125899906842624, 1125899906842624
1125899906842624, 1125899906842624

With the latest revision, the call to Execute() gives an empty result rather than two paths similar to the smaller integer values.

@AngusJohnson
Copy link
Owner

Aaron, thank you for your very clear and concise bug report.
I'll check it out and hopefully have a fix soon (<24hrs).

@here-abarany
Copy link
Author

Thanks. I should also clarify that I am using the C++ library.

@reunanen
Copy link
Contributor

reunanen commented May 9, 2024

Can reproduce. Yeah, with these inputs (which are exactly ±0x4000000000000) and using integer math, a*b wraps to 0, as does c*d. So perhaps an error in this direction is more critical than an error in the other direction after all (see the comment here).

@AngusJohnson
Copy link
Owner

This has been fixed in the latest repository revision.

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 a pull request may close this issue.

3 participants