[PATCH 0/5] MR10652: gdiplus: Rewrite GdipCreateBitmapFromGdiDib.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=40409 The main thing that changes here is that we reference bits directly instead of copying to an intermediate HBITMAP. Most of the tests are to make sure we don't regress anything by parsing BITMAPINFOHEADER directly. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10652
From: Esme Povirk <esme@codeweavers.com> --- dlls/gdiplus/tests/image.c | 57 +++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index 0c8f24bd768..674d80d8a77 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -272,6 +272,38 @@ static void test_Scan0(void) ok( !bm, "expected null bitmap\n" ); } +#define check_bitmap_bits(bitmap, width, height, stride, format, scan0, todo_scan0) \ + check_bitmap_bits_(bitmap, width, height, stride, format, scan0, todo_scan0, __FILE__, __LINE__); + +static void check_bitmap_bits_(GpBitmap *bitmap, UINT width, UINT height, int stride, + PixelFormat format, void* scan0, BOOL todo_scan0, const char* file, int line) +{ + GpStatus stat; + PixelFormat actual_format; + BitmapData lock; + + stat = GdipGetImagePixelFormat((GpImage*)bitmap, &actual_format); + ok_(file, line)(stat == Ok, "GdipGetImagePixelFormat failed, status=%i\n", stat); + ok_(file, line)(format == actual_format, "GdipGetImagePixelFormat returned format %x, expected %x\n", actual_format, format); + + stat = GdipBitmapLockBits(bitmap, NULL, 0, actual_format, &lock); + ok_(file, line)(stat == Ok, "GdipBitmapLockBits failed, status=%i\n", stat); + ok_(file, line)(width == lock.Width, "BitmapData.Width == %d, expected %d\n", lock.Width, width); + ok_(file, line)(height == lock.Height, "BitmapData.Height == %d, expected %d\n", lock.Height, height); +todo_wine_if(stride < 0) + ok_(file, line)(stride == lock.Stride, "BitmapData.Stride == %i, expected %i\n", lock.Stride, stride); + ok_(file, line)(format == lock.PixelFormat, "BitmapData.PixelFormat == %x, expected %x\n", lock.PixelFormat, format); + if (scan0) + { +todo_wine + ok_(file, line)(scan0 == lock.Scan0, "BitmapData.Scan0 == %p, expected %p\n", lock.Scan0, scan0); + } + ok_(file, line)(lock.Reserved == 0, "BitmapData.Reserved == %p, expected 0\n", (void*)lock.Reserved); + + stat = GdipBitmapUnlockBits(bitmap, &lock); + ok_(file, line)(stat == Ok, "GdipBitmapUnlockBits failed, status=%i\n", stat); +} + static void test_FromGdiDib(void) { GpBitmap *bm; @@ -279,7 +311,6 @@ static void test_FromGdiDib(void) BYTE buff[400]; BYTE rbmi[sizeof(BITMAPINFOHEADER)+256*sizeof(RGBQUAD)]; BITMAPINFO *bmi = (BITMAPINFO*)rbmi; - PixelFormat format; bm = NULL; @@ -306,9 +337,7 @@ static void test_FromGdiDib(void) ok(NULL != bm, "Expected bitmap to be initialized\n"); if (stat == Ok) { - stat = GdipGetImagePixelFormat((GpImage*)bm, &format); - expect(Ok, stat); - expect(PixelFormat32bppRGB, format); + check_bitmap_bits(bm, 10, 10, -40, PixelFormat32bppRGB, &buff[40*9], TRUE); GdipDisposeImage((GpImage*)bm); } @@ -319,9 +348,7 @@ static void test_FromGdiDib(void) ok(NULL != bm, "Expected bitmap to be initialized\n"); if (stat == Ok) { - stat = GdipGetImagePixelFormat((GpImage*)bm, &format); - expect(Ok, stat); - expect(PixelFormat24bppRGB, format); + check_bitmap_bits(bm, 10, 10, -32, PixelFormat24bppRGB, &buff[32*9], TRUE); GdipDisposeImage((GpImage*)bm); } @@ -332,9 +359,7 @@ static void test_FromGdiDib(void) ok(NULL != bm, "Expected bitmap to be initialized\n"); if (stat == Ok) { - stat = GdipGetImagePixelFormat((GpImage*)bm, &format); - expect(Ok, stat); - expect(PixelFormat16bppRGB555, format); + check_bitmap_bits(bm, 10, 10, -20, PixelFormat16bppRGB555, &buff[20*9], TRUE); GdipDisposeImage((GpImage*)bm); } @@ -345,9 +370,7 @@ static void test_FromGdiDib(void) ok(NULL != bm, "Expected bitmap to be initialized\n"); if (stat == Ok) { - stat = GdipGetImagePixelFormat((GpImage*)bm, &format); - expect(Ok, stat); - expect(PixelFormat8bppIndexed, format); + check_bitmap_bits(bm, 10, 10, -12, PixelFormat8bppIndexed, &buff[12*9], TRUE); GdipDisposeImage((GpImage*)bm); } @@ -358,9 +381,7 @@ static void test_FromGdiDib(void) ok(NULL != bm, "Expected bitmap to be initialized\n"); if (stat == Ok) { - stat = GdipGetImagePixelFormat((GpImage*)bm, &format); - expect(Ok, stat); - expect(PixelFormat4bppIndexed, format); + check_bitmap_bits(bm, 10, 10, -8, PixelFormat4bppIndexed, &buff[8*9], TRUE); GdipDisposeImage((GpImage*)bm); } @@ -371,9 +392,7 @@ static void test_FromGdiDib(void) ok(NULL != bm, "Expected bitmap to be initialized\n"); if (stat == Ok) { - stat = GdipGetImagePixelFormat((GpImage*)bm, &format); - expect(Ok, stat); - expect(PixelFormat1bppIndexed, format); + check_bitmap_bits(bm, 10, 10, -4, PixelFormat1bppIndexed, &buff[4*9], TRUE); GdipDisposeImage((GpImage*)bm); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10652
From: Esme Povirk <esme@codeweavers.com> --- dlls/gdiplus/tests/image.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index 674d80d8a77..92fa251aced 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -318,7 +318,7 @@ static void test_FromGdiDib(void) bmi->bmiHeader.biSize = sizeof(BITMAPINFOHEADER); bmi->bmiHeader.biWidth = 10; - bmi->bmiHeader.biHeight = 10; + bmi->bmiHeader.biHeight = -10; bmi->bmiHeader.biPlanes = 1; bmi->bmiHeader.biBitCount = 32; bmi->bmiHeader.biCompression = BI_RGB; @@ -332,6 +332,18 @@ static void test_FromGdiDib(void) stat = GdipCreateBitmapFromGdiDib(bmi, buff, NULL); expect(InvalidParameter, stat); + stat = GdipCreateBitmapFromGdiDib(bmi, buff, &bm); + expect(Ok, stat); + ok(NULL != bm, "Expected bitmap to be initialized\n"); + if (stat == Ok) + { + check_bitmap_bits(bm, 10, 10, 40, PixelFormat32bppRGB, buff, TRUE); + + GdipDisposeImage((GpImage*)bm); + } + + bmi->bmiHeader.biHeight = 10; + stat = GdipCreateBitmapFromGdiDib(bmi, buff, &bm); expect(Ok, stat); ok(NULL != bm, "Expected bitmap to be initialized\n"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10652
From: Esme Povirk <esme@codeweavers.com> --- dlls/gdiplus/image.c | 3 +++ dlls/gdiplus/tests/image.c | 42 +++++++++++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c index d8a63a1d39a..6e99cd30db6 100644 --- a/dlls/gdiplus/image.c +++ b/dlls/gdiplus/image.c @@ -1477,6 +1477,9 @@ GpStatus WINGDIPAPI GdipCreateBitmapFromGdiDib(GDIPCONST BITMAPINFO* info, if (!info || !bits || !bitmap) return InvalidParameter; + if (info->bmiHeader.biSize < sizeof(BITMAPINFOHEADER)) + return InvalidParameter; + hbm = CreateDIBSection(0, info, DIB_RGB_COLORS, &bmbits, NULL, 0); if (!hbm) return InvalidParameter; diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index 92fa251aced..39b48d65637 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -309,8 +309,9 @@ static void test_FromGdiDib(void) GpBitmap *bm; GpStatus stat; BYTE buff[400]; - BYTE rbmi[sizeof(BITMAPINFOHEADER)+256*sizeof(RGBQUAD)]; + BYTE rbmi[sizeof(BITMAPV5HEADER)+256*sizeof(RGBQUAD)]; BITMAPINFO *bmi = (BITMAPINFO*)rbmi; + BITMAPCOREINFO *bmci = (BITMAPCOREINFO*)rbmi; bm = NULL; @@ -409,9 +410,48 @@ static void test_FromGdiDib(void) GdipDisposeImage((GpImage*)bm); } + bmi->bmiHeader.biSize = sizeof(BITMAPV5HEADER); + stat = GdipCreateBitmapFromGdiDib(bmi, buff, &bm); + expect(Ok, stat); + ok(NULL != bm, "Expected bitmap to be initialized\n"); + if (stat == Ok) + { + check_bitmap_bits(bm, 10, 10, -4, PixelFormat1bppIndexed, &buff[4*9], TRUE); + + GdipDisposeImage((GpImage*)bm); + } + + bmi->bmiHeader.biSize = sizeof(BITMAPV4HEADER); + stat = GdipCreateBitmapFromGdiDib(bmi, buff, &bm); + expect(Ok, stat); + ok(NULL != bm, "Expected bitmap to be initialized\n"); + if (stat == Ok) + { + check_bitmap_bits(bm, 10, 10, -4, PixelFormat1bppIndexed, &buff[4*9], TRUE); + + GdipDisposeImage((GpImage*)bm); + } + +#if 0 + bmi->bmiHeader.biSize = sizeof(BITMAPV4HEADER)+4; + stat = GdipCreateBitmapFromGdiDib(bmi, buff, &bm); + expect(InvalidParameter, stat); // Native sometimes fails and sometimes succeeds + if (stat == Ok) + GdipDisposeImage((GpImage*)bm); +#endif + + bmi->bmiHeader.biSize = sizeof(BITMAPV4HEADER); bmi->bmiHeader.biBitCount = 0; stat = GdipCreateBitmapFromGdiDib(bmi, buff, &bm); expect(InvalidParameter, stat); + + bmci->bmciHeader.bcSize = sizeof(BITMAPCOREHEADER); + bmci->bmciHeader.bcWidth = 10; + bmci->bmciHeader.bcHeight = 10; + bmci->bmciHeader.bcPlanes = 1; + bmci->bmciHeader.bcBitCount = 1; + stat = GdipCreateBitmapFromGdiDib(bmi, buff, &bm); + expect(InvalidParameter, stat); } static void test_GetImageDimension(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10652
From: Esme Povirk <esme@codeweavers.com> --- dlls/gdiplus/tests/image.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index 39b48d65637..011bc116cd8 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -308,9 +308,10 @@ static void test_FromGdiDib(void) { GpBitmap *bm; GpStatus stat; - BYTE buff[400]; + BYTE buff[600]; BYTE rbmi[sizeof(BITMAPV5HEADER)+256*sizeof(RGBQUAD)]; BITMAPINFO *bmi = (BITMAPINFO*)rbmi; + BITMAPV4HEADER *bm4h = (BITMAPV4HEADER*)rbmi; BITMAPCOREINFO *bmci = (BITMAPCOREINFO*)rbmi; bm = NULL; @@ -355,6 +356,10 @@ static void test_FromGdiDib(void) GdipDisposeImage((GpImage*)bm); } + bmi->bmiHeader.biBitCount = 48; + stat = GdipCreateBitmapFromGdiDib(bmi, buff, &bm); + expect(InvalidParameter, stat); + bmi->bmiHeader.biBitCount = 24; stat = GdipCreateBitmapFromGdiDib(bmi, buff, &bm); expect(Ok, stat); @@ -445,6 +450,34 @@ static void test_FromGdiDib(void) stat = GdipCreateBitmapFromGdiDib(bmi, buff, &bm); expect(InvalidParameter, stat); + bm4h->bV4BitCount = 16; + bm4h->bV4V4Compression = BI_BITFIELDS; + bm4h->bV4RedMask = 0x7c00; + bm4h->bV4GreenMask = 0x3e0; + bm4h->bV4BlueMask = 0x1f; + stat = GdipCreateBitmapFromGdiDib(bmi, buff, &bm); + expect(Ok, stat); + ok(NULL != bm, "Expected bitmap to be initialized\n"); + if (stat == Ok) + { + check_bitmap_bits(bm, 10, 10, -20, PixelFormat16bppRGB555, &buff[20*9], TRUE); + + GdipDisposeImage((GpImage*)bm); + } + + bm4h->bV4RedMask = 0xf800; + bm4h->bV4GreenMask = 0x7e0; + bm4h->bV4BlueMask = 0x1f; + stat = GdipCreateBitmapFromGdiDib(bmi, buff, &bm); + expect(Ok, stat); + ok(NULL != bm, "Expected bitmap to be initialized\n"); + if (stat == Ok) + { + check_bitmap_bits(bm, 10, 10, -20, PixelFormat16bppRGB565, &buff[20*9], TRUE); + + GdipDisposeImage((GpImage*)bm); + } + bmci->bmciHeader.bcSize = sizeof(BITMAPCOREHEADER); bmci->bmciHeader.bcWidth = 10; bmci->bmciHeader.bcHeight = 10; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10652
From: Esme Povirk <esme@codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=40409 --- dlls/gdiplus/image.c | 65 +++++++++++++++++++++++++++++++------- dlls/gdiplus/tests/image.c | 2 -- 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c index 6e99cd30db6..e6b460e7a3c 100644 --- a/dlls/gdiplus/image.c +++ b/dlls/gdiplus/image.c @@ -1468,9 +1468,8 @@ GpStatus WINGDIPAPI GdipCreateBitmapFromGdiDib(GDIPCONST BITMAPINFO* info, VOID *bits, GpBitmap **bitmap) { DWORD height, stride; - HBITMAP hbm; - void *bmbits; - GpStatus status; + PixelFormat format; + BYTE *scan0; TRACE("(%p, %p, %p)\n", info, bits, bitmap); @@ -1480,20 +1479,62 @@ GpStatus WINGDIPAPI GdipCreateBitmapFromGdiDib(GDIPCONST BITMAPINFO* info, if (info->bmiHeader.biSize < sizeof(BITMAPINFOHEADER)) return InvalidParameter; - hbm = CreateDIBSection(0, info, DIB_RGB_COLORS, &bmbits, NULL, 0); - if (!hbm) - return InvalidParameter; - height = abs(info->bmiHeader.biHeight); stride = ((info->bmiHeader.biWidth * info->bmiHeader.biBitCount + 31) >> 3) & ~3; - TRACE("height %lu, stride %lu, image size %lu\n", height, stride, height * stride); + scan0 = bits; - memcpy(bmbits, bits, height * stride); + if (info->bmiHeader.biHeight > 0) + { + scan0 = scan0 + (height - 1) * stride; + stride = -stride; + } - status = GdipCreateBitmapFromHBITMAP(hbm, NULL, bitmap); - DeleteObject(hbm); + switch (info->bmiHeader.biBitCount) + { + case 1: + format = PixelFormat1bppIndexed; + break; + case 4: + format = PixelFormat4bppIndexed; + break; + case 8: + format = PixelFormat8bppIndexed; + break; + case 16: + { + if (info->bmiHeader.biCompression == BI_RGB) + { + format = PixelFormat16bppRGB555; + break; + } + if (info->bmiHeader.biCompression == BI_BITFIELDS && info->bmiHeader.biSize >= FIELD_OFFSET(BITMAPV4HEADER, bV4AlphaMask)) + { + const BITMAPV4HEADER *header = (const BITMAPV4HEADER*)info; + if (header->bV4RedMask == 0x7c00 && header->bV4GreenMask == 0x3e0 && header->bV4BlueMask == 0x1f) + { + format = PixelFormat16bppRGB555; + break; + } + if (header->bV4RedMask == 0xf800 && header->bV4GreenMask == 0x7e0 && header->bV4BlueMask == 0x1f) + { + format = PixelFormat16bppRGB565; + break; + } + } + return InvalidParameter; + } + case 24: + format = PixelFormat24bppRGB; + break; + case 32: + format = PixelFormat32bppRGB; + break; + default: + FIXME("don't know how to handle %d bpp\n", info->bmiHeader.biBitCount); + return InvalidParameter; + } - return status; + return GdipCreateBitmapFromScan0(info->bmiHeader.biWidth, height, stride, format, scan0, bitmap); } /* FIXME: no icm */ diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index 011bc116cd8..7346ba94880 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -290,12 +290,10 @@ static void check_bitmap_bits_(GpBitmap *bitmap, UINT width, UINT height, int st ok_(file, line)(stat == Ok, "GdipBitmapLockBits failed, status=%i\n", stat); ok_(file, line)(width == lock.Width, "BitmapData.Width == %d, expected %d\n", lock.Width, width); ok_(file, line)(height == lock.Height, "BitmapData.Height == %d, expected %d\n", lock.Height, height); -todo_wine_if(stride < 0) ok_(file, line)(stride == lock.Stride, "BitmapData.Stride == %i, expected %i\n", lock.Stride, stride); ok_(file, line)(format == lock.PixelFormat, "BitmapData.PixelFormat == %x, expected %x\n", lock.PixelFormat, format); if (scan0) { -todo_wine ok_(file, line)(scan0 == lock.Scan0, "BitmapData.Scan0 == %p, expected %p\n", lock.Scan0, scan0); } ok_(file, line)(lock.Reserved == 0, "BitmapData.Reserved == %p, expected 0\n", (void*)lock.Reserved); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10652
participants (2)
-
Esme Povirk -
Esme Povirk (＠madewokherd)