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.