diff --git a/src/DiffEngineViewer.Windows.Tests/FormsHeadTests.cs b/src/DiffEngineViewer.Windows.Tests/FormsHeadTests.cs index 80068532..0c35a84b 100644 --- a/src/DiffEngineViewer.Windows.Tests/FormsHeadTests.cs +++ b/src/DiffEngineViewer.Windows.Tests/FormsHeadTests.cs @@ -119,6 +119,57 @@ public async Task EveryPictureEverDrawnStaysDecoded() await Assert.That(count).IsLessThanOrEqualTo(2); } + /// + /// Through the real canvas: a pair of pictures painted twice at one size is composed once, and + /// the second paint copies it. Both paints show the pictures. + /// + [Test] + public async Task RepaintingAPictureComposesItOnce() + { + using var host = new CanvasHost(); + var screen = ScreenBuilder.Build(ViewerSession.Resize(Fixtures.Images(), columns, rows)); + host.Canvas.Draw(screen); + host.Canvas.LoadPictures(); + + var first = host.Draw(screen); + var second = host.Draw(screen); + + await Assert.That(host.Canvas.Composed()).IsEqualTo(2); + // The left picture's colour, which only a drawn picture puts on the canvas + var red = Bounds(second, _ => _.R == 198 && _.G == 64 && _.B == 64); + await Assert.That(red).IsNotNull(); + await Assert.That(Bounds(first, _ => _.R == 198 && _.G == 64 && _.B == 64)).IsEqualTo(red); + } + + /// + /// The footer's buttons are pooled and relabelled as the screen changes, and each is sized to + /// the label it has now. At WinForms' default, GrowOnly, a button stayed as wide as the longest + /// label it had ever held, so the footer was the history of the session - and the pixel + /// baselines the history of the test run, which moved whenever a test was added. + /// + [Test] + public async Task AFooterButtonIsSizedToItsCurrentLabel() + { + using var host = new FormHost(Fixtures.File(Fixtures.Long(true), Fixtures.Long(false))); + host.Frame(); + var button = FooterButtons(host.Form)[4]; + var before = (button.Text, button.Width); + + host.Form.Apply(ScreenBuilder.Build(ViewerSession.Apply(host.State, CommandKind.ToggleMinimal))); + var after = (button.Text, button.Width); + + Console.WriteLine($"{before} then {after}"); + await Assert.That(before.Text).IsEqualTo("Changes only"); + await Assert.That(after.Text).IsEqualTo("All lines"); + await Assert.That(after.Width).IsLessThan(before.Width); + await Assert.That(after.Width).IsGreaterThanOrEqualTo(button.MinimumSize.Width); + } + + static List FooterButtons(ViewerForm form) => + ((IList) typeof(ViewerForm).GetField("pool", BindingFlags.Instance | BindingFlags.NonPublic)!.GetValue(form)!) + .Cast() + .ToList(); + /// /// The default 1100 by 700 window at the common scales, through the canvas's own layout /// code with the cell MonoFont measures at that scale and the chrome the form takes there: the @@ -918,6 +969,9 @@ public static int BodyTop(this ViewerCanvas canvas) => public static (int Left, int Half, int Width) Panes(this ViewerCanvas canvas) => ((int, int, int)) typeof(ViewerCanvas).GetMethod("Panes", flags)!.Invoke(canvas, null)!; + public static int Composed(this ViewerCanvas canvas) => + ((ImageCache) typeof(ViewerCanvas).GetField("images", flags)!.GetValue(canvas)!).Composed; + public static (int Count, long Bytes) CachedImages(this ViewerCanvas canvas) { var cache = typeof(ViewerCanvas).GetField("images", flags)!.GetValue(canvas)!; diff --git a/src/DiffEngineViewer.Windows.Tests/ImageCacheTests.cs b/src/DiffEngineViewer.Windows.Tests/ImageCacheTests.cs index 843177f6..b8f28c72 100644 --- a/src/DiffEngineViewer.Windows.Tests/ImageCacheTests.cs +++ b/src/DiffEngineViewer.Windows.Tests/ImageCacheTests.cs @@ -1,3 +1,5 @@ +using System.Collections.Concurrent; + /// /// The decode the WinForms head puts under an image pane's rows. /// @@ -68,6 +70,71 @@ public async Task RemembersAFailure() await Assert.That(cache.Get(path, null)).IsNull(); } + /// + /// The window's decode runs on the pool and comes back through the post the window gives the + /// cache, which is BeginInvoke there and a queue here. Until it does, the pane has no picture + /// and the window is free to paint its rows. + /// + [Test] + public async Task ADecodeWithSomewhereToPostItIsHandedBack() + { + var path = Write("posted.png", SamplePng.Build(8, 6, 200, 40, 40)); + using var posted = new BlockingCollection(); + using var cache = new ImageCache(posted.Add); + var loaded = 0; + + await Assert.That(cache.Get(path, null, () => loaded++)).IsNull(); + await Assert.That(posted.TryTake(out var handBack, TimeSpan.FromSeconds(10))).IsTrue(); + handBack!(); + + await Assert.That(loaded).IsEqualTo(1); + await Assert.That(cache.Get(path, null, () => loaded++)!.Width).IsEqualTo(8); + } + + /// + /// A decode that finishes after its picture left the screen is thrown away rather than cached, + /// or navigating quickly through a queue of pictures would hold every one of them. + /// + [Test] + public async Task ADecodeForAPictureNoLongerOnScreenIsDropped() + { + var path = Write("left-behind.png", SamplePng.Build(8, 6, 200, 40, 40)); + using var posted = new BlockingCollection(); + using var cache = new ImageCache(posted.Add); + var loaded = 0; + cache.Keep([path]); + + cache.Get(path, null, () => loaded++); + await Assert.That(posted.TryTake(out var handBack, TimeSpan.FromSeconds(10))).IsTrue(); + cache.Keep([]); + handBack!(); + + await Assert.That(loaded).IsEqualTo(0); + await Assert.That(cache.Composite(path, new(8, 6), (_, size) => new(size.Width, size.Height))).IsNull(); + } + + /// + /// A pane paints the picture over its checkerboard, scaled, once per size, and copies that on + /// every paint after. Scaling it on every paint cost 46 to 66 ms a paint for a pair of 2000 by + /// 1500 pictures, on every wheel notch. + /// + [Test] + public async Task APictureIsComposedOncePerSize() + { + var path = Write("composed.png", SamplePng.Build(8, 6, 200, 40, 40)); + using var cache = new ImageCache(); + await Assert.That(cache.Get(path, null)).IsNotNull(); + Func build = (_, size) => new(size.Width, size.Height); + + var first = cache.Composite(path, new(4, 3), build); + var again = cache.Composite(path, new(4, 3), build); + var resized = cache.Composite(path, new(6, 4), build); + + await Assert.That(ReferenceEquals(first, again)).IsTrue(); + await Assert.That(resized!.Size).IsEqualTo(new Size(6, 4)); + await Assert.That(cache.Composed).IsEqualTo(2); + } + [Test] public async Task MissingFile() { diff --git a/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.GroupedConflictedQueue.verified.png b/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.GroupedConflictedQueue.verified.png index c7719a24..5d70f45f 100644 Binary files a/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.GroupedConflictedQueue.verified.png and b/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.GroupedConflictedQueue.verified.png differ diff --git a/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.InlineAccepted.verified.png b/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.InlineAccepted.verified.png index 488850f9..93cc6daf 100644 Binary files a/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.InlineAccepted.verified.png and b/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.InlineAccepted.verified.png differ diff --git a/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.InlineQueue.verified.png b/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.InlineQueue.verified.png index a9aaff11..5dec36bc 100644 Binary files a/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.InlineQueue.verified.png and b/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.InlineQueue.verified.png differ diff --git a/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.InlineSingle.verified.png b/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.InlineSingle.verified.png index 8bf1cdac..cb255537 100644 Binary files a/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.InlineSingle.verified.png and b/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.InlineSingle.verified.png differ diff --git a/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.LongQueueLabel.verified.png b/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.LongQueueLabel.verified.png index a2fdd29b..15d7904c 100644 Binary files a/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.LongQueueLabel.verified.png and b/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.LongQueueLabel.verified.png differ diff --git a/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.Minimal.verified.png b/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.Minimal.verified.png index 1f4642f2..5b5baac7 100644 Binary files a/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.Minimal.verified.png and b/src/DiffEngineViewer.Windows.Tests/WindowsPixelTests.Minimal.verified.png differ diff --git a/src/DiffEngineViewer.Windows/FormsViewerWindow.cs b/src/DiffEngineViewer.Windows/FormsViewerWindow.cs index cfefab9b..1d082b24 100644 --- a/src/DiffEngineViewer.Windows/FormsViewerWindow.cs +++ b/src/DiffEngineViewer.Windows/FormsViewerWindow.cs @@ -172,6 +172,7 @@ public bool Capture(Screen screen, int width, int height, string pngPath) { form.ClientSize = new(width, height); form.Apply(screen); + form.LoadPictures(); form.PerformLayout(); // Invalidate only marks dirty; the paint has to have happened before the bitmap. form.Surface.Refresh(); diff --git a/src/DiffEngineViewer.Windows/ImageCache.cs b/src/DiffEngineViewer.Windows/ImageCache.cs index 7fd2324b..41401929 100644 --- a/src/DiffEngineViewer.Windows/ImageCache.cs +++ b/src/DiffEngineViewer.Windows/ImageCache.cs @@ -10,71 +10,229 @@ /// 400 by 300 pairs held 9 MB of unmanaged memory the collector does not see. /// /// -/// A cache and not a convenience: OnPaint 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 -/// that is busy. +/// A cache and not a convenience: OnPaint runs on every wheel notch and every resize. Two +/// things are held for that. The decoded picture, which is decoded off the UI thread when there is +/// somewhere to post the result (), because decoding a +/// pair of 2000 by 1500 pictures held the window for over 100 ms. And the picture as painted, at +/// the size it was painted (): scaling it and drawing the checkerboard under +/// it on every paint cost 46 to 66 ms a paint for that pair, where copying the result costs +/// almost nothing. +/// +/// +/// Everything here is touched only from the UI thread. A background decode hands its result back +/// through post, which is the canvas's BeginInvoke. /// /// -sealed class ImageCache : IDisposable +sealed class ImageCache(Action? post = null) : IDisposable { readonly Dictionary entries = new(StringComparer.OrdinalIgnoreCase); + /// + /// Decodes started and not yet handed back, by path, with the stamp each was started for. + /// + readonly Dictionary pending = new(StringComparer.OrdinalIgnoreCase); + + /// + /// The paths the last named. A decode that finishes for a path no longer on + /// screen is thrown away rather than cached. Null until the first Keep, which takes everything. + /// + HashSet? wanted; + + bool disposed; + + record struct Stamp(long WriteTicksUtc, long Length, string? Hash); + /// /// A null is a remembered failure. Kept rather than dropped, so a file /// this machine cannot decode is attempted once instead of once per frame. /// - record Entry(long WriteTicksUtc, long Length, string? Hash, Image? Image); + sealed record Entry(Stamp Stamp, Image? Image) + { + /// + /// The picture as last painted, at the size it was painted: see . + /// + public Bitmap? Composite { get; set; } + + public void Dispose() + { + Image?.Dispose(); + Composite?.Dispose(); + } + } /// /// Drops every picture not at one of , which is what is on screen. /// public void Keep(IReadOnlyCollection paths) { - foreach (var path in entries.Keys.Where(_ => !paths.Contains(_, StringComparer.OrdinalIgnoreCase)).ToList()) + wanted = new(paths, StringComparer.OrdinalIgnoreCase); + foreach (var path in entries.Keys.Where(_ => !wanted.Contains(_)).ToList()) { Forget(path); } } + /// + /// The picture at , decoded here and now if it is not cached. For a + /// caller that has to have it this frame, such as a capture. + /// public Image? Get(string path, string? hash) { - long ticks; - long length; + if (!TryStamp(path, hash, out var stamp)) + { + return null; + } + + if (TryCached(path, stamp, out var cached)) + { + return cached; + } + + var image = Load(path); + // Supersedes a background decode of the same file, whose result would only replace this + pending.Remove(path); + entries.Add(path, new(stamp, image)); + return image; + } + + /// + /// The picture at if it is decoded, and otherwise null while it is + /// decoded on the pool, after which is called on the UI thread. With + /// nowhere to post the result this decodes here and now, as + /// does. + /// + public Image? Get(string path, string? hash, Action loaded) + { + if (post is null) + { + return Get(path, hash); + } + + if (!TryStamp(path, hash, out var stamp)) + { + return null; + } + + if (TryCached(path, stamp, out var cached)) + { + return cached; + } + + if (pending.TryGetValue(path, out var started) && + started == stamp) + { + return null; + } + + pending[path] = stamp; + Task.Run(() => Load(path)) + .ContinueWith( + task => + { + var image = task.Result; + try + { + post(() => Loaded(path, stamp, image, loaded)); + } + catch (InvalidOperationException) + { + // The window went away before the decode finished, and with it the thread + // this would have been handed to + image?.Dispose(); + } + }, + CancellationToken.None, + TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); + return null; + } + + void Loaded(string path, Stamp stamp, Image? image, Action loaded) + { + // Only the decode the path is still waiting on. An older one, for a stamp the file has + // since moved past, would put back the picture the newer decode is replacing + if (disposed || + !pending.TryGetValue(path, out var started) || + started != stamp || + (wanted is not null && !wanted.Contains(path))) + { + image?.Dispose(); + return; + } + + pending.Remove(path); + Forget(path); + entries.Add(path, new(stamp, image)); + loaded(); + } + + /// + /// The picture at as it is painted at , built + /// by from the decoded picture the first time that size is asked for + /// and kept until another size is, or the picture goes. Null when the picture is not decoded. + /// + public Bitmap? Composite(string path, Size size, Func build) + { + if (!entries.TryGetValue(path, out var entry) || + entry.Image is null) + { + return null; + } + + if (entry.Composite is { } composite && + composite.Size == size) + { + return composite; + } + + entry.Composite?.Dispose(); + entry.Composite = build(entry.Image, size); + Composed++; + return entry.Composite; + } + + /// + /// How many composites have been built, for the tests that pin how often that happens. + /// + internal int Composed { get; private set; } + + bool TryStamp(string path, string? hash, out Stamp stamp) + { try { var info = new FileInfo(path); - if (!info.Exists) + if (info.Exists) { - Forget(path); - return null; + stamp = new(info.LastWriteTimeUtc.Ticks, info.Length, hash); + return true; } - - ticks = info.LastWriteTimeUtc.Ticks; - length = info.Length; } catch { // A file that cannot be stat'd cannot be drawn, and the rows have already said what // the model made of it. - Forget(path); - return null; } + Forget(path); + stamp = default; + return false; + } + + bool TryCached(string path, Stamp stamp, out Image? image) + { if (entries.TryGetValue(path, out var entry)) { - if (entry.WriteTicksUtc == ticks && - entry.Length == length && - entry.Hash == hash) + if (entry.Stamp == stamp) { - return entry.Image; + image = entry.Image; + return true; } Forget(path); } - var image = Load(path); - entries.Add(path, new(ticks, length, hash, image)); - return image; + image = null; + return false; } static Image? Load(string path) @@ -98,17 +256,19 @@ void Forget(string path) { if (entries.Remove(path, out var entry)) { - entry.Image?.Dispose(); + entry.Dispose(); } } public void Dispose() { + disposed = true; foreach (var entry in entries.Values) { - entry.Image?.Dispose(); + entry.Dispose(); } entries.Clear(); + pending.Clear(); } } diff --git a/src/DiffEngineViewer.Windows/ViewerCanvas.cs b/src/DiffEngineViewer.Windows/ViewerCanvas.cs index e4a4d307..bfdf0c12 100644 --- a/src/DiffEngineViewer.Windows/ViewerCanvas.cs +++ b/src/DiffEngineViewer.Windows/ViewerCanvas.cs @@ -54,7 +54,7 @@ sealed class ViewerCanvas : Control readonly QueueTips tips = new(); - readonly ImageCache images = new(); + readonly ImageCache images; Screen? screen; @@ -105,8 +105,16 @@ public ViewerCanvas() ControlStyles.ResizeRedraw, true); BackColor = Palette.Background; + images = new(Post); } + /// + /// Hands a finished background decode back to this thread. Throws when the handle has gone, + /// which the cache takes as the window having closed under the decode. + /// + void Post(Action action) => + BeginInvoke(action); + public event Action? QueueItemClicked; /// @@ -147,11 +155,48 @@ public void Draw(Screen value) { screen = value; images.Keep(PicturesOn(value)); + // Started now rather than at the first paint, which is at least a pump away. The pane + // draws its rows meanwhile, and the picture under them once it is decoded + foreach (var image in ImagesOn(value)) + { + images.Get(image.Path, image.Hash, Invalidate); + } + // A new screen renumbers the rows, so a kept index would describe a different entry. tips.Forget(this); Invalidate(); } + /// + /// Decodes whatever the current screen shows, here and now. For a capture, which draws one frame + /// and has no later paint for a background decode to arrive in time for. + /// + public void LoadPictures() + { + if (screen is null) + { + return; + } + + foreach (var image in ImagesOn(screen)) + { + images.Get(image.Path, image.Hash); + } + } + + static IEnumerable ImagesOn(Screen screen) + { + if (screen.Left.Image is { } left) + { + yield return left; + } + + if (screen.Right.Image is { } right) + { + yield return right; + } + } + static List PicturesOn(Screen screen) { var paths = new List(2); @@ -387,8 +432,7 @@ void DrawImage(Graphics graphics, Pane pane, int left, int width, int bodyTop, i return; } - var picture = images.Get(image.Path, image.Hash); - if (picture is null) + if (images.Get(image.Path, image.Hash, Invalidate) is null) { return; } @@ -421,13 +465,19 @@ void DrawImage(Graphics graphics, Pane pane, int left, int width, int bodyTop, i drawn.Width, drawn.Height); - DrawChecker(graphics, bounds); + // Copied rather than drawn: the checkerboard and the scaled picture are composed once per + // picture and size, and every paint after that is a copy of the result + var composite = images.Composite(image.Path, drawn, Compose); + if (composite is null) + { + return; + } var interpolation = graphics.InterpolationMode; var offset = graphics.PixelOffsetMode; - graphics.InterpolationMode = InterpolationMode.HighQualityBicubic; - graphics.PixelOffsetMode = PixelOffsetMode.HighQuality; - graphics.DrawImage(picture, bounds); + graphics.InterpolationMode = InterpolationMode.NearestNeighbor; + graphics.PixelOffsetMode = PixelOffsetMode.Half; + graphics.DrawImage(composite, bounds); // Put back, because the text drawing this shares a Graphics with is set up once by Painter // and would otherwise inherit whichever picture was drawn last. graphics.InterpolationMode = interpolation; @@ -438,6 +488,23 @@ void DrawImage(Graphics graphics, Pane pane, int left, int width, int bodyTop, i graphics.DrawRectangle(pen, bounds.X - 1, bounds.Y - 1, bounds.Width + 1, bounds.Height + 1); } + /// + /// The picture as a pane shows it at : over the checkerboard, so an image + /// with transparency reads as one, and scaled with the high quality filter a downscale needs. + /// Premultiplied, which is what the double buffer it is copied into holds. + /// + static Bitmap Compose(Image picture, Size size) + { + var composite = new Bitmap(size.Width, size.Height, PixelFormat.Format32bppPArgb); + using var graphics = Graphics.FromImage(composite); + var bounds = new Rectangle(Point.Empty, size); + DrawChecker(graphics, bounds); + graphics.InterpolationMode = InterpolationMode.HighQualityBicubic; + graphics.PixelOffsetMode = PixelOffsetMode.HighQuality; + graphics.DrawImage(picture, bounds); + return composite; + } + static void DrawChecker(Graphics graphics, Rectangle bounds) { graphics.FillRectangle(Painter.Brush(Palette.CheckerLight), bounds); diff --git a/src/DiffEngineViewer.Windows/ViewerForm.cs b/src/DiffEngineViewer.Windows/ViewerForm.cs index 75343a01..30ca6a81 100644 --- a/src/DiffEngineViewer.Windows/ViewerForm.cs +++ b/src/DiffEngineViewer.Windows/ViewerForm.cs @@ -356,6 +356,12 @@ public void Apply(Screen screen) ApplyMenu(screen); } + /// + /// See . + /// + public void LoadPictures() => + canvas.LoadPictures(); + /// /// The bar follows the model rather than owning the position, so it agrees with the keyboard /// and the wheel. The left pane, because that is the row count the session clamps against. @@ -414,6 +420,14 @@ void ApplyButtons(Screen screen) var button = new FormsButton { AutoSize = true, + // Sized to the label it has now. The pool relabels its buttons as the screen + // changes, and a button's default, GrowOnly, kept each one as wide as the longest + // label it had ever held: the footer's layout was the history of the session, and + // the pixel baselines were the history of the test run. + AutoSizeMode = AutoSizeMode.GrowAndShrink, + // The size a button has by default, which GrowOnly never went under, so a short + // label does not make a button smaller than it always was + MinimumSize = LogicalToDeviceUnits(new Size(75, 23)), Margin = new(0, 0, 6, 0), // Standard rather than System: WinForms draws these itself, including in dark // mode, so their pixels are pinned to the .NET version rather than to whatever diff --git a/todo.md b/todo.md index badf41e6..8bf0a0d7 100644 --- a/todo.md +++ b/todo.md @@ -28,4 +28,3 @@ Native ## Perf - [ ] macOS repaints the whole window every frame (`native/swift/Sources/Deview/Runtime.swift:139-140`). Redraw only when the frame, bounds or a picture stamp change, and cache scaled pictures. -- [ ] Windows image panes rescale from full resolution and redraw the checkerboard on every paint (11 to 40 ms per image), and decode on the UI thread (`ViewerCanvas.cs:385-420`, `ImageCache.cs:56-71`). Cache the composited scaled bitmap per path, stamp and size.