search

dotnet / wpf

WPF is a .NET Core UI framework for building Windows desktop applications.
MIT License
7.07k stars 1.17k forks source link

BitmapImage: app crash with FileFormatException when loading specific image and DecodePixelWidth #3503

Open campersau opened 4 years ago

campersau commented 4 years ago

Known affected products:

See also https://github.com/NuGetPackageExplorer/NuGetPackageExplorer/issues/1105

Actual behavior: App crashes:

System.IO.FileFormatException
  HResult=0x80131537
  Message=Der Bitmap-Farbkontext ist ungültig.
  Source=PresentationCore
  StackTrace:
   at System.Windows.Media.Imaging.ColorConvertedBitmap.FinalizeCreation()
   at System.Windows.Media.Imaging.ColorConvertedBitmap..ctor(BitmapSource source, ColorContext sourceColorContext, ColorContext destinationColorContext, PixelFormat format)
   at System.Windows.Media.Imaging.BitmapImage.FinalizeCreation()
   at System.Windows.Media.Imaging.BitmapImage.OnDownloadCompleted(Object sender, EventArgs e)
   at System.Windows.Media.UniqueEventHelper.InvokeEvents(Object sender, EventArgs args)
   at System.Windows.Media.Imaging.LateBoundBitmapDecoder.DownloadCallback(Object arg)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
   at System.Windows.Threading.DispatcherOperation.InvokeImpl()
   at MS.Internal.CulturePreservingExecutionContext.CallbackWrapper(Object obj)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at MS.Internal.CulturePreservingExecutionContext.Run(CulturePreservingExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Windows.Threading.DispatcherOperation.Invoke()
   at System.Windows.Threading.Dispatcher.ProcessQueue()
   at System.Windows.Threading.Dispatcher.WndProcHook(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at MS.Win32.HwndWrapper.WndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at MS.Win32.HwndSubclass.DispatcherCallbackOperation(Object o)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
   at System.Windows.Threading.Dispatcher.LegacyInvokeImpl(DispatcherPriority priority, TimeSpan timeout, Delegate method, Object args, Int32 numArgs)
   at MS.Win32.HwndSubclass.SubclassWndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam)
   at MS.Win32.UnsafeNativeMethods.DispatchMessage(MSG& msg)
   at System.Windows.Threading.Dispatcher.PushFrameImpl(DispatcherFrame frame)
   at System.Windows.Threading.Dispatcher.PushFrame(DispatcherFrame frame)
   at System.Windows.Window.ShowHelper(Object booleanBox)
   at System.Windows.Window.Show()
   at System.Windows.Window.ShowDialog()
   at PackageExplorer.PackageChooserDialog.ShowDialog(String searchTerm) in C:\dev_projects\NuGetPackageExplorer\PackageExplorer\PackageChooser\PackageChooserDialog.xaml.cs:line 152

Inner Exception 1:
COMException: Das angegebene Farbprofil ist ungültig. (0x800707DB)

Expected behavior: No crash and invoke DecodeFailed event with this exception instead.

Minimal repro:

var bitmap = new BitmapImage();
bitmap.BeginInit();
bitmap.UriSource = new Uri("https://cdn.statically.io/gh/Starz0r/ChocolateyPackagingScripts/2055976c/assets/legendary.png");
bitmap.DecodePixelWidth = 32;
bitmap.EndInit();
// set source of an <Image>
image.Source = bitmap;
miloush commented 4 years ago

DecodeFailed "occurs when the image fails to load, due to a corrupt image header". Since there is no header corruption here, either the event should not be fired or the documentation needs to be changed.

The image file is of Gray16 pixel format and contains an embedded ICC profile. However, since a specific pixel width is requested, the image goes through a TransformedBitmap which for some reason changes the pixel format to Gray16Float. The closest DUCE format for both is Bgr32. The ColorConvertedBitmap that tries to preserve the ICC profile then fails with unsupported pixel format of Gray16Float, so the code resorts to first converting the bitmap to Bgr32 (note that it would have been fine with Gray16). The second ColorConvertedBitmap attempt then fails with invalid color profile, as reported above. I guess that's because it got a grayscale ICC profile for color pixel format.

I agree this is a bug in the framework: the codepath that tries to recover by converting to the closets DUCE format first wrongly assumes that the color context is valid for the new format too.

I would also consider the change of pixel format by the TransformedBitmap to be unexpected (there are other reports of the issue e.g. here). Either these cases should be documented, or the WIC shouldn't do that.

Last but not least, I am surprised by this transformation chain when the DecodePixelWidth documentation claims that the JPEG and PNG codecs decode into the required dimensions natively. Why is that not happening?

@campersau There is nothing wrong with the file. You have a few workaround options: If you are in control of the image, either do not include ICC profile in the image or use color pixel format. If not, do not use DecodePixelWidth or load and scale the image yourself. I am not sure why you need to use DecodePixelWidth though, rather than setting the Image.Width to a fixed value? That seems to cause extra allocations and processing that yields blurry results under higher DPI.

campersau commented 4 years ago

Thanks for your detailed response! DecodeFailed currently does indeed not fire, it was just my suggestion to do so instead of crashing the app. (And I haven't read the documentation of the event at that time.) But your response indicates that the issue is somewhere else and that the exception might be avoided entirely which would be even better.

The code was added years ago in https://github.com/NuGet/NuGet.Client/commit/6d112fb8459899b3dc098b1621e08c56c11fa88f, https://github.com/NuGet/NuGet.Client/commit/a66e74ef6aa40ceb032bf52b1684cc3bf57f63f9 and ported to NuGetPackageManager in https://github.com/NuGetPackageExplorer/NuGetPackageExplorer/commit/46488622a7a0921714b5a0df002007ac6c8e5438 Here is the current relevant code: https://github.com/NuGetPackageExplorer/NuGetPackageExplorer/blob/4da26d5ea2e6f1af0fda767236552f47672c041f/PackageExplorer/Converters/IconUrlToImageCacheConverter.cs#L52-L70

The BitmapImages are cached in an additional ObjectCache to improve performance, maybe that's why they were trying to make the images smaller.

I will try to do some memory profiling to see if we can drop DecodePixelWidth or need to do the scaling on our own.

We also noticed the blurryness and tried to fix that by using this code:

RenderOptions.SetBitmapScalingMode(iconBitmapImage, BitmapScalingMode.HighQuality);
miloush commented 4 years ago

@campersau since you are not using BitmapCacheOption.None when loading the image, the WPF caches the original bitmap image anyway, so there shouldn't be any need for additional layer of caching. If you do some profiling, I would be curious to see the results.

Another workaround I forgot to mention is using BitmapCreateOptions.IgnoreColorProfile when initializing the image - it's probably the easiest you can do if you wanted to keep DecodePixelWidth. Obviously that might potentially give you different color appearance, but since you are dealing with icons rather than photos, it might be acceptable.

RenderOptions.SetBitmapScalingMode(iconBitmapImage, BitmapScalingMode.HighQuality);

This is a blurry option. You can get sharp but pixelated image using NearestNeighbor. For best quality, I would suggest not throwing away pixels using the DecodePixelWidth in the first place.