Merge branch 'imbris/test' into 'master'

Use separate crate to compile anim as a cdylib to avoid compiling both all the time and improve voxygen pipelining

See merge request veloren/veloren!2140

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"

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" }
+vek = { git = "https://gitlab.com/veloren/vek.git", branch = "fix_intrinsics2" }

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"

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"}

voxygen/anim/dyn/Cargo.toml

@@ -0,0 +1,17 @@
+[package]
+authors = ["Imbris <imbrisf@gmail.com>"]
+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 = "../" }

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::*;

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
+        )
+    });
 }