From 82d149b8718d2c52726a75324a4432d49b19bb14 Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Tue, 21 Jul 2020 11:54:42 -0400 Subject: [PATCH] Naga update, remove spirv_headers dependency --- Cargo.lock | 4 +-- player/src/lib.rs | 25 ++++++++------ wgpu-core/Cargo.toml | 4 +-- wgpu-core/src/command/bundle.rs | 1 + wgpu-core/src/device/mod.rs | 39 ++++++++++++--------- wgpu-core/src/pipeline.rs | 6 ++-- wgpu-core/src/swap_chain.rs | 13 +++---- wgpu-core/src/validation.rs | 60 +++++++++++++++------------------ 8 files changed, 80 insertions(+), 72 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d1456bd34b..1d6dd9f3e8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -780,13 +780,14 @@ dependencies = [ [[package]] name = "naga" version = "0.1.0" -source = "git+https://github.com/gfx-rs/naga?rev=a9228d2aed38c71388489a95817238ff98198fa3#a9228d2aed38c71388489a95817238ff98198fa3" +source = "git+https://github.com/gfx-rs/naga?rev=94802078c3bc5d138f497419ea3e7a869f10916d#94802078c3bc5d138f497419ea3e7a869f10916d" dependencies = [ "bitflags", "fxhash", "log", "num-traits", "spirv_headers", + "thiserror", ] [[package]] @@ -1617,7 +1618,6 @@ dependencies = [ "ron", "serde", "smallvec", - "spirv_headers", "thiserror", "thread-id", "tracing", diff --git a/player/src/lib.rs b/player/src/lib.rs index ef69ef797b..e23459ec77 100644 --- a/player/src/lib.rs +++ b/player/src/lib.rs @@ -12,7 +12,7 @@ use wgc::device::trace; -use std::{ffi::CString, fmt::Debug, fs, marker::PhantomData, path::Path, ptr}; +use std::{borrow::Cow, ffi::CString, fmt::Debug, fs, marker::PhantomData, path::Path, ptr}; #[macro_export] macro_rules! gfx_select { @@ -226,16 +226,19 @@ impl GlobalPlay for wgc::hub::Global { self.bind_group_destroy::(id); } A::CreateShaderModule { id, data } => { - let byte_vec = fs::read(dir.join(data)).unwrap(); - let spv = byte_vec - .chunks(4) - .map(|c| u32::from_le_bytes([c[0], c[1], c[2], c[3]])) - .collect::>(); - self.device_create_shader_module::( - device, - wgc::pipeline::ShaderModuleSource::SpirV(&spv), - id, - ); + let source = if data.ends_with(".wgsl") { + let code = fs::read_to_string(dir.join(data)).unwrap(); + wgc::pipeline::ShaderModuleSource::Wgsl(Cow::Owned(code)) + } else { + let byte_vec = fs::read(dir.join(data)).unwrap(); + let spv = byte_vec + .chunks(4) + .map(|c| u32::from_le_bytes([c[0], c[1], c[2], c[3]])) + .collect::>(); + wgc::pipeline::ShaderModuleSource::SpirV(Cow::Owned(spv)) + }; + self.device_create_shader_module::(device, source, id) + .unwrap(); } A::DestroyShaderModule(id) => { self.shader_module_destroy::(id); diff --git a/wgpu-core/Cargo.toml b/wgpu-core/Cargo.toml index 731a6b0aca..2f64c13ba0 100644 --- a/wgpu-core/Cargo.toml +++ b/wgpu-core/Cargo.toml @@ -35,7 +35,6 @@ raw-window-handle = { version = "0.3", optional = true } ron = { version = "0.5", optional = true } serde = { version = "1.0", features = ["serde_derive"], optional = true } smallvec = "1" -spirv_headers = { version = "1.4.2" } thread-id = { version = "3", optional = true } tracing = { version = "0.1", default-features = false, features = ["std"] } tracing-subscriber = { version = "0.2", optional = true } @@ -43,7 +42,8 @@ thiserror = "1" [dependencies.naga] git = "https://github.com/gfx-rs/naga" -rev = "a9228d2aed38c71388489a95817238ff98198fa3" +rev = "94802078c3bc5d138f497419ea3e7a869f10916d" +features = ["spirv"] [dependencies.gfx-descriptor] git = "https://github.com/gfx-rs/gfx-extras" diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index fadb322d2b..32e0442d93 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -35,6 +35,7 @@ buffer. Thanks to the "normalized" property, it doesn't track any bind group invalidations or index format changes. !*/ +#![allow(clippy::reversed_empty_ranges)] use crate::{ binding_model::PushConstantUploadError, diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index f2f9d2a92f..3899131af4 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -28,12 +28,10 @@ use thiserror::Error; use wgt::{BufferAddress, BufferSize, InputStepMode, TextureDimension, TextureFormat}; use std::{ - collections::hash_map::Entry, ffi, iter, marker::PhantomData, mem, ops::Range, ptr, - sync::atomic::Ordering, + borrow::Cow, collections::hash_map::Entry, ffi, iter, marker::PhantomData, mem, ops::Range, + ptr, sync::atomic::Ordering, }; -use spirv_headers::ExecutionModel; - mod life; mod queue; #[cfg(any(feature = "trace", feature = "replay"))] @@ -1888,14 +1886,13 @@ impl Global { device_id: id::DeviceId, source: pipeline::ShaderModuleSource, id_in: Input, - ) -> id::ShaderModuleId { + ) -> Result { span!(_guard, INFO, "Device::create_shader_module"); let hub = B::hub(self); let mut token = Token::root(); let (device_guard, mut token) = hub.devices.read(&mut token); let device = &device_guard[device_id]; - let spv_owned; let spv_flags = if cfg!(debug_assertions) { naga::back::spv::WriterFlags::DEBUG } else { @@ -1920,10 +1917,10 @@ impl Global { (spv, module) } pipeline::ShaderModuleSource::Wgsl(code) => { - let module = naga::front::wgsl::parse_str(code).unwrap(); - spv_owned = naga::back::spv::Writer::new(&module.header, spv_flags).write(&module); + let module = naga::front::wgsl::parse_str(&code).unwrap(); + let spv = naga::back::spv::Writer::new(&module.header, spv_flags).write(&module); ( - spv_owned.as_slice(), + Cow::Owned(spv), if device.private_features.shader_validation { Some(module) } else { @@ -1932,9 +1929,9 @@ impl Global { ) } pipeline::ShaderModuleSource::Naga(module) => { - spv_owned = naga::back::spv::Writer::new(&module.header, spv_flags).write(&module); + let spv = naga::back::spv::Writer::new(&module.header, spv_flags).write(&module); ( - spv_owned.as_slice(), + Cow::Owned(spv), if device.private_features.shader_validation { Some(module) } else { @@ -1944,8 +1941,12 @@ impl Global { } }; + if let Some(ref module) = naga { + naga::proc::Validator::new().validate(module)?; + } + let shader = pipeline::ShaderModule { - raw: unsafe { device.raw.create_shader_module(spv).unwrap() }, + raw: unsafe { device.raw.create_shader_module(&spv).unwrap() }, device_id: Stored { value: device_id, ref_count: device.life_guard.add_ref(), @@ -1967,7 +1968,7 @@ impl Global { } None => {} }; - id + Ok(id) } pub fn shader_module_destroy(&self, shader_module_id: id::ShaderModuleId) { @@ -2259,7 +2260,7 @@ impl Global { module, &group_layouts, &entry_point_name, - ExecutionModel::Vertex, + naga::ShaderStage::Vertex, interface, ) .map_err(|error| pipeline::RenderPipelineError::Stage { flag, error })?; @@ -2286,7 +2287,7 @@ impl Global { module, &group_layouts, &entry_point_name, - ExecutionModel::Fragment, + naga::ShaderStage::Fragment, interface, ) .map_err(|error| { @@ -2486,7 +2487,7 @@ impl Global { module, &group_layouts, &entry_point_name, - ExecutionModel::GLCompute, + naga::ShaderStage::Compute, interface, ) .map_err(pipeline::ComputePipelineError::Stage)?; @@ -3012,3 +3013,9 @@ pub enum BufferMapError { #[derive(Clone, Debug, Error)] #[error("buffer is not mapped")] pub struct BufferNotMappedError; + +#[derive(Clone, Debug, Error)] +pub enum CreateShaderModuleError { + #[error(transparent)] + Validation(#[from] naga::proc::ValidationError), +} diff --git a/wgpu-core/src/pipeline.rs b/wgpu-core/src/pipeline.rs index 75113cbda6..215a9b5c3c 100644 --- a/wgpu-core/src/pipeline.rs +++ b/wgpu-core/src/pipeline.rs @@ -8,14 +8,14 @@ use crate::{ validation::StageError, LifeGuard, RefCount, Stored, }; -use std::borrow::Borrow; +use std::borrow::{Borrow, Cow}; use wgt::{BufferAddress, IndexFormat, InputStepMode}; #[repr(C)] #[derive(Debug)] pub enum ShaderModuleSource<'a> { - SpirV(&'a [u32]), - Wgsl(&'a str), + SpirV(Cow<'a, [u32]>), + Wgsl(Cow<'a, str>), Naga(naga::Module), } diff --git a/wgpu-core/src/swap_chain.rs b/wgpu-core/src/swap_chain.rs index ffd8b78611..8b43ff7f73 100644 --- a/wgpu-core/src/swap_chain.rs +++ b/wgpu-core/src/swap_chain.rs @@ -41,10 +41,7 @@ use crate::{ resource, span, LifeGuard, PrivateFeatures, Stored, SubmissionIndex, }; -use hal::{ - self, device::Device as _, queue::CommandQueue as _, - window::PresentationSurface as _, -}; +use hal::{self, device::Device as _, queue::CommandQueue as _, window::PresentationSurface as _}; use thiserror::Error; use wgt::{SwapChainDescriptor, SwapChainStatus}; @@ -129,12 +126,16 @@ impl Global { Err(err) => ( None, match err { - hal::window::AcquireError::OutOfMemory(_) => return Err(SwapChainError::OutOfMemory), + hal::window::AcquireError::OutOfMemory(_) => { + return Err(SwapChainError::OutOfMemory) + } hal::window::AcquireError::NotReady => unreachable!(), // we always set a timeout hal::window::AcquireError::Timeout => SwapChainStatus::Timeout, hal::window::AcquireError::OutOfDate => SwapChainStatus::Outdated, hal::window::AcquireError::SurfaceLost(_) => SwapChainStatus::Lost, - hal::window::AcquireError::DeviceLost(_) => return Err(SwapChainError::DeviceLost), + hal::window::AcquireError::DeviceLost(_) => { + return Err(SwapChainError::DeviceLost) + } }, ), }; diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index abe56ca484..faa9f5082f 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -3,7 +3,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use crate::{binding_model::BindEntryMap, FastHashMap}; -use spirv_headers as spirv; use thiserror::Error; use wgt::{BindGroupLayoutEntry, BindingType}; @@ -60,7 +59,10 @@ pub enum BindingError { #[error("buffer structure size {0}, added to one element of an unbound array, if it's the last field, ended up greater than the given `min_binding_size`")] WrongBufferSize(wgt::BufferAddress), #[error("view dimension {dim:?} (is array: {is_array}) doesn't match the shader")] - WrongTextureViewDimension { dim: spirv::Dim, is_array: bool }, + WrongTextureViewDimension { + dim: naga::ImageDimension, + is_array: bool, + }, #[error("component type {0:?} of a sampled texture doesn't match the shader")] WrongTextureComponentType(Option), #[error("texture sampling capability doesn't match the shader")] @@ -83,7 +85,7 @@ pub enum InputError { #[derive(Clone, Debug, Error)] pub enum StageError { #[error("unable to find an entry point matching the {0:?} execution model")] - MissingEntryPoint(spirv::ExecutionModel), + MissingEntryPoint(naga::ShaderStage), #[error("error matching global binding at index {binding} in set {set} against the pipeline layout: {error}")] Binding { set: u32, @@ -143,7 +145,14 @@ fn get_aligned_type_size( None => get_aligned_type_size(module, base, false), }, Ti::Struct { ref members } => members.last().map_or(0, |member| { - member.offset as wgt::BufferAddress + get_aligned_type_size(module, member.ty, false) + let offset = match member.origin { + naga::MemberOrigin::BuiltIn(_) => { + log::error!("Missing offset on a struct member"); + 0 // TODO: make it a proper error + } + naga::MemberOrigin::Offset(offset) => offset as wgt::BufferAddress, + }; + offset + get_aligned_type_size(module, member.ty, false) }), _ => panic!("Unexpected struct field"), } @@ -155,12 +164,7 @@ fn check_binding( entry: &BindGroupLayoutEntry, usage: naga::GlobalUse, ) -> Result<(), BindingError> { - let mut ty_inner = &module.types[var.ty].inner; - //TODO: change naga's IR to avoid a pointer here - if let naga::TypeInner::Pointer { base, class: _ } = *ty_inner { - ty_inner = &module.types[base].inner; - } - let allowed_usage = match *ty_inner { + let allowed_usage = match module.types[var.ty].inner { naga::TypeInner::Struct { ref members } => { let (allowed_usage, min_size) = match entry.ty { BindingType::UniformBuffer { @@ -224,8 +228,8 @@ fn check_binding( }; if flags.contains(naga::ImageFlags::ARRAYED) { match (dim, view_dimension) { - (spirv::Dim::Dim2D, wgt::TextureViewDimension::D2Array) => (), - (spirv::Dim::DimCube, wgt::TextureViewDimension::CubeArray) => (), + (naga::ImageDimension::D2, wgt::TextureViewDimension::D2Array) => (), + (naga::ImageDimension::Cube, wgt::TextureViewDimension::CubeArray) => (), _ => { return Err(BindingError::WrongTextureViewDimension { dim, @@ -235,10 +239,10 @@ fn check_binding( } } else { match (dim, view_dimension) { - (spirv::Dim::Dim1D, wgt::TextureViewDimension::D1) => (), - (spirv::Dim::Dim2D, wgt::TextureViewDimension::D2) => (), - (spirv::Dim::Dim3D, wgt::TextureViewDimension::D3) => (), - (spirv::Dim::DimCube, wgt::TextureViewDimension::Cube) => (), + (naga::ImageDimension::D1, wgt::TextureViewDimension::D1) => (), + (naga::ImageDimension::D2, wgt::TextureViewDimension::D2) => (), + (naga::ImageDimension::D3, wgt::TextureViewDimension::D3) => (), + (naga::ImageDimension::Cube, wgt::TextureViewDimension::Cube) => (), _ => { return Err(BindingError::WrongTextureViewDimension { dim, @@ -658,7 +662,7 @@ pub fn check_stage<'a>( module: &'a naga::Module, group_layouts: &[&BindEntryMap], entry_point_name: &str, - execution_model: spirv::ExecutionModel, + stage: naga::ShaderStage, inputs: StageInterface<'a>, ) -> Result, StageError> { // Since a shader module can have multiple entry points with the same name, @@ -666,16 +670,12 @@ pub fn check_stage<'a>( let entry_point = module .entry_points .iter() - .find(|entry_point| { - entry_point.name == entry_point_name && entry_point.exec_model == execution_model - }) - .ok_or(StageError::MissingEntryPoint(execution_model))?; - let stage_bit = match execution_model { - spirv::ExecutionModel::Vertex => wgt::ShaderStage::VERTEX, - spirv::ExecutionModel::Fragment => wgt::ShaderStage::FRAGMENT, - spirv::ExecutionModel::GLCompute => wgt::ShaderStage::COMPUTE, - // the entry point wouldn't match otherwise - _ => unreachable!(), + .find(|entry_point| entry_point.name == entry_point_name && entry_point.stage == stage) + .ok_or(StageError::MissingEntryPoint(stage))?; + let stage_bit = match stage { + naga::ShaderStage::Vertex => wgt::ShaderStage::VERTEX, + naga::ShaderStage::Fragment => wgt::ShaderStage::FRAGMENT, + naga::ShaderStage::Compute => wgt::ShaderStage::COMPUTE, }; let function = &module.functions[entry_point.function]; @@ -707,11 +707,7 @@ pub fn check_stage<'a>( } } Some(naga::Binding::Location(location)) => { - let mut ty = &module.types[var.ty].inner; - //TODO: change naga's IR to not have pointer for varyings - if let naga::TypeInner::Pointer { base, class: _ } = *ty { - ty = &module.types[base].inner; - } + let ty = &module.types[var.ty].inner; if usage.contains(naga::GlobalUse::STORE) { outputs.insert(location, MaybeOwned::Borrowed(ty)); } else {