From dbd3a8b59232c7d801ae51eac9937689851051a2 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Sun, 23 Aug 2026 17:50:54 +1000 Subject: [PATCH 1/2] Repaint when the picture changed and nothing else did The Windows head repaints only when the new screen differs from the last one, because ScreenBuilder allocates a fresh Screen every frame and record equality would repaint sixty times a second. That comparison never looked at the image pane, and an image pane said only where the file was and how big it was. An image side's rows are format, dimensions and byte count, so a re-run that rewrites a received image at the same size changes nothing in the model - for BMP, being uncompressed, that is every re-run. Apply returned before invalidating, the canvas never repainted, and the pane kept the previous picture. The stamp based freshness check in ImageCache was never even consulted, since nothing asked it to paint. ImagePane carries the content hash the model already had, and the pane comparison includes the image. The hash is not for drawing - a head reads the file - but for telling one picture from another at the same path and size. --- .../PaneChangeTests.cs | 56 +++++++++++++++++++ src/DiffEngineViewer.Windows/ViewerForm.cs | 7 ++- src/DiffEngineViewer/Model/ImagePane.cs | 8 ++- src/DiffEngineViewer/ScreenBuilder.cs | 2 +- 4 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 src/DiffEngineViewer.Windows.Tests/PaneChangeTests.cs diff --git a/src/DiffEngineViewer.Windows.Tests/PaneChangeTests.cs b/src/DiffEngineViewer.Windows.Tests/PaneChangeTests.cs new file mode 100644 index 00000000..1c92b3f5 --- /dev/null +++ b/src/DiffEngineViewer.Windows.Tests/PaneChangeTests.cs @@ -0,0 +1,56 @@ +/// +/// Whether two panes are the same pane, which is what decides whether the window repaints. +/// ScreenBuilder allocates a fresh Screen every frame, so record equality would report a change +/// sixty times a second and the comparison is by hand. +/// +public class PaneChangeTests +{ + /// + /// The rows an image side shows are format, dimensions and byte count, and a re-run that + /// rewrites a received image at the same size changes none of them - for BMP, which is + /// uncompressed, that is every re-run. So nothing about the screen differed, Apply returned + /// before repainting, and the pane kept the previous picture while the rows beside it + /// described the new one. + /// + [Test] + public async Task A_picture_that_changed_is_not_the_same_pane() + { + var before = ImagePane("A1B2"); + var after = ImagePane("C3D4"); + + await Assert.That(ViewerForm.Same(before, after)).IsFalse(); + } + + [Test] + public async Task A_picture_that_did_not_change_is_the_same_pane() => + await Assert.That(ViewerForm.Same(ImagePane("A1B2"), ImagePane("A1B2"))).IsTrue(); + + [Test] + public async Task A_picture_replaced_by_one_of_another_size_is_not_the_same_pane() + { + var before = ImagePane("A1B2"); + var after = before with + { + Image = new("sample.received.png", 20, 10, "A1B2") + }; + + await Assert.That(ViewerForm.Same(before, after)).IsFalse(); + } + + [Test] + public async Task Text_panes_are_unaffected() + { + var pane = new Pane("received", [new(1, RowKind.Unchanged, "one")], 0, 1); + + await Assert.That(ViewerForm.Same(pane, pane with { })).IsTrue(); + await Assert.That(ViewerForm.Same(pane, pane with { ScrollTop = 1 })).IsFalse(); + } + + static Pane ImagePane(string hash) => + new( + "received", + [], + 0, + 0, + new("sample.received.png", 10, 10, hash)); +} diff --git a/src/DiffEngineViewer.Windows/ViewerForm.cs b/src/DiffEngineViewer.Windows/ViewerForm.cs index fe3a3456..458e90ef 100644 --- a/src/DiffEngineViewer.Windows/ViewerForm.cs +++ b/src/DiffEngineViewer.Windows/ViewerForm.cs @@ -422,9 +422,14 @@ left is null left.Row == right.Row && left.Labels.SequenceEqual(right.Labels); - static bool Same(Pane left, Pane right) => + internal static bool Same(Pane left, Pane right) => left.Header == right.Header && left.ScrollTop == right.ScrollTop && left.TotalRows == right.TotalRows && + // Records all the way down, so this compares the path, the size and the content stamp. A + // re-run that rewrites a received image at the same size changes nothing else about the + // screen - the rows say format, dimensions and byte count, and for BMP those hold - so + // without it Apply returned before repainting and the pane kept the previous picture. + left.Image == right.Image && left.Rows.SequenceEqual(right.Rows); } diff --git a/src/DiffEngineViewer/Model/ImagePane.cs b/src/DiffEngineViewer/Model/ImagePane.cs index 65a2e68f..442ab821 100644 --- a/src/DiffEngineViewer/Model/ImagePane.cs +++ b/src/DiffEngineViewer/Model/ImagePane.cs @@ -13,4 +13,10 @@ /// worth trying. /// /// -record ImagePane(string Path, int Width, int Height); +/// +/// What the file held when the model was built, so a head can tell one picture from another at the +/// same path and size. Not for drawing - the head reads the file - but for deciding whether what +/// is on screen is still this. A re-run that rewrites a received image at the same dimensions +/// changes nothing else in the model, and for BMP that is every re-run. +/// +record ImagePane(string Path, int Width, int Height, string? Hash); diff --git a/src/DiffEngineViewer/ScreenBuilder.cs b/src/DiffEngineViewer/ScreenBuilder.cs index a77cb81f..8ac43630 100644 --- a/src/DiffEngineViewer/ScreenBuilder.cs +++ b/src/DiffEngineViewer/ScreenBuilder.cs @@ -95,7 +95,7 @@ static Pane BuildPane( return null; } - return new(file.Path, header.Width, header.Height); + return new(file.Path, header.Width, header.Height, file.Hash); } static IReadOnlyList BuildQueue(SessionState state, int body, out int top) From 35268176a700f61e33223a016ca695fe0d236092 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Sun, 23 Aug 2026 18:10:31 +1000 Subject: [PATCH 2/2] Say the hash in the pane the test asserts on --- src/DiffEngineViewer.Tests/ImageScreenTests.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/DiffEngineViewer.Tests/ImageScreenTests.cs b/src/DiffEngineViewer.Tests/ImageScreenTests.cs index a5b7a82f..8e944c3d 100644 --- a/src/DiffEngineViewer.Tests/ImageScreenTests.cs +++ b/src/DiffEngineViewer.Tests/ImageScreenTests.cs @@ -99,8 +99,10 @@ public Task DeleteInQueue() => public async Task PanesCarryThePicture() { var screen = ScreenBuilder.Build(State(Received(), Expected())); - await Assert.That(screen.Left.Image).IsEqualTo(new("temp/sample.received.png", 800, 600)); - await Assert.That(screen.Right.Image).IsEqualTo(new("code/sample.verified.png", 800, 600)); + // The hash the side carried, which is how a head tells one picture from another at the + // same path and size - and how the Windows head knows to repaint for it + await Assert.That(screen.Left.Image).IsEqualTo(new("temp/sample.received.png", 800, 600, "0A")); + await Assert.That(screen.Right.Image).IsEqualTo(new("code/sample.verified.png", 800, 600, "0A")); } [Test]