Skip to content

Introduce self‑intersection normalization for polygons#22

Open
JimBobSquarePants wants to merge 58 commits intomainfrom
js/normalize-self-intersections
Open

Introduce self‑intersection normalization for polygons#22
JimBobSquarePants wants to merge 58 commits intomainfrom
js/normalize-self-intersections

Conversation

@JimBobSquarePants
Copy link
Member

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

This PR introduces PolygonClipper.RemoveSelfIntersections, a new opt‑in normalization step that converts self‑intersecting contours into a valid polygon representation using a positive fill rule. It is intentionally separate from the core boolean pipeline so existing workflows remain unchanged, but consumers can call it explicitly when their input geometry is not guaranteed to be simple.
The new method is especially useful for real‑world inputs such as stroked text outlines, compound paths, or imported artwork where contours may cross or touch themselves. By normalizing these shapes first, the main Martinez boolean operations can be applied reliably to inputs that would otherwise be invalid or produce undefined results.

CC @stefannikolei


// Exteriors are the even-depth contours.
List<int> externals = [];
for (int i = 0; i < count; i++)
Copy link
Contributor

Choose a reason for hiding this comment

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

You could join this with the for loop of line 317

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah we can merge this!


if (hasHoleInfo)
{
for (int i = 0; i < polygon.Count; i++)
Copy link
Contributor

Choose a reason for hiding this comment

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

This could probably also be joined with the for loop on l 459 to 467.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don’t think merging those loops is worthwhile or clean. The first loop exists to decide which containment mode to use (hole‑aware vs XOR). Until you’ve scanned all contours, you can’t safely choose the hole‑aware path. If you merge, you either:

  • Do partial XOR work and then throw it away if you later discover hole metadata, or
  • Add extra bookkeeping (cache externals + a restart), which doesn’t really save work.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah.

But does the first loop really do all iterations? There's a break statement in there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants