Skip to content

[WIP] Add support for rounding line-arc corners#166

Open
laylagi wants to merge 2 commits intomainfrom
lgh/line-arc-rounding
Open

[WIP] Add support for rounding line-arc corners#166
laylagi wants to merge 2 commits intomainfrom
lgh/line-arc-rounding

Conversation

@laylagi
Copy link

@laylagi laylagi commented Mar 6, 2026

This PR implements fillet rounding at vertices where a straight edge meets a circular arc. The existing Rounded style only handled straight-straight corners; line-arc corners were skipped entirely.

TODOs:

  • Solid model pipeline
  • Update documentation

@laylagi laylagi added the enhancement New feature or request label Mar 6, 2026
@laylagi
Copy link
Author

laylagi commented Mar 6, 2026

Tested the code with various line-arc cases. The left and right figures are the layouts before and after rounding, respectively.
Screenshot 2026-03-06 at 12 14 04 PM

@codecov
Copy link

codecov bot commented Mar 6, 2026

Codecov Report

❌ Patch coverage is 95.74468% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/curvilinear.jl 95.74% 6 Missing ⚠️

📢 Thoughts on this report? Let us know!

@gpeairs
Copy link
Member

gpeairs commented Mar 9, 2026

It looks like the 1.10 test failures are related to Julia internals (type narrowing?) -- maybe an unnecessary convert is being called in a constructor, causing a copy to be made, which causes == to fail. If you could review #168 and rebase on that, I think it would fix those tests.

Separately, up until now, I've been thinking of CurvilinearPolygon as "DeviceLayout internals" -- not thoroughly documented or tested for direct use (i.e., outside of the SolidModels rendering pipeline), not included in the HTML docs, doesn't use tolerance in to_polygons. Have you found it robust enough for your use case? At this point I don't think the interface is subject to change, so it seems fair to promote it to feature.

@laylagi laylagi force-pushed the lgh/line-arc-rounding branch 2 times, most recently from 98e19e6 to 770904b Compare March 9, 2026 18:42
@laylagi
Copy link
Author

laylagi commented Mar 10, 2026

Separately, up until now, I've been thinking of CurvilinearPolygon as "DeviceLayout internals" -- not thoroughly documented or tested for direct use (i.e., outside of the SolidModels rendering pipeline), not included in the HTML docs, doesn't use tolerance in to_polygons. Have you found it robust enough for your use case? At this point I don't think the interface is subject to change, so it seems fair to promote it to feature.

I mentioned this caveat in my MR, but worth reiterating here. We probably need a per-corner rounding radius to make it robust - at least for my use case.

laylagi added 2 commits March 10, 2026 12:46
…egion

  Implement fillet rounding at vertices where a straight edge meets a circular arc. The existing Rounded style only handled straight-straight corners; line-arc corners were skipped entirely.

  New in src/curvilinear.jl:
  - edge_type_at_vertex: classifies incoming/outgoing edges as :straight
    or a Paths.Turn curve
  - rounded_corner_line_arc: bisection solver finds fillet center at the
    intersection of a line offset and a circle (R ± r from arc center);
    tries both internal and external tangency, validates via T_line position
  - to_polygons(CurvilinearPolygon, Rounded): rounds all corners (straight-
    straight and line-arc), then discretizes remaining curves with trimming
    at fillet tangent points
  - to_polygons(CurvilinearRegion, Rounded): rounds exterior and holes
    independently before differencing

  Test (test/test_line_arc_rounding.jl) covers 13 arc features:
  - 10 exterior arcs on a 24×16μm rectangle: sweeps from 60° to 270°,
    notches and bumps, all four edges
  - 3 interior pie-slice holes (60°, 90°, 120° arcs) via CurvilinearRegion
  - G1 continuity check on the rounded output
  - Fillet radius larger than arc radius (graceful clamping)

  TODOs:
  - Solid model pipeline
  - Documentation
The bisection bracket (±π/2) failed for arcs where the fillet center was very close to the corner angle, causing rounding to silently skip those corners. Replace the bisection with an analytical line-circle intersection that works for all geometries.
@laylagi laylagi force-pushed the lgh/line-arc-rounding branch from 770904b to 90fafb3 Compare March 10, 2026 19:47
@gpeairs
Copy link
Member

gpeairs commented Mar 11, 2026

We probably need a per-corner rounding radius

For ordinary Polygons, you can stack Rounded styles with different point selections:

poly = Rectangle(1mm, 1mm)
r1_points = [Point(0, 0)mm]
r2_points = [Point(1, 1)mm]
r3_points = [Point(0, 1)mm, Point(1, 0)mm]
r1 = 0.1mm
r2 = 0.2mm
r3 = 0.3mm 
rnd1_poly = Rounded(r1; p0=r1_points)(poly)
rnd12_poly = Rounded(r2; p0=r2_points)(rnd1_poly)
rnd123_poly = Rounded(r3; p0=r3_points)(rnd12_poly)

This works for rendering to Cell because the inner styled entity is resolved to a Polygon first, and then the outer to_polygons can round the result as usual. It works for SolidModel because because the inner styled entity is resolved to a CurvilinearPolygon first, and then SolidModels.styled_loop rounds that CurvilinearPolygon to another CurvilinearPolygon (but only at line-line corners).

Unfortunately, to do this, we had to duplicate rounding logic in polygons.jl and in solidmodels/render.jl. To add line-arc rounding functionality to SolidModel you'll have to add it to SolidModels.round_to_curvilinearpolygon, but then nested rounding will work for free.

However, it still won't work for Cell, because the inner entity gets resolved into a Polygon rather than a CurvilinearPolygon. However, once you've added line-arc rounding to SolidModel.round_to_curvilinearpolygon, something like this should work:

function to_polygons(
    ent::StyledEntity{T, U, V},
    sty::Rounded{S};
    atol=_round_atol(S, T),
    kwargs...
) where {S, T, U, V <: Rounded}
    return to_polygons(
        SolidModels.styled_loop(ent.ent, ent.sty),
        sty;
        atol,
        kwargs...
    )
end

I think ideally the rendering of Rounded entities would always go through CurvilinearPolygon, allowing us to remove the duplicated logic. I would like to fix #47 first, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants