Use BitmapCreateOptions.IgnoreColorProfile to workaround WPF issue when using DecodePixelWidth#3677
Conversation
|
cc @zivkan |
|
@campersau where was the image sourced from? I'm guessing it's not an image you created yourself just to test this fix. I want to make sure it has a license that's compatible. |
zivkan
left a comment
There was a problem hiding this comment.
I just noticed the image is 166 KB. I'm still curious about the license to make sure there's no issue us using it, even if it's just for a test. However, I also want to be more mindful about not bloating the repo by having needlessly large binary files. a 10x10 pixel image with whatever colour palette should be able to reproduce the issue, and be probably less than 1KB.
|
@zivkan The image is from here https://cdn.statically.io/gh/Starz0r/ChocolateyPackagingScripts/2055976c/assets/legendary.png I have changed the original image by resizing it and setting all pixels to black. It is now 9KB mostly because of the color profiles (8KB) which are required for this test. Is this okay? |
donnie-msft
left a comment
There was a problem hiding this comment.
I'm not thrilled with the idea of having to potentially distort colors to avoid this issue, but reading the linked content, it seems the only way to work-around the problem.
I may revisit this when I work on Default Package Icons NuGet/Home#5937
There was a problem hiding this comment.
This is interesting. If the image has a bad color profile or any other bad image data, then the catch at line 170 should have caught the exception. Instead, the whole devenv.exe process crashes.
There was a problem hiding this comment.
I think it is because the image needs to be downloaded from the URI so the BitmapImage creates a BitmapDecoder which uses a LateBoundBitmapDecoder which uses the BitmapDownload helper class which uses a separate thread for when the download finishes. And that's where the exception is thrown and so it crashes the application.
Edit: The test works because it is using a file path and so throws with a different stack trace:
Message:
System.IO.FileFormatException : Der Bitmap-Farbkontext ist ungültig.
---- System.Runtime.InteropServices.COMException : Das angegebene Farbprofil ist ungültig. (Ausnahme von HRESULT: 0x800707DB)
Stack Trace:
ColorConvertedBitmap.FinalizeCreation()
ColorConvertedBitmap.ctor(BitmapSource source, ColorContext sourceColorContext, ColorContext destinationColorContext, PixelFormat format)
BitmapImage.FinalizeCreation()
BitmapSource.CompleteDelayedCreation()
BitmapSource.get_PixelWidth()
otherwise we would have to wait until the image is fully downloaded.
There was a problem hiding this comment.
Normally, IconUrlToImageCacheConverter accepts either embedded icon URIs (an absolute URI to a package filepath with a fragment part, pointing to the relative icon path inside the package) or absolute URIs. Since we are just testing that image is read, it's fine to leave the relative URI in tests.
to workaround WPF issue when using DecodePixelWidth
d5f8128 to
8175b93
Compare
dominoFire
left a comment
There was a problem hiding this comment.
Thank you so much for your contribution, @campersau !
I verified the fix works as expected
Also, I verified that both images (test and legendary.png) has some unknown ICC metadata that tests this fix. Validation is done via wicexplorer. The 'Unknown reader' elements in the tree view represent the unknown metadata contained in the images.
…en using DecodePixelWidth (#3677) Fixes NuGet/Home#10037
Bug
Fixes: NuGet/Home#10037
Regression: No
Fix
Details can be found here dotnet/wpf#3503 as well as the workaround which is implemented by this PR.
See also NuGetPackageExplorer/NuGetPackageExplorer#1105 for more analysis.
Testing/Validation
Tests Added: Yes
Reason for not adding tests:
Validation: