protonscr

Crash at libs/vkd3d/command.c:3224

vkd3dclosed
HansKristian-Work/vkd3d-proton#226 · opened 2020-07-28 by alexfs · updated 2020-10-05 · 12 comments · github
Aalexfs 2020-07-28 github

a typo in the code when desc->info.view is NULL?

if ((binding->flags & VKD3D_SHADER_BINDING_FLAG_IMAGE)
&& (desc->info.view->type == VKD3D_VIEW_TYPE_IMAGE))
{
if (desc->info.view)
{
vk_descriptor->image.imageView = desc->info.view->vk_image_view;
vk_descriptor->image.sampler = VK_NULL_HANDLE;
vk_descriptor->image.imageLayout = desc->info.view->info.texture.vk_layout;
}

HHansKristian-Work maintainer 2020-07-28 github

Looks questionable, yes, although this path should only be hit in the packed descriptor fallback path, so that might explain why it hasn't been a problem.

HHansKristian-Work maintainer 2020-07-28 github

@doitsujin Any ideas?

HHansKristian-Work maintainer 2020-07-28 github

@alexfs Did you actually get a crash (which game?), or is this just a hypothetical crash?

Aalexfs 2020-07-28 github

Not a game but not a theoretical crash either. After bisecting the following commits lead to regression:
"vkd3d: Do not create view for NULL SRV descriptors if possible."
"vkd3d: Do not create view for NULL UAV descriptors if possible."

HHansKristian-Work maintainer 2020-07-28 github

How can we reproduce this crash then?

Aalexfs 2020-07-28 github

I'll try to build a separate test case. Unfortunately my project is not public yet

Ddoitsujin maintainer 2020-07-29 github

I'm more interested in why you're hitting that code path in the first place. Does your Vulkan driver not support descriptor indexing?

We're about to remove that path entirely, which should also fix the crash, but requires descriptor indexing support.

Aalexfs 2020-07-29 github

I do see the indexing extension.
(VK_EXT_descriptor_indexing : extension revision 2)
I don't know that particular path was chosen though.
The full vulkaninfo output attached if that helps

info.zip

Ddoitsujin maintainer 2020-07-29 github

Ok, I guess I can see why it fails.

        maxUpdateAfterBindDescriptorsInAllPools              = 524288
        maxPerStageDescriptorUpdateAfterBindSampledImages    = 524288
        maxPerStageDescriptorUpdateAfterBindStorageImages    = 524288

We need support for at least 1M descriptors per descriptor heap, since that's what D3D12 requires. Makes me wonder how exactly Intel supports D3D12 on that hardware if this isn't just a case of the driver-reported limits being too conservative.

HHansKristian-Work maintainer 2020-07-29 github

The Windows Intel driver reports 1M, not sure why Anvil only reports 512k, 512k is minspec, so it might just be that.

Mmisyltoad maintainer 2020-07-29 github

From ANV:

   /* It's a bit hard to exactly map our implementation to the limits
    * described here.  The bindless surface handle in the extended
    * message descriptors is 20 bits and it's an index into the table of
    * RENDER_SURFACE_STATE structs that starts at bindless surface base
    * address.  Given that most things consume two surface states per
    * view (general/sampled for textures and write-only/read-write for
    * images), we claim 2^19 things.
    *
    * For SSBOs, we just use A64 messages so there is no real limit
    * there beyond the limit on the total size of a descriptor set.
    */

https://gitlab.freedesktop.org/mesa/mesa/-/blob/1cdd161a30297c13908134d8d205a7a377ec807a/src/intel/vulkan/anv_device.c#L1744

HHansKristian-Work maintainer 2020-10-05 github

This code path doesn't exist anymore, closing.