From d5732d7c70fde23bee1525af915c4b36db7e00dd Mon Sep 17 00:00:00 2001 From: Frederik Vestre Date: Thu, 3 Aug 2023 18:34:35 +0200 Subject: [PATCH 1/6] Support alternate color blending source --- CHANGELOG.md | 1 + wgpu-core/src/device/resource.rs | 32 ++++++++++++++++++++++++++ wgpu-core/src/pipeline.rs | 9 ++++++++ wgpu-core/src/validation.rs | 9 +++++++- wgpu-hal/src/dx12/adapter.rs | 4 +++- wgpu-hal/src/dx12/conv.rs | 12 +++++----- wgpu-hal/src/gles/adapter.rs | 4 ++++ wgpu-hal/src/gles/conv.rs | 4 ++++ wgpu-hal/src/metal/adapter.rs | 5 ++++ wgpu-hal/src/metal/conv.rs | 8 +++---- wgpu-hal/src/vulkan/adapter.rs | 2 ++ wgpu-hal/src/vulkan/conv.rs | 4 ++++ wgpu-types/src/lib.rs | 39 +++++++++++++++++++++++++++++++- wgpu/src/backend/web.rs | 9 ++++++++ 14 files changed, 129 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0eb15b5ae..0d4ea4e9fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,7 @@ By @Valaphee in [#3402](https://github.com/gfx-rs/wgpu/pull/3402) - Add validation in accordance with WebGPU `setViewport` valid usage for `x`, `y` and `this.[[attachment_size]]`. By @James2022-rgb in [#4058](https://github.com/gfx-rs/wgpu/pull/4058) - `wgpu::CreateSurfaceError` and `wgpu::RequestDeviceError` now give details of the failure, but no longer implement `PartialEq` and cannot be constructed. By @kpreid in [#4066](https://github.com/gfx-rs/wgpu/pull/4066) and [#4145](https://github.com/gfx-rs/wgpu/pull/4145) - Make `WGPU_POWER_PREF=none` a valid value. By @fornwall in [4076](https://github.com/gfx-rs/wgpu/pull/4076) +- Support dual source blending in OpenGL ES, Metal, Vulkan & DX12. By @freqmod in [4022](https://github.com/gfx-rs/wgpu/pull/4022) #### Vulkan diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index 73f1887e10..97567f781e 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -1276,6 +1276,10 @@ impl Device { .flags .contains(wgt::DownlevelFlags::MULTISAMPLED_SHADING), ); + caps.set( + Caps::DUAL_SOURCE_BLENDING, + self.features.contains(wgt::Features::DUAL_SOURCE_BLENDING), + ); let info = naga::valid::Validator::new(naga::valid::ValidationFlags::all(), caps) .validate(&module) @@ -2560,6 +2564,7 @@ impl Device { let mut vertex_steps = Vec::with_capacity(desc.vertex.buffers.len()); let mut vertex_buffers = Vec::with_capacity(desc.vertex.buffers.len()); let mut total_attributes = 0; + let mut pipeline_expects_dual_source_blending = false; for (i, vb_state) in desc.vertex.buffers.iter().enumerate() { vertex_steps.push(pipeline::VertexStep { stride: vb_state.array_stride, @@ -2701,6 +2706,25 @@ impl Device { break Some(pipeline::ColorStateError::FormatNotMultisampled(cs.format)); } + if let Some(blend_mode) = cs.blend { + for factor in [ + blend_mode.color.src_factor, + blend_mode.color.dst_factor, + blend_mode.alpha.src_factor, + blend_mode.alpha.dst_factor, + ] { + if factor.ref_second_blend_source() { + if i == 0 { + self.require_features(wgt::Features::DUAL_SOURCE_BLENDING)?; + pipeline_expects_dual_source_blending = true; + break; + } else { + return Err(crate::pipeline::CreateRenderPipelineError + ::BlendFactorOnUnsupportedTarget { factor, target: i as u32 }); + } + } + } + } break None; }; if let Some(e) = error { @@ -2807,6 +2831,14 @@ impl Device { error, })?; validated_stages |= flag; + + let dual_source = interface.has_dual_source_blending_entry_point(); + if !pipeline_expects_dual_source_blending && dual_source { + return Err(pipeline::CreateRenderPipelineError::ShaderExpectsPipelineToUseDualSourceBlending); + } + if pipeline_expects_dual_source_blending && !dual_source { + return Err(pipeline::CreateRenderPipelineError::PipelineExpectsShaderToUseDualSourceBlending); + } } hal::ProgrammableStage { diff --git a/wgpu-core/src/pipeline.rs b/wgpu-core/src/pipeline.rs index da06b652ea..c78a79820d 100644 --- a/wgpu-core/src/pipeline.rs +++ b/wgpu-core/src/pipeline.rs @@ -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("Using the blend factor {factor:?} for render target {target} is not possible. Only the first render target may be used when dual-source blending.")] + BlendFactorOnUnsupportedTarget { + factor: wgt::BlendFactor, + target: u32, + }, + #[error("Pipeline expects the shader entry point to make use of dual-source blending.")] + PipelineExpectsShaderToUseDualSourceBlending, + #[error("Shader entry point expects the pipeline to make use of dual-source blending.")] + ShaderExpectsPipelineToUseDualSourceBlending, } bitflags::bitflags! { diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index e3ecb916d3..bd03455b34 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -116,6 +116,7 @@ struct EntryPoint { spec_constants: Vec, sampling_pairs: FastHashSet<(naga::Handle, naga::Handle)>, workgroup_size: [u32; 3], + dual_source_blending: bool, } #[derive(Debug)] @@ -903,7 +904,7 @@ impl Interface { ep.sampling_pairs .insert((resource_mapping[&key.image], resource_mapping[&key.sampler])); } - + ep.dual_source_blending = info.dual_source_blending; ep.workgroup_size = entry_point.workgroup_size; entry_points.insert((entry_point.stage, entry_point.name.clone()), ep); @@ -1177,4 +1178,10 @@ impl Interface { .collect(); Ok(outputs) } + + pub fn has_dual_source_blending_entry_point(&self) -> bool { + self.entry_points + .values() + .any(|point| point.dual_source_blending) + } } diff --git a/wgpu-hal/src/dx12/adapter.rs b/wgpu-hal/src/dx12/adapter.rs index 02cde913ca..564cf8663a 100644 --- a/wgpu-hal/src/dx12/adapter.rs +++ b/wgpu-hal/src/dx12/adapter.rs @@ -250,7 +250,9 @@ impl super::Adapter { | wgt::Features::TEXTURE_FORMAT_16BIT_NORM | wgt::Features::PUSH_CONSTANTS | wgt::Features::SHADER_PRIMITIVE_INDEX - | wgt::Features::RG11B10UFLOAT_RENDERABLE; + | wgt::Features::RG11B10UFLOAT_RENDERABLE + | wgt::Features::BLEND_FUNC_EXTENDED; + //TODO: in order to expose this, we need to run a compute shader // that extract the necessary statistics out of the D3D12 result. // Alternatively, we could allocate a buffer for the query set, diff --git a/wgpu-hal/src/dx12/conv.rs b/wgpu-hal/src/dx12/conv.rs index 908944567a..f484d1a9e2 100644 --- a/wgpu-hal/src/dx12/conv.rs +++ b/wgpu-hal/src/dx12/conv.rs @@ -246,12 +246,12 @@ fn map_blend_factor(factor: wgt::BlendFactor, is_alpha: bool) -> d3d12_ty::D3D12 Bf::Constant => d3d12_ty::D3D12_BLEND_BLEND_FACTOR, Bf::OneMinusConstant => d3d12_ty::D3D12_BLEND_INV_BLEND_FACTOR, Bf::SrcAlphaSaturated => d3d12_ty::D3D12_BLEND_SRC_ALPHA_SAT, - //Bf::Src1Color if is_alpha => d3d12_ty::D3D12_BLEND_SRC1_ALPHA, - //Bf::Src1Color => d3d12_ty::D3D12_BLEND_SRC1_COLOR, - //Bf::OneMinusSrc1Color if is_alpha => d3d12_ty::D3D12_BLEND_INV_SRC1_ALPHA, - //Bf::OneMinusSrc1Color => d3d12_ty::D3D12_BLEND_INV_SRC1_COLOR, - //Bf::Src1Alpha => d3d12_ty::D3D12_BLEND_SRC1_ALPHA, - //Bf::OneMinusSrc1Alpha => d3d12_ty::D3D12_BLEND_INV_SRC1_ALPHA, + Bf::Src1 if is_alpha => d3d12_ty::D3D12_BLEND_SRC1_ALPHA, + Bf::Src1 => d3d12_ty::D3D12_BLEND_SRC1_COLOR, + Bf::OneMinusSrc1 if is_alpha => d3d12_ty::D3D12_BLEND_INV_SRC1_ALPHA, + Bf::OneMinusSrc1 => d3d12_ty::D3D12_BLEND_INV_SRC1_COLOR, + Bf::Src1Alpha => d3d12_ty::D3D12_BLEND_SRC1_ALPHA, + Bf::OneMinusSrc1Alpha => d3d12_ty::D3D12_BLEND_INV_SRC1_ALPHA, } } diff --git a/wgpu-hal/src/gles/adapter.rs b/wgpu-hal/src/gles/adapter.rs index 348f62bc03..3dae58b7c4 100644 --- a/wgpu-hal/src/gles/adapter.rs +++ b/wgpu-hal/src/gles/adapter.rs @@ -363,6 +363,10 @@ impl super::Adapter { wgt::Features::MULTIVIEW, extensions.contains("OVR_multiview2"), ); + features.set( + wgt::Features::DUAL_SOURCE_BLENDING, + extensions.contains("GL_EXT_blend_func_extended"), + ); features.set( wgt::Features::SHADER_PRIMITIVE_INDEX, ver >= (3, 2) || extensions.contains("OES_geometry_shader"), diff --git a/wgpu-hal/src/gles/conv.rs b/wgpu-hal/src/gles/conv.rs index dd5d764c6a..9bfac022a1 100644 --- a/wgpu-hal/src/gles/conv.rs +++ b/wgpu-hal/src/gles/conv.rs @@ -376,6 +376,10 @@ fn map_blend_factor(factor: wgt::BlendFactor) -> u32 { Bf::Constant => glow::CONSTANT_COLOR, Bf::OneMinusConstant => glow::ONE_MINUS_CONSTANT_COLOR, Bf::SrcAlphaSaturated => glow::SRC_ALPHA_SATURATE, + Bf::Src1 => glow::SRC1_COLOR, + Bf::OneMinusSrc1 => glow::ONE_MINUS_SRC1_COLOR, + Bf::Src1Alpha => glow::SRC1_ALPHA, + Bf::OneMinusSrc1Alpha => glow::ONE_MINUS_SRC1_ALPHA, } } diff --git a/wgpu-hal/src/metal/adapter.rs b/wgpu-hal/src/metal/adapter.rs index 126741d257..ced3378e43 100644 --- a/wgpu-hal/src/metal/adapter.rs +++ b/wgpu-hal/src/metal/adapter.rs @@ -796,6 +796,7 @@ impl super::PrivateCapabilities { None }, timestamp_query_support, + blend_func_extended: version.at_least((11, 0), (14, 0), os_is_mac), } } @@ -833,6 +834,10 @@ impl super::PrivateCapabilities { self.timestamp_query_support .contains(TimestampQuerySupport::INSIDE_WGPU_PASSES), ); + features.set( + F::DUAL_SOURCE_BLENDING, + self.msl_version >= MTLLanguageVersion::V1_2, + ); features.set(F::TEXTURE_COMPRESSION_ASTC, self.format_astc); features.set(F::TEXTURE_COMPRESSION_ASTC_HDR, self.format_astc_hdr); features.set(F::TEXTURE_COMPRESSION_BC, self.format_bc); diff --git a/wgpu-hal/src/metal/conv.rs b/wgpu-hal/src/metal/conv.rs index a1ceb287ab..b96b44c535 100644 --- a/wgpu-hal/src/metal/conv.rs +++ b/wgpu-hal/src/metal/conv.rs @@ -155,10 +155,10 @@ pub fn map_blend_factor(factor: wgt::BlendFactor) -> metal::MTLBlendFactor { //Bf::ConstantAlpha => BlendAlpha, //Bf::OneMinusConstantAlpha => OneMinusBlendAlpha, Bf::SrcAlphaSaturated => SourceAlphaSaturated, - //Bf::Src1 => Source1Color, - //Bf::OneMinusSrc1 => OneMinusSource1Color, - //Bf::Src1Alpha => Source1Alpha, - //Bf::OneMinusSrc1Alpha => OneMinusSource1Alpha, + Bf::Src1 => Source1Color, + Bf::OneMinusSrc1 => OneMinusSource1Color, + Bf::Src1Alpha => Source1Alpha, + Bf::OneMinusSrc1Alpha => OneMinusSource1Alpha, } } diff --git a/wgpu-hal/src/vulkan/adapter.rs b/wgpu-hal/src/vulkan/adapter.rs index bcbab85084..78aceeeeef 100644 --- a/wgpu-hal/src/vulkan/adapter.rs +++ b/wgpu-hal/src/vulkan/adapter.rs @@ -177,6 +177,7 @@ impl PhysicalDeviceFeatures { //.shader_resource_residency(requested_features.contains(wgt::Features::SHADER_RESOURCE_RESIDENCY)) .geometry_shader(requested_features.contains(wgt::Features::SHADER_PRIMITIVE_INDEX)) .depth_clamp(requested_features.contains(wgt::Features::DEPTH_CLIP_CONTROL)) + .dual_src_blend(requested_features.contains(wgt::Features::DUAL_SOURCE_BLENDING)) .build(), descriptor_indexing: if requested_features.intersects(indexing_features()) { Some( @@ -460,6 +461,7 @@ impl PhysicalDeviceFeatures { } features.set(F::DEPTH_CLIP_CONTROL, self.core.depth_clamp != 0); + features.set(F::DUAL_SOURCE_BLENDING, self.core.dual_src_blend != 0); if let Some(ref multiview) = self.multiview { features.set(F::MULTIVIEW, multiview.multiview != 0); diff --git a/wgpu-hal/src/vulkan/conv.rs b/wgpu-hal/src/vulkan/conv.rs index e2398c2689..459b7f858f 100644 --- a/wgpu-hal/src/vulkan/conv.rs +++ b/wgpu-hal/src/vulkan/conv.rs @@ -792,6 +792,10 @@ fn map_blend_factor(factor: wgt::BlendFactor) -> vk::BlendFactor { Bf::SrcAlphaSaturated => vk::BlendFactor::SRC_ALPHA_SATURATE, Bf::Constant => vk::BlendFactor::CONSTANT_COLOR, Bf::OneMinusConstant => vk::BlendFactor::ONE_MINUS_CONSTANT_COLOR, + Bf::Src1 => vk::BlendFactor::SRC1_COLOR, + Bf::OneMinusSrc1 => vk::BlendFactor::ONE_MINUS_SRC1_COLOR, + Bf::Src1Alpha => vk::BlendFactor::SRC1_ALPHA, + Bf::OneMinusSrc1Alpha => vk::BlendFactor::ONE_MINUS_SRC1_ALPHA, } } diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index 9f61e2e490..7b01c5a481 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -781,7 +781,19 @@ bitflags::bitflags! { /// This is a native only feature. const SHADER_EARLY_DEPTH_TEST = 1 << 62; - // 62..64 available + /// Allows two outputs from a shader to be used for blending. + /// Note that dual-source blending doesn't support multiple render targets. + /// + /// For more info see the OpenGL ES extension GL_EXT_blend_func_extended. + /// + /// Supported platforms: + /// - OpenGL ES (with GL_EXT_blend_func_extended) + /// - Metal (with MSL 1.2+) + /// - Vulkan (with dualSrcBlend) + /// - DX12 + const DUAL_SOURCE_BLENDING = 1 << 63; + + // no more space left } } @@ -1549,6 +1561,8 @@ impl TextureViewDimension { /// /// Corresponds to [WebGPU `GPUBlendFactor`]( /// https://gpuweb.github.io/gpuweb/#enumdef-gpublendfactor). +/// 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))] @@ -1581,6 +1595,29 @@ pub enum BlendFactor { Constant = 11, /// 1.0 - Constant OneMinusConstant = 12, + /// S1.component + Src1 = 13, + /// 1.0 - S1.component + OneMinusSrc1 = 14, + /// S1.alpha + Src1Alpha = 15, + /// 1.0 - S1.alpha + OneMinusSrc1Alpha = 16, +} + +impl BlendFactor { + /// Returns `true` if the blend factor references the second blend source. + /// + /// Note that the usage of those blend factors require [`Features::DUAL_SOURCE_BLENDING`]. + pub fn ref_second_blend_source(&self) -> bool { + match self { + BlendFactor::Src1 + | BlendFactor::OneMinusSrc1 + | BlendFactor::Src1Alpha + | BlendFactor::OneMinusSrc1Alpha => true, + _ => false, + } + } } /// Alpha blend operation. diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index 2f83d50c55..2eb3025f9f 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -421,6 +421,15 @@ fn map_blend_factor(factor: wgt::BlendFactor) -> web_sys::GpuBlendFactor { BlendFactor::SrcAlphaSaturated => bf::SrcAlphaSaturated, BlendFactor::Constant => bf::Constant, BlendFactor::OneMinusConstant => bf::OneMinusConstant, + BlendFactor::Src1 + | BlendFactor::OneMinusSrc1 + | BlendFactor::Src1Alpha + | BlendFactor::OneMinusSrc1Alpha => { + panic!( + "{:?} is not enabled for this backend", + wgt::Features::DUAL_SOURCE_BLENDING + ) + } } } From 65f83b71fdd49bfd5b5c59881ca796dd21f25462 Mon Sep 17 00:00:00 2001 From: "Frederik M.J.V." Date: Mon, 18 Sep 2023 18:08:45 +0200 Subject: [PATCH 2/6] Remove any non gl references to blend func extended (hopefully makes CI more happy) --- wgpu-hal/src/dx12/adapter.rs | 2 +- wgpu-hal/src/metal/adapter.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/wgpu-hal/src/dx12/adapter.rs b/wgpu-hal/src/dx12/adapter.rs index 564cf8663a..3959deeccd 100644 --- a/wgpu-hal/src/dx12/adapter.rs +++ b/wgpu-hal/src/dx12/adapter.rs @@ -251,7 +251,7 @@ impl super::Adapter { | wgt::Features::PUSH_CONSTANTS | wgt::Features::SHADER_PRIMITIVE_INDEX | wgt::Features::RG11B10UFLOAT_RENDERABLE - | wgt::Features::BLEND_FUNC_EXTENDED; + | wgt::Features::DUAL_SOURCE_BLENDING; //TODO: in order to expose this, we need to run a compute shader // that extract the necessary statistics out of the D3D12 result. diff --git a/wgpu-hal/src/metal/adapter.rs b/wgpu-hal/src/metal/adapter.rs index ced3378e43..bb1e477205 100644 --- a/wgpu-hal/src/metal/adapter.rs +++ b/wgpu-hal/src/metal/adapter.rs @@ -796,7 +796,7 @@ impl super::PrivateCapabilities { None }, timestamp_query_support, - blend_func_extended: version.at_least((11, 0), (14, 0), os_is_mac), + dual_source_blending: version.at_least((11, 0), (14, 0), os_is_mac), } } From fcf2653c5c1706452fcf3a8e4836287a4e99b523 Mon Sep 17 00:00:00 2001 From: "Frederik M.J.V." Date: Mon, 18 Sep 2023 19:34:04 +0200 Subject: [PATCH 3/6] Complain if a pipeline has a shader with dual source blending, but no fragment stage. Also check if the entry point for the fragment stage uses dual source blending (up for discussion in the review) --- wgpu-core/src/device/resource.rs | 16 ++++++++++++++-- wgpu-core/src/pipeline.rs | 2 ++ wgpu-core/src/validation.rs | 15 +++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index 97567f781e..c16ccf7d45 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -2705,7 +2705,6 @@ impl Device { { break Some(pipeline::ColorStateError::FormatNotMultisampled(cs.format)); } - if let Some(blend_mode) = cs.blend { for factor in [ blend_mode.color.src_factor, @@ -2889,12 +2888,25 @@ impl Device { } } + if let Some(ref interface) = shader_module.interface { + // Is it here we should set shader_expects_dual_source_blending. Where should we read it? + if pipeline_expects_dual_source_blending { + interface.is_fragment_entry_dual_source(fragment) + .expect("Internal error: Fragment entrypoint should not be set in function if not present in shader interface"); + } + } + Some(hal::ProgrammableStage { module: &shader_module.raw, entry_point: fragment.stage.entry_point.as_ref(), }) } - None => None, + None => { + if pipeline_expects_dual_source_blending { + return Err(pipeline::CreateRenderPipelineError::ShaderExpectsPipelineToUseDualSourceBlendingNoFragmentStage); + } + None + } }; if validated_stages.contains(wgt::ShaderStages::FRAGMENT) { diff --git a/wgpu-core/src/pipeline.rs b/wgpu-core/src/pipeline.rs index c78a79820d..3ad03495a0 100644 --- a/wgpu-core/src/pipeline.rs +++ b/wgpu-core/src/pipeline.rs @@ -393,6 +393,8 @@ pub enum CreateRenderPipelineError { PipelineExpectsShaderToUseDualSourceBlending, #[error("Shader entry point expects the pipeline to make use of dual-source blending.")] ShaderExpectsPipelineToUseDualSourceBlending, + #[error("Shader entry point expects the pipeline to make use of dual-source blending, but pipeline contains no fragment stage.")] + ShaderExpectsPipelineToUseDualSourceBlendingNoFragmentStage, } bitflags::bitflags! { diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index bd03455b34..3273ec1b75 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -1184,4 +1184,19 @@ impl Interface { .values() .any(|point| point.dual_source_blending) } + pub fn is_fragment_entry_dual_source<'a>( + &self, + fragment: &crate::pipeline::FragmentState<'a>, + ) -> Result { + if let Some(entry_point) = self.entry_points.get(&( + naga::ShaderStage::Fragment, + String::from(fragment.stage.entry_point.as_ref()), + )) { + Ok(entry_point.dual_source_blending) + } else { + Err(StageError::MissingEntryPoint(String::from( + fragment.stage.entry_point.as_ref(), + ))) + } + } } From e1ec5210df901b3dfb1f565b473451c2452cc459 Mon Sep 17 00:00:00 2001 From: Frederik Magnus Johansen Vestre Date: Mon, 18 Sep 2023 22:01:01 +0200 Subject: [PATCH 4/6] Handle shader expects dual source blending Co-authored-by: Teodor Tanasoaia <28601907+teoxoy@users.noreply.github.com> --- wgpu-core/src/device/resource.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index c16ccf7d45..7535ad6cab 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -2889,11 +2889,7 @@ impl Device { } if let Some(ref interface) = shader_module.interface { - // Is it here we should set shader_expects_dual_source_blending. Where should we read it? - if pipeline_expects_dual_source_blending { - interface.is_fragment_entry_dual_source(fragment) - .expect("Internal error: Fragment entrypoint should not be set in function if not present in shader interface"); - } + shader_expects_dual_source_blending = interface.is_fragment_entry_dual_source(fragment); } Some(hal::ProgrammableStage { @@ -2901,14 +2897,16 @@ impl Device { entry_point: fragment.stage.entry_point.as_ref(), }) } - None => { - if pipeline_expects_dual_source_blending { - return Err(pipeline::CreateRenderPipelineError::ShaderExpectsPipelineToUseDualSourceBlendingNoFragmentStage); - } - None - } + None => None, }; + if !pipeline_expects_dual_source_blending && shader_expects_dual_source_blending { + return Err(pipeline::CreateRenderPipelineError::ShaderExpectsPipelineToUseDualSourceBlending); + } + if pipeline_expects_dual_source_blending && !shader_expects_dual_source_blending { + return Err(pipeline::CreateRenderPipelineError::PipelineExpectsShaderToUseDualSourceBlending); + } + if validated_stages.contains(wgt::ShaderStages::FRAGMENT) { for (i, output) in io.iter() { match color_targets.get(*i as usize) { From 344467854da6cc3692e746764c4504c42f837f1f Mon Sep 17 00:00:00 2001 From: "Frederik M.J.V." Date: Mon, 18 Sep 2023 22:08:55 +0200 Subject: [PATCH 5/6] Add variable, and remove/fix compile --- wgpu-core/src/device/resource.rs | 12 +++++++++--- wgpu-core/src/pipeline.rs | 2 -- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index 7535ad6cab..84be01dad9 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -2564,6 +2564,7 @@ impl Device { let mut vertex_steps = Vec::with_capacity(desc.vertex.buffers.len()); let mut vertex_buffers = Vec::with_capacity(desc.vertex.buffers.len()); let mut total_attributes = 0; + let mut shader_expects_dual_source_blending = false; let mut pipeline_expects_dual_source_blending = false; for (i, vb_state) in desc.vertex.buffers.iter().enumerate() { vertex_steps.push(pipeline::VertexStep { @@ -2889,7 +2890,8 @@ impl Device { } if let Some(ref interface) = shader_module.interface { - shader_expects_dual_source_blending = interface.is_fragment_entry_dual_source(fragment); + shader_expects_dual_source_blending = + interface.is_fragment_entry_dual_source(fragment).expect("Internal error: Fragment entrypoint should not be set in function if not present in shader interface"); } Some(hal::ProgrammableStage { @@ -2901,10 +2903,14 @@ impl Device { }; if !pipeline_expects_dual_source_blending && shader_expects_dual_source_blending { - return Err(pipeline::CreateRenderPipelineError::ShaderExpectsPipelineToUseDualSourceBlending); + return Err( + pipeline::CreateRenderPipelineError::ShaderExpectsPipelineToUseDualSourceBlending, + ); } if pipeline_expects_dual_source_blending && !shader_expects_dual_source_blending { - return Err(pipeline::CreateRenderPipelineError::PipelineExpectsShaderToUseDualSourceBlending); + return Err( + pipeline::CreateRenderPipelineError::PipelineExpectsShaderToUseDualSourceBlending, + ); } if validated_stages.contains(wgt::ShaderStages::FRAGMENT) { diff --git a/wgpu-core/src/pipeline.rs b/wgpu-core/src/pipeline.rs index 3ad03495a0..c78a79820d 100644 --- a/wgpu-core/src/pipeline.rs +++ b/wgpu-core/src/pipeline.rs @@ -393,8 +393,6 @@ pub enum CreateRenderPipelineError { PipelineExpectsShaderToUseDualSourceBlending, #[error("Shader entry point expects the pipeline to make use of dual-source blending.")] ShaderExpectsPipelineToUseDualSourceBlending, - #[error("Shader entry point expects the pipeline to make use of dual-source blending, but pipeline contains no fragment stage.")] - ShaderExpectsPipelineToUseDualSourceBlendingNoFragmentStage, } bitflags::bitflags! { From be93a131d06b470e2149498503f8cb94a5158eda Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 19 Sep 2023 11:44:02 +0200 Subject: [PATCH 6/6] a few more tweaks --- wgpu-core/src/device/resource.rs | 18 +++++++----------- wgpu-core/src/validation.rs | 24 +++++++----------------- wgpu-hal/src/metal/adapter.rs | 3 +-- wgpu-hal/src/metal/conv.rs | 2 -- wgpu-types/src/lib.rs | 2 -- 5 files changed, 15 insertions(+), 34 deletions(-) diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index 84be01dad9..ba7006d3d5 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -2714,8 +2714,8 @@ impl Device { blend_mode.alpha.dst_factor, ] { if factor.ref_second_blend_source() { + self.require_features(wgt::Features::DUAL_SOURCE_BLENDING)?; if i == 0 { - self.require_features(wgt::Features::DUAL_SOURCE_BLENDING)?; pipeline_expects_dual_source_blending = true; break; } else { @@ -2831,14 +2831,6 @@ impl Device { error, })?; validated_stages |= flag; - - let dual_source = interface.has_dual_source_blending_entry_point(); - if !pipeline_expects_dual_source_blending && dual_source { - return Err(pipeline::CreateRenderPipelineError::ShaderExpectsPipelineToUseDualSourceBlending); - } - if pipeline_expects_dual_source_blending && !dual_source { - return Err(pipeline::CreateRenderPipelineError::PipelineExpectsShaderToUseDualSourceBlending); - } } hal::ProgrammableStage { @@ -2890,8 +2882,12 @@ impl Device { } if let Some(ref interface) = shader_module.interface { - shader_expects_dual_source_blending = - interface.is_fragment_entry_dual_source(fragment).expect("Internal error: Fragment entrypoint should not be set in function if not present in shader interface"); + shader_expects_dual_source_blending = interface + .fragment_uses_dual_source_blending(&fragment.stage.entry_point) + .map_err(|error| pipeline::CreateRenderPipelineError::Stage { + stage: flag, + error, + })?; } Some(hal::ProgrammableStage { diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index 3273ec1b75..778cc26cd5 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -1179,24 +1179,14 @@ impl Interface { Ok(outputs) } - pub fn has_dual_source_blending_entry_point(&self) -> bool { - self.entry_points - .values() - .any(|point| point.dual_source_blending) - } - pub fn is_fragment_entry_dual_source<'a>( + pub fn fragment_uses_dual_source_blending( &self, - fragment: &crate::pipeline::FragmentState<'a>, + entry_point_name: &str, ) -> Result { - if let Some(entry_point) = self.entry_points.get(&( - naga::ShaderStage::Fragment, - String::from(fragment.stage.entry_point.as_ref()), - )) { - Ok(entry_point.dual_source_blending) - } else { - Err(StageError::MissingEntryPoint(String::from( - fragment.stage.entry_point.as_ref(), - ))) - } + let pair = (naga::ShaderStage::Fragment, entry_point_name.to_string()); + self.entry_points + .get(&pair) + .ok_or(StageError::MissingEntryPoint(pair.1)) + .map(|ep| ep.dual_source_blending) } } diff --git a/wgpu-hal/src/metal/adapter.rs b/wgpu-hal/src/metal/adapter.rs index bb1e477205..da254442bc 100644 --- a/wgpu-hal/src/metal/adapter.rs +++ b/wgpu-hal/src/metal/adapter.rs @@ -796,7 +796,6 @@ impl super::PrivateCapabilities { None }, timestamp_query_support, - dual_source_blending: version.at_least((11, 0), (14, 0), os_is_mac), } } @@ -836,7 +835,7 @@ impl super::PrivateCapabilities { ); features.set( F::DUAL_SOURCE_BLENDING, - self.msl_version >= MTLLanguageVersion::V1_2, + self.msl_version >= MTLLanguageVersion::V1_2 && self.dual_source_blending, ); features.set(F::TEXTURE_COMPRESSION_ASTC, self.format_astc); features.set(F::TEXTURE_COMPRESSION_ASTC_HDR, self.format_astc_hdr); diff --git a/wgpu-hal/src/metal/conv.rs b/wgpu-hal/src/metal/conv.rs index b96b44c535..8f6439b50b 100644 --- a/wgpu-hal/src/metal/conv.rs +++ b/wgpu-hal/src/metal/conv.rs @@ -152,8 +152,6 @@ pub fn map_blend_factor(factor: wgt::BlendFactor) -> metal::MTLBlendFactor { Bf::OneMinusDstAlpha => OneMinusDestinationAlpha, Bf::Constant => BlendColor, Bf::OneMinusConstant => OneMinusBlendColor, - //Bf::ConstantAlpha => BlendAlpha, - //Bf::OneMinusConstantAlpha => OneMinusBlendAlpha, Bf::SrcAlphaSaturated => SourceAlphaSaturated, Bf::Src1 => Source1Color, Bf::OneMinusSrc1 => OneMinusSource1Color, diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index 7b01c5a481..e08b802094 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -792,8 +792,6 @@ bitflags::bitflags! { /// - Vulkan (with dualSrcBlend) /// - DX12 const DUAL_SOURCE_BLENDING = 1 << 63; - - // no more space left } }