From cdbf23813ab6473fe47b467d8b9791be65dd7d09 Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Sun, 7 Jun 2020 11:52:05 -0400 Subject: [PATCH] Return errors on create_render_pipeline --- player/src/main.rs | 3 +- wgpu-core/src/device/mod.rs | 60 +++++++++++++++++++++---------------- wgpu-core/src/pipeline.rs | 16 ++++++++++ 3 files changed, 52 insertions(+), 27 deletions(-) diff --git a/player/src/main.rs b/player/src/main.rs index e7b2a2b558..24daaf569f 100644 --- a/player/src/main.rs +++ b/player/src/main.rs @@ -403,7 +403,8 @@ impl GlobalExt for wgc::hub::Global { alpha_to_coverage_enabled: desc.alpha_to_coverage_enabled, }, id, - ); + ) + .unwrap(); } A::DestroyRenderPipeline(id) => { self.render_pipeline_destroy::(id); diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 959d1e2bb7..edfb75547d 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -1752,17 +1752,17 @@ impl Global { device_id: id::DeviceId, desc: &pipeline::RenderPipelineDescriptor, id_in: Input, - ) -> id::RenderPipelineId { + ) -> Result { let hub = B::hub(self); let mut token = Token::root(); - let sc = desc.sample_count; - assert!( - sc == 1 || sc == 2 || sc == 4 || sc == 8 || sc == 16 || sc == 32, - "Invalid sample_count of {}; must be 1, 2, 4, 8, 16, or 32", - sc - ); - let sc = sc as u8; + let samples = { + let sc = desc.sample_count; + if sc == 0 || sc > 32 || !conv::is_power_of_two(sc) { + return Err(pipeline::RenderPipelineError::InvalidSampleCount(sc)); + } + sc as u8 + }; let color_states = unsafe { slice::from_raw_parts(desc.color_states, desc.color_states_length) }; @@ -1803,13 +1803,14 @@ impl Global { let desc_atts = unsafe { slice::from_raw_parts(vb_state.attributes, vb_state.attributes_length) }; for attribute in desc_atts { - assert_eq!( - 0, - attribute.offset >> 32, - "Offset for attribute {:?} must be < 2^32, but was {}", - attribute, - attribute.offset - ); + if attribute.offset >= 0x10000000 { + return Err( + pipeline::RenderPipelineError::InvalidVertexAttributeOffset { + location: attribute.shader_location, + offset: attribute.offset, + }, + ); + } attributes.alloc().init(hal::pso::AttributeDesc { location: attribute.shader_location, binding: i as u32, @@ -1842,11 +1843,11 @@ impl Global { .map(conv::map_depth_stencil_state_descriptor) .unwrap_or_default(); - let multisampling: Option = if sc == 1 { + let multisampling: Option = if samples == 1 { None } else { Some(hal::pso::Multisampling { - rasterization_samples: sc, + rasterization_samples: samples, sample_shading: None, sample_mask: desc.sample_mask as u64, alpha_coverage: desc.alpha_to_coverage_enabled, @@ -1885,7 +1886,7 @@ impl Global { state.format, device.private_features, )), - samples: sc, + samples, ops: hal::pass::AttachmentOps::PRESERVE, stencil_ops: hal::pass::AttachmentOps::DONT_CARE, layouts: hal::image::Layout::General..hal::image::Layout::General, @@ -1904,7 +1905,7 @@ impl Global { state.format, device.private_features, )), - samples: sc, + samples, ops: hal::pass::AttachmentOps::PRESERVE, stencil_ops: hal::pass::AttachmentOps::PRESERVE, layouts: hal::image::Layout::General..hal::image::Layout::General, @@ -1923,6 +1924,7 @@ impl Global { let shader_module = &shader_module_guard[desc.vertex_stage.module]; if let Some(ref module) = shader_module.module { + let flag = wgt::ShaderStage::VERTEX; interface = validation::check_stage( module, &group_layouts, @@ -1930,8 +1932,8 @@ impl Global { ExecutionModel::Vertex, interface, ) - .unwrap(); - validated_stages |= wgt::ShaderStage::VERTEX; + .map_err(|error| pipeline::RenderPipelineError::Stage { flag, error })?; + validated_stages |= flag; } hal::pso::EntryPoint:: { @@ -1952,6 +1954,7 @@ impl Global { if validated_stages == wgt::ShaderStage::VERTEX { if let Some(ref module) = shader_module.module { + let flag = wgt::ShaderStage::FRAGMENT; interface = validation::check_stage( module, &group_layouts, @@ -1959,8 +1962,10 @@ impl Global { ExecutionModel::Fragment, interface, ) - .unwrap(); - validated_stages |= wgt::ShaderStage::FRAGMENT; + .map_err(|error| { + pipeline::RenderPipelineError::Stage { flag, error } + })?; + validated_stages |= flag; } } @@ -1977,12 +1982,15 @@ impl Global { for (i, state) in color_states.iter().enumerate() { let output = &interface[&(i as wgt::ShaderLocation)]; if !validation::check_texture_format(state.format, output) { - log::error!( + log::warn!( "Incompatible fragment output[{}]. Shader: {:?}. Expected: {:?}", i, state.format, &**output ); + return Err(pipeline::RenderPipelineError::IncompatibleOutputFormat { + index: i as u8, + }); } } } @@ -2069,7 +2077,7 @@ impl Global { flags, index_format: desc.vertex_state.index_format, vertex_strides, - sample_count: sc, + sample_count: samples, life_guard: LifeGuard::new(), }; @@ -2113,7 +2121,7 @@ impl Global { }), None => (), }; - id + Ok(id) } pub fn render_pipeline_destroy(&self, render_pipeline_id: id::RenderPipelineId) { diff --git a/wgpu-core/src/pipeline.rs b/wgpu-core/src/pipeline.rs index 3868fe502a..ca9170e033 100644 --- a/wgpu-core/src/pipeline.rs +++ b/wgpu-core/src/pipeline.rs @@ -94,6 +94,22 @@ pub struct RenderPipelineDescriptor { pub alpha_to_coverage_enabled: bool, } +#[derive(Clone, Debug)] +pub enum RenderPipelineError { + InvalidVertexAttributeOffset { + location: wgt::ShaderLocation, + offset: BufferAddress, + }, + Stage { + flag: wgt::ShaderStage, + error: StageError, + }, + IncompatibleOutputFormat { + index: u8, + }, + InvalidSampleCount(u32), +} + bitflags::bitflags! { #[repr(transparent)] pub struct PipelineFlags: u32 {