Reland "Add ComparisonSampler binding type and validation tests"

This is a reland of 6d9e4f8076
Now that the Chromium-side API change has landed, this CL can land.

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>

Bug: dawn:367
TBR=cwallez@chromium.org,kainino@chromium.org
Change-Id: I325d096e7ce092d17833429c3f54ef7c71189739
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/20045
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
This commit is contained in:
Austin Eng 2020-04-20 23:43:20 +00:00 committed by Commit Bot service account
parent 7e8385c183
commit 7817a9aafe
26 changed files with 199 additions and 30 deletions

View File

@ -115,10 +115,11 @@
{"value": 1, "name": "storage buffer"},
{"value": 2, "name": "readonly storage buffer"},
{"value": 3, "name": "sampler"},
{"value": 4, "name": "sampled texture"},
{"value": 5, "name": "storage texture"},
{"value": 6, "name": "readonly storage texture"},
{"value": 7, "name": "writeonly storage texture"}
{"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"}
]
},
"blend descriptor": {

View File

@ -117,13 +117,30 @@ namespace dawn_native {
return {};
}
MaybeError ValidateSamplerBinding(const DeviceBase* device, const BindGroupEntry& binding) {
MaybeError ValidateSamplerBinding(const DeviceBase* device,
const BindGroupEntry& binding,
wgpu::BindingType bindingType) {
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 {};
}
@ -174,7 +191,8 @@ namespace dawn_native {
bindingInfo));
break;
case wgpu::BindingType::Sampler:
DAWN_TRY(ValidateSamplerBinding(device, binding));
case wgpu::BindingType::ComparisonSampler:
DAWN_TRY(ValidateSamplerBinding(device, binding, bindingInfo.type));
break;
// TODO(jiawei.shao@intel.com): support creating bind group with read-only and
// write-only storage textures.
@ -294,7 +312,8 @@ namespace dawn_native {
SamplerBase* BindGroupBase::GetBindingAsSampler(BindingIndex bindingIndex) {
ASSERT(!IsError());
ASSERT(bindingIndex < mLayout->GetBindingCount());
ASSERT(mLayout->GetBindingInfo(bindingIndex).type == wgpu::BindingType::Sampler);
ASSERT(mLayout->GetBindingInfo(bindingIndex).type == wgpu::BindingType::Sampler ||
mLayout->GetBindingInfo(bindingIndex).type == wgpu::BindingType::ComparisonSampler);
return static_cast<SamplerBase*>(mBindingData.bindings[bindingIndex].Get());
}

View File

@ -52,6 +52,7 @@ 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,6 +53,7 @@ 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;
@ -82,6 +83,7 @@ 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:
@ -151,6 +153,7 @@ 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) {
@ -248,6 +251,7 @@ 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:
@ -320,6 +324,7 @@ 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,6 +48,7 @@ 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,6 +60,7 @@ namespace dawn_native {
}
case wgpu::BindingType::Sampler:
case wgpu::BindingType::ComparisonSampler:
break;
case wgpu::BindingType::StorageTexture:

View File

@ -80,6 +80,10 @@ 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,6 +33,8 @@ 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,10 +110,9 @@ namespace dawn_native {
case shaderc_spvc_binding_type_readonly_storage_buffer:
return wgpu::BindingType::ReadonlyStorageBuffer;
case shaderc_spvc_binding_type_sampler:
case shaderc_spvc_binding_type_comparison_sampler:
// TODO: Break out comparison sampler into its own case, once Dawn has seperate
// handling
return wgpu::BindingType::Sampler;
case shaderc_spvc_binding_type_comparison_sampler:
return wgpu::BindingType::ComparisonSampler;
case shaderc_spvc_binding_type_sampled_texture:
return wgpu::BindingType::SampledTexture;
case shaderc_spvc_binding_type_readonly_storage_texture:
@ -773,6 +772,16 @@ 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;
}
@ -811,6 +820,7 @@ 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,7 +123,8 @@ namespace dawn_native { namespace d3d12 {
viewAllocation.OffsetFrom(viewSizeIncrement, bindingOffsets[bindingIndex]));
break;
}
case wgpu::BindingType::Sampler: {
case wgpu::BindingType::Sampler:
case wgpu::BindingType::ComparisonSampler: {
auto* sampler = ToBackend(GetBindingAsSampler(bindingIndex));
auto& samplerDesc = sampler->GetSamplerDescriptor();
d3d12Device->CreateSampler(

View File

@ -34,6 +34,7 @@ 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();
@ -116,6 +117,7 @@ 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,6 +165,7 @@ 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.
@ -246,6 +247,7 @@ 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,6 +50,7 @@ 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,7 +571,8 @@ namespace dawn_native { namespace metal {
break;
}
case wgpu::BindingType::Sampler: {
case wgpu::BindingType::Sampler:
case wgpu::BindingType::ComparisonSampler: {
auto sampler = ToBackend(group->GetBindingAsSampler(bindingIndex));
if (hasVertStage) {
[render setVertexSamplerState:sampler->GetMTLSamplerState()

View File

@ -45,6 +45,7 @@ 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,7 +280,8 @@ namespace dawn_native { namespace opengl {
break;
}
case wgpu::BindingType::Sampler: {
case wgpu::BindingType::Sampler:
case wgpu::BindingType::ComparisonSampler: {
Sampler* sampler = ToBackend(group->GetBindingAsSampler(bindingIndex));
GLuint samplerIndex = indices[bindingIndex];

View File

@ -136,6 +136,7 @@ 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,6 +38,7 @@ namespace dawn_native { namespace opengl {
uboIndex++;
break;
case wgpu::BindingType::Sampler:
case wgpu::BindingType::ComparisonSampler:
mIndexInfo[group][bindingIndex] = samplerIndex;
samplerIndex++;
break;

View File

@ -53,6 +53,7 @@ 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,7 +71,8 @@ namespace dawn_native { namespace vulkan {
break;
}
case wgpu::BindingType::Sampler: {
case wgpu::BindingType::Sampler:
case wgpu::BindingType::ComparisonSampler: {
Sampler* sampler = ToBackend(GetBindingAsSampler(bindingIndex));
writeImageInfo[numWrites].sampler = sampler->GetHandle();
write.pImageInfo = &writeImageInfo[numWrites];

View File

@ -156,6 +156,7 @@ 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,16 +264,13 @@ 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,16 +122,13 @@ 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,16 +86,13 @@ 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,3 +1506,126 @@ 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,9 +252,6 @@ 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;
}