From 90938eb6aee07efad57d0967ba75fd5d43049136 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Mon, 7 Jun 2021 15:44:31 +0100 Subject: [PATCH 1/7] Update builder to use a shared validation method --- crates/spirv-builder/src/lib.rs | 48 ++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index f65b5f911e..42b933ab6f 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -244,14 +244,17 @@ impl SpirvBuilder { /// Builds the module. If `print_metadata` is [`MetadataPrintout::Full`], you usually don't have to inspect the path /// in the result, as the environment variable for the path to the module will already be set. - pub fn build(self) -> Result { - if (self.print_metadata == MetadataPrintout::Full) && self.multimodule { - return Err(SpirvBuilderError::MultiModuleWithPrintMetadata); - } - if !self.path_to_crate.is_dir() { - return Err(SpirvBuilderError::CratePathDoesntExist(self.path_to_crate)); - } + pub fn build(mut self) -> Result { + self.validate_running_conditions()?; let metadata_file = invoke_rustc(&self)?; + match self.print_metadata { + MetadataPrintout::Full | MetadataPrintout::DependencyOnly => { + leaf_deps(&metadata_file, |artifact| { + println!("cargo:rerun-if-changed={}", artifact) + }); + } + MetadataPrintout::None => (), + } let metadata_contents = File::open(&metadata_file).map_err(SpirvBuilderError::MetadataFileMissing)?; let metadata: CompileResult = serde_json::from_reader(BufReader::new(metadata_contents)) @@ -270,6 +273,18 @@ impl SpirvBuilder { } Ok(metadata) } + + pub(crate) fn validate_running_conditions(&mut self) -> Result<(), SpirvBuilderError> { + if (self.print_metadata == MetadataPrintout::Full) && self.multimodule { + return Err(SpirvBuilderError::MultiModuleWithPrintMetadata); + } + if !self.path_to_crate.is_dir() { + return Err(SpirvBuilderError::CratePathDoesntExist(std::mem::take( + &mut self.path_to_crate, + ))); + } + Ok(()) + } } // https://github.com/rust-lang/cargo/blob/1857880b5124580c4aeb4e8bc5f1198f491d61b1/src/cargo/util/paths.rs#L29-L52 @@ -426,10 +441,6 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { let artifact = get_last_artifact(&stdout); if build.status.success() { - match builder.print_metadata { - MetadataPrintout::Full | MetadataPrintout::DependencyOnly => print_deps_of(&artifact), - MetadataPrintout::None => (), - } Ok(artifact) } else { Err(SpirvBuilderError::BuildFailed) @@ -467,7 +478,8 @@ fn get_last_artifact(out: &str) -> PathBuf { filename.into() } -fn print_deps_of(artifact: &Path) { +/// Internally iterate through the leaf dependencies of the artifact at `artifact` +fn leaf_deps(artifact: &Path, handle: impl FnMut(&RawStr)) { let deps_file = artifact.with_extension("d"); let mut deps_map = HashMap::new(); depfile::read_deps_file(&deps_file, |item, deps| { @@ -475,15 +487,19 @@ fn print_deps_of(artifact: &Path) { Ok(()) }) .expect("Could not read dep file"); - fn recurse(map: &HashMap>, artifact: &RawStr) { + fn recurse( + map: &HashMap>, + artifact: &RawStr, + handle: &mut impl FnMut(&RawStr), + ) { match map.get(artifact) { Some(entries) => { for entry in entries { - recurse(map, entry) + recurse(map, entry, handle) } } - None => println!("cargo:rerun-if-changed={}", artifact), + None => handle(artifact), } } - recurse(&deps_map, artifact.to_str().unwrap().into()); + recurse(&deps_map, artifact.to_str().unwrap().into(), &mut handle); } From 35fb3f45cfae645ac60f1e19d75fa1fc9187dc85 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Mon, 7 Jun 2021 15:46:42 +0100 Subject: [PATCH 2/7] Add the error for using print_metadata and watching We cannot use print_metadata with watching because print_metadata only makes sense in build scripts, but watching does not and would instead stall the script --- crates/spirv-builder/src/lib.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 42b933ab6f..9d34191851 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -71,10 +71,12 @@ pub use rustc_codegen_spirv::rspirv::spirv::Capability; pub use rustc_codegen_spirv::{CompileResult, ModuleResult}; #[derive(Debug)] +#[non_exhaustive] pub enum SpirvBuilderError { CratePathDoesntExist(PathBuf), BuildFailed, MultiModuleWithPrintMetadata, + WatchWithPrintMetadata, MetadataFileMissing(std::io::Error), MetadataFileMalformed(serde_json::Error), } @@ -89,6 +91,9 @@ impl fmt::Display for SpirvBuilderError { SpirvBuilderError::MultiModuleWithPrintMetadata => f.write_str( "Multi-module build cannot be used with print_metadata = MetadataPrintout::Full", ), + SpirvBuilderError::WatchWithPrintMetadata => { + f.write_str("Watching within build scripts will prevent build completion") + } SpirvBuilderError::MetadataFileMissing(_) => { f.write_str("Multi-module metadata file missing") } From 32cfe8806b7d110cf024b95d141b37c25358c65c Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Tue, 8 Jun 2021 10:58:46 +0100 Subject: [PATCH 3/7] Add the initial implementation of watching --- Cargo.lock | 73 +++++++++++++++++++- crates/spirv-builder/Cargo.toml | 3 + crates/spirv-builder/src/lib.rs | 50 ++++++++------ crates/spirv-builder/src/watch.rs | 106 ++++++++++++++++++++++++++++++ 4 files changed, 211 insertions(+), 21 deletions(-) create mode 100644 crates/spirv-builder/src/watch.rs diff --git a/Cargo.lock b/Cargo.lock index 8020e93798..73eef7bd18 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -841,6 +841,15 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "00b0228411908ca8685dba7fc2cdd70ec9990a6e753e89b6ac91a84c40fbaf4b" +[[package]] +name = "fsevent-sys" +version = "4.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c0e564d24da983c053beff1bb7178e237501206840a3e6bf4e267b9e8ae734a" +dependencies = [ + "libc", +] + [[package]] name = "fuchsia-zircon" version = "0.3.3" @@ -1227,6 +1236,26 @@ dependencies = [ "hashbrown", ] +[[package]] +name = "inotify" +version = "0.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b031475cb1b103ee221afb806a23d35e0570bf7271d7588762ceba8127ed43b3" +dependencies = [ + "bitflags", + "inotify-sys", + "libc", +] + +[[package]] +name = "inotify-sys" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e05c02b5e89bff3b946cedeca278abc628fe811e604f027c45a8aa3cf793d0eb" +dependencies = [ + "libc", +] + [[package]] name = "inplace_it" version = "0.3.3" @@ -1485,6 +1514,19 @@ dependencies = [ "winapi 0.2.8", ] +[[package]] +name = "mio" +version = "0.7.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf80d3e903b34e0bd7282b218398aec54e082c840d9baf8339e0080a0c542956" +dependencies = [ + "libc", + "log", + "miow 0.3.7", + "ntapi", + "winapi 0.3.9", +] + [[package]] name = "mio-extras" version = "2.0.6" @@ -1493,7 +1535,7 @@ checksum = "52403fe290012ce777c4626790c8951324a2b9e3316b3143779c72b029742f19" dependencies = [ "lazycell", "log", - "mio", + "mio 0.6.23", "slab", ] @@ -1649,6 +1691,32 @@ dependencies = [ "version_check", ] +[[package]] +name = "notify" +version = "5.0.0-pre.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "51f18203a26893ca1d3526cf58084025d5639f91c44f8b70ab3b724f60e819a0" +dependencies = [ + "bitflags", + "crossbeam-channel", + "filetime", + "fsevent-sys", + "inotify", + "libc", + "mio 0.7.11", + "walkdir", + "winapi 0.3.9", +] + +[[package]] +name = "ntapi" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f6bb902e437b6d86e03cce10a7e2af662292c5dfef23b65899ea3ac9354ad44" +dependencies = [ + "winapi 0.3.9", +] + [[package]] name = "num-integer" version = "0.1.44" @@ -2338,6 +2406,7 @@ name = "spirv-builder" version = "0.4.0-alpha.9" dependencies = [ "memchr", + "notify", "raw-string", "rustc_codegen_spirv", "serde", @@ -3074,7 +3143,7 @@ dependencies = [ "lazy_static", "libc", "log", - "mio", + "mio 0.6.23", "mio-extras", "ndk", "ndk-glue", diff --git a/crates/spirv-builder/Cargo.toml b/crates/spirv-builder/Cargo.toml index 5f94ad1b36..5f5c2b7655 100644 --- a/crates/spirv-builder/Cargo.toml +++ b/crates/spirv-builder/Cargo.toml @@ -10,6 +10,7 @@ license = "MIT OR Apache-2.0" default = ["use-compiled-tools"] use-installed-tools = ["rustc_codegen_spirv/use-installed-tools"] use-compiled-tools = ["rustc_codegen_spirv/use-compiled-tools"] +watch = ["notify"] [dependencies] memchr = "2.3" @@ -18,3 +19,5 @@ serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" # See comment in lib.rs invoke_rustc for why this is here rustc_codegen_spirv = { path = "../rustc_codegen_spirv", default-features = false } + +notify = { version = "5.0.0-pre.10", optional = true } diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 9d34191851..0b0776a341 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -54,6 +54,8 @@ #![allow()] mod depfile; +#[cfg(feature = "watch")] +mod watch; use raw_string::{RawStr, RawString}; use serde::Deserialize; @@ -256,18 +258,40 @@ impl SpirvBuilder { MetadataPrintout::Full | MetadataPrintout::DependencyOnly => { leaf_deps(&metadata_file, |artifact| { println!("cargo:rerun-if-changed={}", artifact) - }); + }) + // Close enough + .map_err(SpirvBuilderError::MetadataFileMissing)?; } MetadataPrintout::None => (), } - let metadata_contents = - File::open(&metadata_file).map_err(SpirvBuilderError::MetadataFileMissing)?; + let metadata = self.parse_metadata_file(&metadata_file)?; + + Ok(metadata) + } + + pub(crate) fn validate_running_conditions(&mut self) -> Result<(), SpirvBuilderError> { + if (self.print_metadata == MetadataPrintout::Full) && self.multimodule { + return Err(SpirvBuilderError::MultiModuleWithPrintMetadata); + } + if !self.path_to_crate.is_dir() { + return Err(SpirvBuilderError::CratePathDoesntExist(std::mem::take( + &mut self.path_to_crate, + ))); + } + Ok(()) + } + + pub(crate) fn parse_metadata_file( + &self, + at: &Path, + ) -> Result { + let metadata_contents = File::open(&at).map_err(SpirvBuilderError::MetadataFileMissing)?; let metadata: CompileResult = serde_json::from_reader(BufReader::new(metadata_contents)) .map_err(SpirvBuilderError::MetadataFileMalformed)?; match &metadata.module { ModuleResult::SingleModule(spirv_module) => { assert!(!self.multimodule); - let env_var = metadata_file.file_name().unwrap().to_str().unwrap(); + let env_var = at.file_name().unwrap().to_str().unwrap(); if self.print_metadata == MetadataPrintout::Full { println!("cargo:rustc-env={}={}", env_var, spirv_module.display()); } @@ -278,18 +302,6 @@ impl SpirvBuilder { } Ok(metadata) } - - pub(crate) fn validate_running_conditions(&mut self) -> Result<(), SpirvBuilderError> { - if (self.print_metadata == MetadataPrintout::Full) && self.multimodule { - return Err(SpirvBuilderError::MultiModuleWithPrintMetadata); - } - if !self.path_to_crate.is_dir() { - return Err(SpirvBuilderError::CratePathDoesntExist(std::mem::take( - &mut self.path_to_crate, - ))); - } - Ok(()) - } } // https://github.com/rust-lang/cargo/blob/1857880b5124580c4aeb4e8bc5f1198f491d61b1/src/cargo/util/paths.rs#L29-L52 @@ -484,14 +496,13 @@ fn get_last_artifact(out: &str) -> PathBuf { } /// Internally iterate through the leaf dependencies of the artifact at `artifact` -fn leaf_deps(artifact: &Path, handle: impl FnMut(&RawStr)) { +fn leaf_deps(artifact: &Path, mut handle: impl FnMut(&RawStr)) -> std::io::Result<()> { let deps_file = artifact.with_extension("d"); let mut deps_map = HashMap::new(); depfile::read_deps_file(&deps_file, |item, deps| { deps_map.insert(item, deps); Ok(()) - }) - .expect("Could not read dep file"); + })?; fn recurse( map: &HashMap>, artifact: &RawStr, @@ -507,4 +518,5 @@ fn leaf_deps(artifact: &Path, handle: impl FnMut(&RawStr)) { } } recurse(&deps_map, artifact.to_str().unwrap().into(), &mut handle); + Ok(()) } diff --git a/crates/spirv-builder/src/watch.rs b/crates/spirv-builder/src/watch.rs new file mode 100644 index 0000000000..c5fd97aa19 --- /dev/null +++ b/crates/spirv-builder/src/watch.rs @@ -0,0 +1,106 @@ +use std::{collections::HashSet, sync::mpsc::sync_channel}; + +use notify::{Event, RecursiveMode, Watcher}; +use rustc_codegen_spirv::CompileResult; + +use crate::{leaf_deps, SpirvBuilder, SpirvBuilderError}; + +impl SpirvBuilder { + /// Watches the module for changes using [`notify`](https://crates.io/crates/notify). + /// + /// This is a blocking operation, wand should never return in the happy path + pub fn watch( + mut self, + on_compilation_finishes: impl Fn(CompileResult), + ) -> Result<(), SpirvBuilderError> { + self.validate_running_conditions()?; + if !matches!(self.print_metadata, crate::MetadataPrintout::None) { + return Err(SpirvBuilderError::WatchWithPrintMetadata); + } + let metadata_result = crate::invoke_rustc(&self); + // Load the dependencies of the thing + let metadata_file = match metadata_result { + Ok(path) => path, + Err(_) => { + let (tx, rx) = sync_channel(0); + // Fall back to watching from the crate root if the inital compilation fails + let mut watcher = + notify::immediate_watcher(move |event: notify::Result| match event { + Ok(e) => match e.kind { + notify::EventKind::Access(_) => (), + notify::EventKind::Any + | notify::EventKind::Create(_) + | notify::EventKind::Modify(_) + | notify::EventKind::Remove(_) + | notify::EventKind::Other => { + let _ = tx.try_send(()); + } + }, + Err(e) => println!("notify error: {:?}", e), + }) + .expect("Could create watcher"); + // This is likely to notice changes in the `target` dir, however, given that `cargo watch` doesn't seem to handle that, + watcher + .watch(&self.path_to_crate, RecursiveMode::Recursive) + .expect("Could watch crate root"); + loop { + rx.recv().expect("Watcher still alive"); + let metadata_file = crate::invoke_rustc(&self); + match metadata_file { + Ok(f) => break f, + Err(_) => (), // Continue the loop until compilation succeeds + } + } + } + }; + let metadata = self.parse_metadata_file(&metadata_file)?; + on_compilation_finishes(metadata); + let mut watched_paths = HashSet::new(); + let (tx, rx) = sync_channel(0); + let mut watcher = + notify::immediate_watcher(move |event: notify::Result| match event { + Ok(e) => match e.kind { + notify::EventKind::Access(_) => (), + notify::EventKind::Any + | notify::EventKind::Create(_) + | notify::EventKind::Modify(_) + | notify::EventKind::Remove(_) + | notify::EventKind::Other => { + let _ = tx.try_send(()); + } + }, + Err(e) => println!("notify error: {:?}", e), + }) + .expect("Could create watcher"); + leaf_deps(&metadata_file, |it| { + let path = it.to_path().unwrap(); + if watched_paths.insert(path.to_owned()) { + watcher + .watch(it.to_path().unwrap(), RecursiveMode::NonRecursive) + .expect("Cargo dependencies are valid files"); + } + }) + .expect("Could read dependencies file"); + loop { + rx.recv().expect("Watcher still alive"); + let metadata_result = crate::invoke_rustc(&self); + match metadata_result { + Ok(file) => { + // We can bubble this error up because it's an internal error (e.g. rustc_codegen_spirv's version of CompileResult is somehow out of sync) + let metadata = self.parse_metadata_file(&file)?; + leaf_deps(&file, |it| { + let path = it.to_path().unwrap(); + if watched_paths.insert(path.to_owned()) { + watcher + .watch(it.to_path().unwrap(), RecursiveMode::NonRecursive) + .expect("Cargo dependencies are valid files"); + } + }) + .expect("Could read dependencies file"); + on_compilation_finishes(metadata); + } + Err(_) => (), // Continue until compilation succeeds + } + } + } +} From 21199e7dbaadf9155fd68204de13d864c86acf68 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Tue, 8 Jun 2021 20:53:40 +0100 Subject: [PATCH 4/7] Make hot reloading work in the wgpu example --- crates/spirv-builder/src/lib.rs | 3 +- crates/spirv-builder/src/watch.rs | 2 + examples/runners/wgpu/Cargo.toml | 2 +- examples/runners/wgpu/src/compute.rs | 5 +- examples/runners/wgpu/src/graphics.rs | 118 +++++++++++++++++--------- examples/runners/wgpu/src/lib.rs | 72 +++++++++++----- 6 files changed, 137 insertions(+), 65 deletions(-) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 0b0776a341..f684c782f6 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -455,9 +455,8 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { // we do that even in case of an error, to let through any useful messages // that ended up on stdout instead of stderr. let stdout = String::from_utf8(build.stdout).unwrap(); - let artifact = get_last_artifact(&stdout); - if build.status.success() { + let artifact = get_last_artifact(&stdout); Ok(artifact) } else { Err(SpirvBuilderError::BuildFailed) diff --git a/crates/spirv-builder/src/watch.rs b/crates/spirv-builder/src/watch.rs index c5fd97aa19..99755acae4 100644 --- a/crates/spirv-builder/src/watch.rs +++ b/crates/spirv-builder/src/watch.rs @@ -88,6 +88,7 @@ impl SpirvBuilder { Ok(file) => { // We can bubble this error up because it's an internal error (e.g. rustc_codegen_spirv's version of CompileResult is somehow out of sync) let metadata = self.parse_metadata_file(&file)?; + leaf_deps(&file, |it| { let path = it.to_path().unwrap(); if watched_paths.insert(path.to_owned()) { @@ -97,6 +98,7 @@ impl SpirvBuilder { } }) .expect("Could read dependencies file"); + on_compilation_finishes(metadata); } Err(_) => (), // Continue until compilation succeeds diff --git a/examples/runners/wgpu/Cargo.toml b/examples/runners/wgpu/Cargo.toml index 9ba3e6cc44..f76338526b 100644 --- a/examples/runners/wgpu/Cargo.toml +++ b/examples/runners/wgpu/Cargo.toml @@ -25,7 +25,7 @@ clap = "3.0.0-beta.2" strum = { version = "0.20", default_features = false, features = ["derive"] } [target.'cfg(not(any(target_os = "android", target_arch = "wasm32")))'.dependencies] -spirv-builder = { path = "../../../crates/spirv-builder", default-features = false } +spirv-builder = { path = "../../../crates/spirv-builder", default-features = false, features = ["watch"] } [target.'cfg(target_os = "android")'.dependencies] ndk-glue = "0.2" diff --git a/examples/runners/wgpu/src/compute.rs b/examples/runners/wgpu/src/compute.rs index 64fd90ec72..30052e57e1 100644 --- a/examples/runners/wgpu/src/compute.rs +++ b/examples/runners/wgpu/src/compute.rs @@ -1,6 +1,6 @@ use wgpu::util::DeviceExt; -use super::{shader_module, Options}; +use super::Options; use futures::future::join; use std::{convert::TryInto, future::Future, num::NonZeroU64, time::Duration}; @@ -15,7 +15,8 @@ fn block_on(future: impl Future) -> T { } pub fn start(options: &Options) { - let shader_binary = shader_module(options.shader); + let rx = crate::maybe_watch(options.shader, true); + let shader_binary = rx.recv().expect("Should send one binary"); block_on(start_internal(options, shader_binary)) } diff --git a/examples/runners/wgpu/src/graphics.rs b/examples/runners/wgpu/src/graphics.rs index d0965dcce9..fce9a9a8c8 100644 --- a/examples/runners/wgpu/src/graphics.rs +++ b/examples/runners/wgpu/src/graphics.rs @@ -1,4 +1,8 @@ -use super::{shader_module, Options}; +use std::thread::spawn; + +use crate::maybe_watch; + +use super::Options; use shared::ShaderConstants; use winit::{ event::{ElementState, Event, KeyboardInput, MouseButton, VirtualKeyCode, WindowEvent}, @@ -35,10 +39,10 @@ fn mouse_button_index(button: MouseButton) -> usize { } async fn run( - event_loop: EventLoop<()>, + event_loop: EventLoop>, window: Window, swapchain_format: wgpu::TextureFormat, - shader_binary: wgpu::ShaderModuleDescriptor<'_>, + shader_binary: wgpu::ShaderModuleDescriptor<'static>, ) { let size = window.inner_size(); let instance = wgpu::Instance::new(wgpu::BackendBit::VULKAN | wgpu::BackendBit::METAL); @@ -80,7 +84,6 @@ async fn run( .expect("Failed to create device"); // Load the shaders from disk - let module = device.create_shader_module(&shader_binary); let pipeline_layout = device.create_pipeline_layout(&wgpu::PipelineLayoutDescriptor { label: None, @@ -91,38 +94,8 @@ async fn run( }], }); - let render_pipeline = device.create_render_pipeline(&wgpu::RenderPipelineDescriptor { - label: None, - layout: Some(&pipeline_layout), - vertex: wgpu::VertexState { - module: &module, - entry_point: shaders::main_vs, - buffers: &[], - }, - primitive: wgpu::PrimitiveState { - topology: wgpu::PrimitiveTopology::TriangleList, - strip_index_format: None, - front_face: wgpu::FrontFace::Ccw, - cull_mode: wgpu::CullMode::None, - polygon_mode: wgpu::PolygonMode::Fill, - }, - depth_stencil: None, - multisample: wgpu::MultisampleState { - count: 1, - mask: !0, - alpha_to_coverage_enabled: false, - }, - fragment: Some(wgpu::FragmentState { - module: &module, - entry_point: shaders::main_fs, - targets: &[wgpu::ColorTargetState { - format: swapchain_format, - alpha_blend: wgpu::BlendState::REPLACE, - color_blend: wgpu::BlendState::REPLACE, - write_mask: wgpu::ColorWrite::ALL, - }], - }), - }); + let mut render_pipeline = + create_pipeline(&device, &pipeline_layout, swapchain_format, shader_binary); let mut sc_desc = wgpu::SwapChainDescriptor { usage: wgpu::TextureUsage::RENDER_ATTACHMENT, @@ -149,7 +122,8 @@ async fn run( // Have the closure take ownership of the resources. // `event_loop.run` never returns, therefore we must do this to ensure // the resources are properly cleaned up. - let _ = (&instance, &adapter, &module, &pipeline_layout); + let _ = (&instance, &adapter, &pipeline_layout); + let render_pipeline = &mut render_pipeline; *control_flow = ControlFlow::Wait; match event { @@ -282,16 +256,78 @@ async fn run( drag_end_y = cursor_y; } } + Event::UserEvent(new_module) => { + *render_pipeline = + create_pipeline(&device, &pipeline_layout, swapchain_format, new_module); + window.request_redraw(); + *control_flow = ControlFlow::Poll; + } _ => {} } }); } +fn create_pipeline( + device: &wgpu::Device, + pipeline_layout: &wgpu::PipelineLayout, + swapchain_format: wgpu::TextureFormat, + shader_binary: wgpu::ShaderModuleDescriptor<'_>, +) -> wgpu::RenderPipeline { + let module = device.create_shader_module(&shader_binary); + let render_pipeline = device.create_render_pipeline(&wgpu::RenderPipelineDescriptor { + label: None, + layout: Some(pipeline_layout), + vertex: wgpu::VertexState { + module: &module, + entry_point: shaders::main_vs, + buffers: &[], + }, + primitive: wgpu::PrimitiveState { + topology: wgpu::PrimitiveTopology::TriangleList, + strip_index_format: None, + front_face: wgpu::FrontFace::Ccw, + cull_mode: wgpu::CullMode::None, + polygon_mode: wgpu::PolygonMode::Fill, + }, + depth_stencil: None, + multisample: wgpu::MultisampleState { + count: 1, + mask: !0, + alpha_to_coverage_enabled: false, + }, + fragment: Some(wgpu::FragmentState { + module: &module, + entry_point: shaders::main_fs, + targets: &[wgpu::ColorTargetState { + format: swapchain_format, + alpha_blend: wgpu::BlendState::REPLACE, + color_blend: wgpu::BlendState::REPLACE, + write_mask: wgpu::ColorWrite::ALL, + }], + }), + }); + render_pipeline +} + pub fn start(options: &Options) { // Build the shader before we pop open a window, since it might take a while. - let shader_binary = shader_module(options.shader); + let rx = maybe_watch(options.shader, false); + let initial_shader = rx.recv().expect("Initial shader is required"); - let event_loop = EventLoop::new(); + let event_loop = EventLoop::with_user_event(); + let proxy = event_loop.create_proxy(); + let thread = spawn(move || loop { + match rx.recv() { + Ok(result) => match proxy.send_event(result) { + Ok(()) => {} + // If something goes wrong, close this thread + Err(_) => break, + }, + // This will occur if we can't watch (e.g. on android) + Err(_) => break, + } + }); + std::mem::forget(thread); let window = winit::window::WindowBuilder::new() .with_title("Rust GPU - wgpu") .with_inner_size(winit::dpi::LogicalSize::new(1280.0, 720.0)) @@ -317,6 +353,7 @@ pub fn start(options: &Options) { event_loop, window, wgpu::TextureFormat::Bgra8Unorm, + initial_shader, )); } else { wgpu_subscriber::initialize_default_subscriber(None); @@ -328,7 +365,8 @@ pub fn start(options: &Options) { } else { wgpu::TextureFormat::Bgra8UnormSrgb }, - shader_binary, + initial_shader, + )); } } diff --git a/examples/runners/wgpu/src/lib.rs b/examples/runners/wgpu/src/lib.rs index 09b0d108a5..e0119a99a1 100644 --- a/examples/runners/wgpu/src/lib.rs +++ b/examples/runners/wgpu/src/lib.rs @@ -42,7 +42,10 @@ rust_2018_idioms )] +use std::sync::mpsc::{self, Receiver, SyncSender}; + use clap::Clap; +use spirv_builder::CompileResult; use strum::{Display, EnumString}; mod compute; @@ -56,7 +59,12 @@ pub enum RustGPUShader { Mouse, } -fn shader_module(shader: RustGPUShader) -> wgpu::ShaderModuleDescriptor<'static> { +fn maybe_watch( + shader: RustGPUShader, + force_no_watch: bool, +) -> Receiver> { + // This bound needs to be 1, because when we don't watch, we + let (tx, rx) = mpsc::sync_channel(1); #[cfg(not(any(target_os = "android", target_arch = "wasm32")))] { use spirv_builder::{Capability, MetadataPrintout, SpirvBuilder}; @@ -86,29 +94,53 @@ fn shader_module(shader: RustGPUShader) -> wgpu::ShaderModuleDescriptor<'static> for &cap in capabilities { builder = builder.capability(cap); } - let compile_result = builder.build().unwrap(); - let module_path = compile_result.module.unwrap_single(); - let data = std::fs::read(module_path).unwrap(); - let spirv = wgpu::util::make_spirv(&data); - let spirv = match spirv { - wgpu::ShaderSource::Wgsl(cow) => wgpu::ShaderSource::Wgsl(Cow::Owned(cow.into_owned())), - wgpu::ShaderSource::SpirV(cow) => { - wgpu::ShaderSource::SpirV(Cow::Owned(cow.into_owned())) - } - }; - wgpu::ShaderModuleDescriptor { - label: None, - source: spirv, - flags: wgpu::ShaderFlags::default(), + if force_no_watch { + let compile_result = builder.build().unwrap(); + handle_builder_result(compile_result, &tx); + } else { + let thread = std::thread::spawn(move || { + builder + .watch(|compile_result| { + handle_builder_result(compile_result, &tx); + }) + .expect("Configuration is correct for watching") + }); + std::mem::forget(thread); + } + fn handle_builder_result( + compile_result: CompileResult, + tx: &SyncSender>, + ) { + let module_path = compile_result.module.unwrap_single(); + let data = std::fs::read(module_path).unwrap(); + let spirv = wgpu::util::make_spirv(&data); + let spirv = match spirv { + wgpu::ShaderSource::Wgsl(cow) => { + wgpu::ShaderSource::Wgsl(Cow::Owned(cow.into_owned())) + } + wgpu::ShaderSource::SpirV(cow) => { + wgpu::ShaderSource::SpirV(Cow::Owned(cow.into_owned())) + } + }; + tx.send(wgpu::ShaderModuleDescriptor { + label: None, + source: spirv, + flags: wgpu::ShaderFlags::default(), + }) + .expect("Rx is still alive"); } } #[cfg(any(target_os = "android", target_arch = "wasm32"))] - match shader { - RustGPUShader::Simplest => wgpu::include_spirv!(env!("simplest_shader.spv")), - RustGPUShader::Sky => wgpu::include_spirv!(env!("sky_shader.spv")), - RustGPUShader::Compute => wgpu::include_spirv!(env!("compute_shader.spv")), - RustGPUShader::Mouse => wgpu::include_spirv!(env!("mouse_shader.spv")), + { + tx.send(match shader { + RustGPUShader::Simplest => wgpu::include_spirv!(env!("simplest_shader.spv")), + RustGPUShader::Sky => wgpu::include_spirv!(env!("sky_shader.spv")), + RustGPUShader::Compute => wgpu::include_spirv!(env!("compute_shader.spv")), + RustGPUShader::Mouse => wgpu::include_spirv!(env!("mouse_shader.spv")), + }) + .expect("rx to be alive") } + rx } fn is_compute_shader(shader: RustGPUShader) -> bool { From 64f63629a8dcb5563a056d30860819612b0f5885 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Tue, 8 Jun 2021 21:26:49 +0100 Subject: [PATCH 5/7] Attempt to address CI failures --- crates/spirv-builder/src/watch.rs | 34 ++++++++++++--------------- examples/runners/wgpu/src/graphics.rs | 15 +++++------- examples/runners/wgpu/src/lib.rs | 7 +++--- 3 files changed, 24 insertions(+), 32 deletions(-) diff --git a/crates/spirv-builder/src/watch.rs b/crates/spirv-builder/src/watch.rs index 99755acae4..8ed8e09980 100644 --- a/crates/spirv-builder/src/watch.rs +++ b/crates/spirv-builder/src/watch.rs @@ -46,9 +46,8 @@ impl SpirvBuilder { loop { rx.recv().expect("Watcher still alive"); let metadata_file = crate::invoke_rustc(&self); - match metadata_file { - Ok(f) => break f, - Err(_) => (), // Continue the loop until compilation succeeds + if let Ok(f) = metadata_file { + break f; } } } @@ -84,24 +83,21 @@ impl SpirvBuilder { loop { rx.recv().expect("Watcher still alive"); let metadata_result = crate::invoke_rustc(&self); - match metadata_result { - Ok(file) => { - // We can bubble this error up because it's an internal error (e.g. rustc_codegen_spirv's version of CompileResult is somehow out of sync) - let metadata = self.parse_metadata_file(&file)?; + if let Ok(file) = metadata_result { + // We can bubble this error up because it's an internal error (e.g. rustc_codegen_spirv's version of CompileResult is somehow out of sync) + let metadata = self.parse_metadata_file(&file)?; - leaf_deps(&file, |it| { - let path = it.to_path().unwrap(); - if watched_paths.insert(path.to_owned()) { - watcher - .watch(it.to_path().unwrap(), RecursiveMode::NonRecursive) - .expect("Cargo dependencies are valid files"); - } - }) - .expect("Could read dependencies file"); + leaf_deps(&file, |it| { + let path = it.to_path().unwrap(); + if watched_paths.insert(path.to_owned()) { + watcher + .watch(it.to_path().unwrap(), RecursiveMode::NonRecursive) + .expect("Cargo dependencies are valid files"); + } + }) + .expect("Could read dependencies file"); - on_compilation_finishes(metadata); - } - Err(_) => (), // Continue until compilation succeeds + on_compilation_finishes(metadata); } } } diff --git a/examples/runners/wgpu/src/graphics.rs b/examples/runners/wgpu/src/graphics.rs index fce9a9a8c8..dc4bd3f250 100644 --- a/examples/runners/wgpu/src/graphics.rs +++ b/examples/runners/wgpu/src/graphics.rs @@ -199,7 +199,7 @@ async fn run( mouse_button_press_time, }; - rpass.set_pipeline(&render_pipeline); + rpass.set_pipeline(render_pipeline); rpass.set_push_constants(wgpu::ShaderStage::all(), 0, unsafe { any_as_u8_slice(&push_constants) }); @@ -274,7 +274,7 @@ fn create_pipeline( shader_binary: wgpu::ShaderModuleDescriptor<'_>, ) -> wgpu::RenderPipeline { let module = device.create_shader_module(&shader_binary); - let render_pipeline = device.create_render_pipeline(&wgpu::RenderPipelineDescriptor { + device.create_render_pipeline(&wgpu::RenderPipelineDescriptor { label: None, layout: Some(pipeline_layout), vertex: wgpu::VertexState { @@ -305,8 +305,7 @@ fn create_pipeline( write_mask: wgpu::ColorWrite::ALL, }], }), - }); - render_pipeline + }) } pub fn start(options: &Options) { @@ -317,14 +316,12 @@ pub fn start(options: &Options) { let event_loop = EventLoop::with_user_event(); let proxy = event_loop.create_proxy(); let thread = spawn(move || loop { - match rx.recv() { - Ok(result) => match proxy.send_event(result) { + while let Ok(result) = rx.recv() { + match proxy.send_event(result) { Ok(()) => {} // If something goes wrong, close this thread Err(_) => break, - }, - // This will occur if we can't watch (e.g. on android) - Err(_) => break, + } } }); std::mem::forget(thread); diff --git a/examples/runners/wgpu/src/lib.rs b/examples/runners/wgpu/src/lib.rs index e0119a99a1..2cc59d4e38 100644 --- a/examples/runners/wgpu/src/lib.rs +++ b/examples/runners/wgpu/src/lib.rs @@ -42,10 +42,9 @@ rust_2018_idioms )] -use std::sync::mpsc::{self, Receiver, SyncSender}; +use std::sync::mpsc::{self, Receiver}; use clap::Clap; -use spirv_builder::CompileResult; use strum::{Display, EnumString}; mod compute; @@ -67,7 +66,7 @@ fn maybe_watch( let (tx, rx) = mpsc::sync_channel(1); #[cfg(not(any(target_os = "android", target_arch = "wasm32")))] { - use spirv_builder::{Capability, MetadataPrintout, SpirvBuilder}; + use spirv_builder::{Capability, CompileResult, MetadataPrintout, SpirvBuilder}; use std::borrow::Cow; use std::path::PathBuf; // Hack: spirv_builder builds into a custom directory if running under cargo, to not @@ -109,7 +108,7 @@ fn maybe_watch( } fn handle_builder_result( compile_result: CompileResult, - tx: &SyncSender>, + tx: &mpsc::SyncSender>, ) { let module_path = compile_result.module.unwrap_single(); let data = std::fs::read(module_path).unwrap(); From 2f111d9b89ee13d06e9a9b4cf581af32b93c272b Mon Sep 17 00:00:00 2001 From: khyperia Date: Wed, 9 Jun 2021 09:44:03 +0200 Subject: [PATCH 6/7] Add exception for notify CC0-1.0 license --- deny.toml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/deny.toml b/deny.toml index 2b7e516c5a..721cbc925c 100644 --- a/deny.toml +++ b/deny.toml @@ -69,6 +69,11 @@ exceptions = [ # https://tldrlegal.com/license/do-what-the-f*ck-you-want-to-public-license-(wtfpl) { allow = ["WTFPL"], name = "xkb" }, { allow = ["WTFPL"], name = "xkbcommon-sys" }, + + # CC0 is a permissive license but somewhat unclear status for source code + # so we prefer to not have dependencies using it + # https://tldrlegal.com/license/creative-commons-cc0-1.0-universal + { allow = ["CC0-1.0"], name = "notify" }, ] copyleft = "deny" From edbcd845d16b1fd8ea40cb61b20539fecdbc935e Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Wed, 9 Jun 2021 08:54:37 +0100 Subject: [PATCH 7/7] Address review comments --- crates/spirv-builder/src/lib.rs | 10 +++++----- examples/runners/wgpu/src/lib.rs | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index f684c782f6..b784fcc893 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -455,9 +455,9 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { // we do that even in case of an error, to let through any useful messages // that ended up on stdout instead of stderr. let stdout = String::from_utf8(build.stdout).unwrap(); + let artifact = get_last_artifact(&stdout); if build.status.success() { - let artifact = get_last_artifact(&stdout); - Ok(artifact) + Ok(artifact.expect("Artifact created when compilation succeeded")) } else { Err(SpirvBuilderError::BuildFailed) } @@ -469,7 +469,7 @@ struct RustcOutput { filenames: Option>, } -fn get_last_artifact(out: &str) -> PathBuf { +fn get_last_artifact(out: &str) -> Option { let last = out .lines() .filter_map(|line| match serde_json::from_str::(line) { @@ -489,9 +489,9 @@ fn get_last_artifact(out: &str) -> PathBuf { .unwrap() .into_iter() .filter(|v| v.ends_with(".spv")); - let filename = filenames.next().expect("Crate had no .spv artifacts"); + let filename = filenames.next()?; assert_eq!(filenames.next(), None, "Crate had multiple .spv artifacts"); - filename.into() + Some(filename.into()) } /// Internally iterate through the leaf dependencies of the artifact at `artifact` diff --git a/examples/runners/wgpu/src/lib.rs b/examples/runners/wgpu/src/lib.rs index 2cc59d4e38..cad4f1b935 100644 --- a/examples/runners/wgpu/src/lib.rs +++ b/examples/runners/wgpu/src/lib.rs @@ -62,7 +62,8 @@ fn maybe_watch( shader: RustGPUShader, force_no_watch: bool, ) -> Receiver> { - // This bound needs to be 1, because when we don't watch, we + // This bound needs to be 1, because in cases where this function is used for direct building (e.g. for the compute example or on android) + // we send the value directly in the same thread. This avoids deadlocking in those cases. let (tx, rx) = mpsc::sync_channel(1); #[cfg(not(any(target_os = "android", target_arch = "wasm32")))] {