Enable the optional feature bgra8unorm-storage on Metal

This patch adds the support of the optional feature "bgra8unorm-storage"
and enable it on  Metal.

Bug: dawn:1641
Test: dawn_end2end_tests
Change-Id: Id58cefd8735f46b8d1807376ebcfada10df2890e
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/111380
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>

dawn.json

@@ -1450,6 +1450,7 @@
             {"value": 8, "name": "indirect first instance"},
             {"value": 9, "name": "shader f16"},
             {"value": 10, "name": "RG11B10 ufloat renderable"},
+            {"value": 11, "name": "BGRA8 unorm storage"},
             {"value": 1001, "name": "dawn shader float 16", "tags": ["dawn"]},
             {"value": 1002, "name": "dawn internal usages", "tags": ["dawn"]},
             {"value": 1003, "name": "dawn multi planar formats", "tags": ["dawn"]},

src/dawn/native/Features.cpp

@@ -79,6 +79,9 @@ static constexpr FeatureEnumAndInfoList kFeatureNameAndInfoList = {{
       "Allows the RENDER_ATTACHMENT usage on textures with format \"rg11b10ufloat\", and also "
       "allows textures of that format to be multisampled.",
       "https://bugs.chromium.org/p/dawn/issues/detail?id=1518", FeatureInfo::FeatureState::Stable}},
+    {Feature::BGRA8UnormStorage,
+     {"bgra8unorm-storage", "Allows the STORAGE usage on textures with format \"bgra8unorm\".",
+      "https://bugs.chromium.org/p/dawn/issues/detail?id=1591", FeatureInfo::FeatureState::Stable}},
     {Feature::DawnInternalUsages,
      {"dawn-internal-usages",
       "Add internal usages to resources to affect how the texture is allocated, but not "
@@ -134,6 +137,8 @@ Feature FromAPIFeature(wgpu::FeatureName feature) {
             return Feature::ShaderF16;
         case wgpu::FeatureName::RG11B10UfloatRenderable:
             return Feature::RG11B10UfloatRenderable;
+        case wgpu::FeatureName::BGRA8UnormStorage:
+            return Feature::BGRA8UnormStorage;
     }
     return Feature::InvalidEnum;
 }
@@ -170,6 +175,8 @@ wgpu::FeatureName ToAPIFeature(Feature feature) {
             return wgpu::FeatureName::ShaderF16;
         case Feature::RG11B10UfloatRenderable:
             return wgpu::FeatureName::RG11B10UfloatRenderable;
+        case Feature::BGRA8UnormStorage:
+            return wgpu::FeatureName::BGRA8UnormStorage;
 
         case Feature::EnumCount:
             break;

src/dawn/native/Features.h

@@ -39,6 +39,7 @@ enum class Feature {
     IndirectFirstInstance,
     ShaderF16,
     RG11B10UfloatRenderable,
+    BGRA8UnormStorage,
 
     // Dawn-specific
     DawnInternalUsages,

src/dawn/native/Format.cpp

@@ -368,7 +368,9 @@ FormatTable BuildFormatTable(const DeviceBase* device) {
         AddColorFormat(wgpu::TextureFormat::RGBA8Snorm, false, true, false, false, 4, kAnyFloat, 4);
         AddColorFormat(wgpu::TextureFormat::RGBA8Uint, true, true, true, false, 4, SampleTypeBit::Uint, 4, 4, 1);
         AddColorFormat(wgpu::TextureFormat::RGBA8Sint, true, true, true, false, 4, SampleTypeBit::Sint, 4, 4, 1);
-        AddColorFormat(wgpu::TextureFormat::BGRA8Unorm, true, false, true, true, 4, kAnyFloat, 4, 8, 1);
+
+        bool BGRA8UnormSupportsStorageUsage = device->HasFeature(Feature::BGRA8UnormStorage);
+        AddColorFormat(wgpu::TextureFormat::BGRA8Unorm, true, BGRA8UnormSupportsStorageUsage, true, true, 4, kAnyFloat, 4, 8, 1);
         AddColorFormat(wgpu::TextureFormat::BGRA8UnormSrgb, true, false, true, true, 4, kAnyFloat, 4, 8, 1, wgpu::TextureFormat::BGRA8Unorm);
         AddColorFormat(wgpu::TextureFormat::RGB10A2Unorm, true, false, true, true, 4, kAnyFloat, 4, 8, 4);
 

src/dawn/native/ShaderModule.cpp

@@ -730,8 +730,10 @@ ResultOrError<std::unique_ptr<EntryPointMetadata>> ReflectEntryPointUsingTint(
                 info.storageTexture.viewDimension =
                     TintTextureDimensionToTextureViewDimension(resource.dim);
 
-                DAWN_INVALID_IF(info.storageTexture.format == wgpu::TextureFormat::BGRA8Unorm,
+                DAWN_INVALID_IF(info.storageTexture.format == wgpu::TextureFormat::BGRA8Unorm &&
-                                "BGRA8Unorm storage textures are not yet supported.");
+                                    !device->HasFeature(Feature::BGRA8UnormStorage),
+                                "BGRA8Unorm storage textures are not supported if optional feature "
+                                "bgra8unorm-storage is not supported.");
 
                 break;
             case BindingInfoType::ExternalTexture:

src/dawn/native/metal/BackendMTL.mm

@@ -512,6 +512,7 @@ class Adapter : public AdapterBase {
         mSupportedFeatures.EnableFeature(Feature::IndirectFirstInstance);
         mSupportedFeatures.EnableFeature(Feature::ShaderF16);
         mSupportedFeatures.EnableFeature(Feature::RG11B10UfloatRenderable);
+        mSupportedFeatures.EnableFeature(Feature::BGRA8UnormStorage);
     }
 
     void InitializeVendorArchitectureImpl() override {

src/dawn/node/binding/Converter.cpp

@@ -1652,8 +1652,8 @@ bool Converter::Convert(wgpu::FeatureName& out, interop::GPUFeatureName in) {
             out = wgpu::FeatureName::RG11B10UfloatRenderable;
             return true;
         case interop::GPUFeatureName::kBgra8UnormStorage:
-            // TODO(dawn:1123) Add support for these extensions when possible.
+            out = wgpu::FeatureName::BGRA8UnormStorage;
-            return false;
+            return true;
     }
     return false;
 }
@@ -1687,6 +1687,9 @@ bool Converter::Convert(interop::GPUFeatureName& out, wgpu::FeatureName in) {
         case wgpu::FeatureName::RG11B10UfloatRenderable:
             out = interop::GPUFeatureName::kRg11B10UfloatRenderable;
             return true;
+        case wgpu::FeatureName::BGRA8UnormStorage:
+            out = interop::GPUFeatureName::kBgra8UnormStorage;
+            return true;
 
         case wgpu::FeatureName::PipelineStatisticsQuery:
         case wgpu::FeatureName::DawnShaderFloat16:

src/dawn/tests/end2end/StorageTextureTests.cpp

@@ -148,6 +148,13 @@ class StorageTextureTests : public DawnTest {
                 break;
             }
 
+            case wgpu::TextureFormat::BGRA8Unorm: {
+                utils::RGBA8* valuePtr = static_cast<utils::RGBA8*>(pixelValuePtr);
+                *valuePtr =
+                    utils::RGBA8(pixelValue * 3, pixelValue * 2, pixelValue, pixelValue * 4);
+                break;
+            }
+
             case wgpu::TextureFormat::RGBA8Snorm:
             case wgpu::TextureFormat::RGBA8Sint: {
                 int8_t* valuePtr = static_cast<int8_t*>(pixelValuePtr);
@@ -237,6 +244,7 @@ class StorageTextureTests : public DawnTest {
 
             // normalized signed/unsigned integer formats
             case wgpu::TextureFormat::RGBA8Unorm:
+            case wgpu::TextureFormat::BGRA8Unorm:
                 return "vec4f(f32(value) / 255.0, f32(value) / 255.0 * 2.0, "
                        "f32(value) / 255.0 * 3.0, f32(value) / 255.0 * 4.0)";
 
@@ -865,6 +873,70 @@ DAWN_INSTANTIATE_TEST(StorageTextureTests,
                       OpenGLESBackend(),
                       VulkanBackend());
 
+class BGRA8UnormStorageTextureTests : public StorageTextureTests {
+  public:
+    std::vector<wgpu::FeatureName> GetRequiredFeatures() override {
+        if (SupportsFeatures({wgpu::FeatureName::BGRA8UnormStorage})) {
+            mIsBGRA8UnormStorageSupported = true;
+            return {wgpu::FeatureName::BGRA8UnormStorage};
+        } else {
+            mIsBGRA8UnormStorageSupported = false;
+            return {};
+        }
+    }
+
+    bool IsBGRA8UnormStorageSupported() { return mIsBGRA8UnormStorageSupported; }
+
+  private:
+    bool mIsBGRA8UnormStorageSupported = false;
+};
+
+// Test that BGRA8Unorm is supported to be used as storage texture in compute shaders when the
+// optional feature 'bgra8unorm-storage' is supported.
+TEST_P(BGRA8UnormStorageTextureTests, WriteonlyStorageTextureInComputeShader) {
+    DAWN_TEST_UNSUPPORTED_IF(!IsBGRA8UnormStorageSupported());
+
+    constexpr wgpu::TextureFormat kFormat = wgpu::TextureFormat::BGRA8Unorm;
+    wgpu::Texture writeonlyStorageTexture =
+        CreateTexture(kFormat, wgpu::TextureUsage::StorageBinding | wgpu::TextureUsage::CopySrc,
+                      {kWidth, kHeight});
+
+    // Write the expected pixel values into the write-only storage texture.
+    const std::string computeShader = CommonWriteOnlyTestCode("compute", kFormat);
+    WriteIntoStorageTextureInComputePass(writeonlyStorageTexture, computeShader.c_str());
+
+    // Verify the pixel data in the write-only storage texture is expected.
+    CheckOutputStorageTexture(writeonlyStorageTexture, kFormat, {kWidth, kHeight});
+}
+
+// Test that BGRA8Unorm is supported to be used as storage texture in fragment shaders when the
+// optional feature 'bgra8unorm-storage' is supported.
+TEST_P(BGRA8UnormStorageTextureTests, WriteonlyStorageTextureInFragmentShader) {
+    DAWN_TEST_UNSUPPORTED_IF(!IsBGRA8UnormStorageSupported());
+
+    constexpr wgpu::TextureFormat kFormat = wgpu::TextureFormat::BGRA8Unorm;
+
+    // Prepare the write-only storage texture.
+    wgpu::Texture writeonlyStorageTexture =
+        CreateTexture(kFormat, wgpu::TextureUsage::StorageBinding | wgpu::TextureUsage::CopySrc,
+                      {kWidth, kHeight});
+
+    // Write the expected pixel values into the write-only storage texture.
+    const std::string fragmentShader = CommonWriteOnlyTestCode("fragment", kFormat);
+    WriteIntoStorageTextureInRenderPass(writeonlyStorageTexture, kSimpleVertexShader,
+                                        fragmentShader.c_str());
+
+    // Verify the pixel data in the write-only storage texture is expected.
+    CheckOutputStorageTexture(writeonlyStorageTexture, kFormat, {kWidth, kHeight});
+}
+
+DAWN_INSTANTIATE_TEST(BGRA8UnormStorageTextureTests,
+                      D3D12Backend(),
+                      MetalBackend(),
+                      OpenGLBackend(),
+                      OpenGLESBackend(),
+                      VulkanBackend());
+
 class StorageTextureZeroInitTests : public StorageTextureTests {
   public:
     static std::vector<uint8_t> GetExpectedData() {

src/dawn/tests/unittests/validation/StorageTextureValidationTests.cpp

@@ -233,9 +233,9 @@ TEST_F(StorageTextureValidationTests, BindGroupLayoutWithStorageTextureBindingTy
 // Validate it is an error to declare a read-only or write-only storage texture in shaders with any
 // format that doesn't support TextureUsage::StorageBinding texture usages.
 TEST_F(StorageTextureValidationTests, StorageTextureFormatInShaders) {
-    // Not include RGBA8UnormSrgb, BGRA8Unorm, BGRA8UnormSrgb because they are not related to any
+    // Not include RGBA8UnormSrgb, BGRA8UnormSrgb because they are neither related to any SPIR-V
-    // SPIR-V Image Formats.
+    // Image Formats nor WGSL texture formats.
-    constexpr std::array<wgpu::TextureFormat, 32> kWGPUTextureFormatSupportedAsSPIRVImageFormats = {
+    constexpr std::array<wgpu::TextureFormat, 33> kWGPUTextureFormatSupportedAsSPIRVImageFormats = {
         wgpu::TextureFormat::R32Uint,      wgpu::TextureFormat::R32Sint,
         wgpu::TextureFormat::R32Float,     wgpu::TextureFormat::RGBA8Unorm,
         wgpu::TextureFormat::RGBA8Snorm,   wgpu::TextureFormat::RGBA8Uint,
@@ -251,7 +251,8 @@ TEST_F(StorageTextureValidationTests, StorageTextureFormatInShaders) {
         wgpu::TextureFormat::RG8Snorm,     wgpu::TextureFormat::RG8Uint,
         wgpu::TextureFormat::RG8Sint,      wgpu::TextureFormat::RG16Uint,
         wgpu::TextureFormat::RG16Sint,     wgpu::TextureFormat::RG16Float,
-        wgpu::TextureFormat::RGB10A2Unorm, wgpu::TextureFormat::RG11B10Ufloat};
+        wgpu::TextureFormat::RGB10A2Unorm, wgpu::TextureFormat::RG11B10Ufloat,
+        wgpu::TextureFormat::BGRA8Unorm};
 
     for (wgpu::StorageTextureAccess storageTextureBindingType : kSupportedStorageTextureAccess) {
         for (wgpu::TextureFormat format : kWGPUTextureFormatSupportedAsSPIRVImageFormats) {
@@ -266,6 +267,26 @@ TEST_F(StorageTextureValidationTests, StorageTextureFormatInShaders) {
     }
 }
 
+class BGRA8UnormStorageTextureInShaderValidationTests : public StorageTextureValidationTests {
+  protected:
+    WGPUDevice CreateTestDevice(dawn::native::Adapter dawnAdapter) override {
+        wgpu::DeviceDescriptor descriptor;
+        wgpu::FeatureName requiredFeatures[1] = {wgpu::FeatureName::BGRA8UnormStorage};
+        descriptor.requiredFeatures = requiredFeatures;
+        descriptor.requiredFeaturesCount = 1;
+        return dawnAdapter.CreateDevice(&descriptor);
+    }
+};
+
+// Test that 'bgra8unorm' is a valid storage texture format if 'bgra8unorm-storage' is enabled.
+TEST_F(BGRA8UnormStorageTextureInShaderValidationTests, BGRA8UnormAsStorageInShader) {
+    for (wgpu::StorageTextureAccess storageTextureBindingType : kSupportedStorageTextureAccess) {
+        std::string computeShader = CreateComputeShaderWithStorageTexture(
+            storageTextureBindingType, wgpu::TextureFormat::BGRA8Unorm);
+        utils::CreateShaderModule(device, computeShader.c_str());
+    }
+}
+
 // Verify that declaring a storage texture format that is not supported in WebGPU causes validation
 // error.
 TEST_F(StorageTextureValidationTests, UnsupportedWGSLStorageTextureFormat) {
@@ -404,6 +425,32 @@ TEST_F(StorageTextureValidationTests, StorageTextureFormatInBindGroupLayout) {
     }
 }
 
+class BGRA8UnormStorageBindGroupLayoutTest : public StorageTextureValidationTests {
+  protected:
+    WGPUDevice CreateTestDevice(dawn::native::Adapter dawnAdapter) override {
+        wgpu::DeviceDescriptor descriptor;
+        wgpu::FeatureName requiredFeatures[1] = {wgpu::FeatureName::BGRA8UnormStorage};
+        descriptor.requiredFeatures = requiredFeatures;
+        descriptor.requiredFeaturesCount = 1;
+        return dawnAdapter.CreateDevice(&descriptor);
+    }
+};
+
+// Test that creating a bind group layout with BGRA8Unorm as storage texture format is valid when
+// The optional feature bgra8unorm-storage is supported.
+TEST_F(BGRA8UnormStorageBindGroupLayoutTest, BGRA8UnormAsStorage) {
+    wgpu::BindGroupLayoutEntry bindGroupLayoutBinding;
+    bindGroupLayoutBinding.binding = 0;
+    bindGroupLayoutBinding.visibility = wgpu::ShaderStage::Compute;
+    bindGroupLayoutBinding.storageTexture.format = wgpu::TextureFormat::BGRA8Unorm;
+    bindGroupLayoutBinding.storageTexture.viewDimension = wgpu::TextureViewDimension::e2D;
+
+    for (wgpu::StorageTextureAccess bindingType : kSupportedStorageTextureAccess) {
+        bindGroupLayoutBinding.storageTexture.access = bindingType;
+        utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding});
+    }
+}
+
 // Verify the storage texture format in the bind group layout must match the declaration in shader.
 TEST_F(StorageTextureValidationTests, BindGroupLayoutStorageTextureFormatMatchesShaderDeclaration) {
     for (wgpu::StorageTextureAccess bindingType : kSupportedStorageTextureAccess) {

src/dawn/tests/unittests/validation/TextureValidationTests.cpp

@@ -908,6 +908,27 @@ TEST_F(RG11B10UfloatTextureFormatsValidationTests, RenderableFeature) {
     device.CreateTexture(&descriptor);
 }
 
+class BGRA8UnormTextureFormatsValidationTests : public TextureValidationTest {
+  protected:
+    WGPUDevice CreateTestDevice(dawn::native::Adapter dawnAdapter) override {
+        wgpu::DeviceDescriptor descriptor;
+        wgpu::FeatureName requiredFeatures[1] = {wgpu::FeatureName::BGRA8UnormStorage};
+        descriptor.requiredFeatures = requiredFeatures;
+        descriptor.requiredFeaturesCount = 1;
+        return dawnAdapter.CreateDevice(&descriptor);
+    }
+};
+
+// Test that BGRA8Unorm format is valid as storage texture if 'bgra8unorm-storage' is enabled.
+TEST_F(BGRA8UnormTextureFormatsValidationTests, StorageFeature) {
+    wgpu::TextureDescriptor descriptor;
+    descriptor.size = {1, 1, 1};
+    descriptor.usage = wgpu::TextureUsage::StorageBinding;
+
+    descriptor.format = wgpu::TextureFormat::BGRA8Unorm;
+    device.CreateTexture(&descriptor);
+}
+
 static void CheckTextureMatchesDescriptor(const wgpu::Texture& tex,
                                           const wgpu::TextureDescriptor& desc) {
     EXPECT_EQ(desc.size.width, tex.GetWidth());

src/dawn/utils/TextureUtils.cpp

@@ -656,6 +656,8 @@ const char* GetWGSLImageFormatQualifier(wgpu::TextureFormat textureFormat) {
             return "rgba8uint";
         case wgpu::TextureFormat::RGBA8Sint:
             return "rgba8sint";
+        case wgpu::TextureFormat::BGRA8Unorm:
+            return "bgra8unorm";
         case wgpu::TextureFormat::RGBA16Uint:
             return "rgba16uint";
         case wgpu::TextureFormat::RGBA16Sint:

src/dawn/wire/SupportedFeatures.cpp

@@ -39,6 +39,7 @@ bool IsFeatureSupported(WGPUFeatureName feature) {
         case WGPUFeatureName_ChromiumExperimentalDp4a:
         case WGPUFeatureName_ShaderF16:
         case WGPUFeatureName_RG11B10UfloatRenderable:
+        case WGPUFeatureName_BGRA8UnormStorage:
             return true;
     }
 

webgpu-cts/expectations.txt

@@ -325,6 +325,15 @@ crbug.com/dawn/1500 [ intel-gen-9 monterey ] webgpu:api,operation,command_buffer
 ################################################################################
 crbug.com/dawn/1599 [ webgpu-adapter-swiftshader ] webgpu:web_platform,copyToTexture,canvas:color_space_conversion:* [ Failure ]
 
+################################################################################
+# Failures about the validation on the storage usage of bgra8unorm
+################################################################################
+crbug.com/dawn/1641 webgpu:api,validation,createBindGroupLayout:storage_texture,formats:format="bgra8unorm" [ Failure ]
+crbug.com/dawn/1641 webgpu:api,validation,createTexture:texture_usage:dimension="1d";format="bgra8unorm" [ Failure ]
+crbug.com/dawn/1641 webgpu:api,validation,createTexture:texture_usage:dimension="2d";format="bgra8unorm" [ Failure ]
+crbug.com/dawn/1641 webgpu:api,validation,createTexture:texture_usage:dimension="3d";format="bgra8unorm" [ Failure ]
+crbug.com/dawn/1641 webgpu:api,validation,createTexture:texture_usage:dimension="_undef_";format="bgra8unorm" [ Failure ]
+
 ################################################################################
 # untriaged failures
 ################################################################################