[PATCH v6 0/6] MR10039: comctl32: Fix the bug when the cInitial is too large which lead to CreateDIBSection fails.
When I was porting a software from windows, I met a problem that the icons in the imagelist were not displaying correctly. In the Application, the correct icons are: {width=269 height=29} But when I run it under wine is like this: {width=271 height=29} When I disassable the application code I found that the Application calls ImageList_Create by given a very large cInitial, which is 0xc0c0c0. This will lead to CreateDIBSection fail. Then the source hbitmap is lost, only hMask is left and shown up on the screen. I use windbg to debug this application on windows, I found that after ImageList_Create is called on Winodws, there is a valid hbitmap handle generated. When ImageList_Add is called for 4 times, it changed and called Bitblt to copy the hbitmap already added. So I guess windows has the error handling for ImageList_Create. If CreateDIBSection failes due to the large image count, it reduced the image count. By observing the action in ImageList_Add, I think the reduced count is 1 (4x1). Then fix the cMaxImage to 1+1 which is same as ImageListImpl_Initialize to make the hbitmap expand. The example in the attachment can reproduce this bug. [imagelist.7z](/uploads/b5f7013fa9383c00bbbc38f82383ff1a/imagelist.7z) -- v6: comctl32: Fix the bug when the cInitial is too large which lead to CreateDIBSection fails. https://gitlab.winehq.org/wine/wine/-/merge_requests/10039
From: Kun Yang <yangkun@uniontech.com> Logs: When CreateDIBSection does not return a valid hbitmap, the source image is lost that only masked icon is shown in imagelist. Signed-off-by: Kun Yang <yangkun@uniontech.com> --- dlls/comctl32/imagelist.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/dlls/comctl32/imagelist.c b/dlls/comctl32/imagelist.c index 93c502ec0bd..443078600e4 100644 --- a/dlls/comctl32/imagelist.c +++ b/dlls/comctl32/imagelist.c @@ -3088,6 +3088,15 @@ static HBITMAP ImageList_CreateImage(HDC hdc, HIMAGELIST himl, UINT count) } } hbmNewBitmap = CreateDIBSection(hdc, bmi, DIB_RGB_COLORS, NULL, 0, 0); + if(!hbmNewBitmap) + { + UINT newImageCount = 1; + //if CreateDIBSection failed, allocate for 1 image only. + bmi->bmiHeader.biHeight = newImageCount * himl->cy; + hbmNewBitmap = CreateDIBSection(hdc, bmi, DIB_RGB_COLORS, NULL, 0, 0); + //the max image count is set to 2, expand when exceeded. + himl->cMaxImage = newImageCount + 1; + } } else /*if (ilc == ILC_COLORDDB)*/ { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10039
From: Kun Yang <yangkun@uniontech.com> Logs: When CreateDIBSection does not return a valid hbitmap, the source image is lost that only masked icon is shown in imagelist. Signed-off-by: Kun Yang <yangkun@uniontech.com> --- dlls/comctl32/tests/imagelist.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/dlls/comctl32/tests/imagelist.c b/dlls/comctl32/tests/imagelist.c index c8a1a25b71d..2d885975f8f 100644 --- a/dlls/comctl32/tests/imagelist.c +++ b/dlls/comctl32/tests/imagelist.c @@ -2207,6 +2207,11 @@ static void test_create_destroy(void) INT cx, cy; BOOL rc; INT ret; + HDC hdc; + HBITMAP hbm; + BITMAPINFO bmi; + void *bits; + IMAGEINFO info; /* list with zero or negative image dimensions */ himl = pImageList_Create(0, 0, ILC_COLOR16, 0, 3); @@ -2263,6 +2268,23 @@ static void test_create_destroy(void) ok(himl == NULL, "got %p\n", himl); himl = pImageList_Create(1, -1, ILC_COLORDDB, 4, 4); ok(himl == NULL, "got %p\n", himl); + + /* Test create large cInitial image count */ + himl = pImageList_Create(32, 32, ILC_COLOR32 | ILC_MASK, 0xc0c0c0, 1); + hdc = CreateCompatibleDC(NULL); + memset(&bmi, 0, sizeof(bmi)); + bmi.bmiHeader.biSize = sizeof(bmi.bmiHeader); + bmi.bmiHeader.biWidth = 32; + bmi.bmiHeader.biHeight = -32; + bmi.bmiHeader.biPlanes = 1; + bmi.bmiHeader.biBitCount = 32; + bmi.bmiHeader.biCompression = BI_RGB; + hbm = CreateDIBSection(hdc, &bmi, DIB_RGB_COLORS, &bits, NULL, 0); + ret = pImageList_AddMasked(himl, hbm, RGB(0, 0, 0)); + ret = pImageList_GetImageInfo(himl, 0, &info); + ok(ret, "got %d\n", ret); + ok(info.hbmImage != NULL, "got %p\n", info.hbmImage); + ok(info.hbmMask != NULL, "got %p\n", info.hbmMask); } static void check_color_table(const char *name, HDC hdc, HIMAGELIST himl, UINT ilc, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10039
From: Kun Yang <yangkun@uniontech.com> Logs: When CreateDIBSection does not return a valid hbitmap, the source image is lost that only masked icon is shown in imagelist. Signed-off-by: Kun Yang <yangkun@uniontech.com> --- dlls/comctl32/imagelist.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/comctl32/imagelist.c b/dlls/comctl32/imagelist.c index 443078600e4..978aeaa7755 100644 --- a/dlls/comctl32/imagelist.c +++ b/dlls/comctl32/imagelist.c @@ -3091,10 +3091,10 @@ static HBITMAP ImageList_CreateImage(HDC hdc, HIMAGELIST himl, UINT count) if(!hbmNewBitmap) { UINT newImageCount = 1; - //if CreateDIBSection failed, allocate for 1 image only. + /*if CreateDIBSection failed, allocate for 1 image only.*/ bmi->bmiHeader.biHeight = newImageCount * himl->cy; hbmNewBitmap = CreateDIBSection(hdc, bmi, DIB_RGB_COLORS, NULL, 0, 0); - //the max image count is set to 2, expand when exceeded. + /*the max image count is set to 2, expand when exceeded.*/ himl->cMaxImage = newImageCount + 1; } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10039
From: Kun Yang <yangkun@uniontech.com> Logs: When CreateDIBSection does not return a valid hbitmap, the source image is lost that only masked icon is shown in imagelist. Signed-off-by: Kun Yang <yangkun@uniontech.com> --- dlls/comctl32/imagelist.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dlls/comctl32/imagelist.c b/dlls/comctl32/imagelist.c index 978aeaa7755..614c185bbf5 100644 --- a/dlls/comctl32/imagelist.c +++ b/dlls/comctl32/imagelist.c @@ -383,6 +383,7 @@ IMAGELIST_InternalExpandBitmaps(HIMAGELIST himl, INT nImageCount) himl->item_flags = HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, himl->item_flags, nNewCount * sizeof(*himl->item_flags)); himl->cMaxImage = nNewCount; + ERR("1111. himl->cCurImage %d\n", himl->cCurImage); DeleteDC (hdcBitmap); } @@ -3094,8 +3095,8 @@ static HBITMAP ImageList_CreateImage(HDC hdc, HIMAGELIST himl, UINT count) /*if CreateDIBSection failed, allocate for 1 image only.*/ bmi->bmiHeader.biHeight = newImageCount * himl->cy; hbmNewBitmap = CreateDIBSection(hdc, bmi, DIB_RGB_COLORS, NULL, 0, 0); - /*the max image count is set to 2, expand when exceeded.*/ - himl->cMaxImage = newImageCount + 1; + /*the max image count is set to 1, expand when exceeded.*/ + himl->cMaxImage = newImageCount; } } else /*if (ilc == ILC_COLORDDB)*/ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10039
From: Kun Yang <yangkun@uniontech.com> Logs: When CreateDIBSection does not return a valid hbitmap, the source image is lost that only masked icon is shown in imagelist. Signed-off-by: Kun Yang <yangkun@uniontech.com> --- dlls/comctl32/imagelist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/comctl32/imagelist.c b/dlls/comctl32/imagelist.c index 614c185bbf5..b4b43c83a5d 100644 --- a/dlls/comctl32/imagelist.c +++ b/dlls/comctl32/imagelist.c @@ -3094,9 +3094,9 @@ static HBITMAP ImageList_CreateImage(HDC hdc, HIMAGELIST himl, UINT count) UINT newImageCount = 1; /*if CreateDIBSection failed, allocate for 1 image only.*/ bmi->bmiHeader.biHeight = newImageCount * himl->cy; - hbmNewBitmap = CreateDIBSection(hdc, bmi, DIB_RGB_COLORS, NULL, 0, 0); /*the max image count is set to 1, expand when exceeded.*/ himl->cMaxImage = newImageCount; + himl->cMaxImage = newImageCount; } } else /*if (ilc == ILC_COLORDDB)*/ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10039
From: Kun Yang <yangkun@uniontech.com> Logs: When CreateDIBSection does not return a valid hbitmap, the source image is lost that only masked icon is shown in imagelist. Signed-off-by: Kun Yang <yangkun@uniontech.com> --- dlls/comctl32/imagelist.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dlls/comctl32/imagelist.c b/dlls/comctl32/imagelist.c index b4b43c83a5d..817bdbc1c21 100644 --- a/dlls/comctl32/imagelist.c +++ b/dlls/comctl32/imagelist.c @@ -383,7 +383,6 @@ IMAGELIST_InternalExpandBitmaps(HIMAGELIST himl, INT nImageCount) himl->item_flags = HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, himl->item_flags, nNewCount * sizeof(*himl->item_flags)); himl->cMaxImage = nNewCount; - ERR("1111. himl->cCurImage %d\n", himl->cCurImage); DeleteDC (hdcBitmap); } @@ -3094,9 +3093,9 @@ static HBITMAP ImageList_CreateImage(HDC hdc, HIMAGELIST himl, UINT count) UINT newImageCount = 1; /*if CreateDIBSection failed, allocate for 1 image only.*/ bmi->bmiHeader.biHeight = newImageCount * himl->cy; + hbmNewBitmap = CreateDIBSection(hdc, bmi, DIB_RGB_COLORS, NULL, 0, 0); /*the max image count is set to 1, expand when exceeded.*/ himl->cMaxImage = newImageCount; - himl->cMaxImage = newImageCount; } } else /*if (ilc == ILC_COLORDDB)*/ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10039
I made a test on windows about cMaxImage. I think in this case, the initial cMaxImage is set to 1. So I fixed my code. But cMaxImage=2 is procedurally correct. I still have problem about wine test for cMaxImage, I think HIMAGELIST struct is different on windows and wine.Here is the windbg result for test in my attachment. I fixed cGrow=0 to make easier observation. I think the cMaxImage and cCurImage is here. {width=434 height=468} When ImageList_Add was called, it grows as expected. {width=409 height=342} The running result in wine is the same as windows. {width=822 height=600} You can see that HIMAGELIST struct is very different from wine.So I wonder is there any sense to make wine test by using wine structure, which may fail on windows. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10039#note_129134
participants (2)
-
Kun Yang -
Kun Yang (＠yangkun)