aws-cqc / aws-cqc/DeviceLayout.jl

Cleanup follow-ups from #236 (render unification)

Open
#247 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Julia
Stars
67
Forks
14
Avg merge
1d 16h
Merged PRs (30d)
21

Description

Consolidated follow-ups from the #236 review, most should land before the next version tag.

- [ ] #244 (compound seg/sty with mismatched tags in SolidModel rendering)
- [x] Offset segments with compound styles render in the wrong arclength frame -- no one's supposed to be styling offset segments, but we can at least throw a hard error for offset seg + compound sty and write a comment
- [x] `src/curvilinear.jl:257` warns and calls the removed 3-arg `to_polygons(f, pathlength(f), s; ...)`; replace with a clear "no render method for (segment, style)" error.
- [ ] Arc over-refinement -- use constant-curvature fast path to avoid leaving `dt` at the 0.01 floor whenever `sqrt(8·tol/κ)/t_scale > 0.1`, and/or rethink that guard
- [ ] `GeneralOffset{BSpline}` discretization integrates `t_to_arclength` from 0 with each step. Re-evaluate after #245 (arclength cache, ~15× per call); if still hot, accumulate arclength incrementally across the grid.
- [x] O(n²) membership scans in `round_to_curvilinearpolygon`: (`src/curvilinear.jl:1031`, `:1063`): `i in corner_indices` / `i in la_indices` on plain `Vector`s per vertex plus `findfirst` over `curve_start_idx` per line-arc corner. Pre-#236 GDS rounding used `Set` + `Dict`; restore that at function entry.
- [x] Items mentioned in #232 as pending refactor (now addressed in that PR)
- [x] Small duplications: `_bspline_signed_curvature` vs `_bspline_curvature` (`src/render/discretization.jl:75/137` — one should call the other); `pathtopolys(..., ::PeriodicStyle)` pair (`src/curvilinear.jl:314/325` — share a helper like the offset pair); the "Ignoring attachments" warn-and-strip body (`src/curvilinear.jl:289/412/749` — one shared helper behind thin dispatch methods).
- [x] Dimensionless relative→absolute rounding flip for `CurvilinearPolygon` + `Rounded(::Real)` is intended but untested — add a pinning test and a CHANGELOG line stating all dimensionless relative rounding is now absolute-by-default as intended.
- [x] Incidentally discovered autofill bug described [in this comment](https://github.com/aws-cqc/DeviceLayout.jl/pull/236#issuecomment-4864422022)

Contributor guide

Open the contributing guide

Research direction

Read the #236 review and the follow-up context in #245 first, then inspect the unchecked work in src/curvilinear.jl and src/render/discretization.jl. Compare the listed rendering, arc-refinement, offset-discretization, and membership-scan items with the already checked changes; done means each remaining follow-up is resolved before the next version tag.

Written by the indexing model from the issue text.

Assessment

Tech stack
julia
Domain
computer-graphics, performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.