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

Unfortunately adding unit tests would require further reworks - Nor do I have an ARM device available to try this on.
So to test the fallbacks I have been adding a throw new System.Exception(); to the try catch that would trigger the fallback I want to test.

---

Summary by cubic

Refactors ImageLoader to make thumbnail loading reliable and consistent, while removing path-mutation side effects. Adds smarter fallbacks: non-images use TryExtractAssociatedIcon, folders use a preloaded folder icon, and images use efficient bitmap decoding.

Summary of changes

• Changed:
  • Split GetThumbnailResult into per-type helpers and added CreateImageResult; images are frozen; paths are never rewritten in logs/cache.
  • Folders: request shell icon-only; size respects loadFullImage (SmallIconSize/FullIconSize); on failure, use preloaded FolderImage.
  • Image files: prefer shell thumbnail; on failure, load bitmap scaled to fit (SmallIconSize for thumbs, FullImageSize for previews); else use MissingImage.
  • SVG files: load via LoadSvgImage; on failure, use MissingImage.
  • Other files: prefer shell thumbnail; on failure, use TryExtractAssociatedIcon; else use MissingImage.
  • Scaling avoids full decodes via metadata-first dimension checks; improved logging with Log.Warn for final failures and Log.Debug for fallbacks.
• Added:
  • Constant.FolderIcon and preloaded FolderImage.
  • Helpers: LoadBitmapImageScaleToFitWithin, LoadBitmapImage, TryExtractAssociatedIcon, CreateImageResult, TryGetBitmapImageDimensionsFromMetadata.
• Removed:
  • Constant.ImageIcon and ImageLoader.Image placeholder.
  • LoadFullImage and all path-mutation side effects.
• Memory:
  • Neutral/slightly better: fewer unnecessary decodes, metadata-first checks, frozen images, and proper disposal of GDI icons.
• Security:
  • No new risks; continues to use OS APIs with proper disposal; no elevated permissions.
• Tests:
  • No unit tests.

Release Note
Thumbnails and icons are more reliable and consistently sized, with smarter fallbacks for folders, images, and files.

^{Written for commit bca3bbd. 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 clearly relates to the changeset, detailing refactoring of ImageLoader thumbnail handling, removal of ImageIcon, addition of FolderIcon, new fallback mechanisms, and helper method reorganization.

_{✏️ 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

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

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.