Composite rounding of a ClippedPolygon drops its holes

[parrangoiz]

If you compose two Rounded styles and apply them to a ClippedPolygon that has holes, the holes get dropped.

Minimal repro w/ Julia 1.12.4 and DL main

using DeviceLayout
using DeviceLayout.PreferredUnits
using Unitful: ustrip

outer = centered(Rectangle(100.0μm, 100.0μm))
hole  = centered(Rectangle(40.0μm, 40.0μm))
cp = difference2d(outer, hole)  # ClippedPolygon: one boundary, one hole

total_area(x) = sum(abs(DeviceLayout.Polygons.area(p)) for p in to_polygons(x))

expected = 100.0^2 - 40.0^2
single   = total_area(Rounded(5.0μm)(cp))                      # one rounding pass
composed = total_area(Rounded(5.0μm)(Rounded(5.0μm)(cp)))      # two rounding passes (nested)

println("expected (100² - 40²)        = $(expected) µm²")
println("single Rounded(5µm)          = $(round(ustrip(μm^2, single), digits=1)) µm²   (correct: hole subtracted)")
println("composed Rounded(5µm)∘(5µm)   = $(round(ustrip(μm^2, composed), digits=1)) µm²   (BUG: ≈100² ⇒ hole dropped)")

Tentative diagnosis (needs double-checking)

Composing two Rounded styles (a nested StyledEntity) routes through

to_polygons(::CurvilinearRegion, ::Rounded)   # src/curvilinear.jl:199

which rounds the exterior and each hole independently and then re-runs difference2d:

difference2d(to_polygons(exterior, sty), [to_polygons(h, sty) for h in holes])

The rounded hole loops come back with the wrong winding, so difference2d no longer
subtracts them.

[gpeairs]

I think that's right, the CurvilinearRegion to_polygons methods expect the holes to have positive winding, but ClippedPolygon hole contours are returned with negative winding, and those are not reversed when constructing the CR. #232 also ran into this. #234 floats normalizing orientation at clipping time, which I think is too breaking in general, but CR holes can probably be safely normalized either on construction or at clipping time.

[parrangoiz]

Why is it necessary to call difference2d again here (and in the next function) in the first place?

[gpeairs]

Maybe it's not -- to_polygons needs to run interiorcuts to cut out any holes, but assuming the documented CurvilinearRegion limitation "These holes cannot intersect each other or the exterior" is followed, I suppose we could construct the input to interiorcuts directly rather than run through another clip operation.

[parrangoiz]

What's really confusing to me right now as I trace through the code is why at some point we end up with a CurvilinearRegion deep in the callstack of to_polygons(::ClippedPolygon, ::Rounded). We're already asking for discretized version of the rounded ClippedPolygon so it's unclear to me why a CurvlinearRegion needs to be constructed at all.

[gpeairs]

In this case it's specifically for line-arc rounding -- when the inner rounded entity gets rounded again, we want to still have the exact curves so that line-arc rounding is still possible. So for to_polygons on a twice-rounded entity, the inner one routes through the CurvilinearRegion constructor. See the comment starting here.

In the future we're actually planning to route all rounding through Curvilinear entities to eliminate the present near-duplication of rounding code between the polygon and SolidModel rendering pipelines, preventing bugs from drift between the two.

[parrangoiz]

Ok I see. In that case I won't try to fix this yet, my guess is that the rendering consolidation will take care of this particular bug (although the orientation bug will remain latent for other code paths).