Feature: Add font file thumbnail generation and display font names #17843
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| var fontDisplayName = FontFileHelper.GetFontName(item.ItemPath); | ||
| if (!string.IsNullOrEmpty(fontDisplayName) && fontDisplayName != item.Name) | ||
| { | ||
| cts.Token.ThrowIfCancellationRequested(); | ||
| await dispatcherQueue.EnqueueOrInvokeAsync(() => | ||
| { | ||
| item.ItemNameRaw = fontDisplayName; | ||
| }); | ||
| await fileListCache.AddDisplayName(item.ItemPath, fontDisplayName); | ||
| } | ||
| } |
There was a problem hiding this comment.
Bug: FontFileHelper.GetFontName() performs synchronous blocking I/O directly, potentially blocking the UI thread.
Severity: HIGH | Confidence: 0.95
🔍 Detailed Analysis
The FontFileHelper.GetFontName() method performs synchronous blocking file I/O operations, including reading font files and binary parsing, directly on the calling thread within LoadExtendedItemPropertiesAsync. This violates established threading patterns, causes inconsistent behavior compared to similar operations in the codebase, and can lead to noticeable UI delays when browsing directories containing multiple font files.
💡 Suggested Fix
Wrap the call to FontFileHelper.GetFontName(item.ItemPath) at line 1228 with await Win32Helper.StartSTATask() to offload the blocking operation to a background thread.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/Files.App/ViewModels/ShellViewModel.cs#L1225-L1238
Potential issue: The `FontFileHelper.GetFontName()` method performs synchronous blocking
file I/O operations, including reading font files and binary parsing, directly on the
calling thread within `LoadExtendedItemPropertiesAsync`. This violates established
threading patterns, causes inconsistent behavior compared to similar operations in the
codebase, and can lead to noticeable UI delays when browsing directories containing
multiple font files.
Did we get this right? 👍 / 👎 to inform future reviews.
| // Ensure size is at least 1 to prevent layout errors | ||
| size = Math.Max(1, size); | ||
|
|
||
| if (!isFolder && !iconOptions.HasFlag(IconOptions.ReturnIconOnly)) |
There was a problem hiding this comment.
Considering the issue with font thumbnails is only in the official fonts folder, would it make things simpler to only run this code in that location?
2 participants
Resolved / Related Issues
Added support for generating font file thumbnails and displaying proper font names instead of filenames when browsing font directories.
Closes #17786
Steps used to test these changes