[PATCH v2 0/6] 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. -- v2: comctl32/imagelist: Support adding 32bpp images to a non-32bpp image list for v6. comctl32/imagelist: Do alpha blending when adding 32bpp images to a non-32bpp image list. comctl32/imagelist: Introduce create_dib_section helper. comctl32/imagelist: Introduce image_list_color_flag helper. comctl32/imagelist: Pre-multiply rgb by the alpha channel in add_dib_bits for v6. comctl32/imagelist: Introduce premultiply_alpha_channel helper. https://gitlab.winehq.org/wine/wine/-/merge_requests/9348
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/comctl32/imagelist.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/dlls/comctl32/imagelist.c b/dlls/comctl32/imagelist.c index 93c502ec0bd..58e54799127 100644 --- a/dlls/comctl32/imagelist.c +++ b/dlls/comctl32/imagelist.c @@ -201,6 +201,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 ) { @@ -1237,15 +1252,7 @@ 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))); - } + premultiply_alpha_channel(bits, cx * cy); } else if (himl->hbmMask) { -- GitLab 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 | 47 +++++++++++++++++++++++++-------- dlls/comctl32/tests/imagelist.c | 3 +-- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/dlls/comctl32/imagelist.c b/dlls/comctl32/imagelist.c index 58e54799127..db846924a36 100644 --- a/dlls/comctl32/imagelist.c +++ b/dlls/comctl32/imagelist.c @@ -216,6 +216,38 @@ static void premultiply_alpha_channel(DWORD *bits, int pixel_count) } } +static void do_alpha_processing(DWORD *bits, int n, int width, int height, int stride, + BOOL generate_mask, BYTE *mask_bits, int mask_stride) +{ + int i, j; + +#if __WINE_COMCTL32_VERSION == 6 + /* Premultiply alpha for each line of the nth image. */ + for (i = 0; i < height; i++) + premultiply_alpha_channel(&bits[i * stride + n * width], width); +#endif /* __WINE_COMCTL32_VERSION == 6 */ + + /* Generate the mask from the alpha channel. */ + if (generate_mask) + { + 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 */ + mask_bits[i * mask_stride + j / 8] &= ~(0x80 >> (j % 8)); + else + mask_bits[i * mask_stride + j / 8] |= 0x80 >> (j % 8); + } +} + +static BOOL is_alpha_premultiplied(void) +{ +#if __WINE_COMCTL32_VERSION == 6 + return TRUE; +#else + return FALSE; +#endif /* __WINE_COMCTL32_VERSION == 6 */ +} + 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 ) { @@ -238,16 +270,8 @@ static void add_dib_bits( HIMAGELIST himl, int pos, int count, int width, int he if (has_alpha) { himl->item_flags[pos + n] = ILIF_ALPHA; - - 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 */ - mask_bits[i * mask_stride + j / 8] &= ~(0x80 >> (j % 8)); - else - mask_bits[i * mask_stride + j / 8] |= 0x80 >> (j % 8); - } + do_alpha_processing(bits, n, width, height, stride, + mask_info && himl->hbmMask, mask_bits, mask_stride); } else if (mask_info) /* mask out the background */ { @@ -1252,7 +1276,8 @@ 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 */ { - premultiply_alpha_channel(bits, cx * cy); + if (!is_alpha_premultiplied()) + premultiply_alpha_channel(bits, cx * cy); } else if (himl->hbmMask) { diff --git a/dlls/comctl32/tests/imagelist.c b/dlls/comctl32/tests/imagelist.c index c8a1a25b71d..6f44c1c30a0 100644 --- a/dlls/comctl32/tests/imagelist.c +++ b/dlls/comctl32/tests/imagelist.c @@ -2581,7 +2581,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]); @@ -2640,7 +2639,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 db846924a36..6a9d1831ab6 100644 --- a/dlls/comctl32/imagelist.c +++ b/dlls/comctl32/imagelist.c @@ -148,6 +148,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); @@ -302,7 +307,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 ); @@ -2254,7 +2259,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; @@ -2566,7 +2571,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); @@ -3077,7 +3082,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 6a9d1831ab6..31ac53ac289 100644 --- a/dlls/comctl32/imagelist.c +++ b/dlls/comctl32/imagelist.c @@ -148,6 +148,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; @@ -1204,7 +1251,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; @@ -1216,19 +1262,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 ); @@ -1288,17 +1323,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 ); @@ -1320,7 +1345,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 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/dlls/comctl32/imagelist.c b/dlls/comctl32/imagelist.c index 31ac53ac289..e9bf9f6f50b 100644 --- a/dlls/comctl32/imagelist.c +++ b/dlls/comctl32/imagelist.c @@ -268,15 +268,36 @@ static void premultiply_alpha_channel(DWORD *bits, int pixel_count) } } -static void do_alpha_processing(DWORD *bits, int n, int width, int height, int stride, +static void do_alpha_processing(HIMAGELIST himl, DWORD *bits, int pos, int n, int width, int height, int stride, BOOL generate_mask, BYTE *mask_bits, int mask_stride) { int i, j; #if __WINE_COMCTL32_VERSION == 6 + BOOL image_list_is_32bpp = (image_list_color_flag(himl) == ILC_COLOR32); + /* Premultiply alpha for each line of the nth image. */ 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 */ /* Generate the mask from the alpha channel. */ @@ -322,7 +343,7 @@ static void add_dib_bits( HIMAGELIST himl, int pos, int count, int width, int he if (has_alpha) { himl->item_flags[pos + n] = ILIF_ALPHA; - do_alpha_processing(bits, n, width, height, stride, + do_alpha_processing(himl, bits, pos, n, width, height, stride, mask_info && himl->hbmMask, mask_bits, mask_stride); } else if (mask_info) /* mask out the background */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9348
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/comctl32/imagelist.c | 19 +++++++++++++------ dlls/comctl32/tests/imagelist.c | 2 -- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/dlls/comctl32/imagelist.c b/dlls/comctl32/imagelist.c index e9bf9f6f50b..a017917b329 100644 --- a/dlls/comctl32/imagelist.c +++ b/dlls/comctl32/imagelist.c @@ -321,6 +321,15 @@ static BOOL is_alpha_premultiplied(void) #endif /* __WINE_COMCTL32_VERSION == 6 */ } +static BOOL can_add_with_alpha(HIMAGELIST himl, BITMAP bm) +{ +#if __WINE_COMCTL32_VERSION == 6 + return bm.bmBitsPixel == 32; +#else + return bm.bmBitsPixel == 32 && image_list_color_flag(himl) == ILC_COLOR32; +#endif /* __WINE_COMCTL32_VERSION == 6 */ +} + 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 ) { @@ -361,7 +370,7 @@ static void add_dib_bits( HIMAGELIST himl, int pos, int count, int width, int he } } -/* add images with an alpha channel when the image list is 32 bpp */ +/* Add images with an alpha channel. */ static BOOL add_with_alpha( HIMAGELIST himl, HDC hdc, int pos, int count, int width, int height, HBITMAP hbmImage, HBITMAP hbmMask ) { @@ -372,11 +381,9 @@ static BOOL add_with_alpha( HIMAGELIST himl, HDC hdc, int pos, int count, BYTE *mask_bits = NULL; DWORD mask_width; - 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 (image_list_color_flag(himl) != ILC_COLOR32) return FALSE; - if (bm.bmBitsPixel != 32) return FALSE; + if (!GetObjectW(hbmImage, sizeof(bm), &bm) + || !can_add_with_alpha(himl, bm)) + return FALSE; SelectObject( hdc, hbmImage ); mask_width = (bm.bmWidth + 31) / 32 * 4; diff --git a/dlls/comctl32/tests/imagelist.c b/dlls/comctl32/tests/imagelist.c index 6f44c1c30a0..e295770bfd7 100644 --- a/dlls/comctl32/tests/imagelist.c +++ b/dlls/comctl32/tests/imagelist.c @@ -2694,13 +2694,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 (!(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)))
Please add spaces before function parameters to keep the style consistent. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9348#note_131080
All the pixels of internal bitmap is 0xffffff
How can you assume the pixels of the internal bitmap are 0xffffff? What if it contains other values?
`*pixel = ((r + 0xff - alpha) << 16) | ((g + 0xff - alpha) << 8) | (b + 0xff - alpha);`
I don't understand this formula. Shouldn't alpha blending a premultiplied-alpha image using something like the following? $$C_{out} = C_{src} + (1 - A_{src}) \times C_{dst}$$ -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9348#note_131081
participants (3)
-
Zhiyi Zhang (＠zhiyi) -
Ziqing Hui -
Ziqing Hui (＠zhui)