Vulkan: Choose D32S8 or D24S8 depending on availability

The Vulkan spec mandates support for only one or the other, which is
why we have the concept of a depth24plus format. This also adds a Toggle
to test both formats in DepthStencilStateTests.

Finally this renames ForceWorkarounds to ForceToggles because toggles
can be more than just workarounds.

BUG=dawn:286

Change-Id: I5b5dc582ffd4ee61c51e3e75563aec815c580511
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/14103
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: David Turner <digit@google.com>

src/dawn_native/Toggles.cpp

@@ -96,6 +96,12 @@ namespace dawn_native {
               "Enable usage of spvc's internal parsing and IR generation code, instead of "
               "spirv_cross's.",
               "https://crbug.com/dawn/288"}},
+            {Toggle::VulkanUseD32S8,
+             {"vulkan_use_d32s8",
+              "Vulkan mandates support of either D32_FLOAT_S8 or D24_UNORM_S8. When available the "
+              "backend will use D32S8 (toggle to on) but setting the toggle to off will make it"
+              "use the D24S8 format when possible.",
+              "https://crbug.com/dawn/286"}},
         }};
 
     }  // anonymous namespace

src/dawn_native/Toggles.h

@@ -35,6 +35,7 @@ namespace dawn_native {
         SkipValidation,
         UseSpvc,
         UseSpvcIRGen,
+        VulkanUseD32S8,
 
         EnumCount,
         InvalidEnum = EnumCount,

src/dawn_native/vulkan/DeviceVk.cpp

@@ -79,6 +79,10 @@ namespace dawn_native { namespace vulkan {
 
         DAWN_TRY(PrepareRecordingContext());
 
+        // The environment can request to use D32S8 or D24S8 when it's not available. Override
+        // the decision if it is not applicable.
+        ApplyDepth24PlusS8Toggle();
+
         return {};
     }
 
@@ -459,6 +463,40 @@ namespace dawn_native { namespace vulkan {
         // TODO(jiawei.shao@intel.com): tighten this workaround when this issue is fixed in both
         // Vulkan SPEC and drivers.
         SetToggle(Toggle::UseTemporaryBufferInCompressedTextureToTextureCopy, true);
+
+        // By default try to use D32S8 for Depth24PlusStencil8
+        SetToggle(Toggle::VulkanUseD32S8, true);
+    }
+
+    void Device::ApplyDepth24PlusS8Toggle() {
+        VkPhysicalDevice physicalDevice = ToBackend(GetAdapter())->GetPhysicalDevice();
+
+        bool supportsD32s8 = false;
+        {
+            VkFormatProperties properties;
+            fn.GetPhysicalDeviceFormatProperties(physicalDevice, VK_FORMAT_D32_SFLOAT_S8_UINT,
+                                                 &properties);
+            supportsD32s8 =
+                properties.optimalTilingFeatures & VK_FORMAT_FEATURE_DEPTH_STENCIL_ATTACHMENT_BIT;
+        }
+
+        bool supportsD24s8 = false;
+        {
+            VkFormatProperties properties;
+            fn.GetPhysicalDeviceFormatProperties(physicalDevice, VK_FORMAT_D24_UNORM_S8_UINT,
+                                                 &properties);
+            supportsD24s8 =
+                properties.optimalTilingFeatures & VK_FORMAT_FEATURE_DEPTH_STENCIL_ATTACHMENT_BIT;
+        }
+
+        ASSERT(supportsD32s8 || supportsD24s8);
+
+        if (!supportsD24s8) {
+            SetToggle(Toggle::VulkanUseD32S8, true);
+        }
+        if (!supportsD32s8) {
+            SetToggle(Toggle::VulkanUseD32S8, false);
+        }
     }
 
     VulkanFunctions* Device::GetMutableFunctions() {
@@ -616,7 +654,7 @@ namespace dawn_native { namespace vulkan {
             return DAWN_VALIDATION_ERROR("External semaphore usage not supported");
         }
         if (!mExternalMemoryService->SupportsImportMemory(
-                VulkanImageFormat(textureDescriptor->format), VK_IMAGE_TYPE_2D,
+                VulkanImageFormat(this, textureDescriptor->format), VK_IMAGE_TYPE_2D,
                 VK_IMAGE_TILING_OPTIMAL,
                 VulkanImageUsage(textureDescriptor->usage,
                                  GetValidInternalFormat(textureDescriptor->format)),

src/dawn_native/vulkan/DeviceVk.h

@@ -68,6 +68,8 @@ namespace dawn_native { namespace vulkan {
         Serial GetPendingCommandSerial() const override;
         MaybeError SubmitPendingCommands();
 
+        // Dawn Native API
+
         TextureBase* CreateTextureWrappingVulkanImage(
             const ExternalImageDescriptor* descriptor,
             ExternalMemoryHandle memoryHandle,
@@ -126,6 +128,7 @@ namespace dawn_native { namespace vulkan {
         void GatherQueueFromDevice();
 
         void InitTogglesFromDriver();
+        void ApplyDepth24PlusS8Toggle();
 
         // To make it easier to use fn it is a public const member. However
         // the Device is allowed to mutate them through these private methods.

src/dawn_native/vulkan/RenderPassCache.cpp

@@ -108,7 +108,7 @@ namespace dawn_native { namespace vulkan {
             attachmentRef.layout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
 
             attachmentDesc.flags = 0;
-            attachmentDesc.format = VulkanImageFormat(query.colorFormats[i]);
+            attachmentDesc.format = VulkanImageFormat(mDevice, query.colorFormats[i]);
             attachmentDesc.samples = vkSampleCount;
             attachmentDesc.loadOp = VulkanAttachmentLoadOp(query.colorLoadOp[i]);
             attachmentDesc.storeOp = VK_ATTACHMENT_STORE_OP_STORE;
@@ -129,7 +129,7 @@ namespace dawn_native { namespace vulkan {
             depthStencilAttachmentRef.layout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL;
 
             attachmentDesc.flags = 0;
-            attachmentDesc.format = VulkanImageFormat(query.depthStencilFormat);
+            attachmentDesc.format = VulkanImageFormat(mDevice, query.depthStencilFormat);
             attachmentDesc.samples = vkSampleCount;
             attachmentDesc.loadOp = VulkanAttachmentLoadOp(query.depthLoadOp);
             attachmentDesc.storeOp = VK_ATTACHMENT_STORE_OP_STORE;
@@ -150,7 +150,7 @@ namespace dawn_native { namespace vulkan {
             attachmentRef.layout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
 
             attachmentDesc.flags = 0;
-            attachmentDesc.format = VulkanImageFormat(query.colorFormats[i]);
+            attachmentDesc.format = VulkanImageFormat(mDevice, query.colorFormats[i]);
             attachmentDesc.samples = VK_SAMPLE_COUNT_1_BIT;
             attachmentDesc.loadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
             attachmentDesc.storeOp = VK_ATTACHMENT_STORE_OP_STORE;

src/dawn_native/vulkan/TextureVk.cpp

@@ -203,7 +203,7 @@ namespace dawn_native { namespace vulkan {
     }  // namespace
 
     // Converts Dawn texture format to Vulkan formats.
-    VkFormat VulkanImageFormat(wgpu::TextureFormat format) {
+    VkFormat VulkanImageFormat(const Device* device, wgpu::TextureFormat format) {
         switch (format) {
             case wgpu::TextureFormat::R8Unorm:
                 return VK_FORMAT_R8_UNORM;
@@ -285,7 +285,15 @@ namespace dawn_native { namespace vulkan {
             case wgpu::TextureFormat::Depth24Plus:
                 return VK_FORMAT_D32_SFLOAT;
             case wgpu::TextureFormat::Depth24PlusStencil8:
-                return VK_FORMAT_D32_SFLOAT_S8_UINT;
+                // Depth24PlusStencil8 maps to either of these two formats because only requires
+                // that one of the two be present. The VulkanUseD32S8 toggle combines the wish of
+                // the environment, default to using D32S8, and availability information so we know
+                // that the format is available.
+                if (device->IsToggleEnabled(Toggle::VulkanUseD32S8)) {
+                    return VK_FORMAT_D32_SFLOAT_S8_UINT;
+                } else {
+                    return VK_FORMAT_D24_UNORM_S8_UINT;
+                }
 
             case wgpu::TextureFormat::BC1RGBAUnorm:
                 return VK_FORMAT_BC1_RGBA_UNORM_BLOCK;
@@ -428,7 +436,7 @@ namespace dawn_native { namespace vulkan {
         createInfo.pNext = nullptr;
         createInfo.flags = 0;
         createInfo.imageType = VulkanImageType(GetDimension());
-        createInfo.format = VulkanImageFormat(GetFormat().format);
+        createInfo.format = VulkanImageFormat(device, GetFormat().format);
         createInfo.extent = VulkanExtent3D(GetSize());
         createInfo.mipLevels = GetNumMipLevels();
         createInfo.arrayLayers = GetArrayLayers();
@@ -484,7 +492,7 @@ namespace dawn_native { namespace vulkan {
     // Internally managed, but imported from external handle
     MaybeError Texture::InitializeFromExternal(const ExternalImageDescriptor* descriptor,
                                                external_memory::Service* externalMemoryService) {
-        VkFormat format = VulkanImageFormat(GetFormat().format);
+        VkFormat format = VulkanImageFormat(ToBackend(GetDevice()), GetFormat().format);
         VkImageUsageFlags usage = VulkanImageUsage(GetUsage(), GetFormat());
         if (!externalMemoryService->SupportsCreateImage(descriptor, format, usage)) {
             return DAWN_VALIDATION_ERROR("Creating an image from external memory is not supported");
@@ -790,7 +798,7 @@ namespace dawn_native { namespace vulkan {
         createInfo.flags = 0;
         createInfo.image = ToBackend(GetTexture())->GetHandle();
         createInfo.viewType = VulkanImageViewType(descriptor->dimension);
-        createInfo.format = VulkanImageFormat(descriptor->format);
+        createInfo.format = VulkanImageFormat(device, descriptor->format);
         createInfo.components = VkComponentMapping{VK_COMPONENT_SWIZZLE_R, VK_COMPONENT_SWIZZLE_G,
                                                    VK_COMPONENT_SWIZZLE_B, VK_COMPONENT_SWIZZLE_A};
         createInfo.subresourceRange.aspectMask = VulkanAspectMask(GetFormat());

src/dawn_native/vulkan/TextureVk.h

@@ -28,7 +28,7 @@ namespace dawn_native { namespace vulkan {
     class Device;
     struct ExternalImageDescriptor;
 
-    VkFormat VulkanImageFormat(wgpu::TextureFormat format);
+    VkFormat VulkanImageFormat(const Device* device, wgpu::TextureFormat format);
     VkImageUsageFlags VulkanImageUsage(wgpu::TextureUsage usage, const Format& format);
     VkSampleCountFlagBits VulkanSampleCount(uint32_t sampleCount);
 

src/tests/DawnTest.cpp

@@ -93,9 +93,9 @@ const DawnTestParam MetalBackend(dawn_native::BackendType::Metal);
 const DawnTestParam OpenGLBackend(dawn_native::BackendType::OpenGL);
 const DawnTestParam VulkanBackend(dawn_native::BackendType::Vulkan);
 
-DawnTestParam ForceWorkarounds(const DawnTestParam& originParam,
+DawnTestParam ForceToggles(const DawnTestParam& originParam,
-                               std::initializer_list<const char*> forceEnabledWorkarounds,
+                           std::initializer_list<const char*> forceEnabledWorkarounds,
-                               std::initializer_list<const char*> forceDisabledWorkarounds) {
+                           std::initializer_list<const char*> forceDisabledWorkarounds) {
     DawnTestParam newTestParam = originParam;
     newTestParam.forceEnabledWorkarounds = forceEnabledWorkarounds;
     newTestParam.forceDisabledWorkarounds = forceDisabledWorkarounds;

src/tests/DawnTest.h

@@ -104,9 +104,9 @@ extern const DawnTestParam MetalBackend;
 extern const DawnTestParam OpenGLBackend;
 extern const DawnTestParam VulkanBackend;
 
-DawnTestParam ForceWorkarounds(const DawnTestParam& originParam,
+DawnTestParam ForceToggles(const DawnTestParam& originParam,
-                               std::initializer_list<const char*> forceEnabledWorkarounds,
+                           std::initializer_list<const char*> forceEnabledWorkarounds,
-                               std::initializer_list<const char*> forceDisabledWorkarounds = {});
+                           std::initializer_list<const char*> forceDisabledWorkarounds = {});
 
 namespace utils {
     class TerribleCommandBuffer;

src/tests/end2end/BufferTests.cpp

@@ -746,7 +746,7 @@ TEST_P(CreateBufferMappedTests, LargeBufferFails) {
 
 DAWN_INSTANTIATE_TEST(CreateBufferMappedTests,
                       D3D12Backend,
-                      ForceWorkarounds(D3D12Backend, {}, {"use_d3d12_resource_heap_tier2"}),
+                      ForceToggles(D3D12Backend, {}, {"use_d3d12_resource_heap_tier2"}),
                       MetalBackend,
                       OpenGLBackend,
                       VulkanBackend);

src/tests/end2end/CompressedTextureFormatTests.cpp

@@ -1054,5 +1054,5 @@ DAWN_INSTANTIATE_TEST(CompressedTextureBCFormatTest,
                       MetalBackend,
                       OpenGLBackend,
                       VulkanBackend,
-                      ForceWorkarounds(VulkanBackend,
+                      ForceToggles(VulkanBackend,
-                                       {"use_temporary_buffer_in_texture_to_texture_copy"}));
+                                   {"use_temporary_buffer_in_texture_to_texture_copy"}));

src/tests/end2end/DepthStencilStateTests.cpp

@@ -682,7 +682,8 @@ TEST_P(DepthStencilStateTest, StencilDepthPass) {
 }
 
 DAWN_INSTANTIATE_TEST(DepthStencilStateTest,
-                     D3D12Backend,
+                      D3D12Backend,
-                     MetalBackend,
+                      MetalBackend,
-                     OpenGLBackend,
+                      OpenGLBackend,
-                     VulkanBackend);
+                      ForceToggles(VulkanBackend, {"vulkan_use_d32s8"}, {}),
+                      ForceToggles(VulkanBackend, {}, {"vulkan_use_d32s8"}));

src/tests/end2end/MultisampledRenderingTests.cpp

@@ -497,13 +497,13 @@ TEST_P(MultisampledRenderingTest, ResolveInto2DArrayTexture) {
 
 DAWN_INSTANTIATE_TEST(MultisampledRenderingTest,
                       D3D12Backend,
-                      ForceWorkarounds(D3D12Backend, {}, {"use_d3d12_resource_heap_tier2"}),
+                      ForceToggles(D3D12Backend, {}, {"use_d3d12_resource_heap_tier2"}),
-                      ForceWorkarounds(D3D12Backend, {}, {"use_d3d12_render_pass"}),
+                      ForceToggles(D3D12Backend, {}, {"use_d3d12_render_pass"}),
                       MetalBackend,
                       OpenGLBackend,
                       VulkanBackend,
-                      ForceWorkarounds(MetalBackend, {"emulate_store_and_msaa_resolve"}),
+                      ForceToggles(MetalBackend, {"emulate_store_and_msaa_resolve"}),
-                      ForceWorkarounds(MetalBackend, {"always_resolve_into_zero_level_and_layer"}),
+                      ForceToggles(MetalBackend, {"always_resolve_into_zero_level_and_layer"}),
-                      ForceWorkarounds(MetalBackend,
+                      ForceToggles(MetalBackend,
-                                       {"always_resolve_into_zero_level_and_layer",
+                                   {"always_resolve_into_zero_level_and_layer",
-                                        "emulate_store_and_msaa_resolve"}));
+                                    "emulate_store_and_msaa_resolve"}));

src/tests/end2end/NonzeroTextureCreationTests.cpp

@@ -166,12 +166,12 @@ TEST_P(NonzeroTextureCreationTests, NonRenderableTextureClearWithMultiArrayLayer
 }
 
 DAWN_INSTANTIATE_TEST(NonzeroTextureCreationTests,
-                      ForceWorkarounds(D3D12Backend,
+                      ForceToggles(D3D12Backend,
-                                       {"nonzero_clear_resources_on_creation_for_testing"},
+                                   {"nonzero_clear_resources_on_creation_for_testing"},
-                                       {"lazy_clear_resource_on_first_use"}),
+                                   {"lazy_clear_resource_on_first_use"}),
-                      ForceWorkarounds(OpenGLBackend,
+                      ForceToggles(OpenGLBackend,
-                                       {"nonzero_clear_resources_on_creation_for_testing"},
+                                   {"nonzero_clear_resources_on_creation_for_testing"},
-                                       {"lazy_clear_resource_on_first_use"}),
+                                   {"lazy_clear_resource_on_first_use"}),
-                      ForceWorkarounds(VulkanBackend,
+                      ForceToggles(VulkanBackend,
-                                       {"nonzero_clear_resources_on_creation_for_testing"},
+                                   {"nonzero_clear_resources_on_creation_for_testing"},
-                                       {"lazy_clear_resource_on_first_use"}));
+                                   {"lazy_clear_resource_on_first_use"}));

src/tests/end2end/RenderPassTests.cpp

@@ -167,7 +167,7 @@ TEST_P(RenderPassTest, NoCorrespondingFragmentShaderOutputs) {
 
 DAWN_INSTANTIATE_TEST(RenderPassTest,
                       D3D12Backend,
-                      ForceWorkarounds(D3D12Backend, {}, {"use_d3d12_render_pass"}),
+                      ForceToggles(D3D12Backend, {}, {"use_d3d12_render_pass"}),
                       MetalBackend,
                       OpenGLBackend,
                       VulkanBackend);

src/tests/end2end/TextureZeroInitTests.cpp

@@ -762,9 +762,9 @@ TEST_P(TextureZeroInitTest, RenderingLoadingDepthStencilStoreOpClear) {
 
 DAWN_INSTANTIATE_TEST(
     TextureZeroInitTest,
-    ForceWorkarounds(D3D12Backend, {"nonzero_clear_resources_on_creation_for_testing"}),
+    ForceToggles(D3D12Backend, {"nonzero_clear_resources_on_creation_for_testing"}),
-    ForceWorkarounds(D3D12Backend,
+    ForceToggles(D3D12Backend,
-                     {"nonzero_clear_resources_on_creation_for_testing"},
+                 {"nonzero_clear_resources_on_creation_for_testing"},
-                     {"use_d3d12_render_pass"}),
+                 {"use_d3d12_render_pass"}),
-    ForceWorkarounds(OpenGLBackend, {"nonzero_clear_resources_on_creation_for_testing"}),
+    ForceToggles(OpenGLBackend, {"nonzero_clear_resources_on_creation_for_testing"}),
-    ForceWorkarounds(VulkanBackend, {"nonzero_clear_resources_on_creation_for_testing"}));
+    ForceToggles(VulkanBackend, {"nonzero_clear_resources_on_creation_for_testing"}));

src/tests/perf_tests/DrawCallPerf.cpp

@@ -607,7 +607,7 @@ TEST_P(DrawCallPerf, Run) {
 DAWN_INSTANTIATE_PERF_TEST_SUITE_P(
     DrawCallPerf,
     {D3D12Backend, MetalBackend, OpenGLBackend, VulkanBackend,
-     ForceWorkarounds(VulkanBackend, {"skip_validation"})},
+     ForceToggles(VulkanBackend, {"skip_validation"})},
     {
         // Baseline
         MakeParam(),