Skip testing of SNORM formats where non-readable.

SNORM textures are non-renderable on vanilla ES, which means they're also
non-readable. Use the EXT_render_snorm extension where available, otherwise
skip the test for now.

Bug: dawn:624 dawn:636 dawn:647 dawn:667
Change-Id: Ic50368032d6168060b6b52889b4ba952ce662f02
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/40420
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Stephen White <senorblanco@chromium.org>

src/dawn_native/Toggles.cpp

@@ -125,6 +125,10 @@ namespace dawn_native {
               "Disables the use of indexed draw buffer state which is unsupported on some "
               "platforms.",
               "https://crbug.com/dawn/582"}},
+            {Toggle::DisableSnormRead,
+             {"disable_snorm_read",
+              "Disables reading from Snorm textures which is unsupported on some platforms.",
+              "https://crbug.com/dawn/667"}},
             {Toggle::UseD3D12SmallShaderVisibleHeapForTesting,
              {"use_d3d12_small_shader_visible_heap",
               "Enable use of a small D3D12 shader visible heap, instead of using a large one by "

src/dawn_native/Toggles.h

@@ -40,6 +40,7 @@ namespace dawn_native {
         DisableBaseVertex,
         DisableBaseInstance,
         DisableIndexedDrawBuffers,
+        DisableSnormRead,
         UseD3D12SmallShaderVisibleHeapForTesting,
         UseDXC,
         DisableRobustness,

src/dawn_native/opengl/CommandBufferGL.cpp

@@ -519,6 +519,11 @@ namespace dawn_native { namespace opengl {
             gl.DeleteFramebuffers(1, &readFBO);
             gl.DeleteFramebuffers(1, &drawFBO);
         }
+        bool TextureFormatIsSnorm(wgpu::TextureFormat format) {
+            return format == wgpu::TextureFormat::RGBA8Snorm ||
+                   format == wgpu::TextureFormat::RG8Snorm ||
+                   format == wgpu::TextureFormat::R8Snorm;
+        }
     }  // namespace
 
     CommandBuffer::CommandBuffer(CommandEncoder* encoder, const CommandBufferDescriptor* descriptor)
@@ -752,9 +757,12 @@ namespace dawn_native { namespace opengl {
                     const GLFormat& format = texture->GetGLFormat();
                     GLenum target = texture->GetGLTarget();
 
-                    // TODO(jiawei.shao@intel.com): support texture-to-buffer copy with compressed
+                    // TODO(crbug.com/dawn/667): Implement validation in WebGPU/Compat to
+                    // avoid this codepath. OpenGL does not support readback from non-renderable
                     // texture formats.
-                    if (formatInfo.isCompressed) {
+                    if (formatInfo.isCompressed ||
+                        (TextureFormatIsSnorm(formatInfo.format) &&
+                         GetDevice()->IsToggleEnabled(Toggle::DisableSnormRead))) {
                         UNREACHABLE();
                     }
 

src/dawn_native/opengl/DeviceGL.cpp

@@ -68,6 +68,9 @@ namespace dawn_native { namespace opengl {
         // TODO(crbug.com/dawn/582): Use OES_draw_buffers_indexed where available.
         bool supportsIndexedDrawBuffers = gl.IsAtLeastGLES(3, 2) || gl.IsAtLeastGL(3, 0);
 
+        bool supportsSnormRead =
+            gl.IsAtLeastGL(4, 4) || gl.IsGLExtensionSupported("GL_EXT_render_snorm");
+
         // TODO(crbug.com/dawn/343): We can support the extension variants, but need to load the EXT
         // procs without the extension suffix.
         // We'll also need emulation of shader builtins gl_BaseVertex and gl_BaseInstance.
@@ -86,6 +89,7 @@ namespace dawn_native { namespace opengl {
         SetToggle(Toggle::DisableBaseVertex, !supportsBaseVertex);
         SetToggle(Toggle::DisableBaseInstance, !supportsBaseInstance);
         SetToggle(Toggle::DisableIndexedDrawBuffers, !supportsIndexedDrawBuffers);
+        SetToggle(Toggle::DisableSnormRead, !supportsSnormRead);
         SetToggle(Toggle::FlushBeforeClientWaitSync, gl.GetVersion().IsES());
     }
 

src/tests/end2end/NonzeroTextureCreationTests.cpp

@@ -117,6 +117,10 @@ TEST_P(NonzeroTextureCreationTests, ArrayLayerClears) {
 
 // Test that nonrenderable texture formats clear 0x01 because toggle is enabled
 TEST_P(NonzeroTextureCreationTests, NonrenderableTextureFormat) {
+    // TODO(crbug.com/dawn/667): Work around the fact that some platforms do not support reading
+    // from Snorm textures.
+    DAWN_SKIP_TEST_IF(HasToggleEnabled("disable_snorm_read"));
+
     wgpu::TextureDescriptor descriptor;
     descriptor.dimension = wgpu::TextureDimension::e2D;
     descriptor.size.width = kSize;
@@ -151,6 +155,10 @@ TEST_P(NonzeroTextureCreationTests, NonrenderableTextureFormat) {
 // Test that textures with more than 1 array layers and nonrenderable texture formats clear to 0x01
 // because toggle is enabled
 TEST_P(NonzeroTextureCreationTests, NonRenderableTextureClearWithMultiArrayLayers) {
+    // TODO(crbug.com/dawn/667): Work around the fact that some platforms do not support reading
+    // from Snorm textures.
+    DAWN_SKIP_TEST_IF(HasToggleEnabled("disable_snorm_read"));
+
     wgpu::TextureDescriptor descriptor;
     descriptor.dimension = wgpu::TextureDimension::e2D;
     descriptor.size.width = kSize;

src/tests/end2end/StorageTextureTests.cpp

@@ -770,8 +770,9 @@ TEST_P(StorageTextureTests, ReadonlyStorageTextureInVertexShader) {
 
 // Test that read-only storage textures are supported in fragment shader.
 TEST_P(StorageTextureTests, ReadonlyStorageTextureInFragmentShader) {
-    // TODO(crbug.com/dawn/624): this test fails on GLES. Investigate why.
+    // TODO(crbug.com/dawn/672): Investigate why this test fails on Linux
-    DAWN_SKIP_TEST_IF(IsOpenGLES());
+    // NVidia OpenGLES drivers.
+    DAWN_SKIP_TEST_IF(IsNvidia() && IsLinux() && IsOpenGLES());
 
     for (wgpu::TextureFormat format : utils::kAllTextureFormats) {
         if (!utils::TextureFormatSupportsStorageTexture(format)) {
@@ -807,20 +808,22 @@ TEST_P(StorageTextureTests, ReadonlyStorageTextureInFragmentShader) {
 
 // Test that write-only storage textures are supported in compute shader.
 TEST_P(StorageTextureTests, WriteonlyStorageTextureInComputeShader) {
-    // TODO(crbug.com/dawn/647): diagnose and fix this OpenGL ES failure.
-    DAWN_SKIP_TEST_IF(IsOpenGLES());
-
     for (wgpu::TextureFormat format : utils::kAllTextureFormats) {
         if (!utils::TextureFormatSupportsStorageTexture(format)) {
             continue;
         }
-        if (!OpenGLESSupportsStorageTexture(format)) {
+        if (IsOpenGLES() && !OpenGLESSupportsStorageTexture(format)) {
             continue;
         }
 
-        // TODO(jiawei.shao@intel.com): investigate why this test fails with RGBA8Snorm on Linux
+        if (format == wgpu::TextureFormat::RGBA8Snorm && HasToggleEnabled("disable_snorm_read")) {
-        // Intel OpenGL driver.
+            continue;
-        if (format == wgpu::TextureFormat::RGBA8Snorm && IsIntel() && IsOpenGL() && IsLinux()) {
+        }
+
+        // TODO(crbug.com/dawn/676): investigate why this test fails with RGBA8Snorm on Linux
+        // Intel OpenGL and OpenGLES drivers.
+        if (format == wgpu::TextureFormat::RGBA8Snorm && IsIntel() &&
+            (IsOpenGL() || IsOpenGLES()) && IsLinux()) {
             continue;
         }
 
@@ -878,8 +881,9 @@ TEST_P(StorageTextureTests, ReadWriteDifferentStorageTextureInOneDispatchInCompu
 
 // Test that write-only storage textures are supported in fragment shader.
 TEST_P(StorageTextureTests, WriteonlyStorageTextureInFragmentShader) {
-    // TODO(crbug.com/dawn/647): diagnose and fix this OpenGL ES failure.
+    // TODO(crbug.com/dawn/672): Investigate why this test fails on Linux
-    DAWN_SKIP_TEST_IF(IsOpenGLES());
+    // NVidia OpenGLES drivers.
+    DAWN_SKIP_TEST_IF(IsNvidia() && IsLinux() && IsOpenGLES());
 
     for (wgpu::TextureFormat format : utils::kAllTextureFormats) {
         if (!utils::TextureFormatSupportsStorageTexture(format)) {
@@ -889,9 +893,14 @@ TEST_P(StorageTextureTests, WriteonlyStorageTextureInFragmentShader) {
             continue;
         }
 
-        // TODO(jiawei.shao@intel.com): investigate why this test fails with RGBA8Snorm on Linux
+        if (format == wgpu::TextureFormat::RGBA8Snorm && HasToggleEnabled("disable_snorm_read")) {
-        // Intel OpenGL driver.
+            continue;
-        if (format == wgpu::TextureFormat::RGBA8Snorm && IsIntel() && IsOpenGL() && IsLinux()) {
+        }
+
+        // TODO(crbug.com/dawn/676): investigate why this test fails with RGBA8Snorm on Linux
+        // Intel OpenGL and OpenGLES drivers.
+        if (format == wgpu::TextureFormat::RGBA8Snorm && IsIntel() &&
+            (IsOpenGL() || IsOpenGLES()) && IsLinux()) {
             continue;
         }
 
@@ -1040,9 +1049,6 @@ TEST_P(StorageTextureTests, ReadonlyAndWriteonlyStorageTexturePingPong) {
 // Test that multiple dispatches to increment values by ping-ponging between a sampled texture and
 // a write-only storage texture are synchronized in one pass.
 TEST_P(StorageTextureTests, SampledAndWriteonlyStorageTexturePingPong) {
-    // TODO(crbug.com/dawn/636): diagnose and fix this failure on OpenGL ES
-    DAWN_SKIP_TEST_IF(IsOpenGLES());
-
     constexpr wgpu::TextureFormat kTextureFormat = wgpu::TextureFormat::R32Uint;
     wgpu::Texture storageTexture1 = CreateTexture(
         kTextureFormat,

src/tests/end2end/TextureZeroInitTests.cpp

@@ -1017,8 +1017,9 @@ TEST_P(TextureZeroInitTest, ComputePassSampledTextureClear) {
 
 // This tests that the code path of CopyTextureToBuffer clears correctly for non-renderable textures
 TEST_P(TextureZeroInitTest, NonRenderableTextureClear) {
-    // TODO(crbug.com/dawn/660): Diagnose and fix this failure on SwANGLE.
+    // TODO(crbug.com/dawn/667): Work around the fact that some platforms do not support reading
-    DAWN_SKIP_TEST_IF(IsANGLE());
+    // from Snorm textures.
+    DAWN_SKIP_TEST_IF(HasToggleEnabled("disable_snorm_read"));
 
     wgpu::TextureDescriptor descriptor =
         CreateTextureDescriptor(1, 1, wgpu::TextureUsage::CopySrc, kNonrenderableColorFormat);
@@ -1049,8 +1050,9 @@ TEST_P(TextureZeroInitTest, NonRenderableTextureClear) {
 
 // This tests that the code path of CopyTextureToBuffer clears correctly for non-renderable textures
 TEST_P(TextureZeroInitTest, NonRenderableTextureClearUnalignedSize) {
-    // TODO(crbug.com/dawn/660): Diagnose and fix this failure on SwANGLE.
+    // TODO(crbug.com/dawn/667): Work around the fact that some platforms do not support reading
-    DAWN_SKIP_TEST_IF(IsANGLE());
+    // from Snorm textures.
+    DAWN_SKIP_TEST_IF(HasToggleEnabled("disable_snorm_read"));
 
     wgpu::TextureDescriptor descriptor =
         CreateTextureDescriptor(1, 1, wgpu::TextureUsage::CopySrc, kNonrenderableColorFormat);
@@ -1084,8 +1086,9 @@ TEST_P(TextureZeroInitTest, NonRenderableTextureClearUnalignedSize) {
 // This tests that the code path of CopyTextureToBuffer clears correctly for non-renderable textures
 // with more than 1 array layers
 TEST_P(TextureZeroInitTest, NonRenderableTextureClearWithMultiArrayLayers) {
-    // TODO(crbug.com/dawn/660): Diagnose and fix this failure on SwANGLE.
+    // TODO(crbug.com/dawn/667): Work around the fact that some platforms do not support reading
-    DAWN_SKIP_TEST_IF(IsANGLE());
+    // from Snorm textures.
+    DAWN_SKIP_TEST_IF(HasToggleEnabled("disable_snorm_read"));
 
     wgpu::TextureDescriptor descriptor =
         CreateTextureDescriptor(1, 2, wgpu::TextureUsage::CopySrc, kNonrenderableColorFormat);
@@ -1427,6 +1430,10 @@ TEST_P(TextureZeroInitTest, PreservesInitializedArrayLayer) {
 // This is a regression test for crbug.com/dawn/451 where the lazy texture
 // init path on D3D12 had a divide-by-zero exception in the copy split logic.
 TEST_P(TextureZeroInitTest, CopyTextureToBufferNonRenderableUnaligned) {
+    // TODO(crbug.com/dawn/667): Work around the fact that some platforms do not support reading
+    // from Snorm textures.
+    DAWN_SKIP_TEST_IF(HasToggleEnabled("disable_snorm_read"));
+
     wgpu::TextureDescriptor descriptor;
     descriptor.size.width = kUnalignedSize;
     descriptor.size.height = kUnalignedSize;