Revert "Add ComparisonSampler binding type and validation tests"

This reverts commit 6d9e4f8076.

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 <enga@chromium.org>
> Commit-Queue: Austin Eng <enga@chromium.org>

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 <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
This commit is contained in:
Corentin Wallez 2020-04-20 17:21:52 +00:00 committed by Commit Bot service account
parent 24730cde43
commit 8edb723dea
26 changed files with 30 additions and 199 deletions

View File

@ -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": {

View File

@ -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<SamplerBase*>(mBindingData.bindings[bindingIndex].Get());
}

View File

@ -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;

View File

@ -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:

View File

@ -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 |

View File

@ -60,7 +60,6 @@ namespace dawn_native {
}
case wgpu::BindingType::Sampler:
case wgpu::BindingType::ComparisonSampler:
break;
case wgpu::BindingType::StorageTexture:

View File

@ -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;

View File

@ -33,8 +33,6 @@ namespace dawn_native {
static SamplerBase* MakeError(DeviceBase* device);
bool HasCompareFunction() const;
// Functors necessary for the unordered_set<SamplerBase*>-based cache.
struct HashFunc {
size_t operator()(const SamplerBase* module) const;

View File

@ -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:

View File

@ -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(

View File

@ -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:

View File

@ -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:

View File

@ -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:

View File

@ -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()

View File

@ -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;

View File

@ -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];

View File

@ -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

View File

@ -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;

View File

@ -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;

View File

@ -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];

View File

@ -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.

View File

@ -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);

View File

@ -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);
}

View File

@ -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);

View File

@ -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)}}));
}
}

View File

@ -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;
}