From 8edb723dea603d7cea0e4443b792a198af0195cf Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Mon, 20 Apr 2020 17:21:52 +0000 Subject: [PATCH] Revert "Add ComparisonSampler binding type and validation tests" This reverts commit 6d9e4f8076b645c557453f4b566bf9c38b4a51eb. Reason for revert: Breaks the roll in Chromium, gpu_sampler.cc must first be fixed to use the new undefined value, before this can be landed. Original change's description: > Add ComparisonSampler binding type and validation tests > > Bug: dawn:367 > Change-Id: Iba1d3d03f6247a356b6f3fabfe7a7ba3c0753171 > Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/18423 > Reviewed-by: Austin Eng > Commit-Queue: Austin Eng TBR=cwallez@chromium.org,kainino@chromium.org,enga@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: dawn:367 Change-Id: Ic071a601df2063bd2da5388b2e75c1a121924a69 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/19983 Reviewed-by: Corentin Wallez Commit-Queue: Corentin Wallez --- dawn.json | 9 +- src/dawn_native/BindGroup.cpp | 25 +--- .../BindGroupAndStorageBarrierTracker.h | 1 - src/dawn_native/BindGroupLayout.cpp | 5 - src/dawn_native/PipelineLayout.cpp | 1 - src/dawn_native/ProgrammablePassEncoder.cpp | 1 - src/dawn_native/Sampler.cpp | 4 - src/dawn_native/Sampler.h | 2 - src/dawn_native/ShaderModule.cpp | 16 +-- src/dawn_native/d3d12/BindGroupD3D12.cpp | 3 +- .../d3d12/BindGroupLayoutD3D12.cpp | 2 - src/dawn_native/d3d12/CommandBufferD3D12.cpp | 2 - src/dawn_native/d3d12/PipelineLayoutD3D12.cpp | 1 - src/dawn_native/metal/CommandBufferMTL.mm | 3 +- src/dawn_native/metal/PipelineLayoutMTL.mm | 1 - src/dawn_native/opengl/CommandBufferGL.cpp | 3 +- src/dawn_native/opengl/PipelineGL.cpp | 1 - src/dawn_native/opengl/PipelineLayoutGL.cpp | 1 - src/dawn_native/vulkan/BindGroupLayoutVk.cpp | 1 - src/dawn_native/vulkan/BindGroupVk.cpp | 3 +- src/dawn_native/vulkan/CommandBufferVk.cpp | 1 - src/tests/end2end/BindGroupTests.cpp | 5 +- src/tests/end2end/SamplerTests.cpp | 5 +- src/tests/end2end/TextureViewTests.cpp | 5 +- .../validation/BindGroupValidationTests.cpp | 123 ------------------ src/utils/WGPUHelpers.cpp | 5 +- 26 files changed, 30 insertions(+), 199 deletions(-) diff --git a/dawn.json b/dawn.json index 7564e53ac6..49a562c0e1 100644 --- a/dawn.json +++ b/dawn.json @@ -115,11 +115,10 @@ {"value": 1, "name": "storage buffer"}, {"value": 2, "name": "readonly storage buffer"}, {"value": 3, "name": "sampler"}, - {"value": 4, "name": "comparison sampler"}, - {"value": 5, "name": "sampled texture"}, - {"value": 6, "name": "storage texture"}, - {"value": 7, "name": "readonly storage texture"}, - {"value": 8, "name": "writeonly storage texture"} + {"value": 4, "name": "sampled texture"}, + {"value": 5, "name": "storage texture"}, + {"value": 6, "name": "readonly storage texture"}, + {"value": 7, "name": "writeonly storage texture"} ] }, "blend descriptor": { diff --git a/src/dawn_native/BindGroup.cpp b/src/dawn_native/BindGroup.cpp index 8d99bad015..61ecf68834 100644 --- a/src/dawn_native/BindGroup.cpp +++ b/src/dawn_native/BindGroup.cpp @@ -117,30 +117,13 @@ namespace dawn_native { return {}; } - MaybeError ValidateSamplerBinding(const DeviceBase* device, - const BindGroupEntry& binding, - wgpu::BindingType bindingType) { + MaybeError ValidateSamplerBinding(const DeviceBase* device, const BindGroupEntry& binding) { if (binding.sampler == nullptr || binding.textureView != nullptr || binding.buffer != nullptr) { return DAWN_VALIDATION_ERROR("expected sampler binding"); } DAWN_TRY(device->ValidateObject(binding.sampler)); - switch (bindingType) { - case wgpu::BindingType::Sampler: - if (binding.sampler->HasCompareFunction()) { - return DAWN_VALIDATION_ERROR("Did not expect comparison sampler"); - } - break; - case wgpu::BindingType::ComparisonSampler: - if (!binding.sampler->HasCompareFunction()) { - return DAWN_VALIDATION_ERROR("Expected comparison sampler"); - } - break; - default: - UNREACHABLE(); - } - return {}; } @@ -191,8 +174,7 @@ namespace dawn_native { bindingInfo)); break; case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: - DAWN_TRY(ValidateSamplerBinding(device, binding, bindingInfo.type)); + DAWN_TRY(ValidateSamplerBinding(device, binding)); break; // TODO(jiawei.shao@intel.com): support creating bind group with read-only and // write-only storage textures. @@ -312,8 +294,7 @@ namespace dawn_native { SamplerBase* BindGroupBase::GetBindingAsSampler(BindingIndex bindingIndex) { ASSERT(!IsError()); ASSERT(bindingIndex < mLayout->GetBindingCount()); - ASSERT(mLayout->GetBindingInfo(bindingIndex).type == wgpu::BindingType::Sampler || - mLayout->GetBindingInfo(bindingIndex).type == wgpu::BindingType::ComparisonSampler); + ASSERT(mLayout->GetBindingInfo(bindingIndex).type == wgpu::BindingType::Sampler); return static_cast(mBindingData.bindings[bindingIndex].Get()); } diff --git a/src/dawn_native/BindGroupAndStorageBarrierTracker.h b/src/dawn_native/BindGroupAndStorageBarrierTracker.h index 94a2be8d65..3b0849fd8c 100644 --- a/src/dawn_native/BindGroupAndStorageBarrierTracker.h +++ b/src/dawn_native/BindGroupAndStorageBarrierTracker.h @@ -52,7 +52,6 @@ namespace dawn_native { case wgpu::BindingType::UniformBuffer: case wgpu::BindingType::ReadonlyStorageBuffer: case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: case wgpu::BindingType::SampledTexture: // Don't require barriers. break; diff --git a/src/dawn_native/BindGroupLayout.cpp b/src/dawn_native/BindGroupLayout.cpp index 907462a6c2..d6763c2e2f 100644 --- a/src/dawn_native/BindGroupLayout.cpp +++ b/src/dawn_native/BindGroupLayout.cpp @@ -53,7 +53,6 @@ namespace dawn_native { case wgpu::BindingType::UniformBuffer: case wgpu::BindingType::ReadonlyStorageBuffer: case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: case wgpu::BindingType::SampledTexture: case wgpu::BindingType::ReadonlyStorageTexture: break; @@ -83,7 +82,6 @@ namespace dawn_native { case wgpu::BindingType::UniformBuffer: case wgpu::BindingType::ReadonlyStorageBuffer: case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: case wgpu::BindingType::SampledTexture: break; default: @@ -153,7 +151,6 @@ namespace dawn_native { break; case wgpu::BindingType::SampledTexture: case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: case wgpu::BindingType::ReadonlyStorageTexture: case wgpu::BindingType::WriteonlyStorageTexture: if (binding.hasDynamicOffset) { @@ -251,7 +248,6 @@ namespace dawn_native { break; case wgpu::BindingType::SampledTexture: case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: case wgpu::BindingType::StorageTexture: case wgpu::BindingType::ReadonlyStorageTexture: case wgpu::BindingType::WriteonlyStorageTexture: @@ -324,7 +320,6 @@ namespace dawn_native { break; case wgpu::BindingType::SampledTexture: case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: case wgpu::BindingType::StorageTexture: case wgpu::BindingType::ReadonlyStorageTexture: case wgpu::BindingType::WriteonlyStorageTexture: diff --git a/src/dawn_native/PipelineLayout.cpp b/src/dawn_native/PipelineLayout.cpp index 6dc39c7384..9b6db27ed6 100644 --- a/src/dawn_native/PipelineLayout.cpp +++ b/src/dawn_native/PipelineLayout.cpp @@ -48,7 +48,6 @@ namespace dawn_native { case wgpu::BindingType::UniformBuffer: case wgpu::BindingType::ReadonlyStorageBuffer: case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: case wgpu::BindingType::SampledTexture: case wgpu::BindingType::ReadonlyStorageTexture: return wgpu::ShaderStage::Vertex | wgpu::ShaderStage::Fragment | diff --git a/src/dawn_native/ProgrammablePassEncoder.cpp b/src/dawn_native/ProgrammablePassEncoder.cpp index 1c8ce83e21..5562dcf17f 100644 --- a/src/dawn_native/ProgrammablePassEncoder.cpp +++ b/src/dawn_native/ProgrammablePassEncoder.cpp @@ -60,7 +60,6 @@ namespace dawn_native { } case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: break; case wgpu::BindingType::StorageTexture: diff --git a/src/dawn_native/Sampler.cpp b/src/dawn_native/Sampler.cpp index 748956e60c..034a8c3eab 100644 --- a/src/dawn_native/Sampler.cpp +++ b/src/dawn_native/Sampler.cpp @@ -80,10 +80,6 @@ namespace dawn_native { return new SamplerBase(device, ObjectBase::kError); } - bool SamplerBase::HasCompareFunction() const { - return mCompareFunction != wgpu::CompareFunction::Undefined; - } - size_t SamplerBase::HashFunc::operator()(const SamplerBase* module) const { size_t hash = 0; diff --git a/src/dawn_native/Sampler.h b/src/dawn_native/Sampler.h index 3829b90d3b..a642422e78 100644 --- a/src/dawn_native/Sampler.h +++ b/src/dawn_native/Sampler.h @@ -33,8 +33,6 @@ namespace dawn_native { static SamplerBase* MakeError(DeviceBase* device); - bool HasCompareFunction() const; - // Functors necessary for the unordered_set-based cache. struct HashFunc { size_t operator()(const SamplerBase* module) const; diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp index 41f84d9511..cfea514045 100644 --- a/src/dawn_native/ShaderModule.cpp +++ b/src/dawn_native/ShaderModule.cpp @@ -110,9 +110,10 @@ namespace dawn_native { case shaderc_spvc_binding_type_readonly_storage_buffer: return wgpu::BindingType::ReadonlyStorageBuffer; case shaderc_spvc_binding_type_sampler: - return wgpu::BindingType::Sampler; case shaderc_spvc_binding_type_comparison_sampler: - return wgpu::BindingType::ComparisonSampler; + // TODO: Break out comparison sampler into its own case, once Dawn has seperate + // handling + return wgpu::BindingType::Sampler; case shaderc_spvc_binding_type_sampled_texture: return wgpu::BindingType::SampledTexture; case shaderc_spvc_binding_type_readonly_storage_texture: @@ -772,16 +773,6 @@ namespace dawn_native { bool validBindingConversion = bindingInfo.type == wgpu::BindingType::StorageBuffer && moduleInfo.type == wgpu::BindingType::ReadonlyStorageBuffer; - - // TODO(crbug.com/dawn/367): Temporarily allow using either a sampler or a - // comparison sampler until we can perform the proper shader analysis of what type - // is used in the shader module. - validBindingConversion |= (bindingInfo.type == wgpu::BindingType::Sampler && - moduleInfo.type == wgpu::BindingType::ComparisonSampler); - validBindingConversion |= - (bindingInfo.type == wgpu::BindingType::ComparisonSampler && - moduleInfo.type == wgpu::BindingType::Sampler); - if (!validBindingConversion) { return false; } @@ -820,7 +811,6 @@ namespace dawn_native { case wgpu::BindingType::ReadonlyStorageBuffer: case wgpu::BindingType::StorageBuffer: case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: break; case wgpu::BindingType::StorageTexture: diff --git a/src/dawn_native/d3d12/BindGroupD3D12.cpp b/src/dawn_native/d3d12/BindGroupD3D12.cpp index e4591ac883..ee48625a36 100644 --- a/src/dawn_native/d3d12/BindGroupD3D12.cpp +++ b/src/dawn_native/d3d12/BindGroupD3D12.cpp @@ -123,8 +123,7 @@ namespace dawn_native { namespace d3d12 { viewAllocation.OffsetFrom(viewSizeIncrement, bindingOffsets[bindingIndex])); break; } - case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: { + case wgpu::BindingType::Sampler: { auto* sampler = ToBackend(GetBindingAsSampler(bindingIndex)); auto& samplerDesc = sampler->GetSamplerDescriptor(); d3d12Device->CreateSampler( diff --git a/src/dawn_native/d3d12/BindGroupLayoutD3D12.cpp b/src/dawn_native/d3d12/BindGroupLayoutD3D12.cpp index c5c799942e..b8012d12e4 100644 --- a/src/dawn_native/d3d12/BindGroupLayoutD3D12.cpp +++ b/src/dawn_native/d3d12/BindGroupLayoutD3D12.cpp @@ -34,7 +34,6 @@ namespace dawn_native { namespace d3d12 { case wgpu::BindingType::ReadonlyStorageTexture: return BindGroupLayout::DescriptorType::SRV; case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: return BindGroupLayout::DescriptorType::Sampler; case wgpu::BindingType::StorageTexture: UNREACHABLE(); @@ -117,7 +116,6 @@ namespace dawn_native { namespace d3d12 { break; case wgpu::BindingType::SampledTexture: case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: case wgpu::BindingType::StorageTexture: case wgpu::BindingType::ReadonlyStorageTexture: case wgpu::BindingType::WriteonlyStorageTexture: diff --git a/src/dawn_native/d3d12/CommandBufferD3D12.cpp b/src/dawn_native/d3d12/CommandBufferD3D12.cpp index 14d0f95e2a..088480e6d6 100644 --- a/src/dawn_native/d3d12/CommandBufferD3D12.cpp +++ b/src/dawn_native/d3d12/CommandBufferD3D12.cpp @@ -165,7 +165,6 @@ namespace dawn_native { namespace d3d12 { case wgpu::BindingType::UniformBuffer: case wgpu::BindingType::ReadonlyStorageBuffer: case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: case wgpu::BindingType::SampledTexture: // Don't require barriers. @@ -247,7 +246,6 @@ namespace dawn_native { namespace d3d12 { break; case wgpu::BindingType::SampledTexture: case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: case wgpu::BindingType::StorageTexture: case wgpu::BindingType::ReadonlyStorageTexture: case wgpu::BindingType::WriteonlyStorageTexture: diff --git a/src/dawn_native/d3d12/PipelineLayoutD3D12.cpp b/src/dawn_native/d3d12/PipelineLayoutD3D12.cpp index 1f060a7344..42be90b268 100644 --- a/src/dawn_native/d3d12/PipelineLayoutD3D12.cpp +++ b/src/dawn_native/d3d12/PipelineLayoutD3D12.cpp @@ -50,7 +50,6 @@ namespace dawn_native { namespace d3d12 { return D3D12_ROOT_PARAMETER_TYPE_SRV; case wgpu::BindingType::SampledTexture: case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: case wgpu::BindingType::StorageTexture: case wgpu::BindingType::ReadonlyStorageTexture: case wgpu::BindingType::WriteonlyStorageTexture: diff --git a/src/dawn_native/metal/CommandBufferMTL.mm b/src/dawn_native/metal/CommandBufferMTL.mm index 6fc8c92446..fb42a3ab1a 100644 --- a/src/dawn_native/metal/CommandBufferMTL.mm +++ b/src/dawn_native/metal/CommandBufferMTL.mm @@ -571,8 +571,7 @@ namespace dawn_native { namespace metal { break; } - case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: { + case wgpu::BindingType::Sampler: { auto sampler = ToBackend(group->GetBindingAsSampler(bindingIndex)); if (hasVertStage) { [render setVertexSamplerState:sampler->GetMTLSamplerState() diff --git a/src/dawn_native/metal/PipelineLayoutMTL.mm b/src/dawn_native/metal/PipelineLayoutMTL.mm index 2f5b569cd4..c98d576869 100644 --- a/src/dawn_native/metal/PipelineLayoutMTL.mm +++ b/src/dawn_native/metal/PipelineLayoutMTL.mm @@ -45,7 +45,6 @@ namespace dawn_native { namespace metal { bufferIndex++; break; case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: mIndexInfo[stage][group][bindingIndex] = samplerIndex; samplerIndex++; break; diff --git a/src/dawn_native/opengl/CommandBufferGL.cpp b/src/dawn_native/opengl/CommandBufferGL.cpp index e6ff9f9f82..19e674f4a6 100644 --- a/src/dawn_native/opengl/CommandBufferGL.cpp +++ b/src/dawn_native/opengl/CommandBufferGL.cpp @@ -280,8 +280,7 @@ namespace dawn_native { namespace opengl { break; } - case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: { + case wgpu::BindingType::Sampler: { Sampler* sampler = ToBackend(group->GetBindingAsSampler(bindingIndex)); GLuint samplerIndex = indices[bindingIndex]; diff --git a/src/dawn_native/opengl/PipelineGL.cpp b/src/dawn_native/opengl/PipelineGL.cpp index 926efc5f05..03d2897e02 100644 --- a/src/dawn_native/opengl/PipelineGL.cpp +++ b/src/dawn_native/opengl/PipelineGL.cpp @@ -136,7 +136,6 @@ namespace dawn_native { namespace opengl { } case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: case wgpu::BindingType::SampledTexture: // These binding types are handled in the separate sampler and texture // emulation diff --git a/src/dawn_native/opengl/PipelineLayoutGL.cpp b/src/dawn_native/opengl/PipelineLayoutGL.cpp index 082a25bc4f..383a0796e1 100644 --- a/src/dawn_native/opengl/PipelineLayoutGL.cpp +++ b/src/dawn_native/opengl/PipelineLayoutGL.cpp @@ -38,7 +38,6 @@ namespace dawn_native { namespace opengl { uboIndex++; break; case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: mIndexInfo[group][bindingIndex] = samplerIndex; samplerIndex++; break; diff --git a/src/dawn_native/vulkan/BindGroupLayoutVk.cpp b/src/dawn_native/vulkan/BindGroupLayoutVk.cpp index 11463b56ef..5c64da0590 100644 --- a/src/dawn_native/vulkan/BindGroupLayoutVk.cpp +++ b/src/dawn_native/vulkan/BindGroupLayoutVk.cpp @@ -53,7 +53,6 @@ namespace dawn_native { namespace vulkan { } return VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: return VK_DESCRIPTOR_TYPE_SAMPLER; case wgpu::BindingType::SampledTexture: return VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE; diff --git a/src/dawn_native/vulkan/BindGroupVk.cpp b/src/dawn_native/vulkan/BindGroupVk.cpp index 3ae94cd0a0..018fa05de4 100644 --- a/src/dawn_native/vulkan/BindGroupVk.cpp +++ b/src/dawn_native/vulkan/BindGroupVk.cpp @@ -71,8 +71,7 @@ namespace dawn_native { namespace vulkan { break; } - case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: { + case wgpu::BindingType::Sampler: { Sampler* sampler = ToBackend(GetBindingAsSampler(bindingIndex)); writeImageInfo[numWrites].sampler = sampler->GetHandle(); write.pImageInfo = &writeImageInfo[numWrites]; diff --git a/src/dawn_native/vulkan/CommandBufferVk.cpp b/src/dawn_native/vulkan/CommandBufferVk.cpp index 3542ee94ed..c2c011040a 100644 --- a/src/dawn_native/vulkan/CommandBufferVk.cpp +++ b/src/dawn_native/vulkan/CommandBufferVk.cpp @@ -156,7 +156,6 @@ namespace dawn_native { namespace vulkan { case wgpu::BindingType::UniformBuffer: case wgpu::BindingType::ReadonlyStorageBuffer: case wgpu::BindingType::Sampler: - case wgpu::BindingType::ComparisonSampler: case wgpu::BindingType::SampledTexture: // Don't require barriers. diff --git a/src/tests/end2end/BindGroupTests.cpp b/src/tests/end2end/BindGroupTests.cpp index 1efdb46524..ffd66a8d4f 100644 --- a/src/tests/end2end/BindGroupTests.cpp +++ b/src/tests/end2end/BindGroupTests.cpp @@ -264,13 +264,16 @@ TEST_P(BindGroupTests, UBOSamplerAndTexture) { wgpu::Buffer buffer = utils::CreateBufferFromData(device, &transform, sizeof(transform), wgpu::BufferUsage::Uniform); - wgpu::SamplerDescriptor samplerDescriptor = {}; + wgpu::SamplerDescriptor samplerDescriptor; samplerDescriptor.minFilter = wgpu::FilterMode::Nearest; samplerDescriptor.magFilter = wgpu::FilterMode::Nearest; samplerDescriptor.mipmapFilter = wgpu::FilterMode::Nearest; samplerDescriptor.addressModeU = wgpu::AddressMode::ClampToEdge; samplerDescriptor.addressModeV = wgpu::AddressMode::ClampToEdge; samplerDescriptor.addressModeW = wgpu::AddressMode::ClampToEdge; + samplerDescriptor.lodMinClamp = kLodMin; + samplerDescriptor.lodMaxClamp = kLodMax; + samplerDescriptor.compare = wgpu::CompareFunction::Never; wgpu::Sampler sampler = device.CreateSampler(&samplerDescriptor); diff --git a/src/tests/end2end/SamplerTests.cpp b/src/tests/end2end/SamplerTests.cpp index 157a26619f..82958d52ce 100644 --- a/src/tests/end2end/SamplerTests.cpp +++ b/src/tests/end2end/SamplerTests.cpp @@ -122,13 +122,16 @@ protected: void TestAddressModes(AddressModeTestCase u, AddressModeTestCase v, AddressModeTestCase w) { wgpu::Sampler sampler; { - wgpu::SamplerDescriptor descriptor = {}; + wgpu::SamplerDescriptor descriptor; descriptor.minFilter = wgpu::FilterMode::Nearest; descriptor.magFilter = wgpu::FilterMode::Nearest; descriptor.mipmapFilter = wgpu::FilterMode::Nearest; descriptor.addressModeU = u.mMode; descriptor.addressModeV = v.mMode; descriptor.addressModeW = w.mMode; + descriptor.lodMinClamp = kLodMin; + descriptor.lodMaxClamp = kLodMax; + descriptor.compare = wgpu::CompareFunction::Never; sampler = device.CreateSampler(&descriptor); } diff --git a/src/tests/end2end/TextureViewTests.cpp b/src/tests/end2end/TextureViewTests.cpp index 879794854e..830398849e 100644 --- a/src/tests/end2end/TextureViewTests.cpp +++ b/src/tests/end2end/TextureViewTests.cpp @@ -86,13 +86,16 @@ protected: wgpu::FilterMode kFilterMode = wgpu::FilterMode::Nearest; wgpu::AddressMode kAddressMode = wgpu::AddressMode::ClampToEdge; - wgpu::SamplerDescriptor samplerDescriptor = {}; + wgpu::SamplerDescriptor samplerDescriptor; samplerDescriptor.minFilter = kFilterMode; samplerDescriptor.magFilter = kFilterMode; samplerDescriptor.mipmapFilter = kFilterMode; samplerDescriptor.addressModeU = kAddressMode; samplerDescriptor.addressModeV = kAddressMode; samplerDescriptor.addressModeW = kAddressMode; + samplerDescriptor.lodMinClamp = kLodMin; + samplerDescriptor.lodMaxClamp = kLodMax; + samplerDescriptor.compare = wgpu::CompareFunction::Never; mSampler = device.CreateSampler(&samplerDescriptor); mVSModule = CreateDefaultVertexShaderModule(device); diff --git a/src/tests/unittests/validation/BindGroupValidationTests.cpp b/src/tests/unittests/validation/BindGroupValidationTests.cpp index 05b65275c9..5749621b81 100644 --- a/src/tests/unittests/validation/BindGroupValidationTests.cpp +++ b/src/tests/unittests/validation/BindGroupValidationTests.cpp @@ -1506,126 +1506,3 @@ TEST_F(BindingsValidationTest, BindGroupsWithLessBindingsThanPipelineLayout) { TestComputePassBindings(bg.data(), kBindingNum, computePipeline, false); } - -class ComparisonSamplerBindingTest : public ValidationTest { - protected: - wgpu::RenderPipeline CreateFragmentPipeline(wgpu::BindGroupLayout* bindGroupLayout, - const char* fragmentSource) { - wgpu::ShaderModule vsModule = - utils::CreateShaderModule(device, utils::SingleShaderStage::Vertex, R"( - #version 450 - void main() { - })"); - - wgpu::ShaderModule fsModule = - utils::CreateShaderModule(device, utils::SingleShaderStage::Fragment, fragmentSource); - - utils::ComboRenderPipelineDescriptor pipelineDescriptor(device); - pipelineDescriptor.vertexStage.module = vsModule; - pipelineDescriptor.cFragmentStage.module = fsModule; - wgpu::PipelineLayout pipelineLayout = - utils::MakeBasicPipelineLayout(device, bindGroupLayout); - pipelineDescriptor.layout = pipelineLayout; - return device.CreateRenderPipeline(&pipelineDescriptor); - } -}; - -// TODO(crbug.com/dawn/367): Disabled until we can perform shader analysis -// of which samplers are comparison samplers. -TEST_F(ComparisonSamplerBindingTest, DISABLED_ShaderAndBGLMatches) { - // Test that sampler binding works with normal sampler in the shader. - { - wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::Sampler}}); - - CreateFragmentPipeline(&bindGroupLayout, R"( - #version 450 - layout(set = 0, binding = 0) uniform sampler samp; - - void main() { - })"); - } - - // Test that comparison sampler binding works with shadow sampler in the shader. - { - wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::ComparisonSampler}}); - - CreateFragmentPipeline(&bindGroupLayout, R"( - #version 450 - layout(set = 0, binding = 0) uniform samplerShadow samp; - - void main() { - })"); - } - - // Test that sampler binding does not work with comparison sampler in the shader. - { - wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::Sampler}}); - - ASSERT_DEVICE_ERROR(CreateFragmentPipeline(&bindGroupLayout, R"( - #version 450 - layout(set = 0, binding = 0) uniform samplerShadow samp; - - void main() { - })")); - } - - // Test that comparison sampler binding does not work with normal sampler in the shader. - { - wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::ComparisonSampler}}); - - ASSERT_DEVICE_ERROR(CreateFragmentPipeline(&bindGroupLayout, R"( - #version 450 - layout(set = 0, binding = 0) uniform sampler samp; - - void main() { - })")); - } -} - -TEST_F(ComparisonSamplerBindingTest, SamplerAndBindGroupMatches) { - // Test that sampler binding works with normal sampler. - { - wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::Sampler}}); - - wgpu::SamplerDescriptor desc = {}; - utils::MakeBindGroup(device, bindGroupLayout, {{0, device.CreateSampler(&desc)}}); - } - - // Test that comparison sampler binding works with sampler w/ compare function. - { - wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::ComparisonSampler}}); - - wgpu::SamplerDescriptor desc = { - .compare = wgpu::CompareFunction::Never, - }; - utils::MakeBindGroup(device, bindGroupLayout, {{0, device.CreateSampler(&desc)}}); - } - - // Test that sampler binding does not work with sampler w/ compare function. - { - wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::Sampler}}); - - wgpu::SamplerDescriptor desc = { - .compare = wgpu::CompareFunction::Never, - }; - ASSERT_DEVICE_ERROR( - utils::MakeBindGroup(device, bindGroupLayout, {{0, device.CreateSampler(&desc)}})); - } - - // Test that comparison sampler binding does not work with normal sampler. - { - wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::ComparisonSampler}}); - - wgpu::SamplerDescriptor desc = {}; - ASSERT_DEVICE_ERROR( - utils::MakeBindGroup(device, bindGroupLayout, {{0, device.CreateSampler(&desc)}})); - } -} diff --git a/src/utils/WGPUHelpers.cpp b/src/utils/WGPUHelpers.cpp index 98227d6334..4ab35bbd62 100644 --- a/src/utils/WGPUHelpers.cpp +++ b/src/utils/WGPUHelpers.cpp @@ -244,7 +244,7 @@ namespace utils { } wgpu::SamplerDescriptor GetDefaultSamplerDescriptor() { - wgpu::SamplerDescriptor desc = {}; + wgpu::SamplerDescriptor desc; desc.minFilter = wgpu::FilterMode::Linear; desc.magFilter = wgpu::FilterMode::Linear; @@ -252,6 +252,9 @@ namespace utils { desc.addressModeU = wgpu::AddressMode::Repeat; desc.addressModeV = wgpu::AddressMode::Repeat; desc.addressModeW = wgpu::AddressMode::Repeat; + desc.lodMinClamp = kLodMin; + desc.lodMaxClamp = kLodMax; + desc.compare = wgpu::CompareFunction::Never; return desc; }