diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 998c2c8..ee7b799 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,12 +32,12 @@ jobs: ls -la /usr/lib/llvm-18/lib/libclang.so which llvm-config-18 - # Build OpenCV 4.12 from source since Ubuntu 24.04 only has 4.6 - echo "Building OpenCV 4.12 from source..." + # Build OpenCV 4.11 from source since Ubuntu 24.04 only has 4.6 + echo "Building OpenCV 4.11 from source..." cd /tmp - wget -q https://github.com/opencv/opencv/archive/4.12.0.tar.gz - tar -xzf 4.12.0.tar.gz - cd opencv-4.12.0 + wget -q https://github.com/opencv/opencv/archive/4.11.0.tar.gz + tar -xzf 4.11.0.tar.gz + cd opencv-4.11.0 mkdir build cd build cmake \ @@ -127,12 +127,12 @@ jobs: ls -la /usr/lib/llvm-18/lib/libclang.so which llvm-config-18 - # Build OpenCV 4.12 from source since Ubuntu 24.04 only has 4.6 - echo "Building OpenCV 4.12 from source..." + # Build OpenCV 4.11 from source since Ubuntu 24.04 only has 4.6 + echo "Building OpenCV 4.11 from source..." cd /tmp - wget -q https://github.com/opencv/opencv/archive/4.12.0.tar.gz - tar -xzf 4.12.0.tar.gz - cd opencv-4.12.0 + wget -q https://github.com/opencv/opencv/archive/4.11.0.tar.gz + tar -xzf 4.11.0.tar.gz + cd opencv-4.11.0 mkdir build cd build cmake \ @@ -218,12 +218,12 @@ jobs: ls -la /usr/lib/llvm-18/lib/libclang.so which llvm-config-18 - # Build OpenCV 4.12 from source since Ubuntu 24.04 only has 4.6 - echo "Building OpenCV 4.12 from source..." + # Build OpenCV 4.11 from source since Ubuntu 24.04 only has 4.6 + echo "Building OpenCV 4.11 from source..." cd /tmp - wget -q https://github.com/opencv/opencv/archive/4.12.0.tar.gz - tar -xzf 4.12.0.tar.gz - cd opencv-4.12.0 + wget -q https://github.com/opencv/opencv/archive/4.11.0.tar.gz + tar -xzf 4.11.0.tar.gz + cd opencv-4.11.0 mkdir build cd build cmake \ @@ -298,12 +298,12 @@ jobs: ls -la /usr/lib/llvm-18/lib/libclang.so which llvm-config-18 - # Build OpenCV 4.12 from source since Ubuntu 24.04 only has 4.6 - echo "Building OpenCV 4.12 from source..." + # Build OpenCV 4.11 from source since Ubuntu 24.04 only has 4.6 + echo "Building OpenCV 4.11 from source..." cd /tmp - wget -q https://github.com/opencv/opencv/archive/4.12.0.tar.gz - tar -xzf 4.12.0.tar.gz - cd opencv-4.12.0 + wget -q https://github.com/opencv/opencv/archive/4.11.0.tar.gz + tar -xzf 4.11.0.tar.gz + cd opencv-4.11.0 mkdir build cd build cmake \ @@ -348,19 +348,19 @@ jobs: - name: Install maturin (Linux/macOS) run: pip install maturin[patchelf] - - name: Install OpenCV (macOS) + - name: Install OpenCV build dependencies (macOS) if: runner.os == 'macOS' run: | - # Install OpenCV and LLVM via Homebrew - brew install opencv llvm + # Build against a pinned OpenCV version instead of Homebrew's moving target + brew install llvm cmake nasm pkg-config # Set up LLVM environment for opencv-rust bindings (keg-only installation) echo "LIBCLANG_PATH=/opt/homebrew/opt/llvm/lib" >> $GITHUB_ENV echo "LLVM_CONFIG_PATH=/opt/homebrew/opt/llvm/bin/llvm-config" >> $GITHUB_ENV echo "DYLD_LIBRARY_PATH=/opt/homebrew/opt/llvm/lib:$DYLD_LIBRARY_PATH" >> $GITHUB_ENV - # Set up OpenCV environment - echo "OPENCV_LINK_LIBS=opencv_calib3d,opencv_core,opencv_dnn,opencv_features2d,opencv_flann,opencv_highgui,opencv_imgcodecs,opencv_imgproc,opencv_ml,opencv_objdetect,opencv_photo,opencv_stitching,opencv_video,opencv_videoio" >> $GITHUB_ENV - echo "OPENCV_LINK_PATHS=/opt/homebrew/lib" >> $GITHUB_ENV - echo "OPENCV_INCLUDE_PATHS=/opt/homebrew/include/opencv4" >> $GITHUB_ENV + + - name: Build static OpenCV (macOS) + if: runner.os == 'macOS' + run: ./scripts/build-opencv-static.sh - name: Build wheels (Linux) if: runner.os == 'Linux' @@ -369,6 +369,11 @@ jobs: - name: Build wheels (macOS) if: runner.os == 'macOS' run: maturin build --release --out dist --no-default-features --features python-bindings,simd,opencv,metal + env: + OPENCV_INCLUDE_PATHS: ${{ github.workspace }}/third_party/opencv-static/include/opencv4 + OPENCV_LINK_PATHS: ${{ github.workspace }}/third_party/opencv-static/lib + OPENCV_LINK_LIBS: static=opencv_world,static=avformat,static=avcodec,static=avfilter,static=swresample,static=swscale,static=avutil,static=jpeg,static=png,static=tiff,static=webp,static=jasper,framework=Accelerate,dylib=c++,framework=OpenCL,z + OPENCV_DISABLE_PROBES: pkg_config,cmake,vcpkg,vcpkg_cmake - name: Upload wheels @@ -398,12 +403,12 @@ jobs: echo "LLVM_CONFIG_PATH=/usr/bin/llvm-config-18" >> $GITHUB_ENV echo "LD_LIBRARY_PATH=/usr/lib/llvm-18/lib:$LD_LIBRARY_PATH" >> $GITHUB_ENV - # Build OpenCV 4.12 from source - echo "Building OpenCV 4.12 from source..." + # Build OpenCV 4.11 from source + echo "Building OpenCV 4.11 from source..." cd /tmp - wget -q https://github.com/opencv/opencv/archive/4.12.0.tar.gz - tar -xzf 4.12.0.tar.gz - cd opencv-4.12.0 + wget -q https://github.com/opencv/opencv/archive/4.11.0.tar.gz + tar -xzf 4.11.0.tar.gz + cd opencv-4.11.0 mkdir build cd build cmake \ diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index d9a1ebb..5cb9eb6 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -49,7 +49,7 @@ jobs: uses: actions/cache@v4 with: path: third_party/opencv-static - key: opencv-static-4.12.0-codecs-jasper-ffmpeg-no-itt-no-openjpeg-linux-${{ matrix.os }}-${{ runner.arch }}-${{ matrix.manylinux }} + key: opencv-static-4.11.0-codecs-jasper-ffmpeg-no-itt-no-openjpeg-linux-${{ matrix.os }}-${{ runner.arch }}-${{ matrix.manylinux }} - name: Install build dependencies run: | @@ -147,7 +147,7 @@ jobs: uses: actions/cache@v4 with: path: third_party/opencv-static - key: opencv-static-4.12.0-codecs-jasper-ffmpeg-no-itt-no-openjpeg-linux-aarch64-${{ runner.arch }}-manylinux_2_39 + key: opencv-static-4.11.0-codecs-jasper-ffmpeg-no-itt-no-openjpeg-linux-aarch64-${{ runner.arch }}-manylinux_2_39 - name: Install build dependencies run: | @@ -229,7 +229,7 @@ jobs: uses: actions/cache@v4 with: path: third_party/opencv-static - key: opencv-static-4.12.0-codecs-jasper-ffmpeg-no-itt-no-openjpeg-macos-${{ runner.os }}-${{ runner.arch }} + key: opencv-static-4.11.0-codecs-jasper-ffmpeg-no-itt-no-openjpeg-macos-${{ runner.os }}-${{ runner.arch }} - name: Install build dependencies run: | diff --git a/Cargo.toml b/Cargo.toml index 63b74a9..edcd53a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,8 @@ num_cpus = "1.16" numpy = {version = "0.22", optional = true} objc = {version = "0.2", optional = true} # OpenCV bindings for performance parity -opencv = {version = "0.93", optional = true} +# Limit binding generation to the modules this crate actually uses. +opencv = {version = "0.98", optional = true, default-features = false, features = ["clang-runtime", "imgproc", "videoio"]} # Python bindings (optional) pyo3 = {version = "0.22", features = ["extension-module"], optional = true} rayon = "1.8" diff --git a/rust-toolchain.toml b/rust-toolchain.toml index b565b64..242f4cb 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,4 +1,3 @@ [toolchain] channel = "1.89" -components = ["rustfmt", "clippy"] profile = "minimal" diff --git a/scripts/build-opencv-static.sh b/scripts/build-opencv-static.sh index 9e981c6..c6e08ed 100755 --- a/scripts/build-opencv-static.sh +++ b/scripts/build-opencv-static.sh @@ -4,7 +4,7 @@ set -euo pipefail # Build static OpenCV bundle for embedding in Python wheels # This eliminates the need for users to install OpenCV system packages -OPENCV_VERSION="4.12.0" +OPENCV_VERSION="4.11.0" FFMPEG_VERSION="6.1.1" SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" PROJECT_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd)" diff --git a/setup-env.sh b/setup-env.sh old mode 100755 new mode 100644 index 00ce289..d8674e7 --- a/setup-env.sh +++ b/setup-env.sh @@ -1,29 +1,299 @@ -#!/bin/bash +#!/usr/bin/env bash # Setup environment for OpenCV Rust development +fail() { + echo "ERROR: $*" >&2 + return 1 2>/dev/null || exit 1 +} + +prepend_path_var() { + local var_name="$1" + local new_path="$2" + local current_value + + eval "current_value=\"\${$var_name:-}\"" + + if [ -z "${new_path}" ] || [ ! -d "${new_path}" ]; then + return 0 + fi + + case ":${current_value}:" in + *":${new_path}:"*) ;; + *) + if [ -n "${current_value}" ]; then + eval "export ${var_name}=\"${new_path}:\$${var_name}\"" + else + eval "export ${var_name}=\"${new_path}\"" + fi + ;; + esac +} + +append_rustflag() { + local flag="$1" + + case " ${RUSTFLAGS:-} " in + *" ${flag} "*) ;; + *) + if [ -n "${RUSTFLAGS:-}" ]; then + export RUSTFLAGS="${RUSTFLAGS} ${flag}" + else + export RUSTFLAGS="${flag}" + fi + ;; + esac +} + +find_llvm_config() { + local candidate + + if [ -n "${LLVM_CONFIG_PATH:-}" ] && [ -x "${LLVM_CONFIG_PATH}" ]; then + printf '%s\n' "${LLVM_CONFIG_PATH}" + return 0 + fi + + if command -v llvm-config >/dev/null 2>&1; then + command -v llvm-config + return 0 + fi + + for candidate in /opt/homebrew/opt/llvm/bin/llvm-config /usr/local/opt/llvm/bin/llvm-config; do + if [ -x "${candidate}" ]; then + printf '%s\n' "${candidate}" + return 0 + fi + done + + candidate="$( + find /usr/lib \ + \( -path '/usr/lib/llvm-*/bin/llvm-config' -o -path '/usr/lib/llvm/*/bin/llvm-config' \) \ + 2>/dev/null \ + | head -n 1 + )" + if [ -n "${candidate}" ] && [ -x "${candidate}" ]; then + printf '%s\n' "${candidate}" + return 0 + fi + + return 1 +} + +find_libclang_path() { + local candidate + + candidate="$(find_llvm_config || true)" + if [ -n "${candidate}" ]; then + candidate="$("${candidate}" --libdir 2>/dev/null || true)" + if [ -n "${candidate}" ] && [ -d "${candidate}" ]; then + printf '%s\n' "${candidate}" + return 0 + fi + fi + + if [ -n "${LIBCLANG_PATH:-}" ] && [ -d "${LIBCLANG_PATH}" ]; then + printf '%s\n' "${LIBCLANG_PATH}" + return 0 + fi + + if [ "$(uname -s)" = "Linux" ] && command -v ldconfig >/dev/null 2>&1; then + candidate="$( + ldconfig -p 2>/dev/null \ + | sed -n 's/.* => \(.*\/libclang[^ ]*\)$/\1/p' \ + | head -n 1 + )" + if [ -n "${candidate}" ]; then + candidate="$(dirname "${candidate}")" + if [ -d "${candidate}" ]; then + printf '%s\n' "${candidate}" + return 0 + fi + fi + fi + + for candidate in /opt/homebrew/opt/llvm/lib /usr/local/opt/llvm/lib /usr/lib64 /usr/lib; do + if [ -d "${candidate}" ]; then + printf '%s\n' "${candidate}" + return 0 + fi + done + + candidate="$( + find /usr/lib \ + \( -path '/usr/lib/llvm-*/lib' -o -path '/usr/lib/llvm/*/lib' -o -path '/usr/lib/llvm-*/lib64' -o -path '/usr/lib/llvm/*/lib64' \) \ + 2>/dev/null \ + | head -n 1 + )" + if [ -n "${candidate}" ] && [ -d "${candidate}" ]; then + printf '%s\n' "${candidate}" + return 0 + fi + + return 1 +} + +detect_opencv_from_pkg_config() { + local libs_raw lib_paths_raw include_paths_raw + + if ! command -v pkg-config >/dev/null 2>&1; then + return 1 + fi + + if ! pkg-config --exists opencv4; then + return 1 + fi + + libs_raw="$(pkg-config --libs-only-l opencv4 2>/dev/null || true)" + lib_paths_raw="$(pkg-config --libs-only-L opencv4 2>/dev/null || true)" + include_paths_raw="$(pkg-config --cflags-only-I opencv4 2>/dev/null || true)" + + OPENCV_LINK_LIBS="$( + printf '%s\n' "${libs_raw}" \ + | tr ' ' '\n' \ + | sed -n 's/^-l//p' \ + | paste -sd, - + )" + OPENCV_LINK_PATHS="$( + printf '%s\n' "${lib_paths_raw}" \ + | tr ' ' '\n' \ + | sed -n 's/^-L//p' \ + | paste -sd, - + )" + OPENCV_INCLUDE_PATHS="$( + printf '%s\n' "${include_paths_raw}" \ + | tr ' ' '\n' \ + | sed -n 's/^-I//p' \ + | paste -sd, - + )" + + export OPENCV_LINK_LIBS + export OPENCV_LINK_PATHS + export OPENCV_INCLUDE_PATHS +} + +detect_opencv_from_defaults() { + case "${OS_NAME}" in + Darwin) + export OPENCV_LINK_LIBS="opencv_calib3d,opencv_core,opencv_dnn,opencv_features2d,opencv_flann,opencv_highgui,opencv_imgcodecs,opencv_imgproc,opencv_ml,opencv_objdetect,opencv_photo,opencv_stitching,opencv_video,opencv_videoio" + + if [ -d /opt/homebrew/include/opencv4 ]; then + export OPENCV_INCLUDE_PATHS="/opt/homebrew/include/opencv4" + export OPENCV_LINK_PATHS="/opt/homebrew/lib" + elif [ -d /usr/local/include/opencv4 ]; then + export OPENCV_INCLUDE_PATHS="/usr/local/include/opencv4" + export OPENCV_LINK_PATHS="/usr/local/lib" + else + fail "Could not detect OpenCV via pkg-config or common macOS install paths" + fi + ;; + Linux) + export OPENCV_LINK_LIBS="opencv_calib3d,opencv_core,opencv_dnn,opencv_features2d,opencv_flann,opencv_highgui,opencv_imgcodecs,opencv_imgproc,opencv_ml,opencv_objdetect,opencv_photo,opencv_stitching,opencv_video,opencv_videoio" + + if [ -d /usr/include/opencv4 ]; then + export OPENCV_INCLUDE_PATHS="/usr/include/opencv4" + elif [ -d /usr/local/include/opencv4 ]; then + export OPENCV_INCLUDE_PATHS="/usr/local/include/opencv4" + else + fail "Could not detect OpenCV headers via pkg-config or common Linux include paths" + fi + + if [ -d /usr/lib/x86_64-linux-gnu ]; then + export OPENCV_LINK_PATHS="/usr/lib/x86_64-linux-gnu" + elif [ -d /usr/lib64 ]; then + export OPENCV_LINK_PATHS="/usr/lib64" + elif [ -d /usr/lib ]; then + export OPENCV_LINK_PATHS="/usr/lib" + else + fail "Could not detect OpenCV library directory via pkg-config or common Linux library paths" + fi + ;; + *) + fail "Unsupported operating system: ${OS_NAME}" + ;; + esac +} + +fill_default_opencv_link_paths() { + if [ -n "${OPENCV_LINK_PATHS:-}" ]; then + return 0 + fi + + case "${OS_NAME}" in + Darwin) + if [ -d /opt/homebrew/lib ]; then + export OPENCV_LINK_PATHS="/opt/homebrew/lib" + elif [ -d /usr/local/lib ]; then + export OPENCV_LINK_PATHS="/usr/local/lib" + fi + ;; + Linux) + if [ -d /usr/lib/x86_64-linux-gnu ]; then + export OPENCV_LINK_PATHS="/usr/lib/x86_64-linux-gnu" + elif [ -d /usr/lib64 ]; then + export OPENCV_LINK_PATHS="/usr/lib64" + elif [ -d /usr/lib ]; then + export OPENCV_LINK_PATHS="/usr/lib" + fi + ;; + esac +} + echo "Setting up OpenCV Rust environment..." -# LLVM/Clang paths for OpenCV bindings -export DYLD_LIBRARY_PATH="/opt/homebrew/Cellar/llvm/21.1.1/lib:$DYLD_LIBRARY_PATH" -export LIBCLANG_PATH="/opt/homebrew/Cellar/llvm/21.1.1/lib" -export LLVM_CONFIG_PATH="/opt/homebrew/Cellar/llvm/21.1.1/bin/llvm-config" +OS_NAME="$(uname -s)" +case "${OS_NAME}" in + Darwin|Linux) ;; + *) fail "Unsupported operating system: ${OS_NAME}" ;; +esac + +LIBCLANG_DIR="$(find_libclang_path || true)" +if [ -z "${LIBCLANG_DIR}" ]; then + fail "Could not detect libclang. Install LLVM/Clang and ensure llvm-config is on PATH." +fi + +LLVM_CONFIG_BIN="$(find_llvm_config || true)" +if [ -n "${LLVM_CONFIG_BIN}" ]; then + export LLVM_CONFIG_PATH="${LLVM_CONFIG_BIN}" +fi + +export LIBCLANG_PATH="${LIBCLANG_DIR}" -# OpenCV paths -export OPENCV_LINK_LIBS="opencv_calib3d,opencv_core,opencv_dnn,opencv_features2d,opencv_flann,opencv_highgui,opencv_imgcodecs,opencv_imgproc,opencv_ml,opencv_objdetect,opencv_photo,opencv_stitching,opencv_video,opencv_videoio" -export OPENCV_LINK_PATHS="/opt/homebrew/lib" -export OPENCV_INCLUDE_PATHS="/opt/homebrew/include/opencv4" +case "${OS_NAME}" in + Darwin) + prepend_path_var DYLD_LIBRARY_PATH "${LIBCLANG_DIR}" + if command -v sw_vers >/dev/null 2>&1; then + export MACOSX_DEPLOYMENT_TARGET="$(sw_vers -productVersion | cut -d. -f1,2)" + fi + ;; + Linux) + prepend_path_var LD_LIBRARY_PATH "${LIBCLANG_DIR}" + ;; +esac -# Rust optimizations -export RUSTFLAGS="-C target-cpu=native" +if ! detect_opencv_from_pkg_config; then + detect_opencv_from_defaults +fi +fill_default_opencv_link_paths -# macOS deployment target - match current system -export MACOSX_DEPLOYMENT_TARGET="$(sw_vers -productVersion | cut -d. -f1,2)" +append_rustflag "-C target-cpu=native" echo "Environment variables set:" -echo " DYLD_LIBRARY_PATH=$DYLD_LIBRARY_PATH" -echo " LIBCLANG_PATH=$LIBCLANG_PATH" -echo " OPENCV_INCLUDE_PATHS=$OPENCV_INCLUDE_PATHS" -echo " MACOSX_DEPLOYMENT_TARGET=$MACOSX_DEPLOYMENT_TARGET" +echo " OS=${OS_NAME}" +echo " LIBCLANG_PATH=${LIBCLANG_PATH}" +if [ -n "${LLVM_CONFIG_PATH:-}" ]; then + echo " LLVM_CONFIG_PATH=${LLVM_CONFIG_PATH}" +fi +echo " OPENCV_LINK_LIBS=${OPENCV_LINK_LIBS}" +echo " OPENCV_LINK_PATHS=${OPENCV_LINK_PATHS}" +echo " OPENCV_INCLUDE_PATHS=${OPENCV_INCLUDE_PATHS}" +if [ "${OS_NAME}" = "Darwin" ]; then + echo " DYLD_LIBRARY_PATH=${DYLD_LIBRARY_PATH:-}" + if [ -n "${MACOSX_DEPLOYMENT_TARGET:-}" ]; then + echo " MACOSX_DEPLOYMENT_TARGET=${MACOSX_DEPLOYMENT_TARGET}" + fi +else + echo " LD_LIBRARY_PATH=${LD_LIBRARY_PATH:-}" +fi +echo " RUSTFLAGS=${RUSTFLAGS}" echo "" echo "You can now run:" echo " cargo clippy --all-features" diff --git a/src/luminance.rs b/src/luminance.rs index 57fb3f9..38e4cac 100644 --- a/src/luminance.rs +++ b/src/luminance.rs @@ -49,23 +49,10 @@ pub unsafe fn calculate_luminance_raw_buffer( height: usize, channels: usize, ) -> f64 { - if channels != 3 { + if channels != 3 || width == 0 || height == 0 { return 0.0; } - let pixel_count = width * height; - - // Adaptive SIMD threshold: use SIMD only for images >64K pixels (256x256) - const SIMD_THRESHOLD: usize = 65536; - - #[cfg(feature = "simd")] - { - if pixel_count > SIMD_THRESHOLD { - // Use SIMD for large images - return calculate_luminance_raw_buffer_simd(rgb_ptr, width, height, channels); - } - } - // Use scalar for small images or when SIMD is not available calculate_luminance_raw_buffer_scalar(rgb_ptr, width, height, channels) } @@ -103,6 +90,7 @@ unsafe fn calculate_luminance_raw_buffer_scalar( /// /// # Safety /// Same safety requirements as `calculate_luminance_raw_buffer` +#[allow(dead_code)] unsafe fn calculate_luminance_raw_buffer_simd( rgb_ptr: *const u8, width: usize, @@ -132,6 +120,7 @@ unsafe fn calculate_luminance_raw_buffer_simd( #[cfg(any(target_arch = "x86_64", target_arch = "x86"))] #[target_feature(enable = "avx2")] +#[allow(dead_code)] unsafe fn calculate_luminance_raw_avx2(rgb_ptr: *const u8, pixel_count: usize) -> f64 { use std::arch::x86_64::*; @@ -187,6 +176,7 @@ unsafe fn calculate_luminance_raw_avx2(rgb_ptr: *const u8, pixel_count: usize) - } #[cfg(target_arch = "aarch64")] +#[allow(dead_code)] unsafe fn calculate_luminance_raw_neon(rgb_ptr: *const u8, pixel_count: usize) -> f64 { use std::arch::aarch64::*; diff --git a/src/python_bindings.rs b/src/python_bindings.rs index bfd124d..79b2526 100644 --- a/src/python_bindings.rs +++ b/src/python_bindings.rs @@ -35,6 +35,18 @@ fn get_buffer_pool() -> &'static Arc> { BUFFER_POOL.get_or_init(|| Arc::new(Mutex::new(BufferPool::new()))) } +#[cfg(feature = "python-bindings")] +fn ensure_c_contiguous(image: &ndarray::ArrayView3<'_, u8>, operation: &str) -> PyResult<()> { + if image.as_slice().is_none() { + return Err(pyo3::exceptions::PyValueError::new_err(format!( + "{} requires a C-contiguous array", + operation + ))); + } + + Ok(()) +} + #[cfg(feature = "python-bindings")] struct BufferPool { pools: std::collections::HashMap<(usize, usize, usize), VecDeque>>, @@ -120,6 +132,7 @@ pub unsafe fn batch_crop_images_zero_copy<'py>( for (image, &(x, y, width, height)) in images.iter().zip(crop_boxes.iter()) { let img_view = image.as_array(); + ensure_c_contiguous(&img_view, "batch_crop_images_zero_copy")?; let (src_height, src_width, channels) = img_view.dim(); if x + width > src_width || y + height > src_height { @@ -168,6 +181,7 @@ pub unsafe fn batch_center_crop_images_zero_copy<'py>( for (image, &(target_width, target_height)) in images.iter().zip(target_sizes.iter()) { let img_view = image.as_array(); + ensure_c_contiguous(&img_view, "batch_center_crop_images_zero_copy")?; let (src_height, src_width, channels) = img_view.dim(); let start_x = (src_width.saturating_sub(target_width)) / 2; @@ -204,49 +218,13 @@ pub unsafe fn batch_center_crop_images_zero_copy<'py>( pub fn batch_calculate_luminance_zero_copy( images: Vec>, ) -> PyResult> { - use rayon::prelude::*; - - // Extract raw pointers and dimensions first (on main thread) - #[derive(Clone, Copy)] - struct ImageData { - ptr: *const u8, - width: usize, - height: usize, - channels: usize, - } - - unsafe impl Send for ImageData {} - unsafe impl Sync for ImageData {} - - let image_data: Vec = images + images .iter() .map(|image| { let img_view = image.as_array(); - let (height, width, channels) = img_view.dim(); - let ptr = img_view.as_ptr(); - ImageData { - ptr, - width, - height, - channels, - } + Ok(crate::luminance::calculate_luminance_array(&img_view)) }) - .collect(); - - // Now process the raw pointers in parallel - let luminances: Vec = image_data - .par_iter() - .map(|data| unsafe { - crate::luminance::calculate_luminance_raw_buffer( - data.ptr, - data.width, - data.height, - data.channels, - ) - }) - .collect(); - - Ok(luminances) + .collect() } #[cfg(all(feature = "python-bindings", feature = "opencv"))] @@ -326,18 +304,19 @@ pub fn batch_resize_images_zero_copy<'py>( .zip(sizes_vec.iter()) .map(|(image, &(target_width, target_height))| { let img_view = image.as_array(); + ensure_c_contiguous(&img_view, "batch_resize_images_zero_copy")?; let (src_height, src_width, src_channels) = img_view.dim(); let src_ptr = img_view.as_ptr(); - ResizeData { + Ok(ResizeData { src_ptr, src_height, src_width, src_channels, target_width, target_height, - } + }) }) - .collect(); + .collect::>()?; // Adaptive processing: use parallel only for larger batches to avoid threading overhead let use_parallel = batch_size >= 8; // Threshold where parallel processing becomes beneficial @@ -506,6 +485,7 @@ fn resize_single_image_direct<'py>( }; let img_view = image.as_array(); + ensure_c_contiguous(&img_view, "batch_resize_images_zero_copy")?; let (src_height, src_width, channels) = img_view.dim(); let (target_width, target_height) = target_size; @@ -722,18 +702,19 @@ pub fn batch_resize_images_iterator<'py>( .zip(target_sizes.iter()) .map(|(image, &(target_width, target_height))| { let img_view = image.as_array(); + ensure_c_contiguous(&img_view, "batch_resize_images_iterator")?; let (src_height, src_width, src_channels) = img_view.dim(); let src_ptr = img_view.as_ptr(); - ResizeData { + Ok(ResizeData { src_ptr, src_height, src_width, src_channels, target_width, target_height, - } + }) }) - .collect(); + .collect::>()?; // Adaptive processing: use parallel only for larger batches let use_parallel = batch_size >= 8; diff --git a/src/tests.rs b/src/tests.rs index d74516f..44e0619 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -137,6 +137,30 @@ mod luminance_tests { // Pure black should have zero luminance assert!((luminance - 0.0).abs() < 1.0); } + + #[test] + fn test_raw_buffer_luminance_matches_scalar_for_large_image() { + let img = Array3::from_shape_fn((512, 512, 3), |(y, x, c)| ((x + y + c * 17) % 256) as u8); + let expected = calculate_luminance_scalar(&img.view()); + let actual = unsafe { + calculate_luminance_raw_buffer(img.as_ptr(), img.dim().1, img.dim().0, img.dim().2) + }; + + assert!((actual - expected).abs() < 1e-9); + } + + #[test] + fn test_raw_buffer_luminance_zero_sized_image_returns_zero() { + let luminance = unsafe { calculate_luminance_raw_buffer(std::ptr::null(), 0, 0, 3) }; + assert_eq!(luminance, 0.0); + } + + #[test] + fn test_raw_buffer_luminance_non_rgb_returns_zero() { + let data = [42u8; 16]; + let luminance = unsafe { calculate_luminance_raw_buffer(data.as_ptr(), 2, 2, 1) }; + assert_eq!(luminance, 0.0); + } } #[cfg(test)] diff --git a/tests/test_performance_benchmarks.py b/tests/test_performance_benchmarks.py index 95b5f99..c0be040 100644 --- a/tests/test_performance_benchmarks.py +++ b/tests/test_performance_benchmarks.py @@ -1,6 +1,7 @@ """Enhanced performance benchmarks and stress tests for training_sample_rust.""" import gc +import importlib.util import threading import time @@ -21,7 +22,7 @@ except ImportError: HAS_OPENCV = False -HAS_BENCHMARK = True +HAS_BENCHMARK = importlib.util.find_spec("pytest_benchmark") is not None pytestmark = pytest.mark.skipif( not HAS_BINDINGS, reason="Python bindings not available" diff --git a/tests/test_python_bindings.py b/tests/test_python_bindings.py index abeae64..d00637f 100644 --- a/tests/test_python_bindings.py +++ b/tests/test_python_bindings.py @@ -1,5 +1,7 @@ """Tests for Python bindings of training_sample_rust.""" +import importlib.util + import numpy as np import pytest @@ -10,7 +12,7 @@ except ImportError: HAS_BINDINGS = False -HAS_BENCHMARK = True +HAS_BENCHMARK = importlib.util.find_spec("pytest_benchmark") is not None pytestmark = pytest.mark.skipif( not HAS_BINDINGS, reason="Python bindings not available" @@ -94,6 +96,67 @@ def test_batch_resize_videos(self, sample_video): assert results[0].shape == (10, 64, 64, 3) +class TestZeroCopyBindings: + """Test zero-copy binding behavior and safety checks.""" + + def test_zero_copy_luminance_matches_standard_path(self): + """Zero-copy luminance should match the standard implementation.""" + image = np.random.randint(0, 255, (512, 512, 3), dtype=np.uint8) + + expected = tsr.batch_calculate_luminance([image])[0] + actual = tsr.batch_calculate_luminance_zero_copy([image])[0] + + assert actual == pytest.approx(expected, abs=1e-6) + + def test_zero_copy_luminance_accepts_non_contiguous_views(self): + """Zero-copy luminance should handle strided arrays safely.""" + image = np.random.randint(0, 255, (256, 256, 3), dtype=np.uint8) + strided = image[:, ::2, :] + + expected = tsr.batch_calculate_luminance([strided])[0] + actual = tsr.batch_calculate_luminance_zero_copy([strided])[0] + + assert actual == pytest.approx(expected, abs=1e-6) + + def test_zero_copy_crop_rejects_non_contiguous_array(self): + """Unsafe zero-copy crop path should reject non-contiguous input.""" + image = np.random.randint(0, 255, (64, 64, 3), dtype=np.uint8) + strided = image[:, ::2, :] + + with pytest.raises(ValueError, match="C-contiguous"): + tsr.batch_crop_images_zero_copy([strided], [(0, 0, 16, 16)]) + + def test_zero_copy_center_crop_rejects_non_contiguous_array(self): + """Unsafe zero-copy center crop path should reject non-contiguous input.""" + image = np.random.randint(0, 255, (64, 64, 3), dtype=np.uint8) + strided = image[::2, :, :] + + with pytest.raises(ValueError, match="C-contiguous"): + tsr.batch_center_crop_images_zero_copy([strided], [(16, 16)]) + + def test_zero_copy_resize_rejects_non_contiguous_array(self): + """Unsafe zero-copy resize path should reject non-contiguous input.""" + if not hasattr(tsr, "batch_resize_images_zero_copy"): + pytest.skip("OpenCV zero-copy resize bindings not available") + + image = np.random.randint(0, 255, (64, 64, 3), dtype=np.uint8) + strided = image[:, ::2, :] + + with pytest.raises(ValueError, match="C-contiguous"): + tsr.batch_resize_images_zero_copy(strided, (16, 16)) + + def test_zero_copy_resize_iterator_rejects_non_contiguous_array(self): + """Unsafe zero-copy iterator resize path should reject non-contiguous input.""" + if not hasattr(tsr, "batch_resize_images_iterator"): + pytest.skip("OpenCV resize iterator bindings not available") + + image = np.random.randint(0, 255, (64, 64, 3), dtype=np.uint8) + strided = image[:, ::2, :] + + with pytest.raises(ValueError, match="C-contiguous"): + tsr.batch_resize_images_iterator([strided], [(16, 16)]) + + class TestErrorHandling: """Test error handling and edge cases."""