[PATCH 0/1] MR10356: winex11: Zero fill image data on allocation.
This fixes artifacts being displayed to screen during start-up of the game 'Draw & Guess'. The artifacts are the result of random data in the memory allocated by `malloc` making their way to an `XPutImage` call within `x11drv_surface_flush`. I'm not 100% sure if this is the correct fix. It could be that `x11drv_surface_flush` should have a means to detect when the image data has not been initialised and then skip the `XPutImage` call. It looks like the current allocation was introduced in 2012 as a part of [70a511739eb7 ](https://gitlab.winehq.org/wine/wine/-/commit/70a511739eb7502340e800bbab0fa08...). But things have changed enough since then that it might now make sense to zero this allocation. Notably, there is a second allocation in that commit that is zeroed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10356
From: Brendan McGrath <bmcgrath@codeweavers.com> --- dlls/winex11.drv/bitblt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/winex11.drv/bitblt.c b/dlls/winex11.drv/bitblt.c index b6e08eaa6b8..b3a0b2ab2f5 100644 --- a/dlls/winex11.drv/bitblt.c +++ b/dlls/winex11.drv/bitblt.c @@ -1738,7 +1738,7 @@ static struct x11drv_image *x11drv_image_create( const BITMAPINFO *info, const X { if (!(image->ximage = XCreateImage( gdi_display, vis->visual, vis->depth, ZPixmap, 0, NULL, width, height, 32, 0 ))) goto failed; - if (!(image->ximage->data = malloc( info->bmiHeader.biSizeImage ))) goto failed; + if (!(image->ximage->data = calloc( 1, info->bmiHeader.biSizeImage ))) goto failed; } return image; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10356
Probably a bit more details of what is going on are needed here. x11drv_surface_flush is supposed to get the image data from the in-memory surface created and managed through win32u. The data there are fully initialized on creation. It is not quite clear why would XPutImage from x11drv_surface_flush ends up with data allocated in X image but not filled from win32u surface data. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10356#note_132576
I don't have a great high level understanding of how this all works, so apologies if I'm covering uninteresting paths (or skipping over relevant parts). But what I can see happening is that the surface is being created as a part of a call to `NtUserSetWindowPos` in one thread, and then another thread calls `NtUserPeekMessage` (which results in a call to `flush_window_surfaces`). **NtUserSetWindowPos** `set_window_pos` (called by `NtUserSetWindowPos`) calls `get_window_surface` and then `apply_window_pos`: - `get_window_surface` results in the creation of the `ximage` with the `data` field pointing to the `malloc` data in question; and then - `apply_window_pos` calls `register_window_surface` which puts the `window_surface` in the `window_surfaces` list. The `windows_surface` has `color_bitmap` set to a GDI handle (a `HBITMAP`). That handle is ultimately a `BITMAPOBJ` which has `dib.dsBm.bmBits` pointing to the data from the `malloc`. **NtUserPeekMessage** When `flush_window_surfaces` is called, it iterates the `window_surfaces` list and passes this `window_surface` to `window_surface_flush`. `window_surface_flush` calls `window_surface_get_color`. `window_surface_get_color` (via the `surface->color_bitmap` GDI handle) returns a pointer to the `malloc` data. `window_surface_get_color` assigns this to `color_bits` and then passes it to `x11drv_surface_flush`. Within `x11drv_surface_flush`, `color_bits` is assigned to `src`, and `ximage->data` is assigned to `dst`. These both point to the data from the `malloc`, so only the alpha bits are updated. After that, the `XPutImage` function is called, with the RGB data from the `malloc` uninitialised. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10356#note_132582
On Wed Mar 18 01:48:10 2026 +0000, Brendan McGrath wrote:
I don't have a great high level understanding of how this all works, so apologies if I'm covering uninteresting paths (or skipping over relevant parts). But what I can see happening is that the surface is being created as a part of a call to `NtUserSetWindowPos` in one thread, and then another thread calls `NtUserPeekMessage` (which results in a call to `flush_window_surfaces`). **NtUserSetWindowPos** `set_window_pos` (called by `NtUserSetWindowPos`) calls `get_window_surface` and then `apply_window_pos`: - `get_window_surface` results in the creation of the `ximage` with the `data` field pointing to the `malloc` data in question; and then - `apply_window_pos` calls `register_window_surface` which puts the `window_surface` in the `window_surfaces` list. The `windows_surface` has `color_bitmap` set to a GDI handle (a `HBITMAP`). That handle is ultimately a `BITMAPOBJ` which has `dib.dsBm.bmBits` pointing to the data from the `malloc`. **NtUserPeekMessage** When `flush_window_surfaces` is called, it iterates the `window_surfaces` list and passes this `window_surface` to `window_surface_flush`. `window_surface_flush` calls `window_surface_get_color`. `window_surface_get_color` (via the `surface->color_bitmap` GDI handle) returns a pointer to the `malloc` data. `window_surface_flush` assigns this to `color_bits` and then passes it to `x11drv_surface_flush`. Within `x11drv_surface_flush`, `color_bits` is assigned to `src`, and `ximage->data` is assigned to `dst`. These both point to the data from the `malloc`, so only the alpha bits are updated. After that, the `XPutImage` function is called, with the RGB data from the `malloc` uninitialised. Oh - I just realised that the version of wine that I'm testing with doesn't include 18339eb0f5048500efb668e6a9b679ed2f62d7a3. I will test with that cherry-picked.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10356#note_132584
On Wed Mar 18 01:48:10 2026 +0000, Brendan McGrath wrote:
Oh - I just realised that the version of wine that I'm testing with doesn't include 18339eb0f5048500efb668e6a9b679ed2f62d7a3. I will test with that cherry-picked. Before that commit the color was de-facto initialized with black, that only changed that to white.
color_bits which window_surface_get_color() gets are not from winex11.drv, they are from DIB section created with NtGdiCreateDIBSection() in window_surface_create(). NtGdiCreateDIBSection doesn't go to winex11, it allocates memory with NtAllocateVirtualMemory (and so that is initialized with 0 even before the 'white' commit). So it is yet unclear to me how any data in X image allocated in winex11 matter here. x11drv_surface_flush() should copy win32u data to the image. Is the win32u image maybe gets the garbled data before that (somehow blitted from x11 with XGetImage or else), or something goes wrong in x11drv_surface_flush itself so the data is not actually copied (to the required size) but X image is put to screen? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10356#note_132586
On Wed Mar 18 01:58:44 2026 +0000, Paul Gofman wrote:
Before that commit the color was de-facto initialized with black, that only changed that to white. color_bits which window_surface_get_color() gets are not from winex11.drv, they are from DIB section created with NtGdiCreateDIBSection() in window_surface_create(). NtGdiCreateDIBSection doesn't go to winex11, it allocates memory with NtAllocateVirtualMemory (and so that is initialized with 0 even before the 'white' commit). So it is yet unclear to me how any data in X image allocated in winex11 matter here. x11drv_surface_flush() should copy win32u data to the image. Is the win32u image maybe gets the garbled data before that (somehow blitted from x11 with XGetImage or else), or something goes wrong in x11drv_surface_flush itself so the data is not actually copied (to the required size) but X image is put to screen? `create_surface` in `winex11.drv/bitblt.c` calls `NtGdiDdDDICreateDCFromMemory`. This creates the GDI object that is stored in `color_bitmap` (and later fetched by `window_surface_get_color`). Is that what you mean?
Cherry-picking 18339eb0 fixes the issue (and as you say, produces a white screen instead of black). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10356#note_132588
On Wed Mar 18 02:48:25 2026 +0000, Brendan McGrath wrote:
`create_surface` in `winex11.drv/bitblt.c` calls `NtGdiDdDDICreateDCFromMemory`. This creates the GDI object that is stored in `color_bitmap` (and later fetched by `window_surface_get_color`). Is that what you mean? Cherry-picking 18339eb0 fixes the issue (and as you say, produces a white screen instead of black). Oh - I think I see what you mean. If `bitmap` isn't already set, it will call `NtGdiCreateDIBSection` in `window_surface_create`. But in this case it is already set (with the `HBITMAP` returned by `NtGdiDdDDICreateDCFromMemory`).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10356#note_132589
On Wed Mar 18 02:59:25 2026 +0000, Brendan McGrath wrote:
Oh - I think I see what you mean. If `bitmap` isn't already set, it will call `NtGdiCreateDIBSection` in `window_surface_create`. But in this case it is already set (with the `HBITMAP` returned by `NtGdiDdDDICreateDCFromMemory`). I see now, thanks, this is bitmap from winex11.drvcreate_surface() -> x11drv_image_create and that is where it was not initialized. Indeed, that commit should be fixing that then, while the original purpose was different.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10356#note_132592
Closing this MR. The issue is fixed with 18339eb0 (which writes the data with `0xff` instead of zeros). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10356#note_132593
This merge request was closed by Brendan McGrath. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10356
participants (3)
-
Brendan McGrath -
Brendan McGrath (＠redmcg)
-
Paul Gofman (＠gofman)