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>
diff --git a/dawn.json b/dawn.json
index 77ef423..b9c823d 100644
--- a/dawn.json
+++ b/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": "dimension", "type": "texture view dimension"},
+ {"name": "format", "type": "texture format", "default": "none"},
+ {"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"
diff --git a/generator/dawn_json_generator.py b/generator/dawn_json_generator.py
index 44d887e..eaa324c 100644
--- a/generator/dawn_json_generator.py
+++ b/generator/dawn_json_generator.py
@@ -61,11 +61,11 @@
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()
diff --git a/generator/templates/dawn_native/ValidationUtils.cpp b/generator/templates/dawn_native/ValidationUtils.cpp
index ac98da4..2529957 100644
--- a/generator/templates/dawn_native/ValidationUtils.cpp
+++ b/generator/templates/dawn_native/ValidationUtils.cpp
@@ -19,7 +19,7 @@
{% 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 %}
diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp
index b02bda1e..558bb17 100644
--- a/src/dawn_native/Device.cpp
+++ b/src/dawn_native/Device.cpp
@@ -660,8 +660,10 @@
MaybeError DeviceBase::CreateTextureViewInternal(TextureViewBase** result,
TextureBase* texture,
const TextureViewDescriptor* descriptor) {
- DAWN_TRY(ValidateTextureViewDescriptor(this, texture, descriptor));
- DAWN_TRY_ASSIGN(*result, CreateTextureViewImpl(texture, descriptor));
+ DAWN_TRY(ValidateObject(texture));
+ TextureViewDescriptor desc = GetTextureViewDescriptorWithDefaults(texture, descriptor);
+ DAWN_TRY(ValidateTextureViewDescriptor(texture, &desc));
+ DAWN_TRY_ASSIGN(*result, CreateTextureViewImpl(texture, &desc));
return {};
}
diff --git a/src/dawn_native/Texture.cpp b/src/dawn_native/Texture.cpp
index 87433ad..64ba7a9 100644
--- a/src/dawn_native/Texture.cpp
+++ b/src/dawn_native/Texture.cpp
@@ -139,28 +139,20 @@
return {};
}
- TextureViewDescriptor MakeDefaultTextureViewDescriptor(const TextureBase* texture) {
- TextureViewDescriptor descriptor;
- descriptor.format = texture->GetFormat().format;
- descriptor.baseArrayLayer = 0;
- descriptor.arrayLayerCount = texture->GetArrayLayers();
- descriptor.baseMipLevel = 0;
- descriptor.mipLevelCount = texture->GetNumMipLevels();
-
+ dawn::TextureViewDimension GetDefaultViewDimension(const TextureBase* texture) {
// 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 @@
return {};
}
- MaybeError ValidateTextureViewDescriptor(const DeviceBase* device,
- const TextureBase* texture,
+ MaybeError ValidateTextureViewDescriptor(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 @@
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 @@
TextureViewDescriptor descriptor = {};
if (!IsError()) {
- descriptor = MakeDefaultTextureViewDescriptor(this);
+ descriptor = GetTextureViewDescriptorWithDefaults(this, nullptr);
}
return GetDevice()->CreateTextureView(this, &descriptor);
diff --git a/src/dawn_native/Texture.h b/src/dawn_native/Texture.h
index e5c59ee..0e5f8cf 100644
--- a/src/dawn_native/Texture.h
+++ b/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,
- const TextureBase* texture,
+ MaybeError ValidateTextureViewDescriptor(const TextureBase* texture,
const TextureViewDescriptor* descriptor);
+ TextureViewDescriptor GetTextureViewDescriptorWithDefaults(
+ const TextureBase* texture,
+ const TextureViewDescriptor* descriptor);
bool IsValidSampleCount(uint32_t sampleCount);
diff --git a/src/tests/unittests/validation/BindGroupValidationTests.cpp b/src/tests/unittests/validation/BindGroupValidationTests.cpp
index 423dff1..e9cfb84 100644
--- a/src/tests/unittests/validation/BindGroupValidationTests.cpp
+++ b/src/tests/unittests/validation/BindGroupValidationTests.cpp
@@ -221,7 +221,7 @@
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));
diff --git a/src/tests/unittests/validation/RenderBundleValidationTests.cpp b/src/tests/unittests/validation/RenderBundleValidationTests.cpp
index 3314bfc7..271878d 100644
--- a/src/tests/unittests/validation/RenderBundleValidationTests.cpp
+++ b/src/tests/unittests/validation/RenderBundleValidationTests.cpp
@@ -587,6 +587,20 @@
}
}
+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);
diff --git a/src/tests/unittests/validation/TextureValidationTests.cpp b/src/tests/unittests/validation/TextureValidationTests.cpp
index 94cd40d..a5e38b4 100644
--- a/src/tests/unittests/validation/TextureValidationTests.cpp
+++ b/src/tests/unittests/validation/TextureValidationTests.cpp
@@ -257,6 +257,13 @@
}
}
+// 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 {
diff --git a/src/tests/unittests/validation/TextureViewValidationTests.cpp b/src/tests/unittests/validation/TextureViewValidationTests.cpp
index 5871a9b..f2872d9 100644
--- a/src/tests/unittests/validation/TextureViewValidationTests.cpp
+++ b/src/tests/unittests/validation/TextureViewValidationTests.cpp
@@ -69,8 +69,7 @@
texture.CreateView(&descriptor);
}
- // It is an error to specify the layer count of the texture view > 1 when texture view dimension
- // is 2D.
+ // It is an error to view a layer past the end of the texture.
{
dawn::TextureViewDescriptor descriptor = base2DTextureViewDescriptor;
descriptor.arrayLayerCount = 2;
@@ -85,19 +84,36 @@
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 @@
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);
}
}
