From 86e74e0dc1c90574b4caf045ba97575d4012784e Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Tue, 10 Sep 2019 08:58:28 +0000 Subject: [PATCH] Implement BGLBinding::textureDimension In WebGPU the BGLBinding needs to know the texture dimension for compatibility reasons between the texture views passed and the pipelines. This adds the member and implements the restriction. BUG=dawn:22 Change-Id: I95204de1cd621c936994739e840c76351fea9035 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/10960 Reviewed-by: Austin Eng Reviewed-by: Kai Ninomiya Commit-Queue: Corentin Wallez --- dawn.json | 1 + src/dawn_native/BindGroup.cpp | 15 ++++--- src/dawn_native/BindGroupLayout.cpp | 15 ++++++- src/dawn_native/BindGroupLayout.h | 1 + src/dawn_native/Texture.cpp | 6 +++ src/dawn_native/Texture.h | 2 + src/tests/end2end/ObjectCachingTests.cpp | 23 ++++++++-- src/tests/end2end/TextureFormatTests.cpp | 2 +- src/tests/end2end/TextureViewTests.cpp | 44 +++++++++---------- .../validation/BindGroupValidationTests.cpp | 28 +++++++++++- .../unittests/wire/WireArgumentTests.cpp | 7 +-- 11 files changed, 106 insertions(+), 38 deletions(-) diff --git a/dawn.json b/dawn.json index 386d8adeab..c20a4c828c 100644 --- a/dawn.json +++ b/dawn.json @@ -58,6 +58,7 @@ {"name": "type", "type": "binding type"}, {"name": "dynamic", "type": "bool", "default": "false"}, {"name": "multisampled", "type": "bool", "default": "false"}, + {"name": "texture dimension", "type": "texture view dimension", "default": "undefined"}, {"name": "texture component type", "type": "texture component type", "default": "float"} ] }, diff --git a/src/dawn_native/BindGroup.cpp b/src/dawn_native/BindGroup.cpp index 15c971f535..ceaf913b9d 100644 --- a/src/dawn_native/BindGroup.cpp +++ b/src/dawn_native/BindGroup.cpp @@ -65,7 +65,8 @@ namespace dawn_native { const BindGroupBinding& binding, dawn::TextureUsage requiredUsage, bool multisampledBinding, - dawn::TextureComponentType requiredComponentType) { + dawn::TextureComponentType requiredComponentType, + dawn::TextureViewDimension requiredDimension) { if (binding.textureView == nullptr || binding.sampler != nullptr || binding.buffer != nullptr) { return DAWN_VALIDATION_ERROR("expected texture binding"); @@ -86,6 +87,10 @@ namespace dawn_native { return DAWN_VALIDATION_ERROR("texture component type usage mismatch"); } + if (binding.textureView->GetDimension() != requiredDimension) { + return DAWN_VALIDATION_ERROR("texture view dimension mismatch"); + } + return {}; } @@ -145,10 +150,10 @@ namespace dawn_native { DAWN_TRY(ValidateBufferBinding(device, binding, dawn::BufferUsage::Storage)); break; case dawn::BindingType::SampledTexture: - DAWN_TRY( - ValidateTextureBinding(device, binding, dawn::TextureUsage::Sampled, - layoutInfo.multisampled[bindingIndex], - layoutInfo.textureComponentTypes[bindingIndex])); + DAWN_TRY(ValidateTextureBinding(device, binding, dawn::TextureUsage::Sampled, + layoutInfo.multisampled[bindingIndex], + layoutInfo.textureComponentTypes[bindingIndex], + layoutInfo.textureDimensions[bindingIndex])); break; case dawn::BindingType::Sampler: DAWN_TRY(ValidateSamplerBinding(device, binding)); diff --git a/src/dawn_native/BindGroupLayout.cpp b/src/dawn_native/BindGroupLayout.cpp index 57bc2ecf03..ae602095c9 100644 --- a/src/dawn_native/BindGroupLayout.cpp +++ b/src/dawn_native/BindGroupLayout.cpp @@ -38,6 +38,10 @@ namespace dawn_native { DAWN_TRY(ValidateBindingType(binding.type)); DAWN_TRY(ValidateTextureComponentType(binding.textureComponentType)); + if (binding.textureDimension != dawn::TextureViewDimension::Undefined) { + DAWN_TRY(ValidateTextureViewDimension(binding.textureDimension)); + } + if (binding.binding >= kMaxBindingsPerGroup) { return DAWN_VALIDATION_ERROR("some binding index exceeds the maximum value"); } @@ -100,7 +104,7 @@ namespace dawn_native { for (uint32_t binding : IterateBitSet(info.mask)) { HashCombine(&hash, info.visibilities[binding], info.types[binding], - info.textureComponentTypes[binding]); + info.textureComponentTypes[binding], info.textureDimensions[binding]); } return hash; @@ -115,7 +119,8 @@ namespace dawn_native { for (uint32_t binding : IterateBitSet(a.mask)) { if ((a.visibilities[binding] != b.visibilities[binding]) || (a.types[binding] != b.types[binding]) || - (a.textureComponentTypes[binding] != b.textureComponentTypes[binding])) { + (a.textureComponentTypes[binding] != b.textureComponentTypes[binding]) || + (a.textureDimensions[binding] != b.textureDimensions[binding])) { return false; } } @@ -138,6 +143,12 @@ namespace dawn_native { mBindingInfo.types[index] = binding.type; mBindingInfo.textureComponentTypes[index] = binding.textureComponentType; + if (binding.textureDimension == dawn::TextureViewDimension::Undefined) { + mBindingInfo.textureDimensions[index] = dawn::TextureViewDimension::e2D; + } else { + mBindingInfo.textureDimensions[index] = binding.textureDimension; + } + if (binding.dynamic) { mBindingInfo.dynamic.set(index); switch (binding.type) { diff --git a/src/dawn_native/BindGroupLayout.h b/src/dawn_native/BindGroupLayout.h index 1bb747184e..e06459b5b9 100644 --- a/src/dawn_native/BindGroupLayout.h +++ b/src/dawn_native/BindGroupLayout.h @@ -43,6 +43,7 @@ namespace dawn_native { std::array visibilities; std::array types; std::array textureComponentTypes; + std::array textureDimensions; std::bitset dynamic; std::bitset multisampled; std::bitset mask; diff --git a/src/dawn_native/Texture.cpp b/src/dawn_native/Texture.cpp index e5e2c49a8d..ea626edf82 100644 --- a/src/dawn_native/Texture.cpp +++ b/src/dawn_native/Texture.cpp @@ -497,6 +497,7 @@ namespace dawn_native { : ObjectBase(texture->GetDevice()), mTexture(texture), mFormat(GetDevice()->GetValidInternalFormat(descriptor->format)), + mDimension(descriptor->dimension), mBaseMipLevel(descriptor->baseMipLevel), mMipLevelCount(descriptor->mipLevelCount), mBaseArrayLayer(descriptor->baseArrayLayer), @@ -527,6 +528,11 @@ namespace dawn_native { return mFormat; } + dawn::TextureViewDimension TextureViewBase::GetDimension() const { + ASSERT(!IsError()); + return mDimension; + } + uint32_t TextureViewBase::GetBaseMipLevel() const { ASSERT(!IsError()); return mBaseMipLevel; diff --git a/src/dawn_native/Texture.h b/src/dawn_native/Texture.h index 6e26f997c8..c515066f3b 100644 --- a/src/dawn_native/Texture.h +++ b/src/dawn_native/Texture.h @@ -115,6 +115,7 @@ namespace dawn_native { TextureBase* GetTexture(); const Format& GetFormat() const; + dawn::TextureViewDimension GetDimension() const; uint32_t GetBaseMipLevel() const; uint32_t GetLevelCount() const; uint32_t GetBaseArrayLayer() const; @@ -127,6 +128,7 @@ namespace dawn_native { // TODO(cwallez@chromium.org): This should be deduplicated in the Device const Format& mFormat; + dawn::TextureViewDimension mDimension; uint32_t mBaseMipLevel; uint32_t mMipLevelCount; uint32_t mBaseArrayLayer; diff --git a/src/tests/end2end/ObjectCachingTests.cpp b/src/tests/end2end/ObjectCachingTests.cpp index bcf727c975..dcc86f850f 100644 --- a/src/tests/end2end/ObjectCachingTests.cpp +++ b/src/tests/end2end/ObjectCachingTests.cpp @@ -50,13 +50,30 @@ TEST_P(ObjectCachingTest, BindGroupLayoutDynamic) { TEST_P(ObjectCachingTest, BindGroupLayoutTextureComponentType) { dawn::BindGroupLayout bgl = utils::MakeBindGroupLayout( device, {{1, dawn::ShaderStage::Fragment, dawn::BindingType::SampledTexture, false, false, - dawn::TextureComponentType::Float}}); + dawn::TextureViewDimension::e2D, dawn::TextureComponentType::Float}}); dawn::BindGroupLayout sameBgl = utils::MakeBindGroupLayout( device, {{1, dawn::ShaderStage::Fragment, dawn::BindingType::SampledTexture, false, false, - dawn::TextureComponentType::Float}}); + dawn::TextureViewDimension::e2D, dawn::TextureComponentType::Float}}); dawn::BindGroupLayout otherBgl = utils::MakeBindGroupLayout( device, {{1, dawn::ShaderStage::Fragment, dawn::BindingType::SampledTexture, false, false, - dawn::TextureComponentType::Uint}}); + dawn::TextureViewDimension::e2D, dawn::TextureComponentType::Uint}}); + + EXPECT_NE(bgl.Get(), otherBgl.Get()); + EXPECT_EQ(bgl.Get() == sameBgl.Get(), !UsesWire()); +} + +// Test that two similar bind group layouts won't refer to the same one if they differ by +// textureDimension +TEST_P(ObjectCachingTest, BindGroupLayoutTextureDimension) { + dawn::BindGroupLayout bgl = utils::MakeBindGroupLayout( + device, {{1, dawn::ShaderStage::Fragment, dawn::BindingType::SampledTexture, false, false, + dawn::TextureViewDimension::e2D, dawn::TextureComponentType::Float}}); + dawn::BindGroupLayout sameBgl = utils::MakeBindGroupLayout( + device, {{1, dawn::ShaderStage::Fragment, dawn::BindingType::SampledTexture, false, false, + dawn::TextureViewDimension::e2D, dawn::TextureComponentType::Float}}); + dawn::BindGroupLayout otherBgl = utils::MakeBindGroupLayout( + device, {{1, dawn::ShaderStage::Fragment, dawn::BindingType::SampledTexture, false, false, + dawn::TextureViewDimension::e2DArray, dawn::TextureComponentType::Float}}); EXPECT_NE(bgl.Get(), otherBgl.Get()); EXPECT_EQ(bgl.Get() == sameBgl.Get(), !UsesWire()); diff --git a/src/tests/end2end/TextureFormatTests.cpp b/src/tests/end2end/TextureFormatTests.cpp index fe3e714ddd..9f1e01f74d 100644 --- a/src/tests/end2end/TextureFormatTests.cpp +++ b/src/tests/end2end/TextureFormatTests.cpp @@ -273,7 +273,7 @@ class TextureFormatTest : public DawnTest { dawn::BindGroupLayout bgl = utils::MakeBindGroupLayout( device, {{0, dawn::ShaderStage::Fragment, dawn::BindingType::Sampler}, {1, dawn::ShaderStage::Fragment, dawn::BindingType::SampledTexture, false, - false, sampleFormatInfo.type}}); + false, dawn::TextureViewDimension::e2D, sampleFormatInfo.type}}); // Prepare objects needed to sample from texture in the renderpass dawn::RenderPipeline pipeline = diff --git a/src/tests/end2end/TextureViewTests.cpp b/src/tests/end2end/TextureViewTests.cpp index b89ca68af0..76554ca4a0 100644 --- a/src/tests/end2end/TextureViewTests.cpp +++ b/src/tests/end2end/TextureViewTests.cpp @@ -83,12 +83,6 @@ protected: mRenderPass = utils::CreateBasicRenderPass(device, kRTSize, kRTSize); - mBindGroupLayout = utils::MakeBindGroupLayout( - device, { - {0, dawn::ShaderStage::Fragment, dawn::BindingType::Sampler}, - {1, dawn::ShaderStage::Fragment, dawn::BindingType::SampledTexture}, - }); - dawn::FilterMode kFilterMode = dawn::FilterMode::Nearest; dawn::AddressMode kAddressMode = dawn::AddressMode::ClampToEdge; @@ -104,8 +98,6 @@ protected: samplerDescriptor.compare = dawn::CompareFunction::Never; mSampler = device.CreateSampler(&samplerDescriptor); - mPipelineLayout = utils::MakeBasicPipelineLayout(device, &mBindGroupLayout); - mVSModule = CreateDefaultVertexShaderModule(device); } @@ -157,11 +149,19 @@ protected: queue.Submit(1, ©); } - void Verify(const dawn::TextureView &textureView, const char* fragmentShader, int expected) { - dawn::BindGroup bindGroup = utils::MakeBindGroup(device, mBindGroupLayout, { - {0, mSampler}, - {1, textureView} - }); + void Verify(const dawn::TextureView& textureView, + dawn::TextureViewDimension dimension, + const char* fragmentShader, + int expected) { + dawn::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout( + device, { + {0, dawn::ShaderStage::Fragment, dawn::BindingType::Sampler}, + {1, dawn::ShaderStage::Fragment, dawn::BindingType::SampledTexture, false, + false, dimension, dawn::TextureComponentType::Float}, + }); + + dawn::BindGroup bindGroup = + utils::MakeBindGroup(device, bindGroupLayout, {{0, mSampler}, {1, textureView}}); dawn::ShaderModule fsModule = utils::CreateShaderModule(device, utils::SingleShaderStage::Fragment, fragmentShader); @@ -169,7 +169,7 @@ protected: utils::ComboRenderPipelineDescriptor textureDescriptor(device); textureDescriptor.vertexStage.module = mVSModule; textureDescriptor.cFragmentStage.module = fsModule; - textureDescriptor.layout = mPipelineLayout; + textureDescriptor.layout = utils::MakeBasicPipelineLayout(device, &bindGroupLayout); textureDescriptor.cColorStates[0]->format = mRenderPass.colorFormat; dawn::RenderPipeline pipeline = device.CreateRenderPipeline(&textureDescriptor); @@ -224,7 +224,7 @@ protected: )"; const int expected = GenerateTestPixelValue(textureViewBaseLayer, textureViewBaseMipLevel); - Verify(textureView, fragmentShader, expected); + Verify(textureView, dawn::TextureViewDimension::e2D, fragmentShader, expected); } void Texture2DArrayViewTest(uint32_t textureArrayLayers, @@ -268,7 +268,7 @@ protected: for (int i = 0; i < static_cast(kTextureViewLayerCount); ++i) { expected += GenerateTestPixelValue(textureViewBaseLayer + i, textureViewBaseMipLevel); } - Verify(textureView, fragmentShader, expected); + Verify(textureView, dawn::TextureViewDimension::e2DArray, fragmentShader, expected); } std::string CreateFragmentShaderForCubeMapFace(uint32_t layer, bool isCubeMapArray) { @@ -320,10 +320,12 @@ protected: ASSERT_TRUE((textureViewLayerCount == 6) || (isCubeMapArray && textureViewLayerCount % 6 == 0)); + dawn::TextureViewDimension dimension = (isCubeMapArray) + ? dawn::TextureViewDimension::CubeArray + : dawn::TextureViewDimension::Cube; dawn::TextureViewDescriptor descriptor = mDefaultTextureViewDescriptor; - descriptor.dimension = (isCubeMapArray) ? - dawn::TextureViewDimension::CubeArray : dawn::TextureViewDimension::Cube; + descriptor.dimension = dimension; descriptor.baseArrayLayer = textureViewBaseLayer; descriptor.arrayLayerCount = textureViewLayerCount; @@ -335,12 +337,10 @@ protected: CreateFragmentShaderForCubeMapFace(layer, isCubeMapArray); int expected = GenerateTestPixelValue(textureViewBaseLayer + layer, 0); - Verify(cubeMapTextureView, fragmentShader.c_str(), expected); + Verify(cubeMapTextureView, dimension, fragmentShader.c_str(), expected); } } - dawn::BindGroupLayout mBindGroupLayout; - dawn::PipelineLayout mPipelineLayout; dawn::Sampler mSampler; dawn::Texture mTexture; dawn::TextureViewDescriptor mDefaultTextureViewDescriptor; @@ -376,7 +376,7 @@ TEST_P(TextureViewSamplingTest, Default2DArrayTexture) { const int expected = GenerateTestPixelValue(0, 0) + GenerateTestPixelValue(1, 0) + GenerateTestPixelValue(2, 0); - Verify(textureView, fragmentShader, expected); + Verify(textureView, dawn::TextureViewDimension::e2DArray, fragmentShader, expected); } // Test sampling from a 2D texture view created on a 2D array texture. diff --git a/src/tests/unittests/validation/BindGroupValidationTests.cpp b/src/tests/unittests/validation/BindGroupValidationTests.cpp index ba3bd1f0f1..6d464bf2d1 100644 --- a/src/tests/unittests/validation/BindGroupValidationTests.cpp +++ b/src/tests/unittests/validation/BindGroupValidationTests.cpp @@ -303,12 +303,12 @@ TEST_F(BindGroupValidationTest, TextureUsage) { TEST_F(BindGroupValidationTest, TextureComponentType) { dawn::BindGroupLayout layout = utils::MakeBindGroupLayout( device, {{0, dawn::ShaderStage::Fragment, dawn::BindingType::SampledTexture, false, false, - dawn::TextureComponentType::Float}}); + dawn::TextureViewDimension::e2D, dawn::TextureComponentType::Float}}); // Control case: setting a Float typed texture view works. utils::MakeBindGroup(device, layout, {{0, mSampledTextureView}}); - // Make an output attachment texture and try to set it for a SampledTexture binding + // Make a Uint component typed texture and try to set it to a Float component binding. dawn::TextureDescriptor descriptor; descriptor.dimension = dawn::TextureDimension::e2D; descriptor.size = {16, 16, 1}; @@ -323,6 +323,30 @@ TEST_F(BindGroupValidationTest, TextureComponentType) { ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, layout, {{0, uintTextureView}})); } +// Check that a texture must have the correct dimension +TEST_F(BindGroupValidationTest, TextureDimension) { + dawn::BindGroupLayout layout = utils::MakeBindGroupLayout( + device, {{0, dawn::ShaderStage::Fragment, dawn::BindingType::SampledTexture, false, false, + dawn::TextureViewDimension::e2D, dawn::TextureComponentType::Float}}); + + // Control case: setting a 2D texture view works. + utils::MakeBindGroup(device, layout, {{0, mSampledTextureView}}); + + // Make a 2DArray texture and try to set it to a 2D binding. + dawn::TextureDescriptor descriptor; + descriptor.dimension = dawn::TextureDimension::e2D; + descriptor.size = {16, 16, 1}; + descriptor.arrayLayerCount = 2; + descriptor.sampleCount = 1; + descriptor.format = dawn::TextureFormat::RGBA8Uint; + descriptor.mipLevelCount = 1; + descriptor.usage = dawn::TextureUsage::Sampled; + dawn::Texture arrayTexture = device.CreateTexture(&descriptor); + dawn::TextureView arrayTextureView = arrayTexture.CreateView(); + + ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, layout, {{0, arrayTextureView}})); +} + // Check that a UBO must have the correct usage TEST_F(BindGroupValidationTest, BufferUsageUBO) { dawn::BindGroupLayout layout = utils::MakeBindGroupLayout( diff --git a/src/tests/unittests/wire/WireArgumentTests.cpp b/src/tests/unittests/wire/WireArgumentTests.cpp index ebd70038c3..88c7cc0c06 100644 --- a/src/tests/unittests/wire/WireArgumentTests.cpp +++ b/src/tests/unittests/wire/WireArgumentTests.cpp @@ -331,11 +331,12 @@ TEST_F(WireArgumentTests, StructureOfStructureArrayArgument) { static constexpr int NUM_BINDINGS = 3; DawnBindGroupLayoutBinding bindings[NUM_BINDINGS]{ {0, DAWN_SHADER_STAGE_VERTEX, DAWN_BINDING_TYPE_SAMPLER, false, false, - DAWN_TEXTURE_COMPONENT_TYPE_FLOAT}, + DAWN_TEXTURE_VIEW_DIMENSION_2D, DAWN_TEXTURE_COMPONENT_TYPE_FLOAT}, {1, DAWN_SHADER_STAGE_VERTEX, DAWN_BINDING_TYPE_SAMPLED_TEXTURE, false, false, - DAWN_TEXTURE_COMPONENT_TYPE_FLOAT}, + DAWN_TEXTURE_VIEW_DIMENSION_2D, DAWN_TEXTURE_COMPONENT_TYPE_FLOAT}, {2, static_cast(DAWN_SHADER_STAGE_VERTEX | DAWN_SHADER_STAGE_FRAGMENT), - DAWN_BINDING_TYPE_UNIFORM_BUFFER, false, false, DAWN_TEXTURE_COMPONENT_TYPE_FLOAT}, + DAWN_BINDING_TYPE_UNIFORM_BUFFER, false, false, DAWN_TEXTURE_VIEW_DIMENSION_2D, + DAWN_TEXTURE_COMPONENT_TYPE_FLOAT}, }; DawnBindGroupLayoutDescriptor bglDescriptor; bglDescriptor.bindingCount = NUM_BINDINGS;