Implement new defaults for Texture::CreateView

https://github.com/gpuweb/gpuweb/pull/389 except for removing createDefaultView
https://github.com/gpuweb/gpuweb/pull/407

This will need to slightly change again when
https://github.com/gpuweb/gpuweb/pull/424 lands.

Bug: dawn:214
Change-Id: Id904b55cea6d77fcf7d971bd743468c7d82a9aa8
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/10440
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>

dawn.json

@@ -1137,7 +1137,7 @@
                 "name": "create view",
                 "returns": "texture view",
                 "args": [
-                    {"name": "descriptor", "type": "texture view descriptor", "annotation": "const*"}
+                    {"name": "descriptor", "type": "texture view descriptor", "annotation": "const*", "optional": true}
                 ]
             },
             {
@@ -1252,7 +1252,8 @@
             {"value": 48, "name": "BC6H RGB ufloat"},
             {"value": 49, "name": "BC6H RGB sfloat"},
             {"value": 50, "name": "BC7 RGBA unorm"},
-            {"value": 51, "name": "BC7 RGBA unorm srgb"}
+            {"value": 51, "name": "BC7 RGBA unorm srgb"},
+            {"value": 4294967295, "name": "none", "valid": false}
         ]
     },
     "texture usage bit": {
@@ -1271,12 +1272,12 @@
         "category": "structure",
         "extensible": true,
         "members": [
-            {"name": "format", "type": "texture format"},
+            {"name": "format", "type": "texture format", "default": "none"},
-            {"name": "dimension", "type": "texture view dimension"},
+            {"name": "dimension", "type": "texture view dimension", "default": "none"},
             {"name": "base mip level", "type": "uint32_t", "default": "0"},
-            {"name": "mip level count", "type": "uint32_t", "default": "1"},
+            {"name": "mip level count", "type": "uint32_t", "default": "0"},
             {"name": "base array layer", "type": "uint32_t", "default": "0"},
-            {"name": "array layer count", "type": "uint32_t", "default": "1"}
+            {"name": "array layer count", "type": "uint32_t", "default": "0"}
         ],
         "TODO": [
             "jiawei.shao@intel.com: Allow choosing the aspect (depth vs. stencil)"
@@ -1293,7 +1294,8 @@
             {"value": 2, "name": "2D array"},
             {"value": 3, "name": "cube"},
             {"value": 4, "name": "cube array"},
-            {"value": 5, "name": "3D"}
+            {"value": 5, "name": "3D"},
+            {"value": 4294967295, "name": "none", "valid": false}
         ],
         "TODO": [
             "jiawei.shao@intel.com: support 1D and 3D texture views"

generator/dawn_json_generator.py

@@ -61,11 +61,11 @@ class Type:
         self.name = Name(name, native=native)
         self.category = json_data['category']
 
-EnumValue = namedtuple('EnumValue', ['name', 'value'])
+EnumValue = namedtuple('EnumValue', ['name', 'value', 'valid'])
 class EnumType(Type):
     def __init__(self, name, json_data):
         Type.__init__(self, name, json_data)
-        self.values = [EnumValue(Name(m['name']), m['value']) for m in self.json_data['values']]
+        self.values = [EnumValue(Name(m['name']), m['value'], m.get('valid', True)) for m in self.json_data['values']]
 
         # Assert that all values are unique in enums
         all_values = set()

generator/templates/dawn_native/ValidationUtils.cpp

@@ -19,7 +19,7 @@ namespace dawn_native {
     {% for type in by_category["enum"] %}
         MaybeError Validate{{type.name.CamelCase()}}(dawn::{{as_cppType(type.name)}} value) {
             switch (value) {
-                {% for value in type.values %}
+                {% for value in type.values if value.valid %}
                     case dawn::{{as_cppType(type.name)}}::{{as_cppEnum(value.name)}}:
                         return {};
                 {% endfor %}

src/dawn_native/Device.cpp

@@ -660,8 +660,10 @@ namespace dawn_native {
     MaybeError DeviceBase::CreateTextureViewInternal(TextureViewBase** result,
                                                      TextureBase* texture,
                                                      const TextureViewDescriptor* descriptor) {
-        DAWN_TRY(ValidateTextureViewDescriptor(this, texture, descriptor));
+        DAWN_TRY(ValidateObject(texture));
-        DAWN_TRY_ASSIGN(*result, CreateTextureViewImpl(texture, descriptor));
+        TextureViewDescriptor desc = GetTextureViewDescriptorWithDefaults(texture, descriptor);
+        DAWN_TRY(ValidateTextureViewDescriptor(texture, &desc));
+        DAWN_TRY_ASSIGN(*result, CreateTextureViewImpl(texture, &desc));
         return {};
     }
 

src/dawn_native/Texture.cpp

@@ -139,28 +139,20 @@ namespace dawn_native {
             return {};
         }
 
-        TextureViewDescriptor MakeDefaultTextureViewDescriptor(const TextureBase* texture) {
+        dawn::TextureViewDimension GetDefaultViewDimension(const TextureBase* texture) {
-            TextureViewDescriptor descriptor;
-            descriptor.format = texture->GetFormat().format;
-            descriptor.baseArrayLayer = 0;
-            descriptor.arrayLayerCount = texture->GetArrayLayers();
-            descriptor.baseMipLevel = 0;
-            descriptor.mipLevelCount = texture->GetNumMipLevels();
-
             // TODO(jiawei.shao@intel.com): support all texture dimensions.
             switch (texture->GetDimension()) {
                 case dawn::TextureDimension::e2D:
+                    ASSERT(texture->GetArrayLayers() != 0);
                     if (texture->GetArrayLayers() == 1u) {
-                        descriptor.dimension = dawn::TextureViewDimension::e2D;
+                        return dawn::TextureViewDimension::e2D;
                     } else {
-                        descriptor.dimension = dawn::TextureViewDimension::e2DArray;
+                        return dawn::TextureViewDimension::e2DArray;
                     }
-                    break;
                 default:
                     UNREACHABLE();
+                    return dawn::TextureViewDimension::e1D;
             }
-
-            return descriptor;
         }
 
         MaybeError ValidateTextureSize(const TextureDescriptor* descriptor, const Format* format) {
@@ -238,14 +230,15 @@ namespace dawn_native {
         return {};
     }
 
-    MaybeError ValidateTextureViewDescriptor(const DeviceBase* device,
+    MaybeError ValidateTextureViewDescriptor(const TextureBase* texture,
-                                             const TextureBase* texture,
                                              const TextureViewDescriptor* descriptor) {
         if (descriptor->nextInChain != nullptr) {
             return DAWN_VALIDATION_ERROR("nextInChain must be nullptr");
         }
 
-        DAWN_TRY(device->ValidateObject(texture));
+        // Parent texture should have been already validated.
+        ASSERT(texture);
+        ASSERT(!texture->IsError());
         if (texture->GetTextureState() == TextureBase::TextureState::Destroyed) {
             return DAWN_VALIDATION_ERROR("Destroyed texture used to create texture view");
         }
@@ -279,6 +272,31 @@ namespace dawn_native {
         return {};
     }
 
+    TextureViewDescriptor GetTextureViewDescriptorWithDefaults(
+        const TextureBase* texture,
+        const TextureViewDescriptor* descriptor) {
+        ASSERT(texture);
+
+        TextureViewDescriptor desc = {};
+        if (descriptor) {
+            desc = *descriptor;
+        }
+
+        if (desc.format == dawn::TextureFormat::None) {
+            desc.format = texture->GetFormat().format;
+        }
+        if (desc.dimension == dawn::TextureViewDimension::None) {
+            desc.dimension = GetDefaultViewDimension(texture);
+        }
+        if (desc.arrayLayerCount == 0) {
+            desc.arrayLayerCount = texture->GetArrayLayers() - desc.baseArrayLayer;
+        }
+        if (desc.mipLevelCount == 0) {
+            desc.mipLevelCount = texture->GetNumMipLevels() - desc.baseMipLevel;
+        }
+        return desc;
+    }
+
     bool IsValidSampleCount(uint32_t sampleCount) {
         switch (sampleCount) {
             case 1:
@@ -439,7 +457,7 @@ namespace dawn_native {
         TextureViewDescriptor descriptor = {};
 
         if (!IsError()) {
-            descriptor = MakeDefaultTextureViewDescriptor(this);
+            descriptor = GetTextureViewDescriptorWithDefaults(this, nullptr);
         }
 
         return GetDevice()->CreateTextureView(this, &descriptor);

src/dawn_native/Texture.h

@@ -26,9 +26,11 @@
 namespace dawn_native {
     MaybeError ValidateTextureDescriptor(const DeviceBase* device,
                                          const TextureDescriptor* descriptor);
-    MaybeError ValidateTextureViewDescriptor(const DeviceBase* device,
+    MaybeError ValidateTextureViewDescriptor(const TextureBase* texture,
-                                             const TextureBase* texture,
                                              const TextureViewDescriptor* descriptor);
+    TextureViewDescriptor GetTextureViewDescriptorWithDefaults(
+        const TextureBase* texture,
+        const TextureViewDescriptor* descriptor);
 
     bool IsValidSampleCount(uint32_t sampleCount);
 

src/tests/unittests/validation/BindGroupValidationTests.cpp

@@ -221,7 +221,7 @@ TEST_F(BindGroupValidationTest, TextureBindingType) {
         viewDesc.baseMipLevel = 0;
         viewDesc.mipLevelCount = 0;
         viewDesc.baseArrayLayer = 0;
-        viewDesc.arrayLayerCount = 0;
+        viewDesc.arrayLayerCount = 1000;
 
         dawn::TextureView errorView;
         ASSERT_DEVICE_ERROR(errorView = mSampledTexture.CreateView(&viewDesc));

src/tests/unittests/validation/RenderBundleValidationTests.cpp

@@ -587,6 +587,20 @@ TEST_F(RenderBundleValidationTest, RequiresAtLeastOneTextureFormat) {
     }
 }
 
+TEST_F(RenderBundleValidationTest, ColorFormatNone) {
+    utils::ComboRenderBundleEncoderDescriptor desc = {};
+    desc.colorFormatsCount = 1;
+    desc.cColorFormats[0] = dawn::TextureFormat::None;
+    ASSERT_DEVICE_ERROR(device.CreateRenderBundleEncoder(&desc));
+}
+
+TEST_F(RenderBundleValidationTest, DepthStencilFormatNone) {
+    utils::ComboRenderBundleEncoderDescriptor desc = {};
+    const dawn::TextureFormat kFormatNone = dawn::TextureFormat::None;
+    desc.depthStencilFormat = &kFormatNone;
+    ASSERT_DEVICE_ERROR(device.CreateRenderBundleEncoder(&desc));
+}
+
 // Test that resource usages are validated inside render bundles.
 TEST_F(RenderBundleValidationTest, UsageTracking) {
     DummyRenderPass renderPass(device);

src/tests/unittests/validation/TextureValidationTests.cpp

@@ -257,6 +257,13 @@ TEST_F(TextureValidationTest, NonRenderableAndOutputAttachment) {
     }
 }
 
+// Test it is an error to create a texture with format "None".
+TEST_F(TextureValidationTest, TextureFormatNone) {
+    dawn::TextureDescriptor descriptor = CreateDefaultTextureDescriptor();
+    descriptor.format = dawn::TextureFormat::None;
+    ASSERT_DEVICE_ERROR(device.CreateTexture(&descriptor));
+}
+
 // TODO(jiawei.shao@intel.com): add tests to verify we cannot create 1D or 3D textures with
 // compressed texture formats.
 class CompressedTextureFormatsValidationTests : public TextureValidationTest {

src/tests/unittests/validation/TextureViewValidationTests.cpp

@@ -69,8 +69,7 @@ TEST_F(TextureViewValidationTest, CreateTextureViewOnTexture2D) {
         texture.CreateView(&descriptor);
     }
 
-    // It is an error to specify the layer count of the texture view > 1 when texture view dimension
+    // It is an error to view a layer past the end of the texture.
-    // is 2D.
     {
         dawn::TextureViewDescriptor descriptor = base2DTextureViewDescriptor;
         descriptor.arrayLayerCount = 2;
@@ -85,19 +84,36 @@ TEST_F(TextureViewValidationTest, CreateTextureViewOnTexture2D) {
         texture.CreateView(&descriptor);
     }
 
-    // It is an error to specify mipLevelCount == 0.
+    // baseMipLevel == k && mipLevelCount == 0 means to use levels k..end.
     {
         dawn::TextureViewDescriptor descriptor = base2DTextureViewDescriptor;
         descriptor.mipLevelCount = 0;
+
+        descriptor.baseMipLevel = 0;
+        texture.CreateView(&descriptor);
+        descriptor.baseMipLevel = 1;
+        texture.CreateView(&descriptor);
+        descriptor.baseMipLevel = kDefaultMipLevels - 1;
+        texture.CreateView(&descriptor);
+        descriptor.baseMipLevel = kDefaultMipLevels;
         ASSERT_DEVICE_ERROR(texture.CreateView(&descriptor));
     }
 
     // It is an error to make the mip level out of range.
     {
         dawn::TextureViewDescriptor descriptor = base2DTextureViewDescriptor;
+        descriptor.baseMipLevel = 0;
+        descriptor.mipLevelCount = kDefaultMipLevels + 1;
+        ASSERT_DEVICE_ERROR(texture.CreateView(&descriptor));
+        descriptor.baseMipLevel = 1;
+        descriptor.mipLevelCount = kDefaultMipLevels;
+        ASSERT_DEVICE_ERROR(texture.CreateView(&descriptor));
         descriptor.baseMipLevel = kDefaultMipLevels - 1;
         descriptor.mipLevelCount = 2;
         ASSERT_DEVICE_ERROR(texture.CreateView(&descriptor));
+        descriptor.baseMipLevel = kDefaultMipLevels;
+        descriptor.mipLevelCount = 1;
+        ASSERT_DEVICE_ERROR(texture.CreateView(&descriptor));
     }
 }
 
@@ -125,18 +141,121 @@ TEST_F(TextureViewValidationTest, CreateTextureViewOnTexture2DArray) {
         texture.CreateView(&descriptor);
     }
 
-    // It is an error to specify arrayLayerCount == 0.
+    // baseArrayLayer == k && arrayLayerCount == 0 means to use layers k..end.
     {
         dawn::TextureViewDescriptor descriptor = base2DArrayTextureViewDescriptor;
         descriptor.arrayLayerCount = 0;
+
+        descriptor.baseArrayLayer = 0;
+        texture.CreateView(&descriptor);
+        descriptor.baseArrayLayer = 1;
+        texture.CreateView(&descriptor);
+        descriptor.baseArrayLayer = kDefaultArrayLayers - 1;
+        texture.CreateView(&descriptor);
+        descriptor.baseArrayLayer = kDefaultArrayLayers;
         ASSERT_DEVICE_ERROR(texture.CreateView(&descriptor));
     }
 
-    // It is an error to make the array layer out of range.
+    // It is an error for the array layer range of the view to exceed that of the texture.
     {
         dawn::TextureViewDescriptor descriptor = base2DArrayTextureViewDescriptor;
+        descriptor.baseArrayLayer = 0;
         descriptor.arrayLayerCount = kDefaultArrayLayers + 1;
         ASSERT_DEVICE_ERROR(texture.CreateView(&descriptor));
+        descriptor.baseArrayLayer = 1;
+        descriptor.arrayLayerCount = kDefaultArrayLayers;
+        ASSERT_DEVICE_ERROR(texture.CreateView(&descriptor));
+        descriptor.baseArrayLayer = kDefaultArrayLayers - 1;
+        descriptor.arrayLayerCount = 2;
+        ASSERT_DEVICE_ERROR(texture.CreateView(&descriptor));
+        descriptor.baseArrayLayer = kDefaultArrayLayers;
+        descriptor.arrayLayerCount = 1;
+        ASSERT_DEVICE_ERROR(texture.CreateView(&descriptor));
+    }
+}
+
+// Using the "none" ("default") values validates the same as explicitly
+// specifying the values they're supposed to default to.
+// Variant for a texture with more than 1 array layer.
+TEST_F(TextureViewValidationTest, TextureViewDescriptorDefaultsArray) {
+    constexpr uint32_t kDefaultArrayLayers = 6;
+    dawn::Texture texture = Create2DArrayTexture(device, kDefaultArrayLayers);
+
+    {
+        dawn::TextureViewDescriptor descriptor;
+        texture.CreateView(&descriptor);
+    }
+    {
+        dawn::TextureViewDescriptor descriptor;
+        descriptor.format = dawn::TextureFormat::None;
+        texture.CreateView(&descriptor);
+        descriptor.format = dawn::TextureFormat::RGBA8Unorm;
+        texture.CreateView(&descriptor);
+        descriptor.format = dawn::TextureFormat::R8Unorm;
+        ASSERT_DEVICE_ERROR(texture.CreateView(&descriptor));
+    }
+    {
+        dawn::TextureViewDescriptor descriptor;
+        descriptor.dimension = dawn::TextureViewDimension::None;
+        texture.CreateView(&descriptor);
+        descriptor.dimension = dawn::TextureViewDimension::e2DArray;
+        texture.CreateView(&descriptor);
+        descriptor.dimension = dawn::TextureViewDimension::e2D;
+        ASSERT_DEVICE_ERROR(texture.CreateView(&descriptor));
+    }
+    {
+        dawn::TextureViewDescriptor descriptor;
+        descriptor.arrayLayerCount = kDefaultArrayLayers;
+        texture.CreateView(&descriptor);
+        descriptor.mipLevelCount = kDefaultMipLevels;
+        texture.CreateView(&descriptor);
+    }
+}
+
+// Using the "none" ("default") values validates the same as explicitly
+// specifying the values they're supposed to default to.
+// Variant for a texture with only 1 array layer.
+TEST_F(TextureViewValidationTest, TextureViewDescriptorDefaultsNonArray) {
+    constexpr uint32_t kDefaultArrayLayers = 1;
+    dawn::Texture texture = Create2DArrayTexture(device, kDefaultArrayLayers);
+
+    {
+        dawn::TextureViewDescriptor descriptor;
+        texture.CreateView(&descriptor);
+    }
+    {
+        dawn::TextureViewDescriptor descriptor;
+        descriptor.format = dawn::TextureFormat::None;
+        texture.CreateView(&descriptor);
+        descriptor.format = dawn::TextureFormat::RGBA8Unorm;
+        texture.CreateView(&descriptor);
+        descriptor.format = dawn::TextureFormat::R8Unorm;
+        ASSERT_DEVICE_ERROR(texture.CreateView(&descriptor));
+    }
+    {
+        dawn::TextureViewDescriptor descriptor;
+        descriptor.dimension = dawn::TextureViewDimension::None;
+        texture.CreateView(&descriptor);
+        descriptor.dimension = dawn::TextureViewDimension::e2D;
+        texture.CreateView(&descriptor);
+        descriptor.dimension = dawn::TextureViewDimension::e2DArray;
+        texture.CreateView(&descriptor);
+    }
+    {
+        dawn::TextureViewDescriptor descriptor;
+        descriptor.arrayLayerCount = 0;
+        texture.CreateView(&descriptor);
+        descriptor.arrayLayerCount = 1;
+        texture.CreateView(&descriptor);
+        descriptor.arrayLayerCount = 2;
+        ASSERT_DEVICE_ERROR(texture.CreateView(&descriptor));
+    }
+    {
+        dawn::TextureViewDescriptor descriptor;
+        descriptor.mipLevelCount = kDefaultMipLevels;
+        texture.CreateView(&descriptor);
+        descriptor.arrayLayerCount = kDefaultArrayLayers;
+        texture.CreateView(&descriptor);
     }
 }