Curve-recovery additions: _as_entities for Node/StyledEntity/Rounded ClippedPolygon, silent-loss warning, sort-tolerant walks, opt-in partial recovery

[ybrightye]

Hey @gpeairs — this is the follow-up branch to #232 with curve-recovery additions for our fast-mesh / digital-twin pipeline. Marking ready for review now that julia-format is green.

Pushing as a PR against your gp/provenance-curve-recovery (not main) because a few of these might fit naturally into #232 rather than land as their own PR — happy to rework, split, squash, or rebase however works best.

Summary of the 8 commits

#	Commit	What it does
1	773387a	_as_entities(::Paths.Node) so a Node-wrapped curve recovers through booleans; ctor fix for CurvilinearPolygon{T}
2	a7ce027	_as_entities(::StyledEntity) default — unwraps style wrappers (e.g. MeshSized) so a wrapped Node/Rounded-Polygon/CurvilinearPolygon still curve-recovers (mirrors to_primitives(::StyledEntity) on the render side)
3	55bd4a7	Tiny CurvilinearPolygon(::Polygon{T}) ctor fix (was calling the wrong 1-arg form)
4	81bd91a	Qualify Paths.ContinuousStyle in _as_entities(::Paths.Node) (was UndefVarError on the zero-length-node path)
5	153c8dc	_as_entities(::StyledEntity{ClippedPolygon, Rounded/StyleDict}) — Rounded post-clip rounded polygons now recover as exact fillet arcs by reusing your render-side SolidModels.to_curvilinear_regions
6	4563d75	Warn-once when recover_curves silently discretizes a curve-bearing entity (Kasa-fit circle detector + per-type curve_loss_log() tally)
7	f388049	Sort-tolerant to_polygons + _collect_provenance! walks — defensive issorted ? : sortperm so deserialized polygons predating your constructor sort don't trigger the wrap-seam pinch; also qualifies ustrip (was hard-crashing in #6's path)
8	99e6675	Opt-in partial-curve recovery: substitute_curves_partial[] flag + match_run_partial (longest-contiguous-substring fallback) + _sub_curve (splits the original Segment at the surviving sub-block) — default off, no behavior change for existing pipelines
9	9ba41f0	JuliaFormatter pass to satisfy the julia-format CI gate — pure whitespace, no logic changes, only touches files already modified in this branch

State of the commits

• API design is preliminary. The substitute_curves_partial Ref{Bool} global-flag pattern in Path halo issues #8 follows the existing warn_on_curve_loss precedent, but probably wants a more principled per-call option if you'd like it refactored.
• Test coverage is minimal. Added one regression test in test_line_arc_rounding.jl for the wrap-seam case; the new _as_entities methods and the partial-recovery path lean on the existing test_provenance_recovery setup but aren't yet directly exercised there.
• Not deeply optimized. match_run_partial (Path halo issues #8) is O(n·m) substring matching, no early-exit cleverness. Works in practice on chip-scale runs; happy to profile if you'd like.

Conflicts resolved during rebase

Two overlaps with your recent commits, resolved keeping the union of both improvements:

• _as_entities(::Paths.Node): kept your pathtopolys(p.seg, p.sty) + my Paths.ContinuousStyle qualification (complementary).
• CurvilinearPolygon constructor csi sort: dropped my version, kept yours — functionally identical. So my "Sort-tolerant walks" commit (Keyword arg slurping leads to silent errors #7) is now narrower: only the defensive walks remain.

Verification

• Pipeline byte-identity: a downstream pipeline build on top of this branch produces an output file byte-identical to one built on the pre-rebase reference. The rebase didn't change any rendered geometry.
• The one new test (test_line_arc_rounding.jl "CurvilinearPolygon walk is curve-order independent") uses a generic styled_loop box — no project-specific geometry.
• julia scripts/format.jl check now passes locally; CI should reflect on this push.

Suggested disposition (yours to decide)

• Induced subgraphs #5, Getting started with Julia #2, Better MetaGraphs usage #6, and the bug-fix bits of Developer docs #1/Diagram geometry type relationships #3/Improve postrenderer interface #4 might fold into Add provenance-based curve recovery #232 since they're improvements to the same curve-recovery surface you're building.
• Keyword arg slurping leads to silent errors #7 (defensive walks) is small and complementary; could go either way.
• Path halo issues #8 (partial recovery) is the biggest standalone addition (flag-gated, off by default) — probably wants its own PR after Add provenance-based curve recovery #232 lands.

No rush — happy to rework, split, or squash however you'd like. Downstream, our fast-mesh / digital-twin-pipeline is running against this branch daily, so anything that lands cleanly here reduces our patch surface.

[ybrightye]

Hi @gpeairs — bumping this now that it's out of DRAFT: julia-format was the only red check, and 9ba41f0 is a pure formatter pass that clears it (no logic touched, only files this branch already modified).

Totally understand #232 is still awaiting @laylagi's review and this stacks on top of it, so no rush on the merge side. But if you have a few minutes to skim the disposition suggestions in the description — especially whether any of #5 / #2 / #6 / the small ctor fixes want to fold into #232 vs. stay here — that would help me plan on our end. Happy to rebase, split, or squash however works best.

Thanks for the recent iterations on #232 — we're running downstream against this branch daily on digital-twin-pipeline, so anything that lands cleanly reduces our patch surface directly.

[laylagi]

Thanks @ybrightye for the ping and sorry for the wait. Will take a look tomorrow.

[gpeairs]

Thanks for driving this @ybrightye! Here's what I'm thinking:

• Every fix in 1-5 is now on the Add provenance-based curve recovery #232 branch (some landed directly, some via the to_curvilinear unification which generalized your StyledEntity unwrap).
• 6 is folded into Add provenance-based curve recovery #232 in reworked form: the loss warning is now type-based rather than Kasa-fit-based (same observability, no heuristic). Let me know if that works for you.
• 7 -- I'm reluctant to imply any promises about what happens if you bypass the constructor, particularly via code tied to a user extension of an old version. Is it possible to reconstitute CurvilinearPolygons with sorted indices on your end?
• 8 (partial recovery): This wants its own PR. My thought is a fresh implementation after Add provenance-based curve recovery #232 merges, which I have my own sketch for. The longest-substring matching + _split-based sub-curve emission carries over from this version, but the parameter mapping should be recorded at capture time (linear index -> arclength is unsound for adaptive grids and BSplines, maybe the source of the residual-vertex symptoms you mentioned), consumption will be per-interval across contours, partials will get a distinct :partial report status, and the opt-in will be a per-call kwarg instead of the global Ref{Bool}. Your test case also carries over. Eventually Clipper2 Z-tagging may simplify this further.
• After that lands, if we accounted for everything you need, Curve-recovery additions: _as_entities for Node/StyledEntity/Rounded ClippedPolygon, silent-loss warning, sort-tolerant walks, opt-in partial recovery #243 can be closed.

[ybrightye]

Thanks Greg — dispositions all look right on our end.

Comments 1-5 fully retire once #232 merges. The type-based curve-loss warning is a strict improvement over Kasa-fit — no false-positive risk on rectilinear-with-collinear-shorts polygons. Good to close out #6.

On #7 (sort tolerance): confirmed unnecessary on our end. Every CurvilinearPolygon construction downstream (verified across all call sites) goes through the sorting inner constructor at curvilinear.jl:118-122, no bypass anywhere. Happy to drop that commit.

On #8: your capture-time arclength approach addresses exactly the residual-vertex symptom I was hitting at BSpline tails. Per-call kwarg is much cleaner than the Ref{Bool} global. Please tag me when the follow-up PR opens — happy to donate test geometry.

One thing worth flagging before closeout — while testing coverage against #232 HEAD on our test geometry, we noticed a BSpline + SimpleCPW path-node regression against the currently pinned commit:

	current pin (by/curve-recovery-additions)	#232 HEAD
discretize_with_provenance samples/curve	~110-125	~480-760
union2d_curved output regions	2	5 (3 empty)
union2d_curved output vertex count	O(hundreds)	~3×
net area	matches to <0.2%	(same)

Geometry is preserved but topology and vertex count change. Traced to #236 replacing adapted_grid (angle-based) with the new discretization_grid chord-height marching kernel. discretization.jl:~192 already flags it as a known limitation:

"the curvature guard below is t_scale-dependent, so it over-refines short, tight arcs (a 30°/2μm fillet gets ~101 points vs ~10 from circular_arc)."

Not a blocker on #243 as-is — happy for #232 to land with the disposition you laid out. But wanted to flag it in case fixing the "known limitation" is something you'd want to bundle with #232 or split as its own follow-up. Is it on your radar?

Will close #243 once #232 merges and we bump the DL pin downstream.

[gpeairs]

I think all BSplines were previously rendering with the marching kernel, but using the curvature of the base BSpline. In #236 they started taking into account the effect of the trace/gap offset on curvature, which could get it blocked by the curvature guard and trigger over-refinement. It's harder to see why there would be empty output regions. If you can share the specific geometry, that would help me look into it. EDIT: I've been convinced this is very likely because an offset curve has cusps (min curvature radius of the base BSpline is less than trace/2 [+ gap]). So the offset contribution to curvature causes it to diverge, giving you your over-refinement, and also creating the pathological cusp geometry that gets resolved to empty regions. This is not a great regime to be in, and I think elsewhere we've said 'create cusps at your own risk', but we should handle it gracefully.

We have a follow up in #250 that avoids over-refinement for turns, but no fix for BSplines yet. Ideally we can get it fixed before tagging a new version. Please continue to report any issues or suspected issues like that, since we want to be very confident in changes to the rendering pipeline like that before tagging a new version.

[ybrightye]

Confirmed — this reproducer IS in cusp territory, matching your diagnosis. The base BSpline has a fold at t≈0.8 with min curvature radius ~0.22 μm, while the outer offset (trace/2 + gap = 1.0 μm) exceeds that → 1 − offset·κ_max = −3.6 (sign flip → offset curve pinches through a cusp).

Minimal reproducer (no external deps beyond DeviceLayout + Unitful):

using DeviceLayout, Unitful
using DeviceLayout: Point, Rectangle
import DeviceLayout.Paths
using DeviceLayout.Curvilinear: CurvilinearRegion, discretize_with_provenance
using Unitful: @u_str
const μm = u"μm"

pth = Paths.Path(Point(0.0μm, 0.0μm); α0=0.0u"°")
Paths.bspline!(pth,
    [Point(5.0μm, 2.0μm), Point(10.0μm, 0.0μm)],
    0.0u"°",
    Paths.SimpleCPW(1.0μm, 0.5μm))
node = only(pth)

# Curvature diagnostic
seg = node.seg
max_κ = 0.0
for t in 0.0:0.01:1.0
    g = Paths.Interpolations.gradient(seg.r, t)[1]
    h = Paths.Interpolations.hessian(seg.r, t)[1]
    κ = abs(g.x*h.y - g.y*h.x) / (g.x^2 + g.y^2)^(3//2)
    max_κ = max(max_κ, Unitful.ustrip(u"μm^-1", κ))
end
# outer offset = trace/2 + gap = 1.0 μm; speed at max κ = 1 - 1.0·κ_max ≈ -3.6 → cusp

far = Rectangle(Point(-100.0μm, -100.0μm), Point(-99.0μm, -99.0μm))
T = typeof(0.0u"nm")
ptp = DeviceLayout.Curvilinear.pathtopolys(node)
_, runs = discretize_with_provenance(ptp, T)
@show [length(r.run) for r in runs]

regions = DeviceLayout.union2d_curved([node], [far])
@show length(regions)
for r in regions
    r isa CurvilinearRegion || continue
    @show length(r.exterior.p), length(r.holes)
end

Observed:

	by/curve-recovery-additions	gp/provenance-curve-recovery HEAD
discretize_with_provenance samples/curve	109, 113, 124, 119	478, 539, 761, 672
union2d_curved regions	2	5 (3 with empty exterior)
union2d_curved total vertices	350	1166

pathtopolys output is identical between branches (2 CurvilinearPolygons × 2 ConstantOffset{BSpline} curves each), so the split is downstream of the curve capture — in discretize_with_provenance → discretize_curve → discretization_grid.

Good to know #250 addresses Turns already. Glad to keep watching for BSpline-side follow-ups once you're ready.

[gpeairs]

#259 will guard against over-refinement near cusps and warn when curve discretization finds diverging curvature. It could also make sense to prune degenerate regions as part of recover_curves.

However, in general, cusps are still problematic, especially in the common case where they appear on self-intersecting offset curves. Various operations in DeviceLayout do not handle self-intersecting curves consistently [e.g., I think GDS treats inverted loops as positive while SolidModel drops them], and we're unlikely to start making promises about how they're handled -- users create them at their own risk. Partial curve recovery can in principle handle self intersections but we're unlikely to go out of our way to support it or establish a canonical handling of self-loops (drop, treat as positive, or treat as negative -- all plausible user intent depending on the situation, though "drop" makes the most sense here).

[ybrightye]

Thanks Greg — took a closer look with a production build and want to correct my earlier characterization.

Audited our production layout for BSpline+CPW nodes: 130 BSpline path-nodes total (118 SimpleCPW, 12 TaperCPW). Zero cusps. Curvature-radius / outer-offset ratio distribution: tightest 4.5×, p25 24×, median 53×. My earlier MRE hit a cusp at ratio 0.22× — that was pathological toy geometry from a hand-picked BSpline where the endpoint tangent constraint forced a hairpin fold. Not representative of anything our router produces. Sorry for the false alarm.

Ran a full end-to-end mesh build on our production fixture, comparing by/curve-recovery-additions (pin) vs gp/provenance-curve-recovery HEAD (post-#232 merge, same SKIP_ADC=1 USE_REAL_L1=0 flags):

	pin	#232 HEAD	Δ
2D triangles	840,139	839,893	-0.03%
3D tets	8,427,805	8,427,529	-0.003%
generate(2) time	502.9s	492.9s	-2%
worst kappa_proxy	2518	2338	slightly better

Extra Clipper samples on outer-offset BSplines (~15-18% on tightest cases in isolation) don't propagate downstream — substitute_curves succeeds on uncut BSplines, and the extra vertices are internal-only. On real geometry the mesh is effectively unchanged.

Also: reading discretization_grid(::ConstantOffset, ...) — the extra density is a correctness fix, not gratuitous. t_scale = pathlength(s) on baseline is the base-curve length; the actual outer-offset arclength is longer, so baseline was silently violating the 1 nm chord tolerance. Fine by us — we're at μm scales.

So on the cusp policy: fully agree, users create them at their own risk. Our router avoids them by construction. #259 is a nice-to-have guardrail; not blocking anything on our end.

Once #232 shows up on a tagged release we'll bump the DTP pin and close #243.