From c38918275867121865484c048a61a04ea8251698 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Fri, 17 Sep 2021 17:07:43 +0000 Subject: [PATCH] Remove readonly storage textures Bug: dawn:1025 Change-Id: I1759639142589470e278b4906d9cad5cb485f9a5 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/63743 Commit-Queue: Corentin Wallez Reviewed-by: Brandon Jones --- dawn.json | 3 +- src/dawn_native/BindGroupLayout.cpp | 6 - src/dawn_native/PassResourceUsageTracker.cpp | 3 - src/dawn_native/ShaderModule.cpp | 2 - src/dawn_native/Texture.cpp | 6 - src/dawn_native/Texture.h | 2 +- src/dawn_native/d3d12/BindGroupD3D12.cpp | 11 - .../d3d12/BindGroupLayoutD3D12.cpp | 2 - src/dawn_native/d3d12/TextureD3D12.cpp | 2 +- src/dawn_native/dawn_platform.h | 5 +- src/dawn_native/opengl/CommandBufferGL.cpp | 3 - src/dawn_native/opengl/ShaderModuleGL.cpp | 2 - src/dawn_native/vulkan/TextureVk.cpp | 10 +- src/tests/end2end/BindGroupTests.cpp | 21 +- src/tests/end2end/DeprecatedAPITests.cpp | 13 - .../end2end/GpuMemorySynchronizationTests.cpp | 71 ---- src/tests/end2end/StorageTextureTests.cpp | 381 ------------------ .../validation/BindGroupValidationTests.cpp | 72 ---- .../validation/ResourceUsageTrackingTests.cpp | 198 +++------ .../StorageTextureValidationTests.cpp | 228 ++--------- .../validation/TextureSubresourceTests.cpp | 22 +- .../white_box/InternalResourceUsageTests.cpp | 12 - 22 files changed, 100 insertions(+), 975 deletions(-) diff --git a/dawn.json b/dawn.json index 91107cacbb..be95924591 100644 --- a/dawn.json +++ b/dawn.json @@ -165,8 +165,7 @@ "category": "enum", "values": [ {"value": 0, "name": "undefined", "jsrepr": "undefined", "valid": false}, - {"value": 1, "name": "read only", "jsrepr": "readonly"}, - {"value": 2, "name": "write only", "jsrepr": "writeonly"} + {"value": 1, "name": "write only", "jsrepr": "writeonly"} ] }, "storage texture binding layout": { diff --git a/src/dawn_native/BindGroupLayout.cpp b/src/dawn_native/BindGroupLayout.cpp index 6aafe84062..84f40504ce 100644 --- a/src/dawn_native/BindGroupLayout.cpp +++ b/src/dawn_native/BindGroupLayout.cpp @@ -149,12 +149,6 @@ namespace dawn_native { if (storageTexture.access == wgpu::StorageTextureAccess::WriteOnly) { allowedStages &= ~wgpu::ShaderStage::Vertex; } - - // TODO(crbug.com/dawn/1025): Remove after the deprecation period. - if (storageTexture.access == wgpu::StorageTextureAccess::ReadOnly) { - device->EmitDeprecationWarning( - "Readonly storage textures are deprecated and will be removed."); - } } const ExternalTextureBindingLayout* externalTextureBindingLayout = nullptr; diff --git a/src/dawn_native/PassResourceUsageTracker.cpp b/src/dawn_native/PassResourceUsageTracker.cpp index 2ebdc0238c..470eee17fc 100644 --- a/src/dawn_native/PassResourceUsageTracker.cpp +++ b/src/dawn_native/PassResourceUsageTracker.cpp @@ -115,9 +115,6 @@ namespace dawn_native { case BindingInfoType::StorageTexture: { TextureViewBase* view = group->GetBindingAsTextureView(bindingIndex); switch (bindingInfo.storageTexture.access) { - case wgpu::StorageTextureAccess::ReadOnly: - TextureViewUsedAs(view, kReadOnlyStorageTexture); - break; case wgpu::StorageTextureAccess::WriteOnly: TextureViewUsedAs(view, wgpu::TextureUsage::StorageBinding); break; diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp index f86e3aad99..d4f11524ad 100644 --- a/src/dawn_native/ShaderModule.cpp +++ b/src/dawn_native/ShaderModule.cpp @@ -319,8 +319,6 @@ namespace dawn_native { ResultOrError TintResourceTypeToStorageTextureAccess( tint::inspector::ResourceBinding::ResourceType resource_type) { switch (resource_type) { - case tint::inspector::ResourceBinding::ResourceType::kReadOnlyStorageTexture: - return wgpu::StorageTextureAccess::ReadOnly; case tint::inspector::ResourceBinding::ResourceType::kWriteOnlyStorageTexture: return wgpu::StorageTextureAccess::WriteOnly; default: diff --git a/src/dawn_native/Texture.cpp b/src/dawn_native/Texture.cpp index cb1f741d2e..0194772663 100644 --- a/src/dawn_native/Texture.cpp +++ b/src/dawn_native/Texture.cpp @@ -460,12 +460,6 @@ namespace dawn_native { if (internalUsageDesc != nullptr) { mInternalUsage |= internalUsageDesc->internalUsage; } - - // Add readonly storage usage if the texture has a storage usage. The validation rules in - // ValidateSyncScopeResourceUsage will make sure we don't use both at the same time. - if (mInternalUsage & wgpu::TextureUsage::StorageBinding) { - mInternalUsage |= kReadOnlyStorageTexture; - } } static Format kUnusedFormat; diff --git a/src/dawn_native/Texture.h b/src/dawn_native/Texture.h index c3e161d68c..88a3338787 100644 --- a/src/dawn_native/Texture.h +++ b/src/dawn_native/Texture.h @@ -40,7 +40,7 @@ namespace dawn_native { bool IsValidSampleCount(uint32_t sampleCount); static constexpr wgpu::TextureUsage kReadOnlyTextureUsages = - wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::TextureBinding | kReadOnlyStorageTexture; + wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::TextureBinding; class TextureBase : public ObjectBase { public: diff --git a/src/dawn_native/d3d12/BindGroupD3D12.cpp b/src/dawn_native/d3d12/BindGroupD3D12.cpp index 47c501aa6e..f002ece353 100644 --- a/src/dawn_native/d3d12/BindGroupD3D12.cpp +++ b/src/dawn_native/d3d12/BindGroupD3D12.cpp @@ -160,17 +160,6 @@ namespace dawn_native { namespace d3d12 { } switch (bindingInfo.storageTexture.access) { - case wgpu::StorageTextureAccess::ReadOnly: { - // Readonly storage is implemented as SRV so it can be used at the same - // time as a sampled texture. - auto& srv = view->GetSRVDescriptor(); - d3d12Device->CreateShaderResourceView( - resource, &srv, - viewAllocation.OffsetFrom(viewSizeIncrement, - descriptorHeapOffsets[bindingIndex])); - break; - } - case wgpu::StorageTextureAccess::WriteOnly: { D3D12_UNORDERED_ACCESS_VIEW_DESC uav = view->GetUAVDescriptor(); d3d12Device->CreateUnorderedAccessView( diff --git a/src/dawn_native/d3d12/BindGroupLayoutD3D12.cpp b/src/dawn_native/d3d12/BindGroupLayoutD3D12.cpp index dd191e1cb3..761b8f74a7 100644 --- a/src/dawn_native/d3d12/BindGroupLayoutD3D12.cpp +++ b/src/dawn_native/d3d12/BindGroupLayoutD3D12.cpp @@ -47,8 +47,6 @@ namespace dawn_native { namespace d3d12 { case BindingInfoType::StorageTexture: switch (bindingInfo.storageTexture.access) { - case wgpu::StorageTextureAccess::ReadOnly: - return D3D12_DESCRIPTOR_RANGE_TYPE_SRV; case wgpu::StorageTextureAccess::WriteOnly: return D3D12_DESCRIPTOR_RANGE_TYPE_UAV; case wgpu::StorageTextureAccess::Undefined: diff --git a/src/dawn_native/d3d12/TextureD3D12.cpp b/src/dawn_native/d3d12/TextureD3D12.cpp index e7a97b3127..d0fdec82a1 100644 --- a/src/dawn_native/d3d12/TextureD3D12.cpp +++ b/src/dawn_native/d3d12/TextureD3D12.cpp @@ -50,7 +50,7 @@ namespace dawn_native { namespace d3d12 { if (usage & wgpu::TextureUsage::CopyDst) { resourceState |= D3D12_RESOURCE_STATE_COPY_DEST; } - if (usage & (wgpu::TextureUsage::TextureBinding | kReadOnlyStorageTexture)) { + if (usage & (wgpu::TextureUsage::TextureBinding)) { resourceState |= (D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE | D3D12_RESOURCE_STATE_NON_PIXEL_SHADER_RESOURCE); } diff --git a/src/dawn_native/dawn_platform.h b/src/dawn_native/dawn_platform.h index 537a5029e7..4b9e195a9f 100644 --- a/src/dawn_native/dawn_platform.h +++ b/src/dawn_native/dawn_platform.h @@ -23,12 +23,9 @@ #include namespace dawn_native { - // Add an extra buffer usage (readonly storage buffer usage) and an extra texture usage - // (readonly storage texture usage) for render pass resource tracking + // Add an extra buffer usage (readonly storage buffer usage) for render pass resource tracking static constexpr wgpu::BufferUsage kReadOnlyStorageBuffer = static_cast(0x80000000); - static constexpr wgpu::TextureUsage kReadOnlyStorageTexture = - static_cast(0x80000000); // Internal usage to help tracking when a subresource is used as render attachment usage // more than once in a render pass. diff --git a/src/dawn_native/opengl/CommandBufferGL.cpp b/src/dawn_native/opengl/CommandBufferGL.cpp index fdb1eba51a..2253247327 100644 --- a/src/dawn_native/opengl/CommandBufferGL.cpp +++ b/src/dawn_native/opengl/CommandBufferGL.cpp @@ -339,9 +339,6 @@ namespace dawn_native { namespace opengl { GLenum access; switch (bindingInfo.storageTexture.access) { - case wgpu::StorageTextureAccess::ReadOnly: - access = GL_READ_ONLY; - break; case wgpu::StorageTextureAccess::WriteOnly: access = GL_WRITE_ONLY; break; diff --git a/src/dawn_native/opengl/ShaderModuleGL.cpp b/src/dawn_native/opengl/ShaderModuleGL.cpp index 9412a2e9e9..5019593ba6 100644 --- a/src/dawn_native/opengl/ShaderModuleGL.cpp +++ b/src/dawn_native/opengl/ShaderModuleGL.cpp @@ -164,8 +164,6 @@ namespace dawn_native { namespace opengl { spirv_cross::Bitset flags = compiler.get_decoration_bitset(resource.id); if (flags.get(spv::DecorationNonReadable)) { info->storageTexture.access = wgpu::StorageTextureAccess::WriteOnly; - } else if (flags.get(spv::DecorationNonWritable)) { - info->storageTexture.access = wgpu::StorageTextureAccess::ReadOnly; } else { return DAWN_VALIDATION_ERROR( "Read-write storage textures are not supported"); diff --git a/src/dawn_native/vulkan/TextureVk.cpp b/src/dawn_native/vulkan/TextureVk.cpp index c39854c1eb..3aae083163 100644 --- a/src/dawn_native/vulkan/TextureVk.cpp +++ b/src/dawn_native/vulkan/TextureVk.cpp @@ -73,9 +73,6 @@ namespace dawn_native { namespace vulkan { if (usage & wgpu::TextureUsage::StorageBinding) { flags |= VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT; } - if (usage & kReadOnlyStorageTexture) { - flags |= VK_ACCESS_SHADER_READ_BIT; - } if (usage & wgpu::TextureUsage::RenderAttachment) { if (format.HasDepthOrStencil()) { flags |= VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT | @@ -119,7 +116,7 @@ namespace dawn_native { namespace vulkan { if (usage & (wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::CopyDst)) { flags |= VK_PIPELINE_STAGE_TRANSFER_BIT; } - if (usage & (wgpu::TextureUsage::TextureBinding | kReadOnlyStorageTexture)) { + if (usage & wgpu::TextureUsage::TextureBinding) { // TODO(crbug.com/dawn/851): Only transition to the usage we care about to avoid // introducing FS -> VS dependencies that would prevent parallelization on tiler // GPUs @@ -437,7 +434,7 @@ namespace dawn_native { namespace vulkan { if (usage & wgpu::TextureUsage::TextureBinding) { flags |= VK_IMAGE_USAGE_SAMPLED_BIT; } - if (usage & (wgpu::TextureUsage::StorageBinding | kReadOnlyStorageTexture)) { + if (usage & wgpu::TextureUsage::StorageBinding) { flags |= VK_IMAGE_USAGE_STORAGE_BIT; } if (usage & wgpu::TextureUsage::RenderAttachment) { @@ -462,7 +459,7 @@ namespace dawn_native { namespace vulkan { if (!wgpu::HasZeroOrOneBits(usage)) { // Sampled | ReadOnlyStorage is the only possible multi-bit usage, if more appear we // might need additional special-casing. - ASSERT(usage == (wgpu::TextureUsage::TextureBinding | kReadOnlyStorageTexture)); + ASSERT(usage == wgpu::TextureUsage::TextureBinding); return VK_IMAGE_LAYOUT_GENERAL; } @@ -495,7 +492,6 @@ namespace dawn_native { namespace vulkan { // and store operations on storage images can only be done on the images in // VK_IMAGE_LAYOUT_GENERAL layout. case wgpu::TextureUsage::StorageBinding: - case kReadOnlyStorageTexture: return VK_IMAGE_LAYOUT_GENERAL; case wgpu::TextureUsage::RenderAttachment: diff --git a/src/tests/end2end/BindGroupTests.cpp b/src/tests/end2end/BindGroupTests.cpp index be912e9059..5098285797 100644 --- a/src/tests/end2end/BindGroupTests.cpp +++ b/src/tests/end2end/BindGroupTests.cpp @@ -1487,12 +1487,9 @@ TEST_P(BindGroupTests, ReallyLargeBindGroup) { bgEntries.push_back({nullptr, binding, nullptr, 0, 0, nullptr, texture.CreateView()}); interface << "[[group(0), binding(" << binding++ << ")]] " - << "var image" << i << " : texture_storage_2d;\n"; + << "var image" << i << " : texture_storage_2d;\n"; - body << "if (textureLoad(image" << i << ", vec2(0, 0)).r != " << expectedValue++ - << "u) {\n"; - body << " return;\n"; - body << "}\n"; + body << "ignore(image" << i << ");"; } for (uint32_t i = 0; i < kMaxUniformBuffersPerShaderStage; ++i) { @@ -1547,9 +1544,7 @@ TEST_P(BindGroupTests, ReallyLargeBindGroup) { wgpu::ComputePipelineDescriptor cpDesc; cpDesc.compute.module = utils::CreateShaderModule(device, shader.c_str()); cpDesc.compute.entryPoint = "main"; - wgpu::ComputePipeline cp; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is passed. - EXPECT_DEPRECATION_WARNINGS(cp = device.CreateComputePipeline(&cpDesc), 4); + wgpu::ComputePipeline cp = device.CreateComputePipeline(&cpDesc); wgpu::BindGroupDescriptor bgDesc = {}; bgDesc.layout = cp.GetBindGroupLayout(0); @@ -1621,13 +1616,9 @@ TEST_P(BindGroupTests, CreateWithDestroyedResource) { // Test a storage texture. { - wgpu::BindGroupLayout bgl; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - EXPECT_DEPRECATION_WARNING( - bgl = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::StorageTextureAccess::ReadOnly, - wgpu::TextureFormat::R32Uint}})); + wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Fragment, wgpu::StorageTextureAccess::WriteOnly, + wgpu::TextureFormat::R32Uint}}); wgpu::TextureDescriptor textureDesc; textureDesc.usage = wgpu::TextureUsage::StorageBinding; diff --git a/src/tests/end2end/DeprecatedAPITests.cpp b/src/tests/end2end/DeprecatedAPITests.cpp index 3248c99ab5..59bab602c3 100644 --- a/src/tests/end2end/DeprecatedAPITests.cpp +++ b/src/tests/end2end/DeprecatedAPITests.cpp @@ -34,19 +34,6 @@ class DeprecationTests : public DawnTest { } }; -// Test that readonly storage textures are deprecated -TEST_P(DeprecationTests, ReadOnlyStorageTextures) { - // Control case: WriteOnly storage textures are allowed. - utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::StorageTextureAccess::WriteOnly, - wgpu::TextureFormat::R32Float}}); - - // Error case: ReadOnly storage textures are not allowed. - EXPECT_DEPRECATION_WARNING(utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::StorageTextureAccess::ReadOnly, - wgpu::TextureFormat::R32Float}})); -} - DAWN_INSTANTIATE_TEST(DeprecationTests, D3D12Backend(), MetalBackend(), diff --git a/src/tests/end2end/GpuMemorySynchronizationTests.cpp b/src/tests/end2end/GpuMemorySynchronizationTests.cpp index 5a57cc1d7a..a6fe1418d4 100644 --- a/src/tests/end2end/GpuMemorySynchronizationTests.cpp +++ b/src/tests/end2end/GpuMemorySynchronizationTests.cpp @@ -225,77 +225,6 @@ TEST_P(GpuMemorySyncTests, ComputePassToRenderPass) { EXPECT_PIXEL_RGBA8_EQ(RGBA8(2, 0, 0, 255), renderPass.color, 0, 0); } -// Use an image as both sampled and readonly storage in a compute pass. This is a regression test -// for the Vulkan backend choosing different layouts for Sampled and ReadOnlyStorage. -TEST_P(GpuMemorySyncTests, SampledAndROStorageTextureInComputePass) { - // TODO(crbug.com/dawn/646): diagnose and fix this OpenGL ES backend validation failure. - // "GL_INVALID_OPERATION error generated. Image variable update is not allowed." - DAWN_SUPPRESS_TEST_IF(IsOpenGLES() && IsBackendValidationEnabled()); - - // Create a storage + sampled texture of one texel initialized to 1 - wgpu::TextureDescriptor texDesc; - texDesc.format = wgpu::TextureFormat::R32Uint; - texDesc.size = {1, 1, 1}; - texDesc.usage = wgpu::TextureUsage::StorageBinding | wgpu::TextureUsage::TextureBinding | - wgpu::TextureUsage::CopyDst; - wgpu::Texture tex = device.CreateTexture(&texDesc); - - wgpu::ImageCopyTexture copyDst; - copyDst.texture = tex; - wgpu::TextureDataLayout layout; - wgpu::Extent3D copySize = {1, 1, 1}; - uint32_t kOne = 1; - queue.WriteTexture(©Dst, &kOne, sizeof(kOne), &layout, ©Size); - - // Create a pipeline that loads the texture from both the sampled and storage paths. - wgpu::ComputePipelineDescriptor pipelineDesc; - pipelineDesc.compute.entryPoint = "main"; - pipelineDesc.compute.module = utils::CreateShaderModule(device, R"( - [[block]] struct Output { - sampledOut: u32; - storageOut: u32; - }; - [[group(0), binding(0)]] var output : Output; - [[group(0), binding(1)]] var sampledTex : texture_2d; - [[group(0), binding(2)]] var storageTex : texture_storage_2d; - - [[stage(compute), workgroup_size(1)]] fn main() { - output.sampledOut = textureLoad(sampledTex, vec2(0, 0), 0).x; - output.storageOut = textureLoad(storageTex, vec2(0, 0)).x; - } - )"); - wgpu::ComputePipeline pipeline; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is passed. - EXPECT_DEPRECATION_WARNING(pipeline = device.CreateComputePipeline(&pipelineDesc)); - - // Run the compute pipeline and store the result in the buffer. - wgpu::BufferDescriptor outputDesc; - outputDesc.size = 8; - outputDesc.usage = wgpu::BufferUsage::Storage | wgpu::BufferUsage::CopySrc; - wgpu::Buffer outputBuffer = device.CreateBuffer(&outputDesc); - - wgpu::BindGroup bg = utils::MakeBindGroup(device, pipeline.GetBindGroupLayout(0), - { - {0, outputBuffer}, - {1, tex.CreateView()}, - {2, tex.CreateView()}, - }); - - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - wgpu::ComputePassEncoder pass = encoder.BeginComputePass(); - pass.SetBindGroup(0, bg); - pass.SetPipeline(pipeline); - pass.Dispatch(1); - pass.EndPass(); - - wgpu::CommandBuffer commands = encoder.Finish(); - queue.Submit(1, &commands); - - // Check the buffer's content is what we expect. - EXPECT_BUFFER_U32_EQ(1, outputBuffer, 0); - EXPECT_BUFFER_U32_EQ(1, outputBuffer, 4); -} - DAWN_INSTANTIATE_TEST(GpuMemorySyncTests, D3D12Backend(), MetalBackend(), diff --git a/src/tests/end2end/StorageTextureTests.cpp b/src/tests/end2end/StorageTextureTests.cpp index 68e9138d5b..74579a258b 100644 --- a/src/tests/end2end/StorageTextureTests.cpp +++ b/src/tests/end2end/StorageTextureTests.cpp @@ -746,131 +746,6 @@ fn IsEqualTo(pixel : vec4, expected : vec4) -> bool { const char* kComputeExpectedValue = "1 + x + size.x * (y + size.y * slice)"; }; -// Test that read-only storage textures are supported in compute shader. -TEST_P(StorageTextureTests, ReadonlyStorageTextureInComputeShader) { - for (wgpu::TextureFormat format : utils::kAllTextureFormats) { - if (!utils::TextureFormatSupportsStorageTexture(format)) { - continue; - } - if (IsOpenGLES() && !OpenGLESSupportsStorageTexture(format)) { - continue; - } - - // Prepare the read-only storage texture and fill it with the expected data. - const std::vector kInitialTextureData = GetExpectedData(format); - wgpu::Texture readonlyStorageTexture = - CreateTextureWithTestData(kInitialTextureData, format); - - // Create a compute shader that reads the pixels from the read-only storage texture and - // writes 1 to DstBuffer if they all have the expected value. - std::ostringstream csStream; - csStream << R"( -[[block]] struct DstBuffer { - result : u32; -}; - -[[group(0), binding(1)]] var dstBuffer : DstBuffer; -)" << CommonReadOnlyTestCode(format) - << R"( -[[stage(compute), workgroup_size(1)]] fn main() { - if (doTest()) { - dstBuffer.result = 1u; - } else { - dstBuffer.result = 0u; - } -})"; - - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - EXPECT_DEPRECATION_WARNING( - CheckResultInStorageBuffer(readonlyStorageTexture, csStream.str())); - } -} - -// Test that read-only storage textures are supported in vertex shader. -TEST_P(StorageTextureTests, ReadonlyStorageTextureInVertexShader) { - for (wgpu::TextureFormat format : utils::kAllTextureFormats) { - if (!utils::TextureFormatSupportsStorageTexture(format)) { - continue; - } - if (IsOpenGLES() && !OpenGLESSupportsStorageTexture(format)) { - continue; - } - - // Prepare the read-only storage texture and fill it with the expected data. - const std::vector kInitialTextureData = GetExpectedData(format); - wgpu::Texture readonlyStorageTexture = - CreateTextureWithTestData(kInitialTextureData, format); - - // Create a rendering pipeline that reads the pixels from the read-only storage texture and - // uses green as the output color, otherwise uses red instead. - std::ostringstream vsStream; - vsStream << R"( -struct VertexOut { - [[location(0)]] color : vec4; - [[builtin(position)]] position : vec4; -}; -)" << CommonReadOnlyTestCode(format) - << R"( -[[stage(vertex)]] fn main() -> VertexOut { - var output : VertexOut; - output.position = vec4(0.0, 0.0, 0.0, 1.0); - if (doTest()) { - output.color = vec4(0.0, 1.0, 0.0, 1.0); - } else { - output.color = vec4(1.0, 0.0, 0.0, 1.0); - } - return output; -})"; - const char* kFragmentShader = R"( -[[stage(fragment)]] -fn main([[location(0)]] color : vec4) -> [[location(0)]] vec4 { - return color; -})"; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - EXPECT_DEPRECATION_WARNING( - CheckDrawsGreen(vsStream.str().c_str(), kFragmentShader, readonlyStorageTexture)); - } -} - -// Test that read-only storage textures are supported in fragment shader. -TEST_P(StorageTextureTests, ReadonlyStorageTextureInFragmentShader) { - // TODO(crbug.com/dawn/672): Investigate why this test fails on Linux - // NVidia OpenGLES drivers. - DAWN_SUPPRESS_TEST_IF(IsNvidia() && IsLinux() && IsOpenGLES()); - - for (wgpu::TextureFormat format : utils::kAllTextureFormats) { - if (!utils::TextureFormatSupportsStorageTexture(format)) { - continue; - } - if (IsOpenGLES() && !OpenGLESSupportsStorageTexture(format)) { - continue; - } - - // Prepare the read-only storage texture and fill it with the expected data. - const std::vector kInitialTextureData = GetExpectedData(format); - wgpu::Texture readonlyStorageTexture = - CreateTextureWithTestData(kInitialTextureData, format); - - // Create a rendering pipeline that reads the pixels from the read-only storage texture and - // uses green as the output color if the pixel value is expected, otherwise uses red - // instead. - std::ostringstream fsStream; - fsStream << CommonReadOnlyTestCode(format) << R"( -[[stage(fragment)]] fn main() -> [[location(0)]] vec4 { - if (doTest()) { - return vec4(0.0, 1.0, 0.0, 1.0); - } - return vec4(1.0, 0.0, 0.0, 1.0); -})"; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - EXPECT_DEPRECATION_WARNING( - CheckDrawsGreen(kSimpleVertexShader, fsStream.str().c_str(), readonlyStorageTexture)); - } -} - // Test that write-only storage textures are supported in compute shader. TEST_P(StorageTextureTests, WriteonlyStorageTextureInComputeShader) { for (wgpu::TextureFormat format : utils::kAllTextureFormats) { @@ -905,47 +780,6 @@ TEST_P(StorageTextureTests, WriteonlyStorageTextureInComputeShader) { } } -// Test that reading from one read-only storage texture then writing into another write-only storage -// texture in one dispatch are supported in compute shader. -TEST_P(StorageTextureTests, ReadWriteDifferentStorageTextureInOneDispatchInComputeShader) { - // TODO(crbug.com/dawn/636): diagnose and fix this failure on OpenGL ES - DAWN_SUPPRESS_TEST_IF(IsOpenGLES()); - - for (wgpu::TextureFormat format : utils::kAllTextureFormats) { - if (!utils::TextureFormatSupportsStorageTexture(format)) { - continue; - } - if (IsOpenGLES() && !OpenGLESSupportsStorageTexture(format)) { - continue; - } - - // TODO(jiawei.shao@intel.com): investigate why this test fails with RGBA8Snorm on Linux - // Intel OpenGL driver. - if (format == wgpu::TextureFormat::RGBA8Snorm && IsIntel() && IsOpenGL() && IsLinux()) { - continue; - } - - // Prepare the read-only storage texture. - const std::vector kInitialTextureData = GetExpectedData(format); - wgpu::Texture readonlyStorageTexture = - CreateTextureWithTestData(kInitialTextureData, format); - - // Prepare the write-only storage texture. - wgpu::Texture writeonlyStorageTexture = - CreateTexture(format, wgpu::TextureUsage::StorageBinding | wgpu::TextureUsage::CopySrc); - - // Write the expected pixel values into the write-only storage texture. - const std::string computeShader = CommonReadWriteTestCode(format); - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - EXPECT_DEPRECATION_WARNING(ReadWriteIntoStorageTextureInComputePass( - readonlyStorageTexture, writeonlyStorageTexture, computeShader.c_str())); - - // Verify the pixel data in the write-only storage texture is expected. - CheckOutputStorageTexture(writeonlyStorageTexture, format); - } -} - // Test that write-only storage textures are supported in fragment shader. TEST_P(StorageTextureTests, WriteonlyStorageTextureInFragmentShader) { // TODO(crbug.com/dawn/672): Investigate why this test fails on Linux @@ -985,52 +819,6 @@ TEST_P(StorageTextureTests, WriteonlyStorageTextureInFragmentShader) { } } -// Verify 2D array and 3D read-only storage textures work correctly. -TEST_P(StorageTextureTests, Readonly2DArrayOr3DStorageTexture) { - // TODO(crbug.com/dawn/547): implement 3D storage texture on OpenGL and OpenGLES. - DAWN_TEST_UNSUPPORTED_IF(IsOpenGL() || IsOpenGLES()); - - constexpr uint32_t kSliceCount = 3u; - - constexpr wgpu::TextureFormat kTextureFormat = wgpu::TextureFormat::R32Uint; - - const std::vector initialTextureData = GetExpectedData(kTextureFormat, kSliceCount); - - wgpu::TextureViewDimension dimensions[] = { - wgpu::TextureViewDimension::e2DArray, - wgpu::TextureViewDimension::e3D, - }; - - for (wgpu::TextureViewDimension dimension : dimensions) { - wgpu::Texture readonlyStorageTexture = - CreateTextureWithTestData(initialTextureData, kTextureFormat, dimension); - - // Create a compute shader that reads the pixels from the read-only storage texture and - // writes 1 to DstBuffer if they all have the expected value. - std::ostringstream csStream; - csStream << R"( -[[block]] struct DstBuffer { - result : u32; -}; - -[[group(0), binding(1)]] var dstBuffer : DstBuffer; -)" << CommonReadOnlyTestCode(kTextureFormat, dimension) - << R"( -[[stage(compute), workgroup_size(1)]] fn main() { - if (doTest()) { - dstBuffer.result = 1u; - } else { - dstBuffer.result = 0u; - } -})"; - - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - EXPECT_DEPRECATION_WARNING( - CheckResultInStorageBuffer(readonlyStorageTexture, csStream.str(), dimension)); - } -} - // Verify 2D array and 3D write-only storage textures work correctly. TEST_P(StorageTextureTests, Writeonly2DArrayOr3DStorageTexture) { // TODO(crbug.com/dawn/547): implement 3D storage texture on OpenGL and OpenGLES. @@ -1062,123 +850,6 @@ TEST_P(StorageTextureTests, Writeonly2DArrayOr3DStorageTexture) { } } -// Verify 2D array and 3D read-write storage textures work correctly. -TEST_P(StorageTextureTests, ReadWrite2DArrayOr3DStorageTexture) { - // TODO(crbug.com/dawn/547): implement 3D storage texture on OpenGL and OpenGLES. - DAWN_TEST_UNSUPPORTED_IF(IsOpenGL() || IsOpenGLES()); - - constexpr uint32_t kSliceCount = 3u; - - constexpr wgpu::TextureFormat kTextureFormat = wgpu::TextureFormat::R32Uint; - - wgpu::TextureViewDimension dimensions[] = { - wgpu::TextureViewDimension::e2DArray, - wgpu::TextureViewDimension::e3D, - }; - - const std::vector initialTextureData = GetExpectedData(kTextureFormat, kSliceCount); - - for (wgpu::TextureViewDimension dimension : dimensions) { - // Prepare the read-only storage texture. - wgpu::Texture readonlyStorageTexture = - CreateTextureWithTestData(initialTextureData, kTextureFormat, dimension); - // Prepare the write-only storage texture. - wgpu::Texture writeonlyStorageTexture = CreateTexture( - kTextureFormat, wgpu::TextureUsage::StorageBinding | wgpu::TextureUsage::CopySrc, - kWidth, kHeight, kSliceCount, utils::ViewDimensionToTextureDimension(dimension)); - - // Read values from read-only storage texture and write into the write-only storage texture. - const std::string computeShader = CommonReadWriteTestCode(kTextureFormat, dimension); - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - EXPECT_DEPRECATION_WARNING(ReadWriteIntoStorageTextureInComputePass( - readonlyStorageTexture, writeonlyStorageTexture, computeShader.c_str(), dimension)); - - // Verify the data in the write-only storage texture is expected. - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - CheckOutputStorageTexture(writeonlyStorageTexture, kTextureFormat, kSliceCount); - } -} - -// Test that multiple dispatches to increment values by ping-ponging between a read-only storage -// texture and a write-only storage texture are synchronized in one pass. -TEST_P(StorageTextureTests, ReadonlyAndWriteonlyStorageTexturePingPong) { - // TODO(crbug.com/dawn/636): diagnose and fix this failure on OpenGL ES - DAWN_SUPPRESS_TEST_IF(IsOpenGLES()); - - constexpr wgpu::TextureFormat kTextureFormat = wgpu::TextureFormat::R32Uint; - wgpu::Texture storageTexture1 = CreateTexture( - kTextureFormat, wgpu::TextureUsage::StorageBinding | wgpu::TextureUsage::CopySrc, 1u, 1u); - wgpu::Texture storageTexture2 = CreateTexture( - kTextureFormat, wgpu::TextureUsage::StorageBinding | wgpu::TextureUsage::CopySrc, 1u, 1u); - - wgpu::ShaderModule module = utils::CreateShaderModule(device, R"( -[[group(0), binding(0)]] var Src : texture_storage_2d; -[[group(0), binding(1)]] var Dst : texture_storage_2d; -[[stage(compute), workgroup_size(1)]] fn main() { - var srcValue : vec4 = textureLoad(Src, vec2(0, 0)); - srcValue.x = srcValue.x + 1u; - textureStore(Dst, vec2(0, 0), srcValue); -} - )"); - - wgpu::ComputePipelineDescriptor pipelineDesc = {}; - pipelineDesc.compute.module = module; - pipelineDesc.compute.entryPoint = "main"; - wgpu::ComputePipeline pipeline; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is passed. - EXPECT_DEPRECATION_WARNING(pipeline = device.CreateComputePipeline(&pipelineDesc)); - - // In bindGroupA storageTexture1 is bound as read-only storage texture and storageTexture2 is - // bound as write-only storage texture. - wgpu::BindGroup bindGroupA = utils::MakeBindGroup(device, pipeline.GetBindGroupLayout(0), - { - {0, storageTexture1.CreateView()}, - {1, storageTexture2.CreateView()}, - }); - - // In bindGroupA storageTexture2 is bound as read-only storage texture and storageTexture1 is - // bound as write-only storage texture. - wgpu::BindGroup bindGroupB = utils::MakeBindGroup(device, pipeline.GetBindGroupLayout(0), - { - {0, storageTexture2.CreateView()}, - {1, storageTexture1.CreateView()}, - }); - - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - wgpu::ComputePassEncoder pass = encoder.BeginComputePass(); - pass.SetPipeline(pipeline); - - // After the first dispatch the value in storageTexture2 should be 1u. - pass.SetBindGroup(0, bindGroupA); - pass.Dispatch(1); - - // After the second dispatch the value in storageTexture1 should be 2u; - pass.SetBindGroup(0, bindGroupB); - pass.Dispatch(1); - - pass.EndPass(); - - wgpu::BufferDescriptor bufferDescriptor; - bufferDescriptor.size = sizeof(uint32_t); - bufferDescriptor.usage = wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::CopyDst; - wgpu::Buffer resultBuffer = device.CreateBuffer(&bufferDescriptor); - - wgpu::ImageCopyTexture imageCopyTexture; - imageCopyTexture.texture = storageTexture1; - - wgpu::ImageCopyBuffer imageCopyBuffer = utils::CreateImageCopyBuffer(resultBuffer, 0, 256, 1); - wgpu::Extent3D extent3D = {1, 1, 1}; - encoder.CopyTextureToBuffer(&imageCopyTexture, &imageCopyBuffer, &extent3D); - - wgpu::CommandBuffer commands = encoder.Finish(); - queue.Submit(1, &commands); - - constexpr uint32_t kFinalPixelValueInTexture1 = 2u; - EXPECT_BUFFER_U32_EQ(kFinalPixelValueInTexture1, resultBuffer, 0); -} - // 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) { @@ -1305,58 +976,6 @@ fn doTest() -> bool { })"; }; -// Verify that the texture is correctly cleared to 0 before its first usage as a read-only storage -// texture in a render pass. -TEST_P(StorageTextureZeroInitTests, ReadonlyStorageTextureClearsToZeroInRenderPass) { - wgpu::Texture readonlyStorageTexture = - CreateTexture(wgpu::TextureFormat::R32Uint, wgpu::TextureUsage::StorageBinding); - - // Create a rendering pipeline that reads the pixels from the read-only storage texture and uses - // green as the output color, otherwise uses red instead. - const char* kVertexShader = kSimpleVertexShader; - const std::string kFragmentShader = std::string(R"( -[[group(0), binding(0)]] var srcImage : texture_storage_2d; -)") + kCommonReadOnlyZeroInitTestCode + - R"( -[[stage(fragment)]] fn main() -> [[location(0)]] vec4 { - if (doTest()) { - return vec4(0.0, 1.0, 0.0, 1.0); - } - return vec4(1.0, 0.0, 0.0, 1.0); -})"; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is passed. - EXPECT_DEPRECATION_WARNING( - CheckDrawsGreen(kVertexShader, kFragmentShader.c_str(), readonlyStorageTexture)); -} - -// Verify that the texture is correctly cleared to 0 before its first usage as a read-only storage -// texture in a compute pass. -TEST_P(StorageTextureZeroInitTests, ReadonlyStorageTextureClearsToZeroInComputePass) { - wgpu::Texture readonlyStorageTexture = - CreateTexture(wgpu::TextureFormat::R32Uint, wgpu::TextureUsage::StorageBinding); - - // Create a compute shader that reads the pixels from the read-only storage texture and writes 1 - // to DstBuffer if they all have the expected value. - const std::string kComputeShader = std::string(R"( -[[block]] struct DstBuffer { - result : u32; -}; - -[[group(0), binding(0)]] var srcImage : texture_storage_2d; -[[group(0), binding(1)]] var dstBuffer : DstBuffer; -)") + kCommonReadOnlyZeroInitTestCode + R"( -[[stage(compute), workgroup_size(1)]] fn main() { - if (doTest()) { - dstBuffer.result = 1u; - } else { - dstBuffer.result = 0u; - } -})"; - - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is passed. - EXPECT_DEPRECATION_WARNING(CheckResultInStorageBuffer(readonlyStorageTexture, kComputeShader)); -} - // Verify that the texture is correctly cleared to 0 before its first usage as a write-only storage // storage texture in a render pass. TEST_P(StorageTextureZeroInitTests, WriteonlyStorageTextureClearsToZeroInRenderPass) { diff --git a/src/tests/unittests/validation/BindGroupValidationTests.cpp b/src/tests/unittests/validation/BindGroupValidationTests.cpp index a53cb7d1d2..109b93ca7e 100644 --- a/src/tests/unittests/validation/BindGroupValidationTests.cpp +++ b/src/tests/unittests/validation/BindGroupValidationTests.cpp @@ -1015,78 +1015,6 @@ TEST_F(BindGroupLayoutValidationTest, PerStageLimits) { } } -// This is the same test as PerStageLimits but for the deprecated ReadOnly storage textures. -// TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is passed. -TEST_F(BindGroupLayoutValidationTest, PerStageLimits_ReadOnlyStorageTexture) { - uint32_t maxCount = kMaxStorageTexturesPerShaderStage; - wgpu::BindGroupLayoutEntry entry = - BGLEntryType(wgpu::StorageTextureAccess::ReadOnly, wgpu::TextureFormat::RGBA8Unorm); - wgpu::BindGroupLayoutEntry otherEntry = BGLEntryType(wgpu::BufferBindingType::Uniform); - - wgpu::BindGroupLayout bgl[2]; - std::vector maxBindings; - - for (uint32_t i = 0; i < maxCount; ++i) { - entry.binding = i; - maxBindings.push_back(entry); - } - - // Creating with the maxes works. - EXPECT_DEPRECATION_WARNINGS( - bgl[0] = MakeBindGroupLayout(maxBindings.data(), maxBindings.size()), maxCount); - - // Adding an extra binding of a different type works. - { - std::vector bindings = maxBindings; - wgpu::BindGroupLayoutEntry newEntry = otherEntry; - newEntry.binding = maxCount; - bindings.push_back(newEntry); - EXPECT_DEPRECATION_WARNINGS(MakeBindGroupLayout(bindings.data(), bindings.size()), - maxCount); - } - - // Adding an extra binding of the maxed type in a different stage works - { - std::vector bindings = maxBindings; - wgpu::BindGroupLayoutEntry newEntry = entry; - newEntry.binding = maxCount; - newEntry.visibility = wgpu::ShaderStage::Fragment; - bindings.push_back(newEntry); - EXPECT_DEPRECATION_WARNINGS(MakeBindGroupLayout(bindings.data(), bindings.size()), - maxCount + 1); - } - - // Adding an extra binding of the maxed type and stage exceeds the per stage limit. - { - std::vector bindings = maxBindings; - wgpu::BindGroupLayoutEntry newEntry = entry; - newEntry.binding = maxCount; - bindings.push_back(newEntry); - EXPECT_DEPRECATION_WARNINGS( - ASSERT_DEVICE_ERROR(MakeBindGroupLayout(bindings.data(), bindings.size())), - maxCount + 1); - } - - // Creating a pipeline layout from the valid BGL works. - TestCreatePipelineLayout(bgl, 1, true); - - // Adding an extra binding of a different type in a different BGL works - bgl[1] = utils::MakeBindGroupLayout(device, {otherEntry}); - TestCreatePipelineLayout(bgl, 2, true); - - { - // Adding an extra binding of the maxed type in a different stage works - wgpu::BindGroupLayoutEntry newEntry = entry; - newEntry.visibility = wgpu::ShaderStage::Fragment; - EXPECT_DEPRECATION_WARNING(bgl[1] = utils::MakeBindGroupLayout(device, {newEntry})); - TestCreatePipelineLayout(bgl, 2, true); - } - - // Adding an extra binding of the maxed type in a different BGL exceeds the per stage limit. - EXPECT_DEPRECATION_WARNING(bgl[1] = utils::MakeBindGroupLayout(device, {entry})); - TestCreatePipelineLayout(bgl, 2, false); -} - // External textures require multiple binding slots (3 sampled texture, 1 uniform buffer, 1 // sampler), so ensure that these count towards the limit when combined non-external texture // bindings. diff --git a/src/tests/unittests/validation/ResourceUsageTrackingTests.cpp b/src/tests/unittests/validation/ResourceUsageTrackingTests.cpp index 58ab3ed74b..49c57337b7 100644 --- a/src/tests/unittests/validation/ResourceUsageTrackingTests.cpp +++ b/src/tests/unittests/validation/ResourceUsageTrackingTests.cpp @@ -814,51 +814,6 @@ namespace { } } - // Test that using a single texture in multiple read usages in the same pass is allowed. - TEST_F(ResourceUsageTrackingTest, TextureWithMultipleReadUsages) { - // Create a texture that will be used as both sampled and readonly storage texture - wgpu::Texture texture = - CreateTexture(wgpu::TextureUsage::TextureBinding | wgpu::TextureUsage::StorageBinding); - wgpu::TextureView view = texture.CreateView(); - - // Create a bind group to use the texture as sampled and readonly storage bindings - wgpu::BindGroupLayout bgl; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - EXPECT_DEPRECATION_WARNING( - bgl = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment | wgpu::ShaderStage::Compute, - wgpu::TextureSampleType::Float}, - {1, wgpu::ShaderStage::Fragment | wgpu::ShaderStage::Compute, - wgpu::StorageTextureAccess::ReadOnly, kFormat}})); - wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, view}, {1, view}}); - - // Test render pass - { - // Use the texture as both sampled and readonly storage in the same render pass - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - DummyRenderPass dummyRenderPass(device); - wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&dummyRenderPass); - pass.SetBindGroup(0, bg); - pass.EndPass(); - encoder.Finish(); - } - - // Test compute pass - { - // Use the texture as both sampled and readonly storage in the same compute pass - wgpu::ComputePipeline cp = CreateNoOpComputePipeline({bgl}); - - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - wgpu::ComputePassEncoder pass = encoder.BeginComputePass(); - pass.SetBindGroup(0, bg); - pass.SetPipeline(cp); - pass.Dispatch(1); - pass.EndPass(); - encoder.Finish(); - } - } - // Test that it is invalid to use the same texture as both readable and writable in the same // render pass. It is invalid in the same dispatch in compute pass. TEST_F(ResourceUsageTrackingTest, TextureWithReadAndWriteUsage) { @@ -1100,24 +1055,20 @@ namespace { // Test compute pass { // Create a texture that will be used storage texture - wgpu::Texture texture = CreateTexture(wgpu::TextureUsage::StorageBinding); + wgpu::Texture texture = CreateTexture(wgpu::TextureUsage::TextureBinding | + wgpu::TextureUsage::StorageBinding); wgpu::TextureView view = texture.CreateView(); - // Create bind groups to use the texture as readonly and writeonly bindings - wgpu::BindGroupLayout readBGL; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - EXPECT_DEPRECATION_WARNING( - readBGL = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::ReadOnly, - kFormat}})); + // Create bind groups to use the texture as sampled and writeonly bindings + wgpu::BindGroupLayout readBGL = utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Compute, wgpu::TextureSampleType::Float}}); wgpu::BindGroupLayout writeBGL = utils::MakeBindGroupLayout( device, {{0, wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::WriteOnly, kFormat}}); wgpu::BindGroup readBG = utils::MakeBindGroup(device, readBGL, {{0, view}}); wgpu::BindGroup writeBG = utils::MakeBindGroup(device, writeBGL, {{0, view}}); - // Use the textures as both readonly and writeonly storages in different passes + // Use the textures as both sampled and writeonly storages in different passes wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); wgpu::ComputePassEncoder pass0 = encoder.BeginComputePass(); @@ -1134,24 +1085,20 @@ namespace { // Test compute pass and render pass mixed together with resource dependency { // Create a texture that will be used a storage texture - wgpu::Texture texture = CreateTexture(wgpu::TextureUsage::StorageBinding); + wgpu::Texture texture = CreateTexture(wgpu::TextureUsage::TextureBinding | + wgpu::TextureUsage::StorageBinding); wgpu::TextureView view = texture.CreateView(); - // Create bind groups to use the texture as readonly and writeonly bindings + // Create bind groups to use the texture as sampled and writeonly bindings wgpu::BindGroupLayout writeBGL = utils::MakeBindGroupLayout( device, {{0, wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::WriteOnly, kFormat}}); - wgpu::BindGroupLayout readBGL; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - EXPECT_DEPRECATION_WARNING( - readBGL = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::StorageTextureAccess::ReadOnly, - kFormat}})); + wgpu::BindGroupLayout readBGL = utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Fragment, wgpu::TextureSampleType::Float}}); wgpu::BindGroup writeBG = utils::MakeBindGroup(device, writeBGL, {{0, view}}); wgpu::BindGroup readBG = utils::MakeBindGroup(device, readBGL, {{0, view}}); - // Use the texture as writeonly and readonly storage in compute pass and render + // Use the texture as writeonly and sampled storage in compute pass and render // pass respectively wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); @@ -1210,14 +1157,9 @@ namespace { // Test compute pass { - // Create bind groups to use the texture as readonly and writeonly storage bindings - wgpu::BindGroupLayout readBGL; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - EXPECT_DEPRECATION_WARNING( - readBGL = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::ReadOnly, - kFormat}})); + // Create bind groups to use the texture as sampled and writeonly storage bindings + wgpu::BindGroupLayout readBGL = utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Compute, wgpu::TextureSampleType::Float}}); wgpu::BindGroupLayout writeBGL = utils::MakeBindGroupLayout( device, {{0, wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::WriteOnly, kFormat}}); @@ -1285,14 +1227,9 @@ namespace { // Test compute pass { - // Create the bind group to use the texture as readonly and writeonly storage bindings - wgpu::BindGroupLayout readBGL; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - EXPECT_DEPRECATION_WARNING( - readBGL = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::ReadOnly, - kFormat}})); + // Create the bind group to use the texture as sampled and writeonly storage bindings + wgpu::BindGroupLayout readBGL = utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Compute, wgpu::TextureSampleType::Float}}); wgpu::BindGroupLayout writeBGL = utils::MakeBindGroupLayout( device, {{0, wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::WriteOnly, kFormat}}); @@ -1411,23 +1348,19 @@ namespace { // Test compute pass { // Create a texture that will be used both as storage texture - wgpu::Texture texture0 = CreateTexture(wgpu::TextureUsage::StorageBinding); + wgpu::Texture texture0 = CreateTexture(wgpu::TextureUsage::TextureBinding | + wgpu::TextureUsage::StorageBinding); wgpu::TextureView view0 = texture0.CreateView(); - wgpu::Texture texture1 = CreateTexture(wgpu::TextureUsage::StorageBinding); + wgpu::Texture texture1 = CreateTexture(wgpu::TextureUsage::TextureBinding); wgpu::TextureView view1 = texture1.CreateView(); - // Create the bind group to use the texture as readonly and writeonly bindings + // Create the bind group to use the texture as sampled and writeonly bindings wgpu::BindGroupLayout writeBGL = utils::MakeBindGroupLayout( device, {{0, wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::WriteOnly, kFormat}}); - wgpu::BindGroupLayout readBGL; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - EXPECT_DEPRECATION_WARNING( - readBGL = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::ReadOnly, - kFormat}})); + wgpu::BindGroupLayout readBGL = utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Compute, wgpu::TextureSampleType::Float}}); wgpu::BindGroup writeBG0 = utils::MakeBindGroup(device, writeBGL, {{0, view0}}); wgpu::BindGroup readBG0 = utils::MakeBindGroup(device, readBGL, {{0, view0}}); @@ -1437,7 +1370,7 @@ namespace { wgpu::ComputePipeline cp = CreateNoOpComputePipeline({writeBGL, readBGL}); // Set bind group on the same index twice. The second one overwrites the first one. - // No texture is used as both readonly and writeonly storage in the same dispatch so + // No texture is used as both sampled and writeonly storage in the same dispatch so // there are no errors. { wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); @@ -1452,7 +1385,7 @@ namespace { } // Set bind group on the same index twice. The second one overwrites the first one. - // texture0 is used as both writeonly and readonly storage in the same dispatch, which + // texture0 is used as both writeonly and sampled storage in the same dispatch, which // is an error. { wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); @@ -1472,22 +1405,18 @@ namespace { // visible to the programmable pass where it is used. TEST_F(ResourceUsageTrackingTest, TextureUsageConflictBetweenInvisibleStagesInBindGroup) { // Create texture and texture view - wgpu::Texture texture = CreateTexture(wgpu::TextureUsage::StorageBinding); + wgpu::Texture texture = + CreateTexture(wgpu::TextureUsage::TextureBinding | wgpu::TextureUsage::StorageBinding); wgpu::TextureView view = texture.CreateView(); - // Test render pass for bind group. The conflict of readonly storage and writeonly storage + // Test render pass for bind group. The conflict of sampled storage and writeonly storage // usage doesn't reside in render related stages at all { // Create a bind group whose bindings are not visible in render pass - wgpu::BindGroupLayout bgl; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - EXPECT_DEPRECATION_WARNING( - bgl = utils::MakeBindGroupLayout( - device, - {{0, wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::ReadOnly, kFormat}, - {1, wgpu::ShaderStage::None, wgpu::StorageTextureAccess::WriteOnly, - kFormat}})); + wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout( + device, + {{0, wgpu::ShaderStage::Compute, wgpu::TextureSampleType::Float}, + {1, wgpu::ShaderStage::None, wgpu::StorageTextureAccess::WriteOnly, kFormat}}); wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, view}, {1, view}}); // These two bindings are invisible in render pass. But we still track these bindings. @@ -1499,19 +1428,14 @@ namespace { ASSERT_DEVICE_ERROR(encoder.Finish()); } - // Test compute pass for bind group. The conflict of readonly storage and writeonly storage + // Test compute pass for bind group. The conflict of sampled storage and writeonly storage // usage doesn't reside in compute related stage at all { // Create a bind group whose bindings are not visible in compute pass - wgpu::BindGroupLayout bgl; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - EXPECT_DEPRECATION_WARNING( - bgl = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::StorageTextureAccess::ReadOnly, - kFormat}, - {1, wgpu::ShaderStage::None, wgpu::StorageTextureAccess::WriteOnly, - kFormat}})); + wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout( + device, + {{0, wgpu::ShaderStage::Fragment, wgpu::TextureSampleType::Float}, + {1, wgpu::ShaderStage::None, wgpu::StorageTextureAccess::WriteOnly, kFormat}}); wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, view}, {1, view}}); // Create a no-op compute pipeline. @@ -1532,8 +1456,9 @@ namespace { // visible to the programmable pass where it is used. TEST_F(ResourceUsageTrackingTest, TextureUsageConflictWithInvisibleStageInBindGroup) { // Create texture and texture view - wgpu::Texture texture = CreateTexture(wgpu::TextureUsage::StorageBinding | - wgpu::TextureUsage::RenderAttachment); + wgpu::Texture texture = + CreateTexture(wgpu::TextureUsage::TextureBinding | wgpu::TextureUsage::StorageBinding | + wgpu::TextureUsage::RenderAttachment); wgpu::TextureView view = texture.CreateView(); // Test render pass @@ -1541,14 +1466,9 @@ namespace { // Create the render pass that will use the texture as an render attachment utils::ComboRenderPassDescriptor renderPass({view}); - // Create a bind group which use the texture as readonly storage in compute stage - wgpu::BindGroupLayout bgl; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - EXPECT_DEPRECATION_WARNING( - bgl = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::ReadOnly, - kFormat}})); + // Create a bind group which use the texture as sampled storage in compute stage + wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Compute, wgpu::TextureSampleType::Float}}); wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, view}}); // Texture usage in compute stage in bind group conflicts with render target. And @@ -1564,15 +1484,10 @@ namespace { // Test compute pass { // Create a bind group which contains both fragment and compute stages - wgpu::BindGroupLayout bgl; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - EXPECT_DEPRECATION_WARNING( - bgl = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::StorageTextureAccess::ReadOnly, - kFormat}, - {1, wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::WriteOnly, - kFormat}})); + wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout( + device, + {{0, wgpu::ShaderStage::Fragment, wgpu::TextureSampleType::Float}, + {1, wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::WriteOnly, kFormat}}); wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, view}, {1, view}}); // Create a no-op compute pipeline. @@ -1595,17 +1510,14 @@ namespace { // used in the pipeline. TEST_F(ResourceUsageTrackingTest, TextureUsageConflictWithUnusedPipelineBindings) { // Create texture and texture view - wgpu::Texture texture = CreateTexture(wgpu::TextureUsage::StorageBinding); + wgpu::Texture texture = + CreateTexture(wgpu::TextureUsage::TextureBinding | wgpu::TextureUsage::StorageBinding); wgpu::TextureView view = texture.CreateView(); // Create bind groups. - wgpu::BindGroupLayout readBGL; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - EXPECT_DEPRECATION_WARNING( - readBGL = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment | wgpu::ShaderStage::Compute, - wgpu::StorageTextureAccess::ReadOnly, kFormat}})); + wgpu::BindGroupLayout readBGL = utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Fragment | wgpu::ShaderStage::Compute, + wgpu::TextureSampleType::Float}}); wgpu::BindGroupLayout writeBGL = utils::MakeBindGroupLayout( device, {{0, wgpu::ShaderStage::Fragment | wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::WriteOnly, kFormat}}); @@ -1614,14 +1526,14 @@ namespace { // Test render pass { - // Create a passthrough render pipeline with a readonly storage texture + // Create a passthrough render pipeline with a sampled storage texture wgpu::ShaderModule vsModule = utils::CreateShaderModule(device, R"( [[stage(vertex)]] fn main() -> [[builtin(position)]] vec4 { return vec4(); })"); wgpu::ShaderModule fsModule = utils::CreateShaderModule(device, R"( - [[group(0), binding(0)]] var tex : texture_storage_2d; + [[group(0), binding(0)]] var tex : texture_2d; [[stage(fragment)]] fn main() { })"); utils::ComboRenderPipelineDescriptor pipelineDescriptor; diff --git a/src/tests/unittests/validation/StorageTextureValidationTests.cpp b/src/tests/unittests/validation/StorageTextureValidationTests.cpp index 0cfa3af6b2..65287712b1 100644 --- a/src/tests/unittests/validation/StorageTextureValidationTests.cpp +++ b/src/tests/unittests/validation/StorageTextureValidationTests.cpp @@ -70,9 +70,6 @@ class StorageTextureValidationTests : public ValidationTest { const char* imageTypeDeclaration = "texture_storage_2d") { const char* access = ""; switch (storageTextureBindingType) { - case wgpu::StorageTextureAccess::ReadOnly: - access = "read"; - break; case wgpu::StorageTextureAccess::WriteOnly: access = "write"; break; @@ -110,61 +107,13 @@ class StorageTextureValidationTests : public ValidationTest { wgpu::ShaderModule mDefaultVSModule; wgpu::ShaderModule mDefaultFSModule; - const std::array kSupportedStorageTextureAccess = { - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is - // passed. - wgpu::StorageTextureAccess::ReadOnly, wgpu::StorageTextureAccess::WriteOnly}; + const std::array kSupportedStorageTextureAccess = { + wgpu::StorageTextureAccess::WriteOnly}; }; -// TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is passed. -#define WARNING_IF_READONLY(statement, access) \ - do { \ - if (access == wgpu::StorageTextureAccess::ReadOnly) { \ - EXPECT_DEPRECATION_WARNING(statement); \ - } else { \ - statement; \ - } \ - } while (0) - // Validate read-only storage textures can be declared in vertex and fragment shaders, while // writeonly storage textures cannot be used in vertex shaders. TEST_F(StorageTextureValidationTests, RenderPipeline) { - // Readonly storage texture can be declared in a vertex shader. - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is passed. - { - wgpu::ShaderModule vsModule = utils::CreateShaderModule(device, R"( - [[group(0), binding(0)]] var image0 : texture_storage_2d; - [[stage(vertex)]] fn main( - [[builtin(vertex_index)]] VertexIndex : u32 - ) -> [[builtin(position)]] vec4 { - return textureLoad(image0, vec2(i32(VertexIndex), 0)); - })"); - - utils::ComboRenderPipelineDescriptor descriptor; - descriptor.layout = nullptr; - descriptor.vertex.module = vsModule; - descriptor.cFragment.module = mDefaultFSModule; - EXPECT_DEPRECATION_WARNING(device.CreateRenderPipeline(&descriptor)); - } - - // Read-only storage textures can be declared in a fragment shader. - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is passed. - { - wgpu::ShaderModule fsModule = utils::CreateShaderModule(device, R"( - [[group(0), binding(0)]] var image0 : texture_storage_2d; - [[stage(fragment)]] fn main( - [[builtin(position)]] FragCoord : vec4 - ) -> [[location(0)]] vec4 { - return textureLoad(image0, vec2(FragCoord.xy)); - })"); - - utils::ComboRenderPipelineDescriptor descriptor; - descriptor.layout = nullptr; - descriptor.vertex.module = mDefaultVSModule; - descriptor.cFragment.module = fsModule; - EXPECT_DEPRECATION_WARNING(device.CreateRenderPipeline(&descriptor)); - } - // Write-only storage textures cannot be declared in a vertex shader. { wgpu::ShaderModule vsModule = utils::CreateShaderModule(device, R"( @@ -202,29 +151,6 @@ TEST_F(StorageTextureValidationTests, RenderPipeline) { // Validate both read-only and write-only storage textures can be declared in // compute shaders. TEST_F(StorageTextureValidationTests, ComputePipeline) { - // Read-only storage textures can be declared in a compute shader. - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is passed. - { - wgpu::ShaderModule csModule = utils::CreateShaderModule(device, R"( - [[group(0), binding(0)]] var image0 : texture_storage_2d; - - [[block]] struct Buf { - data : f32; - }; - [[group(0), binding(1)]] var buf : Buf; - - [[stage(compute), workgroup_size(1)]] fn main([[builtin(local_invocation_id)]] LocalInvocationID : vec3) { - buf.data = textureLoad(image0, vec2(LocalInvocationID.xy)).x; - })"); - - wgpu::ComputePipelineDescriptor descriptor; - descriptor.layout = nullptr; - descriptor.compute.module = csModule; - descriptor.compute.entryPoint = "main"; - - EXPECT_DEPRECATION_WARNING(device.CreateComputePipeline(&descriptor)); - } - // Write-only storage textures can be declared in a compute shader. { wgpu::ShaderModule csModule = utils::CreateShaderModule(device, R"( @@ -282,11 +208,8 @@ TEST_F(StorageTextureValidationTests, BindGroupLayoutWithStorageTextureBindingTy bool valid; }; constexpr std::array kTestSpecs = { - {{wgpu::ShaderStage::Vertex, wgpu::StorageTextureAccess::ReadOnly, true}, - {wgpu::ShaderStage::Vertex, wgpu::StorageTextureAccess::WriteOnly, false}, - {wgpu::ShaderStage::Fragment, wgpu::StorageTextureAccess::ReadOnly, true}, + {{wgpu::ShaderStage::Vertex, wgpu::StorageTextureAccess::WriteOnly, false}, {wgpu::ShaderStage::Fragment, wgpu::StorageTextureAccess::WriteOnly, true}, - {wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::ReadOnly, true}, {wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::WriteOnly, true}}}; for (const auto& testSpec : kTestSpecs) { @@ -298,10 +221,9 @@ TEST_F(StorageTextureValidationTests, BindGroupLayoutWithStorageTextureBindingTy descriptor.entries = &entry; if (testSpec.valid) { - WARNING_IF_READONLY(device.CreateBindGroupLayout(&descriptor), testSpec.type); + device.CreateBindGroupLayout(&descriptor); } else { - WARNING_IF_READONLY(ASSERT_DEVICE_ERROR(device.CreateBindGroupLayout(&descriptor)), - testSpec.type); + ASSERT_DEVICE_ERROR(device.CreateBindGroupLayout(&descriptor)); } } } @@ -409,8 +331,6 @@ TEST_F(StorageTextureValidationTests, BindGroupLayoutEntryTypeMatchesShaderDecla {0, wgpu::ShaderStage::Compute, wgpu::BufferBindingType::ReadOnlyStorage}, {0, wgpu::ShaderStage::Compute, wgpu::SamplerBindingType::Filtering}, {0, wgpu::ShaderStage::Compute, wgpu::TextureSampleType::Float}, - {0, wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::ReadOnly, - kStorageTextureFormat}, {0, wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::WriteOnly, kStorageTextureFormat}}; @@ -431,10 +351,8 @@ TEST_F(StorageTextureValidationTests, BindGroupLayoutEntryTypeMatchesShaderDecla defaultComputePipelineDescriptor; // Create bind group layout with different binding types. - wgpu::BindGroupLayout bindGroupLayout; - WARNING_IF_READONLY( - bindGroupLayout = utils::MakeBindGroupLayout(device, {bindingLayoutEntry}), - bindingLayoutEntry.storageTexture.access); + wgpu::BindGroupLayout bindGroupLayout = + utils::MakeBindGroupLayout(device, {bindingLayoutEntry}); computePipelineDescriptor.layout = utils::MakeBasicPipelineLayout(device, &bindGroupLayout); @@ -476,8 +394,7 @@ TEST_F(StorageTextureValidationTests, StorageTextureFormatInBindGroupLayout) { bindGroupLayoutBinding.storageTexture.access = bindingType; bindGroupLayoutBinding.storageTexture.format = textureFormat; if (utils::TextureFormatSupportsStorageTexture(textureFormat)) { - WARNING_IF_READONLY(utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}), - bindingType); + utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}); } else { ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding})); } @@ -519,10 +436,8 @@ TEST_F(StorageTextureValidationTests, BindGroupLayoutStorageTextureFormatMatches wgpu::BindGroupLayoutEntry bindGroupLayoutBinding = defaultBindGroupLayoutEntry; bindGroupLayoutBinding.storageTexture.format = storageTextureFormatInBindGroupLayout; - wgpu::BindGroupLayout bindGroupLayout; - WARNING_IF_READONLY( - bindGroupLayout = utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}), - bindingType); + wgpu::BindGroupLayout bindGroupLayout = + utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}); // Create the compute pipeline with the bind group layout. wgpu::ComputePipelineDescriptor computePipelineDescriptor = @@ -570,10 +485,8 @@ TEST_F(StorageTextureValidationTests, BindGroupLayoutViewDimensionMatchesShaderD // Create the bind group layout with the given texture view dimension. wgpu::BindGroupLayoutEntry bindGroupLayoutBinding = defaultBindGroupLayoutEntry; bindGroupLayoutBinding.storageTexture.viewDimension = dimensionInBindGroupLayout; - wgpu::BindGroupLayout bindGroupLayout; - WARNING_IF_READONLY( - bindGroupLayout = utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}), - bindingType); + wgpu::BindGroupLayout bindGroupLayout = + utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}); // Create the compute pipeline with the bind group layout. wgpu::ComputePipelineDescriptor computePipelineDescriptor = @@ -604,10 +517,8 @@ TEST_F(StorageTextureValidationTests, StorageTextureBindingTypeInBindGroup) { bindGroupLayoutBinding.visibility = wgpu::ShaderStage::Compute; bindGroupLayoutBinding.storageTexture.access = storageBindingType; bindGroupLayoutBinding.storageTexture.format = kStorageTextureFormat; - wgpu::BindGroupLayout bindGroupLayout; - WARNING_IF_READONLY( - bindGroupLayout = utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}), - storageBindingType); + wgpu::BindGroupLayout bindGroupLayout = + utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}); // Buffers are not allowed to be used as storage textures in a bind group. { @@ -650,10 +561,8 @@ TEST_F(StorageTextureValidationTests, StorageTextureUsageInBindGroup) { bindGroupLayoutBinding.visibility = wgpu::ShaderStage::Compute; bindGroupLayoutBinding.storageTexture.access = storageBindingType; bindGroupLayoutBinding.storageTexture.format = wgpu::TextureFormat::R32Float; - wgpu::BindGroupLayout bindGroupLayout; - WARNING_IF_READONLY( - bindGroupLayout = utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}), - storageBindingType); + wgpu::BindGroupLayout bindGroupLayout = + utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}); for (wgpu::TextureUsage usage : kTextureUsages) { // Create texture views with different texture usages @@ -689,10 +598,8 @@ TEST_F(StorageTextureValidationTests, StorageTextureFormatInBindGroup) { // Create a bind group layout with given storage texture format. wgpu::BindGroupLayoutEntry bindGroupLayoutBinding = defaultBindGroupLayoutEntry; bindGroupLayoutBinding.storageTexture.format = formatInBindGroupLayout; - wgpu::BindGroupLayout bindGroupLayout; - WARNING_IF_READONLY( - bindGroupLayout = utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}), - storageBindingType); + wgpu::BindGroupLayout bindGroupLayout = + utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}); for (wgpu::TextureFormat textureViewFormat : utils::kAllTextureFormats) { if (!utils::TextureFormatSupportsStorageTexture(textureViewFormat)) { @@ -747,10 +654,8 @@ TEST_F(StorageTextureValidationTests, StorageTextureViewDimensionInBindGroup) { // Create a bind group layout with given texture view dimension. wgpu::BindGroupLayoutEntry bindGroupLayoutBinding = defaultBindGroupLayoutEntry; bindGroupLayoutBinding.storageTexture.viewDimension = dimensionInBindGroupLayout; - wgpu::BindGroupLayout bindGroupLayout; - WARNING_IF_READONLY( - bindGroupLayout = utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}), - storageBindingType); + wgpu::BindGroupLayout bindGroupLayout = + utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}); for (wgpu::TextureViewDimension dimensionOfTextureView : kSupportedDimensions) { // Create a texture view with given texture view dimension. @@ -796,11 +701,8 @@ TEST_F(StorageTextureValidationTests, StorageTextureInRenderPass) { for (wgpu::StorageTextureAccess storageTextureType : kSupportedStorageTextureAccess) { // Create a bind group that contains a storage texture. - wgpu::BindGroupLayout bindGroupLayout; - WARNING_IF_READONLY( - bindGroupLayout = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, storageTextureType, kFormat}}), - storageTextureType); + wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Fragment, storageTextureType, kFormat}}); wgpu::BindGroup bindGroupWithStorageTexture = utils::MakeBindGroup(device, bindGroupLayout, {{0, storageTexture.CreateView()}}); @@ -830,12 +732,9 @@ TEST_F(StorageTextureValidationTests, StorageTextureAndSampledTextureInOneRender for (wgpu::StorageTextureAccess storageTextureType : kSupportedStorageTextureAccess) { // Create a bind group that binds the same texture as both storage texture and sampled // texture. - wgpu::BindGroupLayout bindGroupLayout; - WARNING_IF_READONLY( - bindGroupLayout = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, storageTextureType, kFormat}, - {1, wgpu::ShaderStage::Fragment, wgpu::TextureSampleType::Float}}), - storageTextureType); + wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Fragment, storageTextureType, kFormat}, + {1, wgpu::ShaderStage::Fragment, wgpu::TextureSampleType::Float}}); wgpu::BindGroup bindGroup = utils::MakeBindGroup( device, bindGroupLayout, {{0, storageTexture.CreateView()}, {1, storageTexture.CreateView()}}); @@ -848,9 +747,6 @@ TEST_F(StorageTextureValidationTests, StorageTextureAndSampledTextureInOneRender renderPassEncoder.SetBindGroup(0, bindGroup); renderPassEncoder.EndPass(); switch (storageTextureType) { - case wgpu::StorageTextureAccess::ReadOnly: - encoder.Finish(); - break; case wgpu::StorageTextureAccess::WriteOnly: ASSERT_DEVICE_ERROR(encoder.Finish()); break; @@ -871,11 +767,8 @@ TEST_F(StorageTextureValidationTests, StorageTextureAndRenderAttachmentInOneRend for (wgpu::StorageTextureAccess storageTextureType : kSupportedStorageTextureAccess) { // Create a bind group that contains a storage texture. - wgpu::BindGroupLayout bindGroupLayout; - WARNING_IF_READONLY( - bindGroupLayout = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, storageTextureType, kFormat}}), - storageTextureType); + wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Fragment, storageTextureType, kFormat}}); wgpu::BindGroup bindGroupWithStorageTexture = utils::MakeBindGroup(device, bindGroupLayout, {{0, storageTexture.CreateView()}}); @@ -889,36 +782,6 @@ TEST_F(StorageTextureValidationTests, StorageTextureAndRenderAttachmentInOneRend } } -// Verify it is invalid to use a a texture as both read-only storage texture and write-only storage -// texture in one render pass. -TEST_F(StorageTextureValidationTests, ReadOnlyAndWriteOnlyStorageTextureInOneRenderPass) { - constexpr wgpu::TextureFormat kFormat = wgpu::TextureFormat::RGBA8Unorm; - wgpu::Texture storageTexture = CreateTexture(wgpu::TextureUsage::StorageBinding, kFormat); - - // Create a bind group that uses the same texture as both read-only and write-only storage - // texture. - wgpu::BindGroupLayout bindGroupLayout; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is passed. - EXPECT_DEPRECATION_WARNING( - bindGroupLayout = utils::MakeBindGroupLayout( - device, - {{0, wgpu::ShaderStage::Fragment, wgpu::StorageTextureAccess::ReadOnly, kFormat}, - {1, wgpu::ShaderStage::Fragment, wgpu::StorageTextureAccess::WriteOnly, kFormat}})); - wgpu::BindGroup bindGroup = - utils::MakeBindGroup(device, bindGroupLayout, - {{0, storageTexture.CreateView()}, {1, storageTexture.CreateView()}}); - - // It is invalid to use a texture as both read-only storage texture and write-only storage - // texture in one render pass. - wgpu::Texture outputAttachment = CreateTexture(wgpu::TextureUsage::RenderAttachment, kFormat); - utils::ComboRenderPassDescriptor renderPassDescriptor({outputAttachment.CreateView()}); - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - wgpu::RenderPassEncoder renderPassEncoder = encoder.BeginRenderPass(&renderPassDescriptor); - renderPassEncoder.SetBindGroup(0, bindGroup); - renderPassEncoder.EndPass(); - ASSERT_DEVICE_ERROR(encoder.Finish()); -} - // Verify it is valid to use a texture as both storage texture (read-only or write-only) and // sampled texture in one compute pass. TEST_F(StorageTextureValidationTests, StorageTextureAndSampledTextureInOneComputePass) { @@ -929,12 +792,9 @@ TEST_F(StorageTextureValidationTests, StorageTextureAndSampledTextureInOneComput for (wgpu::StorageTextureAccess storageTextureType : kSupportedStorageTextureAccess) { // Create a bind group that binds the same texture as both storage texture and sampled // texture. - wgpu::BindGroupLayout bindGroupLayout; - WARNING_IF_READONLY( - bindGroupLayout = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Compute, storageTextureType, kFormat}, - {1, wgpu::ShaderStage::Compute, wgpu::TextureSampleType::Float}}), - storageTextureType); + wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Compute, storageTextureType, kFormat}, + {1, wgpu::ShaderStage::Compute, wgpu::TextureSampleType::Float}}); wgpu::BindGroup bindGroup = utils::MakeBindGroup( device, bindGroupLayout, {{0, storageTexture.CreateView()}, {1, storageTexture.CreateView()}}); @@ -948,31 +808,3 @@ TEST_F(StorageTextureValidationTests, StorageTextureAndSampledTextureInOneComput encoder.Finish(); } } - -// Verify it is valid to use a texture as both read-only storage texture and write-only storage -// texture in one compute pass. -TEST_F(StorageTextureValidationTests, ReadOnlyAndWriteOnlyStorageTextureInOneComputePass) { - constexpr wgpu::TextureFormat kFormat = wgpu::TextureFormat::RGBA8Unorm; - wgpu::Texture storageTexture = CreateTexture(wgpu::TextureUsage::StorageBinding, kFormat); - - // Create a bind group that uses the same texture as both read-only and write-only storage - // texture. - wgpu::BindGroupLayout bindGroupLayout; - // TODO(crbug.com/dawn/1025): Remove once ReadOnly storage texture deprecation period is passed. - EXPECT_DEPRECATION_WARNING( - bindGroupLayout = utils::MakeBindGroupLayout( - device, - {{0, wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::ReadOnly, kFormat}, - {1, wgpu::ShaderStage::Compute, wgpu::StorageTextureAccess::WriteOnly, kFormat}})); - wgpu::BindGroup bindGroup = - utils::MakeBindGroup(device, bindGroupLayout, - {{0, storageTexture.CreateView()}, {1, storageTexture.CreateView()}}); - - // It is valid to use a texture as both read-only storage texture and write-only storage - // texture in one compute pass. - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - wgpu::ComputePassEncoder computePassEncoder = encoder.BeginComputePass(); - computePassEncoder.SetBindGroup(0, bindGroup); - computePassEncoder.EndPass(); - encoder.Finish(); -} diff --git a/src/tests/unittests/validation/TextureSubresourceTests.cpp b/src/tests/unittests/validation/TextureSubresourceTests.cpp index 87c0d43673..f49c86d6fd 100644 --- a/src/tests/unittests/validation/TextureSubresourceTests.cpp +++ b/src/tests/unittests/validation/TextureSubresourceTests.cpp @@ -67,26 +67,8 @@ namespace { encoder.Finish(); } - // It is valid to has multiple read from a subresource and one single write into another - // subresource - { - wgpu::BindGroup bindGroup = utils::MakeBindGroup(device, bgl, {{0, samplerView}}); - - wgpu::BindGroupLayout bgl1; - EXPECT_DEPRECATION_WARNING( - bgl1 = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, - wgpu::StorageTextureAccess::ReadOnly, kFormat}})); - - wgpu::BindGroup bindGroup1 = utils::MakeBindGroup(device, bgl1, {{0, samplerView}}); - - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPassDesc); - pass.SetBindGroup(0, bindGroup); - pass.SetBindGroup(1, bindGroup1); - pass.EndPass(); - encoder.Finish(); - } + // It is not currently possible to test that it is valid to have multiple reads from a + // subresource while there is a single write in another subresource. // It is invalid to read and write into the same subresources { diff --git a/src/tests/white_box/InternalResourceUsageTests.cpp b/src/tests/white_box/InternalResourceUsageTests.cpp index 92c28a479c..a260fe5dd6 100644 --- a/src/tests/white_box/InternalResourceUsageTests.cpp +++ b/src/tests/white_box/InternalResourceUsageTests.cpp @@ -37,18 +37,6 @@ TEST_P(InternalResourceUsageTests, InternalBufferUsage) { ASSERT_DEVICE_ERROR(CreateBuffer(dawn_native::kInternalStorageBuffer)); } -// Verify it is an error to create a texture with a texture usage that should only be used -// internally. -TEST_P(InternalResourceUsageTests, InternalTextureUsage) { - DAWN_TEST_UNSUPPORTED_IF(HasToggleEnabled("skip_validation")); - - wgpu::TextureDescriptor descriptor; - descriptor.format = wgpu::TextureFormat::RGBA8Unorm; - descriptor.size = {1, 1, 1}; - descriptor.usage = dawn_native::kReadOnlyStorageTexture; - ASSERT_DEVICE_ERROR(device.CreateTexture(&descriptor)); -} - DAWN_INSTANTIATE_TEST(InternalResourceUsageTests, NullBackend()); class InternalBindingTypeTests : public DawnTest {};