Skip to content

Add provenance-based curve recovery#232

Open
gpeairs wants to merge 10 commits into
mainfrom
gp/provenance-curve-recovery
Open

Add provenance-based curve recovery#232
gpeairs wants to merge 10 commits into
mainfrom
gp/provenance-curve-recovery

Conversation

@gpeairs

@gpeairs gpeairs commented Jun 2, 2026

Copy link
Copy Markdown
Member

Boolean operations (union2d, difference2d, etc.) discretize curved geometry to polygons
before passing them to the Clipper library. Normally, the original curves (arcs, splines)
are lost in this process. The recover_curves function and its convenience
wrappers (difference2d_curved, union2d_curved, intersect2d_curved, xor2d_curved)
track each input curve's discretized integer-grid footprint and substitute the original
curve back into the result wherever that footprint survived the boolean operation intact.

The curve-preserving variants return a Vector{CurvilinearRegion} rather than a single
ClippedPolygon. Each region in the vector corresponds to one outer contour in the clipped
result (the disjoint pieces each become a separate region). For example:

# Standard clipping discretizes curves to polygons:
result = difference2d(a, b)  # ClippedPolygon

# Curve-preserving variant recovers arcs where possible:
regions = difference2d_curved(a, b)  # Vector{CurvilinearRegion}, arcs preserved

This required moving some of the SolidModel (rounded Polygon -> CurvilinearPolygon) rendering pipeline out of the SolidModels module and into src/render/ so that it's available to curvilinear.jl. I think that's the right place for this code, and eventually we'll want to include curvilinear.jl before src/render/ so we can put all the rendering code in there. Either way, the direct Rounded Polygon -> Polygon code will eventually be replaced with the CurvilinearPolygon intermediate as we unify our rendering pipelines. In the future we'll want to move more like this out of SolidModels that but this was the minimal change to get this working with Rounded polygons.

Current limitations:

  • A curve is recovered only if its entire discretized run survives
    the boolean operation with exact integer equality. If the operation cuts through a curve
    (e.g., a straight edge crossing an arc's interior), that curve falls back to a polyline. This will be addressed in a follow-up PR
  • Curves can only be recovered on CurvilinearRegion/CurvilinearPolygon,
    Path nodes, and Rounded-styled Polygon/Rectangle entities. Rounded applied to
    other entities (like ClippedPolygon), styled Curvilinear entities, and nested styles do not yet support curve recovery.
    Those will require more refactoring of the rendering pipeline, then a minimal follow-up PR once that's done.
  • No layerwise and tiled versions -- these will be added in another follow-up PR

I also experimented with arc recognition without provenance, but I found it very difficult to make robust.

@codecov

codecov Bot commented Jun 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.79006% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/render/render.jl 92.85% 3 Missing ⚠️
src/curve_recovery.jl 99.26% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@gpeairs gpeairs requested a review from laylagi June 5, 2026 14:34
@ybrightye

Copy link
Copy Markdown

Bug

to_polygons(::CurvilinearPolygon) produces incorrect geometry (duplicate paths) or throws AssertionError when called on output from difference2d_curved that went through partial curve recovery.

Minimal Reproducible Example

using DeviceLayout
using DeviceLayout.PreferredUnits
using DeviceLayout.Polygons
import DeviceLayout: to_polygons, styled

# Create a rounded rectangle (10μm corner radius)
rect = centered(Polygons.Rectangle(60000nm, 40000nm))
rounded_rect = styled(rect, Polygons.Rounded(10000nm))

# A cutter that slices horizontally through the top arcs
cutter = Polygons.Rectangle(80000nm, 10000nm) + Point(-40000nm, 14000nm)

# difference2d_curved triggers partial arc recovery for the cut arcs
result = difference2d_curved(rounded_rect, cutter)
ext = result[1].exterior

# BUG: to_polygons fails
to_polygons(ext)
# => AssertionError: Curve 1 must end at point 3!

Root Cause

Two related issues in curve_recovery.jl:

  1. _reverse() in curvilinear.jl produces unsorted curve_start_idx: When Clipper outputs CW-wound holes, substitute_curves calls _reverse() to flip to CCW. The reversal transform scrambles the ordering of curve_start_idx. Since to_polygons iterates curves sequentially assuming sorted indices, unsorted indices produce garbage vertex ranges (empty or reversed slices of .p).

  2. Partial matching leaves residual vertices: When match_run_partial recovers only a sub-arc (offset > 1), the unmatched head/tail vertices from the original arc discretization remain in .p. These create straight-line chord segments that duplicate the recovered sub-arc geometry.

Impact

  • to_polygons (GDS export): incorrect geometry or assertion failure
  • _add_loop! (OCC/Gmsh mesh rendering): NOT affected — uses findfirst(isequal(i), curve_start_idx) per edge, which is order-independent

Suggested Fix

Sort curve_start_idx (and reorder curves to match) in the CurvilinearPolygon constructor:

function CurvilinearPolygon(p, curves, curve_start_idx)
    perm = sortperm(curve_start_idx)
    new(p, curves[perm], curve_start_idx[perm])
end

This is the most defensive fix as multiple code paths (to_polygons, _collect_provenance!) assume sorted indices.

For the residual vertex issue, match_run_partial should additionally drop contour vertices corresponding to the unmatched head/tail of the original arc run.

@ybrightye

Copy link
Copy Markdown

Bug

union2d_curved (and all *_curved boolean ops) throws a MethodError when an element has a Rounded style wrapping an inner StyledEntity whose recursive _as_entities resolution produces a plain polygon (not a CurvilinearPolygon).

The simplest trigger is Rounded(MeshSized(rect)) — a common pattern when applying both mesh sizing and corner rounding to the same element.

MRE

using DeviceLayout, DeviceLayout.Polygons, DeviceLayout.Curvilinear
using Unitful: μm

rect = Rectangle(10.0μm, 5.0μm)

# Works: MeshSized wrapping Rounded (generic unwrap strips MeshSized, then Rounded dispatches)
a = styled(styled(rect, Rounded(1.0μm)), MeshSized(0.5μm))
@assert length(union2d_curved([a])) == 1  # OK

# FAILS: Rounded wrapping MeshSized — inner _as_entities returns a Rectangle, not CurvilinearPolygon
b = styled(styled(rect, MeshSized(0.5μm)), Rounded(1.0μm))
union2d_curved([b])  # MethodError: Cannot convert Rectangle to CurvilinearPolygon

Expected behavior

union2d_curved([b]) should produce a single CurvilinearRegion — the rectangle with rounded corners (the Rounded style applied to the resolved inner geometry, which is just a plain rectangle after MeshSized is stripped).

Root cause

In src/curve_recovery.jl, the dispatch at line ~896:

function _as_entities(
    p::StyledEntity{T, <:StyledEntity{T}, <:Polygons.Rounded}
) where {T}
    sty = p.sty
    inner_cpolys = _as_entities(p.ent)  # recursively resolve inner
    out = CurvilinearPolygon{T}[]
    for cpoly in inner_cpolys
        if cpoly isa CurvilinearPolygon{T}
            # ... round the curvilinear polygon ...
            push!(out, rounded)
        else
            push!(out, cpoly)  # BUG: tries to push Rectangle into Vector{CurvilinearPolygon}
        end
    end
    return out
end

When _as_entities(p.ent) resolves to a plain Polygon/Rectangle (because the inner style was MeshSized which just strips away), the else branch attempts push!(out::Vector{CurvilinearPolygon{T}}, cpoly::Rectangle{T}) which fails because there's no convert(CurvilinearPolygon, Rectangle).

Suggested fix

The else branch should apply rounding to the non-curvilinear polygon, mirroring what the StyledEntity{T, <:Polygon, <:Rounded} dispatch does:

else
    # Inner entity resolved to a plain polygon — apply Rounded to it directly
    if cpoly isa Union{Polygon{T}, Polygons.Rectangle{T}}
        rounded = round_to_curvilinearpolygon(
            cpoly, radius(sty);
            min_side_len=sty.min_side_len,
            corner_indices=cornerindices(cpoly, sty),
            min_angle=sty.min_angle
        )
        push!(out, rounded)
    else
        # Fallback: push as-is (may need widening the output vector type)
        push!(out, cpoly)
    end
end

Affected combinations

Nesting Result
Rounded(rect) OK
Rounded(Rounded(rect)) OK
MeshSized(Rounded(rect)) OK
Rounded(MeshSized(rect)) FAILS
Rounded(MeshSized(Rounded(rect))) OK (inner resolves to CurvilinearPolygon)
MeshSized(Rounded(Rounded(rect))) OK

The bug is specifically triggered when the recursive inner resolution produces a non-curvilinear entity that then needs rounding applied.

Branch

Found on gp/provenance-curve-recovery at commit 6a19e5f.

@gpeairs

gpeairs commented Jun 11, 2026

Copy link
Copy Markdown
Member Author

I've fixed the first issue with sorted curve start indices. I'm not sure what match_run_partial refers to -- partial matching is planned for later and isn't part of this PR. Is there an issue with curves that return with :clipped rather than :recovered?

On the second issue, this is what I mean by

Curves can only be recovered on CurvilinearRegion/CurvilinearPolygon,
Path nodes, and Rounded-styled Polygon/Rectangle entities. Rounded applied to
other entities (like ClippedPolygon), styled Curvilinear entities, and nested styles do not yet support curve recovery.
Those will require more refactoring of the rendering pipeline, then a minimal follow-up PR once that's done.

We'll get this fixed more or less for free once the refactor is done -- that is, the SolidModel pipeline already strips non-rendering styles by that point, and that's how those nested styles will be handled in the future. I'd also note that the inner Rounded curves you tested are not being recovered despite not erroring. So I'm going to say this is out of scope, although maybe it's an argument that this should wait for the refactor.

@ybrightye

Copy link
Copy Markdown

Thanks for fixing the sorted indices issue! Indeed match_run_partial is from a local extension I've been carrying on top of this branch (partial sub-arc matching), it's not part of this PR -- apologies for the confusion.

Bug #2 (the Rounded(MeshSized(rect)) MethodError) still applies but I see you've scoped it out pending the render refactor, which makes sense.

@ybrightye

Copy link
Copy Markdown

bug with discretize_curve(ConstantOffset{Straight})

Summary

discretize_curve applied to a ConstantOffset{T, Straight{T}} (the constant offset of a straight line = another straight line) uses the generic marching algorithm, producing ~101 evenly-spaced points. This should produce exactly 2 points (start and end), since the result is geometrically a straight line.

This causes union2d_curved to output dense polylines (53+ short edges at ~1μm spacing) on straight CPW boundaries when the path runs at a non-axis-aligned angle.

Minimal reproducible example

using DeviceLayout, DeviceLayout.PreferredUnits
import DeviceLayout.Paths

path = Path(Point(0.0, 0.0)μm, α0=45°)  # diagonal triggers the bug
sty = Paths.SimpleCPW(10.0μm, 6.0μm)
Paths.straight!(path, 115.0μm, sty)

nodes = collect(Paths.Node, path)
entities = vcat(DeviceLayout.Curvilinear._as_entities.(nodes)...)
regions = DeviceLayout.union2d_curved(entities)

for (i, r) in enumerate(regions)
    println("Region $i: $(length(r.exterior.p)) pts")
end

Expected: Region 1: 4 pts / Region 2: 4 pts (clean rectangles)
Actual: Region 1: 75 pts / Region 2: 75 pts (dense polylines with 53 edges at 1150nm)

Control: Same code with α0=0°4 pts each (axis-aligned points are exactly collinear on Clipper's integer grid, so Clipper simplifies them).

Root cause

_collect_provenance! calls discretize_curve(c, atol) for each curve in a CurvilinearPolygon. For ConstantOffset{T, Straight{T}}, this dispatches to the generic marching algorithm at atol=1nm, producing ~101 uniformly-spaced points along the straight line.

These 101 points enter Clipper's integer grid. At diagonal angles, floating-point→integer rounding introduces sub-nanometer deviations from perfect collinearity. Clipper's output preserves all non-collinear points, so ~75 of the 101 survive in the final polygon (some consumed at polygon merge junctions).

At axis-aligned angles (0°, 90°), all points happen to land on the same grid row/column → exactly collinear → Clipper simplifies to 2 endpoints. This makes the bug angle-dependent.

Impact

  • Mesh quality: Each surviving point becomes an individual OCC Line entity. These create poor mesh element aspect ratios near CPW boundaries.
  • Asymmetric density: When adjacent CPW elements merge at junctions, one wall's point run may be partially consumed (different junction topology on inner vs outer wall). This creates asymmetric vertex density between parallel CPW walls, which can crash Gmsh's constrained Delaunay triangulation.
  • Performance: 101 unnecessary points per straight edge × thousands of edges = significant overhead in provenance matching and OCC geometry creation.

Proposed fix

Add a specialized method that short-circuits for constant offsets of straight segments:

function discretize_curve(seg::ConstantOffset{T, Straight{T}}, atol; kwargs...) where T
    return [seg(zero(pathlength(seg.seg))), seg(pathlength(seg.seg))]
end

A 2-point provenance run trivially survives any Clipper operation (both points are polygon corner vertices that Clipper always preserves). match_run will find the 2-point sequence in the output, and the curve will be correctly marked as :recovered — though for a straight line this is cosmetic, the key benefit is that no dense intermediate vertices are injected into the polygon.

More broadly, any Segment subtype whose discretize_curve output is guaranteed to be a straight line (constant offset of a straight, zero-curvature BSpline, etc.) should return just 2 points.

Environment

@ybrightye

Copy link
Copy Markdown

Bug: union2d_curved drops Rounded when wrapped in StyleDict

Summary

union2d_curved correctly expands styled(poly, Rounded(r)) into Turn curves, but silently drops the rounding when the style is wrapped in StyleDict: styled(poly, StyleDict(Rounded(r))).

Since StyleDict is what you get when using inverse_selection=true or a p0 exclusion list (i.e., every component that selectively rounds corners), this causes all finger-tip arcs in ParallelIDC_cutout, SeriesIDC_cutout, etc. to be lost through union2d_curved / difference2d_curved.

MRE

using DeviceLayout, DeviceLayout.PreferredUnits
import DeviceLayout.Paths

finger_w = 5.0µm
finger_l = 50.0µm
finger_gap = 5.0µm
lead_gap = 6.0µm
npairs = 2

fingers_cell = Cell("fingers", nm)
DeviceLayout.SimpleShapes.interdigit!(fingers_cell, finger_w, finger_l, finger_gap, lead_gap, npairs, true, GDSMeta())
polyfinger_outer = elements(DeviceLayout.flatten(fingers_cell))[1:npairs]
polybound = bounds(fingers_cell)
enclosing = Rectangle(polybound.ll, polybound.ur)
comb = difference2d(enclosing, union2d(vcat(polyfinger_outer)))

# Works: 16 pts, 8 Turns
r1 = union2d_curved([styled(comb, Rounded(1.0µm))])
# → 16 pts, 8 Turns ✓

# Broken: 8 pts, 0 Turns
r2 = union2d_curved([styled(comb, StyleDict(Rounded(1.0µm)))])
# → 8 pts, 0 Turns ← BUG

Root cause

_collect_provenance! in src/curve_recovery.jl has a dispatch for StyledEntity with a plain Rounded style that expands it into a CurvilinearPolygon with Turn curves before discretizing. But when the style is StyleDict{Rounded}, this dispatch is missed — the entity falls through to the ClippedPolygon path which sees a sharp-cornered polygon with no arcs to detect.

Impact

Any component using StyleDict(Rounded{T}(; abs_r=r, p0=..., inverse_selection=true)) (which is the standard pattern for selective rounding) loses all rounded corners through union2d_curved / difference2d_curved. In the full-chip SolidModel render, this manifests as ~20 discretized polyline arcs per IDC hole instead of native OCC circle arcs.

Suggested fix

Add a _collect_provenance! dispatch (or adjust the existing StyledEntity dispatch) to handle StyleDict{<:Rounded} the same way it handles plain Rounded — expand the style into a CurvilinearPolygon before passing to Clipper.

@ybrightye

Copy link
Copy Markdown

BUG: union2d_curved loses arcs from rounded interdigitated polygons

Summary

When a ClippedPolygon with discretized arc vertices (from difference2d of two Rounded shapes) passes through union2d_curved, _detect_arc_provenance! fails to recover some arcs. Adjacent quarter-circles separated by short straight edges get lumped into a single run that fails circle fitting.

MRE

using DeviceLayout, DeviceLayout.PreferredUnits
import DeviceLayout.Paths

cell = Cell("f", nm)
DeviceLayout.SimpleShapes.interdigit!(cell, 5µm, 50µm, 5µm, 6µm, 2, true, GDSMeta())
bb = bounds(cell)
outer = elements(DeviceLayout.flatten(cell))[1:2]
inner = elements(DeviceLayout.flatten(cell))[3:end]
comb = styled(difference2d(Rectangle(bb.ll, bb.ur), union2d(outer)), Rounded(1µm))
result = styled(difference2d(comb, styled(union2d(vcat(inner, Rectangle(bb.ur, Point(bb.ur.x+2.5µm, bb.ll.y)))), Rounded(1µm))), MeshSized(5µm, -1.0))

cs = CoordinateSystem("a", nm); render!(cs, result, GDSMeta(1,0))
cs2 = CoordinateSystem("b", nm); push!(cs2.refs, sref(cs, rot=90°)); DeviceLayout.flatten!(cs2)

regions = union2d_curved(cs2.elements)
# Expected: all arcs recovered
# Actual: 5 recovered, 7 lost as discretized polylines

Root cause

_detect_arc_provenance! in src/curve_recovery.jl finds short-edge runs using threshold = 6000 (line ~112). When two quarter-circle arcs (18 edges × ~87nm) are separated by a gap edge shorter than 6µm, the gap gets included in the run. The combined run (arc + gap + arc = ~37 edges) fails _fit_arc_for_provenance because it's not a single circle.

Suggested fix

When _fit_arc_for_provenance fails on a long run, split at outlier edges (>10× the median edge length in the run) and retry each sub-run independently. The gap edge (5000nm) is 57× the arc edges (87nm) and would be cleanly identified as a split point.

@gpeairs

gpeairs commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

bug with discretize_curve(ConstantOffset{Straight})

Fixed by using the correct pathtopolys(::Node) (which then dispatches based on linear segment+style combination) rather than pathtopolys(seg, sty) directly

Bug: union2d_curved drops Rounded when wrapped in StyleDict

BUG: union2d_curved loses arcs from rounded interdigitated polygons

Expected, out of scope / pending refactor (actually I'm surprised any arcs on rounded ClippedPolygons are recovered) such that there's only one code path for turning arbitrary nested styles on arbitrary entities to CurvilinearPolygon/Region

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.

2 participants