Decode theme thumbnails with SkiaSharp so WebP themes work - #700
Open
rmbakker88 wants to merge 2 commits into
Open
Decode theme thumbnails with SkiaSharp so WebP themes work#700rmbakker88 wants to merge 2 commits into
rmbakker88 wants to merge 2 commits into
Conversation
Thumbnails were decoded with System.Drawing, which cannot read WebP, so generating one for a WebP theme threw ExternalException even though the preview renderer added in 5.7.0 already decodes WebP through SKCodec. Decode through SKCodec here as well and scale with the same Mitchell resampler the preview uses, then copy the pixels into a Bitmap for the ListView image list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A cached thumbnail was only used when its size matched exactly, so a thumbnail.png supplied with a theme, or one cached before the display scaling changed, was thrown away and regenerated from the theme images on every load. Scale the cached thumbnail to the requested size instead, and fall back to generating a new one if it cannot be read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new decode path can throw non-OOM exceptions (e.g., ArgumentException) in common failure cases and there are avoidable allocations in pixel-copy that should be addressed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates theme thumbnail generation to decode and scale images via SkiaSharp (SKCodec) so themes using WebP (and other non-GDI+ formats) can generate thumbnails reliably, and improves thumbnail-cache reuse across DPI/scaling sizes.
Changes:
- Decode thumbnail inputs with
SKCodecand scale usingSKCubicResampler.Mitchell, then copy pixels into aSystem.Drawing.Bitmapfor the WinFormsImageList. - Reuse existing cached/bundled thumbnails even when their dimensions don’t exactly match the requested size by scaling them instead of discarding/regenerating.
- Add error logging when cached thumbnail loading fails so the system can fall back to generating a new thumbnail.
File summaries
| File | Description |
|---|---|
| src/ThemeThumbLoader.cs | Switch thumbnail decode/scale to SkiaSharp and improve cached thumbnail reuse across requested sizes/DPI. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+109
to
+119
| try | ||
| { | ||
| byte[] pixels = skBitmap.Bytes; | ||
| int rowBytes = Math.Min(skBitmap.RowBytes, bmpData.Stride); | ||
|
|
||
| for (int y = 0; y < skBitmap.Height; y++) | ||
| { | ||
| Marshal.Copy(pixels, y * skBitmap.RowBytes, IntPtr.Add(bmpData.Scan0, y * bmpData.Stride), | ||
| rowBytes); | ||
| } | ||
| } |
Comment on lines
+69
to
74
| using (SKCodec codec = SKCodec.Create(stream)) | ||
| { | ||
| Bitmap bmp = new Bitmap(size.Width, size.Height, PixelFormat.Format32bppArgb); | ||
|
|
||
| using (Graphics g = Graphics.FromImage(bmp)) | ||
| if (codec == null) | ||
| { | ||
| g.DrawImage(tempImage, new Rectangle(0, 0, bmp.Width, bmp.Height)); | ||
| throw new ArgumentException("Image could not be decoded because its format is not supported"); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relates to #690 , and to #696 .
In #690 you mentioned WDD should already handle previewing
.pngand.webp. Preview does —Skia/ImageCache.csdecodes throughSKCodec, which reads WebP fine. Thumbnail generation was missed by the 5.7.0 Skia change though:ThemeThumbLoaderstill decodes withSystem.Drawing, and GDI+ cannot read WebP at all.Image.FromFileon a.webpthrows:So a WebP theme previews but has no thumbnail — and before #NNN is fixed, that exception hangs the import dialog outright.
What this changes
Decode thumbnails with
SKCodec(first commit). Same decoder and the sameSKCubicResampler.Mitchellsampling the preview renderer already uses, then a pixel copy into aBitmapfor theListViewimage list.ScaleImage(Image, Size)is gone since every caller now starts from a path or a stream. No new package —SkiaSharpis already referenced.Reuse cached thumbnails that do not match the requested size (second commit).
GetThumbnailImageonly reused a cached thumbnail when its size matched exactly, and otherwise disposed it and regenerated from the theme images. That meant athumbnail.pngsupplied with a theme was silently ignored unless it happened to be exactly 192x108, and on any display not at 100% scaling — where the requested size is e.g. 256x144 — even the bundled thumbnails were discarded and regenerated on every single load. It now scales the cached thumbnail, and falls back to generating a new one if the file cannot be read.I did not re-save the scaled result to
thumbnail.png, so a thumbnail shipped with a theme stays as the author made it rather than being overwritten by the cache. Happy to change that if you would rather the cache always hold the display-sized version.Testing
dotnet test --filter "type!=system"— 7 passed, unchanged frommain..jpg,.pngand.webpfiles, scaling to 192x108 and upscaling 192x108 to 256x144. Verified the output is the requested size and that four known-colour quadrants land in the right place with the right values, which catches both channel-order and orientation mistakes..webpfails onmainwith the exception above and passes here;.jpgand.pngpass on both.mainnow imports with a real generated thumbnail and previews correctly.Note on scope
This is only about WDD reading WebP theme images. It does not touch how wallpaper is handed to Windows, so it is not the whole of #690 if that also means writing
.webpto the desktop on builds that support it natively — happy to leave that part alone or look at it separately.