From 8360a7c011a02a77d1e6f863a165a62be6ff92b3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 04:28:26 +0000 Subject: [PATCH 1/2] feat: resizable panes, zoom, and a window that opens where it was left [minor] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four changes to how the editor is arranged and looked at. AstGraph.SeparateOverlaps is gone. ktsu.ForceDirectedLayout 3.18.0 does that job in the simulation itself, so the application was undoing overlaps the library had already resolved. The editor test that covered it stays, asserting the rectangles directly: the separation only works when a node's measured size reaches the simulation, and that wiring is still this application's. The panes are divider containers, so the user sizes them rather than the application. Properties and the code preview are stacked in one column with properties on top: they are read at different times and want different shapes, and sharing a column gives each the full width and lets the user decide the split. The editor no longer draws an inspector of its own here — it still can, for a host that wants one — so the toolbar's Inspector checkbox goes, the divider being a better way to do the same thing. The graph can be zoomed, and "Fit to canvas" now zooms as well as centres. The node editor underneath has no zoom, so the scaling is applied to the engine's positions and the font together, put on before the frame is drawn and taken off after: the simulation is never asked to work in a space that changes under it, and a node dragged while zoomed lands where the pointer was. Fitting picks the largest zoom the whole arrangement still fits at, and never magnifies — a graph that already fits is shown at its own size. The window and the pane splits are remembered between runs, alongside the recent files and the preview language, since they are all things the user arranged rather than settings the application should keep choosing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QwzCFb8zhd263sZeRbF7ba --- Coder.Editor/Coder.Editor.csproj | 4 + Coder.Editor/CoderEditorApp.cs | 124 ++++++++++-- Coder.Editor/EditorSettings.cs | 46 +++++ Coder.Graph/AstGraph.cs | 92 +-------- Coder.Graph/AstGraphEditor.cs | 242 +++++++++++++++++++++--- Coder.Test/Coder.Test.csproj | 2 + Coder.Test/Editor/EditorWiringTests.cs | 44 ++++- Coder.Test/Graph/AstGraphEditorTests.cs | 26 ++- Coder.Test/Graph/AstGraphLayoutTests.cs | 134 ++++++------- Directory.Packages.props | 2 + 10 files changed, 511 insertions(+), 205 deletions(-) diff --git a/Coder.Editor/Coder.Editor.csproj b/Coder.Editor/Coder.Editor.csproj index 3771817..0aadd15 100644 --- a/Coder.Editor/Coder.Editor.csproj +++ b/Coder.Editor/Coder.Editor.csproj @@ -14,6 +14,10 @@ + + + + diff --git a/Coder.Editor/CoderEditorApp.cs b/Coder.Editor/CoderEditorApp.cs index 183d597..7550844 100644 --- a/Coder.Editor/CoderEditorApp.cs +++ b/Coder.Editor/CoderEditorApp.cs @@ -12,6 +12,8 @@ namespace ktsu.Coder.Editor; using ktsu.Coder.Languages; using ktsu.ImGui.App; using ktsu.ImGui.SyntaxHighlighting; +using ktsu.ImGui.Widgets; +using Silk.NET.Windowing; /// /// The editor: a graph of the document on the left, the code it generates on the right. @@ -41,7 +43,7 @@ public sealed class CoderEditorApp( /// /// Gets the editor over the current document. /// - public AstGraphEditor Editor { get; private set; } = new(NewDocument()); + public AstGraphEditor Editor { get; private set; } = EditorFor(NewDocument()); /// /// Gets the file the document was last read from or written to, or null for an unsaved one. @@ -78,6 +80,16 @@ public sealed class CoderEditorApp( OnStart = () => Editor.LayoutRunning = Settings.LayoutRunning, OnRender = Draw, + // Opened where it was left. The window is part of what a user arranges, and an application + // that forgets it is one they rearrange on every run. + InitialWindowState = new ImGuiAppWindowState + { + Size = new Vector2(Settings.WindowWidth, Settings.WindowHeight), + Pos = new Vector2(Settings.WindowX, Settings.WindowY), + LayoutState = Settings.WindowMaximized ? WindowState.Maximized : WindowState.Normal, + }, + OnMoveOrResize = RememberWindow, + // The menu goes here rather than inside OnRender: ImGuiApp's main window carries no // ImGuiWindowFlags.MenuBar, so an ImGui.BeginMenuBar() call in the render delegate always // returns false and the menu silently never appears. OnAppMenu runs inside the application's @@ -85,6 +97,36 @@ public sealed class CoderEditorApp( OnAppMenu = DrawMenu, }; + /// + /// Notes where the window is, so the next run opens there. + /// + /// + /// The size and position read back are the ones the window has when it is not maximized, which is + /// what should be restored when a maximized window is un-maximized. Settings are written to disk + /// when the application exits, so this only has to keep them current. + /// + private void RememberWindow() + { + ImGuiAppWindowState state = ImGuiApp.WindowState; + + Settings.WindowWidth = state.Size.X; + Settings.WindowHeight = state.Size.Y; + Settings.WindowX = state.Pos.X; + Settings.WindowY = state.Pos.Y; + Settings.WindowMaximized = state.LayoutState == WindowState.Maximized; + } + + /// + /// Builds an editor over a document, set up the way this application hosts one. + /// + /// The document to edit. + /// The editor. + /// + /// The properties panel is one of this application's own panes, so the editor is told not to draw + /// a second one of its own beside the canvas. + /// + private static AstGraphEditor EditorFor(AstNode document) => new(document) { ShowInspector = false }; + /// /// Builds the document a fresh editor opens with. /// @@ -126,19 +168,72 @@ public void Draw(float deltaTime) HandleShortcuts(); Vector2 available = ImGui.GetContentRegionAvail(); - float graphWidth = available.X * 0.62f; - ImGui.BeginChild("graph-pane", new Vector2(graphWidth, available.Y - StatusBarHeight)); - Editor.Draw(new Vector2(graphWidth - PaneInset, available.Y - StatusBarHeight - PaneInset), deltaTime); + // The container measures itself from the remaining content region, so it is given a child of + // the height that is actually the panes' — otherwise it would take the status bar's row too + // and draw the bar over its own bottom edge. + ImGui.BeginChild("panes", new Vector2(0, available.Y - StatusBarHeight)); + Panes.Tick(deltaTime); ImGui.EndChild(); - ImGui.SameLine(); + DrawStatusBar(); + } - ImGui.BeginChild("code-pane", new Vector2(0, available.Y - StatusBarHeight)); - DrawCodePane(); - ImGui.EndChild(); + /// + /// Gets the pane layout, built on first use. + /// + /// + /// A divider container holds the sizes the user has dragged the panes to, so it has to outlive + /// the frame. It is built lazily rather than in a field initializer because its zones call back + /// into this instance, which a field initializer cannot refer to. + /// + private ImGuiWidgets.DividerContainer Panes => field ??= BuildPanes(); - DrawStatusBar(); + /// + /// Builds the resizable pane layout: the graph, and beside it the properties above the code. + /// + /// The container to tick each frame. + /// + /// Properties and code are stacked rather than placed side by side because they are read at + /// different times and want different shapes: properties are a short column of labelled rows, + /// while generated source is lines that want to be read down. Sharing one column gives each of + /// them the full width and lets the user decide how the height is split between them — which is + /// the point of making these panes rather than fixed regions. + /// + /// The sizes are remembered between runs, so an arrangement the user settled on is the one they + /// come back to. + /// + /// + private ImGuiWidgets.DividerContainer BuildPanes() + { + ImGuiWidgets.DividerContainer side = new( + "coder-side", + container => Settings.PropertiesSplit = container.GetSizes()[0], + ImGuiWidgets.DividerLayout.Rows, + [ + new ImGuiWidgets.DividerZone("properties", Settings.PropertiesSplit, DrawPropertiesPane), + new ImGuiWidgets.DividerZone("code", 1f - Settings.PropertiesSplit, _ => DrawCodePane()), + ]); + + return new ImGuiWidgets.DividerContainer( + "coder-panes", + container => Settings.GraphSplit = container.GetSizes()[0], + ImGuiWidgets.DividerLayout.Columns, + [ + new ImGuiWidgets.DividerZone("graph", Settings.GraphSplit, deltaTime => + Editor.Draw(ImGui.GetContentRegionAvail(), deltaTime)), + new ImGuiWidgets.DividerZone("side", 1f - Settings.GraphSplit, side.Tick), + ]); + } + + /// + /// Draws the selected node's properties, which the editor supplies but does not place. + /// + /// Seconds since the last frame; the panel does not animate, so unused. + private void DrawPropertiesPane(float deltaTime) + { + ImGui.TextUnformatted("Properties"); + Editor.DrawInspector(ImGui.GetContentRegionAvail()); } /// @@ -185,11 +280,6 @@ private void HandleShortcuts() /// private const float StatusBarHeight = 28f; - /// - /// The margin between a pane's edge and what it contains. - /// - private const float PaneInset = 12f; - /// /// Draws the application's File menu. /// @@ -433,7 +523,8 @@ private void DrawStatusBar() /// private void Replace(AstNode document, string status) { - Editor = new AstGraphEditor(document) { LayoutRunning = Settings.LayoutRunning }; + Editor = EditorFor(document); + Editor.LayoutRunning = Settings.LayoutRunning; DocumentPath = null; Status = status; GeneratedCode = string.Empty; @@ -453,7 +544,8 @@ public bool Open(string path) return false; } - Editor = new AstGraphEditor(result.Root) { LayoutRunning = Settings.LayoutRunning }; + Editor = EditorFor(result.Root); + Editor.LayoutRunning = Settings.LayoutRunning; DocumentPath = result.Path; Settings.Remember(result.Path!); Status = $"Opened {result.Path}."; diff --git a/Coder.Editor/EditorSettings.cs b/Coder.Editor/EditorSettings.cs index 7fbc326..a2f7409 100644 --- a/Coder.Editor/EditorSettings.cs +++ b/Coder.Editor/EditorSettings.cs @@ -34,6 +34,52 @@ public sealed class EditorSettings /// public bool LayoutRunning { get; set; } = true; + /// + /// Gets or sets the window's width when it is not maximized. + /// + public float WindowWidth { get; set; } = 1280f; + + /// + /// Gets or sets the window's height when it is not maximized. + /// + public float WindowHeight { get; set; } = 720f; + + /// + /// Gets or sets the window's horizontal position when it is not maximized. + /// + /// + /// Defaulted to the windowing layer's own "no position yet" value, which is deliberately far off + /// screen and means the platform should place the window. A first run therefore opens wherever + /// the system would have put it rather than in a corner this application chose. + /// + public float WindowX { get; set; } = -short.MinValue; + + /// + /// Gets or sets the window's vertical position when it is not maximized. + /// + public float WindowY { get; set; } = -short.MinValue; + + /// + /// Gets or sets a value indicating whether the window was maximized. + /// + /// + /// Kept alongside the size rather than instead of it: a maximized window still has a size to go + /// back to when it is restored, and that is the one worth remembering. + /// + public bool WindowMaximized { get; set; } + + /// + /// Gets or sets the share of the window's width the graph takes, the rest going to the panel + /// beside it. + /// + public float GraphSplit { get; set; } = 0.62f; + + /// + /// Gets or sets the share of that panel's height the properties take, the rest going to the code + /// preview under them. + /// + public float PropertiesSplit { get; set; } = 0.4f; + /// /// Records a file as the most recently opened, without letting the list grow or repeat. /// diff --git a/Coder.Graph/AstGraph.cs b/Coder.Graph/AstGraph.cs index bb4336e..1cb46c4 100644 --- a/Coder.Graph/AstGraph.cs +++ b/Coder.Graph/AstGraph.cs @@ -43,25 +43,6 @@ public sealed class AstGraph /// private const float SiblingSpacing = 110f; - /// - /// The clear space left between the boxes of two nodes. - /// - private const float NodeMargin = 24f; - - /// - /// The furthest a pair of overlapping nodes is moved apart in one step, in pixels. - /// - /// - /// The overlap is otherwise resolved in full each step, because a partial correction loses: - /// between two linked nodes the spring pulls them back together by more, every frame, than a - /// fraction of the overlap pushes them apart, and they come to rest still overlapping. Resolving - /// it in full and capping the step keeps the correction decisive and still lets a deep overlap — - /// two nodes dropped on the same spot — slide apart over several frames rather than jumping. - /// - private const float MaxSeparationStep = 40f; - - private const float SeparationTolerance = 0.5f; - private readonly Dictionary positions = new(ReferenceEqualityComparer.Instance); private readonly Dictionary nodesById = []; private readonly Dictionary idsByNode = new(ReferenceEqualityComparer.Instance); @@ -215,10 +196,9 @@ public int AddDetached(AstNode node, Vector2 position) /// Where the node was asked to go. /// That position, or the nearest free one along a diagonal from it. /// - /// Two nodes at exactly the same point stay there for ever: the layout's repulsion is computed - /// from the direction between them, and coincident points have no direction. Creating two nodes - /// from the palette without moving the mouse is enough to hit that, so the offset is applied when - /// the node is placed rather than left for the simulation to sort out. + /// Creating two nodes from the palette without moving the mouse puts them at the same point. The + /// simulation does pull them apart from there, but a node that appears exactly on top of the last + /// one and then slides out is a worse answer than one that is placed clear to begin with. /// private Vector2 Separated(Vector2 position) { @@ -233,72 +213,6 @@ private Vector2 Separated(Vector2 position) return candidate; } - /// - /// Eases apart any nodes whose boxes are on top of one another, by one step's worth. - /// - /// The deepest overlap found, in pixels, or zero when nothing overlapped. - /// - /// The force-directed layout treats every node as a point: repulsion is computed between centres - /// and the link spring pulls to a fixed length, neither of which knows how wide a node is. Two - /// nodes can therefore sit at a perfectly comfortable distance by that measure and still have - /// their boxes squarely on top of each other, which is what a user sees. This resolves the - /// overlap the layout cannot see, working on the drawn rectangles rather than on centres. - /// - /// Each overlap is resolved along the axis it is shallowest on, which is both the shorter push - /// and the one that leaves the arrangement the layout worked out most nearly as it was, and by no - /// more than at a time, so a deep overlap slides apart over a few - /// frames rather than jumping. - /// - /// - public float SeparateOverlaps() - { - Node[] nodes = [.. Engine.Nodes]; - Vector2[] moved = [.. nodes.Select(node => node.Position)]; - float deepest = 0f; - - for (int i = 0; i < nodes.Length; i++) - { - for (int j = i + 1; j < nodes.Length; j++) - { - Vector2 clearance = ((nodes[i].Dimensions + nodes[j].Dimensions) * 0.5f) + new Vector2(NodeMargin); - Vector2 first = moved[i] + (nodes[i].Dimensions * 0.5f); - Vector2 second = moved[j] + (nodes[j].Dimensions * 0.5f); - Vector2 between = second - first; - Vector2 overlap = clearance - Vector2.Abs(between); - - if (overlap.X <= SeparationTolerance || overlap.Y <= SeparationTolerance) - { - continue; - } - - // Never further than the overlap itself, so the pair cannot be driven past each other and - // back again, and shared equally between them so the arrangement's centre stays put. - float depth = Math.Min(overlap.X, overlap.Y); - float amount = Math.Min(depth, MaxSeparationStep) * 0.5f; - - // A zero component has no side to be on, so the later node is pushed the positive way: - // an arbitrary choice, but a consistent one, which is what stops the pair jittering. - Vector2 push = overlap.X < overlap.Y - ? new Vector2(amount * (between.X < 0f ? -1f : 1f), 0f) - : new Vector2(0f, amount * (between.Y < 0f ? -1f : 1f)); - - moved[j] += push; - moved[i] -= push; - deepest = Math.Max(deepest, depth); - } - } - - for (int i = 0; i < nodes.Length; i++) - { - if (moved[i] != nodes[i].Position) - { - Engine.UpdateNodePosition(nodes[i].Id, moved[i]); - } - } - - return deepest; - } - /// /// Reports where a node currently sits. /// diff --git a/Coder.Graph/AstGraphEditor.cs b/Coder.Graph/AstGraphEditor.cs index d5b1ff0..a74bfb7 100644 --- a/Coder.Graph/AstGraphEditor.cs +++ b/Coder.Graph/AstGraphEditor.cs @@ -42,6 +42,10 @@ public sealed class AstGraphEditor(AstNode root) private bool fitted; + private bool fontScaled; + + private readonly Dictionary viewSpace = []; + private string? editingField; /// @@ -75,8 +79,34 @@ public sealed class AstGraphEditor(AstNode root) public bool ShowDebugOverlays { get; set; } /// - /// Gets or sets a value indicating whether the inspector panel is drawn. + /// Gets or sets how large the graph is drawn, as a multiplier: 1 draws it at its own size. /// + /// + /// A view setting, not a document one. The graph's positions and the layout that arranges them + /// are kept at their own scale whatever this is, and the zoom is applied and undone around the + /// one frame that draws them — so the simulation is never asked to work in a space that changes + /// under it, and a node dragged while zoomed out lands where the pointer was. + /// + /// The node editor underneath has no zoom of its own, so this is what there is: it scales the + /// distances between nodes and the text inside them together, which is what makes a whole graph + /// fit on screen rather than merely spreading it out. + /// + /// + public float Zoom + { + get; + set => field = Math.Clamp(value, MinZoom, MaxZoom); + } = 1f; + + /// + /// Gets or sets a value indicating whether puts the inspector panel beside the + /// canvas itself. + /// + /// + /// On for a host that just wants an editor. A host with a layout of its own turns it off and + /// calls wherever it wants the panel, which is what the Coder + /// application does — there it is a pane the user can resize rather than a fixed column. + /// public bool ShowInspector { get; set; } = true; /// @@ -128,15 +158,23 @@ public void Draw(Vector2 size, float deltaTime) // around an origin that is still zero. The first frame is where that becomes knowable, and // fitting once there is what puts a freshly opened document in the middle of the view // immediately rather than leaving the simulation to drag it in from the corner. + // Fitted on the first frame so a freshly opened document is in the middle of the view + // immediately rather than being dragged in from a corner, and then again until the nodes have + // been measured: how big a node is drawn is only known once it has been, and the zoom a fit + // chooses depends on that. Latching on the first measured frame is what stops it from + // overriding a view the user has since changed. if (!fitted) { - fitted = FitView(); + FitView(); + fitted = Graph.Engine.Nodes.Count > 0 && Graph.Engine.Nodes.All(node => node.Dimensions.X > 0f); statusMessage = string.Empty; } Vector2 origin = ImGui.GetCursorScreenPos(); ImGui.BeginChild("ast-graph-canvas", graphSize, ImGuiChildFlags.None, ImGuiWindowFlags.NoScrollbar | ImGuiWindowFlags.NoScrollWithMouse); + EnterViewSpace(); + renderer.Render(Graph.Engine, graphSize); ApplyNodeMovement(); @@ -150,6 +188,8 @@ public void Draw(Vector2 size, float deltaTime) renderer.RenderDebugOverlays(Graph.Engine, origin, graphSize, showDebug: true); } + LeaveViewSpace(); + ImGui.EndChild(); if (ShowInspector) @@ -162,12 +202,9 @@ public void Draw(Vector2 size, float deltaTime) { // Gravity pulls towards the world origin, which is what keeps an arrangement the user has // not touched in the middle of the view rather than drifting out of it. + // The simulation resolves overlapping node boxes itself, since ktsu.ForceDirectedLayout + // 3.18.0, so nothing here has to undo them afterwards. Graph.Engine.UpdatePhysics(deltaTime); - - // The simulation has no idea how big a node is drawn, so it is content to leave two of - // them sitting on top of one another. Undoing that afterwards is what keeps every node's - // caption readable, which is the whole reason the layout runs. - Graph.SeparateOverlaps(); } Problems = Graph.Validate(); @@ -183,6 +220,26 @@ public void Draw(Vector2 size, float deltaTime) /// private const float MinimumGraphWidth = 160f; + /// + /// The smallest the graph is drawn, as a multiplier of its own size. + /// + private const float MinZoom = 0.25f; + + /// + /// The largest the graph is drawn, as a multiplier of its own size. + /// + private const float MaxZoom = 2f; + + /// + /// How much of the canvas a fitted graph is asked to fill, leaving a margin around it. + /// + private const float FitMargin = 0.9f; + + /// + /// The width of the toolbar's zoom slider. + /// + private const float ZoomSliderWidth = 120f; + /// /// Draws the row of controls above the graph. /// @@ -220,16 +277,19 @@ private void DrawToolbar() ImGui.EndDisabled(); ImGui.SameLine(); - if (ImGui.Button("Fit")) + if (ImGui.Button("Fit to canvas")) { FitView(); } + // Shown and dragged as a percentage, which is how every other application spells zoom, while + // the property itself is the multiplier everything is measured in. ImGui.SameLine(); - bool showInspector = ShowInspector; - if (ImGui.Checkbox("Inspector", ref showInspector)) + ImGui.SetNextItemWidth(ZoomSliderWidth); + float percent = Zoom * 100f; + if (ImGui.SliderFloat("##zoom", ref percent, MinZoom * 100f, MaxZoom * 100f, "%.0f%%")) { - ShowInspector = showInspector; + Zoom = percent / 100f; } ImGui.SameLine(); @@ -273,8 +333,8 @@ private void TrackSelection() /// are edited by dragging; everything else about it is a value, and a value needs somewhere to be /// typed. /// - /// The column to draw it in. - private void DrawInspector(Vector2 size) + /// The area to draw it in. + public void DrawInspector(Vector2 size) { // Bordered rather than separated: down the side of the graph a rule is what tells the panel // apart from the canvas it sits beside, where along the bottom a single line would do. @@ -501,6 +561,113 @@ public void Redo() } } + /// + /// Scales the graph into the space it is drawn in, for the duration of one frame's drawing. + /// + /// + /// The node editor underneath has no zoom, and it takes each node's position straight out of the + /// engine, so the only place a zoom can be applied is the engine itself. Doing that permanently + /// would put the simulation in a space that changes whenever the user drags the slider — its rest + /// length, its repulsion distance and its overlap margin are all lengths, and none of them would + /// mean the same thing afterwards. + /// + /// So the scaling is put on before the frame is drawn and taken off again after, by + /// . Between those two calls the engine holds view positions, which is + /// what the renderer draws and what the user's drag is read back in; outside them it holds the + /// graph's own, which is what the layout runs on and what a fit is measured against. + /// + /// + /// Scaled about the world origin — the middle of the canvas — so zooming keeps whatever is in the + /// middle of the view in the middle of the view, rather than sending the graph towards a corner. + /// The text is scaled to match, because a node's box is sized from the text inside it: without + /// that, zooming out would only move the nodes closer together while they stayed the same size, + /// which packs them tighter instead of showing more. + /// + /// + private void EnterViewSpace() + { + fontScaled = false; + viewSpace.Clear(); + + if (IsUnzoomed) + { + return; + } + + Vector2 centre = Graph.Engine.WorldOrigin; + foreach (Node node in Graph.Engine.Nodes.ToArray()) + { + Vector2 view = ((node.Position - centre) * Zoom) + centre; + viewSpace[node.Id] = new ViewState(node.Position, node.Dimensions, view); + Graph.Engine.UpdateNodePosition(node.Id, view); + } + + ImGui.PushFont(ImGui.GetFont(), ImGui.GetFontSize() * Zoom); + fontScaled = true; + } + + /// + /// Takes the scaling back off, returning the engine to the graph's own space. + /// + /// + /// A node the frame did not touch is put back to exactly the value it had rather than divided by + /// the zoom it was multiplied by, and its size is left alone rather than divided at all. That is + /// the difference between a transform and its inverse being applied once and being applied every + /// frame: a size only arrives from the renderer when it has measured a new one, so dividing them + /// unconditionally would divide the same value again on every frame it did not change, and a + /// graph left alone for a second would have nodes thousands of times their real size. + /// + private void LeaveViewSpace() + { + if (fontScaled) + { + ImGui.PopFont(); + fontScaled = false; + } + + if (IsUnzoomed) + { + return; + } + + Vector2 centre = Graph.Engine.WorldOrigin; + foreach (Node node in Graph.Engine.Nodes.ToArray()) + { + // A node the frame replaced — a rebuild reassigns every identifier — is not one this frame + // put into view space, so it is converted rather than restored. + bool known = viewSpace.TryGetValue(node.Id, out ViewState state); + + Graph.Engine.UpdateNodePosition( + node.Id, + known && state.View == node.Position + ? state.Position + : ((node.Position - centre) / Zoom) + centre); + + // Untouched means the renderer reported no new measurement, so what is there is still the + // size from before the frame and is already in the graph's own space. + if (!known || state.Dimensions != node.Dimensions) + { + Graph.Engine.UpdateNodeDimensions(node.Id, node.Dimensions / Zoom); + } + } + + viewSpace.Clear(); + } + + /// + /// Gets a value indicating whether the view is at the graph's own scale, where the transform is + /// the identity and is skipped rather than applied as one. + /// + private bool IsUnzoomed => Math.Abs(Zoom - 1f) < 0.0001f; + + /// + /// What a node looked like before the frame scaled it, and what it was scaled to. + /// + /// Its position in the graph's own space. + /// Its size in the graph's own space. + /// The position it was drawn at, so a value still equal to it is one nothing moved. + private readonly record struct ViewState(Vector2 Position, Vector2 Dimensions, Vector2 View); + /// /// Writes back the positions the user dragged nodes to, and the sizes ImNodes measured. /// @@ -610,19 +777,26 @@ public void Add(AstNode node, Vector2 position) } /// - /// Brings the whole graph back into view, centred on the origin. + /// Brings the whole graph into view: centred on the canvas, and zoomed out far enough to fit. /// /// True if there was anything to bring into view. /// /// The layout arranges nodes wherever the forces take them, and a user can drag one anywhere, so - /// a document can end up off the edge of the view with no clue which way to scroll back. This - /// moves the arrangement rather than the view: the renderer writes each node's position into the - /// node editor every frame, so panning the editor is undone as soon as it is read back — the - /// positions are the only thing that decides where a node is drawn. + /// a document can end up off the edge of the view with no clue which way to scroll back. Worse, + /// a graph can simply be bigger than the canvas, which no amount of centring fixes. /// - /// Centred on the world origin, which is the middle of the canvas, so fitting puts the graph - /// where gravity is going to hold it anyway. The whole arrangement is translated, so the shape - /// the layout settled into is preserved rather than being disturbed by the act of looking at it. + /// Centring moves the arrangement rather than the view: the renderer writes each node's position + /// into the node editor every frame, so panning the editor is undone as soon as it is read back — + /// the positions are the only thing that decides where a node is drawn. The whole arrangement is + /// translated, so the shape the layout settled into is preserved rather than being disturbed by + /// the act of looking at it. + /// + /// + /// The zoom is then whatever makes the arrangement's own extent fit inside the canvas, with a + /// margin so nothing sits against an edge, and never more than — a graph + /// small enough to be magnified is shown at its own size rather than blown up to fill the room. + /// A graph too big even at is shown as small as the view goes, which is the + /// most of it that can be had. /// /// public bool FitView() @@ -650,10 +824,34 @@ public bool FitView() Graph.Engine.UpdateNodePosition(node.Id, node.Position + offset); } - statusMessage = "Brought the graph back into view."; + // The origin is kept on the middle of the canvas, so the canvas is twice it. + Zoom = FittingZoom(highest - lowest, Graph.Engine.WorldOrigin * 2f); + + statusMessage = "Brought the graph into view."; return true; } + /// + /// Works out the largest zoom an arrangement still fits the canvas at. + /// + /// How much room the arrangement takes at its own scale. + /// The room there is to show it in. + /// The zoom to use, within the range the view allows. + /// + /// Never above one, so fitting only ever zooms out. Magnifying a small graph to fill the canvas + /// would be a surprising answer to "fit": the user asked to see all of it, and they already can. + /// + private static float FittingZoom(Vector2 extent, Vector2 canvas) + { + if (extent.X <= 0f || extent.Y <= 0f || canvas.X <= 0f || canvas.Y <= 0f) + { + return 1f; + } + + float fitting = Math.Min(canvas.X / extent.X, canvas.Y / extent.Y) * FitMargin; + return Math.Clamp(Math.Min(fitting, 1f), MinZoom, MaxZoom); + } + /// /// Selects a node, so the inspector shows it and the graph highlights it. /// diff --git a/Coder.Test/Coder.Test.csproj b/Coder.Test/Coder.Test.csproj index f814d5b..a64e1f2 100644 --- a/Coder.Test/Coder.Test.csproj +++ b/Coder.Test/Coder.Test.csproj @@ -19,6 +19,8 @@ + + diff --git a/Coder.Test/Editor/EditorWiringTests.cs b/Coder.Test/Editor/EditorWiringTests.cs index e89c23d..1e55d45 100644 --- a/Coder.Test/Editor/EditorWiringTests.cs +++ b/Coder.Test/Editor/EditorWiringTests.cs @@ -113,7 +113,17 @@ public async Task RunAsync_ReadsSettingsBeforeStartingAndWritesThemAfter() { using ServiceProvider services = Program.BuildServices(); EditorSettingsStore store = services.GetRequiredService(); - EditorSettings stored = new() { PreviewLanguageId = "cpp" }; + EditorSettings stored = new() + { + PreviewLanguageId = "cpp", + WindowWidth = 1600f, + WindowHeight = 900f, + WindowX = 120f, + WindowY = 80f, + WindowMaximized = true, + GraphSplit = 0.45f, + PropertiesSplit = 0.7f, + }; Assert.IsTrue(await store.SaveAsync(stored).ConfigureAwait(false)); ImGuiAppConfig? handed = null; @@ -130,8 +140,40 @@ public async Task RunAsync_ReadsSettingsBeforeStartingAndWritesThemAfter() Assert.AreEqual("Coder", handed.Title); Assert.AreEqual("cpp", after.PreviewLanguageId, "the stored settings should have been read before starting"); + // The window and the panes are arrangements the user made, so they come back with everything + // else rather than being reset on every run. + Assert.AreEqual(1600f, handed.InitialWindowState.Size.X); + Assert.AreEqual(900f, handed.InitialWindowState.Size.Y); + Assert.AreEqual(120f, handed.InitialWindowState.Pos.X); + Assert.AreEqual(80f, handed.InitialWindowState.Pos.Y); + Assert.AreEqual(Silk.NET.Windowing.WindowState.Maximized, handed.InitialWindowState.LayoutState); + EditorSettings reread = await store.LoadAsync().ConfigureAwait(false); Assert.AreEqual("cpp", reread.PreviewLanguageId, "the settings should have been written back on exit"); + Assert.AreEqual(1600f, reread.WindowWidth, "the window size should have been written back on exit"); + Assert.AreEqual(120f, reread.WindowX); + Assert.IsTrue(reread.WindowMaximized); + Assert.AreEqual(0.45f, reread.GraphSplit, "the pane split should have been written back on exit"); + Assert.AreEqual(0.7f, reread.PropertiesSplit); + } + + /// + /// Tests that a fresh run opens a window the platform places, rather than one this application + /// put in a corner. + /// + [TestMethod] + public void BuildConfig_LeavesAFirstRunsPositionToThePlatform() + { + using ServiceProvider services = Program.BuildServices(); + CoderEditorApp app = new( + services.GetRequiredService(), + services.GetServices(), + new EditorSettings()); + + ImGuiAppConfig config = app.BuildConfig(); + + Assert.AreEqual(new ImGuiAppWindowState().Pos, config.InitialWindowState.Pos); + Assert.AreEqual(Silk.NET.Windowing.WindowState.Normal, config.InitialWindowState.LayoutState); } /// diff --git a/Coder.Test/Graph/AstGraphEditorTests.cs b/Coder.Test/Graph/AstGraphEditorTests.cs index 3f3c535..ed5de4c 100644 --- a/Coder.Test/Graph/AstGraphEditorTests.cs +++ b/Coder.Test/Graph/AstGraphEditorTests.cs @@ -9,6 +9,7 @@ namespace ktsu.Coder.Test.Graph; using ktsu.Coder.Graph; using ktsu.ImGui.App; using ktsu.ImGui.App.Testing; +using ktsu.ImGuiNodeEditor; using Microsoft.VisualStudio.TestTools.UnitTesting; /// @@ -218,9 +219,11 @@ public void Editor_CentresTheDocumentOnTheFirstFrame() /// caption is never hidden behind the node in front of it. /// /// - /// Drawn for real rather than asserted headlessly, because how big a node is drawn is only known - /// once it has been: the layout is told a node's size by the renderer that measured it, and this - /// covers that whole path rather than the arithmetic alone. + /// The separation is the layout library's, not this application's, and it is asserted here rather + /// than left to that library's own tests because it only works when a node's measured size reaches + /// the simulation. That happens by the renderer reading the size back out of the node editor and + /// writing it into the engine, which is this application's wiring and is only exercised by drawing + /// real frames. /// [TestMethod] public void Editor_PullsOverlappingNodesApartAsItRuns() @@ -235,7 +238,22 @@ public void Editor_PullsOverlappingNodesApartAsItRuns() using ImGuiAppHarness harness = ImGuiAppHarness.Start(ConfigFor(editor), Options); harness.Step(180); - Assert.AreEqual(0f, editor.Graph.SeparateOverlaps(), "three seconds of layout left nodes drawn over one another"); + Node[] nodes = [.. editor.Graph.Engine.Nodes]; + Assert.IsTrue(nodes.All(node => node.Dimensions.X > 0f), "the renderer should have measured every node"); + + for (int i = 0; i < nodes.Length; i++) + { + for (int j = i + 1; j < nodes.Length; j++) + { + bool apart = + nodes[i].Position.X + nodes[i].Dimensions.X <= nodes[j].Position.X || + nodes[j].Position.X + nodes[j].Dimensions.X <= nodes[i].Position.X || + nodes[i].Position.Y + nodes[i].Dimensions.Y <= nodes[j].Position.Y || + nodes[j].Position.Y + nodes[j].Dimensions.Y <= nodes[i].Position.Y; + + Assert.IsTrue(apart, $"three seconds of layout left {nodes[i].Name} drawn over {nodes[j].Name}"); + } + } } /// diff --git a/Coder.Test/Graph/AstGraphLayoutTests.cs b/Coder.Test/Graph/AstGraphLayoutTests.cs index 36f92ad..867d3be 100644 --- a/Coder.Test/Graph/AstGraphLayoutTests.cs +++ b/Coder.Test/Graph/AstGraphLayoutTests.cs @@ -116,79 +116,6 @@ public void Graph_SeparatesNodes() } } - /// - /// Tests that two nodes drawn on top of one another are pushed apart until their boxes no longer - /// overlap, which the simulation on its own will not do. - /// - /// - /// The layout treats a node as a point: repulsion is measured between centres and the link spring - /// pulls to a fixed length, so two wide nodes can sit at a distance it is perfectly happy with and - /// still be squarely on top of each other. Node dimensions are measured while rendering, so they - /// are set here the way a frame would set them. - /// - [TestMethod] - public void SeparateOverlaps_PushesOverlappingBoxesApart() - { - AstGraph graph = new(new FunctionDeclaration("total") { ReturnType = "int" }); - graph.AddDetached(new VariableReference("a"), new Vector2(500, 300)); - graph.AddDetached(new VariableReference("b"), new Vector2(520, 310)); - - Vector2 dimensions = new(180, 90); - foreach (Node node in graph.Engine.Nodes.ToArray()) - { - graph.Engine.UpdateNodeDimensions(node.Id, dimensions); - } - - Assert.IsTrue(graph.SeparateOverlaps() > 0f, "the nodes start on top of each other"); - - // Enough steps for a correction that is capped per step to finish. - for (int step = 0; step < 200 && graph.SeparateOverlaps() > 0f; step++) - { - // The separation is the work; the loop only has to run it until it reports nothing left. - } - - Assert.AreEqual(0f, graph.SeparateOverlaps(), "the boxes should no longer overlap"); - - Node[] separated = [.. graph.Engine.Nodes]; - for (int i = 0; i < separated.Length; i++) - { - for (int j = i + 1; j < separated.Length; j++) - { - bool apart = separated[i].Position.X + dimensions.X <= separated[j].Position.X - || separated[j].Position.X + dimensions.X <= separated[i].Position.X - || separated[i].Position.Y + dimensions.Y <= separated[j].Position.Y - || separated[j].Position.Y + dimensions.Y <= separated[i].Position.Y; - - Assert.IsTrue(apart, $"nodes {i} and {j} are still drawn over one another"); - } - } - } - - /// - /// Tests that a graph nothing overlaps in is left exactly as it is, so the separation cannot - /// unsettle an arrangement the layout has already finished. - /// - [TestMethod] - public void SeparateOverlaps_LeavesAClearArrangementAlone() - { - AstGraph graph = new(new FunctionDeclaration("total") { ReturnType = "int" }); - graph.AddDetached(new VariableReference("a"), new Vector2(0, 0)); - graph.AddDetached(new VariableReference("b"), new Vector2(600, 400)); - - foreach (Node node in graph.Engine.Nodes.ToArray()) - { - graph.Engine.UpdateNodeDimensions(node.Id, new Vector2(120, 60)); - } - - // The document's own node was seeded near the origin, so it is moved well clear of both. - graph.Engine.UpdateNodePosition(graph.Engine.Nodes[0].Id, new Vector2(-600, -400)); - - Vector2[] before = [.. graph.Engine.Nodes.Select(node => node.Position)]; - - Assert.AreEqual(0f, graph.SeparateOverlaps()); - CollectionAssert.AreEqual(before, graph.Engine.Nodes.Select(node => node.Position).ToArray()); - } - /// /// Measures how far the furthest node has moved from a remembered arrangement. /// @@ -239,6 +166,67 @@ public void FitView_CentresTheGraphOnTheOrigin() } } + /// + /// Tests that fitting a graph too big for the canvas zooms out until it fits, rather than only + /// centring the part of it that happens to be on screen. + /// + [TestMethod] + public void FitView_ZoomsOutUntilTheGraphFits() + { + AstGraphEditor editor = new(SampleFunction()) { LayoutRunning = false }; + + // A 600x400 canvas, with the origin on the middle of it. + editor.Graph.Engine.WorldOrigin = new Vector2(300, 200); + + // Spread the arrangement well past the canvas's width. + Node[] nodes = [.. editor.Graph.Engine.Nodes]; + for (int i = 0; i < nodes.Length; i++) + { + editor.Graph.Engine.UpdateNodePosition(nodes[i].Id, new Vector2(i * 400f, 0f)); + editor.Graph.Engine.UpdateNodeDimensions(nodes[i].Id, new Vector2(120f, 60f)); + } + + Assert.IsTrue(editor.FitView()); + + float extent = ((nodes.Length - 1) * 400f) + 120f; + Assert.IsTrue(editor.Zoom < 1f, $"a graph {extent} wide should not fit a 600 canvas at {editor.Zoom}"); + Assert.IsTrue(extent * editor.Zoom <= 600f, $"the graph still overflows the canvas at {editor.Zoom}"); + } + + /// + /// Tests that fitting a graph the canvas already has room for leaves it at its own size, since + /// magnifying it is not what "fit" means to anyone who asked to see all of it. + /// + [TestMethod] + public void FitView_DoesNotMagnifyAGraphThatAlreadyFits() + { + AstGraphEditor editor = new(SampleFunction()) { LayoutRunning = false, Zoom = 0.5f }; + editor.Graph.Engine.WorldOrigin = new Vector2(1000, 800); + + Node[] nodes = [.. editor.Graph.Engine.Nodes]; + for (int i = 0; i < nodes.Length; i++) + { + editor.Graph.Engine.UpdateNodePosition(nodes[i].Id, new Vector2(i * 20f, 0f)); + editor.Graph.Engine.UpdateNodeDimensions(nodes[i].Id, new Vector2(40f, 20f)); + } + + Assert.IsTrue(editor.FitView()); + Assert.AreEqual(1f, editor.Zoom, 0.0001f); + } + + /// + /// Tests that the zoom stays within the range the view offers, however it is set. + /// + [TestMethod] + public void Zoom_IsHeldWithinTheRangeTheSliderOffers() + { + AstGraphEditor editor = new(SampleFunction()) { Zoom = 50f }; + Assert.IsTrue(editor.Zoom is > 1f and <= 2f, $"{editor.Zoom} is not a zoom the view offers"); + + editor.Zoom = 0f; + Assert.IsTrue(editor.Zoom is > 0f and < 1f, $"{editor.Zoom} is not a zoom the view offers"); + } + /// /// Tests that a rebuild leaves the world origin where the caller put it, since every structural /// edit rebuilds and clearing the engine resets it. diff --git a/Directory.Packages.props b/Directory.Packages.props index c2450fb..f23a656 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -14,6 +14,8 @@ + + From a66025f5234f55538b27e82fdcbc5f468dba3d9d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 04:33:02 +0000 Subject: [PATCH 2/2] fix: reference the package WindowState actually lives in [patch] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit KTSU0006: the maximized state is Silk.NET.Windowing.Common's, not Silk.NET.Windowing's — the facade beside it. The reference was on the wrong one of the two, so the type was still being used transitively. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QwzCFb8zhd263sZeRbF7ba --- Coder.Editor/Coder.Editor.csproj | 5 +++-- Coder.Test/Coder.Test.csproj | 2 +- Directory.Packages.props | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Coder.Editor/Coder.Editor.csproj b/Coder.Editor/Coder.Editor.csproj index 0aadd15..ee8a977 100644 --- a/Coder.Editor/Coder.Editor.csproj +++ b/Coder.Editor/Coder.Editor.csproj @@ -16,8 +16,9 @@ - - + + diff --git a/Coder.Test/Coder.Test.csproj b/Coder.Test/Coder.Test.csproj index a64e1f2..2e4ca26 100644 --- a/Coder.Test/Coder.Test.csproj +++ b/Coder.Test/Coder.Test.csproj @@ -20,7 +20,7 @@ - + diff --git a/Directory.Packages.props b/Directory.Packages.props index f23a656..2ebd623 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -15,7 +15,7 @@ - +