[Peek] Fix thumbnails being created and not used. Fix icon bitmaps leaking memory. Simplify ImagePreviewer.

[daverayment]

Summary of the Pull Request

This fixes various issues around thumbnail and icon creation. Most significantly, ImagePreviewer was always creating two thumbnails for every file navigated, which were almost always unused.

The PR also consolidates two thumbnail creation classes, and fixes IconHelper failing to delete unmanaged bitmaps.

PR Checklist

• Closes: [Peek] Thumbnail bitmaps created and never used if they can be created quickly enough #34490 [Peek] Previewer icon bitmaps are never deleted, leaking memory #34527
• Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
• Tests: Added/updated and all pass
• Localization: All end user facing strings can be localized
• Dev docs: Added/updated
• New binaries: Added on the required places
  • JSON for signing for new binaries
  • WXS for installer for new binaries and localization folder
  • YML for CI pipeline for new test projects
  • YML for signed pipeline
• Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

1. Thumbnail Creation

Two thumbnail/icon creation classes were used previously, providing similar functionality: IconHelper and ThumbnailHelper. These have now been combined into a new ThumbnailHelper, which includes appropriate comments for the class and public members. The old ThumbnailHelper returned unmanaged bitmap handles, which were not deleted correctly, leading to a memory leak. The new ThumbnailHelper only returns managed ImageSources.

Previewers which used IconHelper have been updated to use ThumbnailHelper, which behaves identically for these calls.

2. ImagePreviewer Refactor

ImagePreviewer had several issues:

1. When starting previewing a file, it kicked off 3 tasks to load the full-size image and 2 thumbnail images. This is wasteful, as the thumbnails are never used if the full-size image loads, and any created thumbnails are automatically added to the Explorer cache. Additionally, the low quality thumbnail task was not required, as the thumbnail creation code already had a fallback path to attempt to load lower-resolution thumbnails if unsuccessful.
2. It relied on the old ThumbnailHelper, for the 2 thumbnail images, which meant it had to translate unmanaged bitmap handles into ImageSource instances, and also mandated the deletion of the unmanaged resources when it was about to go out of scope. This also had the disadvantage of keeping the unmanaged resources around even after they had been translated into ImageSources, which was unnecessary.
3. The 3 tasks attempt to access the same file at once (if the thumbnail were not already present in the Explorer thumbnail cache), potentially leading to contention and a delay in the full-size image being shown.
4. The async code complicated the flow significantly, and fields for the Tasks, the bitmap handles, and the thumbnail images were required.

The updated ImagePreviewer is 50% smaller, only creates a thumbnail when needed (when the full-size image fails to load), and is fully managed, meaning it no longer needs to implement IDisposable. It uses significantly less memory: garbage collection is reduced by ~75% in testing.

The thumbnail retrieval code now deliberately only checks the Explorer thumbnail/icon cache, as it can be assumed after the full-size image load failed that there is an issue with accessing the file. We may wish to implement retry logic in the future.

Validation Steps Performed

Testing was performed on three types of build:

1. Normal. Includes full-size image loading and thumbnail creation when that fails. This represents the code in the PR.
2. Thumbnail testing. In this build, the full-size image load is always set to fail, meaning the thumbnail path was exercised fully.
3. Instrumented builds. Garbage collections were monitored and output with some simple code added to FilePreview.xaml.cs in these builds, to track the difference between old and new approaches for ImagePreviewer.

Tests were performed on a folder containing approximately 90% image files of various resolutions, but mostly 1024x1024 JPEGs. The folder also included 'special' folders (i.e. those with icons in Explorer), zips, exe files, a PDF, an epub file, a supported RAW file (an RW2 sample) and 2 unsupported RAW files (DNG files). The non-image and unsupported files were included so I could confirm the other previewers were unaffected.

I attached to the running instance of PowerToys.Peek.UI.exe in Visual Studio before starting each run. Heap snapshots were taken at the start and then after previewing between 100 and 500 files. (I tried to wait until just after a GC, so this is not an exact count.) The snapshots and process memory used were compared, and the GC counts noted for runs where this was instrumented.

For thumbnail testing runs, special care was taken to clear the thumbnail and icon caches between runs. Just prior to starting each run, the test folder was opened again in Explorer to cache the thumbnails for a select number of files. This let me test the difference between cached and non-cached behaviour and to see whether the PreviewState.Error state was still handled correctly.

Finally, to confirm that the thumbnail routine always retrieved the highest quality thumbnail available, I opened the Details Pane in Explorer and clicked on one or more files in the test folder. This served to prime the thumbnail cache with high resolution previews for those specific images. The resolution difference was noted in the subsequent Peek run by comparing with the other non-previewed images.

Example Run GC Stats

Generation	GCs before	GCs after	Drop
0	94	23	75%
1	89	21	76%
2	59	15	75%

[htcfreek]

Does this pr conflicts/overlaps with #34484?

[daverayment]

Hi @htcfreek . There is no conflict or overlap between this and the other PR, no.

The other PR is required so Dispose is called on Previewer instances before a new one is created, so any unmanaged resources are correctly cleaned up. For example, UnsupportedFilePreviewer unsubscribes from a timer, and I think VideoPreviewer also needs it. Prior to this PR, ImagePreviewer worked with unmanaged thumbnail bitmaps, which also needed disposal.