I only did a cursory review, with the idea that for something like this, unless there's something especially egregious, it's fine to fix things up after the fact. A couple of things did stand out though:
On Mon, 11 Apr 2022 at 19:35, Zebediah Figura zfigura@codeweavers.com wrote:
diff --git a/tests/shader_runner.c b/tests/shader_runner.c index bf3bc0d85..54b667fc7 100644 --- a/tests/shader_runner.c +++ b/tests/shader_runner.c @@ -856,6 +856,8 @@ START_TEST(shader_runner) #ifdef _WIN32 run_shader_tests_d3d9(argc, argv); run_shader_tests_d3d11(argc, argv); +#else
- run_shader_tests_vulkan(argc, argv);
#endif run_shader_tests_d3d12(argc, argv); }
I theory, Windows has Vulkan too.
Now that we have multiple shader runners, the logs can become less than clear; vkd3d_test_check_ok() uses "vkd3d_test_name" as prefix, but that's "shader_runner" for all of these. We may want to consider setting that to e.g. "shader_runner_d3d12" for the d3d12 runner, and so on.
+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.
+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.
+static VkDescriptorSetLayout create_descriptor_set_layout(struct vulkan_shader_runner *runner) +{
[...]
- for (i = 0; i < runner->r.sampler_count; ++i)
- {
VkSamplerCreateInfo sampler_desc = {.sType = VK_STRUCTURE_TYPE_SAMPLER_CREATE_INFO};struct vulkan_sampler *vulkan_sampler = &runner->samplers[i];const struct sampler *sampler = &runner->r.samplers[i];sampler_desc.magFilter = (sampler->filter & 0x4) ? VK_FILTER_LINEAR : VK_FILTER_NEAREST;sampler_desc.minFilter = (sampler->filter & 0x1) ? VK_FILTER_LINEAR : VK_FILTER_NEAREST;sampler_desc.mipmapMode = (sampler->filter & 0x10) ? VK_SAMPLER_MIPMAP_MODE_LINEAR : VK_SAMPLER_MIPMAP_MODE_NEAREST;sampler_desc.addressModeU = vk_address_mode_from_d3d12(sampler->u_address);sampler_desc.addressModeV = vk_address_mode_from_d3d12(sampler->v_address);sampler_desc.addressModeW = vk_address_mode_from_d3d12(sampler->w_address);
We may want to consider storing the filter modes separately to begin with, but if we're going to store these as D3D12_FILTER values, it would make sense to use the appropriate macros like D3D12_DECODE_MAG_FILTER to decode them as well.
+static void bind_resources(struct vulkan_shader_runner *runner, VkPipelineBindPoint bind_point,
VkDescriptorSetLayout set_layout, VkPipelineLayout pipeline_layout)+{
[...]
- if (runner->r.uniform_count)
VK_CALL(vkCmdPushConstants(cmd_buffer, pipeline_layout, VK_SHADER_STAGE_ALL, 0,runner->r.uniform_count * sizeof(*runner->r.uniforms), runner->r.uniforms));
Note that the number of available push constants is typically somewhat limited. A value of 128 (= 32 * float = 8 * vec4) for maxPushConstantsSize is not uncommon. That's probably sufficient for our purposes, but nevertheless something to be aware of. It's certainly a lot less than e.g. d3d9's 256 vec4 constants.