Add provenance-based curve recovery#232
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Bug
Minimal Reproducible Exampleusing 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 CauseTwo related issues in
Impact
Suggested FixSort function CurvilinearPolygon(p, curves, curve_start_idx)
perm = sortperm(curve_start_idx)
new(p, curves[perm], curve_start_idx[perm])
endThis is the most defensive fix as multiple code paths ( For the residual vertex issue, |
Bug
The simplest trigger is MREusing 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 CurvilinearPolygonExpected behavior
Root causeIn 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
endWhen Suggested fixThe 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
endAffected combinations
The bug is specifically triggered when the recursive inner resolution produces a non-curvilinear entity that then needs rounding applied. BranchFound on |
|
I've fixed the first issue with sorted curve start indices. I'm not sure what On the second issue, this is what I mean by
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. |
|
Thanks for fixing the sorted indices issue! Indeed Bug #2 (the |
bug with
|
Bug:
|
BUG:
|
Fixed by using the correct
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 |
Boolean operations (
union2d,difference2d, etc.) discretize curved geometry to polygonsbefore passing them to the Clipper library. Normally, the original curves (arcs, splines)
are lost in this process. The
recover_curvesfunction and its conveniencewrappers (
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 singleClippedPolygon. Each region in the vector corresponds to one outer contour in the clippedresult (the disjoint pieces each become a separate region). For example:
This required moving some of the SolidModel (rounded Polygon -> CurvilinearPolygon) rendering pipeline out of the
SolidModelsmodule and intosrc/render/so that it's available tocurvilinear.jl. I think that's the right place for this code, and eventually we'll want to includecurvilinear.jlbeforesrc/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 withRoundedpolygons.Current limitations:
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
Path nodes, and
Rounded-styledPolygon/Rectangleentities.Roundedapplied toother 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.
I also experimented with arc recognition without provenance, but I found it very difficult to make robust.