[PATCH v2 0/1] MR8999: win32u: Fix garbled 24-bit bitmap rendering
Fixes https://bugs.winehq.org/show_bug.cgi?id=58620 I've made this a separate MR from !8969 because that MR contains a lot of highly debatable architectural decisions (such as FAKE_16BIT_MEMDC_PIXEL_FORMAT and all of its special case branches). This MR is very minimal and straight to the point. -- v2: win32u: Fix garbled 24-bit bitmap rendering https://gitlab.winehq.org/wine/wine/-/merge_requests/8999
From: Zowie van Dillen <zowie+wine(a)vandillen.io> --- dlls/win32u/opengl.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/dlls/win32u/opengl.c b/dlls/win32u/opengl.c index efd6ea63b1a..63b378affd8 100644 --- a/dlls/win32u/opengl.c +++ b/dlls/win32u/opengl.c @@ -1251,9 +1251,10 @@ static BOOL flush_memory_dc( struct wgl_context *context, HDC hdc, BOOL write, v if (!get_image_from_bitmap( bmp, info, &bits, &src )) { - int width = info->bmiHeader.biWidth, height = info->bmiHeader.biSizeImage / 4 / width; - if (write) funcs->p_glDrawPixels( width, height, GL_BGRA, GL_UNSIGNED_BYTE, bits.ptr ); - else funcs->p_glReadPixels( 0, 0, width, height, GL_BGRA, GL_UNSIGNED_BYTE, bits.ptr ); + int width = info->bmiHeader.biWidth, height = abs( info->bmiHeader.biHeight ); + UINT format = (info->bmiHeader.biBitCount == 24 ? GL_BGR : GL_BGRA); + if (write) funcs->p_glDrawPixels( width, height, format, GL_UNSIGNED_BYTE, bits.ptr ); + else funcs->p_glReadPixels( 0, 0, width, height, format, GL_UNSIGNED_BYTE, bits.ptr ); } GDI_ReleaseObj( dc->hBitmap ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8999
Rémi Bernon (@rbernon) commented about dlls/win32u/opengl.c:
if (!get_image_from_bitmap( bmp, info, &bits, &src )) { - int width = info->bmiHeader.biWidth, height = info->bmiHeader.biSizeImage / 4 / width; - if (write) funcs->p_glDrawPixels( width, height, GL_BGRA, GL_UNSIGNED_BYTE, bits.ptr ); - else funcs->p_glReadPixels( 0, 0, width, height, GL_BGRA, GL_UNSIGNED_BYTE, bits.ptr ); + int width = info->bmiHeader.biWidth, height = abs( info->bmiHeader.biHeight ); + UINT format = (info->bmiHeader.biBitCount == 24 ? GL_BGR : GL_BGRA); + if (write) funcs->p_glDrawPixels( width, height, format, GL_UNSIGNED_BYTE, bits.ptr ); + else funcs->p_glReadPixels( 0, 0, width, height, format, GL_UNSIGNED_BYTE, bits.ptr );
I believe there was a buffer overflow as GL / GDI currently don't agree on whether the bitmap is 16bpp or 32bpp, and `height = info->bmiHeader.biSizeImage / 4 / width` was there to avoid reading / writing past the bitmap buffer. This would probably be fixed with your other 16bpp changes, can we keep it for now or is it part of the fix here? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8999#note_116342
On Fri Sep 19 07:47:10 2025 +0000, Rémi Bernon wrote:
I believe there was a buffer overflow as GL / GDI currently don't agree on whether the bitmap is 16bpp or 32bpp, and `height = info->bmiHeader.biSizeImage / 4 / width` was there to avoid reading / writing past the bitmap buffer. This would probably be fixed with your other 16bpp changes, can we keep it for now or is it part of the fix here? For a 32 bit and a 24 bit GDI bitmap, this MR will work just fine, because when you ask OpenGL for GL_BGRA it will give you four bytes, but when you ask for GL_BGR it gives you three bytes. You're right though that 16bpp bitmaps will lead to an out-of-bounds write with only this change. I hadn't thought about that. I think together with the other MR it should work because the 16bpp case will be explicitly handled, so I'll mark this as a draft and I'll reopen it after the other one's merged. I still ought to make some changes to that one.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8999#note_116359
This merge request was closed by Zowie van Dillen. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8999
Superseded by !8969 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8999#note_116500
participants (3)
-
Rémi Bernon -
Zowie van Dillen
-
Zowie van Dillen (＠zowie)