[PATCH 0/1] MR1688: ddraw: Sync draw textures even for hardware device.
Fixes a regression introduced by commit d2d9f713df7932aac4b137ec3bf647d5da7faaef. Fixes black screen in Exceed - Gun Bullet Children. The check in d3d_device_sync_surfaces() assumes that separate draw textures may be created and successfully bound for software device only. This turns out to be not always the case. The case which the game hits is that it creates a surface with DDSCAPS_TEXTURE | DDSCAPS_3DDEVICE, DDSCAPS2_TEXTUREMANAGE. Such surface creation is valid and the resulting surface is bindable as render target of a software device (we have a test for that). Such a texture effectively falls into the case when a separate draw texture is created and that looks correct. However this surface, while cannot be used as a render target on hardware device, can still be used as a texture. So for a general case syncing surfaces for software devices only is wrong (it still looks right for syncing render targets). Just always traversing all the bound textures in d3d_device_sync_surfaces() looks like a waste, while accurately tracking when there are bound textures requiring sync is possible while looks like an unnecessary complication. So triggering the sync by the fact a texture requiring a sync was ever bound (which is in fact not so common case) looks like a reasonable compromise to me. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1688
From: Paul Gofman <pgofman(a)codeweavers.com> Fixes a regression introduced by commit d2d9f713df7932aac4b137ec3bf647d5da7faaef. --- dlls/ddraw/ddraw_private.h | 1 + dlls/ddraw/device.c | 30 +++++++++++++++++++++++++----- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/dlls/ddraw/ddraw_private.h b/dlls/ddraw/ddraw_private.h index 6693a650394..09e8133350b 100644 --- a/dlls/ddraw/ddraw_private.h +++ b/dlls/ddraw/ddraw_private.h @@ -328,6 +328,7 @@ struct d3d_device LONG ref; UINT version; BOOL hardware_device; + BOOL have_draw_textures; IUnknown *outer_unknown; struct wined3d_device *wined3d_device; diff --git a/dlls/ddraw/device.c b/dlls/ddraw/device.c index ac6647b8dad..1cfef5007d5 100644 --- a/dlls/ddraw/device.c +++ b/dlls/ddraw/device.c @@ -3409,11 +3409,11 @@ void d3d_device_sync_surfaces(struct d3d_device *device) struct ddraw_surface *surface; unsigned int i, j; - if (device->hardware_device) - return; - d3d_device_sync_rendertarget(device); + if (!device->have_draw_textures) + return; + for (i = 0; i < ARRAY_SIZE(state->textures); ++i) { if (!state->textures[i]) @@ -4642,7 +4642,17 @@ static HRESULT d3d_device7_SetTexture(IDirect3DDevice7 *iface, TRACE("iface %p, stage %lu, texture %p.\n", iface, stage, texture); if (surf && (surf->surface_desc.ddsCaps.dwCaps & DDSCAPS_TEXTURE)) - wined3d_texture = surf->draw_texture ? surf->draw_texture : surf->wined3d_texture; + { + if (surf->draw_texture) + { + wined3d_texture = surf->draw_texture; + device->have_draw_textures = TRUE; + } + else + { + wined3d_texture = surf->wined3d_texture; + } + } wined3d_mutex_lock(); wined3d_stateblock_set_texture(device->update_state, stage, wined3d_texture); @@ -4682,7 +4692,17 @@ static HRESULT WINAPI d3d_device3_SetTexture(IDirect3DDevice3 *iface, wined3d_mutex_lock(); if (tex && ((tex->surface_desc.ddsCaps.dwCaps & DDSCAPS_TEXTURE) || !device->hardware_device)) - wined3d_texture = tex->draw_texture ? tex->draw_texture : tex->wined3d_texture; + { + if (tex->draw_texture) + { + wined3d_texture = tex->draw_texture; + device->have_draw_textures = TRUE; + } + else + { + wined3d_texture = tex->wined3d_texture; + } + } wined3d_stateblock_set_texture(device->state, stage, wined3d_texture); fixup_texture_alpha_op(device); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1688
On 12/5/22 15:24, Paul Gofman (@gofman) wrote:
So for a general case syncing surfaces for software devices only is wrong (it still looks right for syncing render targets). Just always traversing all the bound textures in d3d_device_sync_surfaces() looks like a waste, while accurately tracking when there are bound textures requiring sync is possible while looks like an unnecessary complication. So triggering the sync by the fact a texture requiring a sync was ever bound (which is in fact not so common case) looks like a reasonable compromise to me.
From my armchair, it doesn't seem that complicated? I'd think you could do something like the attached patch (on top of this one).
On 12/5/22 16:06, Zebediah Figura wrote:
On 12/5/22 15:24, Paul Gofman (@gofman) wrote:
So for a general case syncing surfaces for software devices only is wrong (it still looks right for syncing render targets). Just always traversing all the bound textures in d3d_device_sync_surfaces() looks like a waste, while accurately tracking when there are bound textures requiring sync is possible while looks like an unnecessary complication. So triggering the sync by the fact a texture requiring a sync was ever bound (which is in fact not so common case) looks like a reasonable compromise to me.
From my armchair, it doesn't seem that complicated? I'd think you could do something like the attached patch (on top of this one).
I am afraid it won't work correctly with saved stateblocks?
On 12/5/22 16:07, Paul Gofman wrote:
On 12/5/22 16:06, Zebediah Figura wrote:
On 12/5/22 15:24, Paul Gofman (@gofman) wrote:
So for a general case syncing surfaces for software devices only is wrong (it still looks right for syncing render targets). Just always traversing all the bound textures in d3d_device_sync_surfaces() looks like a waste, while accurately tracking when there are bound textures requiring sync is possible while looks like an unnecessary complication. So triggering the sync by the fact a texture requiring a sync was ever bound (which is in fact not so common case) looks like a reasonable compromise to me.
From my armchair, it doesn't seem that complicated? I'd think you could do something like the attached patch (on top of this one).
I am afraid it won't work correctly with saved stateblocks?
Hmm, right. It doesn't strike me as that bad to handle that in ApplyStateBlock too, much like we do for sysmem buffers in d3d8/9, but I suppose this patch is fine as-is too.
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=127212 Your paranoid android. === debian11 (build log) === error: patch failed: dlls/ddraw/ddraw_private.h:328 error: patch failed: dlls/ddraw/device.c:3411 Task: Patch failed to apply === debian11b (build log) === error: patch failed: dlls/ddraw/ddraw_private.h:328 error: patch failed: dlls/ddraw/device.c:3411 Task: Patch failed to apply
This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1688
This merge request was approved by Jan Sikorski. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1688
participants (7)
-
Jan Sikorski (＠jsikorski) -
Marvin -
Paul Gofman -
Paul Gofman
-
Paul Gofman (＠gofman)
-
Zebediah Figura -
Zebediah Figura (＠zfigura)