[PATCH 0/4] MR9348: comctl32/imagelist: Pre-multiply rgb by the alpha channel in add_dib_bits for v6.
We used to do this when drawing the image. But our tests show that for v6, the image bitmaps have been alpha pre-multiplied before drawing. So we should do this when adding images. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9348
From: Ziqing Hui <zhui(a)codeweavers.com> We used to do this when drawing the image. But our tests show that for v6, the image bitmaps have been alpha pre-multiplied before drawing. So we should do this when adding images. --- dlls/comctl32/imagelist.c | 32 +++++++++++++++++++++++--------- dlls/comctl32/tests/imagelist.c | 3 +-- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/dlls/comctl32/imagelist.c b/dlls/comctl32/imagelist.c index 493aebd0b45..ea9b69e1291 100644 --- a/dlls/comctl32/imagelist.c +++ b/dlls/comctl32/imagelist.c @@ -200,6 +200,21 @@ static inline void imagelist_copy_images( HIMAGELIST himl, HDC hdcSrc, HDC hdcDe } } +static void premultiply_alpha_channel(DWORD *bits, int pixel_count) +{ + DWORD *ptr = bits; + unsigned int i; + + for (i = 0; i < pixel_count; i++, ptr++) + { + DWORD alpha = *ptr >> 24; + *ptr = ((*ptr & 0xff000000) + | (((*ptr & 0x00ff0000) * alpha / 255) & 0x00ff0000) + | (((*ptr & 0x0000ff00) * alpha / 255) & 0x0000ff00) + | (((*ptr & 0x000000ff) * alpha / 255))); + } +} + static void add_dib_bits( HIMAGELIST himl, int pos, int count, int width, int height, BITMAPINFO *info, BITMAPINFO *mask_info, DWORD *bits, BYTE *mask_bits ) { @@ -223,6 +238,11 @@ static void add_dib_bits( HIMAGELIST himl, int pos, int count, int width, int he { himl->item_flags[pos + n] = ILIF_ALPHA; +#if __WINE_COMCTL32_VERSION == 6 + for (i = 0; i < height; i++) + premultiply_alpha_channel(&bits[i * stride + n * width], width); +#endif /* __WINE_COMCTL32_VERSION == 6 */ + if (mask_info && himl->hbmMask) /* generate the mask from the alpha channel */ { for (i = 0; i < height; i++) @@ -1236,15 +1256,9 @@ static BOOL alpha_blend_image( HIMAGELIST himl, HDC dest_dc, int dest_x, int des if (has_alpha) /* we already have an alpha channel in this case */ { - /* pre-multiply by the alpha channel */ - for (i = 0, ptr = bits; i < cx * cy; i++, ptr++) - { - DWORD alpha = *ptr >> 24; - *ptr = ((*ptr & 0xff000000) | - (((*ptr & 0x00ff0000) * alpha / 255) & 0x00ff0000) | - (((*ptr & 0x0000ff00) * alpha / 255) & 0x0000ff00) | - (((*ptr & 0x000000ff) * alpha / 255))); - } +#if __WINE_COMCTL32_VERSION == 5 + premultiply_alpha_channel(bits, cx * cy); +#endif /* __WINE_COMCTL32_VERSION == 5 */ } else if (himl->hbmMask) { diff --git a/dlls/comctl32/tests/imagelist.c b/dlls/comctl32/tests/imagelist.c index 3b547732fd5..7cd23a9da2f 100644 --- a/dlls/comctl32/tests/imagelist.c +++ b/dlls/comctl32/tests/imagelist.c @@ -2580,7 +2580,6 @@ static void test_alpha(BOOL v6) } image_list_get_image_bits_by_bitmap(himl, i / 2, bits); - todo_wine_if(v6 && 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]), "Got bits [%08X, %08X], expected [%08X, %08X].\n", bits[0], bits[1], expected[0], expected[1]); @@ -2639,7 +2638,7 @@ static void test_alpha(BOOL v6) } image_list_get_image_bits_by_bitmap(himl, i / 2, bits); - todo_wine_if(i != 0 && i != 2 && i != 4 && i != 6 && (!v6 || i != 18)) + todo_wine_if(i != 0 && i != 2 && i != 4 && i != 6 && !v6) ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), "Got bits [%08X, %08X], expected [%08X, %08X].\n", bits[0], bits[1], expected[0], expected[1]); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9348
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/comctl32/imagelist.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/dlls/comctl32/imagelist.c b/dlls/comctl32/imagelist.c index ea9b69e1291..76ce078f8e8 100644 --- a/dlls/comctl32/imagelist.c +++ b/dlls/comctl32/imagelist.c @@ -147,6 +147,11 @@ BOOL imagelist_has_alpha( HIMAGELIST himl, UINT index ) return himl->item_flags[index] & ILIF_ALPHA; } +static BOOL image_list_color_flag(HIMAGELIST image_list) +{ + return image_list->flags & 0xfe; +} + static inline UINT imagelist_height( UINT count ) { return ((count + TILE_COUNT - 1)/TILE_COUNT); @@ -282,7 +287,7 @@ static BOOL add_with_alpha( HIMAGELIST himl, HDC hdc, int pos, int count, if (!GetObjectW( hbmImage, sizeof(bm), &bm )) return FALSE; /* if either the imagelist or the source bitmap don't have an alpha channel, bail out now */ - if ((himl->flags & 0xfe) != ILC_COLOR32) return FALSE; + if (image_list_color_flag(himl) != ILC_COLOR32) return FALSE; if (bm.bmBitsPixel != 32) return FALSE; SelectObject( hdc, hbmImage ); @@ -2235,7 +2240,7 @@ HIMAGELIST WINAPI ImageList_Read(IStream *pstm) } else mask_info = NULL; - if ((himl->flags & 0xfe) == ILC_COLOR32 && image_info->bmiHeader.biBitCount == 32) + if (image_list_color_flag(himl) == ILC_COLOR32 && image_info->bmiHeader.biBitCount == 32) { DWORD *ptr = image_bits; BYTE *mask_ptr = mask_bits; @@ -2547,7 +2552,7 @@ ImageList_ReplaceIcon (HIMAGELIST himl, INT nIndex, HICON hIcon) himl->cCurImage++; } - if ((himl->flags & 0xfe) == ILC_COLOR32 && GetIconInfo (hBestFitIcon, &ii)) + if (image_list_color_flag(himl) == ILC_COLOR32 && GetIconInfo (hBestFitIcon, &ii)) { HDC hdcImage = CreateCompatibleDC( 0 ); GetObjectW (ii.hbmMask, sizeof(BITMAP), &bmp); @@ -3058,7 +3063,7 @@ BOOL WINAPI ImageList_Write(HIMAGELIST himl, IStream *pstm) static HBITMAP ImageList_CreateImage(HDC hdc, HIMAGELIST himl, UINT count) { HBITMAP hbmNewBitmap; - UINT ilc = (himl->flags & 0xFE); + UINT ilc = image_list_color_flag(himl); SIZE sz; imagelist_get_bitmap_size( himl, count, &sz ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9348
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/comctl32/imagelist.c | 76 +++++++++++++++++++++++++-------------- 1 file changed, 50 insertions(+), 26 deletions(-) diff --git a/dlls/comctl32/imagelist.c b/dlls/comctl32/imagelist.c index 76ce078f8e8..db5beb14042 100644 --- a/dlls/comctl32/imagelist.c +++ b/dlls/comctl32/imagelist.c @@ -147,6 +147,53 @@ BOOL imagelist_has_alpha( HIMAGELIST himl, UINT index ) return himl->item_flags[index] & ILIF_ALPHA; } +static HBITMAP create_dib_section(HDC hdc, int width, int height, int bpp, void **bits) +{ + BITMAPINFO *info; + HBITMAP bitmap; + + if (!(info = Alloc(FIELD_OFFSET(BITMAPINFO, bmiColors[256])))) + return NULL; + + info->bmiHeader.biSize = sizeof(BITMAPINFOHEADER); + info->bmiHeader.biWidth = width; + info->bmiHeader.biHeight = height; + info->bmiHeader.biPlanes = 1; + info->bmiHeader.biBitCount = bpp; + info->bmiHeader.biCompression = BI_RGB; + info->bmiHeader.biXPelsPerMeter = 0; + info->bmiHeader.biYPelsPerMeter = 0; + info->bmiHeader.biClrUsed = 0; + info->bmiHeader.biClrImportant = 0; + + switch (bpp) + { + case 1: + info->bmiHeader.biSizeImage = (width + 31) / 32 * height * 4; + info->bmiColors[0].rgbRed = 0; + info->bmiColors[0].rgbGreen = 0; + info->bmiColors[0].rgbBlue = 0; + info->bmiColors[0].rgbReserved = 0; + info->bmiColors[1].rgbRed = 0xff; + info->bmiColors[1].rgbGreen = 0xff; + info->bmiColors[1].rgbBlue = 0xff; + info->bmiColors[1].rgbReserved = 0; + break; + case 32: + info->bmiHeader.biSizeImage = width * height * 4; + break; + default: + WARN("Unsupported bpp %d.\n", bpp); + Free(info); + return NULL; + } + + bitmap = CreateDIBSection(hdc, info, DIB_RGB_COLORS, bits, 0, 0); + + Free(info); + return bitmap; +} + static BOOL image_list_color_flag(HIMAGELIST image_list) { return image_list->flags & 0xfe; @@ -1184,7 +1231,6 @@ static BOOL alpha_blend_image( HIMAGELIST himl, HDC dest_dc, int dest_x, int des BOOL ret = FALSE; HDC hdc; HBITMAP bmp = 0, mask = 0; - BITMAPINFO *info; BLENDFUNCTION func; void *bits, *mask_bits; unsigned int *ptr; @@ -1196,19 +1242,8 @@ static BOOL alpha_blend_image( HIMAGELIST himl, HDC dest_dc, int dest_x, int des func.AlphaFormat = AC_SRC_ALPHA; if (!(hdc = CreateCompatibleDC( 0 ))) return FALSE; - if (!(info = Alloc( FIELD_OFFSET( BITMAPINFO, bmiColors[256] )))) goto done; - info->bmiHeader.biSize = sizeof(BITMAPINFOHEADER); - info->bmiHeader.biWidth = cx; - info->bmiHeader.biHeight = cy; - info->bmiHeader.biPlanes = 1; - info->bmiHeader.biBitCount = 32; - info->bmiHeader.biCompression = BI_RGB; - info->bmiHeader.biSizeImage = cx * cy * 4; - info->bmiHeader.biXPelsPerMeter = 0; - info->bmiHeader.biYPelsPerMeter = 0; - info->bmiHeader.biClrUsed = 0; - info->bmiHeader.biClrImportant = 0; - if (!(bmp = CreateDIBSection( himl->hdcImage, info, DIB_RGB_COLORS, &bits, 0, 0 ))) goto done; + if (!(bmp = create_dib_section(himl->hdcImage, cx, cy, 32, &bits))) + goto done; SelectObject( hdc, bmp ); BitBlt( hdc, 0, 0, cx, cy, himl->hdcImage, src_x, src_y, SRCCOPY ); @@ -1269,17 +1304,7 @@ static BOOL alpha_blend_image( HIMAGELIST himl, HDC dest_dc, int dest_x, int des { unsigned int width_bytes = (cx + 31) / 32 * 4; /* generate alpha channel from the mask */ - info->bmiHeader.biBitCount = 1; - info->bmiHeader.biSizeImage = width_bytes * cy; - info->bmiColors[0].rgbRed = 0; - info->bmiColors[0].rgbGreen = 0; - info->bmiColors[0].rgbBlue = 0; - info->bmiColors[0].rgbReserved = 0; - info->bmiColors[1].rgbRed = 0xff; - info->bmiColors[1].rgbGreen = 0xff; - info->bmiColors[1].rgbBlue = 0xff; - info->bmiColors[1].rgbReserved = 0; - if (!(mask = CreateDIBSection( himl->hdcMask, info, DIB_RGB_COLORS, &mask_bits, 0, 0 ))) + if (!(mask = create_dib_section(himl->hdcMask, cx, cy, 1, &mask_bits))) goto done; SelectObject( hdc, mask ); BitBlt( hdc, 0, 0, cx, cy, himl->hdcMask, src_x, src_y, SRCCOPY ); @@ -1301,7 +1326,6 @@ done: DeleteDC( hdc ); if (bmp) DeleteObject( bmp ); if (mask) DeleteObject( mask ); - Free( info ); return ret; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9348
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/comctl32/imagelist.c | 25 +++++++++++++++++++++++-- dlls/comctl32/tests/imagelist.c | 2 -- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/dlls/comctl32/imagelist.c b/dlls/comctl32/imagelist.c index db5beb14042..edf0d40e476 100644 --- a/dlls/comctl32/imagelist.c +++ b/dlls/comctl32/imagelist.c @@ -270,6 +270,7 @@ static void premultiply_alpha_channel(DWORD *bits, int pixel_count) static void add_dib_bits( HIMAGELIST himl, int pos, int count, int width, int height, BITMAPINFO *info, BITMAPINFO *mask_info, DWORD *bits, BYTE *mask_bits ) { + BOOL image_list_is_32bpp = (image_list_color_flag(himl) == ILC_COLOR32); int i, j, n; POINT pt; int stride = info->bmiHeader.biWidth; @@ -293,13 +294,32 @@ static void add_dib_bits( HIMAGELIST himl, int pos, int count, int width, int he #if __WINE_COMCTL32_VERSION == 6 for (i = 0; i < height; i++) premultiply_alpha_channel(&bits[i * stride + n * width], width); + + if (!image_list_is_32bpp) + { + himl->item_flags[pos + n] = 0; + + /* Image list is not 32bpp, no alpha channel in image list bitmap. + * We need to do alpha blend here by ourselves. */ + for (i = 0; i < height; i++) + { + for (j = n * width; j < (n + 1) * width; j++) + { + DWORD *pixel = &bits[i * stride + j], alpha = *pixel >> 24; + DWORD r = (*pixel & 0x00ff0000) >> 16; + DWORD g = (*pixel & 0x0000ff00) >> 8; + DWORD b = *pixel & 0x000000ff; + *pixel = ((r + 0xff - alpha) << 16) | ((g + 0xff - alpha) << 8) | (b + 0xff - alpha); + } + } + } #endif /* __WINE_COMCTL32_VERSION == 6 */ if (mask_info && himl->hbmMask) /* generate the mask from the alpha channel */ { for (i = 0; i < height; i++) for (j = n * width; j < (n + 1) * width; j++) - if ((bits[i * stride + j] >> 24) > 25) /* more than 10% alpha */ + if (!image_list_is_32bpp || (bits[i * stride + j] >> 24) > 25) /* more than 10% alpha */ mask_bits[i * mask_stride + j / 8] &= ~(0x80 >> (j % 8)); else mask_bits[i * mask_stride + j / 8] |= 0x80 >> (j % 8); @@ -333,8 +353,9 @@ static BOOL add_with_alpha( HIMAGELIST himl, HDC hdc, int pos, int count, if (!GetObjectW( hbmImage, sizeof(bm), &bm )) return FALSE; - /* if either the imagelist or the source bitmap don't have an alpha channel, bail out now */ +#if __WINE_COMCTL32_VERSION == 5 if (image_list_color_flag(himl) != ILC_COLOR32) return FALSE; +#endif /* __WINE_COMCTL32_VERSION == 5 */ if (bm.bmBitsPixel != 32) return FALSE; SelectObject( hdc, hbmImage ); diff --git a/dlls/comctl32/tests/imagelist.c b/dlls/comctl32/tests/imagelist.c index 7cd23a9da2f..98f061ba712 100644 --- a/dlls/comctl32/tests/imagelist.c +++ b/dlls/comctl32/tests/imagelist.c @@ -2693,13 +2693,11 @@ static void test_alpha(BOOL v6) } image_list_get_image_bits_by_bitmap(himl, i / 2, bits); - todo_wine_if(v6 && i != 0 && i != 2 && i != 4 && i != 6 && i != 18) ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], 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(v6 && i != 0 && i != 2 && i != 4 && i != 6 && i != 18) ok(colour_match(bits[0], expected[0]) && colour_match(bits[1], expected[1]), "Got bits [%08X, %08X], expected [%08X, %08X].\n", bits[0], bits[1], expected[0], expected[1]); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9348
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/imagelist.c:
+ + if (!image_list_is_32bpp) + { + himl->item_flags[pos + n] = 0; + + /* Image list is not 32bpp, no alpha channel in image list bitmap. + * We need to do alpha blend here by ourselves. */ + for (i = 0; i < height; i++) + { + for (j = n * width; j < (n + 1) * width; j++) + { + DWORD *pixel = &bits[i * stride + j], alpha = *pixel >> 24; + DWORD r = (*pixel & 0x00ff0000) >> 16; + DWORD g = (*pixel & 0x0000ff00) >> 8; + DWORD b = *pixel & 0x000000ff; + *pixel = ((r + 0xff - alpha) << 16) | ((g + 0xff - alpha) << 8) | (b + 0xff - alpha); The formula seems wrong. If alpha is 1, then the result color is very close to 0xffffff.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9348#note_120708
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/imagelist.c:
if (has_alpha) /* we already have an alpha channel in this case */ { - /* pre-multiply by the alpha channel */ - for (i = 0, ptr = bits; i < cx * cy; i++, ptr++) - { - DWORD alpha = *ptr >> 24; - *ptr = ((*ptr & 0xff000000) | - (((*ptr & 0x00ff0000) * alpha / 255) & 0x00ff0000) | - (((*ptr & 0x0000ff00) * alpha / 255) & 0x0000ff00) | - (((*ptr & 0x000000ff) * alpha / 255))); - } +#if __WINE_COMCTL32_VERSION == 5 + premultiply_alpha_channel(bits, cx * cy);
Let's not make changes to both v5 and v6 at the same time. It would be easier for review if you could split them. Also, try to avoid the __WINE_COMCTL32_VERSION checks in the middle of the function. See if you could move them into a helper. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9348#note_120710
On Tue Nov 4 14:38:16 2025 +0000, Zhiyi Zhang wrote:
The formula seems wrong. If alpha is 1, then the result color is very close to 0xffffff. I think it is good. We are alpha blending the image to the internal bitmap of imagelist. All the pixels of internal bitmap is 0xffffff. If the alpha is 1, which is the least alpha, it means the source pixels are blended 1/255, and the dest pixels are blended 254/255, the result color is vert close to 0xffffff. Also we have tests for the fomula: https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/comctl32/tests/imagel...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9348#note_122482
Let's not make changes to both v5 and v6 at the same time. It would be easier for review if you could split them.
In fact, although I use `#if __WINE_COMCTL32_VERSION == 5` here, it only changes v6. For v6 I added code that premultiply alpha channel in `add_dib_bits`, so we need to remove this line for v6. To keep it the same as before for v5, I added a v5 guard.
Also, try to avoid the \__WINE_COMCTL32_VERSION checks in the middle of the function. See if you could move them into a helper.
OK, I'll try to move them to helpers. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9348#note_122483
Let's not make changes to both v5 and v6 at the same time. It would be easier for review if you could split them.
In fact, although I use `#if __WINE_COMCTL32_VERSION == 5` here, it only changes v6. For v6 I added code that premultiply alpha channel in `add_dib_bits`, so we need to remove this line for v6. To keep it the same as before for v5, I added a v5 guard.
Maybe it would be better if I find some way to replace `#if __WINE_COMCTL32_VERSION == 5` with `#if __WINE_COMCTL32_VERSION == 6`? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9348#note_122484
On Mon Nov 17 04:10:29 2025 +0000, Ziqing Hui wrote:
Let's not make changes to both v5 and v6 at the same time. It would be easier for review if you could split them.
In fact, although I use `#if __WINE_COMCTL32_VERSION == 5` here, it only changes v6. For v6 I added code that premultiply alpha channel in `add_dib_bits`, so we need to remove this line for v6. To keep it the same as before for v5, I added a v5 guard. Maybe it would be better if I find some way to replace `#if __WINE_COMCTL32_VERSION == 5` with `#if __WINE_COMCTL32_VERSION == 6`? If it's cleaner, yes. The main point is to avoid making changes to both v5 and v6 in the same patch.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9348#note_122696
On Tue Nov 18 03:54:14 2025 +0000, Zhiyi Zhang wrote:
If it's cleaner, yes. The main point is to avoid making changes to both v5 and v6 in the same patch. OK, I'll submit a new version to see if it is acceptable.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9348#note_122914
participants (3)
-
Zhiyi Zhang (＠zhiyi) -
Ziqing Hui
-
Ziqing Hui (＠zhui)