[PATCH v2 0/1] MR970: wined3d: Do not enforce map access for resources with WINED3D_RESOURCE_ACCESS_CPU.
From: Zebediah Figura zfigura@codeweavers.com
d3d maps of such resources will map the CPU copy, and uploads and downloads use glBufferSubData() and glGetBufferSubData() respectively. There is no need to map the BO.
This improves performance of Indivisible on NVidia GPUs. The game uses a d3d9 MANAGED buffer for streaming vertex data, which results in poor performance on NVidia when using GL_MAP_READ_BIT | GL_MAP_PERSISTENT_BIT. With this change we use neither. --- dlls/wined3d/resource.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/dlls/wined3d/resource.c b/dlls/wined3d/resource.c index 08edce3630c..c32c9580ae9 100644 --- a/dlls/wined3d/resource.c +++ b/dlls/wined3d/resource.c @@ -379,10 +379,13 @@ GLbitfield wined3d_resource_gl_storage_flags(const struct wined3d_resource *reso
if (resource->usage & WINED3DUSAGE_DYNAMIC) flags |= GL_CLIENT_STORAGE_BIT; - if (access & WINED3D_RESOURCE_ACCESS_MAP_W) - flags |= GL_MAP_WRITE_BIT; - if (access & WINED3D_RESOURCE_ACCESS_MAP_R) - flags |= GL_MAP_READ_BIT; + if (!(access & WINED3D_RESOURCE_ACCESS_CPU)) + { + if (access & WINED3D_RESOURCE_ACCESS_MAP_W) + flags |= GL_MAP_WRITE_BIT; + if (access & WINED3D_RESOURCE_ACCESS_MAP_R) + flags |= GL_MAP_READ_BIT; + }
return flags; }
On Mon Oct 10 03:01:56 2022 +0000, **** wrote:
Zebediah Figura replied on the mailing list:
On 10/5/22 09:13, Jan Sikorski (@jsikorski) wrote: >> d3d maps of such resources will map the CPU copy, and uploads and downloads can > use a staging BO. > > I'm concerned about performance implications of this change. If we use a staging (temporary) BO for downloads, then we're going to stall waiting for the download to complete in `wined3d_texture_vk_download_data`. What's the story here for D3D11_USAGE_STAGING resources? > > I understand there's some reason to do this for the GL backend, what motivation there is for the Vulkan change? > Sadly I did not think this through very completely :-/ I wrote the change for the GL backend, tacitly assumed that it should apply to Vulkan as well, checked that it worked functionally, and then submitted the patch. IIRC I *did* discuss this with an NVidia driver developer, and the comment I got was that MAP_READ_BIT | MAP_WRITE_BIT would return a memory type corresponding to Vulkan's HOST_CACHED, so if the managed BO performs badly with MAP_READ_BIT | MAP_WRITE_BIT then I'd expect it to perform badly with the current Vulkan backend as well. But there's no way of testing, and it's not clear that this *is* an improvement, so I think it makes the most sense just to leave out this patch for now. I'll resend with just 1/2.
I have a couple of thoughts on this:
- It seems desirable to use the same basic placement logic for OpenGL and Vulkan. - This series shouldn't affect the creation of staging bo's for texture downloads. More broadly, vk_memory_type_from_access_flags() isn't used for textures, and e.g. the bo created by wined3d_texture_vk_prepare_buffer_object() will just use VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, which seems right.
This merge request was approved by Jan Sikorski.
On Tue Oct 11 10:58:30 2022 +0000, Henri Verbeet wrote:
I have a couple of thoughts on this:
- It seems desirable to use the same basic placement logic for OpenGL
and Vulkan.
- This series shouldn't affect the creation of staging bo's for texture
downloads. More broadly, vk_memory_type_from_access_flags() isn't used for textures, and e.g. the bo created by wined3d_texture_vk_prepare_buffer_object() will just use VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, which seems right.
Right, maybe it's fine, but to me it's not immediately clear what are the consequences for D3D11. It would be helpful to explain it in the commit message.
-
Henri Verbeet (＠hverbeet) -
Jan Sikorski (＠jsikorski)
-
Zebediah Figura
-
Zebediah Figura (＠zfigura)