Skip to content

Fix circle edge intersection ordering - #699

Open
vera-rubin wants to merge 1 commit into
google:masterfrom
vera-rubin:fix/circle-intersection-ordering
Open

vera-rubin wants to merge 1 commit into
google:masterfrom
vera-rubin:fix/circle-intersection-ordering

Conversation

@vera-rubin

Copy link
Copy Markdown

Two edges that cross M at the same position can compare unequally. This returns +1 although the documented result is 0:

s2pred::CircleEdgeIntersectionOrdering(
    S2Point(.6, .8, 0), S2Point(.6, -.8, 0),
    S2Point(5. / 13, 12. / 13, 0), S2Point(5. / 13, -12. / 13, 0),
    S2Point(0, 1, 0), S2Point(.8, 0, -.6));

Different edge lengths can also reverse the ordering of distinct crossings. We account for each intersection vector's length in both floating point and exact comparisons, and correct the crossed-circle names in the preconditions. The new regression and existing predicate tests pass.

— ChipLab

Compare normalized intersection positions rather than raw dot products.
Different edge lengths can otherwise make coincident crossings unequal or
reverse the order of distinct crossings.

This branch has not been deployed

No deployments
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.

1 participant