From 30e34aad4408179a50e5146a9279ab632b40f972 Mon Sep 17 00:00:00 2001 From: Imbris Date: Fri, 16 Apr 2021 19:22:06 -0400 Subject: [PATCH] Use separate crate to compile anim as a cdylib to avoid compiling both all the time and improve voxygen pipelining --- Cargo.lock | 7 ++++ Cargo.toml | 3 +- voxygen/Cargo.toml | 2 +- voxygen/anim/Cargo.toml | 10 ++---- voxygen/anim/dyn/Cargo.toml | 17 +++++++++ voxygen/anim/dyn/src/lib.rs | 14 ++++++++ voxygen/anim/src/dyn_lib.rs | 69 ++++++++++++++++++++++--------------- 7 files changed, 84 insertions(+), 38 deletions(-) create mode 100644 voxygen/anim/dyn/Cargo.toml create mode 100644 voxygen/anim/dyn/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 406d047083..0079321be3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5775,6 +5775,13 @@ dependencies = [ "veloren-common", ] +[[package]] +name = "veloren-voxygen-anim-dyn" +version = "0.9.0" +dependencies = [ + "veloren-voxygen-anim", +] + [[package]] name = "veloren-world" version = "0.9.0" diff --git a/Cargo.toml b/Cargo.toml index 1b657f0627..4ef02f3feb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,7 @@ members = [ "server-cli", "voxygen", "voxygen/anim", + "voxygen/anim/dyn", "world", "network", "network/protocol" @@ -94,4 +95,4 @@ debug = 1 [patch.crates-io] # macos CI fix isn't merged yet winit = { git = "https://gitlab.com/veloren/winit.git", branch = "macos-test-spiffed" } -vek = { git = "https://gitlab.com/veloren/vek.git", branch = "fix_intrinsics2" } \ No newline at end of file +vek = { git = "https://gitlab.com/veloren/vek.git", branch = "fix_intrinsics2" } diff --git a/voxygen/Cargo.toml b/voxygen/Cargo.toml index 234a508e4c..06cbf713a5 100644 --- a/voxygen/Cargo.toml +++ b/voxygen/Cargo.toml @@ -27,7 +27,7 @@ common-frontend = {package = "veloren-common-frontend", path = "../common/fronte common-net = {package = "veloren-common-net", path = "../common/net"} common-sys = {package = "veloren-common-sys", path = "../common/sys"} -anim = {package = "veloren-voxygen-anim", path = "anim", default-features = false} +anim = {package = "veloren-voxygen-anim", path = "anim"} # Graphics gfx = "0.18.2" diff --git a/voxygen/anim/Cargo.toml b/voxygen/anim/Cargo.toml index bbfafd2ea5..2070490ea1 100644 --- a/voxygen/anim/Cargo.toml +++ b/voxygen/anim/Cargo.toml @@ -4,18 +4,12 @@ edition = "2018" name = "veloren-voxygen-anim" version = "0.9.0" -[lib] -name = "voxygen_anim" -# Uncomment to use animation hot reloading -# Note: this breaks `cargo test` -crate-type = ["lib", "cdylib"] - [features] -be-dyn-lib = [] use-dyn-lib = ["libloading", "notify", "lazy_static", "tracing", "find_folder"] +be-dyn-lib = [] simd = ["vek/platform_intrinsics"] -default = ["be-dyn-lib", "simd"] +default = ["simd"] [dependencies] common = {package = "veloren-common", path = "../../common"} diff --git a/voxygen/anim/dyn/Cargo.toml b/voxygen/anim/dyn/Cargo.toml new file mode 100644 index 0000000000..becc2a2697 --- /dev/null +++ b/voxygen/anim/dyn/Cargo.toml @@ -0,0 +1,17 @@ +[package] +authors = ["Imbris "] +edition = "2018" +name = "veloren-voxygen-anim-dyn" +version = "0.9.0" + +[lib] +# Using dylib instead of cdylib increases the size 3 -> 13 mb +# but it is needed to expose the symbols from the anim crate :( +# effect on compile time appears to be insignificant +crate-type = ["dylib"] + +[features] +be-dyn-lib = ["veloren-voxygen-anim/be-dyn-lib"] + +[dependencies] +veloren-voxygen-anim = { path = "../" } diff --git a/voxygen/anim/dyn/src/lib.rs b/voxygen/anim/dyn/src/lib.rs new file mode 100644 index 0000000000..9887b30eb7 --- /dev/null +++ b/voxygen/anim/dyn/src/lib.rs @@ -0,0 +1,14 @@ +//! This crate hacks around the inability to dynamically specify the +//! `crate-type` for cargo to build. +//! +//! For more details on the issue this is a decent starting point: https://github.com/rust-lang/cargo/pull/8789 +//! +//! This crate avoids use building the dynamic lib when it isn't needed and the +//! same with the non dynamic build. Additionally, this allows compilation to +//! start earlier since a cdylib doesn't pipeline with it's dependencies. +//! +//! NOTE: the `be-dyn-lib` feature must be used for this crate to be useful, it +//! is not on by default becaue this causes cargo to switch the feature on in +//! the anim crate when compiling the static lib into voxygen. +#[cfg(feature = "be-dyn-lib")] +pub use veloren_voxygen_anim::*; diff --git a/voxygen/anim/src/dyn_lib.rs b/voxygen/anim/src/dyn_lib.rs index 8c7feaf030..3e8366ed32 100644 --- a/voxygen/anim/src/dyn_lib.rs +++ b/voxygen/anim/src/dyn_lib.rs @@ -4,7 +4,6 @@ use notify::{immediate_watcher, EventKind, RecursiveMode, Watcher}; use std::{ process::{Command, Stdio}, sync::{mpsc, Mutex}, - thread, time::Duration, }; @@ -13,14 +12,14 @@ use std::{env, path::PathBuf}; use tracing::{debug, error, info}; #[cfg(target_os = "windows")] -const COMPILED_FILE: &str = "voxygen_anim.dll"; +const COMPILED_FILE: &str = "veloren_voxygen_anim_dyn.dll"; #[cfg(target_os = "windows")] -const ACTIVE_FILE: &str = "voxygen_anim_active.dll"; +const ACTIVE_FILE: &str = "veloren_voxygen_anim_dyn_active.dll"; #[cfg(not(target_os = "windows"))] -const COMPILED_FILE: &str = "libvoxygen_anim.so"; +const COMPILED_FILE: &str = "libveloren_voxygen_anim_dyn.so"; #[cfg(not(target_os = "windows"))] -const ACTIVE_FILE: &str = "libvoxygen_anim_active.so"; +const ACTIVE_FILE: &str = "libveloren_voxygen_anim_dyn_active.so"; // This option is required as `hotreload()` moves the `LoadedLib`. lazy_static! { @@ -29,7 +28,7 @@ lazy_static! { /// LoadedLib holds a loaded dynamic library and the location of library file /// with the appropriate OS specific name and extension i.e. -/// `libvoxygen_anim_active.dylib`, `voxygen_anim_active.dll`. +/// `libvoxygen_anim_dyn_active.dylib`, `voxygen_anim_dyn_active.dll`. /// /// # NOTE /// DOES NOT WORK ON MACOS, due to some limitations with hot-reloading the @@ -74,10 +73,8 @@ impl LoadedLib { let lib = match unsafe { Library::new(lib_path.clone()) } { Ok(lib) => lib, Err(e) => panic!( - "Tried to load dynamic library from {:?}, but it could not be found. The first \ - reason might be that you need to uncomment a line in `voxygen/anim/Cargo.toml` \ - to build the library required for hot reloading. The second is we may require a \ - special case for your OS so we can find it. {:?}", + "Tried to load dynamic library from {:?}, but it could not be found. A potential \ + reason is we may require a special case for your OS so we can find it. {:?}", lib_path, e ), }; @@ -141,28 +138,37 @@ pub fn init() { // Start watcher let mut watcher = immediate_watcher(move |res| event_fn(res, &reload_send)).unwrap(); - watcher.watch("anim", RecursiveMode::Recursive).unwrap(); + + // Search for the anim directory. + let anim_dir = Search::Kids(1) + .for_folder("anim") + .expect("Could not find the anim crate directory relative to the current directory"); + + watcher.watch(anim_dir, RecursiveMode::Recursive).unwrap(); // Start reloader that watcher signals // "Debounces" events since I can't find the option to do this in the latest // `notify` - std::thread::Builder::new("voxygen_anim_watcher".to_owned()).spawn(move || { - let mut modified_paths = std::collections::HashSet::new(); - while let Ok(path) = reload_recv.recv() { - modified_paths.insert(path); - // Wait for any additional modify events before reloading - while let Ok(path) = reload_recv.recv_timeout(Duration::from_millis(300)) { + std::thread::Builder::new() + .name("voxygen_anim_watcher".into()) + .spawn(move || { + let mut modified_paths = std::collections::HashSet::new(); + while let Ok(path) = reload_recv.recv() { modified_paths.insert(path); + // Wait for any additional modify events before reloading + while let Ok(path) = reload_recv.recv_timeout(Duration::from_millis(300)) { + modified_paths.insert(path); + } + + info!( + ?modified_paths, + "Hot reloading animations because files in `anim` modified." + ); + + hotreload(); } - - info!( - ?modified_paths, - "Hot reloading animations because files in `anim` modified." - ); - - hotreload(); - } - }); + }) + .unwrap(); // Let the watcher live forever std::mem::forget(watcher); @@ -220,7 +226,9 @@ fn compile() -> bool { .stdout(Stdio::inherit()) .arg("build") .arg("--package") - .arg("veloren-voxygen-anim") + .arg("veloren-voxygen-anim-dyn") + .arg("--features") + .arg("veloren-voxygen-anim-dyn/be-dyn-lib") .output() .unwrap(); @@ -241,5 +249,10 @@ fn copy(lib_path: &PathBuf) { // Copy the library file from where it is output, to where we are going to // load it from i.e. lib_path. - std::fs::copy(lib_compiled_path, lib_output_path).expect("Failed to rename dynamic library."); + std::fs::copy(&lib_compiled_path, &lib_output_path).unwrap_or_else(|err| { + panic!( + "Failed to rename dynamic library from {:?} to {:?}. {:?}", + lib_compiled_path, lib_output_path, err + ) + }); }