From b7779904f67d8242dac0bc624ca746e650b4c6bc Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Wed, 14 Oct 2026 20:39:58 -0400 Subject: [PATCH] feat(desktop): add Cancel to Expand Spacing Expand Spacing ran its search on the UI thread, so a complex layout froze the app for minutes with no way out. Run the search on a worker; when it lasts past 0.3 s, show a small modal window with elapsed time and Cancel. Cancel, Escape and the close box request cancellation through the Expander token and the window stays open until the worker stops, so the layout is never touched afterward. A cancelled run moves no part and reports it in the status bar instead of the "select two parts" notice. --- .../Forms/OperationProgressFormTests.cs | 208 ++++++++++++++++++ OpenNest/Controls/PlateView.cs | 4 +- OpenNest/Controls/SelectionManager.cs | 21 +- OpenNest/Forms/MainForm.cs | 12 +- OpenNest/Forms/OperationProgressForm.cs | 194 ++++++++++++++++ 5 files changed, 433 insertions(+), 6 deletions(-) create mode 100644 OpenNest.WinForms.Tests/Forms/OperationProgressFormTests.cs create mode 100644 OpenNest/Forms/OperationProgressForm.cs diff --git a/OpenNest.WinForms.Tests/Forms/OperationProgressFormTests.cs b/OpenNest.WinForms.Tests/Forms/OperationProgressFormTests.cs new file mode 100644 index 0000000..c7529eb --- /dev/null +++ b/OpenNest.WinForms.Tests/Forms/OperationProgressFormTests.cs @@ -0,0 +1,208 @@ +using System.Diagnostics; +using System.Windows.Forms; +using OpenNest.Controls; +using OpenNest.Forms; +using OpenNest.Geometry; + +namespace OpenNest.WinForms.Tests.Forms; + +public class OperationProgressFormTests +{ + private static readonly TimeSpan Timeout = TimeSpan.FromSeconds(15); + + [Fact] + public void CancelWaitsForTheWorkerBeforeClosing() => StaTestThread.Run(() => + { + using var worker = new HeldWorker(); + using var form = new OperationProgressForm("Expand Spacing", "Working", worker.Task, worker.Source); + try + { + form.Show(); + worker.WaitForEntry(); + + // Escape clicks the same CancelButton. + Assert.NotNull(form.CancelButton); + form.CancelButton.PerformClick(); + + Assert.True(form.CancelRequested); + Assert.False(form.CancelEnabled); + Assert.StartsWith("Cancelling", form.StatusText); + PumpFor(TimeSpan.FromMilliseconds(200)); + Assert.False(form.IsDisposed, "Cancel closed the window while the worker was still running."); + Assert.True(form.Visible); + + worker.Release(); + PumpUntil(() => form.IsDisposed, "The window did not close after the worker stopped."); + Assert.True(Completed(worker.Task), "The worker did not observe cancellation."); + } + finally + { + worker.Release(); + } + }, TimeSpan.FromMinutes(1), "The STA test did not complete."); + + [Fact] + public void ClosingWhileRunningRequestsCancellationAndWaits() => StaTestThread.Run(() => + { + using var worker = new HeldWorker(); + using var form = new OperationProgressForm("Expand Spacing", "Working", worker.Task, worker.Source); + try + { + form.Show(); + worker.WaitForEntry(); + + form.Close(); + + Assert.True(form.CancelRequested); + Assert.False(form.IsDisposed, "Closing did not wait for the worker."); + Assert.True(form.Visible); + + worker.Release(); + PumpUntil(() => form.IsDisposed, "The window did not close after the worker stopped."); + } + finally + { + worker.Release(); + } + }, TimeSpan.FromMinutes(1), "The STA test did not complete."); + + [Fact] + public void RunReturnsAQuickResultAndRethrowsItsFailure() => StaTestThread.Run(() => + { + // A long show delay keeps both runs below it, so no window can open. + var delay = TimeSpan.FromSeconds(30); + + Assert.Equal(42, OperationProgressForm.Run(null, "Title", "Working", _ => 42, delay)); + + var error = Assert.Throws(() => + OperationProgressForm.Run( + null, + "Title", + "Working", + _ => throw new InvalidOperationException("search failed"), + delay + ) + ); + Assert.Equal("search failed", error.Message); + }, TimeSpan.FromMinutes(1), "The STA test did not complete."); + + [Fact] + public void CancelledExpandMovesNoPartAndReportsIt() => StaTestThread.Run(() => + { + using var view = TwoPartView(); + var before = view.Plate.Parts.Select(p => p.Location).ToArray(); + + var handled = view.ExpandSelected(work => + { + using var cancelled = new CancellationTokenSource(); + cancelled.Cancel(); + return work(cancelled.Token); + }); + + Assert.True(handled); + Assert.Equal(before, view.Plate.Parts.Select(p => p.Location).ToArray()); + Assert.Contains("cancelled", view.Status); + }, TimeSpan.FromMinutes(1), "The STA test did not complete."); + + [Fact] + public void ExpandRunsItsSearchThroughTheSuppliedRunner() => StaTestThread.Run(() => + { + using var view = TwoPartView(); + var before = view.Plate.Parts.Select(p => p.Location).ToArray(); + var runs = 0; + + var handled = view.ExpandSelected(work => + { + runs++; + return work(CancellationToken.None); + }); + + Assert.True(handled); + Assert.Equal(1, runs); + Assert.NotEqual(before, view.Plate.Parts.Select(p => p.Location).ToArray()); + Assert.StartsWith("Expanded spacing to", view.Status); + }, TimeSpan.FromMinutes(1), "The STA test did not complete."); + + private static PlateView TwoPartView() + { + var plate = new Plate(100, 100); + plate.Parts.Add(new Part(new Drawing("left", Square()), new Vector(1, 1))); + plate.Parts.Add(new Part(new Drawing("right", Square()), new Vector(12, 1))); + var view = new PlateView { Plate = plate }; + view.SelectAll(); + return view; + } + + private static CNC.Program Square() + { + var program = new CNC.Program(); + program.MoveTo(0, 0); + program.LineTo(0, 10); + program.LineTo(10, 10); + program.LineTo(10, 0); + program.LineTo(0, 0); + return program; + } + + private static void PumpFor(TimeSpan duration) + { + var watch = Stopwatch.StartNew(); + while (watch.Elapsed < duration) + { + Application.DoEvents(); + Thread.Sleep(10); + } + } + + private static void PumpUntil(Func condition, string message) + { + var watch = Stopwatch.StartNew(); + while (!condition()) + { + Assert.True(watch.Elapsed < Timeout, message); + Application.DoEvents(); + Thread.Sleep(10); + } + } + + private static bool Completed(Task task) + { + Assert.True(task.Wait(Timeout), "The worker did not finish."); + return task.Result; + } + + /// A worker held until released; its result says whether it saw cancellation. + private sealed class HeldWorker : IDisposable + { + private readonly ManualResetEventSlim entered = new(); + private readonly ManualResetEventSlim release = new(); + + public HeldWorker() + { + var token = Source.Token; + Task = System.Threading.Tasks.Task.Run(() => + { + entered.Set(); + release.Wait(Timeout); + return token.IsCancellationRequested; + }); + } + + public CancellationTokenSource Source { get; } = new(); + + public Task Task { get; } + + public void WaitForEntry() => Assert.True(entered.Wait(Timeout), "Worker did not start."); + + public void Release() => release.Set(); + + public void Dispose() + { + release.Set(); + Task.Wait(Timeout); + Source.Dispose(); + entered.Dispose(); + release.Dispose(); + } + } +} diff --git a/OpenNest/Controls/PlateView.cs b/OpenNest/Controls/PlateView.cs index 6dfc282..c137b7c 100644 --- a/OpenNest/Controls/PlateView.cs +++ b/OpenNest/Controls/PlateView.cs @@ -1048,7 +1048,9 @@ namespace OpenNest.Controls public void PushSelected(PushDirection direction) => selection.PushSelected(direction); - public bool ExpandSelected() => selection.ExpandSelected(); + public bool ExpandSelected( + Func, Expander.Result> run = null + ) => selection.ExpandSelected(run); public void RotateSelectedParts(double angle) => selection.RotateSelectedParts(angle); diff --git a/OpenNest/Controls/SelectionManager.cs b/OpenNest/Controls/SelectionManager.cs index 6384f2d..4024a55 100644 --- a/OpenNest/Controls/SelectionManager.cs +++ b/OpenNest/Controls/SelectionManager.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; using System.Drawing; using System.Linq; +using System.Threading; using OpenNest.CNC; using OpenNest.Engine.Fill; using OpenNest.Geometry; @@ -167,19 +168,31 @@ namespace OpenNest.Controls /// /// Grows the part-to-part spacing of the selection as far as the plate - /// allows (Expander). Returns false when nothing could be done. + /// allows (Expander). executes the search and may + /// cancel it through its token (the desktop shows a Cancel window); by + /// default it runs synchronously. A cancelled run moves no part. Returns + /// false only when fewer than two parts are selected. /// - public bool ExpandSelected() + public bool ExpandSelected( + Func, Expander.Result> run = null + ) { var movingParts = selectedParts.Select(p => p.BasePart).ToList(); if (movingParts.Count < 2) return false; - var result = Expander.Expand(movingParts, view.Plate); + var plate = view.Plate; + Expander.Result Work(CancellationToken token) => + Expander.Expand(movingParts, plate, null, token); + + var result = run != null ? run(Work) : Work(CancellationToken.None); if (result.Cancelled) - return false; + { + view.Status = "Expand Spacing cancelled - no parts were moved"; + return true; + } selectedParts.ForEach(p => p.IsDirty = true); diff --git a/OpenNest/Forms/MainForm.cs b/OpenNest/Forms/MainForm.cs index de1c7dc..4aa5a0d 100644 --- a/OpenNest/Forms/MainForm.cs +++ b/OpenNest/Forms/MainForm.cs @@ -1302,7 +1302,17 @@ namespace OpenNest.Forms if (activeForm == null) return; - if (!activeForm.PlateView.ExpandSelected()) + // Complex layouts can search for minutes; run behind a Cancel window. + var expanded = activeForm.PlateView.ExpandSelected(work => + OperationProgressForm.Run( + this, + "Expand Spacing", + "Expanding spacing between the selected parts...", + work + ) + ); + + if (!expanded) MessageBox.Show( "Select at least two parts on the plate to expand.", "Expand Spacing", diff --git a/OpenNest/Forms/OperationProgressForm.cs b/OpenNest/Forms/OperationProgressForm.cs new file mode 100644 index 0000000..a2d138c --- /dev/null +++ b/OpenNest/Forms/OperationProgressForm.cs @@ -0,0 +1,194 @@ +using System; +using System.Diagnostics; +using System.Drawing; +using System.Threading; +using System.Threading.Tasks; +using System.Windows.Forms; + +namespace OpenNest.Forms; + +/// +/// Small "working" window with a Cancel button for a long plate operation that +/// runs on a worker thread. Cancel, Escape and the close box only request +/// cancellation: the window stays open showing "Cancelling..." until the worker +/// has actually stopped, so the caller never continues while it is still running. +/// +internal sealed class OperationProgressForm : Form +{ + /// Runs that finish within this delay never show the window. + internal static readonly TimeSpan DefaultShowDelay = TimeSpan.FromMilliseconds(300); + + private readonly Task task; + private readonly CancellationTokenSource cts; + private readonly SynchronizationContext uiContext; + private readonly Stopwatch stopwatch = Stopwatch.StartNew(); + private readonly Label statusLabel; + private readonly Button cancelButton; + private readonly System.Windows.Forms.Timer elapsedTimer; + private bool finished; + + internal OperationProgressForm(string title, string message, Task task, CancellationTokenSource cts) + { + this.task = task ?? throw new ArgumentNullException(nameof(task)); + this.cts = cts ?? throw new ArgumentNullException(nameof(cts)); + + Text = title; + StartPosition = FormStartPosition.CenterParent; + ClientSize = new Size(380, 128); + FormBorderStyle = FormBorderStyle.FixedDialog; + MinimizeBox = false; + MaximizeBox = false; + ShowInTaskbar = false; + + var messageLabel = new Label + { + Text = message, + AutoEllipsis = true, + Location = new Point(12, 12), + Size = new Size(356, 20), + }; + var bar = new ProgressBar + { + Style = ProgressBarStyle.Marquee, + MarqueeAnimationSpeed = 30, + Location = new Point(12, 38), + Size = new Size(356, 18), + }; + statusLabel = new Label + { + Location = new Point(12, 64), + Size = new Size(356, 20), + }; + cancelButton = new Button + { + Text = "Cancel", + Location = new Point(293, 92), + Size = new Size(75, 26), + }; + cancelButton.Click += (_, _) => RequestCancel(); + + Controls.Add(messageLabel); + Controls.Add(bar); + Controls.Add(statusLabel); + Controls.Add(cancelButton); + + // Escape clicks Cancel. Assigning CancelButton gives the button + // DialogResult.Cancel, which would close the window at once, so clear it: + // closing waits for the worker. + CancelButton = cancelButton; + cancelButton.DialogResult = DialogResult.None; + + elapsedTimer = new System.Windows.Forms.Timer { Interval = 500 }; + elapsedTimer.Tick += (_, _) => UpdateStatus(); + UpdateStatus(); + + uiContext = SynchronizationContext.Current ?? new WindowsFormsSynchronizationContext(); + } + + internal bool CancelRequested => cts.IsCancellationRequested; + + internal string StatusText => statusLabel.Text; + + internal bool CancelEnabled => cancelButton.Enabled; + + /// + /// Runs on a worker thread and returns its result, + /// rethrowing its exception. A run still going after + /// shows a modal window whose Cancel requests + /// cancellation through the token passed to ; this + /// returns only once the worker has stopped, whether or not it honored it. + /// + internal static T Run( + IWin32Window owner, + string title, + string message, + Func work, + TimeSpan? showDelay = null + ) + { + ArgumentNullException.ThrowIfNull(work); + + using var cts = new CancellationTokenSource(); + var token = cts.Token; + var task = Task.Run(() => work(token)); + + // A wait handle, unlike Task.Wait, does not throw for a faulted run. + if (!((IAsyncResult)task).AsyncWaitHandle.WaitOne(showDelay ?? DefaultShowDelay)) + { + using var form = new OperationProgressForm(title, message, task, cts); + form.ShowDialog(owner); + } + + // The window closes only after the worker stops. Should it ever return + // early, stop the worker rather than continue beside it. + if (!task.IsCompleted) + cts.Cancel(); + + return task.GetAwaiter().GetResult(); + } + + protected override void OnShown(EventArgs e) + { + base.OnShown(e); + elapsedTimer.Start(); + task.ContinueWith( + _ => uiContext.Post(_ => Finish(), null), + CancellationToken.None, + TaskContinuationOptions.None, + TaskScheduler.Default + ); + } + + protected override void OnFormClosing(FormClosingEventArgs e) + { + if (!finished) + { + e.Cancel = true; + RequestCancel(); + } + + base.OnFormClosing(e); + } + + protected override void Dispose(bool disposing) + { + if (disposing) + { + // The caller owns the token source; only ask a still-running worker to stop. + if (!finished && !task.IsCompleted) + cts.Cancel(); + elapsedTimer.Dispose(); + } + + base.Dispose(disposing); + } + + private void RequestCancel() + { + if (finished || cts.IsCancellationRequested) + return; + + cts.Cancel(); + cancelButton.Enabled = false; + UpdateStatus(); + } + + private void Finish() + { + if (finished || IsDisposed) + return; + + finished = true; + elapsedTimer.Stop(); + Close(); + } + + private void UpdateStatus() + { + var elapsed = stopwatch.Elapsed; + var time = $"{(int)elapsed.TotalMinutes}:{elapsed.Seconds:00}"; + statusLabel.Text = cts.IsCancellationRequested + ? $"Cancelling... waiting for the current step to stop ({time})" + : $"Elapsed {time}"; + } +}