Skip to content

Commit

Permalink
[rs] Merge gfx-rs#453
Browse files Browse the repository at this point in the history
453: Added Static Lifetime to Statically Loaded SPIR-V Modules r=kvark a=Andful

The commit enables `include_spirv!` to return a `ShaderModuleSource<'static>`.
This allows `ShaderModuleSource` to outlive the context in which `include_spirv!` was called in.
An example where this implementation would work but the previews would not is the following.
```rust
fn get_vertex_module<'a>() -> wgpu::ShaderModuleSource<'a> {
    wgpu::include_spirv!("shader.vert.spv")
}
let vs_module = device.create_shader_module(get_vertex_module());
```
Also it makes logical sense for a statically loaded module to have a static lifetime.  

The only downside that this change might bring is the redundant presence of the binary string (the non aligned binary string and the aligned binary string) but from the produced assembly I could only find one copy of the binary string so I don't think this is the case.

Co-authored-by: Andrea Nardi <[email protected]>
  • Loading branch information
bors[bot] and Andful authored Jul 20, 2020
2 parents 86a3089 + 3604988 commit 6bdd7cd
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 14 deletions.
4 changes: 2 additions & 2 deletions wgpu/src/backend/direct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -550,8 +550,8 @@ impl crate::Context for Context {
source: ShaderModuleSource,
) -> Self::ShaderModuleId {
let desc = match source {
ShaderModuleSource::SpirV(spv) => wgc::pipeline::ShaderModuleSource::SpirV(spv),
ShaderModuleSource::Wgsl(code) => wgc::pipeline::ShaderModuleSource::Wgsl(code),
ShaderModuleSource::SpirV(ref spv) => wgc::pipeline::ShaderModuleSource::SpirV(spv),
ShaderModuleSource::Wgsl(ref code) => wgc::pipeline::ShaderModuleSource::Wgsl(code),
};
gfx_select!(*device => self.device_create_shader_module(*device, desc, PhantomData))
}
Expand Down
2 changes: 1 addition & 1 deletion wgpu/src/backend/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,7 @@ impl crate::Context for Context {
) -> Self::ShaderModuleId {
let desc = match source {
ShaderModuleSource::SpirV(spv) => {
web_sys::GpuShaderModuleDescriptor::new(&js_sys::Uint32Array::from(spv))
web_sys::GpuShaderModuleDescriptor::new(&js_sys::Uint32Array::from(&*spv))
}
ShaderModuleSource::Wgsl(_code) => {
panic!("WGSL is not yet supported by the Web backend")
Expand Down
5 changes: 3 additions & 2 deletions wgpu/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub mod util;
mod macros;

use std::{
borrow::Cow,
error::Error,
fmt::{Debug, Display},
future::Future,
Expand Down Expand Up @@ -624,14 +625,14 @@ pub enum ShaderModuleSource<'a> {
///
/// wgpu will attempt to parse and validate it, but the original binary
/// is passed to `gfx-rs` and `spirv_cross` for translation.
SpirV(&'a [u32]),
SpirV(Cow<'a, [u32]>),
/// WGSL module as a string slice.
///
/// wgpu-rs will parse it and use for validation. It will attempt
/// to build a SPIR-V module internally and panic otherwise.
///
/// Note: WGSL is not yet supported on the Web.
Wgsl(&'a str),
Wgsl(Cow<'a, str>),
}

/// Handle to a pipeline layout.
Expand Down
2 changes: 1 addition & 1 deletion wgpu/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,6 @@ fn test_vertex_attr_array() {
#[macro_export]
macro_rules! include_spirv {
($($token:tt)*) => {
$crate::util::make_spirv(&$crate::util::WordAligned(*include_bytes!($($token)*)).0)
$crate::util::make_spirv(include_bytes!($($token)*))
};
}
35 changes: 27 additions & 8 deletions wgpu/src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,50 @@
mod belt;

use std::{
borrow::Cow,
mem::{align_of, size_of},
ptr::copy_nonoverlapping,
};

#[cfg(all(not(target_arch = "wasm32"), feature = "subscriber"))]
pub use wgc::logging::subscriber::{initialize_default_subscriber, ChromeTracingLayer};

pub use belt::StagingBelt;

/// Wrapper aligning contents to at least 4.
#[repr(align(4))]
pub struct WordAligned<Bytes: ?Sized>(pub Bytes);

/// Treat the given byte slice as a SPIR-V module.
///
/// # Panic
///
/// This function panics if:
///
/// - Input isn't aligned to 4 bytes
/// - Input length isn't multiple of 4
/// - Input is longer than [`usize::max_value`]
/// - SPIR-V magic number is missing from beginning of stream
pub fn make_spirv<'a>(data: &'a [u8]) -> super::ShaderModuleSource<'a> {
const MAGIC_NUMBER: u32 = 0x0723_0203;

let (pre, words, post) = unsafe { data.align_to::<u32>() };
assert_eq!(pre, &[], "data offset is not aligned to words!");
assert_eq!(post, &[], "data size is not aligned to words!");
assert_eq!(
data.len() % size_of::<u32>(),
0,
"data size is not a multiple of 4"
);

//If the data happens to be aligned, directly use the byte array,
// otherwise copy the byte array in an owned vector and use that instead.
let words = if data.as_ptr().align_offset(align_of::<u32>()) == 0 {
let (pre, words, post) = unsafe { data.align_to::<u32>() };
debug_assert_eq!(pre, &[]);
debug_assert_eq!(post, &[]);
Cow::from(words)
} else {
let mut words = vec![0u32; data.len() / size_of::<u32>()];
unsafe {
copy_nonoverlapping(data.as_ptr(), words.as_mut_ptr() as *mut u8, data.len());
}
Cow::from(words)
};

assert_eq!(
words[0], MAGIC_NUMBER,
"wrong magic word {:x}. Make sure you are using a binary SPIRV file.",
Expand Down

0 comments on commit 6bdd7cd

Please sign in to comment.