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(a)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.