From fff431ba55120ed1d7bc2644f27222e99feabb45 Mon Sep 17 00:00:00 2001 From: Tom Gowan Date: Mon, 29 Apr 2019 18:17:47 +1000 Subject: [PATCH] no expects or unimpl --- examples/compile.rs | 2 +- src/compiler.rs | 15 ++++--- src/error.rs | 12 +++++- src/layouts.rs | 16 ++++---- src/lib.rs | 4 +- src/reflection.rs | 95 ++++++++++++++++++++++----------------------- src/srvk.rs | 49 +++++++++++++---------- src/watch.rs | 41 ++++++++++--------- tests/tests.rs | 2 +- 9 files changed, 126 insertions(+), 110 deletions(-) diff --git a/examples/compile.rs b/examples/compile.rs index 46be1da..fe6b915 100644 --- a/examples/compile.rs +++ b/examples/compile.rs @@ -8,6 +8,6 @@ fn main() { let mut frag_path = project_root.clone(); frag_path.push(PathBuf::from("examples/shaders/frag.glsl")); let shader = sr::load(vert_path, frag_path).expect("Failed to compile"); - let vulkano_entry = sr::parse(&shader); + let vulkano_entry = sr::parse(&shader).expect("failed to parse"); dbg!(vulkano_entry); } diff --git a/src/compiler.rs b/src/compiler.rs index 60bdf56..e5dff76 100644 --- a/src/compiler.rs +++ b/src/compiler.rs @@ -2,25 +2,24 @@ use std::fs::File; use std::path::Path; use shaderc::ShaderKind; use std::io::Read; +use crate::error::CompileError; -pub fn compile(path: T, shader_kind: ShaderKind) -> shaderc::Result> +pub fn compile(path: T, shader_kind: ShaderKind) -> Result, CompileError> where T: AsRef, { // TODO Probably shouldn't create this every time. - let mut compiler = shaderc::Compiler::new().expect("failed to create compiler"); - let mut f = File::open(&path).expect("failed to open shader src"); + let mut compiler = shaderc::Compiler::new().ok_or(CompileError::CreateCompiler)?; + let mut f = File::open(&path).map_err(CompileError::Open)?; let mut src = String::new(); - f.read_to_string(&mut src).expect("failed to read src"); - let mut options = shaderc::CompileOptions::new().unwrap(); - options.add_macro_definition("EP", Some("main")); + f.read_to_string(&mut src).map_err(CompileError::Open)?; let result = compiler.compile_into_spirv( src.as_str(), shader_kind, - path.as_ref().to_str().expect("failed to make path string"), + path.as_ref().to_str().ok_or(CompileError::InvalidPath)?, "main", None, - )?; + ).map_err(CompileError::Compile)?; let data = result.as_binary(); Ok(data.to_owned()) } diff --git a/src/error.rs b/src/error.rs index 2eb1075..7b5937f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,10 +1,20 @@ #[derive(Debug)] pub enum Error { - Compile(shaderc::Error), + Compile(CompileError), Layout(ConvertError), + LoadingData(String), + FileWatch(notify::Error), } #[derive(Debug)] pub enum ConvertError { Unimplemented, } + +#[derive(Debug)] +pub enum CompileError { + Compile(shaderc::Error), + Open(std::io::Error), + InvalidPath, + CreateCompiler, +} diff --git a/src/layouts.rs b/src/layouts.rs index 10f6266..e66ce15 100644 --- a/src/layouts.rs +++ b/src/layouts.rs @@ -56,14 +56,14 @@ unsafe impl PipelineLayoutDesc for FragLayout { self.layout_data.num_sets } fn num_bindings_in_set(&self, set: usize) -> Option { - self.layout_data.num_bindings.get(&set).map(|&i| i) + self.layout_data.num_bindings.get(&set).copied() } fn descriptor(&self, set: usize, binding: usize) -> Option { self.layout_data.descriptions.get(&set) .and_then(|s|s.get(&binding)) .map(|desc| { let mut desc = desc.clone(); - desc.stages = self.stages.clone(); + desc.stages = self.stages; desc }) @@ -74,8 +74,8 @@ unsafe impl PipelineLayoutDesc for FragLayout { fn push_constants_range(&self, num: usize) -> Option { self.layout_data.pc_ranges.get(num) .map(|desc| { - let mut desc = desc.clone(); - desc.stages = self.stages.clone(); + let mut desc = *desc; + desc.stages = self.stages; desc }) @@ -123,14 +123,14 @@ unsafe impl PipelineLayoutDesc for VertLayout { self.layout_data.num_sets } fn num_bindings_in_set(&self, set: usize) -> Option { - self.layout_data.num_bindings.get(&set).map(|&i| i) + self.layout_data.num_bindings.get(&set).copied() } fn descriptor(&self, set: usize, binding: usize) -> Option { self.layout_data.descriptions.get(&set) .and_then(|s|s.get(&binding)) .map(|desc| { let mut desc = desc.clone(); - desc.stages = self.stages.clone(); + desc.stages = self.stages; desc }) @@ -141,8 +141,8 @@ unsafe impl PipelineLayoutDesc for VertLayout { fn push_constants_range(&self, num: usize) -> Option { self.layout_data.pc_ranges.get(num) .map(|desc| { - let mut desc = desc.clone(); - desc.stages = self.stages.clone(); + let mut desc = *desc; + desc.stages = self.stages; desc }) diff --git a/src/lib.rs b/src/lib.rs index f6442bc..16909fc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,8 +24,8 @@ pub fn load(vertex: T, fragment: T) -> Result where T: AsRef, { - let vertex = compiler::compile(vertex, ShaderKind::Vertex).map_err(|e| Error::Compile(e))?; - let fragment = compiler::compile(fragment, ShaderKind::Fragment).map_err(|e| Error::Compile(e))?; + let vertex = compiler::compile(vertex, ShaderKind::Vertex).map_err(Error::Compile)?; + let fragment = compiler::compile(fragment, ShaderKind::Fragment).map_err(Error::Compile)?; Ok(CompiledShaders{ vertex, fragment }) } diff --git a/src/reflection.rs b/src/reflection.rs index 13ae3e9..159303b 100644 --- a/src/reflection.rs +++ b/src/reflection.rs @@ -1,3 +1,4 @@ +use crate::error::Error; use crate::layouts::*; use crate::sr; use crate::srvk::{DescriptorDescInfo, SpirvTy}; @@ -5,7 +6,6 @@ use crate::vk::descriptor::descriptor::*; use crate::vk::descriptor::pipeline_layout::PipelineLayoutDescPcRange; use crate::vk::pipeline::shader::ShaderInterfaceDefEntry; use crate::CompiledShaders; -use crate::error::Error; use std::borrow::Cow; use std::collections::HashMap; use std::convert::TryFrom; @@ -25,9 +25,9 @@ pub struct LayoutData { } pub fn create_entry(shaders: &CompiledShaders) -> Result { - let vertex_interfaces = create_interfaces(&shaders.vertex); + let vertex_interfaces = create_interfaces(&shaders.vertex)?; let vertex_layout = create_layouts(&shaders.vertex)?; - let fragment_interfaces = create_interfaces(&shaders.fragment); + let fragment_interfaces = create_interfaces(&shaders.fragment)?; let fragment_layout = create_layouts(&shaders.fragment)?; let frag_input = FragInput { inputs: fragment_interfaces.inputs, @@ -65,62 +65,60 @@ pub fn create_entry(shaders: &CompiledShaders) -> Result { }) } -fn create_interfaces(data: &[u32]) -> ShaderInterfaces { +fn create_interfaces(data: &[u32]) -> Result { sr::ShaderModule::load_u32_data(data) + .map_err(|e| Error::LoadingData(e.to_string())) .map(|m| { let inputs = m .enumerate_input_variables(None) - .map(|inputs| { + .map_err(|e| Error::LoadingData(e.to_string())) + .and_then(|inputs| { inputs .iter() .filter(|i| { !i.decoration_flags .contains(sr::types::ReflectDecorationFlags::BUILT_IN) }) - .map(|i| ShaderInterfaceDefEntry { + .map(|i| Ok(ShaderInterfaceDefEntry { location: i.location..(i.location + 1), - format: SpirvTy::from(i.format).inner(), + format: SpirvTy::try_from(i.format)?.inner(), name: Some(Cow::from(i.name.clone())), - }) - .collect::>() - }) - .expect("Failed to pass inputs"); + })) + .collect::, _>>() + }); let outputs = m .enumerate_output_variables(None) - .map(|outputs| { + .map_err(|e| Error::LoadingData(e.to_string())) + .and_then(|outputs| { outputs .iter() .filter(|i| { !i.decoration_flags .contains(sr::types::ReflectDecorationFlags::BUILT_IN) }) - .map(|i| ShaderInterfaceDefEntry { + .map(|i| Ok(ShaderInterfaceDefEntry { location: i.location..(i.location + 1), - format: SpirvTy::from(i.format).inner(), + format: SpirvTy::try_from(i.format)?.inner(), name: Some(Cow::from(i.name.clone())), - }) - .collect::>() - }) - .expect("Failed to pass outputs"); - ShaderInterfaces { inputs, outputs } + })) + .collect::, _>>() + }); + inputs.and_then(|inputs| outputs.map(|outputs| ShaderInterfaces { inputs, outputs } )) }) - .expect("failed to load module") + .and_then(|t| t) } fn create_layouts(data: &[u32]) -> Result { sr::ShaderModule::load_u32_data(data) .map(|m| { - //let (num_sets, num_bindings, descriptions) = m - let descs = - m.enumerate_descriptor_sets(None) - .map(|sets| { + let descs: Result<_, Error> = m + .enumerate_descriptor_sets(None) + .map_err(|e| Error::LoadingData(e.to_string())) + .and_then(|sets| { let num_sets = sets.len(); let num_bindings = sets .iter() - .map(|i| { - dbg!(&i); - (i.set as usize, i.bindings.len()) - }) + .map(|i| (i.set as usize, i.bindings.len())) .collect::>(); let descriptions = sets .iter() @@ -133,8 +131,7 @@ fn create_layouts(data: &[u32]) -> Result { descriptor_type: b.descriptor_type, image: b.image, }; - let ty = SpirvTy::::try_from(info)?; - let ty = ty.inner(); + let ty = SpirvTy::::try_from(info)?.inner(); let stages = ShaderStages::none(); let d = DescriptorDesc { ty, @@ -146,17 +143,15 @@ fn create_layouts(data: &[u32]) -> Result { }; Ok((b.binding as usize, d)) }) - .flat_map(|d| d.ok()) - .collect::>(); - (i.set as usize, desc) + .collect::, Error>>(); + desc.and_then(|d| Ok((i.set as usize, d))) }) - .collect::>>(); - (num_sets, num_bindings, descriptions) - }) - .into_iter(); - //let (num_constants, pc_ranges) = m - let pcs = - m.enumerate_push_constant_blocks(None) + .collect::, Error>>(); + descriptions.map(|d| (num_sets, num_bindings, d)) + }); + let pcs = m + .enumerate_push_constant_blocks(None) + .map_err(|e| Error::LoadingData(e.to_string())) .map(|constants| { let num_constants = constants.len(); let pc_ranges = constants @@ -168,15 +163,17 @@ fn create_layouts(data: &[u32]) -> Result { }) .collect::>(); (num_constants, pc_ranges) + }); + descs.and_then(|(num_sets, num_bindings, descriptions)| { + pcs.map(|(num_constants, pc_ranges)| LayoutData { + num_sets, + num_bindings, + descriptions, + num_constants, + pc_ranges, }) - .into_iter(); - descs.flat_map(|(num_sets, num_bindings, descriptions)| pcs.map(|(num_constants, pc_ranges)| - LayoutData { - num_sets, - num_bindings, - descriptions, - num_constants, - pc_ranges, - })).next() + }) }) + .map_err(|e| Error::LoadingData(e.to_string())) + .and_then(|t| t) } diff --git a/src/srvk.rs b/src/srvk.rs index e7f7643..e74690d 100644 --- a/src/srvk.rs +++ b/src/srvk.rs @@ -29,7 +29,7 @@ impl TryFrom for SpirvTy { match d.descriptor_type { SR::Undefined => Err(ConvertError::Unimplemented), SR::Sampler => Ok(VK::Sampler), - SR::CombinedImageSampler => Ok(VK::CombinedImageSampler(SpirvTy::from(d.image).inner())), + SR::CombinedImageSampler => Ok(VK::CombinedImageSampler(SpirvTy::try_from(d.image)?.inner())), SR::SampledImage => Err(ConvertError::Unimplemented), SR::StorageImage => Err(ConvertError::Unimplemented), SR::UniformTexelBuffer => Err(ConvertError::Unimplemented), @@ -42,12 +42,13 @@ impl TryFrom for SpirvTy { SR::AccelerationStructureNV => Err(ConvertError::Unimplemented), } .map(|t| SpirvTy{ inner: t }) - .map_err(|e| Error::Layout(e)) + .map_err(Error::Layout) } } -impl From for SpirvTy { - fn from(d: sr::types::ReflectImageTraits) -> Self { +impl TryFrom for SpirvTy { + type Error = Error; + fn try_from(d: sr::types::ReflectImageTraits) -> Result { let conv_array_layers = |a, d|{ if a != 0 { DescriptorImageDescArray::Arrayed{max_layers: Some(d)} @@ -57,29 +58,31 @@ impl From for SpirvTy { }; let t = DescriptorImageDesc { sampled: d.sampled != 0, - dimensions: SpirvTy::from(d.dim).inner(), + dimensions: SpirvTy::try_from(d.dim)?.inner(), // TODO figure out how to do format correctly //format: Some(SpirvTy::from(d.image_format).inner()), format: None, multisampled: d.ms != 0, array_layers: conv_array_layers(d.arrayed, d.depth), }; - SpirvTy{inner: t} + Ok(SpirvTy{inner: t}) } } -impl From for SpirvTy { - fn from(d: sr::types::variable::ReflectDimension) -> Self { +impl TryFrom for SpirvTy { + type Error = Error; + fn try_from(d: sr::types::variable::ReflectDimension) -> Result { use sr::types::variable::ReflectDimension::*; use DescriptorImageDescDimensions::*; - let inner = match d { - Type1d => OneDimensional, - Type2d => TwoDimensional, - Type3d => ThreeDimensional, - sr::types::variable::ReflectDimension::Cube => DescriptorImageDescDimensions::Cube, - _ => unimplemented!(), - }; - SpirvTy{ inner } + match d { + Type1d => Ok(OneDimensional), + Type2d => Ok(TwoDimensional), + Type3d => Ok(ThreeDimensional), + sr::types::variable::ReflectDimension::Cube => Ok(DescriptorImageDescDimensions::Cube), + _ => Err(ConvertError::Unimplemented), + } + .map(|t| SpirvTy{ inner: t }) + .map_err(Error::Layout) } } @@ -130,16 +133,20 @@ impl From for SpirvTy { R16_UINT =>R16Uint, R8_UINT =>R8Uint, }; - SpirvTy{ inner } + SpirvTy{ inner }; + // This function shouldn't be called yet because + // it is not implemented correctly + unreachable!() } } -impl From for SpirvTy { - fn from(f: sr::types::ReflectFormat) -> Self { +impl TryFrom for SpirvTy { + type Error = Error; + fn try_from(f: sr::types::ReflectFormat) -> Result { use sr::types::ReflectFormat::*; use Format::*; let t = match f { - Undefined => unimplemented!(), + Undefined => Err(Error::Layout(ConvertError::Unimplemented))?, R32_UINT => R32Uint, R32_SINT => R32Sint, R32_SFLOAT => R32Sfloat, @@ -153,6 +160,6 @@ impl From for SpirvTy { R32G32B32A32_SINT => R32G32B32A32Sint, R32G32B32A32_SFLOAT => R32G32B32A32Sfloat, }; - SpirvTy { inner: t } + Ok(SpirvTy { inner: t }) } } diff --git a/src/watch.rs b/src/watch.rs index 9fd593f..9d52c0d 100644 --- a/src/watch.rs +++ b/src/watch.rs @@ -25,18 +25,18 @@ pub struct Message { } impl Watch { - pub fn new(vertex: T, fragment: T) -> Self + pub fn create(vertex: T, fragment: T) -> Result where T: AsRef, { let (handler, rx) = create_watch( vertex.as_ref().to_path_buf(), fragment.as_ref().to_path_buf(), - ); - Watch { + )?; + Ok(Watch { _handler: handler, rx, - } + }) } } @@ -56,7 +56,8 @@ impl Loader { match crate::load(&self.vertex, &self.fragment) { Ok(shaders) => { let entry = crate::parse(&shaders); - self.tx.send(Ok(Message { shaders, entry })).ok() + let msg = entry.map(|entry| Message { shaders, entry }); + self.tx.send(msg).ok() } Err(e) => self.tx.send(Err(e)).ok(), }; @@ -79,31 +80,33 @@ impl Drop for Handler { } fn create_watch( - mut vert_path: PathBuf, - mut frag_path: PathBuf, -) -> (Handler, mpsc::Receiver>) { + vert_path: PathBuf, + frag_path: PathBuf, +) -> Result<(Handler, mpsc::Receiver>), Error> { let (notify_tx, notify_rx) = mpsc::channel(); let (thread_tx, thread_rx) = mpsc::channel(); let mut watcher: RecommendedWatcher = - Watcher::new(notify_tx, Duration::from_millis(50)).expect("failed to create watcher"); + Watcher::new(notify_tx, Duration::from_millis(50)).map_err(Error::FileWatch)?; - vert_path.pop(); - frag_path.pop(); + let mut vp = vert_path.clone(); + let mut fp = frag_path.clone(); + vp.pop(); + fp.pop(); watcher - .watch(&vert_path, RecursiveMode::NonRecursive) - .expect("failed to add vertex shader to notify"); + .watch(&vp, RecursiveMode::NonRecursive) + .map_err(Error::FileWatch)?; watcher - .watch(&frag_path, RecursiveMode::NonRecursive) - .expect("failed to add fragment shader to notify"); + .watch(&fp, RecursiveMode::NonRecursive) + .map_err(Error::FileWatch)?; let (loader, rx) = Loader::create(vert_path, frag_path); let handle = thread::spawn(move || 'watch_loop: loop { - if let Ok(_) = thread_rx.try_recv() { + if thread_rx.try_recv().is_ok() { break 'watch_loop; } - if let Ok(notify::DebouncedEvent::Create(_)) - | Ok(notify::DebouncedEvent::Write(_)) = notify_rx.recv_timeout(Duration::from_secs(1)) + if let Ok(notify::DebouncedEvent::Create(_)) | Ok(notify::DebouncedEvent::Write(_)) = + notify_rx.recv_timeout(Duration::from_secs(1)) { loader.reload(); } @@ -114,5 +117,5 @@ fn create_watch( handle, _watcher: watcher, }; - (handler, rx) + Ok((handler, rx)) } diff --git a/tests/tests.rs b/tests/tests.rs index 187a5ec..8c96e90 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -65,7 +65,7 @@ where let mut fragment_path = path.clone(); fragment_path.push(fragment); let shader = shade_runner::load(vertex_path, fragment_path).expect("Failed to compile"); - shade_runner::parse(&shader) + shade_runner::parse(&shader).unwrap() } fn do_test(a: &T, b: &T)