encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Allowed mismatch sampler/comparison sampler when using SPIRV-Cross 

When SPIRV-Cross we always reflect samplers as regular samplers. A
recent change implementing proper validation broke programs using
comparison samplers in that path.

Skip that bit of validation when using SPIRV-Cross, that hack can be
removed once we use Tint unconditionally for reflection.

Also reenables some BGL-ShaderModule compatibility tests with
SPIRV-Cross to cover the "fixed" validation.

Bug: dawn:571
Bug: dawn:367
Change-Id: I2d1a65aaea717b4927ac4e838942547a1b413d33
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/56900
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
This commit is contained in:
Corentin Wallez 2021-07-05 15:07:49 +00:00 committed by Dawn LUCI CQ
parent 3be120026d
commit 7ae959da60
2 changed files with 12 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/dawn_native/ShaderModule.cpp
View File

@ -484,7 +484,7 @@ namespace dawn_native {
return std::move(result);
}
MaybeError ValidateCompatibilityWithBindGroupLayout(DeviceBase*,
MaybeError ValidateCompatibilityWithBindGroupLayout(DeviceBase* device,
BindGroupIndex group,
const EntryPointMetadata& entryPoint,
const BindGroupLayoutBase* layout) {
@ -622,6 +622,13 @@ namespace dawn_native {
}
case BindingInfoType::Sampler:
// Allow mismatched samplers when using SPIRV-Cross since we can't reflect
// data that's precise enough.
// TODO(dawn:571): Remove once we use Tint unconditionnally for reflection.
if (!device->IsToggleEnabled(Toggle::UseTintGenerator)) {
break;
}
if ((layoutInfo.sampler.type == wgpu::SamplerBindingType::Comparison) !=
shaderInfo.sampler.isComparison) {
return DAWN_VALIDATION_ERROR(

11
src/tests/unittests/validation/BindGroupValidationTests.cpp
View File

@ -2359,9 +2359,6 @@ class SamplerTypeBindingTest : public ValidationTest {
// Test that the use of sampler and comparison_sampler in the shader must match the bind group
// layout.
TEST_F(SamplerTypeBindingTest, ShaderAndBGLMatches) {
// Tint needed for proper shader reflection.
DAWN_SKIP_TEST_IF(!HasToggleEnabled("use_tint_generator"));
// Test that a filtering sampler binding works with normal sampler in the shader.
{
wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout(
@ -2399,7 +2396,7 @@ TEST_F(SamplerTypeBindingTest, ShaderAndBGLMatches) {
}
// Test that filtering sampler binding does not work with comparison sampler in the shader.
{
if (HasToggleEnabled("use_tint_generator")) {
wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout(
device, {{0, wgpu::ShaderStage::Fragment, wgpu::SamplerBindingType::Filtering}});
@ -2411,7 +2408,7 @@ TEST_F(SamplerTypeBindingTest, ShaderAndBGLMatches) {
}
// Test that non-filtering sampler binding does not work with comparison sampler in the shader.
{
if (HasToggleEnabled("use_tint_generator")) {
wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout(
device, {{0, wgpu::ShaderStage::Fragment, wgpu::SamplerBindingType::NonFiltering}});
@ -2423,7 +2420,7 @@ TEST_F(SamplerTypeBindingTest, ShaderAndBGLMatches) {
}
// Test that comparison sampler binding does not work with normal sampler in the shader.
{
if (HasToggleEnabled("use_tint_generator")) {
wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout(
device, {{0, wgpu::ShaderStage::Fragment, wgpu::SamplerBindingType::Comparison}});
@ -2505,7 +2502,7 @@ TEST_F(SamplerTypeBindingTest, ShaderAndBGLMatches) {
}
// Test that a filtering sampler cannot be used to sample an unfilterable-float texture.
{
if (HasToggleEnabled("use_tint_generator")) {
wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout(
device, {{0, wgpu::ShaderStage::Fragment, wgpu::SamplerBindingType::Filtering},
{1, wgpu::ShaderStage::Fragment, wgpu::TextureSampleType::UnfilterableFloat}});