From c15731fb1b1f4ba82545642f83976bab8a3087a8 Mon Sep 17 00:00:00 2001 From: flemming-it Date: Mon, 1 Jun 2026 16:12:15 +0200 Subject: [PATCH] =?UTF-8?q?fix(editor):=20inputs/outputs=20in=20layout=20s?= =?UTF-8?q?idecar=20=E2=80=94=20endpoints=20stop=20drifting?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 0.2.x canvas auto-recomputed the outputs endpoint's X position from `max(step.x) + nodeWidth + 120` every build. Dragging any step rebuilt the layout, the recompute kicked in, and the outputs panel + every edge entering it shifted across the screen. Mirror image: the inputs panel was nailed to a hardcoded (40, 80) but the right side of the world moved every time the operator touched a node. Visually catastrophic; Stefan flagged it as the same kind of "everything moves when I touch one thing" failure that shelved the jai_client editor. Structural fix: treat inputs and outputs as first-class nodes with positions stored in the same layout sidecar as every step. There is no more auto-recompute path. Concretely: - AutoLayout.layout now seeds NodePositions for the reserved IDs `__inputs__` (column −1) and `__outputs__` (column max+1) on first open. Existing positions are never overwritten so subsequent layout passes don't fight operator-chosen placements. - FlowCanvas reads inputs/outputs positions from layout.positions instead of hardcoded `_inputsX` / `_inputsY` constants and the computed `_outputsX(...)`. Both functions are deleted; the canvas now has one single source of truth for ALL node positions, the sidecar. - The inputs / outputs endpoints render via a new `_endpointPositioned` helper that mirrors `_stepPositioned` — same FlowNode widget, same drag handler, same `controller.moveStep` path. The operator can grab the inputs panel and slide it wherever; the position persists to the sidecar like every step. - All port-position helpers (`_outputPortPosition`, `_inputPortPosition`, the inputs-endpoint port position) now take a single `FlowLayout` and read coordinates from there. No more `double outputsX` parameter threaded through every method. - Fit-to-content now expands the bounding box using the endpoints' SIDECAR positions rather than a re-derived outputs X. Same single source of truth. Side-benefit: the editor's coordinate model is now strictly sidecar-driven. No state derivation lives in render code. This rules out the catastrophic "drag-mid-flight-redoes- coordinate-transforms" failure mode jai_client hit (where internal offsets grew during a drag and broke port positions). flutter analyze clean, 12/12 tests pass. Signed-off-by: flemming-it --- lib/src/model/auto_layout.dart | 31 ++- lib/src/widgets/flow_canvas.dart | 319 ++++++++++++++++--------------- 2 files changed, 196 insertions(+), 154 deletions(-) diff --git a/lib/src/model/auto_layout.dart b/lib/src/model/auto_layout.dart index 2f2dba0..3ae49ca 100644 --- a/lib/src/model/auto_layout.dart +++ b/lib/src/model/auto_layout.dart @@ -30,8 +30,20 @@ class AutoLayout { static const double originX = 320; static const double originY = 80; - /// Produce positions for every step in [graph], filling in - /// any not already covered by [existing]. + /// Reserved node ids for the two flow endpoints. These + /// behave like step nodes layout-wise — they have a + /// position in the sidecar, the operator drags them where + /// they want, and once placed they STAY (no auto-recompute + /// from step positions every build). + static const String inputsNodeId = '__inputs__'; + static const String outputsNodeId = '__outputs__'; + + /// Produce positions for every step in [graph] plus the + /// inputs / outputs endpoint nodes, filling in any not + /// already covered by [existing]. Existing positions are + /// never overwritten — once the operator has dragged a + /// node, that position survives every subsequent layout + /// pass. static FlowLayout layout(FlowGraph graph, FlowLayout existing) { final cols = _assignColumns(graph); final byColumn = >{}; @@ -49,6 +61,21 @@ class AutoLayout { result[step.id] = NodePosition(x, y); } }); + // Inputs endpoint sits at column -1, just left of the + // first column of steps. Stable across edits. + if (!result.containsKey(inputsNodeId)) { + result[inputsNodeId] = const NodePosition(40, originY); + } + // Outputs endpoint sits right of the rightmost column. + // We seed it ONCE based on the current layout snapshot + // — subsequent drags by the operator take precedence. + if (!result.containsKey(outputsNodeId)) { + final maxCol = cols.values.isEmpty + ? 0 + : cols.values.reduce((a, b) => a > b ? a : b); + final x = originX + (maxCol + 1) * (nodeWidth + hGap); + result[outputsNodeId] = NodePosition(x, originY); + } return FlowLayout(positions: result); } diff --git a/lib/src/widgets/flow_canvas.dart b/lib/src/widgets/flow_canvas.dart index 2292e47..0237184 100644 --- a/lib/src/widgets/flow_canvas.dart +++ b/lib/src/widgets/flow_canvas.dart @@ -35,6 +35,7 @@ import 'package:flutter/material.dart'; import '../editor_controller.dart'; +import '../model/auto_layout.dart'; import '../model/flow_graph.dart'; import '../model/layout_store.dart'; import '../tokens.dart'; @@ -46,12 +47,13 @@ import 'flow_node.dart'; /// scales as needed. const double _canvasWidth = 4000; const double _canvasHeight = 3000; -// Fixed canvas-coords for the inputs/outputs pseudo-nodes. -// Steps auto-layout starts at AutoLayout.originX (=320), so -// inputs at x=40 leaves a comfortable gap; outputs slides to -// the right of the right-most step on render. -const double _inputsX = 40; -const double _inputsY = 80; +// Fallback positions when the layout sidecar somehow lacks an +// entry for the inputs/outputs endpoint nodes (shouldn't +// happen — AutoLayout always seeds them — but defending in +// depth so a corrupt sidecar never renders an off-screen +// endpoint). +const NodePosition _inputsFallback = NodePosition(40, 80); +const NodePosition _outputsFallback = NodePosition(1200, 80); class FlowCanvas extends StatefulWidget { final FlowEditorController controller; @@ -95,7 +97,19 @@ class _FlowCanvasState extends State { final theme = Theme.of(context); final graph = widget.controller.graph; final layout = widget.controller.layout; - final outputsX = _outputsX(graph, layout); + // Endpoint positions now live in the layout sidecar + // alongside every step's position — see + // AutoLayout.layout() which seeds defaults. Reading them + // here (instead of recomputing _outputsX from current + // step positions every build) means the endpoints stay + // put when the operator drags a step around. The whole + // "outputs panel shifts when I move a node" pain Stefan + // flagged is solved structurally: there is no auto- + // recompute path left to trigger. + final inputsPos = + layout.positions[AutoLayout.inputsNodeId] ?? _inputsFallback; + final outputsPos = + layout.positions[AutoLayout.outputsNodeId] ?? _outputsFallback; // Auto-fit on first build for each flow so the operator // sees the whole graph immediately, even on flows whose // auto-layout pushes nodes past the default viewport. @@ -129,7 +143,7 @@ class _FlowCanvasState extends State { child: IgnorePointer( child: CustomPaint( painter: EdgePainter( - segments: _buildSegments(graph, layout, outputsX), + segments: _buildSegments(graph, layout), baseColor: theme.colorScheme.onSurfaceVariant .withValues(alpha: 0.55), highlightColor: theme.colorScheme.primary, @@ -138,43 +152,33 @@ class _FlowCanvasState extends State { ), ), ), - // Inputs pseudo-node. - Positioned( - left: _inputsX, - top: _inputsY, - child: FlowNode( - id: '__inputs__', - title: 'inputs', - kind: NodeVisualKind.inputs, - inputPortLabels: graph.inputs.keys - .map((k) => '$k: ${graph.inputs[k]!.type}') - .toList(), - selected: false, - ), + // Inputs endpoint — draggable, position + // from the layout sidecar. No selection + // affordance because there's no per-step + // properties to edit; click on the row + // does nothing for now. + _endpointPositioned( + nodeId: AutoLayout.inputsNodeId, + pos: inputsPos, + title: 'inputs', + kind: NodeVisualKind.inputs, + labels: graph.inputs.keys + .map((k) => '$k: ${graph.inputs[k]!.type}') + .toList(), ), - // Outputs pseudo-node. - Positioned( - left: outputsX, - top: _inputsY, - child: FlowNode( - id: '__outputs__', - title: 'outputs', - kind: NodeVisualKind.outputs, - inputPortLabels: graph.outputs.keys.toList(), - selected: false, - ), + // Outputs endpoint — same model. + _endpointPositioned( + nodeId: AutoLayout.outputsNodeId, + pos: outputsPos, + title: 'outputs', + kind: NodeVisualKind.outputs, + labels: graph.outputs.keys.toList(), ), // Step nodes — positioned absolutely, drag to // move, click to select. - for (final step in graph.steps) - _stepPositioned(step, layout, outputsX), - // Port hit-targets for connection drawing. A - // transparent overlay positioned over each port - // — easier to manage than per-port GestureDetectors - // inside the node widget because connection drags - // need to cross node boundaries (start in one node, - // end in another). - ..._portOverlays(graph, layout, outputsX), + for (final step in graph.steps) _stepPositioned(step, layout), + // Port hit-targets for connection drawing. + ..._portOverlays(graph, layout), if (_draft != null) Positioned.fill( child: IgnorePointer( @@ -222,8 +226,8 @@ class _FlowCanvasState extends State { ); } - /// Compute the bounding box of every visible node + the - /// inputs / outputs sidebars, then set the + /// Compute the bounding box of every visible node — steps + /// + the inputs / outputs endpoints — then set the /// TransformationController so the box fills the visible /// viewport with breathing room. No-op when there's no /// active flow (nothing to fit). @@ -232,18 +236,25 @@ class _FlowCanvasState extends State { final layout = widget.controller.layout; if (widget.controller.activeName == null) return; if (graph.steps.isEmpty && graph.inputs.isEmpty) return; - // Bounding box: start with the inputs pseudo-node. - double minX = _inputsX; - double minY = _inputsY; - double maxX = _inputsX + NodeGeometry.width; + final inputsPos = + layout.positions[AutoLayout.inputsNodeId] ?? _inputsFallback; + final outputsPos = + layout.positions[AutoLayout.outputsNodeId] ?? _outputsFallback; + // Bounding box: start with the inputs + outputs endpoints + // since they're always present, then expand to include + // every step. + double minX = inputsPos.x; + double minY = inputsPos.y; + double maxX = outputsPos.x + NodeGeometry.width; double maxY = - _inputsY + + inputsPos.y + NodeGeometry.heightFor(graph.inputs.length).clamp(110.0, 600.0); - // Outputs. - final outputsX = _outputsX(graph, layout); - maxX = outputsX + NodeGeometry.width > maxX - ? outputsX + NodeGeometry.width - : maxX; + if (outputsPos.x < minX) minX = outputsPos.x; + if (outputsPos.y < minY) minY = outputsPos.y; + final outputsBottom = + outputsPos.y + + NodeGeometry.heightFor(graph.outputs.length).clamp(110.0, 600.0); + if (outputsBottom > maxY) maxY = outputsBottom; // Step nodes. for (final step in graph.steps) { final pos = layout.positions[step.id]; @@ -275,9 +286,9 @@ class _FlowCanvasState extends State { ..scaleByDouble(finalScale, finalScale, 1, 1); } - // --- Step positioning + drag --- + // --- Node positioning + drag --- - Widget _stepPositioned(FlowStep step, FlowLayout layout, double _) { + Widget _stepPositioned(FlowStep step, FlowLayout layout) { final pos = layout.positions[step.id]; if (pos == null) return const SizedBox.shrink(); final selected = widget.controller.selectedStepId == step.id; @@ -295,78 +306,105 @@ class _FlowCanvasState extends State { selected: selected, status: status, onTap: () => widget.controller.selectStep(step.id), - onDrag: (delta) { - final scale = _transform.value.getMaxScaleOnAxis(); - final scaledDelta = delta / scale; - final newPos = NodePosition( - (pos.x + scaledDelta.dx).clamp( - 0.0, - _canvasWidth - NodeGeometry.width, - ), - (pos.y + scaledDelta.dy).clamp( - 0.0, - _canvasHeight - NodeGeometry.heightFor(step.with_.length), - ), - ); - widget.controller.moveStep(step.id, newPos); - }, + onDrag: (delta) => _applyDrag( + step.id, + pos, + delta, + NodeGeometry.heightFor(step.with_.length), + ), ), ); } + /// Endpoint nodes (inputs / outputs) live on the canvas + /// just like step nodes — same drag handler, same position + /// stored in the layout sidecar. Difference: no select + /// affordance (no per-step properties to edit) and no + /// status indicator (endpoints don't run). + Widget _endpointPositioned({ + required String nodeId, + required NodePosition pos, + required String title, + required NodeVisualKind kind, + required List labels, + }) { + return Positioned( + left: pos.x, + top: pos.y, + child: FlowNode( + id: nodeId, + title: title, + kind: kind, + inputPortLabels: labels, + selected: false, + onDrag: (delta) => _applyDrag( + nodeId, + pos, + delta, + NodeGeometry.heightFor(labels.length), + ), + ), + ); + } + + /// Single drag entry point used by every node on the + /// canvas. Converts a screen-space delta to canvas-space + /// (via the current InteractiveViewer scale), clamps the + /// new position to the canvas bounds, and forwards to the + /// controller which persists to the sidecar. + void _applyDrag( + String nodeId, + NodePosition current, + Offset delta, + double nodeHeight, + ) { + final scale = _transform.value.getMaxScaleOnAxis(); + final scaledDelta = delta / scale; + final newPos = NodePosition( + (current.x + scaledDelta.dx).clamp( + 0.0, + _canvasWidth - NodeGeometry.width, + ), + (current.y + scaledDelta.dy).clamp(0.0, _canvasHeight - nodeHeight), + ); + widget.controller.moveStep(nodeId, newPos); + } + // --- Port positions in canvas coordinates --- - Offset _outputPortPosition( - String nodeId, - FlowLayout layout, - double outputsX, - ) { - if (nodeId == '__inputs__') { - // Doesn't have an "output" in the canvas sense — the - // inputs pseudo-node exposes its declared inputs as - // ports on its right edge, one per input. The - // _outputPortPosition is called only for step nodes. - // This branch is unreachable; return safely. - return const Offset(0, 0); - } + /// Right-edge output port for a step node. + Offset _outputPortPosition(String nodeId, FlowLayout layout) { final pos = layout.positions[nodeId]; - if (pos == null) return const Offset(0, 0); + if (pos == null) return Offset.zero; return Offset( pos.x + NodeGeometry.width, pos.y + NodeGeometry.outputPortY(), ); } - Offset _inputPortPosition( - String nodeId, - int portIndex, - FlowLayout layout, - double outputsX, - ) { - if (nodeId == '__outputs__') { - return Offset(outputsX, _inputsY + NodeGeometry.inputPortY(portIndex)); - } + /// Left-edge input port for any node. Works for step nodes + /// AND the outputs endpoint — both have input ports on + /// their left, both have a layout position. + Offset _inputPortPosition(String nodeId, int portIndex, FlowLayout layout) { final pos = layout.positions[nodeId]; - if (pos == null) return const Offset(0, 0); + if (pos == null) return Offset.zero; return Offset(pos.x, pos.y + NodeGeometry.inputPortY(portIndex)); } - // Inputs node exposes one port per declared input on its - // RIGHT edge — every input is a "source" of data. - Offset _inputsPseudoPortPosition(int portIndex) { + /// Inputs endpoint exposes one port per declared input on + /// its RIGHT edge — every declared input is a "source" of + /// data that downstream steps can read from. + Offset _inputsEndpointPortPosition(int portIndex, FlowLayout layout) { + final pos = layout.positions[AutoLayout.inputsNodeId] ?? _inputsFallback; return Offset( - _inputsX + NodeGeometry.width, - _inputsY + NodeGeometry.inputPortY(portIndex), + pos.x + NodeGeometry.width, + pos.y + NodeGeometry.inputPortY(portIndex), ); } // --- Edge build (graph -> render segments) --- - List _buildSegments( - FlowGraph graph, - FlowLayout layout, - double outputsX, - ) { + List _buildSegments(FlowGraph graph, FlowLayout layout) { final out = []; final inputsList = graph.inputs.keys.toList(); for (final edge in graph.edges) { @@ -375,9 +413,9 @@ class _FlowCanvasState extends State { if (edge.fromKind == EdgeEndpointKind.inputs) { final idx = inputsList.indexOf(edge.fromField); - if (idx >= 0) from = _inputsPseudoPortPosition(idx); + if (idx >= 0) from = _inputsEndpointPortPosition(idx, layout); } else if (edge.fromKind == EdgeEndpointKind.step) { - from = _outputPortPosition(edge.fromId, layout, outputsX); + from = _outputPortPosition(edge.fromId, layout); } if (edge.toKind == EdgeEndpointKind.step) { final step = graph.steps.firstWhere( @@ -386,13 +424,13 @@ class _FlowCanvasState extends State { ); final idx = step.with_.keys.toList().indexOf(edge.toField); if (idx >= 0) { - to = _inputPortPosition(edge.toId, idx, layout, outputsX); + to = _inputPortPosition(edge.toId, idx, layout); } } else if (edge.toKind == EdgeEndpointKind.outputs) { final outputsList = graph.outputs.keys.toList(); final idx = outputsList.indexOf(edge.toField); if (idx >= 0) { - to = _inputPortPosition('__outputs__', idx, layout, outputsX); + to = _inputPortPosition(AutoLayout.outputsNodeId, idx, layout); } } if (from == null || to == null) continue; @@ -412,15 +450,10 @@ class _FlowCanvasState extends State { // --- Port overlays (drag handles for creating edges) --- - Iterable _portOverlays( - FlowGraph graph, - FlowLayout layout, - double outputsX, - ) sync* { - // Output ports — only on step nodes. Inputs node has its - // own input-list port handles below. + Iterable _portOverlays(FlowGraph graph, FlowLayout layout) sync* { + // Output ports — step nodes' right edges. for (final step in graph.steps) { - final p = _outputPortPosition(step.id, layout, outputsX); + final p = _outputPortPosition(step.id, layout); yield _portDot( center: p, isSource: true, @@ -432,10 +465,10 @@ class _FlowCanvasState extends State { ), ); } - // Inputs pseudo-node output ports (one per input). + // Inputs endpoint output ports — one per declared input. final inputs = graph.inputs.keys.toList(); for (var i = 0; i < inputs.length; i++) { - final p = _inputsPseudoPortPosition(i); + final p = _inputsEndpointPortPosition(i, layout); final fieldName = inputs[i]; yield _portDot( center: p, @@ -448,11 +481,11 @@ class _FlowCanvasState extends State { ), ); } - // Step input port targets. + // Step input port targets — left edges. for (final step in graph.steps) { final keys = step.with_.keys.toList(); for (var i = 0; i < keys.length; i++) { - final p = _inputPortPosition(step.id, i, layout, outputsX); + final p = _inputPortPosition(step.id, i, layout); yield _portDot( center: p, isSource: false, @@ -460,15 +493,15 @@ class _FlowCanvasState extends State { ); } } - // Outputs pseudo-node input ports. + // Outputs endpoint input ports. final outs = graph.outputs.keys.toList(); for (var i = 0; i < outs.length; i++) { - final p = _inputPortPosition('__outputs__', i, layout, outputsX); + final p = _inputPortPosition(AutoLayout.outputsNodeId, i, layout); yield _portDot( center: p, isSource: false, onDropTarget: _dropTargetFor( - '__outputs__', + AutoLayout.outputsNodeId, outs[i], _DraftTargetKind.outputsField, ), @@ -489,8 +522,7 @@ class _FlowCanvasState extends State { // drop will snap to if they release now. final dragging = _draft != null; final isInputDuringDrag = dragging && !isSource; - final isClosest = - isInputDuringDrag && _isClosestDropTarget(center); + final isClosest = isInputDuringDrag && _isClosestDropTarget(center); final size = isClosest ? 22.0 : 16.0; return Positioned( left: center.dx - size / 2, @@ -528,11 +560,10 @@ class _FlowCanvasState extends State { color: isClosest ? theme.colorScheme.primary : isSource - ? theme.colorScheme.primary - : isInputDuringDrag - ? theme.colorScheme.primary - .withValues(alpha: 0.35) - : theme.colorScheme.surfaceContainerHighest, + ? theme.colorScheme.primary + : isInputDuringDrag + ? theme.colorScheme.primary.withValues(alpha: 0.35) + : theme.colorScheme.surfaceContainerHighest, border: Border.all( color: theme.colorScheme.primary, width: isClosest ? 2.5 : 1.5, @@ -540,8 +571,7 @@ class _FlowCanvasState extends State { boxShadow: isClosest ? [ BoxShadow( - color: theme.colorScheme.primary - .withValues(alpha: 0.5), + color: theme.colorScheme.primary.withValues(alpha: 0.5), blurRadius: 10, ), ] @@ -562,14 +592,13 @@ class _FlowCanvasState extends State { if (draft == null) return false; final graph = widget.controller.graph; final layout = widget.controller.layout; - final outputsX = _outputsX(graph, layout); const maxDist = 32.0; double bestDist = double.infinity; Offset? best; for (final step in graph.steps) { final keys = step.with_.keys.toList(); for (var i = 0; i < keys.length; i++) { - final p = _inputPortPosition(step.id, i, layout, outputsX); + final p = _inputPortPosition(step.id, i, layout); final d = (draft.cursor - p).distance; if (d < bestDist && d <= maxDist) { bestDist = d; @@ -579,7 +608,7 @@ class _FlowCanvasState extends State { } final outs = graph.outputs.keys.toList(); for (var i = 0; i < outs.length; i++) { - final p = _inputPortPosition('__outputs__', i, layout, outputsX); + final p = _inputPortPosition(AutoLayout.outputsNodeId, i, layout); final d = (draft.cursor - p).distance; if (d < bestDist && d <= maxDist) { bestDist = d; @@ -609,14 +638,13 @@ class _FlowCanvasState extends State { // Find the closest input port within tolerance. final graph = widget.controller.graph; final layout = widget.controller.layout; - final outputsX = _outputsX(graph, layout); _DropTarget? best; double bestDist = double.infinity; const maxDist = 32.0; for (final step in graph.steps) { final keys = step.with_.keys.toList(); for (var i = 0; i < keys.length; i++) { - final p = _inputPortPosition(step.id, i, layout, outputsX); + final p = _inputPortPosition(step.id, i, layout); final d = (draft.cursor - p).distance; if (d < bestDist && d <= maxDist) { bestDist = d; @@ -630,13 +658,13 @@ class _FlowCanvasState extends State { } final outs = graph.outputs.keys.toList(); for (var i = 0; i < outs.length; i++) { - final p = _inputPortPosition('__outputs__', i, layout, outputsX); + final p = _inputPortPosition(AutoLayout.outputsNodeId, i, layout); final d = (draft.cursor - p).distance; if (d < bestDist && d <= maxDist) { bestDist = d; best = _DropTarget( kind: _DraftTargetKind.outputsField, - id: '__outputs__', + id: AutoLayout.outputsNodeId, field: outs[i], ); } @@ -679,19 +707,6 @@ class _FlowCanvasState extends State { } } - // --- Outputs node placement --- - - double _outputsX(FlowGraph graph, FlowLayout layout) { - double maxX = _inputsX + NodeGeometry.width + 200; - for (final step in graph.steps) { - final pos = layout.positions[step.id]; - if (pos == null) continue; - final right = pos.x + NodeGeometry.width; - if (right > maxX) maxX = right; - } - return maxX + 120; - } - // --- Background --- Widget _grid(ThemeData theme) {