[PATCH 0/3] MR4038: ddraw: Always create surfaces with WINED3D_BIND_SHADER_RESOURCE if possible.
Surfaces without DDSCAPS_TEXTURE can still be textured when used with a software device. wined3d currently doesn't care about the bind flags for surfaces set with wined3d_stateblock_set_texture(), but we currently set the right bind flags anyway. More saliently, I plan to change this fact, and internally translate those textures to SRVs, in order to simplify some internal code and make d3d1-9 support for the Vulkan backend easier. In this case we really will need WINED3D_BIND_SHADER_RESOURCE to be set. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4038
From: Zebediah Figura <zfigura(a)codeweavers.com> NO3D implies SYSTEMMEMORY, and is incompatible with TEXTUREMANAGE, so this case is unreachable. --- dlls/ddraw/surface.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/dlls/ddraw/surface.c b/dlls/ddraw/surface.c index 33f38949c15..d242d460a1d 100644 --- a/dlls/ddraw/surface.c +++ b/dlls/ddraw/surface.c @@ -6189,15 +6189,12 @@ static void wined3d_resource_desc_from_ddraw(struct ddraw *ddraw, } else { - if (!(ddraw->flags & DDRAW_NO3D)) - { - if (caps & DDSCAPS_TEXTURE) - wined3d_desc->bind_flags |= WINED3D_BIND_SHADER_RESOURCE; - if (caps & DDSCAPS_ZBUFFER) - wined3d_desc->bind_flags |= WINED3D_BIND_DEPTH_STENCIL; - else if (caps & DDSCAPS_3DDEVICE) - wined3d_desc->bind_flags |= WINED3D_BIND_RENDER_TARGET; - } + if (caps & DDSCAPS_TEXTURE) + wined3d_desc->bind_flags |= WINED3D_BIND_SHADER_RESOURCE; + if (caps & DDSCAPS_ZBUFFER) + wined3d_desc->bind_flags |= WINED3D_BIND_DEPTH_STENCIL; + else if (caps & DDSCAPS_3DDEVICE) + wined3d_desc->bind_flags |= WINED3D_BIND_RENDER_TARGET; if (caps2 & (DDSCAPS2_TEXTUREMANAGE | DDSCAPS2_D3DTEXTUREMANAGE)) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4038
From: Zebediah Figura <zfigura(a)codeweavers.com> Try to do it in one place instead of two. --- dlls/ddraw/surface.c | 49 +++++++++++++++----------------------------- 1 file changed, 16 insertions(+), 33 deletions(-) diff --git a/dlls/ddraw/surface.c b/dlls/ddraw/surface.c index d242d460a1d..2be29834d78 100644 --- a/dlls/ddraw/surface.c +++ b/dlls/ddraw/surface.c @@ -6182,6 +6182,13 @@ static void wined3d_resource_desc_from_ddraw(struct ddraw *ddraw, wined3d_desc->depth = 1; wined3d_desc->size = 0; + if ((caps & DDSCAPS_TEXTURE) || (caps2 & DDSCAPS2_CUBEMAP)) + wined3d_desc->bind_flags |= WINED3D_BIND_SHADER_RESOURCE; + if (caps & DDSCAPS_ZBUFFER) + wined3d_desc->bind_flags |= WINED3D_BIND_DEPTH_STENCIL; + else if (caps & DDSCAPS_3DDEVICE) + wined3d_desc->bind_flags |= WINED3D_BIND_RENDER_TARGET; + if ((caps & DDSCAPS_SYSTEMMEMORY) && !(caps2 & (DDSCAPS2_TEXTUREMANAGE | DDSCAPS2_D3DTEXTUREMANAGE))) { wined3d_desc->access = WINED3D_RESOURCE_ACCESS_CPU @@ -6189,23 +6196,15 @@ static void wined3d_resource_desc_from_ddraw(struct ddraw *ddraw, } else { - if (caps & DDSCAPS_TEXTURE) - wined3d_desc->bind_flags |= WINED3D_BIND_SHADER_RESOURCE; - if (caps & DDSCAPS_ZBUFFER) - wined3d_desc->bind_flags |= WINED3D_BIND_DEPTH_STENCIL; - else if (caps & DDSCAPS_3DDEVICE) - wined3d_desc->bind_flags |= WINED3D_BIND_RENDER_TARGET; - if (caps2 & (DDSCAPS2_TEXTUREMANAGE | DDSCAPS2_D3DTEXTUREMANAGE)) { - wined3d_desc->bind_flags &= ~WINED3D_BIND_RENDER_TARGET; wined3d_desc->access |= WINED3D_RESOURCE_ACCESS_CPU; wined3d_desc->usage |= WINED3DUSAGE_MANAGED; } else if (caps & DDSCAPS_VIDEOMEMORY) { /* Dynamic resources can't be written by the GPU. */ - if (!(wined3d_desc->bind_flags & (WINED3D_BIND_RENDER_TARGET | WINED3D_BIND_DEPTH_STENCIL))) + if (!(caps & (DDSCAPS_3DDEVICE | DDSCAPS_ZBUFFER))) wined3d_desc->usage |= WINED3DUSAGE_DYNAMIC; } } @@ -6220,19 +6219,6 @@ static void wined3d_resource_desc_from_ddraw(struct ddraw *ddraw, wined3d_desc->usage |= WINED3DUSAGE_LEGACY_CUBEMAP; } -static bool need_draw_texture(unsigned int draw_bind_flags, const struct wined3d_resource_desc *desc) -{ - if (!draw_bind_flags) - return false; - - /* If we have a GPU texture that includes all the necessary bind flags, we - * don't need a separate draw texture. */ - if ((desc->access & WINED3D_RESOURCE_ACCESS_GPU) && ((desc->bind_flags & draw_bind_flags) == draw_bind_flags)) - return false; - - return true; -} - static HRESULT ddraw_texture_init(struct ddraw_texture *texture, struct ddraw *ddraw, unsigned int layers, unsigned int levels, bool sysmem_fallback, bool reserve_memory) { @@ -6242,7 +6228,7 @@ static HRESULT ddraw_texture_init(struct ddraw_texture *texture, struct ddraw *d struct wined3d_resource_desc wined3d_desc; struct wined3d_texture *wined3d_texture; struct ddraw_surface *parent, *root; - unsigned int bind_flags; + unsigned int draw_bind_flags; unsigned int pitch = 0; unsigned int i, j; HRESULT hr; @@ -6281,22 +6267,19 @@ static HRESULT ddraw_texture_init(struct ddraw_texture *texture, struct ddraw *d } } - bind_flags = 0; - if ((desc->ddsCaps.dwCaps2 & DDSCAPS2_CUBEMAP) - || (desc->ddsCaps.dwCaps & DDSCAPS_TEXTURE)) - bind_flags |= WINED3D_BIND_SHADER_RESOURCE; + draw_bind_flags = wined3d_desc.bind_flags; - if (desc->ddsCaps.dwCaps & DDSCAPS_ZBUFFER) - bind_flags |= WINED3D_BIND_DEPTH_STENCIL; - else if (desc->ddsCaps.dwCaps & DDSCAPS_3DDEVICE) - bind_flags |= WINED3D_BIND_RENDER_TARGET; + if (!(wined3d_desc.access & WINED3D_RESOURCE_ACCESS_GPU)) + wined3d_desc.bind_flags = 0; + else if (wined3d_desc.usage & WINED3DUSAGE_MANAGED) + wined3d_desc.bind_flags &= ~WINED3D_BIND_RENDER_TARGET; - if (need_draw_texture(bind_flags, &wined3d_desc)) + if (draw_bind_flags && (wined3d_desc.bind_flags != draw_bind_flags)) { struct wined3d_resource_desc draw_texture_desc; draw_texture_desc = wined3d_desc; - draw_texture_desc.bind_flags = bind_flags; + draw_texture_desc.bind_flags = draw_bind_flags; draw_texture_desc.access = WINED3D_RESOURCE_ACCESS_GPU; draw_texture_desc.usage = 0; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4038
From: Zebediah Figura <zfigura(a)codeweavers.com> -- I really don't like the "retry without WINED3D_BIND_SHADER_RESOURCE" logic, but I couldn't think of anything nicer. --- dlls/ddraw/surface.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/dlls/ddraw/surface.c b/dlls/ddraw/surface.c index 2be29834d78..f3c12bfccdb 100644 --- a/dlls/ddraw/surface.c +++ b/dlls/ddraw/surface.c @@ -6182,8 +6182,9 @@ static void wined3d_resource_desc_from_ddraw(struct ddraw *ddraw, wined3d_desc->depth = 1; wined3d_desc->size = 0; - if ((caps & DDSCAPS_TEXTURE) || (caps2 & DDSCAPS2_CUBEMAP)) - wined3d_desc->bind_flags |= WINED3D_BIND_SHADER_RESOURCE; + /* Always specify WINED3D_BIND_SHADER_RESOURCE. + * Surfaces without it can be bound on software devices on ddraw < 4. */ + wined3d_desc->bind_flags |= WINED3D_BIND_SHADER_RESOURCE; if (caps & DDSCAPS_ZBUFFER) wined3d_desc->bind_flags |= WINED3D_BIND_DEPTH_STENCIL; else if (caps & DDSCAPS_3DDEVICE) @@ -6307,10 +6308,29 @@ static HRESULT ddraw_texture_init(struct ddraw_texture *texture, struct ddraw *d } else { + /* DDSCAPS_TEXTURE is not actually necessary to bind a surface as a + * texture for a software device. Worse, one can bind a hardware + * surface to a software device. However, some formats (e.g. YUY2) can + * live on the GPU but truly aren't texturable, and we don't want to + * fail to create them. + * + * Hence, if we fail to create a texture, and the user didn't + * explicitly ask for a texture, strip WINED3D_BIND_SHADER_RESOURCE and + * try again. */ + if (FAILED(hr = wined3d_texture_create(wined3d_device, &wined3d_desc, layers, levels, WINED3D_TEXTURE_CREATE_GET_DC_LENIENT, NULL, texture, &ddraw_texture_wined3d_parent_ops, &wined3d_texture))) - return hr; + { + if ((desc->ddsCaps.dwCaps & DDSCAPS_TEXTURE) || (desc->ddsCaps.dwCaps2 & DDSCAPS2_CUBEMAP)) + return hr; + + wined3d_desc.bind_flags &= ~WINED3D_BIND_SHADER_RESOURCE; + if (FAILED(hr = wined3d_texture_create(wined3d_device, &wined3d_desc, + layers, levels, WINED3D_TEXTURE_CREATE_GET_DC_LENIENT, NULL, + texture, &ddraw_texture_wined3d_parent_ops, &wined3d_texture))) + return hr; + } } if ((desc->dwFlags & DDSD_LPSURFACE) @@ -6788,19 +6808,11 @@ HRESULT ddraw_surface_create(struct ddraw *ddraw, const DDSURFACEDESC2 *surface_ { if (!(desc->ddsCaps.dwCaps2 & (DDSCAPS2_TEXTUREMANAGE | DDSCAPS2_D3DTEXTUREMANAGE))) { - unsigned int bind_flags = 0; + unsigned int bind_flags = WINED3D_BIND_SHADER_RESOURCE; DWORD usage = 0; if (desc->ddsCaps.dwCaps2 & DDSCAPS2_CUBEMAP) - { usage |= WINED3DUSAGE_LEGACY_CUBEMAP; - bind_flags |= WINED3D_BIND_SHADER_RESOURCE; - } - else if (desc->ddsCaps.dwCaps & DDSCAPS_TEXTURE) - { - bind_flags |= WINED3D_BIND_SHADER_RESOURCE; - } - if (desc->ddsCaps.dwCaps & DDSCAPS_ZBUFFER) bind_flags |= WINED3D_BIND_DEPTH_STENCIL; else if (desc->ddsCaps.dwCaps & DDSCAPS_3DDEVICE) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4038
``` I really don't like the "retry without WINED3D_BIND_SHADER_RESOURCE" logic, but I couldn't think of anything nicer. ```
I don't know about nicer, but we do have wined3d_check_device_format(), and we already use that for a fairly similar purpose in ddraw_surface_create(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4038#note_48020
I don't know about nicer, but we do have wined3d_check_device_format(), and we already use that for a fairly similar purpose in ddraw_surface_create().
I thought about that, but it didn't seem simpler than the current approach, and I'm not sure if it's really a conceptual improvement either. Though if it strikes you as one I can certainly change the patch accordingly. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4038#note_48042
I don't know about nicer, but we do have wined3d_check_device_format(), and we already use that for a fairly similar purpose in ddraw_surface_create().
I thought about that, but it didn't seem simpler than the current approach, and I'm not sure if it's really a conceptual improvement either. Though if it strikes you as one I can certainly change the patch accordingly.
I don't have a strong opinion on the matter. The one benefit of that approach that I'd point out would be that it'll potentially make the resulting debug logs a bit more straightforward to follow. I.e., I could imagine the failing wined3d_texture_create() calls to be potentially confusing to someone less familiar with the code. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4038#note_48045
This merge request was approved by Jan Sikorski. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4038
participants (4)
-
Henri Verbeet (＠hverbeet) -
Jan Sikorski (＠jsikorski)
-
Zebediah Figura
-
Zebediah Figura (＠zfigura)