refactor: Refactor wrapper API (#4)

.github/workflows/benchmark.yml

@@ -0,0 +1,44 @@
+name: Benchmarks
+
+on:
+  push:
+    branches: [ main ]
+  pull_request:
+    branches: [ main ]
+
+env:
+  CARGO_TERM_COLOR: always
+
+jobs:
+  benchmarks:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v3
+    - name: Install dependencies
+      run: |
+        sudo apt update
+        sudo apt install -y openslide-tools
+        sudo ln -s /usr/lib/x86_64-linux-gnu/libopenslide.so.0 /usr/lib/x86_64-linux-gnu/libopenslide.so
+    - uses: actions-rs/toolchain@v1
+      with:
+        toolchain: stable
+        override: true
+
+    - name: Run cargo bench
+      run: cargo bench --workspace | tee bench-output.txt
+
+    - name: Store benchmark result
+      uses: rhysd/github-action-benchmark@v1
+      with:
+        name: openslide-rs Benchmark
+        tool: 'cargo'
+        save-data-file: ${{ github.event_name != 'pull_request' }}
+        output-file-path: bench-output.txt
+        benchmark-data-dir-path: '.'
+        max-items-in-chart: 30
+        github-token: ${{ secrets.GITHUB_TOKEN }}
+        auto-push: true
+        alert-threshold: '200%'
+        comment-on-alert: true
+        fail-on-alert: false
+        alert-comment-cc-users: '@AzHicham'

.github/workflows/clippy.yml

@@ -22,25 +22,21 @@ jobs:
         toolchain: nightly
         override: true
         components: rustfmt, clippy
-
     - name: Run cargo clippy
       uses: actions-rs/cargo@v1
       with:
         command: clippy
         args: --all --fix --all-targets --allow-dirty
-
     - name: Run cargo fmt
       uses: actions-rs/cargo@v1
       with:
         command: fmt
         args: --all
-
     - name: Run cargo check
       uses: actions-rs/cargo@v1
       with:
         command: check
         args: --workspace --verbose
-
     - name: Commit and push changes
       continue-on-error: true
       uses: peter-evans/create-pull-request@v4

.github/workflows/workflow.yml

@@ -43,10 +43,9 @@ jobs:
       run: |
         sudo apt update
         sudo apt install -y openslide-tools
-    - name: Build & Test
-      run: |
         sudo ln -s /usr/lib/x86_64-linux-gnu/libopenslide.so.0 /usr/lib/x86_64-linux-gnu/libopenslide.so
-        cargo test --workspace ${{ matrix.build_type.flag }} --features "${{ matrix.features }}"
+    - name: Build & Test
+      run: cargo test --workspace ${{ matrix.build_type.flag }} --features "${{ matrix.features }}"
 
   pre-commit:
     runs-on: ubuntu-latest

Cargo.toml

@@ -19,10 +19,15 @@ image = ["dep:image"]
 [dependencies]
 libc = "0.2.135"
 thiserror = "1.0.37"
-regex = "1"
+regex = "1.6.0"
 lazy_static = "1.4.0"
 image = { version = "0.24.4", optional = true}
 
 [dev-dependencies]
 rstest = "0.15.0"
 assert_approx_eq = "1.1.0"
+bencher = "0.1.5"
+
+[[bench]]
+name = "bench_read"
+harness = false

benches/bench_read.rs

@@ -0,0 +1,84 @@
+use bencher::{benchmark_group, benchmark_main, Bencher};
+
+use image::imageops::FilterType;
+use openslide_rs::{Address, DeepZoomGenerator, OpenSlide, Region, Size};
+use std::path::Path;
+
+fn openslide_read_region_256(bench: &mut Bencher) {
+    let slide = OpenSlide::new(Path::new("tests/assets/default.svs")).unwrap();
+
+    bench.iter(|| {
+        slide.read_region(&Region {
+            address: Address { x: 0, y: 0 },
+            level: 0,
+            size: Size { w: 256, h: 256 },
+        })
+    });
+}
+
+fn openslide_read_region_512(bench: &mut Bencher) {
+    let slide = OpenSlide::new(Path::new("tests/assets/default.svs")).unwrap();
+
+    bench.iter(|| {
+        slide.read_region(&Region {
+            address: Address { x: 0, y: 0 },
+            level: 0,
+            size: Size { w: 512, h: 512 },
+        })
+    });
+}
+
+fn openslide_read_image_256(bench: &mut Bencher) {
+    let slide = OpenSlide::new(Path::new("tests/assets/default.svs")).unwrap();
+
+    bench.iter(|| {
+        slide.read_image(&Region {
+            address: Address { x: 0, y: 0 },
+            level: 0,
+            size: Size { w: 256, h: 256 },
+        })
+    });
+}
+
+fn openslide_read_image_512(bench: &mut Bencher) {
+    let slide = OpenSlide::new(Path::new("tests/assets/default.svs")).unwrap();
+
+    bench.iter(|| {
+        slide.read_image(&Region {
+            address: Address { x: 0, y: 0 },
+            level: 0,
+            size: Size { w: 512, h: 512 },
+        })
+    });
+}
+
+fn deepzoom_read_image_256(bench: &mut Bencher) {
+    let slide = OpenSlide::new(Path::new("tests/assets/default.svs")).unwrap();
+    let dz = DeepZoomGenerator::new(&slide, 256, 0, false).unwrap();
+
+    bench.iter(|| dz.get_tile(12, Address { x: 0, y: 0 }, FilterType::Lanczos3));
+}
+
+fn deepzoom_read_image_512(bench: &mut Bencher) {
+    let slide = OpenSlide::new(Path::new("tests/assets/default.svs")).unwrap();
+    let dz = DeepZoomGenerator::new(&slide, 512, 0, false).unwrap();
+
+    bench.iter(|| dz.get_tile(12, Address { x: 0, y: 0 }, FilterType::Lanczos3));
+}
+
+benchmark_group!(
+    benches_region,
+    openslide_read_region_256,
+    openslide_read_region_512
+);
+benchmark_group!(
+    benches_image,
+    openslide_read_image_256,
+    openslide_read_image_512
+);
+benchmark_group!(
+    deepzoom_image,
+    deepzoom_read_image_256,
+    deepzoom_read_image_512
+);
+benchmark_main!(benches_region, benches_image, deepzoom_image);

rust-toolchain.toml

@@ -0,0 +1,3 @@
+[toolchain]
+channel = "1.64.0"
+components = ["rustfmt", "clippy"]

src/bindings.rs

@@ -5,6 +5,7 @@
 
 use crate::{errors::OpenSlideError, Result};
 
+use crate::errors::map_string_error;
 use std::{ffi, ops::Deref};
 
 /// wrapper around openslide_t "C" type in OpenSlide
@@ -107,23 +108,26 @@ extern "C" {
 }
 
 pub fn detect_vendor(filename: &str) -> Result<String> {
-    let c_filename = ffi::CString::new(filename).map_err(OpenSlideError::FFINulError)?;
-    let vendor = unsafe {
+    let c_filename = ffi::CString::new(filename).map_err(map_string_error)?;
+    unsafe {
         let c_vendor = openslide_detect_vendor(c_filename.as_ptr());
-        ffi::CStr::from_ptr(c_vendor).to_string_lossy().into_owned()
-    };
-    Ok(vendor)
+        if !c_vendor.is_null() {
+            let vendor = ffi::CStr::from_ptr(c_vendor).to_string_lossy().into_owned();
+            Ok(vendor)
+        } else {
+            Err(OpenSlideError::UnsupportedFile(filename.to_string()))
+        }
+    }
 }
 
 pub fn open(filename: &str) -> Result<*const OpenSlideT> {
-    let c_filename = ffi::CString::new(filename).map_err(OpenSlideError::FFINulError)?;
+    let c_filename = ffi::CString::new(filename).map_err(map_string_error)?;
     let slide = unsafe { openslide_open(c_filename.as_ptr()) };
     if !slide.is_null() {
         get_error(slide)?;
         Ok(slide)
     } else {
-        let error = format!("Cannot open {filename}");
-        Err(OpenSlideError::CoreError(error))
+        Err(OpenSlideError::UnsupportedFile(filename.to_string()))
     }
 }
 
@@ -136,7 +140,10 @@ pub fn close(osr: *const OpenSlideT) {
 pub fn get_level_count(osr: *const OpenSlideT) -> Result<i32> {
     let num_levels = unsafe { openslide_get_level_count(osr) };
     if num_levels == -1 {
-        force_error(osr)?
+        get_error(osr)?;
+        return Err(OpenSlideError::CoreError(
+            "Cannot get level count".to_string(),
+        ));
     }
     Ok(num_levels)
 }
@@ -148,7 +155,10 @@ pub fn get_level0_dimensions(osr: *const OpenSlideT) -> Result<(i64, i64)> {
         openslide_get_level0_dimensions(osr, &mut width, &mut height);
     }
     if width == -1 || height == -1 {
-        force_error(osr)?
+        get_error(osr)?;
+        return Err(OpenSlideError::CoreError(
+            "Cannot get dimensions of level 0".to_string(),
+        ));
     }
     Ok((width, height))
 }
@@ -160,23 +170,30 @@ pub fn get_level_dimensions(osr: *const OpenSlideT, level: i32) -> Result<(i64,
         openslide_get_level_dimensions(osr, level, &mut width, &mut height);
     }
     if width == -1 || height == -1 {
-        force_error(osr)?
+        get_error(osr)?;
+        return Err(OpenSlideError::CoreError(format!("Invalid level {level}")));
     }
     Ok((width, height))
 }
 
 pub fn get_level_downsample(osr: *const OpenSlideT, level: i32) -> Result<f64> {
     let downsampling_factor = unsafe { openslide_get_level_downsample(osr, level) };
     if downsampling_factor == -1.0 {
-        force_error(osr)?
+        get_error(osr)?;
+        return Err(OpenSlideError::CoreError(format!(
+            "Cannot compute downsample for level {level}"
+        )));
     }
     Ok(downsampling_factor)
 }
 
 pub fn get_best_level_for_downsample(osr: *const OpenSlideT, downsample: f64) -> Result<i32> {
     let level = unsafe { openslide_get_best_level_for_downsample(osr, downsample) };
     if level == -1 {
-        force_error(osr)?
+        get_error(osr)?;
+        return Err(OpenSlideError::CoreError(format!(
+            "Cannot compute level for downsample {downsample}"
+        )));
     }
     Ok(level)
 }
@@ -205,7 +222,10 @@ pub fn get_property_names(osr: *const OpenSlideT) -> Result<Vec<String>> {
     let string_values = unsafe {
         let null_terminated_array_ptr = openslide_get_property_names(osr);
         if null_terminated_array_ptr.is_null() {
-            get_error(osr)?
+            get_error(osr)?;
+            return Err(OpenSlideError::CoreError(
+                "Cannot get property names".to_string(),
+            ));
         }
         let mut counter = 0;
         let mut loc = null_terminated_array_ptr;
@@ -227,10 +247,17 @@ pub fn get_property_names(osr: *const OpenSlideT) -> Result<Vec<String>> {
 }
 
 pub fn get_property_value(osr: *const OpenSlideT, name: &str) -> Result<String> {
-    let c_name = ffi::CString::new(name).map_err(OpenSlideError::FFINulError)?;
+    let c_name = ffi::CString::new(name).map_err(map_string_error)?;
     let value = unsafe {
         let c_value = openslide_get_property_value(osr, c_name.as_ptr());
-        ffi::CStr::from_ptr(c_value).to_string_lossy().into_owned()
+        if c_value.is_null() {
+            get_error(osr)?;
+            return Err(OpenSlideError::CoreError(format!(
+                "Error with property named {name}"
+            )));
+        } else {
+            ffi::CStr::from_ptr(c_value).to_string_lossy().into_owned()
+        }
     };
     Ok(value)
 }
@@ -239,7 +266,10 @@ pub fn get_associated_image_names(osr: *const OpenSlideT) -> Result<Vec<String>>
     let string_values = unsafe {
         let null_terminated_array_ptr = openslide_get_associated_image_names(osr);
         if null_terminated_array_ptr.is_null() {
-            get_error(osr)?
+            get_error(osr)?;
+            return Err(OpenSlideError::CoreError(
+                "Cannot get associated image names".to_string(),
+            ));
         }
         let mut counter = 0;
         let mut loc = null_terminated_array_ptr;
@@ -261,20 +291,23 @@ pub fn get_associated_image_names(osr: *const OpenSlideT) -> Result<Vec<String>>
 }
 
 pub fn get_associated_image_dimensions(osr: *const OpenSlideT, name: &str) -> Result<(i64, i64)> {
-    let c_name = ffi::CString::new(name).map_err(OpenSlideError::FFINulError)?;
+    let c_name = ffi::CString::new(name).map_err(map_string_error)?;
     let mut width: i64 = 0;
     let mut height: i64 = 0;
     unsafe {
         openslide_get_associated_image_dimensions(osr, c_name.as_ptr(), &mut width, &mut height);
     }
     if width == -1 || height == -1 {
-        force_error(osr)?
+        get_error(osr)?;
+        return Err(OpenSlideError::CoreError(
+            "Unknown associated image".to_string(),
+        ));
     }
     Ok((width, height))
 }
 
 pub fn read_associated_image(osr: *const OpenSlideT, name: &str) -> Result<((i64, i64), Vec<u8>)> {
-    let c_name = ffi::CString::new(name).map_err(OpenSlideError::FFINulError)?;
+    let c_name = ffi::CString::new(name).map_err(map_string_error)?;
     let (width, height) = get_associated_image_dimensions(osr, name)?;
     let size = (width * height * 4) as usize;
     let mut buffer: Vec<u8> = Vec::with_capacity(size);
@@ -300,8 +333,3 @@ pub fn get_error(osr: *const OpenSlideT) -> Result<()> {
     };
     value
 }
-
-pub fn force_error(osr: *const OpenSlideT) -> Result<()> {
-    get_error(osr)?;
-    Err(OpenSlideError::CoreError("Unknown error".to_string()))
-}