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

Reduce precision to eliminate duplicate points from InflatePaths #91643

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

charlieb
Copy link

@charlieb charlieb commented May 7, 2024

Fixes #91607 by locally reducing the precision of InflatePaths to the Clipper2 default. This prevents duplicate points from being generated and thus it doesn't cause decompose_polygon_in_convex to fail.
This also replaces pull request 91627

@charlieb charlieb requested a review from a team as a code owner May 7, 2024 03:42
core/math/geometry_2d.cpp Outdated Show resolved Hide resolved
@akien-mga akien-mga requested review from rburing and smix8 May 7, 2024 06:07
@akien-mga akien-mga added this to the 4.3 milestone May 7, 2024
@rburing
Copy link
Member

rburing commented May 7, 2024

I'd like to restore this to the value it was before my PR

which accidentally increased the precision.

Before that PR it was 0.25f * (real_t)SCALE_FACTOR where SCALE_FACTOR = 100000.0, and now we have the constant PRECISION = 5 which is related to SCALE_FACTOR by SCALE_FACTOR = pow(10, PRECISION), so we should now use 0.879588001734407 * PRECISION (or rounded 0.88 * PRECISION) to restore the previous value, since $$(\log(10^5 \cdot 0.25)/\log(10)) / 5 \approx 0.879588001734407.$$

Please test if that also solves the problem.

Edit: Actually 0.25 * SCALE_FACTOR was used as the arc_tolerance.

@charlieb
Copy link
Author

charlieb commented May 7, 2024

@rburing
The InflatePaths interface specifies an int for the precision parameter so 4.8 isn't going to work unless I pull the InflatePaths function apart like I did in this branch.

I'm wondering if it's possible to just change the #define PRECISION to 4 instead of 5 and call it done? Do the other functions really need 0.00001 which would be equivalent to 10 microns?

@akien-mga akien-mga modified the milestones: 4.3, 4.4 Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convex decomposition fails for offset_polyline of simple Curve2D
3 participants