Skip to content

Commit

Permalink
Validate blend sources in pipeline and shader endpoint
Browse files Browse the repository at this point in the history
  • Loading branch information
freqmod committed Sep 6, 2023
1 parent 1b2b740 commit dc34c8a
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 3 deletions.
10 changes: 10 additions & 0 deletions wgpu-core/src/device/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2873,6 +2873,16 @@ impl<A: HalApi> Device<A> {
for (i, output) in io.iter() {
match color_targets.get(*i as usize) {
Some(&Some(ref state)) => {
let shader_module = desc.fragment.as_ref().and_then(|fragment| {
shader_module_guard.get(fragment.stage.module).ok()
});
validation::check_target_blend_source(
self,
shader_module,
&desc.fragment,
state,
*i,
)?;
validation::check_texture_format(state.format, &output.ty).map_err(
|pipeline| {
pipeline::CreateRenderPipelineError::ColorState(
Expand Down
9 changes: 9 additions & 0 deletions wgpu-core/src/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,15 @@ pub enum CreateRenderPipelineError {
},
#[error("In the provided shader, the type given for group {group} binding {binding} has a size of {size}. As the device does not support `DownlevelFlags::BUFFER_BINDINGS_NOT_16_BYTE_ALIGNED`, the type must have a size that is a multiple of 16 bytes.")]
UnalignedShader { group: u32, binding: u32, size: u64 },
#[error("Blend factor {factor:?} using S1 target used for invalid render target {target}.")]
AlternateBlendSourceOnUnsupportedTarget {
factor: wgt::BlendFactor,
target: u32,
},
#[error(
"Blend factor {factor:?} refer second blend source, but the shader does not provide it."
)]
NonExistingSecondBlendSourceReferenced { factor: wgt::BlendFactor },
}

bitflags::bitflags! {
Expand Down
66 changes: 65 additions & 1 deletion wgpu-core/src/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ pub struct InterfaceVar {
pub ty: NumericType,
interpolation: Option<naga::Interpolation>,
sampling: Option<naga::Sampling>,
second_blend_source: bool,
}

impl InterfaceVar {
Expand All @@ -80,6 +81,7 @@ impl InterfaceVar {
ty: NumericType::from_vertex_format(format),
interpolation: None,
sampling: None,
second_blend_source: false,
}
}
}
Expand Down Expand Up @@ -762,6 +764,67 @@ pub fn check_texture_format(
}
}

/// Returns an error if the target contains a second blend source and it is not
/// supported by the device or shader endpoint
pub fn check_target_blend_source<'a, A: crate::hal_api::HalApi>(
device: &crate::device::resource::Device<A>,
shader: Option<&crate::pipeline::ShaderModule<A>>,
fragment_state: &Option<crate::pipeline::FragmentState<'a>>,
state: &wgt::ColorTargetState,
target_index: u32,
) -> Result<(), crate::pipeline::CreateRenderPipelineError> {
if let Some(blend_mode) = state.blend {
let mut got_second_blend_source = false;
if device
.features
.contains(wgt::Features::DUAL_SOURCE_BLENDING)
{
if let Some(fragment_state) = fragment_state.as_ref() {
if let Some(interface) = shader.and_then(|shader| shader.interface.as_ref()) {
if let Some(entry_point) = interface.entry_points.get(&(
naga::ShaderStage::Fragment,
String::from(fragment_state.stage.entry_point.as_ref()),
)) {
for output in entry_point.outputs.iter() {
if let &Varying::Local {
ref location,
ref iv,
} = output
{
if *location == 0 && iv.second_blend_source {
got_second_blend_source = true;
break;
}
}
}
}
}
}
}

let check_target =
|factor: wgt::BlendFactor| -> Result<(), crate::pipeline::CreateRenderPipelineError> {
if factor.is_second_blend_source() {
device.require_features(wgt::Features::DUAL_SOURCE_BLENDING)?;
if target_index != 0 {
return Err(crate::pipeline::CreateRenderPipelineError
::AlternateBlendSourceOnUnsupportedTarget { factor, target: target_index });
}
if !got_second_blend_source {
return Err(crate::pipeline::CreateRenderPipelineError
::NonExistingSecondBlendSourceReferenced { factor });
}
}
Ok(())
};
check_target(blend_mode.color.src_factor)?;
check_target(blend_mode.color.dst_factor)?;
check_target(blend_mode.alpha.src_factor)?;
check_target(blend_mode.alpha.dst_factor)?;
}

Ok(())
}
pub type StageIo = FastHashMap<wgt::ShaderLocation, InterfaceVar>;

impl Interface {
Expand Down Expand Up @@ -812,13 +875,14 @@ impl Interface {
location,
interpolation,
sampling,
second_blend_source: _,
second_blend_source,
}) => Varying::Local {
location,
iv: InterfaceVar {
ty: numeric_ty,
interpolation,
sampling,
second_blend_source,
},
},
Some(&naga::Binding::BuiltIn(built_in)) => Varying::BuiltIn(built_in),
Expand Down
5 changes: 4 additions & 1 deletion wgpu-hal/src/metal/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -808,7 +808,10 @@ impl super::PrivateCapabilities {
| F::DEPTH32FLOAT_STENCIL8
| F::MULTI_DRAW_INDIRECT;

features.set(F::DUAL_SOURCE_BLENDING, self.msl_version >= MTLLanguageVersion::V1_2);
features.set(
F::DUAL_SOURCE_BLENDING,
self.msl_version >= MTLLanguageVersion::V1_2,
);
features.set(F::TIMESTAMP_QUERY, self.support_timestamp_query);
// TODO: Not yet implemented.
// features.set(
Expand Down
17 changes: 16 additions & 1 deletion wgpu-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1562,7 +1562,8 @@ impl TextureViewDimension {
///
/// Corresponds to [WebGPU `GPUBlendFactor`](
/// https://gpuweb.github.io/gpuweb/#enumdef-gpublendfactor).
/// Values using S1 requires [`Features::DUAL_SOURCE_BLENDING`].
/// Values using S1 requires [`Features::DUAL_SOURCE_BLENDING`] and can only be
/// used with the first render target.
#[repr(C)]
#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)]
#[cfg_attr(feature = "trace", derive(Serialize))]
Expand Down Expand Up @@ -1605,6 +1606,20 @@ pub enum BlendFactor {
OneMinusSrc1Alpha = 16,
}

impl BlendFactor {
/** Distinguish if the blend source is refering to an alternate buffer (S1)
and require [`Features::DUAL_SOURCE_BLENDING`]. */
pub fn is_second_blend_source(&self) -> bool {
match self {
BlendFactor::Src1
| BlendFactor::OneMinusSrc1
| BlendFactor::Src1Alpha
| BlendFactor::OneMinusSrc1Alpha => true,
_ => false,
}
}
}

/// Alpha blend operation.
///
/// Alpha blending is very complicated: see the OpenGL or Vulkan spec for more information.
Expand Down

0 comments on commit dc34c8a

Please sign in to comment.