Fix the Windows head items from the review

src/DiffEngineViewer.Windows.Tests/ImageCacheTests.cs

@@ -14,11 +14,11 @@ public async Task DecodesOnceAndKeepsIt()
         var path = Write("decoded.png", SamplePng.Build(8, 6, 200, 40, 40));
         using var cache = new ImageCache();
 
-        var first = cache.Get(path);
+        var first = cache.Get(path, null);
         await Assert.That(first).IsNotNull();
         await Assert.That(first!.Width).IsEqualTo(8);
         await Assert.That(first.Height).IsEqualTo(6);
-        await Assert.That(ReferenceEquals(cache.Get(path), first)).IsTrue();
+        await Assert.That(ReferenceEquals(cache.Get(path, null), first)).IsTrue();
     }
 
     /// <summary>
@@ -31,7 +31,7 @@ public async Task LeavesNoHandleOnTheFile()
     {
         var path = Write("copied-over.png", SamplePng.Build(8, 6, 200, 40, 40));
         using var cache = new ImageCache();
-        await Assert.That(cache.Get(path)).IsNotNull();
+        await Assert.That(cache.Get(path, null)).IsNotNull();
 
         var replacement = Write("replacement.png", SamplePng.Build(4, 4, 40, 200, 40));
         File.Copy(replacement, path, true);
@@ -46,12 +46,12 @@ public async Task RedecodesWhenTheFileChanges()
     {
         var path = Write("rewritten.png", SamplePng.Build(8, 6, 200, 40, 40));
         using var cache = new ImageCache();
-        await Assert.That(cache.Get(path)!.Width).IsEqualTo(8);
+        await Assert.That(cache.Get(path, null)!.Width).IsEqualTo(8);
 
         // A different size, so the change is visible whatever the file system's timestamp
         // resolution turns out to be.
         await File.WriteAllBytesAsync(path, SamplePng.Build(4, 4, 40, 200, 40));
-        await Assert.That(cache.Get(path)!.Width).IsEqualTo(4);
+        await Assert.That(cache.Get(path, null)!.Width).IsEqualTo(4);
     }
 
     /// <summary>
@@ -64,15 +64,15 @@ public async Task RemembersAFailure()
         var path = Write("notreally.png", "the quick brown fox"u8.ToArray());
         using var cache = new ImageCache();
 
-        await Assert.That(cache.Get(path)).IsNull();
-        await Assert.That(cache.Get(path)).IsNull();
+        await Assert.That(cache.Get(path, null)).IsNull();
+        await Assert.That(cache.Get(path, null)).IsNull();
     }
 
     [Test]
     public async Task MissingFile()
     {
         using var cache = new ImageCache();
-        await Assert.That(cache.Get(Path.Combine(Directory(), "gone.png"))).IsNull();
+        await Assert.That(cache.Get(Path.Combine(Directory(), "gone.png"), null)).IsNull();
     }
 
     static string Write(string name, byte[] content)
@@ -88,4 +88,41 @@ static string Directory()
         System.IO.Directory.CreateDirectory(path);
         return path;
     }
+
+    /// <summary>
+    /// A picture rewritten with different pixels at the same length and
+    /// the same write time, which is what a rewrite inside the file system's timestamp granularity
+    /// looks like to a stat. The model's hash sees it.
+    /// </summary>
+    [Test]
+    public async Task ARewriteWithTheSameStampKeepsTheOldPicture()
+    {
+        var path = Path.Combine(Directory(), "Same.received.png");
+        File.WriteAllBytes(path, SamplePng.Build(8, 6, 200, 40, 40));
+        var stamp = File.GetLastWriteTimeUtc(path);
+        using var cache = new ImageCache();
+        var before = ((Bitmap) cache.Get(path, FileSide.Read(path).Image!.Value.Hash)!).GetPixel(0, 0);
+        var hashBefore = FileSide.Read(path).Image!.Value.Hash;
+
+        File.WriteAllBytes(path, SamplePng.Build(8, 6, 40, 200, 40));
+        File.SetLastWriteTimeUtc(path, stamp);
+        var hashAfter = FileSide.Read(path).Image!.Value.Hash;
+        var after = ((Bitmap) cache.Get(path, hashAfter)!).GetPixel(0, 0);
+
+        // How often two writes in a row land on one stamp here, for how reachable that is.
+        var ticks = new List<long>();
+        for (var index = 0; index < 200; index++)
+        {
+            File.WriteAllBytes(path, [(byte) index]);
+            ticks.Add(File.GetLastWriteTimeUtc(path).Ticks);
+        }
+
+        var repeats = ticks.Zip(ticks.Skip(1)).Count(_ => _.First == _.Second);
+        var smallest = ticks.Zip(ticks.Skip(1)).Select(_ => _.Second - _.First).Where(_ => _ > 0).DefaultIfEmpty(0).Min();
+        File.Delete(path);
+        Console.WriteLine(
+            $"hash changed {hashBefore != hashAfter}; pixel before {before}, after {after}; " +
+            $"back to back writes on this volume: {repeats} of 199 kept the stamp, smallest step {smallest / 10}us");
+        await Assert.That(after.ToArgb()).IsNotEqualTo(before.ToArgb());
+    }
 }

src/DiffEngineViewer.Windows/FormsViewerWindow.cs

@@ -5,11 +5,15 @@
 /// Pumped rather than inverted onto <c>Application.Run</c>. ViewerProgram owns the loop for all
 /// three heads, and keeping it that way means the scroll amplification, the button lookup and the
 /// close-means-hide rule stay in one place. <c>DoEvents</c> is usually a smell, but the conditions
-/// that make it one are absent here: no modal dialogs, no nested message loops, and session state
-/// already behind its own lock.
+/// that make it one are absent here: no modal dialogs, and session state already behind its own
+/// lock. user32's own modal loops - a scroll bar thumb being dragged, the window being moved or
+/// sized - do hold the thread inside DoEvents, and <see cref="ILoopHooks"/> is how frames keep
+/// coming while they do.
 /// </para>
 /// </summary>
-sealed class FormsViewerWindow : IViewerWindow
+sealed class FormsViewerWindow :
+    IViewerWindow,
+    ILoopHooks
 {
     /// <summary>
     /// Roughly sixty frames a second, which is what the shim's SetTargetFPS gives the other heads.
@@ -75,6 +79,13 @@ public bool Present(Screen screen)
     /// </summary>
     void Wait()
     {
+        // Input already waiting is the next frame's, now: a key and a click that land together are
+        // two frames, and sleeping between them would put the second a frame behind for nothing.
+        if (form.Pending)
+        {
+            return;
+        }
+
         var timeout = form.Visible ? frameMilliseconds - 1 : hiddenMilliseconds;
         MsgWaitForMultipleObjectsEx(0, IntPtr.Zero, (uint) timeout, allInput, inputAvailable);
     }
@@ -89,6 +100,16 @@ void Wait()
     public ViewerInput Poll() =>
         form.IsDisposed ? default : form.Drain();
 
+    public Func<Screen>? Frame
+    {
+        set => form.Frame = value;
+    }
+
+    public Action? SessionEnding
+    {
+        set => form.SessionEnding = value;
+    }
+
     /// <summary>
     /// Visibility only. Assigning ShowInTaskbar recreates the window handle, and doing that under
     /// a loop that is pumping with DoEvents means tearing the handle out from under an in flight

src/DiffEngineViewer.Windows/ImageCache.cs

@@ -1,7 +1,14 @@
 /// <summary>
 /// Decoded pictures for the panes, keyed by the path the screen model handed over and invalidated
 /// by the file's write time and length — the same freshness test the queue poller uses, so a re-run
-/// that rewrites a received image refreshes the pane rather than leaving the old one up.
+/// that rewrites a received image refreshes the pane rather than leaving the old one up — and by
+/// the content hash the model carries, since a same-length rewrite inside the file system's
+/// timestamp granularity looks unchanged to a stat.
+/// <para>
+/// Only what the current screen shows is kept (<see cref="Keep"/>). Every picture ever drawn stayed
+/// decoded otherwise, for the life of a process the tray can keep hidden for days: ten accepted
+/// 400 by 300 pairs held 9 MB of unmanaged memory the collector does not see.
+/// </para>
 /// <para>
 /// A cache and not a convenience: <c>OnPaint</c> runs on every wheel notch and every resize, and
 /// decoding a picture per frame is what turns a window that is merely showing something into one
@@ -16,9 +23,20 @@ sealed class ImageCache : IDisposable
     /// A null <paramref name="Image"/> is a remembered failure. Kept rather than dropped, so a file
     /// this machine cannot decode is attempted once instead of once per frame.
     /// </summary>
-    record Entry(long WriteTicksUtc, long Length, Image? Image);
+    record Entry(long WriteTicksUtc, long Length, string? Hash, Image? Image);
+
+    /// <summary>
+    /// Drops every picture not at one of <paramref name="paths"/>, which is what is on screen.
+    /// </summary>
+    public void Keep(IReadOnlyCollection<string> paths)
+    {
+        foreach (var path in entries.Keys.Where(_ => !paths.Contains(_, StringComparer.OrdinalIgnoreCase)).ToList())
+        {
+            Forget(path);
+        }
+    }
 
-    public Image? Get(string path)
+    public Image? Get(string path, string? hash)
     {
         long ticks;
         long length;
@@ -45,7 +63,8 @@ record Entry(long WriteTicksUtc, long Length, Image? Image);
         if (entries.TryGetValue(path, out var entry))
         {
             if (entry.WriteTicksUtc == ticks &&
-                entry.Length == length)
+                entry.Length == length &&
+                entry.Hash == hash)
             {
                 return entry.Image;
             }
@@ -54,7 +73,7 @@ record Entry(long WriteTicksUtc, long Length, Image? Image);
         }
 
         var image = Load(path);
-        entries.Add(path, new(ticks, length, image));
+        entries.Add(path, new(ticks, length, hash, image));
         return image;
     }
 
@@ -65,7 +84,7 @@ record Entry(long WriteTicksUtc, long Length, Image? Image);
             // Decoded from a copy of the bytes and then copied again. GDI+ holds on to the stream
             // it was handed for as long as the image lives, and a viewer keeping a handle on the
             // received file is one that blocks the accept it exists to perform.
-            using var stream = new MemoryStream(File.ReadAllBytes(path));
+            using var stream = new MemoryStream(FileSide.ReadBytes(path));
             using var decoded = new Bitmap(stream);
             return new Bitmap(decoded);
         }

src/DiffEngineViewer.Windows/MonoFont.cs

@@ -29,6 +29,17 @@ public static Size Cell(Graphics graphics, Font font)
             Math.Max(1, (int) Math.Ceiling(font.GetHeight(graphics))));
     }
 
+    /// <summary>
+    /// Where one glyph starts after the last, unrounded. <see cref="Cell"/> is whole pixels, which
+    /// is what the grid is laid out in, but Graphics.DrawString places glyphs at this advance. At
+    /// 96 DPI that is 8.8 against a cell of 9, so anything positioned by the cell - the selection
+    /// highlight, and the column a click lands in - was a character off from about column 22 and
+    /// two by column 66, and at 175% more than three. Measured over a run rather than one glyph,
+    /// for the precision a long line needs.
+    /// </summary>
+    public static float Advance(Graphics graphics, Font font) =>
+        graphics.MeasureString(new('M', 100), font, PointF.Empty, Painter.Format).Width / 100;
+
     static FontFamily Register()
     {
         var bytes = EmbeddedFont.Bytes();

src/DiffEngineViewer.Windows/ViewerCanvas.cs

@@ -146,11 +146,28 @@ public ViewerCanvas()
     public void Draw(Screen value)
     {
         screen = value;
+        images.Keep(PicturesOn(value));
         // A new screen renumbers the rows, so a kept index would describe a different entry.
         tips.Forget(this);
         Invalidate();
     }
 
+    static List<string> PicturesOn(Screen screen)
+    {
+        var paths = new List<string>(2);
+        if (screen.Left.Image is { } left)
+        {
+            paths.Add(left.Path);
+        }
+
+        if (screen.Right.Image is { } right)
+        {
+            paths.Add(right.Path);
+        }
+
+        return paths;
+    }
+
     Size Cell
     {
         get
@@ -159,12 +176,34 @@ Size Cell
             {
                 using var graphics = CreateGraphics();
                 cell = MonoFont.Cell(graphics, font);
+                advance = MonoFont.Advance(graphics, font);
             }
 
             return cell;
         }
     }
 
+    /// <summary>
+    /// Where glyphs actually land within a line, for anything placed under or against them. The
+    /// cell stays whole pixels for laying out the grid. See <see cref="MonoFont.Advance"/>.
+    /// </summary>
+    float Advance
+    {
+        get
+        {
+            _ = Cell;
+            return advance;
+        }
+    }
+
+    float advance;
+
+    /// <summary>
+    /// The pixel offset of a column into a line of text.
+    /// </summary>
+    int Offset(int column) =>
+        (int) Math.Round(column * Advance);
+
     /// <summary>
     /// Everything drawn here is laid out in character cells, and a cell is measured in pixels from
     /// a Graphics, which is per display. Dragging the window to a display with different scaling
@@ -271,7 +310,7 @@ int ScrollTop(PaneSide side) =>
     /// pointing at is the one they mean.
     /// </summary>
     int ColumnAt(int x, PaneSide side) =>
-        Math.Max(0, (x - TextLeft(side) + Cell.Width / 2) / Cell.Width);
+        Math.Max(0, (int) Math.Floor((x - TextLeft(side)) / Advance + 0.5f));
 
     /// <summary>
     /// The body row a point is on, clamped into the body. Used while dragging, where a pointer
@@ -348,7 +387,7 @@ void DrawImage(Graphics graphics, Pane pane, int left, int width, int bodyTop, i
             return;
         }
 
-        var picture = images.Get(image.Path);
+        var picture = images.Get(image.Path, image.Hash);
         if (picture is null)
         {
             return;
@@ -427,7 +466,7 @@ void DrawTitle(Graphics graphics, int lineHeight)
             return;
         }
 
-        var width = screen.Subtitle.Length * Cell.Width;
+        var width = Offset(screen.Subtitle.Length);
         Painter.Draw(graphics, screen.Subtitle, font, Palette.Dim, Cellular(Width - padding - width, padding, width, lineHeight));
     }
 
@@ -487,9 +526,9 @@ void DrawRow(Graphics graphics, Pane pane, int index, Rectangle bounds)
                 Painter.Brush(Palette.Selection),
                 Rectangle.Intersect(
                     new(
-                        bounds.X + gutter + row.Selection.Start * Cell.Width,
+                        bounds.X + gutter + Offset(row.Selection.Start),
                         bounds.Y,
-                        row.Selection.Length * Cell.Width,
+                        Offset(row.Selection.Start + row.Selection.Length) - Offset(row.Selection.Start),
                         bounds.Height),
                     bounds));
         }
@@ -595,6 +634,14 @@ int QueueRowAt(Point point)
     protected override void OnMouseMove(MouseEventArgs e)
     {
         base.OnMouseMove(e);
+        // A move with the button up is a drag whose release went somewhere else - the button let go
+        // over another window after an Alt+Tab, say - and not one still going.
+        if ((selecting || dragging) &&
+            (e.Button & MouseButtons.Left) == 0)
+        {
+            EndDrag();
+        }
+
         if (selecting)
         {
             // Against the side the press landed in, whatever the pointer has wandered over since:
@@ -649,6 +696,26 @@ void ApplyTooltip(Point point)
     protected override void OnMouseUp(MouseEventArgs e)
     {
         base.OnMouseUp(e);
+        EndDrag();
+    }
+
+    /// <summary>
+    /// The mouse was taken away mid drag: Alt+Tab, the Windows key, a UAC prompt, another window
+    /// grabbing it. Only the window holding capture hears the button come up, so without this the
+    /// selection followed the pointer with no button held, and the splitter dragged the queue
+    /// column along, until the next click happened to land here.
+    /// </summary>
+    protected override void OnMouseCaptureChanged(EventArgs e)
+    {
+        base.OnMouseCaptureChanged(e);
+        if (!Capture)
+        {
+            EndDrag();
+        }
+    }
+
+    void EndDrag()
+    {
         if (dragging)
         {
             dragging = false;