Introduce SamplerBindingType enum and fix bug in validation (#2181)

* Introduce SamplerBindingType enum and fix bug in validation

This matches the WebGPU spec more closely and also lets us implement the
validation as it's written in the spec[1]. This fixes a bug which
previously prevented the "shadow" example from running in Firefox.

Previously the validation would check whether the filtering was
appropiate even if it was a comparison sampler. This didn't matter when
running the shadow example natively since the example was setting
`filtering: true` in addition to `comparison: true`. But this failed
when running through a browser since there the used WebIDL has the
proper enum representation and eventually resulted in `filtering: false`
being passed to wgpu-core which would then fail validation.[2]

[1]: https://gpuweb.github.io/gpuweb/#bind-group-creation
[2]: 674b6582ba/dom/webgpu/ipc/WebGPUChild.cpp (L502-L513)

* Fix remaining examples

* Fix deno_webgpu
This commit is contained in:
Boris-Chengbiao Zhou
2021-11-17 05:08:03 +01:00
committed by GitHub
parent 1ff70329f1
commit 3545a4fdd5
13 changed files with 80 additions and 109 deletions

View File

@@ -56,34 +56,7 @@ impl From<GpuBufferBindingType> for wgpu_types::BufferBindingType {
#[derive(Deserialize)]
#[serde(rename_all = "camelCase")]
struct GpuSamplerBindingLayout {
r#type: GpuSamplerBindingType,
}
#[derive(Deserialize)]
#[serde(rename_all = "kebab-case")]
enum GpuSamplerBindingType {
Filtering,
NonFiltering,
Comparison,
}
impl From<GpuSamplerBindingType> for wgpu_types::BindingType {
fn from(binding_type: GpuSamplerBindingType) -> Self {
match binding_type {
GpuSamplerBindingType::Filtering => wgpu_types::BindingType::Sampler {
filtering: true,
comparison: false,
},
GpuSamplerBindingType::NonFiltering => wgpu_types::BindingType::Sampler {
filtering: false,
comparison: false,
},
GpuSamplerBindingType::Comparison => wgpu_types::BindingType::Sampler {
filtering: true,
comparison: true,
},
}
}
r#type: wgpu_types::SamplerBindingType,
}
#[derive(Deserialize)]
@@ -170,7 +143,7 @@ impl TryFrom<GpuBindingType> for wgpu_types::BindingType {
has_dynamic_offset: buffer.has_dynamic_offset,
min_binding_size: std::num::NonZeroU64::new(buffer.min_binding_size),
},
GpuBindingType::Sampler(sampler) => sampler.r#type.into(),
GpuBindingType::Sampler(sampler) => wgpu_types::BindingType::Sampler(sampler.r#type),
GpuBindingType::Texture(texture) => wgpu_types::BindingType::Texture {
sample_type: texture.sample_type.into(),
view_dimension: texture.view_dimension,

View File

@@ -1521,31 +1521,36 @@ impl<A: HalApi> Device<A> {
}
Br::Sampler(id) => {
match decl.ty {
wgt::BindingType::Sampler {
filtering,
comparison,
} => {
wgt::BindingType::Sampler(ty) => {
let sampler = used
.samplers
.use_extend(&*sampler_guard, id, (), ())
.map_err(|_| Error::InvalidSampler(id))?;
// Check the actual sampler to also (not) be a comparison sampler
if sampler.comparison != comparison {
// Allowed sampler values for filtering and comparison
let (allowed_filtering, allowed_comparison) = match ty {
wgt::SamplerBindingType::Filtering => (None, false),
wgt::SamplerBindingType::NonFiltering => (Some(false), false),
wgt::SamplerBindingType::Comparison => (None, true),
};
if let Some(allowed_filtering) = allowed_filtering {
if allowed_filtering != sampler.filtering {
return Err(Error::WrongSamplerFiltering {
binding,
layout_flt: allowed_filtering,
sampler_flt: sampler.filtering,
});
}
}
if allowed_comparison != sampler.comparison {
return Err(Error::WrongSamplerComparison {
binding,
layout_cmp: comparison,
layout_cmp: allowed_comparison,
sampler_cmp: sampler.comparison,
});
}
// Check the actual sampler to be non-filtering, if required
if sampler.filtering && !filtering {
return Err(Error::WrongSamplerFiltering {
binding,
layout_flt: filtering,
sampler_flt: sampler.filtering,
});
}
let res_index = hal_samplers.len();
hal_samplers.push(&sampler.raw);

View File

@@ -391,11 +391,8 @@ impl Resource {
allowed_usage
}
ResourceType::Sampler { comparison } => match entry.ty {
BindingType::Sampler {
filtering: _,
comparison: cmp,
} => {
if cmp == comparison {
BindingType::Sampler(ty) => {
if (ty == wgt::SamplerBindingType::Comparison) == comparison {
GlobalUse::READ
} else {
return Err(BindingError::WrongSamplerComparison);
@@ -536,10 +533,11 @@ impl Resource {
has_dynamic_offset: false,
min_binding_size: Some(size),
},
ResourceType::Sampler { comparison } => BindingType::Sampler {
filtering: true,
comparison,
},
ResourceType::Sampler { comparison } => BindingType::Sampler(if comparison {
wgt::SamplerBindingType::Comparison
} else {
wgt::SamplerBindingType::Filtering
}),
ResourceType::Texture {
dim,
arrayed,
@@ -1031,9 +1029,11 @@ impl Interface {
sample_type: wgt::TextureSampleType::Float { filterable },
..
} => match sampler_layout.ty {
wgt::BindingType::Sampler {
filtering: true, ..
} if !filterable => Some(FilteringError::NonFilterable),
wgt::BindingType::Sampler(wgt::SamplerBindingType::Filtering)
if !filterable =>
{
Some(FilteringError::NonFilterable)
}
_ => None,
},
wgt::BindingType::Texture {

View File

@@ -177,10 +177,7 @@ impl<A: hal::Api> Example<A> {
wgt::BindGroupLayoutEntry {
binding: 2,
visibility: wgt::ShaderStages::FRAGMENT,
ty: wgt::BindingType::Sampler {
filtering: true,
comparison: false,
},
ty: wgt::BindingType::Sampler(wgt::SamplerBindingType::Filtering),
count: None,
},
],

View File

@@ -3194,6 +3194,25 @@ pub enum StorageTextureAccess {
ReadWrite,
}
/// Specific type of a sampler binding.
///
/// WebGPU spec: <https://gpuweb.github.io/gpuweb/#enumdef-gpusamplerbindingtype>
#[repr(C)]
#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
#[cfg_attr(feature = "trace", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
#[cfg_attr(feature = "serde", serde(rename_all = "kebab-case"))]
pub enum SamplerBindingType {
/// The sampling result is produced based on more than a single color sample from a texture,
/// e.g. when bilinear interpolation is enabled.
Filtering,
/// The sampling result is produced based on a single color sample from a texture.
NonFiltering,
/// Use as a comparison sampler instead of a normal sampler.
/// For more info take a look at the analogous functionality in OpenGL: <https://www.khronos.org/opengl/wiki/Sampler_Object#Comparison_mode>.
Comparison,
}
/// Specific type of a binding.
///
/// WebGPU spec: the enum of
@@ -3227,16 +3246,7 @@ pub enum BindingType {
/// layout(binding = 0)
/// uniform sampler s;
/// ```
Sampler {
/// The sampling result is produced based on more than a single color sample from a texture,
/// e.g. when bilinear interpolation is enabled.
///
/// A filtering sampler can only be used with a filterable texture.
filtering: bool,
/// Use as a comparison sampler instead of a normal sampler.
/// For more info take a look at the analogous functionality in OpenGL: <https://www.khronos.org/opengl/wiki/Sampler_Object#Comparison_mode>.
comparison: bool,
},
Sampler(SamplerBindingType),
/// A texture binding.
///
/// Example GLSL syntax:

View File

@@ -77,10 +77,7 @@ impl framework::Example for Example {
wgpu::BindGroupLayoutEntry {
binding: 2,
visibility: wgpu::ShaderStages::FRAGMENT,
ty: wgpu::BindingType::Sampler {
filtering: true,
comparison: false,
},
ty: wgpu::BindingType::Sampler(wgpu::SamplerBindingType::Filtering),
count: None,
},
],

View File

@@ -183,10 +183,7 @@ impl framework::Example for Example {
wgpu::BindGroupLayoutEntry {
binding: 1,
visibility: wgpu::ShaderStages::FRAGMENT,
ty: wgpu::BindingType::Sampler {
filtering: false,
comparison: false,
},
ty: wgpu::BindingType::Sampler(wgpu::SamplerBindingType::NonFiltering),
count: None,
},
],

View File

@@ -580,10 +580,7 @@ impl framework::Example for Example {
wgpu::BindGroupLayoutEntry {
binding: 3,
visibility: wgpu::ShaderStages::FRAGMENT,
ty: wgpu::BindingType::Sampler {
comparison: true,
filtering: true,
},
ty: wgpu::BindingType::Sampler(wgpu::SamplerBindingType::Comparison),
count: None,
},
],

View File

@@ -166,10 +166,7 @@ impl framework::Example for Skybox {
wgpu::BindGroupLayoutEntry {
binding: 2,
visibility: wgpu::ShaderStages::FRAGMENT,
ty: wgpu::BindingType::Sampler {
comparison: false,
filtering: true,
},
ty: wgpu::BindingType::Sampler(wgpu::SamplerBindingType::Filtering),
count: None,
},
],

View File

@@ -188,10 +188,7 @@ impl framework::Example for Example {
wgpu::BindGroupLayoutEntry {
binding: 1,
visibility: wgpu::ShaderStages::FRAGMENT,
ty: wgpu::BindingType::Sampler {
comparison: false,
filtering: true,
},
ty: wgpu::BindingType::Sampler(wgpu::SamplerBindingType::Filtering),
count: None,
},
],

View File

@@ -391,10 +391,7 @@ impl framework::Example for Example {
wgpu::BindGroupLayoutEntry {
binding: 3,
visibility: wgpu::ShaderStages::FRAGMENT,
ty: wgpu::BindingType::Sampler {
comparison: false,
filtering: true,
},
ty: wgpu::BindingType::Sampler(wgpu::SamplerBindingType::Filtering),
count: None,
},
],

View File

@@ -1283,15 +1283,18 @@ impl crate::Context for Context {
});
mapped_entry.buffer(&buffer);
}
wgt::BindingType::Sampler {
comparison,
filtering,
} => {
wgt::BindingType::Sampler(ty) => {
let mut sampler = web_sys::GpuSamplerBindingLayout::new();
sampler.type_(match (comparison, filtering) {
(false, false) => web_sys::GpuSamplerBindingType::NonFiltering,
(false, true) => web_sys::GpuSamplerBindingType::Filtering,
(true, _) => web_sys::GpuSamplerBindingType::Comparison,
sampler.type_(match ty {
wgt::SamplerBindingType::NonFiltering => {
web_sys::GpuSamplerBindingType::NonFiltering
}
wgt::SamplerBindingType::Filtering => {
web_sys::GpuSamplerBindingType::Filtering
}
wgt::SamplerBindingType::Comparison => {
web_sys::GpuSamplerBindingType::Comparison
}
});
mapped_entry.sampler(&sampler);
}

View File

@@ -32,13 +32,14 @@ pub use wgt::{
DynamicOffset, Extent3d, Face, Features, FilterMode, FrontFace, ImageDataLayout,
ImageSubresourceRange, IndexFormat, Limits, MultisampleState, Origin3d,
PipelineStatisticsTypes, PolygonMode, PowerPreference, PresentMode, PrimitiveState,
PrimitiveTopology, PushConstantRange, QueryType, RenderBundleDepthStencil, SamplerBorderColor,
ShaderLocation, ShaderModel, ShaderStages, StencilFaceState, StencilOperation, StencilState,
StorageTextureAccess, SurfaceConfiguration, SurfaceStatus, TextureAspect, TextureDimension,
TextureFormat, TextureFormatFeatureFlags, TextureFormatFeatures, TextureSampleType,
TextureUsages, TextureViewDimension, VertexAttribute, VertexFormat, VertexStepMode,
COPY_BUFFER_ALIGNMENT, COPY_BYTES_PER_ROW_ALIGNMENT, MAP_ALIGNMENT, PUSH_CONSTANT_ALIGNMENT,
QUERY_RESOLVE_BUFFER_ALIGNMENT, QUERY_SET_MAX_QUERIES, QUERY_SIZE, VERTEX_STRIDE_ALIGNMENT,
PrimitiveTopology, PushConstantRange, QueryType, RenderBundleDepthStencil, SamplerBindingType,
SamplerBorderColor, ShaderLocation, ShaderModel, ShaderStages, StencilFaceState,
StencilOperation, StencilState, StorageTextureAccess, SurfaceConfiguration, SurfaceStatus,
TextureAspect, TextureDimension, TextureFormat, TextureFormatFeatureFlags,
TextureFormatFeatures, TextureSampleType, TextureUsages, TextureViewDimension, VertexAttribute,
VertexFormat, VertexStepMode, COPY_BUFFER_ALIGNMENT, COPY_BYTES_PER_ROW_ALIGNMENT,
MAP_ALIGNMENT, PUSH_CONSTANT_ALIGNMENT, QUERY_RESOLVE_BUFFER_ALIGNMENT, QUERY_SET_MAX_QUERIES,
QUERY_SIZE, VERTEX_STRIDE_ALIGNMENT,
};
use backend::{BufferMappedRange, Context as C};