From 84a57756db5378dadd3c63b4388f786c136c53eb Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Thu, 5 Dec 2019 14:02:11 +0000 Subject: [PATCH] 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 Reviewed-by: David Turner --- src/dawn_native/Toggles.cpp | 6 +++ src/dawn_native/Toggles.h | 1 + src/dawn_native/vulkan/DeviceVk.cpp | 40 ++++++++++++++++++- src/dawn_native/vulkan/DeviceVk.h | 3 ++ src/dawn_native/vulkan/RenderPassCache.cpp | 6 +-- src/dawn_native/vulkan/TextureVk.cpp | 18 ++++++--- src/dawn_native/vulkan/TextureVk.h | 2 +- src/tests/DawnTest.cpp | 6 +-- src/tests/DawnTest.h | 6 +-- src/tests/end2end/BufferTests.cpp | 2 +- .../end2end/CompressedTextureFormatTests.cpp | 4 +- src/tests/end2end/DepthStencilStateTests.cpp | 9 +++-- .../end2end/MultisampledRenderingTests.cpp | 14 +++---- .../end2end/NonzeroTextureCreationTests.cpp | 18 ++++----- src/tests/end2end/RenderPassTests.cpp | 2 +- src/tests/end2end/TextureZeroInitTests.cpp | 12 +++--- src/tests/perf_tests/DrawCallPerf.cpp | 2 +- 17 files changed, 104 insertions(+), 47 deletions(-) diff --git a/src/dawn_native/Toggles.cpp b/src/dawn_native/Toggles.cpp index b46c9efd83..34c469278d 100644 --- a/src/dawn_native/Toggles.cpp +++ b/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 diff --git a/src/dawn_native/Toggles.h b/src/dawn_native/Toggles.h index aa5c4f9bcd..e97a2738b5 100644 --- a/src/dawn_native/Toggles.h +++ b/src/dawn_native/Toggles.h @@ -35,6 +35,7 @@ namespace dawn_native { SkipValidation, UseSpvc, UseSpvcIRGen, + VulkanUseD32S8, EnumCount, InvalidEnum = EnumCount, diff --git a/src/dawn_native/vulkan/DeviceVk.cpp b/src/dawn_native/vulkan/DeviceVk.cpp index a5f7788473..232d9b8dbf 100644 --- a/src/dawn_native/vulkan/DeviceVk.cpp +++ b/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)), diff --git a/src/dawn_native/vulkan/DeviceVk.h b/src/dawn_native/vulkan/DeviceVk.h index e5210d6b9f..13eb152587 100644 --- a/src/dawn_native/vulkan/DeviceVk.h +++ b/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. diff --git a/src/dawn_native/vulkan/RenderPassCache.cpp b/src/dawn_native/vulkan/RenderPassCache.cpp index 1f3f940379..87424589f2 100644 --- a/src/dawn_native/vulkan/RenderPassCache.cpp +++ b/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; diff --git a/src/dawn_native/vulkan/TextureVk.cpp b/src/dawn_native/vulkan/TextureVk.cpp index 5d11976455..1594417490 100644 --- a/src/dawn_native/vulkan/TextureVk.cpp +++ b/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()); diff --git a/src/dawn_native/vulkan/TextureVk.h b/src/dawn_native/vulkan/TextureVk.h index 82366ae0ef..f904452614 100644 --- a/src/dawn_native/vulkan/TextureVk.h +++ b/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); diff --git a/src/tests/DawnTest.cpp b/src/tests/DawnTest.cpp index 29ffccee09..5c3ca2984c 100644 --- a/src/tests/DawnTest.cpp +++ b/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, - std::initializer_list forceEnabledWorkarounds, - std::initializer_list forceDisabledWorkarounds) { +DawnTestParam ForceToggles(const DawnTestParam& originParam, + std::initializer_list forceEnabledWorkarounds, + std::initializer_list forceDisabledWorkarounds) { DawnTestParam newTestParam = originParam; newTestParam.forceEnabledWorkarounds = forceEnabledWorkarounds; newTestParam.forceDisabledWorkarounds = forceDisabledWorkarounds; diff --git a/src/tests/DawnTest.h b/src/tests/DawnTest.h index e65e14816d..73fe149f79 100644 --- a/src/tests/DawnTest.h +++ b/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, - std::initializer_list forceEnabledWorkarounds, - std::initializer_list forceDisabledWorkarounds = {}); +DawnTestParam ForceToggles(const DawnTestParam& originParam, + std::initializer_list forceEnabledWorkarounds, + std::initializer_list forceDisabledWorkarounds = {}); namespace utils { class TerribleCommandBuffer; diff --git a/src/tests/end2end/BufferTests.cpp b/src/tests/end2end/BufferTests.cpp index e75863e456..09640fa080 100644 --- a/src/tests/end2end/BufferTests.cpp +++ b/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); diff --git a/src/tests/end2end/CompressedTextureFormatTests.cpp b/src/tests/end2end/CompressedTextureFormatTests.cpp index 4a6b627e85..c7204628b1 100644 --- a/src/tests/end2end/CompressedTextureFormatTests.cpp +++ b/src/tests/end2end/CompressedTextureFormatTests.cpp @@ -1054,5 +1054,5 @@ DAWN_INSTANTIATE_TEST(CompressedTextureBCFormatTest, MetalBackend, OpenGLBackend, VulkanBackend, - ForceWorkarounds(VulkanBackend, - {"use_temporary_buffer_in_texture_to_texture_copy"})); + ForceToggles(VulkanBackend, + {"use_temporary_buffer_in_texture_to_texture_copy"})); diff --git a/src/tests/end2end/DepthStencilStateTests.cpp b/src/tests/end2end/DepthStencilStateTests.cpp index 1692172116..d48500427b 100644 --- a/src/tests/end2end/DepthStencilStateTests.cpp +++ b/src/tests/end2end/DepthStencilStateTests.cpp @@ -682,7 +682,8 @@ TEST_P(DepthStencilStateTest, StencilDepthPass) { } DAWN_INSTANTIATE_TEST(DepthStencilStateTest, - D3D12Backend, - MetalBackend, - OpenGLBackend, - VulkanBackend); + D3D12Backend, + MetalBackend, + OpenGLBackend, + ForceToggles(VulkanBackend, {"vulkan_use_d32s8"}, {}), + ForceToggles(VulkanBackend, {}, {"vulkan_use_d32s8"})); diff --git a/src/tests/end2end/MultisampledRenderingTests.cpp b/src/tests/end2end/MultisampledRenderingTests.cpp index 6c9d644da3..80db4642f2 100644 --- a/src/tests/end2end/MultisampledRenderingTests.cpp +++ b/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"}), - ForceWorkarounds(D3D12Backend, {}, {"use_d3d12_render_pass"}), + ForceToggles(D3D12Backend, {}, {"use_d3d12_resource_heap_tier2"}), + ForceToggles(D3D12Backend, {}, {"use_d3d12_render_pass"}), MetalBackend, OpenGLBackend, VulkanBackend, - ForceWorkarounds(MetalBackend, {"emulate_store_and_msaa_resolve"}), - ForceWorkarounds(MetalBackend, {"always_resolve_into_zero_level_and_layer"}), - ForceWorkarounds(MetalBackend, - {"always_resolve_into_zero_level_and_layer", - "emulate_store_and_msaa_resolve"})); + ForceToggles(MetalBackend, {"emulate_store_and_msaa_resolve"}), + ForceToggles(MetalBackend, {"always_resolve_into_zero_level_and_layer"}), + ForceToggles(MetalBackend, + {"always_resolve_into_zero_level_and_layer", + "emulate_store_and_msaa_resolve"})); diff --git a/src/tests/end2end/NonzeroTextureCreationTests.cpp b/src/tests/end2end/NonzeroTextureCreationTests.cpp index 2313f3f3eb..405a09fd78 100644 --- a/src/tests/end2end/NonzeroTextureCreationTests.cpp +++ b/src/tests/end2end/NonzeroTextureCreationTests.cpp @@ -166,12 +166,12 @@ TEST_P(NonzeroTextureCreationTests, NonRenderableTextureClearWithMultiArrayLayer } DAWN_INSTANTIATE_TEST(NonzeroTextureCreationTests, - ForceWorkarounds(D3D12Backend, - {"nonzero_clear_resources_on_creation_for_testing"}, - {"lazy_clear_resource_on_first_use"}), - ForceWorkarounds(OpenGLBackend, - {"nonzero_clear_resources_on_creation_for_testing"}, - {"lazy_clear_resource_on_first_use"}), - ForceWorkarounds(VulkanBackend, - {"nonzero_clear_resources_on_creation_for_testing"}, - {"lazy_clear_resource_on_first_use"})); + ForceToggles(D3D12Backend, + {"nonzero_clear_resources_on_creation_for_testing"}, + {"lazy_clear_resource_on_first_use"}), + ForceToggles(OpenGLBackend, + {"nonzero_clear_resources_on_creation_for_testing"}, + {"lazy_clear_resource_on_first_use"}), + ForceToggles(VulkanBackend, + {"nonzero_clear_resources_on_creation_for_testing"}, + {"lazy_clear_resource_on_first_use"})); diff --git a/src/tests/end2end/RenderPassTests.cpp b/src/tests/end2end/RenderPassTests.cpp index 223d99ba8d..6d8aa31886 100644 --- a/src/tests/end2end/RenderPassTests.cpp +++ b/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); diff --git a/src/tests/end2end/TextureZeroInitTests.cpp b/src/tests/end2end/TextureZeroInitTests.cpp index 13f1d9e7e7..b31f2b7b33 100644 --- a/src/tests/end2end/TextureZeroInitTests.cpp +++ b/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"}), - ForceWorkarounds(D3D12Backend, - {"nonzero_clear_resources_on_creation_for_testing"}, - {"use_d3d12_render_pass"}), - ForceWorkarounds(OpenGLBackend, {"nonzero_clear_resources_on_creation_for_testing"}), - ForceWorkarounds(VulkanBackend, {"nonzero_clear_resources_on_creation_for_testing"})); + ForceToggles(D3D12Backend, {"nonzero_clear_resources_on_creation_for_testing"}), + ForceToggles(D3D12Backend, + {"nonzero_clear_resources_on_creation_for_testing"}, + {"use_d3d12_render_pass"}), + ForceToggles(OpenGLBackend, {"nonzero_clear_resources_on_creation_for_testing"}), + ForceToggles(VulkanBackend, {"nonzero_clear_resources_on_creation_for_testing"})); diff --git a/src/tests/perf_tests/DrawCallPerf.cpp b/src/tests/perf_tests/DrawCallPerf.cpp index 50b54f1096..2a1eba7769 100644 --- a/src/tests/perf_tests/DrawCallPerf.cpp +++ b/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(),