aws-cqc / aws-cqc/DeviceLayout.jl

clip-based operations silently drop clockwise-wound polygons

Open
#234 2 comments 0 reactions 0 assignees View on GitHub
2.0 bug
Dominant language
Julia
Stars
67
Forks
14
Avg merge
1d 16h
Merged PRs (30d)
21

Description

## Summary

`union2d` and `union2d_curved` (and all other `clip`-based operations) silently drop clockwise-wound polygons with no error or warning. A CW polygon passed to `union2d` simply disappears from the output.

This happens because `clip` defaults to `PolyFillTypePositive`, which treats CW contours (winding number = -1) as holes. When a standalone CW polygon is passed (not contained within an outer CCW contour), it is effectively a "hole in nothing" and gets discarded.

While the docstring mentions "Clockwise polygons are holes", this behavior is a significant footgun because:
1. Users can easily construct CW polygons (e.g., via `Polygon(reverse(points(p)))`, coordinate transformations, or importing from external sources)
2. The operation completes silently -- no error, no warning
3. The polygon simply vanishes from the result

## Minimal Reproducible Example

```julia
using DeviceLayout
using DeviceLayout.Polygons
using DeviceLayout.Curvilinear

# Two non-overlapping rectangles
r1 = Polygon(Point.([(0,0), (10,0), (10,5), (0,5)])) # CCW
r2 = Polygon(Point.([(20,0), (30,0), (30,5), (20,5)])) # CCW
r2_cw = Polygon(reverse(points(r2))) # CW (reversed)

# Verify winding
shoelace(p) = let pts = points(p)
sum(getx(pts[i])*gety(pts[mod1(i+1,length(pts))]) -
getx(pts[mod1(i+1,length(pts))])*gety(pts[i]) for i in 1:length(pts)) / 2
end
println("r1 signed area (CCW>0): ", shoelace(r1)) # 50.0
println("r2_cw signed area (CW<0): ", shoelace(r2_cw)) # -50.0

# BUG: CW polygon silently dropped
println(length(to_polygons(union2d([r1, r2_cw])))) # 1 (expected 2!)
println(length(union2d_curved([r1, r2_cw]))) # 1 (expected 2!)

# Control: both CCW works fine
println(length(to_polygons(union2d([r1, r2])))) # 2 (correct)
```

## Observed behavior

| Input | `union2d` result | Expected |
|-------|-----------------|----------|
| `[ccw, cw]` (non-overlapping) | 1 region | 2 regions |
| `[cw, cw]` (non-overlapping) | 0 regions | 2 regions |
| `[cw]` (single polygon) | 0 regions | 1 region |
| `[ccw, ccw]` (non-overlapping) | 2 regions | 2 regions |

## Root cause

In `src/clipping.jl`, all `clip`-based operations default to `PolyFillTypePositive`:

```julia
function clip(op::Clipper.ClipType, s::AbstractVector{Polygon{T}}, c::AbstractVector{Polygon{T}};
pfs::Clipper.PolyFillType=Clipper.PolyFillTypePositive,
pfc::Clipper.PolyFillType=Clipper.PolyFillTypePositive) where {T}
```

With `PolyFillTypePositive`, only regions with positive winding number (CCW) are considered "filled". CW contours have winding number -1 and are treated as holes. A standalone CW polygon (not inside any CCW contour) becomes a "hole in nothing" and is discarded.

## Affected functions

All `clip`-based functions share this behavior:
- `union2d` / `union2d_curved`
- `difference2d` / `difference2d_curved`
- `intersect2d` / `intersect2d_curved`
- `xor2d` / `xor2d_curved`

## Suggested fix

Consider one or more of:

1. **Emit a warning** when a CW polygon is passed to `union2d` (or other operations) that would be silently dropped
2. **Auto-normalize** input polygon winding to CCW before clipping (users who intentionally want holes can use the `pfs`/`pfc` kwargs)
3. **Use `PolyFillTypeNonZero`** or `PolyFillTypeEvenOdd` as the default, which treats both CW and CCW as filled regions (but changes hole semantics)

Option 1 (warning) is the least invasive. Option 2 is probably the best UX for `union2d` specifically, since "union of geometry" should not depend on winding order.

Contributor guide

Open the contributing guide

Research direction

Start in src/clipping.jl at clip and inspect how PolyFillTypePositive is passed through union2d, difference2d, intersect2d, and xor2d, including curved variants. Run the minimal reproduction from the issue and compare the available fill-type or winding-handling options. Done should include regression coverage showing standalone clockwise polygons are not silently dropped while intended hole behavior remains defined.

Written by the indexing model from the issue text.

Assessment

Tech stack
julia
Domain
computer-graphics
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.