hierarchy: port d3-hierarchy layouts (tree, cluster, pack, partition, stratify) - #592
hierarchy: port d3-hierarchy layouts (tree, cluster, pack, partition, stratify)#592cedrickcooke wants to merge 7 commits into
Conversation
Add d3-hierarchy Node accessors additively and introduce a shared Point layout type. Also fixes an ancestors() bug where the parent chain was not advanced (it always re-read the receiver's parent). Co-authored-by: Cedrick Cooke <development@cedrickc.net>
Port d3-hierarchy's tree layout using Buchheim et al.'s linear-time algorithm. Supports size/nodeSize and configurable separation, attaching a Point layout to each node. Co-authored-by: Cedrick Cooke <development@cedrickc.net>
Port d3-hierarchy's cluster layout, aligning leaves at equal depth with size/nodeSize and configurable separation. Co-authored-by: Cedrick Cooke <development@cedrickc.net>
Port d3-hierarchy's pack layout: front-chain sibling packing plus Welzl enclosing-circle, with radius/size/padding. Geometry runs in Double to match d3's precision and avoid enclose-basis failures. Co-authored-by: Cedrick Cooke <development@cedrickc.net>
Port d3-hierarchy's partition layout, subdividing width by weight per depth band with size/padding/round, attaching a Cell (x0/y0/x1/y1) layout. Co-authored-by: Cedrick Cooke <development@cedrickc.net>
Build a hierarchy from a flat list via id/parentId accessors, validating single-root, missing/ambiguous parents, and cycles. Co-authored-by: Cedrick Cooke <development@cedrickc.net>
Co-authored-by: Cedrick Cooke <development@cedrickc.net>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f941dd3. Configure here.
| val k = circleOf(typedRoot).r / min(width, height) | ||
| typedRoot.traversePostOrder().forEach { node -> packChildren(node, padding, k, random, ::circleOf) } | ||
| val translation = min(width, height) / (2.0 * circleOf(typedRoot).r) | ||
| typedRoot.traversePreOrder().forEach { node -> translateChild(node, translation, ::circleOf) } |
There was a problem hiding this comment.
Pack divides by zero radius
Medium Severity
In the default Pack.layout path, translation divides by 2.0 * circleOf(typedRoot).r. If every leaf weight is zero (or packing yields a zero root radius), that denominator is zero, producing infinite or NaN coordinates and radii on all nodes.
Reviewed by Cursor Bugbot for commit f941dd3. Configure here.
| while (end !== ancestor) { | ||
| nodes.add(split, end) | ||
| end = checkNotNull(end.parent) | ||
| } |
There was a problem hiding this comment.
Path crashes unrelated nodes
Low Severity
Node.path assumes target shares a hierarchy with the receiver. If leastCommonAncestor returns null, the upward walk still uses checkNotNull(start.parent) and throws once the parent chain ends, instead of returning a result or a clear error.
Reviewed by Cursor Bugbot for commit f941dd3. Configure here.
2 participants
Summary
Expands the
hierarchymodule with the remainingd3-hierarchylayouts, following the existing generic-layout (Node<T, L>) style used bytreemap. Only thehierarchymodule is touched.New capabilities:
com.juul.krayon.hierarchy.tree) — Reingold–Tilford "tidy" tree via Buchheim et al.'s linear-time algorithm. Configurablesize/nodeSizeandseparation; attaches aPoint(x, y)layout.…hierarchy.cluster) — dendrogram with leaves aligned at equal depth.size/nodeSize/separation; attachesPoint.…hierarchy.pack) — circle packing (front-chain sibling packing + Welzl enclosing circle).radius/size/padding; attachesCircle(x, y, radius).…hierarchy.partition) — icicle/sunburst rectangles.size/padding/round; attachesCell(x0, y0, x1, y1).…hierarchy.stratify) — builds a hierarchy from a flat list viaid/parentIdaccessors, with validation (single root, missing/ambiguous parents, cycles).leaves(),links()(+Link),path(target),descendants(),copy(), added additively alongside existing traversals. A sharedPointtype is introduced for tree/cluster.Layouts follow the treemap convention: a configurable layout class plus a
Node<T, *>.layoutWith(...)extension that mutates and returns the same nodes with the layout attached.Notes
Doublefor pack geometry: circle packing is ported usingDoubleinternally (publicCircleremainsFloat).Floatprecision caused the Welzl enclose-basis search to fail (Unable to extend enclosing basis);Doublealso reproduces d3's exact coordinates.ancestors()bug fix: the existingancestors()advanced the chain withcurrent = parent(always the receiver's parent) instead ofcurrent = current.parent, yielding an infinite sequence for depth > 1.path()depends on a correct ancestor walk, so this one-line bug is fixed.count()semantics: Krayon'scount()weights every node (not only leaves as d3 does), so partition/pack fixtures usesum { value }with explicit leaf weights, which is identical between Krayon and d3.Testing
Common tests (
kotlin.test) verify each layout against exactd3-hierarchy@3values on small fixtures (tree/cluster coordinates, pack radii + containment, partition cells, stratify structure, node helpers)../gradlew :hierarchy:jvmTest— 24 tests pass../gradlew :hierarchy:lintKotlin— clean../gradlew :hierarchy:jvmApiCheck :hierarchy:klibApiCheck— pass; API dumps regenerated and committed.