Repository navigation
Let a backward edge's endpoints slide around each other to reorder - #362
Merged
Merged
Conversation
…order A backward edge could never fix itself. The ordering force pulled its two endpoints together to swap them, and SeparateOverlaps resolved the pair along its axis of least overlap - which for two bodies at the same height is X, the very axis the swap has to travel. The two passes fought to a standstill and the pair sat at exactly the horizontal clearance distance, on the wrong side of one another, for as long as the simulation ran. In the node editor that leaves the link drawn behind both bodies, since links render beneath the node backgrounds. MarkReorderingBodies flags each endpoint of a backward edge every substep, and SeparateOverlaps separates a flagged pair vertically instead. That leaves X free for the ordering force to walk them past each other, and because they go around rather than through, nothing is ever drawn overlapping. Once the order is right the edge is no longer backward, the flag clears and the pair settles back into a row. The flag is per body rather than per pair, so a body that is reordering also slides around unrelated bodies standing between it and its place - the shape any graph past two nodes takes. A force was tried first and cannot work here: a spring that decays to zero at the clearance loses to gravity, whose magnitude is normalised and so constant. They balance around 120px for the repro pair against the 165px needed, so the vertical overlap never clears. The correction has to be positional. DirectionalBias still gates all of this, so a layout with ordering off behaves exactly as before. Choosing the axis up front also retires the nested ternary the push used to build. Covered by BackwardEdge_SwapsTheEndpointsIntoOrder, which pins the repro (it crosses within about 30 steps and settles to a clean row), and BackwardEdge_SlidesPastAnUnrelatedBodyInItsPath for the pinned obstacle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D2KNUKr1xJPTEdDmeF2tQN
The quality gate failed at 16.4% duplication on new code against a limit of 3%, and Analyze & Release failed only because of that gate. Coverage passed at 100%. Both new tests spelled out the same thirteen lines: build a layout, build a body list, build a one-edge list, step it 1200 times, then work out the two horizontal centres. SettleBackwardEdge now owns everything from the layout to the centres it returns, and a Body factory collapses each placement from a five-argument constructor to one call, so what is left in each test is the graph it places and the claim it makes about it. No behaviour change: same graphs, same step count, same assertions, and the new lines in LayoutCore stay fully covered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D2KNUKr1xJPTEdDmeF2tQN
The duplication gate stayed at 16.1% after the tests were shared, because the tests were never the bulk of it. MarkReorderingBodies opened with the same fifteen lines as CalculateDirectionalForces and ApplyDirectionalConstraints - the bias guard, the edge loop, the index bounds check and both centre X values - token for token, which is most of what that method was. So there is no third pass now. CalculateDirectionalForces already computes currentGap for every edge, and a backward edge is exactly currentGap < 0, so it sets the flags where it stands. The array is cleared at the top of that pass, ahead of the bias guard, so a layout with ordering off clears rather than keeps stale flags. The flags are now read a substep after they are written, since this pass runs before integration and SeparateOverlaps runs after. A body moves at most MaxVelocity * dt in that window, well under the clearance the flag guards. LayoutCore's new lines drop from 25 to 15, all still covered, and the behaviour is unchanged: 33 layout tests and 71 node editor tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D2KNUKr1xJPTEdDmeF2tQN
|
matt-edmondson
deleted the
claude/force-directed-vertical-links-4zjaqb
branch
September 8, 2026 07:43
This was referenced Sep 8, 2026
This was referenced Sep 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A backward edge — one whose source sits to the right of its target — could never fix itself. In the node editor that leaves the link drawn behind both bodies, because links render in a channel beneath the node backgrounds, so only the two pin stubs are visible.
Two passes were fighting:
CalculateDirectionalForcespulls the endpoints together along X to swap them.SeparateOverlapsresolves an overlapping pair along its axis of least overlap. For two bodies at the same height that axis is X — the very axis the swap has to travel — so it shoves them straight back apart.They fight to a standstill and the pair parks at exactly the horizontal clearance, on the wrong side, for as long as the simulation runs. The repro pins it precisely: two bodies 160 and 270 wide stall
235pxapart, which is(160 + 270) / 2 + 20—SeparateOverlaps's clearance to the pixel.Fix
CalculateDirectionalForcesalready computescurrentGapfor every edge, and a backward edge is exactlycurrentGap < 0, so it flags both endpoints as reordering where it stands — no extra pass over the edges.SeparateOverlapsthen separates a flagged pair vertically instead of along its least-overlap axis. That leaves X free for the ordering force to walk them past each other, and because they go around rather than through, nothing is ever drawn overlapping on the way. Once the order is right the edge is no longer backward, the flag clears, and the pair settles back into a row.The flag is per body, not per pair, so a body that is reordering also slides around unrelated bodies standing between it and its place — the shape any graph past two nodes takes.
The flags are written before integration and read by
SeparateOverlapsafter it, so they are one substep stale. A body moves at mostMaxVelocity * dtin that window — about 0.4px at the default 120Hz — far under the clearance the flag guards.A force was tried first, and cannot work
The natural first attempt is a vertical splay force that opens a gap wider than the pair's clearance. It equilibrates short of the target and the vertical overlap never clears:
A spring that decays to zero exactly at the clearance always loses to gravity, whose magnitude is normalised and therefore constant. The correction has to be positional, which is why it lives in
SeparateOverlapsrather than in a force pass.Notes
DirectionalBiasstill gates all of it, so a layout with ordering off behaves exactly as before. The flag array is cleared ahead of that guard, so ordering-off clears stale flags rather than keeping them.S3358pair SonarCloud had been reporting on this file.LayoutCore.Testing
BackwardEdge_SwapsTheEndpointsIntoOrder— pins the repro. Measured: the pair crosses within ~30 steps (0.5 simulated seconds) and settles todx=235, dy≈0, a clean horizontal row. Also asserts they end up clear, not stacked, proving they went around rather than through.BackwardEdge_SlidesPastAnUnrelatedBodyInItsPath— a pinned third body in the path, which must not move.Results:
ForceDirectedLayout.Tests33/33,ImGui.NodeEditor.Tests71/71, andImGui.NodeEditor/ForceDirectedLayout.Native/ImGuiAppDemoall build clean in Release. I also ranSonarAnalyzer.CSharp10.33 locally over the changed projects: zero S-rule findings, and the pre-existingS3358pair is gone.Not verified visually — this container is headless, and the demo UI suite cannot start here because the repo's PNGs are Git LFS pointers and
git-lfsis not installed. The geometry is measured and unit-tested, but whether the reordering looks right in the editor is worth a glance.🤖 Generated with Claude Code
https://claude.ai/code/session_01D2KNUKr1xJPTEdDmeF2tQN