From 9cb8c0200da23bbe32980aa3c010b22185ca23c8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 11:04:49 +0000 Subject: [PATCH] [minor] Raise MaxVelocity so a graph settles in seconds, not minutes A graph that opened as a tall narrow column with near-vertical links was not laid out wrongly - it had not finished. Measured on a twenty node, twenty edge graph the size of a small class, starting clustered: 600 steps 740 x 1063 aspect 0.70 mean angle 64.8 deg 1800 steps 1050 x 728 aspect 1.44 mean angle 40.5 deg 6000 steps 1448 x 833 aspect 1.74 mean angle 28.8 deg 20000 steps 1447 x 833 aspect 1.74 mean angle 28.8 deg 600 steps is ten seconds at sixty frames a second, which is about as long as anyone watches. The shape at that point is what looked broken; the settled one is fine. Starting position makes no difference - scattered, clustered and coincident all converge to the same shape - so this was not a local minimum. MaxVelocity was the bound. A body has to travel several hundred units to reach its place, and a cap of 50 a second means seconds per body. At 600 steps: v50 gives aspect 0.70 at 64.8 deg, v150 1.17 at 39.0, v250 1.44 at 38.8, v400 1.75 at 33.9. Past that it stops helping - v600 comes back to 1.34 at 34.3 - so there is an overshoot limit and the default now sits inside it at 250, which reaches its settled shape roughly five times sooner while settling to much the same place (1.49 at 28.5 deg against 1.74 at 28.8). Repulsion is still doing its job, which is what prompted looking. Settled, with it the graph is 1313 x 881 at 28.5 deg; without it 594 x 840 at 72.0 deg. The overlap pass only keeps bodies off one another, it creates no room beyond that, so without repulsion the graph collapses into the column the levelling force cannot then recover from. Two tests cover what nothing did: ASmallGraph_IsReadableWithinTenSeconds pins the settling speed, and Repulsion_IsWhatSpreadsAGraphOut pins why repulsion stays. Step_WithOverlapMarginZero_LeavesTheBodiesOverlapping was asserting a transient. Two 400 wide bodies belong 420 apart once the ordering bias has its way, and the pair only stayed overlapping for 120 frames because the old cap made them too slow to get there. It now turns off the forces that separate a pair, so it measures the overlap pass rather than how far anything travelled. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D2KNUKr1xJPTEdDmeF2tQN --- ForceDirectedLayout/LayoutSettings.cs | 8 +- ForceDirectedLayout/PhysicsSettings.cs | 8 +- ForceDirectedLayout/README.md | 2 +- .../ForceLayoutTests.cs | 149 ++++++++++++++++++ .../OverlapSeparationTests.cs | 7 + 5 files changed, 169 insertions(+), 5 deletions(-) diff --git a/ForceDirectedLayout/LayoutSettings.cs b/ForceDirectedLayout/LayoutSettings.cs index 4576b25e..13ce9cd6 100644 --- a/ForceDirectedLayout/LayoutSettings.cs +++ b/ForceDirectedLayout/LayoutSettings.cs @@ -57,7 +57,11 @@ public struct LayoutSettings /// Per-body force magnitude cap (applied before integration). public double MaxForce; - /// Per-body velocity magnitude cap (applied after integration). + /// + /// Per-body velocity magnitude cap (applied after integration). This is what bounds how long a + /// graph takes to settle: a body has to travel hundreds of units to reach its place, so too low a + /// cap leaves a graph still visibly unfolding many seconds after it opens. + /// public double MaxVelocity; /// Target substep frequency. Per-frame substep count is ceil(deltaTime * TargetPhysicsHz). @@ -87,7 +91,7 @@ public struct LayoutSettings MinRepulsionDistance = 50.0, RestLinkLength = 225.0, MaxForce = 5000.0, - MaxVelocity = 50.0, + MaxVelocity = 250.0, TargetPhysicsHz = 120.0, StabilityThreshold = 1.0, OverlapMargin = 20.0, diff --git a/ForceDirectedLayout/PhysicsSettings.cs b/ForceDirectedLayout/PhysicsSettings.cs index ade561a0..d6e397b9 100644 --- a/ForceDirectedLayout/PhysicsSettings.cs +++ b/ForceDirectedLayout/PhysicsSettings.cs @@ -49,8 +49,12 @@ public sealed record PhysicsSettings /// Per-body force magnitude cap (applied before integration). public double MaxForce { get; init; } = 5000.0; - /// Per-body velocity magnitude cap (applied after integration). - public double MaxVelocity { get; init; } = 50.0; + /// + /// Per-body velocity magnitude cap (applied after integration). This is what bounds how long a + /// graph takes to settle: a body has to travel hundreds of units to reach its place, so too low a + /// cap leaves a graph still visibly unfolding many seconds after it opens. + /// + public double MaxVelocity { get; init; } = 250.0; /// Target substep frequency. Per-frame substep count is ceil(deltaTime * TargetPhysicsHz). public double TargetPhysicsHz { get; init; } = 120.0; diff --git a/ForceDirectedLayout/README.md b/ForceDirectedLayout/README.md index 87f05cbf..4a92e718 100644 --- a/ForceDirectedLayout/README.md +++ b/ForceDirectedLayout/README.md @@ -114,7 +114,7 @@ PhysicsSettings settings = new() OriginAnchorWeight = 1.0, // 0 = centroid, 1 = world origin DampingFactor = 0.5, // velocity retained per second MaxForce = 5000.0, - MaxVelocity = 50.0, + MaxVelocity = 250.0, // also bounds how fast a graph settles TargetPhysicsHz = 120.0, // substep rate, independent of frame rate StabilityThreshold = 1.0, OverlapMargin = 20.0, diff --git a/tests/ForceDirectedLayout.Tests/ForceLayoutTests.cs b/tests/ForceDirectedLayout.Tests/ForceLayoutTests.cs index d95cd13f..c7bfb4a2 100644 --- a/tests/ForceDirectedLayout.Tests/ForceLayoutTests.cs +++ b/tests/ForceDirectedLayout.Tests/ForceLayoutTests.cs @@ -4,6 +4,7 @@ namespace ktsu.ForceDirectedLayout.Tests; using System; using System.Collections.Generic; +using System.Linq; using ktsu.ForceDirectedLayout; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -402,6 +403,154 @@ private static ForceDirectedLayout CreatePinnedLayout(Phys Settings = settings, }; + /// A node in the benchmark graph, sized as the editor draws it. + private sealed record Box(int Id, double W, double H, int InputRows, int OutputRows); + + /// An edge that knows which row it leaves and enters on. + private sealed record Wire(int From, int FromRow, int To, int ToRow); + + /// + /// The Counter document as the editor lays it out: twenty nodes, twenty edges, five levels deep. + /// + private static (List Bodies, List Edges) CounterGraph(double spread) + { + // id, width, height, input rows, output rows + Box[] boxes = + [ + new(1, 60, 50, 0, 1), new(2, 115, 62, 1, 1), // 0 -> var count + new(3, 60, 50, 0, 1), new(4, 115, 62, 1, 1), // 1 -> var step + new(5, 110, 50, 0, 1), // param amount + new(6, 75, 50, 0, 1), new(7, 75, 50, 0, 1), new(8, 75, 50, 0, 1), new(9, 75, 50, 0, 1), + new(10, 90, 75, 2, 1), // binary + (Add) + new(11, 90, 70, 2, 1), // assign + new(12, 105, 60, 1, 1), // return (Add) + new(13, 118, 180, 7, 1), // function Add + new(14, 75, 50, 0, 1), new(15, 75, 50, 0, 1), + new(16, 90, 75, 2, 1), // binary + (Next) + new(17, 115, 62, 1, 1), // var result + new(18, 105, 60, 1, 1), // return (Next) + new(19, 118, 160, 6, 1), // function Next + new(20, 117, 160, 6, 1), // class Counter + ]; + + Wire[] wires = + [ + new(1, 0, 2, 0), new(2, 0, 20, 0), + new(3, 0, 4, 0), new(4, 0, 20, 1), + new(5, 0, 13, 0), + new(6, 0, 11, 0), new(7, 0, 10, 0), new(8, 0, 10, 1), new(10, 0, 11, 1), + new(11, 0, 13, 3), new(9, 0, 12, 0), new(12, 0, 13, 4), new(13, 0, 20, 2), + new(14, 0, 16, 0), new(15, 0, 16, 1), new(16, 0, 17, 0), new(17, 0, 19, 2), + new(18, 0, 19, 3), new(19, 0, 20, 3), + new(9, 0, 18, 0), + ]; + + Dictionary byId = boxes.ToDictionary(b => b.Id); + List bodies = []; + int seed = 0; + foreach (Box b in boxes) + { + // Scattered start, so the layout has to do the work rather than inherit an answer. + seed = ((seed * 1103515245) + 12345) & 0x7fffffff; + double x = seed % 1000 * spread; + seed = ((seed * 1103515245) + 12345) & 0x7fffffff; + double y = seed % 1000 * spread; + bodies.Add(Body(b.Id, x, y, b.W, b.H)); + } + + // A row's pin sits a header down plus its own row height, which is what ImNodes produces. + static double RowY(Box b, int row) => Math.Min(28.0 + (row * 21.0) + 10.0, b.H - 8.0); + + List edges = [.. wires.Select(w => new PinnedEdge( + w.From, w.To, + new Vec2D(byId[w.From].W, RowY(byId[w.From], byId[w.From].InputRows + w.FromRow)), + new Vec2D(0, RowY(byId[w.To], w.ToRow))))]; + + return (bodies, edges); + } + + /// Mean angle off horizontal across a graph's edges, and its bounding box. + private static (double Width, double Height, double MeanAngle) Shape(List bodies, List edges) + { + double width = bodies.Max(b => b.Position.X + b.Dimensions.X) - bodies.Min(b => b.Position.X); + double height = bodies.Max(b => b.Position.Y + b.Dimensions.Y) - bodies.Min(b => b.Position.Y); + + Dictionary byId = bodies.ToDictionary(b => b.Id); + double total = 0; + foreach (PinnedEdge e in edges) + { + Vec2D from = byId[e.SourceId].Position + e.SourcePin; + Vec2D to = byId[e.TargetId].Position + e.TargetPin; + double angle = Math.Abs(Math.Atan2(to.Y - from.Y, to.X - from.X) * 180.0 / Math.PI); + total += angle > 90 ? 180 - angle : angle; + } + + return (width, height, total / edges.Count); + } + + /// + /// Tests that a graph the size of a small class reaches a readable shape within the first few + /// seconds, rather than only after a minute and a half of settling. + /// + /// + /// 600 steps is ten seconds in an editor running at sixty frames a second, which is about as long + /// as anyone watches a graph unfold. With MaxVelocity at 50 this same graph was still a tall narrow + /// column at that point - bodies simply could not travel the several hundred units to their places + /// fast enough - and only squared up around 6000 steps. Nothing measured settling speed, so the + /// layout looked broken while being perfectly correct. + /// + [TestMethod] + public void ASmallGraph_IsReadableWithinTenSeconds() + { + (List bodies, List edges) = CounterGraph(0.05); + ForceDirectedLayout layout = CreatePinnedLayout(new PhysicsSettings { Enabled = true }); + + for (int i = 0; i < 600; i++) + { + layout.Step(bodies, edges, 0.016); + } + + (double width, double height, double meanAngle) = Shape(bodies, edges); + + Assert.IsTrue(width > height, + $"A left-to-right graph should be wider than it is tall by now; it is {width:F0} x {height:F0}."); + Assert.IsTrue(meanAngle < 45.0, + $"Its edges should be nearer horizontal than vertical by now; mean angle is {meanAngle:F1} degrees."); + } + + /// + /// Tests that repulsion is still what spreads a graph out, now that an overlap pass also keeps + /// bodies off one another. + /// + /// + /// The overlap pass only guarantees bodies do not sit on top of each other; it creates no room + /// beyond that. Without repulsion a settled graph collapses into a tall column whose edges run + /// close to vertical, which is the shape the levelling force then cannot recover from. + /// + [TestMethod] + public void Repulsion_IsWhatSpreadsAGraphOut() + { + (List withBodies, List withEdges) = CounterGraph(0.05); + ForceDirectedLayout with = CreatePinnedLayout(new PhysicsSettings { Enabled = true }); + + (List withoutBodies, List withoutEdges) = CounterGraph(0.05); + ForceDirectedLayout without = CreatePinnedLayout(new PhysicsSettings { Enabled = true, RepulsionStrength = 0 }); + + for (int i = 0; i < 6000; i++) + { + with.Step(withBodies, withEdges, 0.016); + without.Step(withoutBodies, withoutEdges, 0.016); + } + + (double withWidth, double withHeight, double withAngle) = Shape(withBodies, withEdges); + (double withoutWidth, double withoutHeight, double withoutAngle) = Shape(withoutBodies, withoutEdges); + + Assert.IsTrue(withWidth / withHeight > withoutWidth / withoutHeight, + $"Repulsion should leave the graph wider; with {withWidth / withHeight:F2}, without {withoutWidth / withoutHeight:F2}."); + Assert.IsTrue(withAngle < withoutAngle - 15.0, + $"Repulsion should leave the edges far flatter; with {withAngle:F1} deg, without {withoutAngle:F1} deg."); + } + [TestMethod] public void PinOffsets_LevelThePinsRatherThanTheBodyCentres() { diff --git a/tests/ForceDirectedLayout.Tests/OverlapSeparationTests.cs b/tests/ForceDirectedLayout.Tests/OverlapSeparationTests.cs index 03019956..8f092226 100644 --- a/tests/ForceDirectedLayout.Tests/OverlapSeparationTests.cs +++ b/tests/ForceDirectedLayout.Tests/OverlapSeparationTests.cs @@ -115,8 +115,15 @@ public void Step_WithOverlapMarginZero_LeavesTheBodiesOverlapping() { // Opting out has to be possible for a consumer that arranges its own bodies, and leaving them // overlapping is what says the separation is this pass's doing rather than the forces'. + // Repulsion and the ordering bias both push a pair apart on their own - the ordering bias alone + // targets halfWidths + 20, which for these bodies is wider than they are - so they are turned + // off here. Otherwise this measures how far the pair has travelled rather than whether the pass + // is what separates them. LayoutSettings settings = EnabledDefaults(); settings.OverlapMargin = 0.0; + settings.RepulsionStrength = 0.0; + settings.DirectionalBias = 0.0; + settings.LinkFlatteningStrength = 0.0; Vec2D[] dimensions = [new Vec2D(400, 100), new Vec2D(400, 100)]; ForceLayout layout = new(settings);