From 76cd4332cced1767eadcb337fa4da7fa6cff5620 Mon Sep 17 00:00:00 2001 From: Stephen White Date: Fri, 10 Mar 2023 19:20:15 +0000 Subject: [PATCH] Vulkan: enable frag shader placeholder. The Vulkan spec mandates the use of a fragment shader unless rasterizer discard is enabled. For now, enable the placeholder fragment shader in all cases, with a bug logged to optimize it for the rasterizer discard case. Bug: dawn:1696 dawn:1698 Change-Id: I9e85e6308a9952fc505382488c618897bd9abc7b Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/123503 Reviewed-by: Corentin Wallez Kokoro: Kokoro Commit-Queue: Stephen White --- src/dawn/native/Toggles.cpp | 3 ++- src/dawn/native/vulkan/AdapterVk.cpp | 6 ++++++ src/dawn/tests/end2end/QueryTests.cpp | 4 ---- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/dawn/native/Toggles.cpp b/src/dawn/native/Toggles.cpp index 028ded6b0b..e72b2e2898 100644 --- a/src/dawn/native/Toggles.cpp +++ b/src/dawn/native/Toggles.cpp @@ -226,7 +226,8 @@ static constexpr ToggleEnumAndInfoList kToggleNameAndInfoList = {{ {Toggle::UsePlaceholderFragmentInVertexOnlyPipeline, {"use_placeholder_fragment_in_vertex_only_pipeline", "Use a placeholder empty fragment shader in vertex only render pipeline. This toggle must be " - "enabled for OpenGL ES backend, and serves as a workaround by default enabled on some Metal " + "enabled for OpenGL ES backend, the Vulkan Backend, and serves as a workaround by default " + "enabled on some Metal " "devices with Intel GPU to ensure the depth result is correct.", "https://crbug.com/dawn/136", ToggleStage::Device}}, {Toggle::FxcOptimizations, diff --git a/src/dawn/native/vulkan/AdapterVk.cpp b/src/dawn/native/vulkan/AdapterVk.cpp index 80d61121ba..d29136a7cb 100644 --- a/src/dawn/native/vulkan/AdapterVk.cpp +++ b/src/dawn/native/vulkan/AdapterVk.cpp @@ -447,6 +447,12 @@ void Adapter::SetupBackendDeviceToggles(TogglesState* deviceToggles) const { // By default try to initialize workgroup memory with OpConstantNull according to the Vulkan // extension VK_KHR_zero_initialize_workgroup_memory. deviceToggles->Default(Toggle::VulkanUseZeroInitializeWorkgroupMemoryExtension, true); + + // Inject fragment shaders in all vertex-only pipelines. + // TODO(crbug.com/dawn/1698): relax this requirement where the Vulkan spec allows. + // In particular, enable rasterizer discard if the depth-stencil stage is a no-op, and skip + // insertion of the placeholder fragment shader. + deviceToggles->Default(Toggle::UsePlaceholderFragmentInVertexOnlyPipeline, true); } ResultOrError> Adapter::CreateDeviceImpl(const DeviceDescriptor* descriptor, diff --git a/src/dawn/tests/end2end/QueryTests.cpp b/src/dawn/tests/end2end/QueryTests.cpp index d4ebbbd25b..dd63fa53b7 100644 --- a/src/dawn/tests/end2end/QueryTests.cpp +++ b/src/dawn/tests/end2end/QueryTests.cpp @@ -864,10 +864,6 @@ TEST_P(TimestampQueryTests, TimestampWritesOnRenderPassWithNoPipline) { TEST_P(TimestampQueryTests, TimestampWritesOnRenderPassWithOnlyVertexStage) { DAWN_TEST_UNSUPPORTED_IF(HasToggleEnabled("use_placeholder_fragment_in_vertex_only_pipeline")); - // TODO(dawn:1696): VVL claims that this configuration is invalid without a fragment shader. - // Investigate and remove this suppression. - DAWN_SUPPRESS_TEST_IF(IsVulkan()); - wgpu::QuerySet querySet = CreateQuerySetForTimestamp(2); TestTimestampWritesOnRenderPass({{querySet, 0, wgpu::RenderPassTimestampLocation::Beginning}, {querySet, 1, wgpu::RenderPassTimestampLocation::End}},