[PATCH 0/5] MR8261: comctl32/tests: Check RGB value in test_alpha.
We only check alpha value before, now also check red, green, blue. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261
From: Ziqing Hui <zhui(a)codeweavers.com> We only check alpha value before, now also check red, green, blue. --- dlls/comctl32/tests/imagelist.c | 126 ++++++++++++++++++-------------- 1 file changed, 73 insertions(+), 53 deletions(-) diff --git a/dlls/comctl32/tests/imagelist.c b/dlls/comctl32/tests/imagelist.c index a1da365ece8..e8917562609 100644 --- a/dlls/comctl32/tests/imagelist.c +++ b/dlls/comctl32/tests/imagelist.c @@ -42,6 +42,7 @@ #include "resources.h" #define IMAGELIST_MAGIC (('L' << 8) | 'I') +#define get_alpha(argb) (((argb) & 0xff000000) >> 24) #pragma pack(push,2) /* Header used by ImageList_Read() and ImageList_Write() */ @@ -278,6 +279,44 @@ static void check_bits(HWND hwnd, HIMAGELIST himl, int idx, int size, dump_bits(bits, checkbits, size); } +static UINT32 get_premultiplied_argb(UINT32 pixel) +{ + UINT32 alpha = (pixel & 0xff000000) >> 24; + return ((pixel & 0xff000000) + | (((pixel & 0x00ff0000) * alpha / 0xff) & 0x00ff0000) + | (((pixel & 0x0000ff00) * alpha / 0xff) & 0x0000ff00) + | (((pixel & 0x000000ff) * alpha / 0xff))); +} + +static void image_list_get_image_bits_by_draw(HIMAGELIST image_list, int index, UINT32 *bits) +{ + BITMAPINFO bitmap_info = {}; + int ret, width, height; + void *bitmap_bits; + HBITMAP hbm_dst; + HDC hdc_dst; + + ret = pImageList_GetIconSize(image_list, &width, &height); + ok(ret, "ImageList_GetIconSize failed.\n"); + + bitmap_info.bmiHeader.biSize = sizeof(BITMAPINFOHEADER); + bitmap_info.bmiHeader.biWidth = width; + bitmap_info.bmiHeader.biHeight = -height; + bitmap_info.bmiHeader.biPlanes = 1; + bitmap_info.bmiHeader.biBitCount = 32; + bitmap_info.bmiHeader.biCompression = BI_RGB; + + hdc_dst = CreateCompatibleDC(0); + hbm_dst = CreateDIBSection(hdc_dst, &bitmap_info, DIB_RGB_COLORS, &bitmap_bits, NULL, 0); + SelectObject(hdc_dst, hbm_dst); + + pImageList_Draw(image_list, index, hdc_dst, 0, 0, ILD_TRANSPARENT); + memcpy(bits, bitmap_bits, (size_t)(width * height * 32 / 8)); + + DeleteObject(hbm_dst); + DeleteDC(hdc_dst); +} + static void test_begindrag(void) { HIMAGELIST himl = createImageList(7,13); @@ -2388,25 +2427,6 @@ static void test_loadimage(void) pImageList_Destroy( list ); } -#define GetAValue(argb) ((BYTE) ((argb) >> 24)) - -static void get_image_bits(HIMAGELIST himl, int index, int width, int height, UINT32 *bits) -{ - HBITMAP hbm_dst; - void *bitmap_bits; - BITMAPINFO bitmap_info = {{sizeof(BITMAPINFOHEADER), width, height, 1, 32, BI_RGB, 0, 0, 0, 0, 0}}; - - HDC hdc_dst = CreateCompatibleDC(0); - hbm_dst = CreateDIBSection(hdc_dst, &bitmap_info, DIB_RGB_COLORS, &bitmap_bits, NULL, 0); - SelectObject(hdc_dst, hbm_dst); - - pImageList_Draw(himl, index, hdc_dst, 0, 0, ILD_TRANSPARENT); - memcpy(bits, bitmap_bits, (size_t)(width * height * 32 / 8)); - - DeleteObject(hbm_dst); - DeleteDC(hdc_dst); -} - static void test_alpha(void) { /* each line is a 2*1 bitmap */ @@ -2429,36 +2449,38 @@ static void test_alpha(void) }; const static BYTE mask_bits = 0xAA; - int i, ret; - HDC hdc; + UINT32 bits[2], expected[2]; HBITMAP hbm_test, hbm_mask; + const UINT32 *bitmap_bits; HIMAGELIST himl; - UINT32 bits[2]; + int i, ret; + HDC hdc; hdc = CreateCompatibleDC(0); himl = pImageList_Create(2, 1, ILC_COLOR32 | ILC_MASK, 0, 15); for (i = 0; i < ARRAY_SIZE(test_bitmaps); i += 2) { - hbm_test = create_test_bitmap(hdc, 2, 1, 32, test_bitmaps + i); - ret = pImageList_AddMasked(himl, hbm_test, RGB(0x65,0x43,0x21)); + bitmap_bits = test_bitmaps + i; + + hbm_test = create_test_bitmap(hdc, 2, 1, 32, bitmap_bits); + ret = pImageList_AddMasked(himl, hbm_test, RGB(0x65, 0x43, 0x21)); ok(ret == i / 2, "ImageList_AddMasked returned %d, expected %d\n", ret, i / 2); DeleteObject(hbm_test); - get_image_bits(himl, i / 2, 2, 1, bits); - ok(GetAValue(bits[0]) == GetAValue(test_bitmaps[i]) && GetAValue(bits[1]) == GetAValue(test_bitmaps[i + 1]), - "Bitmap [%08X, %08X] returned alpha value [%02X, %02X], expected [%02X, %02X]\n", - test_bitmaps[i], test_bitmaps[i + 1], GetAValue(bits[0]), GetAValue(bits[1]), - GetAValue(test_bitmaps[i]), GetAValue(test_bitmaps[i + 1])); - - /* If all alpha values are zero, the image is considered to have no alpha and gets masked */ - if (!GetAValue(bits[0]) && !GetAValue(bits[1])) - ok(bits[0] == (test_bitmaps[i] == 0x654321 ? 0 : test_bitmaps[i]) && - bits[1] == (test_bitmaps[i + 1] == 0x654321 ? 0 : test_bitmaps[i + 1]), - "Bitmap [%08X, %08X] returned [%08X, %08X], expected [%08X, %08X]\n", - test_bitmaps[i], test_bitmaps[i + 1], bits[0], bits[1], - test_bitmaps[i] == 0x654321 ? 0 : test_bitmaps[i], - test_bitmaps[i + 1] == 0x654321 ? 0 : test_bitmaps[i + 1]); + expected[0] = get_premultiplied_argb(bitmap_bits[0]); + expected[1] = get_premultiplied_argb(bitmap_bits[1]); + /* If all alpha values are zero, the image is considered to have no alpha and gets masked. */ + if (!get_alpha(bitmap_bits[0]) && !get_alpha(bitmap_bits[1])) + { + expected[0] = (bitmap_bits[0] == 0x654321 ? 0 : bitmap_bits[0]); + expected[1] = (bitmap_bits[1] == 0x654321 ? 0 : bitmap_bits[1]); + } + + image_list_get_image_bits_by_draw(himl, i / 2, bits); + ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), + "Bitmap [%08X, %08X] returned bits [%08X, %08X], expected [%08X, %08X].\n", + bitmap_bits[0], bitmap_bits[1], bits[0], bits[1], expected[0], expected[1]); } pImageList_Destroy(himl); @@ -2467,28 +2489,26 @@ static void test_alpha(void) for (i = 0; i < ARRAY_SIZE(test_bitmaps); i += 2) { + bitmap_bits = test_bitmaps + i; + hbm_test = create_test_bitmap(hdc, 2, 1, 32, test_bitmaps + i); ret = pImageList_Add(himl, hbm_test, hbm_mask); ok(ret == i / 2, "ImageList_Add returned %d, expected %d\n", ret, i / 2); DeleteObject(hbm_test); - get_image_bits(himl, i / 2, 2, 1, bits); - ok(GetAValue(bits[0]) == GetAValue(test_bitmaps[i]) && GetAValue(bits[1]) == GetAValue(test_bitmaps[i + 1]), - "Bitmap [%08X, %08X] returned alpha value [%02X, %02X], expected [%02X, %02X]\n", - test_bitmaps[i], test_bitmaps[i + 1], GetAValue(bits[0]), GetAValue(bits[1]), - GetAValue(test_bitmaps[i]), GetAValue(test_bitmaps[i + 1])); - - /* If all alpha values are zero, the image is considered to have no alpha and gets masked */ - if (!GetAValue(bits[0]) && !GetAValue(bits[1])) - ok(!bits[0] && bits[1] == test_bitmaps[i + 1], - "Bitmap [%08X, %08X] returned [%08X, %08X], expected [%08X, %08X]\n", - test_bitmaps[i], test_bitmaps[i + 1], bits[0], bits[1], 0, test_bitmaps[i + 1]); - else + expected[0] = get_premultiplied_argb(bitmap_bits[0]); + expected[1] = get_premultiplied_argb(bitmap_bits[1]); + /* If all alpha values are zero, the image is considered to have no alpha and gets masked. */ + if (!get_alpha(bitmap_bits[0]) && !get_alpha(bitmap_bits[1])) { - if (GetAValue(bits[0]) >= 0x80) - ok(bits[0] & 0x00FFFFFF, "Bitmap [%08X, %08X] has alpha and masked first pixel [%08X]\n", - test_bitmaps[i], test_bitmaps[i + 1], bits[0]); + expected[0] = 0; + expected[1] = bitmap_bits[1]; } + + image_list_get_image_bits_by_draw(himl, i / 2, bits); + ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), + "Bitmap [%08X, %08X] returned bits [%08X, %08X], expected [%08X, %08X].\n", + bitmap_bits[0], bitmap_bits[1], bits[0], bits[1], expected[0], expected[1]); } pImageList_Destroy(himl); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8261
From: Ziqing Hui <zhui(a)codeweavers.com> We use ImageList_Draw and test the draw result before. Now adding tests which get bitmap bits directly. --- dlls/comctl32/tests/imagelist.c | 65 +++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/dlls/comctl32/tests/imagelist.c b/dlls/comctl32/tests/imagelist.c index e8917562609..9f48aea4812 100644 --- a/dlls/comctl32/tests/imagelist.c +++ b/dlls/comctl32/tests/imagelist.c @@ -288,6 +288,59 @@ static UINT32 get_premultiplied_argb(UINT32 pixel) | (((pixel & 0x000000ff) * alpha / 0xff))); } +/* Remember to free the bits after usage. */ +static void bitmap_get_bits(HBITMAP bitmap, UINT32 **bits) +{ + BITMAPINFO bitmap_info; + UINT32 image_size; + BITMAP bm; + HDC hdc; + int ret; + + ret = GetObjectW(bitmap, sizeof(bm), &bm); + ok(ret, "GetObjectW failed.\n"); + + image_size = bm.bmWidth * bm.bmHeight * 4; + *bits = calloc(1, image_size); + ok(!!*bits, "Failed to allocate buffer of %u bytes.\n", image_size); + + bitmap_info.bmiHeader.biSize = sizeof(BITMAPINFOHEADER); + bitmap_info.bmiHeader.biWidth = bm.bmWidth; + bitmap_info.bmiHeader.biHeight = -bm.bmHeight; + bitmap_info.bmiHeader.biPlanes = 1; + bitmap_info.bmiHeader.biBitCount = 32; + bitmap_info.bmiHeader.biCompression = BI_RGB; + bitmap_info.bmiHeader.biSizeImage = image_size; + bitmap_info.bmiHeader.biXPelsPerMeter = 0; + bitmap_info.bmiHeader.biYPelsPerMeter = 0; + bitmap_info.bmiHeader.biClrUsed = 0; + bitmap_info.bmiHeader.biClrImportant = 0; + + hdc = CreateCompatibleDC(NULL); + ok(!!hdc, "CreateCompatibleDC failed.\n"); + ret = GetDIBits(hdc, bitmap, 0, bm.bmHeight, *bits, &bitmap_info, DIB_RGB_COLORS); + ok(ret, "GetDIBits failed.\n"); + + DeleteDC(hdc); +} + +static void image_list_get_image_bits_by_bitmap(HIMAGELIST image_list, int index, UINT32 *bits) +{ + int ret, width, height; + IMAGEINFO image_info; + UINT32 *image_bits; + + ret = pImageList_GetImageInfo(image_list, index, &image_info); + ok(ret, "ImageList_GetImageInfo failed.\n"); + ret = pImageList_GetIconSize(image_list, &width, &height); + ok(ret, "ImageList_GetIconSize failed.\n"); + + bitmap_get_bits(image_info.hbmImage, &image_bits); + memcpy(bits, image_bits + width * height * index, width * height * 4); + + free(image_bits); +} + static void image_list_get_image_bits_by_draw(HIMAGELIST image_list, int index, UINT32 *bits) { BITMAPINFO bitmap_info = {}; @@ -2477,6 +2530,12 @@ static void test_alpha(void) expected[1] = (bitmap_bits[1] == 0x654321 ? 0 : bitmap_bits[1]); } + image_list_get_image_bits_by_bitmap(himl, i / 2, bits); + todo_wine_if(i != 0 && i != 2 && i != 4 && i != 6 && i != 12 && i != 18) + ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), + "Bitmap [%08X, %08X] returned bits [%08X, %08X], expected [%08X, %08X].\n", + bitmap_bits[0], bitmap_bits[1], bits[0], bits[1], expected[0], expected[1]); + image_list_get_image_bits_by_draw(himl, i / 2, bits); ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), "Bitmap [%08X, %08X] returned bits [%08X, %08X], expected [%08X, %08X].\n", @@ -2505,6 +2564,12 @@ static void test_alpha(void) expected[1] = bitmap_bits[1]; } + image_list_get_image_bits_by_bitmap(himl, i / 2, bits); + todo_wine_if(i != 0 && i != 2 && i != 4 && i != 6 && i != 18) + ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), + "Bitmap [%08X, %08X] returned bits [%08X, %08X], expected [%08X, %08X].\n", + bitmap_bits[0], bitmap_bits[1], bits[0], bits[1], expected[0], expected[1]); + image_list_get_image_bits_by_draw(himl, i / 2, bits); ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), "Bitmap [%08X, %08X] returned bits [%08X, %08X], expected [%08X, %08X].\n", -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8261
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/comctl32/tests/imagelist.c | 66 +++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/dlls/comctl32/tests/imagelist.c b/dlls/comctl32/tests/imagelist.c index 9f48aea4812..c40382b09ad 100644 --- a/dlls/comctl32/tests/imagelist.c +++ b/dlls/comctl32/tests/imagelist.c @@ -288,6 +288,32 @@ static UINT32 get_premultiplied_argb(UINT32 pixel) | (((pixel & 0x000000ff) * alpha / 0xff))); } +static UINT32 get_alpha_blended(UINT32 dst, UINT32 src) +{ + UINT32 src_a, src_r, src_g, src_b; + UINT32 dst_a, dst_r, dst_g, dst_b; + UINT32 ret_a, ret_r, ret_g, ret_b; + + src = get_premultiplied_argb(src); + + src_a = (src & 0xff000000) >> 24; + src_r = (src & 0x00ff0000) >> 16; + src_g = (src & 0x0000ff00) >> 8; + src_b = (src & 0x000000ff); + + dst_a = (dst & 0xff000000) >> 24; + dst_r = (dst & 0x00ff0000) >> 16; + dst_g = (dst & 0x0000ff00) >> 8; + dst_b = (dst & 0x000000ff); + + ret_a = dst_a ? (src_a + (0xff - src_a) * dst_a / 0xff) : 0; + ret_r = src_r + (0xff - src_a) * dst_r / 0xff; + ret_g = src_g + (0xff - src_a) * dst_g / 0xff; + ret_b = src_b + (0xff - src_a) * dst_b / 0xff; + + return (ret_a << 24) | (ret_r << 16) | (ret_g << 8) | ret_b; +} + /* Remember to free the bits after usage. */ static void bitmap_get_bits(HBITMAP bitmap, UINT32 **bits) { @@ -2510,6 +2536,8 @@ static void test_alpha(void) HDC hdc; hdc = CreateCompatibleDC(0); + + /* Test ImageList_AddMasked with alpha. */ himl = pImageList_Create(2, 1, ILC_COLOR32 | ILC_MASK, 0, 15); for (i = 0; i < ARRAY_SIZE(test_bitmaps); i += 2) @@ -2543,6 +2571,8 @@ static void test_alpha(void) } pImageList_Destroy(himl); + + /* Test ImageList_Add with alpha. */ hbm_mask = CreateBitmap(2, 1, 1, 1, &mask_bits); himl = pImageList_Create(2, 1, ILC_COLOR32 | ILC_MASK, 0, 15); @@ -2578,6 +2608,42 @@ static void test_alpha(void) pImageList_Destroy(himl); DeleteObject(hbm_mask); + + /* Test adding 32bpp images with alpha to 24bpp image list. */ + himl = pImageList_Create(2, 1, ILC_COLOR24 | ILC_MASK, 0, 15); + + for (i = 0; i < ARRAY_SIZE(test_bitmaps); i += 2) + { + bitmap_bits = test_bitmaps + i; + + hbm_test = create_test_bitmap(hdc, 2, 1, 32, test_bitmaps + i); + ret = pImageList_Add(himl, hbm_test, NULL); + ok(ret == i / 2, "ImageList_Add returned %d, expected %d\n", ret, i / 2); + DeleteObject(hbm_test); + + expected[0] = bitmap_bits[0]; + expected[1] = bitmap_bits[1]; + if (get_alpha(bitmap_bits[0]) || get_alpha(bitmap_bits[1])) + { + expected[0] = get_alpha_blended(0x00ffffff, bitmap_bits[0]); + expected[1] = get_alpha_blended(0x00ffffff, bitmap_bits[1]); + } + + image_list_get_image_bits_by_bitmap(himl, i / 2, bits); + todo_wine_if(i != 0 && i != 2 && i != 4 && i != 6 && i != 18) + ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), + "Bitmap [%08X, %08X] returned bits [%08X, %08X], expected [%08X, %08X].\n", + bitmap_bits[0], bitmap_bits[1], bits[0], bits[1], expected[0], expected[1]); + + image_list_get_image_bits_by_draw(himl, i / 2, bits); + todo_wine_if(i != 0 && i != 2 && i != 4 && i != 6 && i != 18) + ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), + "Bitmap [%08X, %08X] returned bits [%08X, %08X], expected [%08X, %08X].\n", + bitmap_bits[0], bitmap_bits[1], bits[0], bits[1], expected[0], expected[1]); + } + + pImageList_Destroy(himl); + DeleteDC(hdc); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8261
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/comctl32/tests/imagelist.c | 36 ++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/dlls/comctl32/tests/imagelist.c b/dlls/comctl32/tests/imagelist.c index c40382b09ad..c870467da4a 100644 --- a/dlls/comctl32/tests/imagelist.c +++ b/dlls/comctl32/tests/imagelist.c @@ -2544,6 +2544,8 @@ static void test_alpha(void) { bitmap_bits = test_bitmaps + i; + winetest_push_context("Bitmap [%08X, %08X]", bitmap_bits[0], bitmap_bits[1]); + hbm_test = create_test_bitmap(hdc, 2, 1, 32, bitmap_bits); ret = pImageList_AddMasked(himl, hbm_test, RGB(0x65, 0x43, 0x21)); ok(ret == i / 2, "ImageList_AddMasked returned %d, expected %d\n", ret, i / 2); @@ -2561,13 +2563,15 @@ static void test_alpha(void) image_list_get_image_bits_by_bitmap(himl, i / 2, bits); todo_wine_if(i != 0 && i != 2 && i != 4 && i != 6 && i != 12 && i != 18) ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), - "Bitmap [%08X, %08X] returned bits [%08X, %08X], expected [%08X, %08X].\n", - bitmap_bits[0], bitmap_bits[1], bits[0], bits[1], expected[0], expected[1]); + "Got bits [%08X, %08X], expected [%08X, %08X].\n", + bits[0], bits[1], expected[0], expected[1]); image_list_get_image_bits_by_draw(himl, i / 2, bits); ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), - "Bitmap [%08X, %08X] returned bits [%08X, %08X], expected [%08X, %08X].\n", - bitmap_bits[0], bitmap_bits[1], bits[0], bits[1], expected[0], expected[1]); + "Got bits [%08X, %08X], expected [%08X, %08X].\n", + bits[0], bits[1], expected[0], expected[1]); + + winetest_pop_context(); } pImageList_Destroy(himl); @@ -2580,6 +2584,8 @@ static void test_alpha(void) { bitmap_bits = test_bitmaps + i; + winetest_push_context("Bitmap [%08X, %08X]", bitmap_bits[0], bitmap_bits[1]); + hbm_test = create_test_bitmap(hdc, 2, 1, 32, test_bitmaps + i); ret = pImageList_Add(himl, hbm_test, hbm_mask); ok(ret == i / 2, "ImageList_Add returned %d, expected %d\n", ret, i / 2); @@ -2597,13 +2603,15 @@ static void test_alpha(void) image_list_get_image_bits_by_bitmap(himl, i / 2, bits); todo_wine_if(i != 0 && i != 2 && i != 4 && i != 6 && i != 18) ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), - "Bitmap [%08X, %08X] returned bits [%08X, %08X], expected [%08X, %08X].\n", - bitmap_bits[0], bitmap_bits[1], bits[0], bits[1], expected[0], expected[1]); + "Got bits [%08X, %08X], expected [%08X, %08X].\n", + bits[0], bits[1], expected[0], expected[1]); image_list_get_image_bits_by_draw(himl, i / 2, bits); ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), - "Bitmap [%08X, %08X] returned bits [%08X, %08X], expected [%08X, %08X].\n", - bitmap_bits[0], bitmap_bits[1], bits[0], bits[1], expected[0], expected[1]); + "Got bits [%08X, %08X], expected [%08X, %08X].\n", + bits[0], bits[1], expected[0], expected[1]); + + winetest_pop_context(); } pImageList_Destroy(himl); @@ -2616,6 +2624,8 @@ static void test_alpha(void) { bitmap_bits = test_bitmaps + i; + winetest_push_context("Bitmap [%08X, %08X]", bitmap_bits[0], bitmap_bits[1]); + hbm_test = create_test_bitmap(hdc, 2, 1, 32, test_bitmaps + i); ret = pImageList_Add(himl, hbm_test, NULL); ok(ret == i / 2, "ImageList_Add returned %d, expected %d\n", ret, i / 2); @@ -2632,14 +2642,16 @@ static void test_alpha(void) image_list_get_image_bits_by_bitmap(himl, i / 2, bits); todo_wine_if(i != 0 && i != 2 && i != 4 && i != 6 && i != 18) ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), - "Bitmap [%08X, %08X] returned bits [%08X, %08X], expected [%08X, %08X].\n", - bitmap_bits[0], bitmap_bits[1], bits[0], bits[1], expected[0], expected[1]); + "Got bits [%08X, %08X], expected [%08X, %08X].\n", + bits[0], bits[1], expected[0], expected[1]); image_list_get_image_bits_by_draw(himl, i / 2, bits); todo_wine_if(i != 0 && i != 2 && i != 4 && i != 6 && i != 18) ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), - "Bitmap [%08X, %08X] returned bits [%08X, %08X], expected [%08X, %08X].\n", - bitmap_bits[0], bitmap_bits[1], bits[0], bits[1], expected[0], expected[1]); + "Got bits [%08X, %08X], expected [%08X, %08X].\n", + bits[0], bits[1], expected[0], expected[1]); + + winetest_pop_context(); } pImageList_Destroy(himl); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8261
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/comctl32/tests/imagelist.c | 38 +++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/dlls/comctl32/tests/imagelist.c b/dlls/comctl32/tests/imagelist.c index c870467da4a..e73e333931e 100644 --- a/dlls/comctl32/tests/imagelist.c +++ b/dlls/comctl32/tests/imagelist.c @@ -2528,10 +2528,14 @@ static void test_alpha(void) }; const static BYTE mask_bits = 0xAA; + IImageList2 *image_list_iface; UINT32 bits[2], expected[2]; HBITMAP hbm_test, hbm_mask; const UINT32 *bitmap_bits; + DWORD flag, expected_flag; HIMAGELIST himl; + BOOL has_alpha; + HRESULT hr; int i, ret; HDC hdc; @@ -2540,6 +2544,9 @@ static void test_alpha(void) /* Test ImageList_AddMasked with alpha. */ himl = pImageList_Create(2, 1, ILC_COLOR32 | ILC_MASK, 0, 15); + hr = pHIMAGELIST_QueryInterface(himl, &IID_IImageList2, (void **)&image_list_iface); + ok(hr == S_OK, "QueryInterface returned %#lx.\n", hr); + for (i = 0; i < ARRAY_SIZE(test_bitmaps); i += 2) { bitmap_bits = test_bitmaps + i; @@ -2551,15 +2558,22 @@ static void test_alpha(void) ok(ret == i / 2, "ImageList_AddMasked returned %d, expected %d\n", ret, i / 2); DeleteObject(hbm_test); + has_alpha = get_alpha(bitmap_bits[0]) || get_alpha(bitmap_bits[1]); expected[0] = get_premultiplied_argb(bitmap_bits[0]); expected[1] = get_premultiplied_argb(bitmap_bits[1]); + expected_flag = ILIF_ALPHA; /* If all alpha values are zero, the image is considered to have no alpha and gets masked. */ - if (!get_alpha(bitmap_bits[0]) && !get_alpha(bitmap_bits[1])) + if (!has_alpha) { expected[0] = (bitmap_bits[0] == 0x654321 ? 0 : bitmap_bits[0]); expected[1] = (bitmap_bits[1] == 0x654321 ? 0 : bitmap_bits[1]); + expected_flag = 0; } + hr = IImageList2_GetItemFlags(image_list_iface, i / 2, &flag); + ok(hr == S_OK, "GetItemFlags returned %#lx.\n", hr); + ok(flag == expected_flag, "Unexpected flag %#lx, expected %#lx.\n", flag, expected_flag); + image_list_get_image_bits_by_bitmap(himl, i / 2, bits); todo_wine_if(i != 0 && i != 2 && i != 4 && i != 6 && i != 12 && i != 18) ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), @@ -2574,12 +2588,16 @@ static void test_alpha(void) winetest_pop_context(); } + IImageList2_Release(image_list_iface); pImageList_Destroy(himl); /* Test ImageList_Add with alpha. */ hbm_mask = CreateBitmap(2, 1, 1, 1, &mask_bits); himl = pImageList_Create(2, 1, ILC_COLOR32 | ILC_MASK, 0, 15); + hr = pHIMAGELIST_QueryInterface(himl, &IID_IImageList2, (void **)&image_list_iface); + ok(hr == S_OK, "QueryInterface returned %#lx.\n", hr); + for (i = 0; i < ARRAY_SIZE(test_bitmaps); i += 2) { bitmap_bits = test_bitmaps + i; @@ -2591,15 +2609,22 @@ static void test_alpha(void) ok(ret == i / 2, "ImageList_Add returned %d, expected %d\n", ret, i / 2); DeleteObject(hbm_test); + has_alpha = get_alpha(bitmap_bits[0]) || get_alpha(bitmap_bits[1]); expected[0] = get_premultiplied_argb(bitmap_bits[0]); expected[1] = get_premultiplied_argb(bitmap_bits[1]); + expected_flag = ILIF_ALPHA; /* If all alpha values are zero, the image is considered to have no alpha and gets masked. */ - if (!get_alpha(bitmap_bits[0]) && !get_alpha(bitmap_bits[1])) + if (!has_alpha) { expected[0] = 0; expected[1] = bitmap_bits[1]; + expected_flag = 0; } + hr = IImageList2_GetItemFlags(image_list_iface, i / 2, &flag); + ok(hr == S_OK, "GetItemFlags returned %#lx.\n", hr); + ok(flag == expected_flag, "Unexpected flag %#lx, expected %#lx.\n", flag, expected_flag); + image_list_get_image_bits_by_bitmap(himl, i / 2, bits); todo_wine_if(i != 0 && i != 2 && i != 4 && i != 6 && i != 18) ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), @@ -2614,12 +2639,16 @@ static void test_alpha(void) winetest_pop_context(); } + IImageList2_Release(image_list_iface); pImageList_Destroy(himl); DeleteObject(hbm_mask); /* Test adding 32bpp images with alpha to 24bpp image list. */ himl = pImageList_Create(2, 1, ILC_COLOR24 | ILC_MASK, 0, 15); + hr = pHIMAGELIST_QueryInterface(himl, &IID_IImageList2, (void **)&image_list_iface); + ok(hr == S_OK, "QueryInterface returned %#lx.\n", hr); + for (i = 0; i < ARRAY_SIZE(test_bitmaps); i += 2) { bitmap_bits = test_bitmaps + i; @@ -2639,6 +2668,10 @@ static void test_alpha(void) expected[1] = get_alpha_blended(0x00ffffff, bitmap_bits[1]); } + hr = IImageList2_GetItemFlags(image_list_iface, i / 2, &flag); + ok(hr == S_OK, "GetItemFlags returned %#lx.\n", hr); + ok(flag == 0, "Unexpected flag %#lx.\n", flag); + image_list_get_image_bits_by_bitmap(himl, i / 2, bits); todo_wine_if(i != 0 && i != 2 && i != 4 && i != 6 && i != 18) ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), @@ -2654,6 +2687,7 @@ static void test_alpha(void) winetest_pop_context(); } + IImageList2_Release(image_list_iface); pImageList_Destroy(himl); DeleteDC(hdc); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8261
There are corresponding fix patches following. But to make this MR smaller, I first submit the tests MR. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_105980
How does this interact with comctl32 v5 imagelist? I mean for things we need fixing, what would be the impact for old imagelist, that presumably doesn't support transparency? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_106375
On Fri Jun 13 07:36:45 2025 +0000, Nikolay Sivov wrote:
How does this interact with comctl32 v5 imagelist? I mean for things we need fixing, what would be the impact for old imagelist, that presumably doesn't support transparency? For this tests MR, it only changes test_alpha(), and test_alpha() is only called by v6 tests, so it won't have any affect on v5 imagelist.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_106463
On Fri Jun 13 07:36:45 2025 +0000, Ziqing Hui wrote:
For this tests MR, it only changes test_alpha(), and test_alpha() is only called by v6 tests, so it won't have any affect on v5 imagelist. For tests sure, but supposedly you'll have to fix something in implementation too.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_106464
On Fri Jun 13 07:38:58 2025 +0000, Nikolay Sivov wrote:
For tests sure, but supposedly you'll have to fix something in implementation too. Here is the implementation patches: https://gitlab.winehq.org/zhui/wine/-/commits/imagelist
The patches focus on the situation where you are adding a 32bpp image with alpha channel to a imagelist without alpha channel(like 24bpp imagelist). So if you are not adding 32bpp images to imagelist, it won't have any affection. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_106465
On Fri Jun 13 07:48:27 2025 +0000, Ziqing Hui wrote:
Here is the implementation patches: https://gitlab.winehq.org/zhui/wine/-/commits/imagelist The patches focus on the situation where you are adding a 32bpp image with alpha channel to a imagelist without alpha channel(like 24bpp imagelist). So if you are not adding 32bpp images to imagelist, it won't have any affection. The implementation mainly changes add_dib_bits() which will only be called when adding 32bpp images
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_106466
On Fri Jun 13 07:50:50 2025 +0000, Ziqing Hui wrote:
The implementation mainly changes add_dib_bits() which will only be called when adding 32bpp images This will make changes to both v5 and v6 with tests only conforming with v6. So the question remains, what will happens to v5 behavior and if we can test that.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_106467
On Fri Jun 13 07:51:59 2025 +0000, Nikolay Sivov wrote:
This will make changes to both v5 and v6 with tests only conforming with v6. So the question remains, what will happens to v5 behavior and if we can test that. OK, so we should add v5 tests about this.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_106468
On Fri Jun 13 07:53:58 2025 +0000, Ziqing Hui wrote:
OK, so we should add v5 tests about this. I tested v5 with test_alpha(), it does behavior differently with v6.
When adding 32bpp images to 24bpp imagelist, v6 will alpha blend it to imagelist's internal bitmap, but v5 won't. How should we handle the difference? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_106976
On Wed Jun 18 03:41:17 2025 +0000, Ziqing Hui wrote:
I tested v5 with test_alpha(), it does behavior differently with v6. When adding 32bpp images to 24bpp imagelist, v6 will alpha blend it to imagelist's internal bitmap, but v5 won't. How should we handle the difference? That's why I'm asking, I don't know what would break if we fix it only for one case. Depending on how the list is created, maybe we can infer which version was used.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_107204
Depending on how the list is created, maybe we can infer which version was used.
Sorry, I don't understand. How do we distinguish v5 and v6 imagelists from the creation? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_107205
On Thu Jun 19 10:16:38 2025 +0000, Ziqing Hui wrote:
Depending on how the list is created, maybe we can infer which version was used. Sorry, I don't understand. How do we distinguish v5 and v6 imagelists from the creation? Maybe check for DLL redirection? For example, `FindActCtxSectionStringA(0, NULL, ACTIVATION_CONTEXT_SECTION_DLL_REDIRECTION, "comctl32.dll", &data)` in load_v6_module(). And of course, v5 and v6 should be separated in the future. Are there necessary things missing for doing the separation? I know we tried to delay it. But maybe it's time.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_109461
On Thu Jul 10 11:23:12 2025 +0000, Zhiyi Zhang wrote:
Maybe check for DLL redirection? For example, `FindActCtxSectionStringA(0, NULL, ACTIVATION_CONTEXT_SECTION_DLL_REDIRECTION, "comctl32.dll", &data)` in load_v6_module(). And of course, v5 and v6 should be separated in the future. Are there necessary things missing for doing the separation? I know we tried to delay it. But maybe it's time. It's easier with window classes, but yes, this is basically our only option at the moment. It's not going to work properly in all cases obviously, and it can also produce false positives - you can have current context with redirection but exported function could still be from comctl32 v5. For separation I think the main issue is to make it work nicely with wine build tree, right now you can't have multiple modules with the same name in it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_109462
On Thu Jul 10 12:09:54 2025 +0000, Nikolay Sivov wrote:
It's easier with window classes, but yes, this is basically our only option at the moment. It's not going to work properly in all cases obviously, and it can also produce false positives - you can have current context with redirection but exported function could still be from comctl32 v5. For separation I think the main issue is to make it work nicely with wine build tree, right now you can't have multiple modules with the same name in it. I made an attempt to separate v5 and v6 for a PoC. Currently I copied all the files from v5 to v6. Please see https://gitlab.winehq.org/zhiyi/wine/-/commits/comctl32-separation. It's not that complicated as I previously thought. The next step is to share the source. I plan to spend more time on this. Let me know what you think.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_109464
On Thu Jul 10 14:26:10 2025 +0000, Zhiyi Zhang wrote:
I made an attempt to separate v5 and v6 for a PoC. Currently I copied all the files from v5 to v6. Please see https://gitlab.winehq.org/zhiyi/wine/-/commits/comctl32-separation. It's not that complicated as I previously thought. The next step is to share the source. I plan to spend more time on this. Let me know what you think. Maybe we could use single source dir, and then ifdef things in there, building two different dlls.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_109465
Maybe we could use single source dir, and then ifdef things in there, building two different DLLs.
Is that possible without changes to the build system? Having a separate dir for v6 doesn't seem too bad. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_109469
On Thu Jul 10 15:30:47 2025 +0000, Zhiyi Zhang wrote:
Maybe we could use single source dir, and then ifdef things in there, building two different DLLs. Is that possible without changes to the build system? Having a separate dir for v6 doesn't seem too bad. We can still do ifdef and share sources for v5 and v6. You can do that already with PARENTSRC.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_109471
On Thu Jul 10 15:33:49 2025 +0000, Nikolay Sivov wrote:
You can do that already with PARENTSRC. Yeah, that's what I meant, using PARENTSRC to share source. Add a comctl32_v6 dir for v6, but the source is still in comctl32. Then use ifdef when appropriate. It seems that I misunderstood you. I thought you meant using a single comctl32 dir for building v5 and v6 dll.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_109474
On Thu Jul 10 16:02:29 2025 +0000, Zhiyi Zhang wrote:
Yeah, that's what I meant, using PARENTSRC to share source. Add a comctl32_v6 dir for v6, but the source is still in comctl32. Then use ifdef when appropriate. It seems that I misunderstood you. I thought you meant using a single comctl32 dir for building v5 and v6 dll. Important things to check:
* if `dlls/<name>/` affects anything, we don't want comctl32_v6 appearing anywhere; * new dlls should be installed in sxs dir; * version resources should be adjusted; * user32 controls copies should exist only in v6 module; * I don't know if starting without a prefix will set up everything properly and sxs module is going to picked up, it's possible that there is more work to be done there. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_109480
user32 controls copies should exist only in v6 module;
All the points except this one are addressed in the initial PoC branch. A list of things to do. - [X] if `dlls/<name>/` affects anything, we don't want comctl32_v6 appearing anywhere; - [X] new dlls should be installed in sxs dir; - [X] version resources should be adjusted; - [ ] user32 controls copies should exist only in v6 module; - [X] I don't know if starting without a prefix will set up everything properly and sxs module is going to picked up, it's possible that there is more work to be done there. - [ ] Remove theming from v5. - [ ] Use PARENTSRC to share source. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_109548
On Mon Jul 14 09:38:03 2025 +0000, Zhiyi Zhang wrote: > > user32 controls copies should exist only in v6 module; > ~~All the points except this one are addressed in the initial PoC > branch.(I had wine aliased to another branch and tested the wrong > version).~~ A list of things to do. > - [X] if `dlls/<name>/` affects anything, we don't want comctl32_v6 > appearing anywhere; > - [x] new dlls should be installed in sxs dir; > - [X] version resources should be adjusted; > - [x] user32 controls copies should exist only in v6 module; > - [x] I don't know if starting without a prefix will set up everything > properly and sxs module is going to picked up, it's possible that there > is more work to be done there. > - [ ] Remove theming from v5. > - [x] Use PARENTSRC to share source. > - [x] Update MAINTAINERS to include comctl32_v6. > - [x] Test upgrading from old prefix. > - [x] Add Wine-Bug. I think Zhiyi is working on splitting v5 and v6. I'll rebase this MR after he finish his work. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_109728
On Mon Jul 14 01:06:28 2025 +0000, Ziqing Hui wrote:
I think Zhiyi is working on splitting v5 and v6. I'll rebase this MR after he finish his work. This MR is for tests, so it doesn't have to wait for anything.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_109754
On Mon Jul 14 11:58:47 2025 +0000, Nikolay Sivov wrote:
This MR is for tests, so it doesn't have to wait for anything. OK, I'll add v5 tests to this MR to get it merged.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8261#note_110015
participants (4)
-
Nikolay Sivov (＠nsivov) -
Zhiyi Zhang (＠zhiyi)
-
Ziqing Hui
-
Ziqing Hui (＠zhui)