[PATCH v2 0/4] MR3657: gdiplus: GdipCreateBitmapFromHICON fixes part 2/2

[Jeffrey Smith (＠whydoubt)]

--
  v2: gdiplus: Fix GdipCreateBitmapFromHICON bitmap data.
      gdiplus/tests: Add test for bitmap locked data from GdipCreateBitmapFromHICON.
      gdiplus/tests: Add test for non-square icon with GdipCreateBitmapFromHICON.
      gdiplus/tests: Add test for 32 bpp icon with GdipCreateBitmapFromHICON.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3657

[PATCH v2 1/4] gdiplus/tests: Add test for 32 bpp icon with GdipCreateBitmapFromHICON.

[Jeff Smith]

From: Jeff Smith <whydoubt(a)gmail.com>

---
 dlls/gdiplus/tests/image.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c
index cbc07b877a9..a66be781128 100644
--- a/dlls/gdiplus/tests/image.c
+++ b/dlls/gdiplus/tests/image.c
@@ -1646,6 +1646,28 @@ static void test_fromhicon(void)
         GdipDisposeImage((GpImage*)bitmap);
     }
     DestroyIcon(hIcon);
+
+    /* 32 bpp icon */
+    hbmMask = CreateBitmap(16, 16, 1, 1, bmp_bits);
+    ok(hbmMask != 0, "CreateBitmap failed\n");
+    hbmColor = CreateBitmap(16, 16, 1, 32, bmp_bits);
+    ok(hbmColor != 0, "CreateBitmap failed\n");
+    info = iconinfo_base;
+    info.hbmMask = hbmMask;
+    info.hbmColor = hbmColor;
+    hIcon = CreateIconIndirect(&info);
+    ok(hIcon != 0, "CreateIconIndirect failed\n");
+    DeleteObject(hbmMask);
+    DeleteObject(hbmColor);
+
+    stat = GdipCreateBitmapFromHICON(hIcon, &bitmap);
+    expect(Ok, stat);
+    if(stat == Ok){
+        expect_image_properties((GpImage*)bitmap, 16, 16, __LINE__);
+        expect_rawformat(&ImageFormatMemoryBMP, (GpImage*)bitmap, __LINE__, FALSE);
+        GdipDisposeImage((GpImage*)bitmap);
+    }
+    DestroyIcon(hIcon);
 }
 
 /* 1x1 pixel png */
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/3657

[PATCH v2 2/4] gdiplus/tests: Add test for non-square icon with GdipCreateBitmapFromHICON.

[Jeff Smith]

From: Jeff Smith <whydoubt(a)gmail.com>

---
 dlls/gdiplus/tests/image.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c
index a66be781128..103a0c56a0f 100644
--- a/dlls/gdiplus/tests/image.c
+++ b/dlls/gdiplus/tests/image.c
@@ -1668,6 +1668,28 @@ static void test_fromhicon(void)
         GdipDisposeImage((GpImage*)bitmap);
     }
     DestroyIcon(hIcon);
+
+    /* non-square 32 bpp icon */
+    hbmMask = CreateBitmap(16, 8, 1, 1, bmp_bits);
+    ok(hbmMask != 0, "CreateBitmap failed\n");
+    hbmColor = CreateBitmap(16, 8, 1, 32, bmp_bits);
+    ok(hbmColor != 0, "CreateBitmap failed\n");
+    info = iconinfo_base;
+    info.hbmMask = hbmMask;
+    info.hbmColor = hbmColor;
+    hIcon = CreateIconIndirect(&info);
+    ok(hIcon != 0, "CreateIconIndirect failed\n");
+    DeleteObject(hbmMask);
+    DeleteObject(hbmColor);
+
+    stat = GdipCreateBitmapFromHICON(hIcon, &bitmap);
+    expect(Ok, stat);
+    if(stat == Ok){
+        expect_image_properties((GpImage*)bitmap, 16, 8, __LINE__);
+        expect_rawformat(&ImageFormatMemoryBMP, (GpImage*)bitmap, __LINE__, FALSE);
+        GdipDisposeImage((GpImage*)bitmap);
+    }
+    DestroyIcon(hIcon);
 }
 
 /* 1x1 pixel png */
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/3657

[PATCH v2 3/4] gdiplus/tests: Add test for bitmap locked data from GdipCreateBitmapFromHICON.

[Jeff Smith]

From: Jeff Smith <whydoubt(a)gmail.com>

---
 dlls/gdiplus/tests/image.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c
index 103a0c56a0f..f90375478e1 100644
--- a/dlls/gdiplus/tests/image.c
+++ b/dlls/gdiplus/tests/image.c
@@ -106,6 +106,26 @@ static void expect_image_properties(GpImage *image, UINT width, UINT height, int
     ok_(__FILE__, line)(format == PixelFormat32bppARGB, "Expected %d, got %d\n", PixelFormat32bppARGB, format);
 }
 
+static void expect_bitmap_locked_data(GpBitmap *bitmap, const BYTE *expect_bits,
+        UINT width, UINT height, UINT stride, int line)
+{
+    GpStatus stat;
+    BitmapData lockeddata;
+
+    memset(&lockeddata, 0x55, sizeof(lockeddata));
+    stat = GdipBitmapLockBits(bitmap, NULL, ImageLockModeRead, PixelFormat32bppARGB, &lockeddata);
+    ok_(__FILE__, line)(stat == Ok, "Expected %d, got %d\n", Ok, stat);
+    ok_(__FILE__, line)(lockeddata.Width == width, "Expected %d, got %d\n", width, lockeddata.Width);
+    ok_(__FILE__, line)(lockeddata.Height == height, "Expected %d, got %d\n", height, lockeddata.Height);
+    ok_(__FILE__, line)(lockeddata.Stride == stride, "Expected %d, got %d\n", stride, lockeddata.Stride);
+    ok_(__FILE__, line)(lockeddata.PixelFormat == PixelFormat32bppARGB,
+            "Expected %d, got %d\n", PixelFormat32bppARGB, lockeddata.PixelFormat);
+    todo_wine
+    ok_(__FILE__, line)(!memcmp(expect_bits, lockeddata.Scan0, lockeddata.Height * lockeddata.Stride),
+            "data mismatch\n");
+    GdipBitmapUnlockBits(bitmap, &lockeddata);
+}
+
 static BOOL get_encoder_clsid(LPCWSTR mime, GUID *format, CLSID *clsid)
 {
     GpStatus status;
@@ -1560,12 +1580,16 @@ static void test_testcontrol(void)
 
 static void test_fromhicon(void)
 {
-    static const BYTE bmp_bits[1024];
+    BYTE bmp_bits[1024], bmp_bits_masked[1024];
     HBITMAP hbmMask, hbmColor;
     ICONINFO info, iconinfo_base = {TRUE, 0, 0, 0, 0};
     HICON hIcon;
     GpStatus stat;
     GpBitmap *bitmap = NULL;
+    UINT i;
+
+    for (i = 0; i < sizeof(bmp_bits); ++i)
+        bmp_bits[i] = 111 * i;
 
     /* NULL */
     stat = GdipCreateBitmapFromHICON(NULL, NULL);
@@ -1660,11 +1684,18 @@ static void test_fromhicon(void)
     DeleteObject(hbmMask);
     DeleteObject(hbmColor);
 
+    for (i = 0; i < sizeof(bmp_bits_masked)/sizeof(ARGB); i++)
+    {
+        BYTE mask = bmp_bits[i / 8] & (1 << (7 - (i % 8)));
+        ((ARGB *)bmp_bits_masked)[i] = mask ? 0 : ((ARGB *)bmp_bits)[i] | 0xff000000;
+    }
+
     stat = GdipCreateBitmapFromHICON(hIcon, &bitmap);
     expect(Ok, stat);
     if(stat == Ok){
         expect_image_properties((GpImage*)bitmap, 16, 16, __LINE__);
         expect_rawformat(&ImageFormatMemoryBMP, (GpImage*)bitmap, __LINE__, FALSE);
+        expect_bitmap_locked_data(bitmap, bmp_bits_masked, 16, 16, 64, __LINE__);
         GdipDisposeImage((GpImage*)bitmap);
     }
     DestroyIcon(hIcon);
@@ -1687,6 +1718,7 @@ static void test_fromhicon(void)
     if(stat == Ok){
         expect_image_properties((GpImage*)bitmap, 16, 8, __LINE__);
         expect_rawformat(&ImageFormatMemoryBMP, (GpImage*)bitmap, __LINE__, FALSE);
+        expect_bitmap_locked_data(bitmap, bmp_bits_masked, 16, 8, 64, __LINE__);
         GdipDisposeImage((GpImage*)bitmap);
     }
     DestroyIcon(hIcon);
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/3657

[PATCH v2 4/4] gdiplus: Fix GdipCreateBitmapFromHICON bitmap data.

[Jeff Smith]

From: Jeff Smith <whydoubt(a)gmail.com>

---
 dlls/gdiplus/image.c       | 78 ++++++++++++++++++++------------------
 dlls/gdiplus/tests/image.c |  1 -
 2 files changed, 41 insertions(+), 38 deletions(-)

diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c
index 7640c934902..9b66edbd22b 100644
--- a/dlls/gdiplus/image.c
+++ b/dlls/gdiplus/image.c
@@ -1626,12 +1626,13 @@ GpStatus WINGDIPAPI GdipCreateBitmapFromHICON(HICON hicon, GpBitmap** bitmap)
     GpRect rect;
     BitmapData lockeddata;
     HDC screendc;
-    BOOL has_alpha;
     int x, y;
     BITMAPINFOHEADER bih;
     DWORD *src;
     BYTE *dst_row;
     DWORD *dst;
+    BYTE *bits;
+    int mask_scanlines = 0, color_scanlines = 0;
 
     TRACE("%p, %p\n", hicon, bitmap);
 
@@ -1682,52 +1683,55 @@ GpStatus WINGDIPAPI GdipCreateBitmapFromHICON(HICON hicon, GpBitmap** bitmap)
     bih.biClrUsed = 0;
     bih.biClrImportant = 0;
 
-    screendc = CreateCompatibleDC(0);
-    GetDIBits(screendc, iinfo.hbmColor, 0, height, lockeddata.Scan0, (BITMAPINFO*)&bih, DIB_RGB_COLORS);
-
-    if (bm.bmBitsPixel == 32)
+    bits = heap_alloc(height * stride);
+    if (!bits)
     {
-        has_alpha = FALSE;
-
-        /* If any pixel has a non-zero alpha, ignore hbmMask */
-        src = (DWORD*)lockeddata.Scan0;
-        for (y=0; y<height && !has_alpha; y++)
-            for (x=0; x<width && !has_alpha; x++)
-                if ((*src++ & 0xff000000) != 0)
-                    has_alpha = TRUE;
+        DeleteObject(iinfo.hbmColor);
+        DeleteObject(iinfo.hbmMask);
+        GdipBitmapUnlockBits(*bitmap, &lockeddata);
+        GdipDisposeImage(&(*bitmap)->image);
+        return OutOfMemory;
     }
-    else has_alpha = FALSE;
 
-    if (!has_alpha)
+    screendc = CreateCompatibleDC(0);
+    if (screendc)
     {
-        BYTE *bits = heap_alloc(height * stride);
-
-        /* read alpha data from the mask - the mask can safely be assumed to exist */
-        GetDIBits(screendc, iinfo.hbmMask, 0, height, bits, (BITMAPINFO*)&bih, DIB_RGB_COLORS);
+        color_scanlines = GetDIBits(screendc, iinfo.hbmColor, 0, height, lockeddata.Scan0,
+            (BITMAPINFO*)&bih, DIB_RGB_COLORS);
+        mask_scanlines = GetDIBits(screendc, iinfo.hbmMask, 0, height, bits,
+            (BITMAPINFO*)&bih, DIB_RGB_COLORS);
+        DeleteDC(screendc);
+    }
 
-        src = (DWORD*)bits;
-        dst_row = lockeddata.Scan0;
-        for (y=0; y<height; y++)
-        {
-            dst = (DWORD*)dst_row;
-            for (x=0; x<width; x++)
-            {
-                DWORD src_value = *src++;
-                if (src_value)
-                    *dst++ = 0;
-                else
-                    *dst++ |= 0xff000000;
-            }
-            dst_row += lockeddata.Stride;
-        }
+    DeleteObject(iinfo.hbmColor);
+    DeleteObject(iinfo.hbmMask);
 
+    if (!screendc || ((color_scanlines == 0 || mask_scanlines == 0) &&
+                      GetLastError() == ERROR_INVALID_PARAMETER))
+    {
         heap_free(bits);
+        GdipBitmapUnlockBits(*bitmap, &lockeddata);
+        GdipDisposeImage(&(*bitmap)->image);
+        return GenericError;
     }
 
-    DeleteDC(screendc);
+    src = (DWORD*)bits;
+    dst_row = lockeddata.Scan0;
+    for (y=0; y<height; y++)
+    {
+        dst = (DWORD*)dst_row;
+        for (x=0; x<width; x++)
+        {
+            DWORD src_value = *src++;
+            if (src_value)
+                *dst++ = 0;
+            else
+                *dst++ |= 0xff000000;
+        }
+        dst_row += lockeddata.Stride;
+    }
 
-    DeleteObject(iinfo.hbmColor);
-    DeleteObject(iinfo.hbmMask);
+    heap_free(bits);
 
     GdipBitmapUnlockBits(*bitmap, &lockeddata);
 
diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c
index f90375478e1..6f497e416b0 100644
--- a/dlls/gdiplus/tests/image.c
+++ b/dlls/gdiplus/tests/image.c
@@ -120,7 +120,6 @@ static void expect_bitmap_locked_data(GpBitmap *bitmap, const BYTE *expect_bits,
     ok_(__FILE__, line)(lockeddata.Stride == stride, "Expected %d, got %d\n", stride, lockeddata.Stride);
     ok_(__FILE__, line)(lockeddata.PixelFormat == PixelFormat32bppARGB,
             "Expected %d, got %d\n", PixelFormat32bppARGB, lockeddata.PixelFormat);
-    todo_wine
     ok_(__FILE__, line)(!memcmp(expect_bits, lockeddata.Scan0, lockeddata.Height * lockeddata.Stride),
             "data mismatch\n");
     GdipBitmapUnlockBits(bitmap, &lockeddata);
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/3657