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

[arrayTools] fix sectRect boundary case #3354

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

anthrotype
Copy link
Member

use '>' instead of '>=' when comparing intersection of two rectangles, so we don't skip boundary case when the two touch

Fixes #3352

Copy link
Collaborator

@justvanrossum justvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice I've done the same change in my JS port in Fontra, so that makes me +1.

@anthrotype anthrotype force-pushed the fix-sect-rect-edge-case branch 2 times, most recently from 417558c to 513867a Compare November 28, 2023 11:20
use '>' instead of '>=' when comparing intersection of two rectangles, so we don't skip boundary case when the two touch

Fixes #3352
@anthrotype
Copy link
Member Author

anthrotype commented Nov 28, 2023

@simoncozens I noticed that when making this change to sectRect, from no intersections, we now find 4 intersections for the test curves (from dad5bc6) that touch exactly at t=0.5, but they look like duplicates.

[Intersection(pt=(49.853515811264515, 74.99971389770508), t1=0.4990234375, t2=0.4990234375),
 Intersection(pt=(50.146484188735485, 74.99971389770508), t1=0.5009765625, t2=0.4990234375),
 Intersection(pt=(49.853515811264515, 74.99971389770508), t1=0.4990234375, t2=0.5009765625),
 Intersection(pt=(50.146484188735485, 74.99971389770508), t1=0.5009765625, t2=0.5009765625)]

I see that _curve_curve_intersections_t function has a precision parameter set to 1e-3, which is used for two slightly different things:

  1. when determining if an intersection was found, by comparing it against the area of the bounds rectangle
    if rectArea(bounds1) < precision and rectArea(bounds2) < precision:
        return [(midpoint(range1), midpoint(range2))]
  1. when rounding the ts in order to prune duplicates:
    unique_key = lambda ts: (int(ts[0] / precision), int(ts[1] / precision))
    seen = set()
    unique_values = []

    for ts in found:
        key = unique_key(ts)
        if key in seen:
            continue
        seen.add(key)
        unique_values.append(ts)

Maybe we need to use precision*precision (squared) when checking the areas (given area itself is not a single-dimension dinstance but... an area), so that we then obtain more precise intersections whose ts will get rounded off with given precision and will become equal and deduped?

@anthrotype
Copy link
Member Author

basically this bfd874a

@simoncozens
Copy link
Collaborator

This stuff (any kind of cubic-cubic intersection logic) is horribly complicated and full of floating-point imprecision and edge cases and I've basically hit the limit of my knowledge here. (Actually I hit the limit of my knowledge here.) Raph has been working on a new algorithm for Kurbo but I'm not sure what state that's in.

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 this pull request may close these issues.

Curve-curve intersection edge cases
3 participants