Improve ImageLoader thumbnail fallbacks and refactor GetThumbnailResult

[DavidGBrett]

Will resolve #4450, maybe also #4403

Summary

• Refactored GetThumbnailResult by splitting it into per-type helper methods (GetDirectoryThumbnailResult, GetImageFileThumbnailResult, GetSvgFileThumbnailResult, GetFileThumbnailResult)
• Consistently use MissingImage as the final fallback for all but folders. Removed the now-unused ImageIcon constant that was sometimes used instead.
• Added a number of fallbacks if the usual shell thumbnail method fails (e.g. on ARM as in BUG: Missing Icons on ARM #4450)
  • Folders fall back to the default folder icon if shell thumbnail fails
  • Image files fall back to loading the bitmap directly
  • Non-image files fall back to ExtractAssociatedIcon
• Replaced LoadFullImage with new seperate methods for loading the bitmap, scaling it, and extracting dimensions from metadata.
• Removed path mutation from GetThumbnailResult as it no longer served any useful purpose and was a confusing side effect.
• Improved logging by using Log.Warn for final fallbacks and Log.Debug for intermediate fallbacks, removing the excessive use of Log.Exception

Testing

Added ImageLoaderTests, which you can run with dotnet test Flow.Launcher.Test/Flow.Launcher.Test.csproj --filter ImageLoaderTests
This simulates invalid paths and also simulates a failing shell thumbnail to test the fallbacks.

---

Summary by cubic

Improves thumbnail reliability in ImageLoader with per-type fallbacks, no path-mutation side effects, and clearer logging. Adds a size-aware bitmap loader, an injectable ShellThumbnailLoader, and tests that cover key failure paths.

Summary of changes

• Changed:
  • Split GetThumbnailResult into per-type helpers; added CreateImageResult; freeze images; never rewrite paths in logs/cache.
  • Renamed GetThumbnail to GetShellThumbnail; introduced ShellThumbnailLoader delegate for injection.
  • Folders: request shell icon-only; size follows loadFullImage; fallback to preloaded folder image.
  • Image files: prefer shell thumbnail; fallback to scaled bitmap; full image uses size-aware loader.
  • SVG: load via existing SVG path; on error return missing image.
  • Non-image/svg files: prefer shell thumbnail; fallback to TryExtractAssociatedIcon.
  • Logging: Log.Debug for intermediate fallbacks; Log.Warn for final failures.
• Added:
  • Constant.FolderIcon and preloaded FolderImage.
  • LoadBitmapImageScaleToFitWithin, LoadBitmapImage, TryGetBitmapImageDimensionsFromMetadata.
  • TryExtractAssociatedIcon and injectable ShellThumbnailLoader for testability.
  • Unit tests in Flow.Launcher.Test/ImageLoaderTests.cs; .csproj now copies image assets for test runs.
• Removed:
  • Constant.ImageIcon and the ImageLoader.Image placeholder.
  • LoadFullImage and all path-mutation side effects.
• Memory:
  • Slightly lower due to metadata-first sizing, fewer full decodes, frozen images, and disposing GDI icons.
• Security:
  • No new risks; uses OS and WPF APIs without elevated permissions.
• Tests:
  • New tests cover missing-path handling and shell-thumbnail failure fallbacks for folders, image files, and generic files using ShellThumbnailLoader injection.

Release Note
Thumbnails and icons now load more reliably with smarter fallbacks, including a default folder image when needed.

^{Written for commit 47bfb86. Summary will update on new commits. Review in cubic}

[cubic-dev-ai]

No issues found across 2 files

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Replaces ImageIcon with FolderIcon and exposes FolderImage; refactors ImageLoader to dispatch thumbnails by type (directory, image, SVG, generic), adds safe associated-icon extraction, and consolidates bitmap decoding/resizing utilities.

Changes

Image Thumbnail Loading Refactoring

Layer / File(s)	Summary
Constants Update Flow.Launcher.Infrastructure/Constant.cs	ImageIcon constant removed; FolderIcon constant added alongside HistoryIcon and SettingsIcon.
Public API and Imports Flow.Launcher.Infrastructure/Image/ImageLoader.cs	WPF interop using directives added; public cached Image property replaced with FolderImage.
Icon Preloading Flow.Launcher.Infrastructure/Image/ImageLoader.cs	InitializeAsync preload list updated to preload Constant.FolderIcon instead of Constant.ImageIcon.
Retry Logic Integration Flow.Launcher.Infrastructure/Image/ImageLoader.cs	LoadInternalAsync retry now calls GetThumbnailResult(path, loadFullImage) without ref mutation, preserving two-try exception handling.
Thumbnail Dispatcher and Type Handlers Flow.Launcher.Infrastructure/Image/ImageLoader.cs	New GetThumbnailResult(string, bool) dispatcher routes requests to handlers for directories (icon-first, fallback to FolderImage), images (full vs thumbnail with scaling fallback), SVGs (LoadSvgImage), and generic files (shell thumbnail then associated-icon). CreateImageResult and GetMissingThumbnailResult centralize freezing and missing-image mapping.
Supporting Utility Helpers Flow.Launcher.Infrastructure/Image/ImageLoader.cs	TryExtractAssociatedIcon safely extracts and freezes file-type icons. TryGetBitmapImageDimensionsFromMetadata, LoadBitmapImageScaleToFitWithin, and LoadBitmapImage provide metadata-based sizing and flexible decoding with IgnoreColorProfile.

Sequence Diagram

sequenceDiagram
  participant Requester as ImageLoader.LoadInternalAsync
  participant Dispatcher as ImageLoader.GetThumbnailResult
  participant Shell as WindowsThumbnailProvider
  participant Decoder as LoadBitmapImage
  participant Cache as ImageCache/FolderImage

  Requester->>Dispatcher: GetThumbnailResult(path, loadFullImage)
  Dispatcher->>Shell: Try shell thumbnail (directory/image/other)
  Shell-->>Dispatcher: BitmapSource or failure
  alt thumbnail success
    Dispatcher->>Decoder: (optional) scale bitmap
    Decoder-->>Dispatcher: ImageSource (frozen)
    Dispatcher->>Cache: store result
    Dispatcher-->>Requester: ImageResult(success)
  else shell failure for directory
    Dispatcher->>Cache: return FolderImage
    Dispatcher-->>Requester: ImageResult(FolderImage)
  else shell failure for other
    Dispatcher->>Dispatcher: TryExtractAssociatedIcon(path)
    alt extraction success
      Dispatcher->>Cache: store result
      Dispatcher-->>Requester: ImageResult(success)
    else
      Dispatcher->>Cache: store MissingImage
      Dispatcher-->>Requester: ImageResult(MissingImage)
    end
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• Flow-Launcher/Flow.Launcher#3898: Related refactor of ImageLoader initialization and cached/frozen ImageSource wiring.
• Flow-Launcher/Flow.Launcher#3099: Overlaps on thumbnail/full-image loading and icon constants changes.
• Flow-Launcher/Flow.Launcher#3246: Related changes to thumbnail provider exception handling and fallback behavior.

Suggested reviewers

• jjw24
• Jack251970

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main refactoring effort: improving thumbnail fallbacks and refactoring GetThumbnailResult into helper methods, which aligns with the primary changes in the PR.
Linked Issues check	✅ Passed	The PR directly addresses #4450 by catching BadImageFormatException and implementing fallbacks for thumbnails on ARM: folders use FolderImage, images load via bitmap, and other files fall back to ExtractAssociatedIcon, resolving the missing icons issue.
Out of Scope Changes check	✅ Passed	All changes are in-scope: icon path constants refactored (ImageIcon to FolderIcon), thumbnail loading methods split by type, fallback logic added, LoadFullImage removed, and logging improved—all directly supporting the ARM icon reliability objectives.
Description check	✅ Passed	The PR description is detailed and directly related to the changeset, describing the refactoring of GetThumbnailResult, addition of fallbacks, constant updates, and logging improvements.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch improve-icon-handling

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Flow.Launcher.Infrastructure/Image/ImageLoader.cs`:
- Around line 202-203: The directory branch drops the loadFullImage flag causing
folder previews to always use the small icon path; propagate loadFullImage into
the directory path by modifying the call to GetDirectoryThumbnailResult to
accept the loadFullImage parameter (e.g., GetDirectoryThumbnailResult(path,
loadFullImage)) and update GetDirectoryThumbnailResult to pass that flag through
to the GetThumbnail call so it requests FullIconSize when loadFullImage is true
(instead of always calling GetThumbnail(..., IconOnly)). Ensure any other
directory-returning call sites (the block covering 234-244) are updated
similarly.
- Around line 407-424: The method LoadBitmapImageScaleToFitWithin currently
calls LoadBitmapImage(path) which fully decodes the image before checking
dimensions; change it to first read only metadata using BitmapDecoder with
BitmapCreateOptions.DelayCreation and BitmapCacheOption.None to obtain
PixelWidth/PixelHeight without decoding, then if both dimensions are <= maxSize
call LoadBitmapImage(path) to load fully, otherwise call LoadBitmapImage(path,
decodePixelWidth: maxSize) or LoadBitmapImage(path, decodePixelHeight: maxSize)
as the existing widthIsLarger branch does; reference the existing
LoadBitmapImageScaleToFitWithin and LoadBitmapImage functions when making this
change.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1e82a1b0-db57-4015-9fe7-2b4f8be5ef91

📥 Commits

Reviewing files that changed from the base of the PR and between 5779818 and 7ee92e9.

📒 Files selected for processing (2)

• Flow.Launcher.Infrastructure/Constant.cs
• Flow.Launcher.Infrastructure/Image/ImageLoader.cs

[cubic-dev-ai]

1 issue found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="Flow.Launcher.Infrastructure/Image/ImageLoader.cs">

<violation number="1" location="Flow.Launcher.Infrastructure/Image/ImageLoader.cs:166">
P2: The second retry warning logs the wrong exception (`e.Message` instead of `e2.Message`), which misreports the actual second-failure cause.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic}

[jjw24]

Hi @DavidGBrett, could you please update the PR description with what tests you have done so I can see if there are other testing I need to cover.

[DavidGBrett]

Hi @DavidGBrett, could you please update the PR description with what tests you have done so I can see if there are other testing I need to cover.

I've mostly just been adding lines such as throw new System.Exception(); to a try catch where I want to force a fallback.

E.g.

 private static ImageResult GetFileThumbnailResult(string path, bool loadFullImage)
        {
            var size = loadFullImage ? FullIconSize : SmallIconSize;
            try
            {
                throw new System.Exception();

If I wanted to test file icon fallbacks

This is definitely not ideal, though I'm not sure how the existing simple missing icon fallbacks were tested.

I could do a bigger rework with dependency injection throughout to make it properly testable if you would prefer.

If you mean to ask if I found scenarios where this could naturally happen in windows, then I did not, i just forced it.
It likely is an ARM only problem, but I think there is no harm in adding the fallbacks if it helps.

The user confirmed that these changes do fix their problem anyways - see second last comment in #4450
They successfully tested it on their ARM device.

[DavidGBrett]

I've managed to get some unit tests working without a full DI rework - so it should be easier to review now.
I've updated the description with the details on that.

Still using the real users image cache - fine for these tests I believe but probably still worth reworking for DI later
I just don't want to expand the scope of this PR any further.