Repository navigation
Conversation
|
Hi! This PR is blocked waiting for review/approval. Could you please review when you have a chance? The changes are ready and all local checks pass. Thanks! |
|
Friendly ping — this adds a cycle shape. Ready for review; happy to adjust anything. Thanks! |
shape: cycle arranges its children with their centers on a circle and routes edges between them along that circle as exact circular arcs (cubic Bezier chains using the canonical arc control offset), with endpoints trimmed to each child's visible border so arrowheads touch rectangles, circles, polygons alike. - children keep their internal layout: the core layout runs first and containers are moved onto the ring together with their descendants, translating internal and cross-child edge routes along - root-level edges take the shorter way around the ring; on a two-node cycle opposite edges take opposite semicircles - the container renders like a container (styles apply), in both d2svg and d2scenebuild - e2e txtar fixture cycle-diagram with dagre, elk and tala goldens
aa0dbb6 to
f2f2863
Compare
|
Hi! I rebased this onto current Both CI runs for the new head are sitting in Contents unchanged from what you already saw: a |
Design summary
shape: cycleturns a container into a ring: its direct children are placed with their centers on a circle (first child at 12 o'clock), and edges between them are routed along that circle as exact circular arcs whose endpoints are trimmed to each child's visible border.Layout (
d2layouts/d2cycle):k = 4/3·tan(Δθ/4), split into segments of at most 90°, so the drawn path is a perfect circle through the existingIsCurvepath renderer (no renderer changes needed).TraceToShapeBorderfallback. Arrowheads therefore land exactly on the visible outline, which is whatshape: cycle#1578 asks for.style.fill,style.stroke,multiple,3d,double-border, fill patterns, labels and icons all work unchanged.The name follows the issue discussion (
cycle, notradial, keepingradialreserved).Reviewing
New e2e fixture (
e2etests/txtar.txt→cycle-diagram), goldens for dagre, elk and tala:e2etests/testdata/txtar/cycle-diagram/dagre/{board.exp.json,sketch.exp.svg}e2etests/testdata/txtar/cycle-diagram/elk/{board.exp.json,sketch.exp.svg}e2etests/testdata/txtar/cycle-diagram/tala/{board.exp.json,sketch.exp.svg}The fixture covers: a 4-node ring (the case from the issue), a ring of circle/hexagon/diamond children plus a reverse edge, a ring with nested content (
x: {p -> q}), a cross-child edge (x.p -> y), container styling, and a two-node ring with edges in both directions. Reviewers can re-render withTA=1 go test ./e2etests -run 'TestE2E/txtar/cycle-diagram' -v, or paste the fixture board into the playground.Unit tests:
d2layouts/d2cycle/layout_test.go(radius sizing, ring placement, arc circularity + border trimming, nested-content preservation, cross-child edge attachment, two-node semicircles).How this differs from #2760 and #2806
Both rival PRs share the same core approach (circle layout + circular arcs); this PR is a superset of the stronger one (#2806) with the following differences:
b -> ais routed the long way around (e.g. a 240° arc through the other children). Here every edge takes the shorter sweep, and on two-node rings opposite edges split into disjoint semicircles.TestE2E/txtarruns; this PR includes tala goldens.d2scenebuildregistration. Master's newer scene renderer has its own shape switches; neither rival registerscyclethere (paged exports would fall into the typed-shape default path). Registered in all three rectangle cases.pathDatacurve code. cycle-diagram: keep arcs circular at node borders #2760 rewrites the curve branch used by every d2 diagram (with unreachable branches and dead fallbacks), which is a regression risk for all existing curves; exact Bezier control points make that unnecessary here. cycle-diagram: keep arcs circular at node borders #2760 also skips the core layout entirely (children with nested content are never laid out and would panic inFitToGraph's bounding box), renames unrelated files, and leaves unused helpers.cycletype hits nodrawShapecase, so the container is invisible and all container styles are dropped; here it renders as a normal container in both SVG and scene renderers.Validation
go build ./...,go veton touched packages,gofmtcleango test ./d2layouts/d2cycle/ -v— 5/5 passgo test ./e2etests -run 'TestE2E/txtar$'— pass (all fixtures, dagre+elk+tala)go test ./e2etests— full e2e suite passes/claim #1578