wpf icon indicating copy to clipboard operation
wpf copied to clipboard

Fix WriteableBitmap's Write/CopyPixels (Clone) to support up to 4GB backbuffers

Open h3xds1nz opened this issue 1 year ago • 3 comments

Fixes #9438

Description

Fixes working with WriteableBitmap to (WritePixels/Clone) operations on large buffers (over Int32.MaxValue) as those are supported by WpfGfx/Milcore as the underlying layer supports creation and working with such buffers.

Previously you were just able to create such WriteableBitmap but unable to WritePixels to the bottom-right corner or Clone it.

This departs from BitmapSource behaviour which uses WIC and there's an ongoing disparity between IWICBitmapSource_CopyPixels_Proxy and IWICImagingFactory_CreateBitmapFromMemory_Proxy where the former works with up to 4GB buffers but creating such bitmap fails with 0x80070057 even though both bufferSize params are documented as uint.

Base code that would fail with OverflowException before on WritePixels (creation would succeed).

//Base vars
int width = 30000;
int height = 30000;
int tileSize = 1000;
int channels = 4;
int stride = tileSize * channels;
byte[] smallRect = new byte[tileSize * tileSize * channels];
WriteableBitmap bitmap = new WriteableBitmap(width, height, 96, 96, PixelFormats.Pbgra32, null);

//Set teal color
for (int row = 0; row < tileSize; row++)
{
    for (int col = 0; col < tileSize; col++)
    {
        // BGRA
        smallRect[row * stride + col * 4 + 0] = 255;
        smallRect[row * stride + col * 4 + 1] = 230;
        smallRect[row * stride + col * 4 + 2] = 0;
        smallRect[row * stride + col * 4 + 3] = 255;
    }
}

//Top-Left
bitmap.WritePixels(new Int32Rect(0, 0, tileSize, tileSize),
                    smallRect, tileSize * channels, 0, 0);

//Top-Right
bitmap.WritePixels(new Int32Rect(0, 0, tileSize, tileSize),
                    smallRect, tileSize * channels, width - tileSize, 0);
                    
//Middle Rect
bitmap.WritePixels(new Int32Rect(0, 0, tileSize, tileSize),
                    smallRect, tileSize * channels, (width - tileSize) / 2, (height - tileSize) / 2);

//Bottom-Left
bitmap.WritePixels(new Int32Rect(0, 0, tileSize, tileSize),
                    smallRect, tileSize * channels, 0, height - tileSize);

//Bottom-Right
bitmap.WritePixels(new Int32Rect(0, 0, tileSize, tileSize),
                    smallRect, tileSize * channels, width - tileSize, height - tileSize);

//Save (Test CriticalPixelsCopy)
BitmapSource bitmapCopy = bitmap.Clone();

Customer Impact

Ability to write pixels on bitmaps over 23xxx x 23xxxx bitmaps with 4 channels, etc. (using backbuffers over 2GB).

Regression

No.

Testing

Local build, working with WriteableBitmap.

Risk

Low.

Microsoft Reviewers: Open in CodeFlow

h3xds1nz avatar Jul 25 '24 20:07 h3xds1nz

LGTM

miloush avatar Jul 25 '24 21:07 miloush

@miloush I had to add one more fix, for when you use f.e. uint[] array which is over 2GB in size (e.g. you wanna write from a source buffer of 3GB, etc.) Looks like I've missed that one during my tests and it triggered me now.

h3xds1nz avatar Jul 26 '24 16:07 h3xds1nz

I have fixed the merge conflicts and rebased.

h3xds1nz avatar Sep 23 '24 18:09 h3xds1nz

Resolved merge conflicts 07

h3xds1nz avatar Oct 07 '24 16:10 h3xds1nz

Any plans on implementing this PR?

farosch avatar Dec 24 '24 16:12 farosch

@farosch @h3xds1nz I have reviewed this PR, I will ask the team for queuing this for testing. Hopefully we will be able to close this within this month.

@h3xds1nz can we convert the above base code into a test ?

dipeshmsft avatar Mar 13 '25 07:03 dipeshmsft

@dipeshmsft Yeah, I'll put something up over the weekend.

h3xds1nz avatar Mar 13 '25 09:03 h3xds1nz

Codecov Report

Attention: Patch coverage is 92.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 11.25422%. Comparing base (b360342) to head (37ee319). Report is 61 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main       #9470         +/-   ##
===================================================
- Coverage   11.40490%   11.25422%   -0.15068%     
===================================================
  Files           3214        3315        +101     
  Lines         648458      665235      +16777     
  Branches       71511       74668       +3157     
===================================================
+ Hits           73956       74867        +911     
- Misses        573340      589060      +15720     
- Partials        1162        1308        +146     
Flag Coverage Δ
Debug 11.25422% <92.00000%> (-0.15068%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Mar 19 '25 19:03 codecov[bot]

@dipeshmsft So it seems to work on CI but I guess the VM is sweating given the log from running unit tests:

##[warning]Free memory is lower than 5%; Currently used: 98.05%
##[warning]Free memory is lower than 5%; Currently used: 98.05%
##[warning]Free memory is lower than 5%; Currently used: 98.05%
##[warning]Free memory is lower than 5%; Currently used: 98.05%

It's no wonder, this will easily get the testhost to consume 70GB+ on my machine (since the runtime can allow to expand, I guess that CI VM is running with 16 GB or less). It also takes about 50s on CI to run, which is not ideal, I'm happy with 8s on my machine.

I assume we don't have the ability to run different types of tests on-demand with the CI so we could exclude this from regular runs.

h3xds1nz avatar Mar 23 '25 11:03 h3xds1nz

I assume we don't have the ability to run different types of tests on-demand with the CI so we could exclude this from regular runs.

Not yet, I think it would be best to disable this test for now.

dipeshmsft avatar Apr 07 '25 08:04 dipeshmsft

@dipeshmsft The tests for large bitmaps are now skipped.

h3xds1nz avatar Apr 07 '25 08:04 h3xds1nz

Thanks @h3xds1nz for this PR.

dipeshmsft avatar Apr 07 '25 08:04 dipeshmsft

@dipeshmsft Thanks :)

h3xds1nz avatar Apr 07 '25 09:04 h3xds1nz