wip: fix readme for new kaleido feature

Signed-off-by: Andrei Gherghescu <[email protected]>

Author: andrei-ng

README.md

@@ -98,7 +98,16 @@ plot.show(); // The default web browser will open, displaying an interactive plo
 
 ## Exporting a Static Image
 
-To save a plot as a static image, the `kaleido` feature is required:
+To save a plot as a static image, the `kaleido` feature is required as well as installing an **external dependency**.
+
+### Kaleido external dependency
+
+When developing applications for your host, enabling both `kaleido` and `kaleido_download` features will ensure that the Kaleido binary is downloaded for your system's architecture at compile time. After download, it is unpacked into a specific path, e.g., on Linux this is `/home/USERNAME/.config/kaleido`.
+
+When the applications developed with `plotly.rs` are intended for other targets or when the user wants to control where the `Kaleido` binary is installed then `Kaleido` must be manually downloaded and installed. Setting the environment variable `KALEIDO_PATH=/path/installed/kaleido/` will ensure that applications that were built with the `kaleido` feature enabled can locate the executable and use it to generate static images.
+
+Kaleido binaries are available on Github [release page](https://github.com/plotly/Kaleido/releases). It currently supports Linux(`x86_64`), Windows(`x86_64`) and MacOS(`x86_64`/`aarch64`).
+
 
 ```toml
 # Cargo.toml
@@ -121,12 +130,6 @@ plot.add_trace(trace);
 plot.write_image("out.png", ImageFormat::PNG, 800, 600, 1.0);
 ```
 
-### _Kaleido dependency_
-
-On your host, when building this project with the `kaleido` feature enabled the Kaleido binary is downloaded automatically for your system's architecture at compile time from the official Kaleido [release page](https://github.com/plotly/Kaleido/releases). This library currently supports `x86_64` on Linux and Windows, and both `x86_64` and `aarch64` on macOS.
-
-When building application for other targets that depend on this feature, the `Kaleido` binary will need to be installed manually on the target machine. Currently, the location where the binary is expected is hardcoded depending on the target OS. E.g., on Linux this defaults to `~/.config/kaleido`. This is defined in source code [here](https://github.com/plotly/plotly.rs/blob/1405731b5121c1343b491e307222a21ef4becc5e/plotly_kaleido/src/lib.rs#L89)
-
 ## Usage Within a Wasm Environment
 
 Using `Plotly.rs` in a Wasm-based frontend framework is possible by enabling the `wasm` feature:
@@ -198,6 +201,16 @@ The following feature flags are available:
 
 Adds plot save functionality to the following formats: `png`, `jpeg`, `webp`, `svg`, `pdf` and `eps`.
 
+Requires `Kaleido` to have been previously installed on the host machine. See below feature flag and [click on this link](#my-multi-word-header).
+
+### `kaleido_download`
+
+Enable download and install of Kaleido binary at build time from [Kaleido releases](https://github.com/plotly/Kaleido/releases/) on the host machine.
+
+
+
+This will ensure that your applications can export plots to static images on the host machine. However, when porting your applications to a different machine, `Kaleido` will have to be installed manually. See [Setting up Kaleido](#my-multi-word-header).
+
 ### `plotly_image`
 
 Adds trait implementations so that `image::RgbImage` and `image::RgbaImage` can be used more directly with the `plotly::Image` trace.

examples/kaleido/Cargo.toml

@@ -8,4 +8,4 @@ authors = [
 edition = "2021"
 
 [dependencies]
-plotly = { path = "../../plotly", features = ["kaleido", "kaleido_fetch"] }
+plotly = { path = "../../plotly", features = ["kaleido", "kaleido_download"] }

examples/kaleido/src/main.rs

@@ -5,15 +5,21 @@ fn main() {
     let trace = Scatter::new(vec![0, 1, 2], vec![2, 1, 0]);
     plot.add_trace(trace);
 
-    // Adjust these arguments to set the image format, width and height of the
+    // Adjust these arguments to set the width and height of the
     // output image.
     let filename = "out";
-    let image_format = ImageFormat::PNG;
     let width = 800;
     let height = 600;
     let scale = 1.0;
 
     // The image will be saved to format!("{filename}.{image_format}") relative to
     // the current working directory.
-    plot.write_image(filename, image_format, width, height, scale);
+    plot.write_image(filename, ImageFormat::EPS, width, height, scale);
+    plot.write_image(filename, ImageFormat::JPEG, width, height, scale);
+    plot.write_image(filename, ImageFormat::PDF, width, height, scale);
+    plot.write_image(filename, ImageFormat::PNG, width, height, scale);
+    plot.write_image(filename, ImageFormat::SVG, width, height, scale);
+    plot.write_image(filename, ImageFormat::WEBP, width, height, scale);
+
+    let _svg_string = plot.to_svg(width, height, scale);
 }

plotly/Cargo.toml

@@ -15,7 +15,7 @@ exclude = ["target/*"]
 
 [features]
 kaleido = ["plotly_kaleido"]
-kaleido_fetch = ["plotly_kaleido/download"]
+kaleido_download = ["plotly_kaleido/download"]
 
 plotly_ndarray = ["ndarray"]
 plotly_image = ["image"]

plotly/src/plot.rs

@@ -773,7 +773,7 @@ mod tests {
         assert!(!dst.exists());
     }
 
-    #[cfg(not(target_os = "macos"))]
+    // #[cfg(not(target_os = "macos"))]
     #[test]
     #[cfg(feature = "kaleido")]
     fn test_save_to_svg() {
@@ -786,7 +786,7 @@ mod tests {
     }
 
     #[test]
-    #[ignore] // This seems to fail unpredictably on MacOs.
+    // #[ignore] // This seems to fail unpredictably on MacOs.
     #[cfg(feature = "kaleido")]
     fn test_save_to_eps() {
         let plot = create_test_plot();
@@ -797,7 +797,7 @@ mod tests {
         assert!(!dst.exists());
     }
 
-    #[cfg(not(target_os = "macos"))]
+    // #[cfg(not(target_os = "macos"))]
     #[test]
     #[cfg(feature = "kaleido")]
     fn test_save_to_pdf() {
@@ -809,7 +809,7 @@ mod tests {
         assert!(!dst.exists());
     }
 
-    #[cfg(target_os = "linux")]
+    #[cfg(not(target_os = "macos"))]
     #[test]
     #[cfg(feature = "kaleido")]
     fn test_save_to_webp() {

plotly_kaleido/src/lib.rs

@@ -87,7 +87,7 @@ impl Kaleido {
                 Some(compile_time_path) => compile_time_path.to_string(),
                 None => {
                     println!("{}: {}", Self::KALEIDO_PATH_ENV, runtime_env_err);
-                    println!("Use `kaleido_fetch` feature to automatically download, install and use Kaleido when targeting applications that run on the host machine.");
+                    println!("Use `kaleido_download` feature to automatically download, install and use Kaleido when targeting applications that run on the host machine.");
                     println!("Use `{}` environment variable when targeting applications intended to run on different machines. Manually install Kaleido on the target machine and point {} to the installation location.", Self::KALEIDO_PATH_ENV, Self::KALEIDO_PATH_ENV
                     );
                     std::process::exit(1);
@@ -199,6 +199,7 @@ impl Kaleido {
                 "--single-process",
                 "--disable-gpu",
                 "--no-sandbox",
+                "--debug",
             ])
             .stdin(Stdio::piped())
             .stdout(Stdio::piped())
@@ -227,22 +228,19 @@ impl Kaleido {
         let output_lines = BufReader::new(process.stdout.take().unwrap()).lines();
         for line in output_lines.map_while(Result::ok) {
             let res = KaleidoResult::from(line.as_str());
-            match res.result {
-                Some(image_data) => {
-                    // TODO: this should be refactored
-                    // The assumption is that  KaleidoResult contains a single image.
-                    // We should end the loop on the first valid one.
-                    // If that is not the case, prior implementation would have returned the last
-                    // valid image
-                    return Ok(image_data);
-                }
-                None => {
-                    println!("empty line from Kaleido stdout");
-                }
+            if let Some(image_data) = res.result {
+                // TODO: this should be refactored
+                // The assumption is that  KaleidoResult contains a single image.
+                // We should end the loop on the first valid one.
+                // If that is not the case, prior implementation would have returned the last
+                // valid image
+                return Ok(image_data);
             }
         }
 
-        // Don't eat up Kaleido/Chromiu erros but show them in the terminal
+        // Don't eat up Kaleido/Chromium errors but show them in the terminal
+        println!("Kaleido failed to generate static image for format: {format}.");
+        println!("Kaleido stderr output:");
         let stderr = process.stderr.take().unwrap();
         let stderr_lines = BufReader::new(stderr).lines();
         for line in stderr_lines {