dotnet / wpf

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

System.OverflowException in WriteableBitmap.WritePixels #9438

Open petsuter opened 3 months ago

petsuter commented 3 months ago

Description

WriteableBitmap can not exceed 4 GB I think (WriteableBitmap constructor fails) but even for sizes below 4 GB (but above 2GB) there are overflow exceptions in WriteableBitmap.WritePixels, even when only writing a tiny amount of data.

Reproduction Steps

                int size = 30000;
                int tileSize = 100;
                int channels = 4;
                var pix = new byte[tileSize * tileSize * channels];
                var bitmap = new WriteableBitmap(size, size, 96, 96, PixelFormats.Pbgra32, null);
                bitmap.WritePixels(new Int32Rect(0, 0, tileSize, tileSize),
                      pix, tileSize * channels, size-tileSize, size-tileSize);

Expected behavior

No exception. (Or if >2GB is considered unsupported, the constructor should already fail?)

Actual behavior

Exception:

System.OverflowException
  HResult=0x80131516
  Message=Arithmetic operation resulted in an overflow.
  Source=PresentationCore
  StackTrace:
   at System.Windows.Media.Imaging.WriteableBitmap.WritePixelsImpl(Int32Rect sourceRect, IntPtr sourceBuffer, Int32 sourceBufferSize, Int32 sourceBufferStride, Int32 destinationX, Int32 destinationY, Boolean backwardsCompat) in System.Windows.Media.Imaging\WriteableBitmap.cs:line 583
   at System.Windows.Media.Imaging.WriteableBitmap.WritePixels(Int32Rect sourceRect, Array sourceBuffer, Int32 sourceBufferStride, Int32 destinationX, Int32 destinationY) in System.Windows.Media.Imaging\WriteableBitmap.cs:line 271
   at Program.Main() in Program.cs:line 15

Regression?

I don't think so.

Known Workarounds

None

Impact

Unknown

Configuration

.NET 8, Windows 11, x64. I don't think this is specific.

Other information

None.

Maybe uint should be used somewhere instead of int in the internal offset calculations?

h3xds1nz commented 3 months ago

The exception is thrown here in WritePixelsImpl:

https://github.com/dotnet/wpf/blob/0310ea93a4ea4af6123dc5de3da1e3e3ee582100/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Media/Imaging/WriteableBitmap.cs#L917

because the calculation is done in checked context.

However, even if you cast the multiplication to be in uint, then upon creating Bitmap from source (e.g. Cloning), you reach:

int bufferSize = checked(_backBufferStride.Value * source.PixelHeight);
source.CriticalCopyPixels(rcFull, _backBuffer, bufferSize, _backBufferStride.Value);

And the last one is in CriticalCopyPixels where minRequiredDestSize is again calculated in checked context as int.

If you fix those 3, it works as expected and the bitmap is generated with the proper rectangle in the bottom corner.

Since the IWICBitmapSource_CopyPixels_Proxy accepts uint as size as well (documented), I do not see any other reason for this than a bug. Rest of the source would also indicate so.

However, the problem though is being 2GB limitation for e.g. byte[], which means that unless you would init a bitmap source that big (30k*30k*4) from an unmanaged array, you're not able to init it.

So in the end I guess the solutions are either 1) throw on constructor 2) different behaviour for unmanaged buffers.

As per 2), funny enough, IWICImagingFactory_CreateBitmapFromMemory_Proxy fails for buffers larger than Int32.MaxValue with 0x80070057, so no dice I guess. (That's for BitmapSource but it inherits, yet WriteableBitmap uses Mil)

petsuter commented 3 months ago

Could it not be left uninitialized and filled in parts with WritePixels? No full sized byte[] array is required for that. Or is it used internally?

(What is Mil?)

h3xds1nz commented 3 months ago

It is possible to fix the few calcs in WriteableBitmap and it will be possible to write pixels with your code into the bitmaps with size over 23xxx * 23xxx and 4 channels. It is also possible to clone and save it afterwards.

However, initializing BitmapSource from either managed array or unmanaged array (which uses IWICImagingFactory_CreateBitmapFromMemory_Proxy fails with 0x80070057 for buffers bigger than Int32.MaxValue.

I do not know what the call-chain and magic is behind wpfgfx right now as I've never needed it, neither have the time to do the necessary research on why that works currently but given the fact it uses two buffers, it is probably not using the WIC call/interface.

miloush commented 3 months ago

related #5125

2GB limitation for e.g. byte[]

https://learn.microsoft.com/en-us/dotnet/framework/configure-apps/file-schema/runtime/gcallowverylargeobjects-element

1) throw on constructor

Not thrilled about that, it's the WritePixels operation that fails, not creating the bitmap.

What is Mil?

Media Integration Layer

it is probably not using the WIC call/interface

Doesn't the WritePixels just call

https://github.com/dotnet/wpf/blob/0310ea93a4ea4af6123dc5de3da1e3e3ee582100/src/Microsoft.DotNet.Wpf/src/WpfGfx/core/common/exports.cpp#L291

miloush commented 3 months ago

But yes, looks like WIC does not support >2GB bitmaps, at least for the standard pixel formats.

h3xds1nz commented 3 months ago

https://learn.microsoft.com/en-us/dotnet/framework/configure-apps/file-schema/runtime/gcallowverylargeobjects-element

Still doesn't bypass the single-dim array length limit, it just allows the underlying data to be bigger in size than 2GB, see remarks: The maximum size in any single dimension is 2,147,483,591 (0x7FFFFFC7) for byte arrays and arrays of single-byte structures, and 2,146,435,071 (0X7FEFFFFF) for arrays containing other types.

Not thrilled about that, it's the WritePixels operation that fails, not creating the bitmap.

That is true, we could fix this particular thing with WriteableBitmap and document the rest with BitmapSource. I can do it as I've already spent time on researching this and have it basically ready, just wanna run some tests.

Doesn't the WritePixels just call https://github.com/dotnet/wpf/blob/0310ea93a4ea4af6123dc5de3da1e3e3ee582100/src/Microsoft.DotNet.Wpf/src/WpfGfx/core/common/exports.cpp#L291

Yeah, WritePixels -> WritePixelsImpl -> MilUtility_CopyPixelBuffer, I had BitmapSource in mind there which uses the IWICBitmapSource_CopyPixels_Proxy which works with 4GB, but you cannot create it.

h3xds1nz commented 3 months ago

9470 does the job, I couldn't find it breaking anything while testing. Fixes cloning as well.

lindexi commented 3 months ago

I'm worried it won't work on many graphics cards...

DanFTRX commented 3 months ago

https://learn.microsoft.com/en-us/dotnet/framework/configure-apps/file-schema/runtime/gcallowverylargeobjects-element

Still doesn't bypass the single-dim array length limit, it just allows the underlying data to be bigger in size than 2GB, see remarks: The maximum size in any single dimension is 2,147,483,591 (0x7FFFFFC7) for byte arrays and arrays of single-byte structures, and 2,146,435,071 (0X7FEFFFFF) for arrays containing other types.

Not thrilled about that, it's the WritePixels operation that fails, not creating the bitmap.

That is true, we could fix this particular thing with WriteableBitmap and document the rest with BitmapSource. I can do it as I've already spent time on researching this and have it basically ready, just wanna run some tests.

Doesn't the WritePixels just call https://github.com/dotnet/wpf/blob/0310ea93a4ea4af6123dc5de3da1e3e3ee582100/src/Microsoft.DotNet.Wpf/src/WpfGfx/core/common/exports.cpp#L291

Yeah, WritePixels -> WritePixelsImpl -> MilUtility_CopyPixelBuffer, I had BitmapSource in mind there which uses the IWICBitmapSource_CopyPixels_Proxy which works with 4GB, but you cannot create it.

If the single-dim array limit is the issue, using a larger underlying type like a double array instead of a byte array would quadruple the size limit. But I don't know if that would play well with all the native underlying functions.

h3xds1nz commented 3 months ago

I'm worried it won't work on many graphics cards...

I mean, you could create it previously, you just couldn't write to the entirety of its dimension. So that's a bug imho. As a developer using this class/interface, you should be aware of the implications of creating such a large bitmap. That's just common sense. It's like using raw pointers in C#, don't do it if you don't know what it means and how to interact with GC.

If the single-dim array limit is the issue, using a larger underlying type like a double array instead of a byte array would quadruple the size limit. But I don't know if that would play well with all the native underlying functions.

As noted in the PR and here as well, WIC only allows creation of 2GB buffers bitmaps in size, which fits perfectly for single-dim byte array (for BitmapSource). But for WriteableBitmap, that works just fine after the PR as the layer supports it.

petsuter commented 3 months ago

such a large bitmap

(2-3GB is not really large today. Large scientific images can be 2-3TB. Nobody expects to be able to handle such large images directly yet. (Even with machines that have TBs of RAM today!) But would be nice to not crash on comparably much smaller 2-3GB images.)

I'm worried it won't work on many graphics cards...

A valid concern unfortunately. But could be tested maybe.