Re: [PATCH vkd3d 4/4] tests: Introduce a Vulkan shader runner.

[Zebediah Figura]

On 4/12/22 13:57, Henri Verbeet wrote:
> On Tue, 12 Apr 2022 at 18:42, Zebediah Figura  wrote:
> > > > +static void transition_image_layout(struct vulkan_shader_runner *runner,
> > > > -
> > > >    VkImage image, VkImageLayout src_layout, VkImageLayout dst_layout)
> > > >
> > > > +{
> > > > - VkImageMemoryBarrier barrier = {.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER};
> > > > -
> > > > - barrier.srcAccessMask = VK_ACCESS_MEMORY_READ_BIT | VK_ACCESS_MEMORY_WRITE_BIT;
> > > > - barrier.dstAccessMask = VK_ACCESS_MEMORY_READ_BIT | VK_ACCESS_MEMORY_WRITE_BIT;
> > > > - barrier.oldLayout = src_layout;
> > > > - barrier.newLayout = dst_layout;
> > > > - barrier.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED;
> > > > - barrier.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED;
> > > > - barrier.image = image;
> > > > - barrier.subresourceRange.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT;
> > > > - barrier.subresourceRange.baseMipLevel = 0;
> > > > - barrier.subresourceRange.levelCount = 1;
> > > > - barrier.subresourceRange.baseArrayLayer = 0;
> > > > - barrier.subresourceRange.layerCount = 1;
> > > > -
> > > > - VK_CALL(vkCmdPipelineBarrier(runner->cmd_buffer, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT,
> > > > -
> > > >        VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, 0, 0, NULL, 0, NULL, 1, &barrier));
> > > >
> > > > +}
> > >
> > > That function is a fair bit less generic than its name might suggest.
> >
> > I suppose so, although in what respect do you mean specifically?
>
> levelCount and layerCount being 1, although those could easily be
> replaced with VK_REMAINING_MIP_LEVELS and VK_REMAINING_ARRAY_LAYERS.
> VK_IMAGE_ASPECT_COLOR_BIT wouldn't work for depth/stencil resources.

Okay, that seems sensible to adjust.
> > > > +static bool compile_shader(const struct vulkan_shader_runner *runner, const char *source, const char *type,
> > > > -
> > > >    struct vkd3d_shader_code *dxbc, struct vkd3d_shader_code *spirv)
> > > >
> > > > +{
> > >
> > > [...]
> > > > - info.next = &hlsl_info;
> > > > - info.source.code = source;
> > > > - info.source.size = strlen(source);
> > > > - info.source_type = VKD3D_SHADER_SOURCE_HLSL;
> > > > - info.target_type = VKD3D_SHADER_TARGET_DXBC_TPF;
> > > > - info.log_level = VKD3D_SHADER_LOG_WARNING;
> > >
> > > [...]
> > > > - info.next = &spirv_info;
> > > > - info.source = *dxbc;
> > > > - info.source_type = VKD3D_SHADER_SOURCE_DXBC_TPF;
> > > > - info.target_type = VKD3D_SHADER_TARGET_SPIRV_BINARY;
> > >
> > > It should be relatively straightforward for vkd3d-shader to support
> > > compiling HLSL to SPIR-V. There may be a case for a more direct path,
> > > but for a start, compile_hlsl() could essentially just do what we're
> > > doing here.
> >
> > Yes, although in this case we need to reflect the DXBC anyway, in order
> > to retrieve vertex attribute bindings.
>
> Well, we currently do, but I'm not sure that's a necessity. It
> shouldn't be too hard to get
> vkd3d_shader_scan()/vkd3d_shader_compile() to return
> input/output/patch signatures as well.

We do want a way to scan signatures from DXBC shaders regardless.

I'm not sure that helps us on the HLSL -> SPIRV front, though. I guess 
the solution there is "assume that inputs are allocated in order, one 
register per variable", which seems a bit tenuous but ends up working 
out, both for the reference compiler, and for native d3dcompiler (and 
for our DXBC -> SPIRV path.)