From 7a3fe47c4264becee798f93acb9c15c5dd5a250f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Pen=CC=83alba?= Date: Sat, 4 Jul 2026 20:23:03 +0200 Subject: [PATCH 1/8] Improve the layout algorithm to minimize line cross --- .../src/components/graph/layout.test.ts | 22 ++ src/renderer/src/components/graph/layout.ts | 126 +++++------ .../src/components/graph/packing.test.ts | 138 ++++++++++++ src/renderer/src/components/graph/packing.ts | 196 ++++++++++++++++++ 4 files changed, 410 insertions(+), 72 deletions(-) create mode 100644 src/renderer/src/components/graph/packing.test.ts create mode 100644 src/renderer/src/components/graph/packing.ts diff --git a/src/renderer/src/components/graph/layout.test.ts b/src/renderer/src/components/graph/layout.test.ts index 5a61622..5cd1ad6 100644 --- a/src/renderer/src/components/graph/layout.test.ts +++ b/src/renderer/src/components/graph/layout.test.ts @@ -238,6 +238,28 @@ describe('layoutGraph', () => { expect(kinds.filter((k) => k === 'fork')).toHaveLength(2) }) + test('a branch nested inside a longer-lived one packs above it', () => { + // feat forked first but merges LAST; fix forks and merges back entirely + // within feat's lifetime. Fork-order packing would give feat row 1 and + // slice fix's fork/merge connectors through its capsule — merge-column + // order nests the family crossing-free: fix rides row 1, feat hangs + // below with its verticals falling outside fix's span. + const layout = layoutGraph( + input([ + commit('m2', ['m1', 'f2'], 'HEAD -> main', "Merge branch 'feat'"), + commit('f2', ['f1'], 'feat'), + commit('m1', ['b', 'x1'], '', "Merge branch 'fix'"), + commit('x1', ['b'], 'fix'), + commit('b', ['a']), + commit('f1', ['a']), + commit('a', []) + ]) + ) + expect(rowNamed(layout, 'fix').index).toBe(1) + expect(rowNamed(layout, 'feat').index).toBe(2) + expect(layout.rowCount).toBe(3) + }) + test('branch base hash feeds the branch-changes view', () => { const layout = layoutGraph( input([ diff --git a/src/renderer/src/components/graph/layout.ts b/src/renderer/src/components/graph/layout.ts index a146c5e..c72b397 100644 --- a/src/renderer/src/components/graph/layout.ts +++ b/src/renderer/src/components/graph/layout.ts @@ -15,16 +15,20 @@ // deleted after merging) become "unnamed" chains, labelled from the merge // commit's subject when it records the branch name. // -// Rows are then PACKED: the mainline keeps row 0 to itself, release lines -// stack directly beneath it (newest version first — a stable "spine stack" -// that keeps maintenance branches in the same rows), and every other chain -// goes to the lowest row BELOW the chain it forked from where its column span -// (plus room for its label) doesn't collide — child branches hang beneath -// their parent, and short-lived branches that never overlap in time share a -// row instead of staircasing down the canvas. +// Rows are then PACKED (see packing.ts): the mainline keeps row 0 to itself, +// release lines stack directly beneath it (newest version first — a stable +// "spine stack" that keeps maintenance branches in the same rows), and every +// other chain goes to a row BELOW the chain it forked from where its column +// span (plus room for its label) doesn't collide — child branches hang +// beneath their parent, and short-lived branches that never overlap in time +// share a row instead of staircasing down the canvas. Among the rows that +// fit, the packer picks the one whose connector lines cross the least, so a +// branch that merges back sooner sits closer to its parent and long-lived +// branches sink below the families nested inside their lifetime. import type { Commit } from '@shared/types' import { type CommitRef, parseRefs } from '@/lib/format' +import { type PackChain, packRows, type VerticalStub } from './packing' import { compareReleaseVersions, releaseVersionWithOverride } from './releases' export type GraphRowKind = 'branch' | 'remote' | 'detached' | 'unnamed' @@ -457,11 +461,12 @@ export function layoutGraph(input: GraphInput): GraphLayout { span[id].end = slot }) - // ── Row packing: mainline alone on row 0; everyone else first-fit below ─── - // "As near to main as possible": each chain takes the lowest row whose - // occupied column intervals don't collide with its own (padded for the - // label) — but never above the chain it forked from: a child branch always - // hangs beneath its parent, however new its tip or where HEAD sits. + // ── Row packing: mainline alone on row 0; everyone else packed below ────── + // "As near to main as possible, crossing as little as possible": the packer + // (packing.ts) reuses rows wherever footprints allow — never opening a new + // row while an existing one fits — and among the fitting rows takes the one + // whose connector verticals cross the least. A child branch never packs + // above the chain it forked from, however new its tip or where HEAD sits. // Packing priority follows the branch the user is ON — the empty lane when // HEAD sits on a zero-commit branch. The mainline fallback stays the anchor // chain: an empty lane can't own the row-0 spine. @@ -472,38 +477,18 @@ export function layoutGraph(input: GraphInput): GraphLayout { ) const mainChain = defaultChain !== -1 ? defaultChain : (anchorHeadChain ?? (chains.length > 0 ? 0 : -1)) - const rowOfChain = new Map() - if (mainChain >= 0) rowOfChain.set(mainChain, 0) // A chain's fork base: the commit it grew from — for an empty chain, the // anchor commit itself (the lane grows from where its first commit will // fork off). const baseHashOf = (id: number): string | null => chains[id].empty ? chains[id].tipHash : (span[id].oldest?.parents[0] ?? null) - // The chain owning the commit this chain grew from (its fork parent), and - // the fork depth below the mainline (mainline 0, branches off it 1, …). - // Depth orders the packing parent-before-child, so every child's floor row - // is already resolved when its turn comes. + // The chain owning the commit this chain grew from (its fork parent). const parentChainOf = (id: number): number | undefined => { const base = baseHashOf(id) return base === null ? undefined : chainOf.get(base) } - const depths = new Map() - const depthOf = (id: number): number => { - if (id === mainChain) return 0 - const cached = depths.get(id) - if (cached !== undefined) return cached - // Chains can't cycle (bases only point at older columns); keeps the walk total. - depths.set(id, 1) - const parent = parentChainOf(id) - const depth = parent === undefined ? 1 : depthOf(parent) + 1 - depths.set(id, depth) - return depth - } - // Release lines pack before everything else, newest version first, so they - // stack directly under the mainline — maintenance branches always live in - // the same rows, whatever else is going on. Then by fork depth (parents - // before their children), HEAD's chain first within its depth, then the - // rest by start column so packing stays dense. + // Release lines stack directly under the mainline, newest version first — + // maintenance branches always live in the same rows, whatever else is on. const releaseRank = new Map() chains .map((chain, id) => ({ id, version: releaseVersionOfChain(chain, input) })) @@ -512,22 +497,28 @@ export function layoutGraph(input: GraphInput): GraphLayout { .forEach((entry, rank) => { releaseRank.set(entry.id, rank) }) - const NOT_RELEASE = Number.MAX_SAFE_INTEGER - const others = chains - .map((_, id) => id) - .filter((id) => id !== mainChain) - .sort((a, b) => { - const releases = (releaseRank.get(a) ?? NOT_RELEASE) - (releaseRank.get(b) ?? NOT_RELEASE) - if (releases !== 0) return releases - const depth = depthOf(a) - depthOf(b) - if (depth !== 0) return depth - if (a === headChain) return -1 - if (b === headChain) return 1 - return span[a].start - span[b].start - }) - // Occupied intervals per row (rows 1+); n chains → tiny arrays, linear scan. - const occupied: { start: number; end: number }[][] = [] - for (const id of others) { + // Every vertical connector the diagram will draw, as stubs on both of its + // end chains (see render.ts for the routing): the fork drop at a chain's + // base column, and one per merge at the merge commit's column — the tip + // lead-out and received update-merges alike. The packer prices candidate + // rows by the crossings these verticals cause. + const stubsOf: VerticalStub[][] = chains.map(() => []) + const addStubPair = (a?: number, b?: number, column?: number) => { + if (a === undefined || b === undefined || a === b || column === undefined) return + stubsOf[a].push({ column, other: b }) + stubsOf[b].push({ column, other: a }) + } + chains.forEach((_, id) => { + addStubPair(id, parentChainOf(id), columnOf.get(baseHashOf(id) ?? '')) + }) + for (const c of kept) { + for (const parent of c.parents.slice(1)) { + addStubPair(chainOf.get(parent), chainOf.get(c.hash), columnOf.get(c.hash)) + } + } + const packChains: PackChain[] = [] + chains.forEach((chain, id) => { + if (id === mainChain) return // Reserve the chain's whole visual footprint: the label pad and the fork // lead-in to the left, the merge lead-out to the right (the orthogonal // connector runs along the row — see render.ts) — so no other chain on @@ -537,28 +528,19 @@ export function layoutGraph(input: GraphInput): GraphLayout { // commit is the OWNER's lead-out to reserve, not the empty lane's. const mergeChild = chains[id].empty ? undefined : mergeChildOf.get(chains[id].tipHash) const mergeColumn = mergeChild ? columnOf.get(mergeChild.hash) : undefined - const interval = { + packChains.push({ + id, start: Math.min(span[id].start - LABEL_PAD_COLUMNS, forkColumn ?? Number.MAX_SAFE_INTEGER), - end: Math.max(span[id].end + 1, mergeColumn ?? -1) - } - // A child branch never packs above its parent: the scan starts at the - // parent's row, so the first candidate row already sits below it. Release - // lines are exempt — their spine stack under the mainline is fixed. - const parent = parentChainOf(id) - let row = releaseRank.has(id) || parent === undefined ? 0 : (rowOfChain.get(parent) ?? 0) - while (true) { - const taken = occupied[row] - if (!taken?.some((t) => interval.start <= t.end && t.start <= interval.end)) { - const rowIntervals = taken ?? [] - if (!taken) occupied[row] = rowIntervals - rowIntervals.push(interval) - rowOfChain.set(id, row + 1) - break - } - row++ - } - } - const rowCount = chains.length === 0 ? 0 : occupied.length + 1 + end: Math.max(span[id].end + 1, mergeColumn ?? -1), + parent: parentChainOf(id) ?? null, + releaseRank: releaseRank.get(id) ?? null, + isHead: id === headChain, + stubs: stubsOf[id] + }) + }) + const rowOfChain = packRows(packChains, mainChain) + let rowCount = 0 + for (const row of rowOfChain.values()) rowCount = Math.max(rowCount, row + 1) // ── Rows (one per chain), colors, and the sorted output list ────────────── const rows: GraphRow[] = chains.map((chain, id) => ({ diff --git a/src/renderer/src/components/graph/packing.test.ts b/src/renderer/src/components/graph/packing.test.ts new file mode 100644 index 0000000..836c85f --- /dev/null +++ b/src/renderer/src/components/graph/packing.test.ts @@ -0,0 +1,138 @@ +import { describe, expect, test } from 'bun:test' +import { type PackChain, packRows } from './packing' + +const MAIN = 0 + +/** A chain hanging off the mainline unless overridden. */ +function chain(id: number, overrides: Partial = {}): PackChain { + return { + id, + start: 0, + end: 0, + parent: MAIN, + releaseRank: null, + isHead: false, + stubs: [], + ...overrides + } +} + +describe('packRows', () => { + test('the mainline is seeded on row 0 and everything packs below it', () => { + const rows = packRows([chain(1, { start: 0, end: 5 })], MAIN) + expect(rows.get(MAIN)).toBe(0) + expect(rows.get(1)).toBe(1) + }) + + test('non-overlapping chains share a row (density before everything)', () => { + const rows = packRows( + [chain(1, { start: 0, end: 5 }), chain(2, { start: 7, end: 12 })], + MAIN + ) + expect(rows.get(1)).toBe(1) + expect(rows.get(2)).toBe(1) + }) + + test('overlapping chains stack on separate rows', () => { + const rows = packRows( + [chain(1, { start: 0, end: 5 }), chain(2, { start: 3, end: 12 })], + MAIN + ) + expect(rows.get(1)).not.toBe(rows.get(2)) + }) + + test('nested lifetimes pack inner-above-outer, ordered by merge column', () => { + // Outer forked first but merges LAST (end 20); inner lives entirely + // inside its span. Fork-order packing would put outer on row 1 and slice + // inner's connectors through it — merge-column order nests them cleanly. + const outer = chain(1, { start: 0, end: 20, stubs: [{ column: 0, other: MAIN }] }) + const inner = chain(2, { start: 5, end: 12, stubs: [{ column: 12, other: MAIN }] }) + const rows = packRows([outer, inner], MAIN) + expect(rows.get(2)).toBe(1) + expect(rows.get(1)).toBe(2) + }) + + test('a backport drifts down to hug the release line it merges into', () => { + // Rows 1 and 3 both have room, but the chain merges into the release + // line on row 3: parked on row 1 its merge connector would drop across + // row 2's capsule; drifting to the target's own row crosses nothing. + const rows = packRows( + [ + chain(1, { start: 0, end: 15, releaseRank: 0 }), + chain(2, { start: 0, end: 50, releaseRank: 1 }), + chain(3, { start: 0, end: 15, releaseRank: 2 }), + chain(4, { start: 20, end: 30, stubs: [{ column: 30, other: 3 }] }) + ], + MAIN + ) + expect(rows.get(3)).toBe(3) + expect(rows.get(4)).toBe(3) + // Sanity: without the merge connector, the same chain takes row 1. + const calm = packRows( + [ + chain(1, { start: 0, end: 15, releaseRank: 0 }), + chain(2, { start: 0, end: 50, releaseRank: 1 }), + chain(3, { start: 0, end: 15, releaseRank: 2 }), + chain(4, { start: 20, end: 30 }) + ], + MAIN + ) + expect(calm.get(4)).toBe(1) + }) + + test('clarity never opens a new row while an existing row fits', () => { + // b's merge lead-out drops through row 1's open stretch. c fits only + // that stretch among the existing rows — a fresh row below would dodge + // the crossing, but density wins: the diagram must not grow taller. + const rows = packRows( + [ + chain(1, { start: 0, end: 6 }), + chain(2, { start: 0, end: 9, stubs: [{ column: 9, other: MAIN }] }), + chain(3, { start: 8, end: 20 }) + ], + MAIN + ) + expect(rows.get(3)).toBe(1) + }) + + test('release lines keep the spine stack: first-fit by rank, no drift', () => { + // Both releases overlap a vertical-riddled row 1 zone; they must still + // stack in version order right under the mainline. + const noisy = chain(3, { + start: 30, + end: 40, + stubs: [{ column: 35, other: MAIN }] + }) + const rows = packRows( + [ + noisy, + chain(1, { start: 0, end: 50, releaseRank: 0 }), + chain(2, { start: 0, end: 50, releaseRank: 1 }) + ], + MAIN + ) + expect(rows.get(1)).toBe(1) + expect(rows.get(2)).toBe(2) + }) + + test('a child never packs above its parent, whatever the cost says', () => { + const parent = chain(1, { start: 0, end: 10 }) + const child = chain(2, { start: 30, end: 40, parent: 1 }) + const rows = packRows([parent, child], MAIN) + expect(rows.get(2)).toBe((rows.get(1) ?? 0) + 1) + }) + + test('HEAD packs first within its depth', () => { + // Both want row 1 and overlap; HEAD wins it despite merging later. + const other = chain(1, { start: 0, end: 5 }) + const head = chain(2, { start: 3, end: 12, isHead: true }) + const rows = packRows([other, head], MAIN) + expect(rows.get(2)).toBe(1) + expect(rows.get(1)).toBe(2) + }) + + test('no mainline (-1) still packs from row 1', () => { + const rows = packRows([chain(1, { parent: null, start: 0, end: 3 })], -1) + expect(rows.get(1)).toBe(1) + }) +}) diff --git a/src/renderer/src/components/graph/packing.ts b/src/renderer/src/components/graph/packing.ts new file mode 100644 index 0000000..d681f3d --- /dev/null +++ b/src/renderer/src/components/graph/packing.ts @@ -0,0 +1,196 @@ +// Row packing for the Graph tab: assigns every reconstructed chain a packed +// row below the mainline, reusing rows wherever column footprints allow. +// Pure — no DOM, no git — and driven entirely by the footprints and connector +// stubs layout.ts hands it, so it's unit-testable in isolation. +// +// Two forces shape every placement, strictly in this order: +// +// - DENSITY. A chain never opens a new row while an existing row can host its +// footprint — short-lived branches share rows instead of staircasing down +// the canvas, and the diagram stays as compact as plain first-fit. +// - CLARITY. Among the rows that fit, take the one whose connector lines +// cross the least. Every chain hangs off the diagram by a handful of +// VERTICAL connectors (the fork drop from its parent, the merge lead-out +// into its target, one per merge it received — see render.ts for the +// orthogonal routing), and each vertical crosses every row it passes whose +// footprint covers its column. Candidate rows are scored on both sides of +// that relation: the chain's own verticals against the rows already placed, +// and the already-placed verticals against the footprint the chain would +// lay down. A lower-scoring row further from the parent wins only when the +// saved crossings outweigh the drift (ROW_DRIFT_PENALTY). +// +// Placement order also serves clarity: within a fork depth, chains pack by +// MERGE column (footprint end), not fork column. A branch that merges back +// sooner sits closer to its parent, so a family of branches nested inside a +// long-lived one renders with the long one underneath — its verticals fall +// outside the short ones' spans and the family crosses nothing, where +// fork-order packing would slice every short branch's connectors through the +// long capsule. Nested lifetimes overlap pairwise, so this costs no rows. +// +// Hard constraints, never traded away: the mainline owns row 0 alone; +// release lines stack directly beneath it in version order (pure first-fit, +// no drift — their rows are a stable spine the eye navigates by); a child +// chain always packs strictly below its fork parent. + +/** A vertical connector this chain shares with another chain: the fork drop + * from its parent, its merge lead-out, or a merge it received. Becomes a + * real segment (and starts costing crossings) once both ends are placed. */ +export interface VerticalStub { + /** Column the connector runs down. */ + column: number + /** The chain on the connector's other end. */ + other: number +} + +/** One chain as the packer sees it. */ +export interface PackChain { + id: number + /** Reserved footprint: label pad + fork lead-in … merge lead-out. */ + start: number + end: number + /** Chain owning the commit this one forked from — its row floor — or null + * for a chain that starts at a root commit (or off-window). */ + parent: number | null + /** Position in the release spine stack (0 = newest version), or null. */ + releaseRank: number | null + /** The checked-out branch's chain: packs first within its fork depth. */ + isHead: boolean + stubs: readonly VerticalStub[] +} + +/** Crossings a row of drift must save to be worth moving away from the + * parent: one saved crossing justifies sliding up to two rows down, so + * children stay near their family unless moving genuinely clears lines. */ +const ROW_DRIFT_PENALTY = 0.35 + +/** Fitting rows scored per chain. Bounds the cost work on huge (structure- + * only) graphs; with drift priced in, rows past the first few fits almost + * never win anyway. */ +const MAX_CANDIDATES = 6 + +const NOT_RELEASE = Number.MAX_SAFE_INTEGER + +/** + * Pack every chain onto a row. `chains` excludes the mainline; `mainId` (the + * mainline's chain id, or -1 when there is none) is seeded onto row 0 so + * floors and connector spans can resolve against it. Returns chain id → row. + */ +export function packRows(chains: readonly PackChain[], mainId: number): Map { + const rowOf = new Map() + if (mainId >= 0) rowOf.set(mainId, 0) + + // Fork depth below the mainline (mainline 0, branches off it 1, …) orders + // packing parent-before-child, so every child's floor row is already + // resolved when its turn comes. Chains can't cycle (fork bases only point + // at older columns), but the guard keeps a malformed input walk total. + const byId = new Map(chains.map((c) => [c.id, c])) + const depths = new Map() + const depthOf = (id: number): number => { + if (id === mainId) return 0 + const cached = depths.get(id) + if (cached !== undefined) return cached + depths.set(id, 1) + const parent = byId.get(id)?.parent + const depth = parent == null ? 1 : depthOf(parent) + 1 + depths.set(id, depth) + return depth + } + + // Release lines first (newest version first — the spine stack), then by + // fork depth, HEAD's chain first within its depth, then by merge column so + // sooner-merged siblings pack nearer the parent (see the header), ids last + // for determinism. + const order = [...chains].sort( + (a, b) => + (a.releaseRank ?? NOT_RELEASE) - (b.releaseRank ?? NOT_RELEASE) || + depthOf(a.id) - depthOf(b.id) || + Number(b.isHead) - Number(a.isHead) || + a.end - b.end || + a.id - b.id + ) + + // Occupied footprints per row (index 0 is the mainline's — never packed + // into), and the columns of registered verticals passing THROUGH each row. + // n chains → tiny arrays; linear scans stay cheap. + const taken: { start: number; end: number }[][] = [[]] + const through: number[][] = [] + + const fits = (row: number, c: PackChain): boolean => + !taken[row]?.some((t) => c.start <= t.end && t.start <= c.end) + const covered = (row: number, column: number): boolean => + taken[row]?.some((t) => t.start <= column && column <= t.end) === true + + /** Crossings placing `c` at `row` would create, both ways: its verticals + * against placed rows, and placed verticals against its footprint. */ + const crossings = (c: PackChain, row: number): number => { + let cost = 0 + for (const column of through[row] ?? []) { + if (column >= c.start && column <= c.end) cost++ + } + for (const stub of c.stubs) { + const otherRow = rowOf.get(stub.other) + if (otherRow === undefined) continue + const hi = Math.max(otherRow, row) + for (let q = Math.min(otherRow, row) + 1; q < hi; q++) { + if (covered(q, stub.column)) cost++ + } + } + return cost + } + + for (const chain of order) { + // A child never packs above its parent: candidates start strictly below + // it. Release lines are exempt — their spine stack is fixed from row 1. + const floor = + chain.releaseRank !== null || chain.parent === null + ? 1 + : (rowOf.get(chain.parent) ?? 0) + 1 + let row = floor + if (chain.releaseRank !== null) { + // Releases: pure first-fit. Cost-driven drift would let a busy window + // shuffle the maintenance rows users navigate by. + while (!fits(row, chain)) row++ + } else { + // Gather the nearest fitting rows, then keep the cheapest; ties go to + // the row nearest the parent. A NEW row (past the current edge) is a + // candidate only when no existing row fits — clarity never buys itself + // a taller diagram (density is the promise the packer keeps first). + const candidates: number[] = [] + for (let r = floor; candidates.length < MAX_CANDIDATES; r++) { + if (r >= taken.length) { + if (candidates.length === 0) candidates.push(r) + break + } + if (fits(r, chain)) candidates.push(r) + } + let best = Number.MAX_VALUE + for (const r of candidates) { + const cost = crossings(chain, r) + const score = cost + (r - candidates[0]) * ROW_DRIFT_PENALTY + if (score < best) { + best = score + row = r + } + // Candidates ascend, so later scores are at least their (larger) + // drift: a crossing-free row this close can't be beaten. + if (cost === 0) break + } + } + + rowOf.set(chain.id, row) + while (taken.length <= row) taken.push([]) + taken[row].push({ start: chain.start, end: chain.end }) + // Register the verticals whose far end is now placed. Each registers + // exactly once — from whichever end packs second (the mainline never + // packs, so its connectors always register from the other end). + for (const stub of chain.stubs) { + const otherRow = rowOf.get(stub.other) + if (otherRow === undefined || stub.other === chain.id) continue + const hi = Math.max(otherRow, row) + for (let q = Math.min(otherRow, row) + 1; q < hi; q++) { + ;(through[q] ??= []).push(stub.column) + } + } + } + return rowOf +} From 4bec5828bb26636156a7dc3546c969675fb4f20e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Pen=CC=83alba?= Date: Sat, 4 Jul 2026 20:33:55 +0200 Subject: [PATCH 2/8] Fixed tests --- src/renderer/src/components/graph/packing.ts | 208 +++++++++++++++---- 1 file changed, 173 insertions(+), 35 deletions(-) diff --git a/src/renderer/src/components/graph/packing.ts b/src/renderer/src/components/graph/packing.ts index d681f3d..47fa900 100644 --- a/src/renderer/src/components/graph/packing.ts +++ b/src/renderer/src/components/graph/packing.ts @@ -3,38 +3,41 @@ // Pure — no DOM, no git — and driven entirely by the footprints and connector // stubs layout.ts hands it, so it's unit-testable in isolation. // -// Two forces shape every placement, strictly in this order: +// Two forces shape the arrangement, strictly in this order: // -// - DENSITY. A chain never opens a new row while an existing row can host its -// footprint — short-lived branches share rows instead of staircasing down -// the canvas, and the diagram stays as compact as plain first-fit. -// - CLARITY. Among the rows that fit, take the one whose connector lines -// cross the least. Every chain hangs off the diagram by a handful of -// VERTICAL connectors (the fork drop from its parent, the merge lead-out -// into its target, one per merge it received — see render.ts for the -// orthogonal routing), and each vertical crosses every row it passes whose -// footprint covers its column. Candidate rows are scored on both sides of -// that relation: the chain's own verticals against the rows already placed, -// and the already-placed verticals against the footprint the chain would -// lay down. A lower-scoring row further from the parent wins only when the -// saved crossings outweigh the drift (ROW_DRIFT_PENALTY). +// - DENSITY. Chains pack first-fit in fork order (the left-edge rule — the +// row count stays minimal for the footprints given), and a chain never +// opens a new row while an existing row can host it: clarity never buys +// itself a taller diagram. +// - CLARITY. Every chain hangs off the diagram by a handful of VERTICAL +// connectors (the fork drop from its parent, the merge lead-out into its +// target, one per merge it received — see render.ts for the orthogonal +// routing), and each vertical crosses every row it passes whose footprint +// covers its column. Two mechanisms cut those crossings without touching +// the row count: +// 1. Placement drift — among the rows that FIT, take the one whose +// connectors cross the least (the chain's own verticals against the +// rows already placed, and the already-placed verticals against the +// footprint the chain would lay down). A farther row wins only when +// the saved crossings outweigh the drift from the parent +// (ROW_DRIFT_PENALTY) — e.g. a backport slides down to hug the +// release line it merges into. +// 2. Nested-pair swaps — fork-order packing puts a long-lived branch +// ABOVE the short branches that fork and merge back entirely inside +// its lifetime, so every short branch's connectors slice through the +// long capsule. A bounded post-pass swaps such pairs (long one sinks, +// family rises) whenever that strictly reduces crossings: the long +// branch's verticals fall outside the family's spans, and a fully +// nested family unwinds to zero crossings. Swaps exchange existing +// rows, so density is untouched by construction. // -// Placement order also serves clarity: within a fork depth, chains pack by -// MERGE column (footprint end), not fork column. A branch that merges back -// sooner sits closer to its parent, so a family of branches nested inside a -// long-lived one renders with the long one underneath — its verticals fall -// outside the short ones' spans and the family crosses nothing, where -// fork-order packing would slice every short branch's connectors through the -// long capsule. Nested lifetimes overlap pairwise, so this costs no rows. -// -// Hard constraints, never traded away: the mainline owns row 0 alone; -// release lines stack directly beneath it in version order (pure first-fit, -// no drift — their rows are a stable spine the eye navigates by); a child +// Hard constraints, never traded: the mainline owns row 0 alone; release +// lines stack directly beneath it in version order (pure first-fit, no drift, +// no swaps — their rows are a stable spine the eye navigates by); a child // chain always packs strictly below its fork parent. /** A vertical connector this chain shares with another chain: the fork drop - * from its parent, its merge lead-out, or a merge it received. Becomes a - * real segment (and starts costing crossings) once both ends are placed. */ + * from its parent, its merge lead-out, or a merge it received. */ export interface VerticalStub { /** Column the connector runs down. */ column: number @@ -68,14 +71,42 @@ const ROW_DRIFT_PENALTY = 0.35 * never win anyway. */ const MAX_CANDIDATES = 6 +/** Swap sweeps before the improvement pass stops. Each accepted swap + * strictly reduces crossings, so the pass converges — usually in one. */ +const MAX_SWEEPS = 3 + +/** Swap evaluations before the improvement pass bails, and the total work + * those evaluations may cost (each is an O(chains + verticals) scan). The + * work cap shrinks the budget on pathological windows — thousands of + * overlapping chains — where the pass would burn tens of milliseconds + * polishing a graph that's unreadable at that scale anyway; every + * human-readable graph gets the full budget many times over. */ +const SWAP_BUDGET = 4000 +const SWAP_WORK_CAP = 1_500_000 + const NOT_RELEASE = Number.MAX_SAFE_INTEGER +/** A vertical connector with both ends known: `a`/`b` are chain ids (the + * mainline included), deduplicated from the two chains' mirrored stubs. */ +interface Vertical { + column: number + a: number + b: number +} + /** * Pack every chain onto a row. `chains` excludes the mainline; `mainId` (the * mainline's chain id, or -1 when there is none) is seeded onto row 0 so * floors and connector spans can resolve against it. Returns chain id → row. */ export function packRows(chains: readonly PackChain[], mainId: number): Map { + const rowOf = pack(chains, mainId) + swapNestedPairs(chains, mainId, rowOf) + return rowOf +} + +/** First-fit packing in fork order, with crossing-aware drift (see header). */ +function pack(chains: readonly PackChain[], mainId: number): Map { const rowOf = new Map() if (mainId >= 0) rowOf.set(mainId, 0) @@ -97,15 +128,14 @@ export function packRows(chains: readonly PackChain[], mainId: number): Map (a.releaseRank ?? NOT_RELEASE) - (b.releaseRank ?? NOT_RELEASE) || depthOf(a.id) - depthOf(b.id) || Number(b.isHead) - Number(a.isHead) || - a.end - b.end || + a.start - b.start || a.id - b.id ) @@ -120,8 +150,8 @@ export function packRows(chains: readonly PackChain[], mainId: number): Map taken[row]?.some((t) => t.start <= column && column <= t.end) === true - /** Crossings placing `c` at `row` would create, both ways: its verticals - * against placed rows, and placed verticals against its footprint. */ + // Crossings placing `c` at `row` would create, both ways: its verticals + // against placed rows, and placed verticals against its footprint. const crossings = (c: PackChain, row: number): number => { let cost = 0 for (const column of through[row] ?? []) { @@ -153,8 +183,7 @@ export function packRows(chains: readonly PackChain[], mainId: number): Map= taken.length) { @@ -194,3 +223,112 @@ export function packRows(chains: readonly PackChain[], mainId: number): Map +): void { + // Every vertical with both ends in the window, deduplicated: each pair's + // stubs mirror each other, so take a vertical from its lower-id end (the + // mainline carries no stub list — its verticals come from the other end). + const verticals: Vertical[] = [] + for (const c of chains) { + for (const stub of c.stubs) { + if ((c.id < stub.other || stub.other === mainId) && rowOf.has(stub.other)) { + verticals.push({ column: stub.column, a: c.id, b: stub.other }) + } + } + } + const children = new Map() + for (const c of chains) { + if (c.parent === null) continue + let list = children.get(c.parent) + if (!list) children.set(c.parent, (list = [])) + list.push(c) + } + + /** Crossings involving `o` or `i` if they sat on (rowO, rowI) — the only + * terms a swap can change, counted from scratch so the pass needs no + * incremental bookkeeping beyond rowOf itself. */ + const pairCost = (o: PackChain, i: PackChain, rowO: number, rowI: number): number => { + const rowAt = (id: number): number => + id === o.id ? rowO : id === i.id ? rowI : (rowOf.get(id) ?? 0) + const crossesInterval = (v: Vertical, z: PackChain): boolean => { + if (z.id === v.a || z.id === v.b) return false + if (v.column < z.start || v.column > z.end) return false + const rz = rowAt(z.id) + const ra = rowAt(v.a) + const rb = rowAt(v.b) + return Math.min(ra, rb) < rz && rz < Math.max(ra, rb) + } + let cost = 0 + for (const v of verticals) { + if (v.a === o.id || v.b === o.id || v.a === i.id || v.b === i.id) { + for (const z of chains) if (crossesInterval(v, z)) cost++ + } else { + if (crossesInterval(v, o)) cost++ + if (crossesInterval(v, i)) cost++ + } + } + return cost + } + + const fitsAt = (c: PackChain, row: number, ignoring: PackChain): boolean => + !chains.some( + (z) => + z.id !== c.id && + z.id !== ignoring.id && + rowOf.get(z.id) === row && + c.start <= z.end && + z.start <= c.end + ) + /** Parent above, every child below — with the pair's swapped rows. */ + const floorsHold = (c: PackChain, row: number, otherId: number, otherRow: number): boolean => { + const rowAt = (id: number): number | undefined => + id === otherId ? otherRow : rowOf.get(id) + const parentRow = c.parent === null ? 0 : (rowAt(c.parent) ?? 0) + if (row <= parentRow) return false + return (children.get(c.id) ?? []).every((child) => { + const childRow = rowAt(child.id) + return childRow === undefined || childRow > row + }) + } + + // Candidate pairs: `o`'s footprint contains `i`'s, yet `o` sits above it. + // Sorted by start so containment scans stay short. + const movable = chains + .filter((c) => c.releaseRank === null && rowOf.has(c.id)) + .sort((a, b) => a.start - b.start || a.id - b.id) + let budget = Math.min( + SWAP_BUDGET, + Math.ceil(SWAP_WORK_CAP / (chains.length + verticals.length + 1)) + ) + for (let sweep = 0; sweep < MAX_SWEEPS && budget > 0; sweep++) { + let improved = false + for (let x = 0; x < movable.length && budget > 0; x++) { + const o = movable[x] + for (let y = x + 1; y < movable.length && budget > 0; y++) { + const i = movable[y] + if (i.start > o.end) break // sorted: nothing later can nest in o + if (i.end > o.end) continue + const rowO = rowOf.get(o.id) ?? 0 + const rowI = rowOf.get(i.id) ?? 0 + if (rowO >= rowI) continue + budget-- + if (!fitsAt(o, rowI, i) || !fitsAt(i, rowO, o)) continue + if (!floorsHold(o, rowI, i.id, rowO) || !floorsHold(i, rowO, o.id, rowI)) continue + // Drift penalties cancel exactly (the pair trades distances), so a + // swap is judged on crossings alone — and only a STRICT win moves. + if (pairCost(o, i, rowI, rowO) < pairCost(o, i, rowO, rowI)) { + rowOf.set(o.id, rowI) + rowOf.set(i.id, rowO) + improved = true + } + } + } + if (!improved) break + } +} From cd6655fb42a2e0b379d0497632a3b32cc04397f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Pen=CC=83alba?= Date: Sat, 4 Jul 2026 20:34:01 +0200 Subject: [PATCH 3/8] Update CLAUDE --- CLAUDE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8cf6647..0a2fc51 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,8 +57,8 @@ handlers), `menu.ts`, `watcher.ts` (pushes `repo:changed`), `updater.ts`, `store orchestration spine that wires the tabs together); `components/` (one component — or the hook that drives just that feature — per file) grouped by feature: `changes/`, `history/`, `graph/` (the Graph tab's 2D branch explorer: pure layout in `layout.ts`, -canvas drawing in `render.ts`, interactions in `GraphCanvas.tsx` — keep layout logic -pure and tested), `toolbar/`, `app/` (shell screens + app-level dialogs) and `common/` (shared +crossing-aware row packing in `packing.ts`, canvas drawing in `render.ts`, interactions +in `GraphCanvas.tsx` — keep layout logic pure and tested), `toolbar/`, `app/` (shell screens + app-level dialogs) and `common/` (shared widgets and primitives — same-folder imports stay relative, cross-folder go through `@/`); `lib/` (the **shared tier**: pure logic + hooks reused by 2+ features — a hook used by exactly one feature lives in that feature's folder, not here. `lib/staging.ts` is the From f9556365f9ea34eaca70d92925c7271b2a70e6c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Pen=CC=83alba?= Date: Sat, 4 Jul 2026 21:09:12 +0200 Subject: [PATCH 4/8] More compact layout --- .../src/components/graph/layout.test.ts | 11 +- src/renderer/src/components/graph/layout.ts | 2 + .../src/components/graph/packing.test.ts | 39 +++- src/renderer/src/components/graph/packing.ts | 208 +++++++++++++----- 4 files changed, 192 insertions(+), 68 deletions(-) diff --git a/src/renderer/src/components/graph/layout.test.ts b/src/renderer/src/components/graph/layout.test.ts index 5cd1ad6..d4f9eb7 100644 --- a/src/renderer/src/components/graph/layout.test.ts +++ b/src/renderer/src/components/graph/layout.test.ts @@ -335,11 +335,12 @@ describe('layoutGraph', () => { expect(kinds).toEqual(['fork', 'line', 'line', 'merge']) }) - test('packing reserves the merge lead-out so connector runs stay clear', () => { + test('a label pad over a merge lead-out still shares the row', () => { // early sits at column 2 but merges two columns later at m1 (column 4): - // its connector runs along the row to column 4. late's reservation starts - // at column 4 — without the lead-out they'd share row 1 and the connector - // would run under late's footprint; with it, late moves down a row. + // its connector runs along the row to column 4, under late's LABEL PAD + // (columns 4–6). A pad masks connector lines (the pill base is opaque), + // so the two share row 1 — only a capsule or another pad would push + // late down a row (see packing.test.ts for those cases). const layout = layoutGraph( input([ commit('m2', ['c5', 'y'], 'HEAD -> main', "Merge branch 'late'"), @@ -354,7 +355,7 @@ describe('layoutGraph', () => { ]) ) expect(rowNamed(layout, 'early').index).toBe(1) - expect(rowNamed(layout, 'late').index).toBe(2) + expect(rowNamed(layout, 'late').index).toBe(1) }) test('release lines stack directly under the mainline, newest version first', () => { diff --git a/src/renderer/src/components/graph/layout.ts b/src/renderer/src/components/graph/layout.ts index c72b397..2f372b4 100644 --- a/src/renderer/src/components/graph/layout.ts +++ b/src/renderer/src/components/graph/layout.ts @@ -532,6 +532,8 @@ export function layoutGraph(input: GraphInput): GraphLayout { id, start: Math.min(span[id].start - LABEL_PAD_COLUMNS, forkColumn ?? Number.MAX_SAFE_INTEGER), end: Math.max(span[id].end + 1, mergeColumn ?? -1), + capStart: span[id].start, + capEnd: span[id].end, parent: parentChainOf(id) ?? null, releaseRank: releaseRank.get(id) ?? null, isHead: id === headChain, diff --git a/src/renderer/src/components/graph/packing.test.ts b/src/renderer/src/components/graph/packing.test.ts index 836c85f..2ffb2b9 100644 --- a/src/renderer/src/components/graph/packing.test.ts +++ b/src/renderer/src/components/graph/packing.test.ts @@ -3,12 +3,17 @@ import { type PackChain, packRows } from './packing' const MAIN = 0 -/** A chain hanging off the mainline unless overridden. */ +/** A chain hanging off the mainline unless overridden. Defaults make the + * whole footprint capsule (no soft zones), so interval tests read plainly. */ function chain(id: number, overrides: Partial = {}): PackChain { + const start = overrides.start ?? 0 + const end = overrides.end ?? 0 return { id, - start: 0, - end: 0, + start, + end, + capStart: start, + capEnd: end, parent: MAIN, releaseRank: null, isHead: false, @@ -95,6 +100,34 @@ describe('packRows', () => { expect(rows.get(3)).toBe(1) }) + test('a label pad may sit over a neighbor\'s merge lead-out (shares the row)', () => { + // a's capsule ends at 10 but its lead-out runs to column 15; b's label + // pad occupies 13–15. Pads mask connector lines, so the row is shared — + // under a whole-interval rule this 3-column graze would cost b a row. + const a = chain(1, { start: 0, end: 15, capStart: 0, capEnd: 10 }) + const b = chain(2, { start: 13, end: 21, capStart: 16, capEnd: 20 }) + const rows = packRows([a, b], MAIN) + expect(rows.get(1)).toBe(1) + expect(rows.get(2)).toBe(1) + }) + + test('a label pad never covers a neighbor\'s commits', () => { + // b's pad (8–10) would sit on a's capsule tail — that hides real nodes, + // so they must not share the row. + const a = chain(1, { start: 0, end: 11, capStart: 0, capEnd: 10 }) + const b = chain(2, { start: 8, end: 21, capStart: 11, capEnd: 20 }) + const rows = packRows([a, b], MAIN) + expect(rows.get(1)).not.toBe(rows.get(2)) + }) + + test('a merge lead-out never runs under a neighbor\'s capsule', () => { + // a's lead-out (11–15) would pass beneath b's commits starting at 12. + const a = chain(1, { start: 0, end: 15, capStart: 0, capEnd: 10 }) + const b = chain(2, { start: 12, end: 21, capStart: 12, capEnd: 20 }) + const rows = packRows([a, b], MAIN) + expect(rows.get(1)).not.toBe(rows.get(2)) + }) + test('release lines keep the spine stack: first-fit by rank, no drift', () => { // Both releases overlap a vertical-riddled row 1 zone; they must still // stack in version order right under the mainline. diff --git a/src/renderer/src/components/graph/packing.ts b/src/renderer/src/components/graph/packing.ts index 47fa900..880a2d4 100644 --- a/src/renderer/src/components/graph/packing.ts +++ b/src/renderer/src/components/graph/packing.ts @@ -8,7 +8,10 @@ // - DENSITY. Chains pack first-fit in fork order (the left-edge rule — the // row count stays minimal for the footprints given), and a chain never // opens a new row while an existing row can host it: clarity never buys -// itself a taller diagram. +// itself a taller diagram. Footprints themselves are zoned (see PackChain) +// so row-sharing is exactly as tight as the pixels allow — a label pad +// grazing a neighbor's merge lead-out shares the row instead of dropping +// below it for a touch no eye can see. // - CLARITY. Every chain hangs off the diagram by a handful of VERTICAL // connectors (the fork drop from its parent, the merge lead-out into its // target, one per merge it received — see render.ts for the orthogonal @@ -45,12 +48,26 @@ export interface VerticalStub { other: number } -/** One chain as the packer sees it. */ +/** One chain as the packer sees it. Its footprint has three zones: + * + * [start … capStart-1] soft head: label pad + fork lead-in + * [capStart … capEnd] capsule: the commits and spine + * [capEnd+1 … end] soft tail: breathing column + merge lead-out run + * + * Zones let row-sharing be exactly as tight as the pixels allow: nothing may + * overlap a capsule, and two label pads may not collide, but a label pad MAY + * sit over another chain's soft tail — the pill's opaque base masks the + * connector line behind it, so no ink is lost. That one relaxation is what + * lets a branch tuck beside a neighbor whose lead-out barely grazes its + * label's air, instead of dropping a whole row for a 1–3 column touch. */ export interface PackChain { id: number - /** Reserved footprint: label pad + fork lead-in … merge lead-out. */ + /** Full reserved footprint (all three zones) — what verticals cross. */ start: number end: number + /** The capsule zone. layout.ts always leaves end ≥ capEnd + 1. */ + capStart: number + capEnd: number /** Chain owning the commit this one forked from — its row floor — or null * for a chain that starts at a root commit (or off-window). */ parent: number | null @@ -71,18 +88,18 @@ const ROW_DRIFT_PENALTY = 0.35 * never win anyway. */ const MAX_CANDIDATES = 6 -/** Swap sweeps before the improvement pass stops. Each accepted swap - * strictly reduces crossings, so the pass converges — usually in one. */ +/** Improvement sweeps before the pass stops. Each accepted change strictly + * shrinks the objective, so the pass converges — usually in one or two. */ const MAX_SWEEPS = 3 -/** Swap evaluations before the improvement pass bails, and the total work - * those evaluations may cost (each is an O(chains + verticals) scan). The - * work cap shrinks the budget on pathological windows — thousands of - * overlapping chains — where the pass would burn tens of milliseconds - * polishing a graph that's unreadable at that scale anyway; every - * human-readable graph gets the full budget many times over. */ -const SWAP_BUDGET = 4000 -const SWAP_WORK_CAP = 1_500_000 +/** Work units (≈ inner-loop operations) each clarity phase may spend — + * packing's crossing scoring and the improvement pass each get one cap. + * When a phase runs out it degrades gracefully (plain first-fit placement / + * no further moves), keeping layout to a few milliseconds even on + * pathological windows — thousands of overlapping chains — where polishing + * a graph that's unreadable at that scale isn't worth the time; every + * human-readable graph finishes well inside the cap. */ +const PHASE_WORK_CAP = 1_500_000 const NOT_RELEASE = Number.MAX_SAFE_INTEGER @@ -94,6 +111,15 @@ interface Vertical { b: number } +/** Whether two footprints may NOT share a row (see PackChain's zones): + * pads and capsules never overlap each other, and a capsule never sits on + * a soft tail (a connector run through commits) — but a pad over a soft + * tail is fine, so near-miss endpoint touches stop costing whole rows. */ +const conflicts = (a: PackChain, b: PackChain): boolean => + (a.start <= b.capEnd && b.start <= a.capEnd) || // pad∪capsule vs pad∪capsule + (a.capStart <= b.end && b.capEnd < a.capEnd) || // a's capsule on b's tail + (b.capStart <= a.end && a.capEnd < b.capEnd) // b's capsule on a's tail + /** * Pack every chain onto a row. `chains` excludes the mainline; `mainId` (the * mainline's chain id, or -1 when there is none) is seeded onto row 0 so @@ -101,10 +127,20 @@ interface Vertical { */ export function packRows(chains: readonly PackChain[], mainId: number): Map { const rowOf = pack(chains, mainId) - swapNestedPairs(chains, mainId, rowOf) + improvePlacement(chains, mainId, rowOf) + compactRows(rowOf) return rowOf } +/** Drop row numbers that ended up unused (lifts can empty a row out), so the + * diagram never renders a blank lane. Row 0 stays the mainline's. */ +function compactRows(rowOf: Map): void { + const used = [...new Set([0, ...rowOf.values()])].sort((x, y) => x - y) + if (used[used.length - 1] === used.length - 1) return // already dense + const remap = new Map(used.map((row, index) => [row, index])) + for (const [id, row] of rowOf) rowOf.set(id, remap.get(row) ?? row) +} + /** First-fit packing in fork order, with crossing-aware drift (see header). */ function pack(chains: readonly PackChain[], mainId: number): Map { const rowOf = new Map() @@ -142,26 +178,36 @@ function pack(chains: readonly PackChain[], mainId: number): Map // Occupied footprints per row (index 0 is the mainline's — never packed // into), and the columns of registered verticals passing THROUGH each row. // n chains → tiny arrays; linear scans stay cheap. - const taken: { start: number; end: number }[][] = [[]] + const taken: PackChain[][] = [[]] const through: number[][] = [] - const fits = (row: number, c: PackChain): boolean => - !taken[row]?.some((t) => c.start <= t.end && t.start <= c.end) + const fits = (row: number, c: PackChain): boolean => !taken[row]?.some((t) => conflicts(c, t)) + // A vertical crosses a chain where it would cut visible ink: the capsule + // or the lead-out run. Verticals behind a label pad are masked by the + // pill's opaque base — pricing those would make chains dodge crossings + // no eye can see. const covered = (row: number, column: number): boolean => - taken[row]?.some((t) => t.start <= column && column <= t.end) === true + taken[row]?.some((t) => t.capStart <= column && column <= t.end) === true // Crossings placing `c` at `row` would create, both ways: its verticals - // against placed rows, and placed verticals against its footprint. + // against placed rows, and placed verticals against its footprint. Charges + // the work budget; once spent, scoring reports ties and placement degrades + // to plain first-fit (see PHASE_WORK_CAP). + let work = PHASE_WORK_CAP const crossings = (c: PackChain, row: number): number => { + if (work <= 0) return 0 let cost = 0 - for (const column of through[row] ?? []) { - if (column >= c.start && column <= c.end) cost++ + const throughRow = through[row] ?? [] + work -= throughRow.length + for (const column of throughRow) { + if (column >= c.capStart && column <= c.end) cost++ } for (const stub of c.stubs) { const otherRow = rowOf.get(stub.other) if (otherRow === undefined) continue const hi = Math.max(otherRow, row) for (let q = Math.min(otherRow, row) + 1; q < hi; q++) { + work -= taken[q]?.length ?? 1 if (covered(q, stub.column)) cost++ } } @@ -208,7 +254,7 @@ function pack(chains: readonly PackChain[], mainId: number): Map rowOf.set(chain.id, row) while (taken.length <= row) taken.push([]) - taken[row].push({ start: chain.start, end: chain.end }) + taken[row].push(chain) // Register the verticals whose far end is now placed. Each registers // exactly once — from whichever end packs second (the mainline never // packs, so its connectors always register from the other end). @@ -224,22 +270,39 @@ function pack(chains: readonly PackChain[], mainId: number): Map return rowOf } -/** The improvement pass: swap strictly-nested, row-inverted chain pairs - * whenever that reduces crossings (see header, CLARITY 2). Mutates rowOf. */ -function swapNestedPairs( +/** The improvement pass, two bounded local moves over the packed rows: + * + * - SWAPS of strictly-nested, row-inverted chain pairs whenever that + * strictly reduces crossings (see header, CLARITY 2). + * - LIFTS: greedy packing is placement-order myopic — a chain may sit low + * because the rows above were busy when its turn came, even though later + * swaps and lifts freed them. Each sweep re-offers every chain the rows + * above it and moves it up whenever it fits, floors hold, and its + * crossings don't get worse — reclaiming the wasted vertical space for + * free. + * + * Every accepted change strictly shrinks (crossings, total row distance) + * lexicographically, so the pass converges; MAX_SWEEPS caps it anyway. + * Mutates rowOf. */ +function improvePlacement( chains: readonly PackChain[], mainId: number, rowOf: Map ): void { - // Every vertical with both ends in the window, deduplicated: each pair's - // stubs mirror each other, so take a vertical from its lower-id end (the - // mainline carries no stub list — its verticals come from the other end). + // Every vertical with both ends placed, deduplicated by (pair, column): + // mirrored stubs collapse to one segment, and two merges of the same pair + // at one column draw as one line — one crossing, not two. const verticals: Vertical[] = [] + const seen = new Set() for (const c of chains) { for (const stub of c.stubs) { - if ((c.id < stub.other || stub.other === mainId) && rowOf.has(stub.other)) { - verticals.push({ column: stub.column, a: c.id, b: stub.other }) - } + if (!rowOf.has(stub.other) || stub.other === c.id) continue + const a = Math.min(c.id, stub.other) + const b = Math.max(c.id, stub.other) + const key = `${a}:${b}:${stub.column}` + if (seen.has(key)) continue + seen.add(key) + verticals.push({ column: stub.column, a, b }) } } const children = new Map() @@ -250,15 +313,20 @@ function swapNestedPairs( list.push(c) } - /** Crossings involving `o` or `i` if they sat on (rowO, rowI) — the only - * terms a swap can change, counted from scratch so the pass needs no - * incremental bookkeeping beyond rowOf itself. */ - const pairCost = (o: PackChain, i: PackChain, rowO: number, rowI: number): number => { - const rowAt = (id: number): number => - id === o.id ? rowO : id === i.id ? rowI : (rowOf.get(id) ?? 0) - const crossesInterval = (v: Vertical, z: PackChain): boolean => { + /** rowOf with up to two overrides — how a candidate move sees the rows. */ + const at = + (aId: number, aRow: number, bId: number, bRow: number) => + (id: number): number => + id === aId ? aRow : id === bId ? bRow : (rowOf.get(id) ?? 0) + + /** Crossings involving the `moved` chains under `rowAt`'s rows — the only + * terms a local move can change, counted from scratch so the pass needs + * no incremental bookkeeping beyond rowOf itself. */ + const localCost = (moved: readonly PackChain[], rowAt: (id: number) => number): number => { + // Same visible-ink rule as pack(): capsule + lead-out run, pads free. + const crosses = (v: Vertical, z: PackChain): boolean => { if (z.id === v.a || z.id === v.b) return false - if (v.column < z.start || v.column > z.end) return false + if (v.column < z.capStart || v.column > z.end) return false const rz = rowAt(z.id) const ra = rowAt(v.a) const rb = rowAt(v.b) @@ -266,11 +334,10 @@ function swapNestedPairs( } let cost = 0 for (const v of verticals) { - if (v.a === o.id || v.b === o.id || v.a === i.id || v.b === i.id) { - for (const z of chains) if (crossesInterval(v, z)) cost++ + if (moved.some((m) => m.id === v.a || m.id === v.b)) { + for (const z of chains) if (crosses(v, z)) cost++ } else { - if (crossesInterval(v, o)) cost++ - if (crossesInterval(v, i)) cost++ + for (const m of moved) if (crosses(v, m)) cost++ } } return cost @@ -278,12 +345,7 @@ function swapNestedPairs( const fitsAt = (c: PackChain, row: number, ignoring: PackChain): boolean => !chains.some( - (z) => - z.id !== c.id && - z.id !== ignoring.id && - rowOf.get(z.id) === row && - c.start <= z.end && - z.start <= c.end + (z) => z.id !== c.id && z.id !== ignoring.id && rowOf.get(z.id) === row && conflicts(c, z) ) /** Parent above, every child below — with the pair's swapped rows. */ const floorsHold = (c: PackChain, row: number, otherId: number, otherRow: number): boolean => { @@ -297,38 +359,64 @@ function swapNestedPairs( }) } - // Candidate pairs: `o`'s footprint contains `i`'s, yet `o` sits above it. - // Sorted by start so containment scans stay short. const movable = chains .filter((c) => c.releaseRank === null && rowOf.has(c.id)) .sort((a, b) => a.start - b.start || a.id - b.id) - let budget = Math.min( - SWAP_BUDGET, - Math.ceil(SWAP_WORK_CAP / (chains.length + verticals.length + 1)) - ) - for (let sweep = 0; sweep < MAX_SWEEPS && budget > 0; sweep++) { + const evalCost = chains.length + verticals.length + 1 + let work = PHASE_WORK_CAP + for (let sweep = 0; sweep < MAX_SWEEPS && work > 0; sweep++) { let improved = false - for (let x = 0; x < movable.length && budget > 0; x++) { + // ── Swaps: `o`'s footprint contains `i`'s, yet `o` sits above it. + // The start-sorted list keeps containment scans short. + for (let x = 0; x < movable.length && work > 0; x++) { const o = movable[x] - for (let y = x + 1; y < movable.length && budget > 0; y++) { + for (let y = x + 1; y < movable.length && work > 0; y++) { + work-- const i = movable[y] if (i.start > o.end) break // sorted: nothing later can nest in o if (i.end > o.end) continue const rowO = rowOf.get(o.id) ?? 0 const rowI = rowOf.get(i.id) ?? 0 if (rowO >= rowI) continue - budget-- + work -= 3 * evalCost if (!fitsAt(o, rowI, i) || !fitsAt(i, rowO, o)) continue if (!floorsHold(o, rowI, i.id, rowO) || !floorsHold(i, rowO, o.id, rowI)) continue // Drift penalties cancel exactly (the pair trades distances), so a // swap is judged on crossings alone — and only a STRICT win moves. - if (pairCost(o, i, rowI, rowO) < pairCost(o, i, rowO, rowI)) { + const kept = localCost([o, i], at(o.id, rowO, i.id, rowI)) + const swapped = localCost([o, i], at(o.id, rowI, i.id, rowO)) + if (swapped < kept) { rowOf.set(o.id, rowI) rowOf.set(i.id, rowO) improved = true } } } + // ── Lifts: re-offer every chain the rows above it (placement scoring, + // full hindsight). Move up only when crossings don't get worse. + for (const c of movable) { + if (work <= 0) break + const current = rowOf.get(c.id) ?? 0 + const parentRow = c.parent === null ? 0 : (rowOf.get(c.parent) ?? 0) + const currentCost = localCost([c], at(c.id, current, c.id, current)) + let bestRow = current + let bestScore = currentCost + (current - parentRow - 1) * ROW_DRIFT_PENALTY + for (let r = parentRow + 1; r < current && work > 0; r++) { + work -= evalCost + if (!fitsAt(c, r, c)) continue + const cost = localCost([c], at(c.id, r, c.id, r)) + const score = cost + (r - parentRow - 1) * ROW_DRIFT_PENALTY + if (cost <= currentCost && score < bestScore) { + bestRow = r + bestScore = score + if (cost === 0) break // nothing above can score lower + } + } + if (bestRow !== current) { + rowOf.set(c.id, bestRow) + improved = true + } + } if (!improved) break } } From 53743a79e268478a975dcf9da8d500f5a59fcf08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Pen=CC=83alba?= Date: Sat, 4 Jul 2026 21:17:57 +0200 Subject: [PATCH 5/8] Take into account branch labels when packing --- src/renderer/src/components/graph/layout.ts | 27 ++++++--- .../src/components/graph/packing.test.ts | 29 +++++----- src/renderer/src/components/graph/packing.ts | 55 +++++++++++-------- 3 files changed, 68 insertions(+), 43 deletions(-) diff --git a/src/renderer/src/components/graph/layout.ts b/src/renderer/src/components/graph/layout.ts index 2f372b4..39eec0e 100644 --- a/src/renderer/src/components/graph/layout.ts +++ b/src/renderer/src/components/graph/layout.ts @@ -28,6 +28,8 @@ import type { Commit } from '@shared/types' import { type CommitRef, parseRefs } from '@/lib/format' +// Value import from geometry is safe: geometry's layout imports are type-only. +import { COL_W } from './geometry' import { type PackChain, packRows, type VerticalStub } from './packing' import { compareReleaseVersions, releaseVersionWithOverride } from './releases' @@ -167,9 +169,17 @@ export interface GraphInput { structureOnly?: boolean } -/** Columns reserved left of a chain's first node so its label never overlaps - * the previous chain sharing the row. */ -const LABEL_PAD_COLUMNS = 3 +/** Estimated width of a row's label pill, in columns. The pill anchors at + * the first commit and extends RIGHT (geometry.ts labelRect) — past the + * capsule when the name outsizes a short branch — so the packer must know + * its reach to keep pills on a shared row from colliding. An estimate, not + * a measurement: layout stays pure and deterministic (measured widths vary + * by platform font and only exist after first paint). Mirrors render.ts + * labelWidthFor's 6.2 px/char fallback plus the pill's 16px padding and a + * little air before the next pill. */ +function labelColumns(name: string): number { + return Math.ceil((name.length * 6.2 + 16 + 8) / COL_W) +} /** A branch tip: one exact ref name resolved to the commit it points at. */ interface Tip { @@ -519,10 +529,10 @@ export function layoutGraph(input: GraphInput): GraphLayout { const packChains: PackChain[] = [] chains.forEach((chain, id) => { if (id === mainChain) return - // Reserve the chain's whole visual footprint: the label pad and the fork - // lead-in to the left, the merge lead-out to the right (the orthogonal - // connector runs along the row — see render.ts) — so no other chain on - // the row ever sits underneath those runs. + // Reserve the chain's whole visual footprint: the fork lead-in to the + // left and the merge lead-out to the right (orthogonal connector runs + // along the row — see render.ts), plus the label pill's reach in the + // band above (see labelColumns). const forkColumn = columnOf.get(baseHashOf(id) ?? '') // An empty chain's tipHash names another chain's commit — a merge of that // commit is the OWNER's lead-out to reserve, not the empty lane's. @@ -530,10 +540,11 @@ export function layoutGraph(input: GraphInput): GraphLayout { const mergeColumn = mergeChild ? columnOf.get(mergeChild.hash) : undefined packChains.push({ id, - start: Math.min(span[id].start - LABEL_PAD_COLUMNS, forkColumn ?? Number.MAX_SAFE_INTEGER), + start: Math.min(span[id].start, forkColumn ?? Number.MAX_SAFE_INTEGER), end: Math.max(span[id].end + 1, mergeColumn ?? -1), capStart: span[id].start, capEnd: span[id].end, + labelEnd: span[id].start + labelColumns(chain.name) - 1, parent: parentChainOf(id) ?? null, releaseRank: releaseRank.get(id) ?? null, isHead: id === headChain, diff --git a/src/renderer/src/components/graph/packing.test.ts b/src/renderer/src/components/graph/packing.test.ts index 2ffb2b9..863e3bd 100644 --- a/src/renderer/src/components/graph/packing.test.ts +++ b/src/renderer/src/components/graph/packing.test.ts @@ -4,7 +4,8 @@ import { type PackChain, packRows } from './packing' const MAIN = 0 /** A chain hanging off the mainline unless overridden. Defaults make the - * whole footprint capsule (no soft zones), so interval tests read plainly. */ + * whole footprint capsule with a minimal pill, so interval tests read + * plainly. */ function chain(id: number, overrides: Partial = {}): PackChain { const start = overrides.start ?? 0 const end = overrides.end ?? 0 @@ -14,6 +15,7 @@ function chain(id: number, overrides: Partial = {}): PackChain { end, capStart: start, capEnd: end, + labelEnd: overrides.capStart ?? start, parent: MAIN, releaseRank: null, isHead: false, @@ -100,27 +102,28 @@ describe('packRows', () => { expect(rows.get(3)).toBe(1) }) - test('a label pad may sit over a neighbor\'s merge lead-out (shares the row)', () => { - // a's capsule ends at 10 but its lead-out runs to column 15; b's label - // pad occupies 13–15. Pads mask connector lines, so the row is shared — - // under a whole-interval rule this 3-column graze would cost b a row. - const a = chain(1, { start: 0, end: 15, capStart: 0, capEnd: 10 }) - const b = chain(2, { start: 13, end: 21, capStart: 16, capEnd: 20 }) + test("a long pill may hang over a neighbor's connector runs (shares the row)", () => { + // a is a 1-commit branch whose pill reaches column 12, over b's fork + // lead-in (10–15). Pills mask connector lines (opaque base), so the row + // is shared — a whole-interval rule would cost one of them a row. + const a = chain(1, { start: 0, end: 4, capStart: 0, capEnd: 3, labelEnd: 12 }) + const b = chain(2, { start: 10, end: 21, capStart: 16, capEnd: 20, labelEnd: 19 }) const rows = packRows([a, b], MAIN) expect(rows.get(1)).toBe(1) expect(rows.get(2)).toBe(1) }) - test('a label pad never covers a neighbor\'s commits', () => { - // b's pad (8–10) would sit on a's capsule tail — that hides real nodes, - // so they must not share the row. - const a = chain(1, { start: 0, end: 11, capStart: 0, capEnd: 10 }) - const b = chain(2, { start: 8, end: 21, capStart: 11, capEnd: 20 }) + test("a pill never reaches a neighbor's label anchor or commits", () => { + // a's pill reaches column 15 — past b's first commit (14), where b's own + // pill anchors. Overlapping pills (or a pill over commits) hide ink, so + // they must not share the row. + const a = chain(1, { start: 0, end: 4, capStart: 0, capEnd: 3, labelEnd: 15 }) + const b = chain(2, { start: 12, end: 21, capStart: 14, capEnd: 20, labelEnd: 18 }) const rows = packRows([a, b], MAIN) expect(rows.get(1)).not.toBe(rows.get(2)) }) - test('a merge lead-out never runs under a neighbor\'s capsule', () => { + test("a merge lead-out never runs under a neighbor's capsule", () => { // a's lead-out (11–15) would pass beneath b's commits starting at 12. const a = chain(1, { start: 0, end: 15, capStart: 0, capEnd: 10 }) const b = chain(2, { start: 12, end: 21, capStart: 12, capEnd: 20 }) diff --git a/src/renderer/src/components/graph/packing.ts b/src/renderer/src/components/graph/packing.ts index 880a2d4..84efb9e 100644 --- a/src/renderer/src/components/graph/packing.ts +++ b/src/renderer/src/components/graph/packing.ts @@ -9,9 +9,9 @@ // row count stays minimal for the footprints given), and a chain never // opens a new row while an existing row can host it: clarity never buys // itself a taller diagram. Footprints themselves are zoned (see PackChain) -// so row-sharing is exactly as tight as the pixels allow — a label pad -// grazing a neighbor's merge lead-out shares the row instead of dropping -// below it for a touch no eye can see. +// so row-sharing is exactly as tight as the pixels allow — a label pill +// hanging over a neighbor's merge lead-out shares the row instead of +// dropping below it for a touch no eye can see. // - CLARITY. Every chain hangs off the diagram by a handful of VERTICAL // connectors (the fork drop from its parent, the merge lead-out into its // target, one per merge it received — see render.ts for the orthogonal @@ -48,26 +48,35 @@ export interface VerticalStub { other: number } -/** One chain as the packer sees it. Its footprint has three zones: +/** One chain as the packer sees it. Its footprint has two layers, matching + * what the renderer actually draws (see geometry.ts labelRect): * - * [start … capStart-1] soft head: label pad + fork lead-in + * On the row spine — + * [start … capStart-1] fork lead-in run (a connector line) * [capStart … capEnd] capsule: the commits and spine - * [capEnd+1 … end] soft tail: breathing column + merge lead-out run + * [capEnd+1 … end] breathing column + merge lead-out run * - * Zones let row-sharing be exactly as tight as the pixels allow: nothing may - * overlap a capsule, and two label pads may not collide, but a label pad MAY - * sit over another chain's soft tail — the pill's opaque base masks the - * connector line behind it, so no ink is lost. That one relaxation is what - * lets a branch tuck beside a neighbor whose lead-out barely grazes its - * label's air, instead of dropping a whole row for a 1–3 column touch. */ + * In the label band above it — + * [capStart … labelEnd] the label pill, anchored at the first commit and + * extending RIGHT — past the capsule when the name + * outsizes a short branch. + * + * Zones let row-sharing be exactly as tight as the pixels allow: nothing + * solid may overlap a capsule, and two pills may not collide, but a pill + * MAY hang over a neighbor's connector runs — its opaque base masks the + * line behind it, so no ink is lost. That relaxation lets a branch tuck + * beside a neighbor whose lead-out barely grazes it, instead of dropping a + * whole row for a touch no eye can see. */ export interface PackChain { id: number - /** Full reserved footprint (all three zones) — what verticals cross. */ + /** The connector-line footprint on the spine: lead-in … lead-out. */ start: number end: number /** The capsule zone. layout.ts always leaves end ≥ capEnd + 1. */ capStart: number capEnd: number + /** Right edge of the label pill (estimated — see layout.ts), ≥ capStart. */ + labelEnd: number /** Chain owning the commit this one forked from — its row floor — or null * for a chain that starts at a root commit (or off-window). */ parent: number | null @@ -112,13 +121,15 @@ interface Vertical { } /** Whether two footprints may NOT share a row (see PackChain's zones): - * pads and capsules never overlap each other, and a capsule never sits on - * a soft tail (a connector run through commits) — but a pad over a soft - * tail is fine, so near-miss endpoint touches stop costing whole rows. */ + * a capsule tolerates nothing over it — not lines, not pills, not another + * capsule (the breathing column keeps them a col apart) — and two pills + * must not collide in the label band. Everything else overlaps freely: + * pills over connector runs, run over run. */ const conflicts = (a: PackChain, b: PackChain): boolean => - (a.start <= b.capEnd && b.start <= a.capEnd) || // pad∪capsule vs pad∪capsule - (a.capStart <= b.end && b.capEnd < a.capEnd) || // a's capsule on b's tail - (b.capStart <= a.end && a.capEnd < b.capEnd) // b's capsule on a's tail + (a.capStart <= b.end && b.start <= a.capEnd) || // a's capsule vs b's lines + (b.capStart <= a.end && a.start <= b.capEnd) || // b's capsule vs a's lines + // Label band: each chain's solid extent is its pill plus its capsule. + (a.capStart <= Math.max(b.labelEnd, b.capEnd) && b.capStart <= Math.max(a.labelEnd, a.capEnd)) /** * Pack every chain onto a row. `chains` excludes the mainline; `mainId` (the @@ -183,9 +194,9 @@ function pack(chains: readonly PackChain[], mainId: number): Map const fits = (row: number, c: PackChain): boolean => !taken[row]?.some((t) => conflicts(c, t)) // A vertical crosses a chain where it would cut visible ink: the capsule - // or the lead-out run. Verticals behind a label pad are masked by the - // pill's opaque base — pricing those would make chains dodge crossings - // no eye can see. + // or the lead-out run. Verticals behind the label pill are masked by its + // opaque base — pricing those would make chains dodge crossings no eye + // can see. const covered = (row: number, column: number): boolean => taken[row]?.some((t) => t.capStart <= column && column <= t.end) === true From a8e5af8cc03f683d2a8619c4b99205e587c8616b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Pen=CC=83alba?= Date: Sat, 4 Jul 2026 21:22:12 +0200 Subject: [PATCH 6/8] Fix lint: avoid assign-in-expression, drop unused param --- .../src/components/graph/packing.test.ts | 10 ++------ src/renderer/src/components/graph/packing.ts | 23 ++++++++----------- 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/src/renderer/src/components/graph/packing.test.ts b/src/renderer/src/components/graph/packing.test.ts index 863e3bd..25487f1 100644 --- a/src/renderer/src/components/graph/packing.test.ts +++ b/src/renderer/src/components/graph/packing.test.ts @@ -32,19 +32,13 @@ describe('packRows', () => { }) test('non-overlapping chains share a row (density before everything)', () => { - const rows = packRows( - [chain(1, { start: 0, end: 5 }), chain(2, { start: 7, end: 12 })], - MAIN - ) + const rows = packRows([chain(1, { start: 0, end: 5 }), chain(2, { start: 7, end: 12 })], MAIN) expect(rows.get(1)).toBe(1) expect(rows.get(2)).toBe(1) }) test('overlapping chains stack on separate rows', () => { - const rows = packRows( - [chain(1, { start: 0, end: 5 }), chain(2, { start: 3, end: 12 })], - MAIN - ) + const rows = packRows([chain(1, { start: 0, end: 5 }), chain(2, { start: 3, end: 12 })], MAIN) expect(rows.get(1)).not.toBe(rows.get(2)) }) diff --git a/src/renderer/src/components/graph/packing.ts b/src/renderer/src/components/graph/packing.ts index 84efb9e..03ed1cf 100644 --- a/src/renderer/src/components/graph/packing.ts +++ b/src/renderer/src/components/graph/packing.ts @@ -138,7 +138,7 @@ const conflicts = (a: PackChain, b: PackChain): boolean => */ export function packRows(chains: readonly PackChain[], mainId: number): Map { const rowOf = pack(chains, mainId) - improvePlacement(chains, mainId, rowOf) + improvePlacement(chains, rowOf) compactRows(rowOf) return rowOf } @@ -229,9 +229,7 @@ function pack(chains: readonly PackChain[], mainId: number): Map // A child never packs above its parent: candidates start strictly below // it. Release lines are exempt — their spine stack is fixed from row 1. const floor = - chain.releaseRank !== null || chain.parent === null - ? 1 - : (rowOf.get(chain.parent) ?? 0) + 1 + chain.releaseRank !== null || chain.parent === null ? 1 : (rowOf.get(chain.parent) ?? 0) + 1 let row = floor if (chain.releaseRank !== null) { // Releases: pure first-fit. Cost-driven drift would let a busy window @@ -274,7 +272,8 @@ function pack(chains: readonly PackChain[], mainId: number): Map if (otherRow === undefined || stub.other === chain.id) continue const hi = Math.max(otherRow, row) for (let q = Math.min(otherRow, row) + 1; q < hi; q++) { - ;(through[q] ??= []).push(stub.column) + through[q] ??= [] + through[q].push(stub.column) } } } @@ -295,11 +294,7 @@ function pack(chains: readonly PackChain[], mainId: number): Map * Every accepted change strictly shrinks (crossings, total row distance) * lexicographically, so the pass converges; MAX_SWEEPS caps it anyway. * Mutates rowOf. */ -function improvePlacement( - chains: readonly PackChain[], - mainId: number, - rowOf: Map -): void { +function improvePlacement(chains: readonly PackChain[], rowOf: Map): void { // Every vertical with both ends placed, deduplicated by (pair, column): // mirrored stubs collapse to one segment, and two merges of the same pair // at one column draw as one line — one crossing, not two. @@ -320,7 +315,10 @@ function improvePlacement( for (const c of chains) { if (c.parent === null) continue let list = children.get(c.parent) - if (!list) children.set(c.parent, (list = [])) + if (!list) { + list = [] + children.set(c.parent, list) + } list.push(c) } @@ -360,8 +358,7 @@ function improvePlacement( ) /** Parent above, every child below — with the pair's swapped rows. */ const floorsHold = (c: PackChain, row: number, otherId: number, otherRow: number): boolean => { - const rowAt = (id: number): number | undefined => - id === otherId ? otherRow : rowOf.get(id) + const rowAt = (id: number): number | undefined => (id === otherId ? otherRow : rowOf.get(id)) const parentRow = c.parent === null ? 0 : (rowAt(c.parent) ?? 0) if (row <= parentRow) return false return (children.get(c.id) ?? []).every((child) => { From d76bd212091fe32ed10e6d785b457ccc603545b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Pen=CC=83alba?= Date: Sat, 4 Jul 2026 22:19:24 +0200 Subject: [PATCH 7/8] Improve branch selection halo and commit comment margins --- .../src/components/graph/geometry.test.ts | 48 ++++++++++------- src/renderer/src/components/graph/geometry.ts | 53 ++++++++++++------- src/renderer/src/components/graph/render.ts | 41 ++++++++------ 3 files changed, 87 insertions(+), 55 deletions(-) diff --git a/src/renderer/src/components/graph/geometry.test.ts b/src/renderer/src/components/graph/geometry.test.ts index b219dfe..fe9ec46 100644 --- a/src/renderer/src/components/graph/geometry.test.ts +++ b/src/renderer/src/components/graph/geometry.test.ts @@ -3,15 +3,20 @@ import type { Commit } from '@shared/types' import { CAPSULE_HALF_H, CAPTION_FULL_ZOOM, + CAPTION_GAP_WORLD, CAPTION_INK_ABOVE, CAPTION_INK_BELOW, captionAlpha, captionCenterOffset, contentSize, hitTest, + LABEL_GAP, + LABEL_H, + NODE_R, neighborNode, nodeX, nodeY, + ROW_H, rowEndpoint } from './geometry' import { type GraphInput, layoutGraph } from './layout' @@ -119,7 +124,7 @@ describe('graph geometry', () => { expect(neighborNode(layout, m, 'ArrowRight')).toBeNull() }) - test('the caption hit band rides a screen-fixed gap below the capsule', () => { + test('the caption hit band sits a world-fixed gap below the capsule', () => { const layout = sampleLayout() const m = layout.nodeByHash.get('m') if (!m) throw new Error('missing node') @@ -138,27 +143,30 @@ describe('graph geometry', () => { () => 30, scale ) - // At zoom 1 the classic band: nodeY + 26 is the caption line's center. - expect(at(nodeY(m.row) + 26, 1)?.type).toBe('node') - // Zoomed in, the gap shrinks in WORLD units (constant on screen): the - // world point that hit at zoom 1 now lies below the band… - expect(at(nodeY(m.row) + 36, 3)).toBeNull() - // …while a point hugging the capsule edge hits. - expect(at(nodeY(m.row) + 22, 3)?.type).toBe('node') + // The caption rides a fixed WORLD gap below the capsule (its screen distance + // scales with zoom), so the SAME world point lands in its band at any zoom. + const center = captionCenterOffset(1) + expect(captionCenterOffset(3)).toBeCloseTo(center) + expect(at(nodeY(m.row) + center, 1)?.type).toBe('node') + expect(at(nodeY(m.row) + center, 3)?.type).toBe('node') + // A point up near the capsule edge sits in the gap above the band — no hit. + expect(at(nodeY(m.row) + CAPSULE_HALF_H + 1, 3)).toBeNull() }) - test('a squeezed caption splits its air between capsule and next label band', () => { - // Roomy corridor (zoom 2): the design gap below the capsule (11 screen px - // to the text center — see CAPTION_GAP_SCREEN). - expect(captionCenterOffset(2)).toBeCloseTo(CAPSULE_HALF_H + 11 / 2) - // Squeezed corridor (zoom 0.9): equal screen air above the ink and below - // it, and the ink stays clear of the next row's label band, 38 world px - // below the row center. - const offset = captionCenterOffset(0.9) - const airAbove = (offset - CAPSULE_HALF_H) * 0.9 - CAPTION_INK_ABOVE - const airBelow = (38 - offset) * 0.9 - CAPTION_INK_BELOW - expect(airAbove).toBeCloseTo(airBelow) - expect(airBelow).toBeGreaterThan(0) + test('the caption keeps a proportional gap, clear of both rails at every zoom', () => { + // The next row's label band starts this many world px below the row center. + const railBottom = ROW_H - NODE_R - LABEL_GAP - LABEL_H + // The caption center sits the fixed WORLD gap below the capsule — the SAME + // at every zoom, so on screen it scales in proportion with the diagram. + expect(captionCenterOffset(2)).toBeCloseTo(CAPSULE_HALF_H + CAPTION_GAP_WORLD) + expect(captionCenterOffset(3)).toBeCloseTo(CAPSULE_HALF_H + CAPTION_GAP_WORLD) + // At every visible zoom the screen-fixed ink stays clear of both rails: the + // capsule above and the next row's label band below. + for (const scale of [0.9, 1, 2, 3]) { + const offset = captionCenterOffset(scale) + expect(offset - CAPTION_INK_ABOVE / scale).toBeGreaterThan(CAPSULE_HALF_H) + expect(offset + CAPTION_INK_BELOW / scale).toBeLessThan(railBottom) + } }) test('the caption layer is all-or-nothing: full at zoom 1, gone below the fade', () => { diff --git a/src/renderer/src/components/graph/geometry.ts b/src/renderer/src/components/graph/geometry.ts index 8c5a8e6..666756e 100644 --- a/src/renderer/src/components/graph/geometry.ts +++ b/src/renderer/src/components/graph/geometry.ts @@ -8,8 +8,11 @@ import type { GraphLayout, GraphNode, GraphRow } from './layout' // World-space metrics (scaled by the view's zoom when drawn). export const COL_W = 44 // Row pitch leaves clear air between a node's subject text (below the node) -// and the next row's label band — they must never touch (see render.ts). -export const ROW_H = 72 +// and the next row's label band — they must never touch (see render.ts). It's +// sized so a SELECTED branch's halo+ring (screen-fixed, ±8px around the +// capsule — see drawContainers) clears the caption below it at every zoom, the +// same even embrace a selected commit node wears. +export const ROW_H = 78 export const NODE_R = 12 /** Left/right padding before the first and after the last column. */ export const MARGIN_X = 28 @@ -24,15 +27,18 @@ export const LABEL_GAP = 4 * half its height. Shared by the renderer and hit-testing — the capsule is * itself a click target (it IS the branch). */ export const CAPSULE_PAD = 10 -export const CAPSULE_HALF_H = 19 -/** Caption line under a node (subject text). The capsule edge is world-space - * but caption glyphs are screen-fixed (render.ts SUBJECT_FONT), so the gap - * between them must be screen-fixed too — a world-space anchor sinks the - * text into the capsule when zooming out and floats it far below when - * zooming in. SCREEN px from the capsule's bottom edge down to the text - * line's CENTER when the corridor is roomy: 13px type spans ~5px of ink - * above its center, so 11 leaves ~6px of clear air under the capsule. */ -export const CAPTION_GAP_SCREEN = 11 +export const CAPSULE_HALF_H = 17 +/** Caption line under a node (subject text). WORLD gap from the capsule's + * bottom edge down to the caption line's CENTER when the corridor is roomy. + * A WORLD distance (not screen) so the caption sits the same fraction below + * the capsule at EVERY zoom — it scales WITH the diagram, staying in exact + * proportion with the world-scaled selection halo/ring (drawContainers), just + * like a selected node's caption does. Sized to clear that halo (which reaches + * RING_HALO=6 world px below the capsule — see drawContainers) with room for + * the caption's own ink above its center (~5 screen px). Only the TEXT is + * screen-fixed size (render.ts SUBJECT_FONT); at low zoom its now-large world + * footprint is kept off the rails by the squeeze in captionCenterOffset. */ +export const CAPTION_GAP_WORLD = 11 /** SCREEN height of the caption's hit band (13px type plus slop). */ export const CAPTION_BAND_H = 21 /** Approximate SCREEN ink extents of a caption line around its middle @@ -89,17 +95,24 @@ export function captionWidth(layout: GraphLayout, node: GraphNode): number { /** WORLD offset from a row's center down to its caption line's center at a * zoom level. Shared by the renderer, the hover-card anchor and hit-testing. * - * The caption lives in a corridor between two world-space rails: its own - * capsule's bottom edge above, and the next row's label band below. When the - * corridor is roomy the caption hugs the capsule with the design gap; when - * zooming out squeezes the corridor, the remaining slack is split evenly so - * the text keeps equal air on both sides instead of invading the labels. */ + * The caption sits a fixed WORLD gap below the capsule (CAPTION_GAP_WORLD), so + * its distance scales with zoom and stays in proportion with the world-scaled + * selection halo/ring — the whole selection reads the same at any zoom. The + * one screen-fixed thing is the TEXT size, so in WORLD units the ink grows as + * you zoom out; the caption lives in a corridor between two world rails (the + * capsule's bottom edge above, the next row's label band below), so at low + * zoom that fat world-ink is clamped to keep it off both rails. */ export function captionCenterOffset(scale: number): number { const railBottom = ROW_H - NODE_R - LABEL_GAP - LABEL_H - const corridor = (railBottom - CAPSULE_HALF_H) * scale - const slack = corridor - CAPTION_INK_ABOVE - CAPTION_INK_BELOW - const air = Math.max(1, Math.min(CAPTION_GAP_SCREEN - CAPTION_INK_ABOVE, slack / 2)) - return CAPSULE_HALF_H + (air + CAPTION_INK_ABOVE) / scale + const inkAbove = CAPTION_INK_ABOVE / scale + const inkBelow = CAPTION_INK_BELOW / scale + // Window the caption CENTER can occupy without its ink crossing either rail. + const lo = CAPSULE_HALF_H + inkAbove + const hi = railBottom - inkBelow + // Aim for the proportional gap; at low zoom the window collapses (large world + // ink) so center within it instead — the text never overlaps a rail. + const target = CAPSULE_HALF_H + CAPTION_GAP_WORLD + return lo <= hi ? Math.min(Math.max(target, lo), hi) : (lo + hi) / 2 } /** Pan/zoom: screen = world * scale + offset. */ diff --git a/src/renderer/src/components/graph/render.ts b/src/renderer/src/components/graph/render.ts index f523806..797e44c 100644 --- a/src/renderer/src/components/graph/render.ts +++ b/src/renderer/src/components/graph/render.ts @@ -352,25 +352,35 @@ function drawContainers( const lastColumn = row.isHead && wip ? wip.column : row.endColumn const x1 = nodeX(lastColumn) + NODE_R + CAPSULE_PAD if (selected) { - // Exactly the commit treatment: soft halo behind, and the accent ring - // drawn AROUND the capsule — its own branch-colored border stays as is. + // The commit treatment (drawNodes' selection), wrapped around the capsule + // and drawn in the SAME world space so halo/ring/stroke all scale with + // zoom — one selection grammar at every zoom. It hugs the capsule TIGHTER + // than a node's does its node, though: the capsule already pads the + // avatars, so an 8px halo (a node's) would land far below the capsule and + // force EVERY caption down to clear it. A snug halo (RING_HALO) keeps the + // selection close and lets the caption sit near the capsule; the ring is + // an even offset on every side (a capsule's equidistant curve is just a + // larger stadium). captionCenterOffset keeps the caption a proportional + // WORLD gap below, sized to clear this halo at every zoom. + const RING = 3 + const RING_HALO = 6 ctx.beginPath() ctx.roundRect( - x0 - 8, - y - CAPSULE_HALF_H - 8, - x1 - x0 + 16, - CAPSULE_HALF_H * 2 + 16, - CAPSULE_HALF_H + 8 + x0 - RING_HALO, + y - CAPSULE_HALF_H - RING_HALO, + x1 - x0 + RING_HALO * 2, + CAPSULE_HALF_H * 2 + RING_HALO * 2, + CAPSULE_HALF_H + RING_HALO ) ctx.fillStyle = palette.halo ctx.fill() ctx.beginPath() ctx.roundRect( - x0 - 4, - y - CAPSULE_HALF_H - 4, - x1 - x0 + 8, - CAPSULE_HALF_H * 2 + 8, - CAPSULE_HALF_H + 4 + x0 - RING, + y - CAPSULE_HALF_H - RING, + x1 - x0 + RING * 2, + CAPSULE_HALF_H * 2 + RING * 2, + CAPSULE_HALF_H + RING ) ctx.strokeStyle = palette.accent ctx.lineWidth = 2 @@ -852,9 +862,10 @@ function drawCaption( scene: SceneState, text: string, worldX: number, - /** The ROW's center y — the caption anchors a screen-fixed gap below the - * capsule edge (captionCenterOffset), so the margin between capsule and - * text is identical at every zoom instead of scaling with it. */ + /** The ROW's center y — the caption anchors a proportional WORLD gap below + * the capsule edge (captionCenterOffset), so the margin between capsule and + * text scales with zoom, staying in proportion with the diagram (and the + * world-scaled branch selection). */ rowCenterY: number, maxWorldWidth: number, maxScreenCap = CAPTION_MAX_SCREEN_W From 6b80202d97026fb051f1b6c5fe15f4849a246de3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Pen=CC=83alba?= Date: Sat, 4 Jul 2026 22:37:50 +0200 Subject: [PATCH 8/8] Consolidate search hit halos into shared drawHitGlow --- src/renderer/src/components/graph/render.ts | 104 +++++++++++------- .../src/components/graph/searchGlow.ts | 4 +- 2 files changed, 65 insertions(+), 43 deletions(-) diff --git a/src/renderer/src/components/graph/render.ts b/src/renderer/src/components/graph/render.ts index 797e44c..a6e8f06 100644 --- a/src/renderer/src/components/graph/render.ts +++ b/src/renderer/src/components/graph/render.ts @@ -582,24 +582,19 @@ function drawNodes( ctx.globalAlpha = dim ? DIM_ALPHA : 1 // Commit hits wear the text-editor find treatment, in gold: every hit a - // soft radial glow behind the node, the CURRENT hit a wider corona (plus - // a ring and an arrival ping, below). Gold, not the branch hue — hits - // must read across all branch colors at a glance. The glow fades to a - // zero-alpha stop of the SAME color: fading to `transparent` would drag - // the gradient through black and dirty the halo's rim. (Branch and tag - // hits glow their pill/chip instead — drawLabels / drawNodeText.) + // soft halo behind the node, the CURRENT hit a wider one (plus a ring and + // an arrival ping, below). Gold, not the branch hue — hits must read across + // all branch colors at a glance. The very same drawHitGlow a branch pill or + // a tag chip uses, so a result looks identical wherever it lives. const isActiveMatch = scene.activeHit?.kind === 'commit' && scene.activeHit.hash === node.commit.hash const isHit = scene.activeHit !== null && scene.matches?.has(node.commit.hash) === true if (isHit) { - const glowR = NODE_R + (isActiveMatch ? ACTIVE_GLOW : HIT_GLOW) - const glow = ctx.createRadialGradient(x, y, NODE_R - 2, x, y, glowR) - glow.addColorStop(0, withAlpha(palette.match, isActiveMatch ? 0.55 : 0.35)) - glow.addColorStop(1, withAlpha(palette.match, 0)) - ctx.fillStyle = glow - ctx.beginPath() - ctx.arc(x, y, glowR, 0, Math.PI * 2) - ctx.fill() + drawHitGlow(ctx, scene, isActiveMatch, () => { + ctx.beginPath() + ctx.arc(x, y, NODE_R, 0, Math.PI * 2) + ctx.fill() + }) } // Selection halo, under everything else on the node. @@ -949,22 +944,21 @@ function drawNodeText( // the chip yields and reappears as the user pans on. if (labelBoxes.some((b) => b.sticky && intersects(b.rect, chip))) return // A tag-name hit is the CHIP itself, in the same gold find treatment the - // branch labels wear: the chip stays at full strength (its commit didn't - // match — the tag did, so the node beneath keeps its dim), glow behind, - // gold border, and corona + ping when current. + // branch labels and commits wear: the chip stays at full strength (its + // commit didn't match — the tag did, so the node beneath keeps its dim), + // the shared halo behind it, gold border, and corona + ping when current. const isHitTag = scene.hitTags?.has(node.commit.hash) === true const isActiveTag = scene.activeHit?.kind === 'tag' && scene.activeHit.hash === node.commit.hash - if (isHitTag) ctx.globalAlpha = 1 - ctx.beginPath() - ctx.roundRect(chip.x, chip.y, chip.w, chip.h, 4) if (isHitTag) { - ctx.save() - ctx.shadowColor = withAlpha(palette.match, isActiveTag ? 0.95 : 0.65) - ctx.shadowBlur = isActiveTag ? 12 : 8 - ctx.fillStyle = palette.labelBg - ctx.fill() - ctx.restore() + ctx.globalAlpha = 1 + drawHitGlow(ctx, scene, isActiveTag, () => { + ctx.beginPath() + ctx.roundRect(chip.x, chip.y, chip.w, chip.h, 4) + ctx.fill() + }) } + ctx.beginPath() + ctx.roundRect(chip.x, chip.y, chip.w, chip.h, 4) ctx.fillStyle = palette.labelBg ctx.fill() ctx.strokeStyle = isHitTag ? palette.match : palette.tag @@ -1036,12 +1030,20 @@ function drawLabels( // ghost too: a zero-commit branch can never hold a match.) const ghost = lit !== null && !lit.has(row.chain) // A branch-name hit is the LABEL itself: the same gold find treatment - // commits get, pill-shaped — glow behind it, and for the current hit a - // corona ring plus the arrival ping (below). litChains keeps hit labels - // out of the ghost set, so a hit always draws at full strength. + // commits get, pill-shaped — the shared halo behind it, and for the current + // hit a corona ring plus the arrival ping (below). litChains keeps hit + // labels out of the ghost set, so a hit always draws at full strength. const isHitLabel = scene.hitBranches?.has(row.chain) === true const isActiveHit = scene.activeHit?.kind === 'branch' && scene.activeHit.chain === row.chain const inkAlpha = ghost ? LABEL_GHOST_ALPHA : row.kind === 'unnamed' ? 0.8 : 1 + // The halo is cast behind the pill before its opaque body is laid over it. + if (isHitLabel) { + drawHitGlow(ctx, scene, isActiveHit, () => { + ctx.beginPath() + ctx.roundRect(rect.x, rect.y, rect.w, rect.h, 5) + ctx.fill() + }) + } // The opaque base NEVER ghosts: a translucent pill lets the capsule and // spine bleed through the text and reads as a glitch. Ghosting fades the // pill's ink — tint, border, name — while the body keeps masking the @@ -1049,19 +1051,14 @@ function drawLabels( ctx.globalAlpha = row.kind === 'unnamed' ? 0.8 : 1 ctx.beginPath() ctx.roundRect(rect.x, rect.y, rect.w, rect.h, 5) - // A sticky pill floats over diagram content — a soft shadow makes the - // layering read as deliberate. A hit's gold bloom replaces it: a shadow, - // not a radial gradient — it hugs the rounded shape. - if (isHitLabel || sticky) { + // A sticky pill floats over diagram content — a soft drop shadow makes the + // layering read as deliberate. A hit skips it: its gold halo already lifts + // the pill off the diagram. + if (sticky && !isHitLabel) { ctx.save() - if (isHitLabel) { - ctx.shadowColor = withAlpha(palette.match, isActiveHit ? 0.95 : 0.65) - ctx.shadowBlur = isActiveHit ? 14 : 9 - } else { - ctx.shadowColor = 'rgba(0, 0, 0, 0.3)' - ctx.shadowBlur = 6 - ctx.shadowOffsetY = 1 - } + ctx.shadowColor = 'rgba(0, 0, 0, 0.3)' + ctx.shadowBlur = 6 + ctx.shadowOffsetY = 1 ctx.fillStyle = palette.labelBg ctx.fill() ctx.restore() @@ -1087,6 +1084,31 @@ function drawLabels( } } +/** The soft gold bloom a search hit wears — the find grammar every glyph + * shares: node disc, branch pill and tag chip all cast the SAME warm halo, so + * a result reads the same wherever it lives. It's a Gaussian shadow thrown by + * the glyph's own silhouette (`fillGlyph` traces + fills it); the glyph body + * drawn on top hides that fill, leaving only the halo. canvas shadowBlur is + * DEVICE-space — untouched by the world transform — so we pre-multiply it by + * the zoom (`dpr * view.scale`) to hold the halo at a constant WORLD size that + * grows and shrinks with the diagram, matching the node ring and the selection + * halo (never a fixed-pixel glow that drifts as you zoom). */ +function drawHitGlow( + ctx: CanvasRenderingContext2D, + scene: SceneState, + active: boolean, + fillGlyph: () => void +): void { + const { palette, view, dpr } = scene + ctx.save() + ctx.globalAlpha = 1 + ctx.shadowColor = withAlpha(palette.match, active ? 0.9 : 0.6) + ctx.shadowBlur = (active ? ACTIVE_GLOW : HIT_GLOW) * dpr * view.scale + ctx.fillStyle = palette.match + fillGlyph() + ctx.restore() +} + /** The current hit's corona + arrival ping around a pill/chip rect — the * rectangular twin of the node treatment in drawNodes, shared by branch * labels and tag chips. */ diff --git a/src/renderer/src/components/graph/searchGlow.ts b/src/renderer/src/components/graph/searchGlow.ts index dcbe7fe..a97ee38 100644 --- a/src/renderer/src/components/graph/searchGlow.ts +++ b/src/renderer/src/components/graph/searchGlow.ts @@ -3,9 +3,9 @@ // wider corona and lands with a brief sonar ping. Kept free of canvas types // so the geometry and color math are directly testable. -/** How far past the node radius a hit's glow fades to nothing (world px). */ +/** A hit's halo blur radius in WORLD px (drawHitGlow scales it to device). */ export const HIT_GLOW = 9 -/** The current hit's corona reaches further — "you are here" among hits. */ +/** The current hit's halo reaches further — "you are here" among hits. */ export const ACTIVE_GLOW = 16 /** Duration of the arrival ping (ms) — GraphCanvas drives pulse 0→1 over it. */