[Peek] Thumbnail bitmaps created and never used if they can be created quickly enough

[daverayment]

Microsoft PowerToys version

0.83.0

Installation method

PowerToys auto-update

Running as admin

No

Area(s) with issue?

Peek

Steps to reproduce

I've been working on fixing a memory leak in Peek, and found this additional issue inside ImagePreviewer.

ImagePreviewer works by displaying a full size image if it can. It has fallbacks, however. If the full size image load fails, a high quality thumbnail is shown. If the high res thumbnail is unavailable and cannot be created, a low quality thumbnail is shown. If the full size image and both the thumbnails fail to load, a generic preview is shown.

There are 3 tasks in the code which produce bitmaps for the previewer to display: LoadLowQualityThumbnailAsync, LoadHighQualityThumbnailAsync, and LoadFullQualityImageAsync. When the navigated item is updated, the LoadPreviewAsync method begins all the tasks at once with this:

await Task.WhenAll(LowQualityThumbnailTask, HighQualityThumbnailTask, FullQualityImageTask);

The thumbnail tasks use the ThumbnailHelper's GetThumbnail method to retrieve or create the associated Windows Shell thumbnail. Handles to the thumbnails are returned to ImagePreviewer. If the full size image hasn't loaded by the time the handles are returned, the bitmap data is copied to the corresponding thumbnail ImageSource:

if (!IsFullImageLoaded)
{
    var thumbnailBitmap = await BitmapHelper.GetBitmapFromHBitmapAsync(highQualityThumbnail, IsPng(Item), cancellationToken);
    highQualityThumbnailPreview = thumbnailBitmap;
}

(Where highQualityThumbnail is the handle to the unmanaged bitmap.)

Whether or not the copy happens, the unmanaged resource still takes up memory. This was the source of the memory leak because the resources were not freed correctly, but I've fixed that in PR #34484.

The obvious problem with this approach is that the thumbnails are always requested for every file item previewed. If the full size image loads correctly (which will be the case for the vast majority of the time), they will sit unused. Those thumbnails will also be automatically added to the Windows thumbnail cache if they are not already present.

I would like to propose simplifying the ImagePreviewer code to remove the Task.WhenAll creation call, and make the flow synchronous.

Additionally, the code to generate the low quality thumbnail is not required. This is because the options passed to IShellItemImageFactory::GetImage permit the Shell to automatically negotiate the best available thumbnail to return (the combination of BiggerSizeOk and ScaleUp). The fallback code and retry logic in ThumbnailHelper may be removed.

Removing the existing WhenAll call and duplicate thumbnail retrieval code significantly simplifies the ImagePreviewer code and reduces the memory used by Peek. In testing, garbage collection was reduced by 75%+ for all generations.

I have code for this already, so would like to be assigned to this task. It is a low-complexity fix.

✔️ Expected Behavior

I only expect thumbnail bitmaps to be created and displayed within Peek when the full size image cannot be loaded.

❌ Actual Behavior

Both High Quality and Low Quality thumbnail bitmaps are always created. In the majority of cases, the bitmaps are unused because the full size image is subsequently successfully loaded and displayed.

Other Software

No response