[PATCH 0/2] MR10697: win32u: Reject DDBs that are not 1-bit or 32-bit for GetDIBits().
SDL3 applications depend on this to not overwrite the depth of its internal display mode. Fix SDL3 applications using the native display mode instead of the specified display mode when restoring to fullscreen from focus loss for the first time, and the specific display mode is 16-bit. Please see `WIN_UpdateDisplayMode()` in the SDL3 source code. ```c static void WIN_UpdateDisplayMode(SDL_VideoDevice *_this, LPCWSTR deviceName, DWORD index, SDL_DisplayMode *mode) { ... if (index == ENUM_CURRENT_SETTINGS && (hdc = CreateDCW(deviceName, NULL, NULL, NULL)) != NULL) { char bmi_data[sizeof(BITMAPINFOHEADER) + 256 * sizeof(RGBQUAD)]; LPBITMAPINFO bmi; HBITMAP hbm; SDL_zeroa(bmi_data); bmi = (LPBITMAPINFO)bmi_data; bmi->bmiHeader.biSize = sizeof(BITMAPINFOHEADER); hbm = CreateCompatibleBitmap(hdc, 1, 1); GetDIBits(hdc, hbm, 0, 1, NULL, bmi, DIB_RGB_COLORS); GetDIBits(hdc, hbm, 0, 1, NULL, bmi, DIB_RGB_COLORS); DeleteObject(hbm); DeleteDC(hdc); if (bmi->bmiHeader.biCompression == BI_BITFIELDS) { switch (*(Uint32 *)bmi->bmiColors) { ... case 0x7C00: mode->format = SDL_PIXELFORMAT_XRGB1555; break; } ... ``` When entering a 16-bit display mode, `WIN_UpdateDisplayMode(_this, displaydata->DeviceName, ENUM_CURRENT_SETTINGS, mode);` is called in `WIN_SetDisplayMode()`. The second `GetDIBits()` call in `WIN_UpdateDisplayMode()` succeeds and ends up overwriting the format to SDL_PIXELFORMAT_XRGB1555, which is a 15-bit display mode. So when SDL3 tries to enter the 16-bit display mode again, it won't be found because it's now 15-bit. Tests show that the second `GetDIBits()` should fail and thus leave the display mode format unchanged. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10697
From: Zhiyi Zhang <zzhang@codeweavers.com> --- dlls/gdi32/tests/bitmap.c | 87 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 86 insertions(+), 1 deletion(-) diff --git a/dlls/gdi32/tests/bitmap.c b/dlls/gdi32/tests/bitmap.c index d4d04b26927..9040f6c0f41 100644 --- a/dlls/gdi32/tests/bitmap.c +++ b/dlls/gdi32/tests/bitmap.c @@ -2105,9 +2105,10 @@ static void test_GetDIBits(void) 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff, 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff }; + unsigned int depths[] = {1, 2, 4, 8, 15, 16, 24, 32}; HBITMAP hbmp; BITMAP bm; - HDC hdc; + HDC hdc, mem_dc; int i, bytes, lines; BYTE buf[1024]; char bi_buf[sizeof(BITMAPINFOHEADER) + sizeof(RGBQUAD) * 256]; @@ -2418,6 +2419,90 @@ static void test_GetDIBits(void) ok(!memcmp(buf, dib_bits_24, sizeof(dib_bits_24)), "DIB bits don't match\n"); DeleteObject(hbmp); + /* Test a SDL3 behavior where GetDIBits() gets called twice consecutively to get bmiColors. + * However, tests show that the second call fails when hbmp is a compatible bitmap and the + * bitmap depth is not 1-bit or 32-bit. Tests for GetDIBits() when hbmp is a DIB are already + * covered by other tests */ + for (i = 0; i < ARRAY_SIZE(depths); i++) + { + winetest_push_context("depth %u", depths[i]); + + hbmp = CreateBitmap(1, 1, 1, depths[i], NULL); + + /* Simulate SDL3 behavior by calling GetDIBits() twice */ + memset(bi, 0, sizeof(*bi)); + bi->bmiHeader.biSize = sizeof(BITMAPINFOHEADER); + lines = GetDIBits(hdc, hbmp, 0, 0, NULL, bi, DIB_RGB_COLORS); + ok(lines == 1, "GetDIBits failed.\n"); + lines = GetDIBits(hdc, hbmp, 0, 0, NULL, bi, DIB_RGB_COLORS); + if (depths[i] == 1) + { + ok(lines == 1, "GetDIBits failed.\n"); + ok(*(unsigned int *)bi->bmiColors == 0, "Got unexpected bmiColors %#x\n", *(unsigned int *)bi->bmiColors); + } + else if (depths[i] == 32) + { + ok(lines == 1, "GetDIBits failed.\n"); + ok(*(unsigned int *)bi->bmiColors == 0xff0000, "Got unexpected bmiColors %#x\n", *(unsigned int *)bi->bmiColors); + } + else + { + todo_wine + ok(lines == 0, "GetDIBits succeeded.\n"); + todo_wine_if(depths[i] == 15 || depths[i] == 16) + ok(*(unsigned int *)bi->bmiColors == 0, "Got unexpected bmiColors %#x\n", *(unsigned int *)bi->bmiColors); + + /* lines > 0. Still fails */ + lines = GetDIBits(hdc, hbmp, 0, 1, NULL, bi, DIB_RGB_COLORS); + todo_wine + ok(lines == 0, "GetDIBits failed.\n"); + + /* buf != NULL. Still fails */ + lines = GetDIBits(hdc, hbmp, 0, 1, buf, bi, DIB_RGB_COLORS); + todo_wine + ok(lines == 0, "GetDIBits failed.\n"); + + /* Reset biBitCount to 0. Now it succeeds */ + bi->bmiHeader.biBitCount = 0; + lines = GetDIBits(hdc, hbmp, 0, 0, NULL, bi, DIB_RGB_COLORS); + ok(lines == 1, "GetDIBits failed.\n"); + } + + /* Test that GetDIBits() is rejecting DDB that's not 1-bit or 32-bit */ + memset(bi, 0, sizeof(*bi)); + bi->bmiHeader.biSize = sizeof(BITMAPINFOHEADER); + bi->bmiHeader.biWidth = 1; + bi->bmiHeader.biHeight = 1; + bi->bmiHeader.biPlanes = 1; + bi->bmiHeader.biBitCount = depths[i]; + lines = GetDIBits(hdc, hbmp, 0, 1, buf, bi, DIB_RGB_COLORS); + if (depths[i] == 1 || depths[i] == 32) + ok(lines == 1, "GetDIBits failed.\n"); + else + todo_wine_if(depths[i] != 2 && depths[i] != 15) + ok(lines == 0, "GetDIBits succeeded.\n"); + + /* Same result when using a memory DC so it's not related the display DC */ + mem_dc = CreateCompatibleDC(hdc); + SelectObject(mem_dc, hbmp); + memset(bi, 0, sizeof(*bi)); + bi->bmiHeader.biSize = sizeof(BITMAPINFOHEADER); + bi->bmiHeader.biWidth = 1; + bi->bmiHeader.biHeight = 1; + bi->bmiHeader.biPlanes = 1; + bi->bmiHeader.biBitCount = depths[i]; + lines = GetDIBits(mem_dc, hbmp, 0, 1, buf, bi, DIB_RGB_COLORS); + if (depths[i] == 1 || depths[i] == 32) + ok(lines == 1, "GetDIBits failed.\n"); + else + todo_wine_if(depths[i] != 2 && depths[i] != 15) + ok(lines == 0, "GetDIBits succeeded.\n"); + + DeleteDC(mem_dc); + DeleteObject(hbmp); + winetest_pop_context(); + } + ReleaseDC(0, hdc); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10697
From: Zhiyi Zhang <zzhang@codeweavers.com> SDL3 applications depend on this to not overwrite the depth of its internal display mode. Fix SDL3 applications using the native display mode instead of the specified display mode when restoring to fullscreen from focus loss for the first time and the specific display mode is 16-bit. Please see `WIN_UpdateDisplayMode()` in the SDL3 source code. ```c static void WIN_UpdateDisplayMode(SDL_VideoDevice *_this, LPCWSTR deviceName, DWORD index, SDL_DisplayMode *mode) { ... if (index == ENUM_CURRENT_SETTINGS && (hdc = CreateDCW(deviceName, NULL, NULL, NULL)) != NULL) { char bmi_data[sizeof(BITMAPINFOHEADER) + 256 * sizeof(RGBQUAD)]; LPBITMAPINFO bmi; HBITMAP hbm; SDL_zeroa(bmi_data); bmi = (LPBITMAPINFO)bmi_data; bmi->bmiHeader.biSize = sizeof(BITMAPINFOHEADER); hbm = CreateCompatibleBitmap(hdc, 1, 1); GetDIBits(hdc, hbm, 0, 1, NULL, bmi, DIB_RGB_COLORS); GetDIBits(hdc, hbm, 0, 1, NULL, bmi, DIB_RGB_COLORS); DeleteObject(hbm); DeleteDC(hdc); if (bmi->bmiHeader.biCompression == BI_BITFIELDS) { switch (*(Uint32 *)bmi->bmiColors) { ... case 0x7C00: mode->format = SDL_PIXELFORMAT_XRGB1555; break; } ... ``` When entering a 16-bit display mode, `WIN_UpdateDisplayMode(_this, displaydata->DeviceName, ENUM_CURRENT_SETTINGS, mode);` is called in `WIN_SetDisplayMode()`. The second `GetDIBits()` call in `WIN_UpdateDisplayMode()` succeeds and ends up overwriting the format to SDL_PIXELFORMAT_XRGB1555, which is a 15-bit display mode. So when SDL3 tries to enter the 16-bit display mode again, it won't be found because it's now 15-bit. Tests show that the second `GetDIBits()` should fail and thus leave the display mode format unchanged. --- dlls/gdi32/tests/bitmap.c | 6 ------ dlls/win32u/dib.c | 3 +++ 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/dlls/gdi32/tests/bitmap.c b/dlls/gdi32/tests/bitmap.c index 9040f6c0f41..63fe5eae16a 100644 --- a/dlls/gdi32/tests/bitmap.c +++ b/dlls/gdi32/tests/bitmap.c @@ -2447,19 +2447,15 @@ static void test_GetDIBits(void) } else { - todo_wine ok(lines == 0, "GetDIBits succeeded.\n"); - todo_wine_if(depths[i] == 15 || depths[i] == 16) ok(*(unsigned int *)bi->bmiColors == 0, "Got unexpected bmiColors %#x\n", *(unsigned int *)bi->bmiColors); /* lines > 0. Still fails */ lines = GetDIBits(hdc, hbmp, 0, 1, NULL, bi, DIB_RGB_COLORS); - todo_wine ok(lines == 0, "GetDIBits failed.\n"); /* buf != NULL. Still fails */ lines = GetDIBits(hdc, hbmp, 0, 1, buf, bi, DIB_RGB_COLORS); - todo_wine ok(lines == 0, "GetDIBits failed.\n"); /* Reset biBitCount to 0. Now it succeeds */ @@ -2479,7 +2475,6 @@ static void test_GetDIBits(void) if (depths[i] == 1 || depths[i] == 32) ok(lines == 1, "GetDIBits failed.\n"); else - todo_wine_if(depths[i] != 2 && depths[i] != 15) ok(lines == 0, "GetDIBits succeeded.\n"); /* Same result when using a memory DC so it's not related the display DC */ @@ -2495,7 +2490,6 @@ static void test_GetDIBits(void) if (depths[i] == 1 || depths[i] == 32) ok(lines == 1, "GetDIBits failed.\n"); else - todo_wine_if(depths[i] != 2 && depths[i] != 15) ok(lines == 0, "GetDIBits succeeded.\n"); DeleteDC(mem_dc); diff --git a/dlls/win32u/dib.c b/dlls/win32u/dib.c index 8a0d064cd90..8473ae3c429 100644 --- a/dlls/win32u/dib.c +++ b/dlls/win32u/dib.c @@ -1348,6 +1348,9 @@ INT WINAPI NtGdiGetDIBitsInternal( HDC hdc, HBITMAP hbitmap, UINT startscan, UIN if (err) goto done; + if (!is_bitmapobj_dib( bmp ) && (src_info->bmiHeader.biBitCount != 1 && src_info->bmiHeader.biBitCount != 32)) + goto done; + /* fill out the src colour table, if it needs one */ if (src_info->bmiHeader.biBitCount <= 8 && src_info->bmiHeader.biClrUsed == 0) fill_default_color_table( src_info ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10697
The Mac CI failure seems unrelated. It looks like the Mac CI environment is broken. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10697#note_137066
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10697
participants (3)
-
Huw Davies (＠huw) -
Zhiyi Zhang
-
Zhiyi Zhang (＠zhiyi)