diff --git a/frontend/src/lib/components/SettingsPanel.svelte b/frontend/src/lib/components/SettingsPanel.svelte index 634ea43..6641ccb 100644 --- a/frontend/src/lib/components/SettingsPanel.svelte +++ b/frontend/src/lib/components/SettingsPanel.svelte @@ -38,7 +38,6 @@ keep_intermediates: false, }, alignment: { - enabled: true, eye_y_position: 0.35, inter_eye_distance: 0.30, }, @@ -117,7 +116,6 @@ keep_intermediates: false, }, alignment: { - enabled: true, eye_y_position: 0.35, inter_eye_distance: 0.30, }, @@ -367,47 +365,42 @@
Face Alignment - + Always enabled
- {#if config.processing.alignment.enabled} -
- -
- - {(config.processing.alignment.eye_y_position * 100).toFixed(0)}% -
+
+ +
+ + {(config.processing.alignment.eye_y_position * 100).toFixed(0)}%
+
-
- -
- - {(config.processing.alignment.inter_eye_distance * 100).toFixed(0)}% -
+
+ +
+ + {(config.processing.alignment.inter_eye_distance * 100).toFixed(0)}%
- {/if} +
{:else if activeTab === 'output'} diff --git a/src/config.rs b/src/config.rs index 76a9273..16da4e5 100644 --- a/src/config.rs +++ b/src/config.rs @@ -298,11 +298,10 @@ impl EyeFilterConfig { /// /// Aligns faces based on eye positions detected from facial landmarks, /// ensuring consistent eye placement across all images for smoother timelapses. +/// +/// Note: Face alignment is always enabled and is a core part of the pipeline. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct AlignmentConfig { - /// Whether face alignment is enabled. - pub enabled: bool, - /// Target Y position for eyes as percentage from top (0.0-1.0). /// Default 0.35 places eyes at 35% from the top. pub eye_y_position: f32, @@ -315,7 +314,6 @@ pub struct AlignmentConfig { impl Default for AlignmentConfig { fn default() -> Self { Self { - enabled: true, eye_y_position: 0.35, inter_eye_distance: 0.30, } @@ -325,32 +323,30 @@ impl Default for AlignmentConfig { impl AlignmentConfig { /// Validate the configuration values. pub fn validate(&self) -> Result<()> { - if self.enabled { - if self.eye_y_position <= 0.0 || self.eye_y_position >= 1.0 { - return Err(Error::Config( - "Alignment eye_y_position must be between 0.0 and 1.0 (exclusive)".to_string(), - )); - } - if self.eye_y_position < 0.2 || self.eye_y_position > 0.5 { - return Err(Error::Config( - "Alignment eye_y_position should be between 0.2 and 0.5 for best results".to_string(), - )); - } - if self.inter_eye_distance <= 0.0 { - return Err(Error::Config( - "Alignment inter_eye_distance must be greater than 0 to prevent division by zero".to_string(), - )); - } - if self.inter_eye_distance >= 1.0 { - return Err(Error::Config( - "Alignment inter_eye_distance must be less than 1.0".to_string(), - )); - } - if self.inter_eye_distance < 0.2 || self.inter_eye_distance > 0.5 { - return Err(Error::Config( - "Alignment inter_eye_distance should be between 0.2 and 0.5 for best results".to_string(), - )); - } + if self.eye_y_position <= 0.0 || self.eye_y_position >= 1.0 { + return Err(Error::Config( + "Alignment eye_y_position must be between 0.0 and 1.0 (exclusive)".to_string(), + )); + } + if self.eye_y_position < 0.2 || self.eye_y_position > 0.5 { + return Err(Error::Config( + "Alignment eye_y_position should be between 0.2 and 0.5 for best results".to_string(), + )); + } + if self.inter_eye_distance <= 0.0 { + return Err(Error::Config( + "Alignment inter_eye_distance must be greater than 0 to prevent division by zero".to_string(), + )); + } + if self.inter_eye_distance >= 1.0 { + return Err(Error::Config( + "Alignment inter_eye_distance must be less than 1.0".to_string(), + )); + } + if self.inter_eye_distance < 0.2 || self.inter_eye_distance > 0.5 { + return Err(Error::Config( + "Alignment inter_eye_distance should be between 0.2 and 0.5 for best results".to_string(), + )); } Ok(()) } diff --git a/src/pipeline/mod.rs b/src/pipeline/mod.rs index 0c3b3e5..ba2faab 100644 --- a/src/pipeline/mod.rs +++ b/src/pipeline/mod.rs @@ -16,9 +16,9 @@ mod crop_utils; mod debug_utils; mod orientation; +pub mod steps; mod traits; mod types; -pub mod steps; pub use crop_utils::{crop_face_with_intermediate, CropResult}; pub use debug_utils::draw_simple_text; @@ -57,14 +57,9 @@ pub enum PipelineResult { debug_images: Vec<(String, DebugImage)>, }, /// Processing error. - Error { - asset_id: String, - error: String, - }, + Error { asset_id: String, error: String }, /// Cancelled by user. - Cancelled { - asset_id: String, - }, + Cancelled { asset_id: String }, } /// Processing pipeline that executes steps sequentially. @@ -75,7 +70,10 @@ pub struct Pipeline { impl std::fmt::Debug for Pipeline { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("Pipeline") - .field("steps", &self.steps.iter().map(|s| s.id()).collect::>()) + .field( + "steps", + &self.steps.iter().map(|s| s.id()).collect::>(), + ) .finish() } } @@ -100,12 +98,12 @@ impl Pipeline { /// Pipeline order: /// 1. FaceResolutionStep - Validate face size from Immich metadata (if enabled) /// 2. DecodeImageStep - Load and orient the image (always) - /// 3. BrightnessStep - Filter by luminance (if enabled) - /// 4. CropFaceStep - Extract face region with padding (always) + /// 3. CropFaceStep - Extract face region with padding (always) + /// 4. BrightnessStep - Filter by luminance on cropped face (if enabled) /// 5. HeadPoseStep - Filter non-frontal faces (if enabled) - /// 6. LandmarksStep - Detect 68 facial landmarks (if eye_filter or alignment enabled) + /// 6. LandmarksStep - Detect 68 facial landmarks (always) /// 7. EyeFilterStep - Filter closed eyes by EAR (if enabled) - /// 8. AlignmentStep - Align face based on eye positions (if enabled) + /// 8. AlignmentStep - Align face based on eye positions (always) /// 9. ResizeStep - Final resize to output size (always) pub fn with_steps_from_config(config: &Config) -> Self { use steps::*; @@ -120,36 +118,29 @@ impl Pipeline { // Core: Always decode the image pipeline.add_step(Box::new(DecodeImageStep)); - // Optional: Brightness validation + // Core: Always crop the face + pipeline.add_step(Box::new(CropFaceStep)); + + // Optional: Brightness validation (on cropped face region) if config.processing.brightness.enabled { pipeline.add_step(Box::new(BrightnessStep)); } - // Core: Always crop the face - pipeline.add_step(Box::new(CropFaceStep)); - // Optional: Head pose validation if config.processing.head_pose.enabled { pipeline.add_step(Box::new(HeadPoseStep)); } - // Landmarks are needed if eye_filter or alignment is enabled - let needs_landmarks = config.processing.eye_filter.enabled - || config.processing.alignment.enabled; - - if needs_landmarks { - pipeline.add_step(Box::new(LandmarksStep)); - } + // Core: Always detect landmarks (required for alignment) + pipeline.add_step(Box::new(LandmarksStep)); // Optional: Eye filter (requires landmarks) if config.processing.eye_filter.enabled { pipeline.add_step(Box::new(EyeFilterStep)); } - // Optional: Alignment (requires landmarks) - if config.processing.alignment.enabled { - pipeline.add_step(Box::new(AlignmentStep)); - } + // Core: Always align face based on eye positions + pipeline.add_step(Box::new(AlignmentStep)); // Core: Always resize to output size pipeline.add_step(Box::new(ResizeStep)); @@ -165,8 +156,8 @@ impl Pipeline { let mut pipeline = Self::new(); pipeline.add_step(Box::new(FaceResolutionStep)); pipeline.add_step(Box::new(DecodeImageStep)); - pipeline.add_step(Box::new(BrightnessStep)); pipeline.add_step(Box::new(CropFaceStep)); + pipeline.add_step(Box::new(BrightnessStep)); pipeline.add_step(Box::new(HeadPoseStep)); pipeline.add_step(Box::new(LandmarksStep)); pipeline.add_step(Box::new(EyeFilterStep)); @@ -219,7 +210,11 @@ impl Pipeline { } } } - StepOutcome::Skip { mut ctx, reason, detail } => { + StepOutcome::Skip { + mut ctx, + reason, + detail, + } => { // Update skip stats with the step ID as reason skip_stats.increment(&reason); @@ -239,7 +234,8 @@ impl Pipeline { ); // Collect and save debug images before returning - let debug_images: Vec<(String, DebugImage)> = ctx.debug_images.into_iter().collect(); + let debug_images: Vec<(String, DebugImage)> = + ctx.debug_images.into_iter().collect(); if let Some(debug_base) = debug_dir { save_debug_images(debug_base, &debug_images, ×tamp, &asset_id).await; @@ -261,7 +257,8 @@ impl Pipeline { ); // Collect debug images from error context - let debug_images: Vec<(String, DebugImage)> = ctx.debug_images.into_iter().collect(); + let debug_images: Vec<(String, DebugImage)> = + ctx.debug_images.into_iter().collect(); // Save debug images if enabled if let Some(debug_base) = debug_dir { @@ -326,7 +323,13 @@ async fn save_debug_images( let filename = format!("{}_{}.jpg", timestamp, asset_id) .chars() - .map(|c| if c.is_alphanumeric() || c == '-' || c == '_' || c == '.' { c } else { '_' }) + .map(|c| { + if c.is_alphanumeric() || c == '-' || c == '_' || c == '.' { + c + } else { + '_' + } + }) .collect::(); let debug_path = step_dir.join(&filename); @@ -372,16 +375,17 @@ mod tests { config.processing.brightness.enabled = false; config.processing.head_pose.enabled = false; config.processing.eye_filter.enabled = false; - config.processing.alignment.enabled = false; let pipeline = Pipeline::with_steps_from_config(&config); let ids = pipeline.step_ids(); - // Only core steps should be present + // Core steps: decode, crop, landmarks, alignment, resize assert!(ids.contains(&"decode")); assert!(ids.contains(&"crop")); + assert!(ids.contains(&"landmarks")); + assert!(ids.contains(&"alignment")); assert!(ids.contains(&"resize")); - assert_eq!(ids.len(), 3); + assert_eq!(ids.len(), 5); } #[test] @@ -391,17 +395,16 @@ mod tests { config.processing.brightness.enabled = false; config.processing.head_pose.enabled = false; config.processing.eye_filter.enabled = true; - config.processing.alignment.enabled = false; let pipeline = Pipeline::with_steps_from_config(&config); let ids = pipeline.step_ids(); - // Should include landmarks (dependency) and eye_filter + // Core steps (decode, crop, landmarks, alignment, resize) plus eye_filter assert!(ids.contains(&"decode")); assert!(ids.contains(&"crop")); assert!(ids.contains(&"landmarks")); assert!(ids.contains(&"eye_filter")); + assert!(ids.contains(&"alignment")); // Alignment is now always enabled assert!(ids.contains(&"resize")); - assert!(!ids.contains(&"alignment")); } } diff --git a/src/pipeline/steps/alignment.rs b/src/pipeline/steps/alignment.rs index e1ceeb8..a51a6be 100644 --- a/src/pipeline/steps/alignment.rs +++ b/src/pipeline/steps/alignment.rs @@ -29,21 +29,18 @@ impl ProcessingStep for AlignmentStep { } async fn execute(&self, mut ctx: PipelineContext, config: &Config) -> StepOutcome { - // Skip if alignment is disabled - if !config.processing.alignment.enabled { - return StepOutcome::Continue(ctx); - } - - // Get landmarks from previous step + // Get landmarks from previous step (required) let landmarks: crate::pipeline::Landmarks = match ctx .get_computed(computed_keys::LANDMARKS) .and_then(|v| v.as_landmarks()) { Some(l) => l.clone(), None => { - // If landmarks aren't available, skip alignment but continue - tracing::warn!("Landmarks not available, skipping alignment"); - return StepOutcome::Continue(ctx); + // Landmarks should always be available since LandmarksStep is mandatory + return StepOutcome::Error { + ctx, + error: "Landmarks not available for alignment - pipeline misconfigured".to_string(), + }; } }; @@ -176,38 +173,20 @@ mod tests { } #[tokio::test] - async fn test_disabled_skips_alignment() { + async fn test_no_landmarks_errors() { let step = AlignmentStep; let ctx = make_test_ctx(); - let mut config = Config::default(); - config.processing.alignment.enabled = false; - - // Create a dummy image - let img = DynamicImage::ImageRgb8(RgbImage::new(100, 100)); - let ctx = ctx.with_image(img); - - match step.execute(ctx, &config).await { - StepOutcome::Continue(new_ctx) => { - assert!(new_ctx.image.is_some()); - } - other => panic!("Expected Continue when disabled, got {:?}", other), - } - } - - #[tokio::test] - async fn test_no_landmarks_continues() { - let step = AlignmentStep; - let ctx = make_test_ctx(); - let mut config = Config::default(); - config.processing.alignment.enabled = true; + let config = Config::default(); // Create a dummy image but no landmarks let img = DynamicImage::ImageRgb8(RgbImage::new(100, 100)); let ctx = ctx.with_image(img); match step.execute(ctx, &config).await { - StepOutcome::Continue(_) => {} // Expected - continues without alignment - other => panic!("Expected Continue without landmarks, got {:?}", other), + StepOutcome::Error { error, .. } => { + assert!(error.contains("Landmarks not available")); + } + other => panic!("Expected Error without landmarks, got {:?}", other), } } } diff --git a/src/pipeline/steps/landmarks.rs b/src/pipeline/steps/landmarks.rs index bc6f891..601edb8 100644 --- a/src/pipeline/steps/landmarks.rs +++ b/src/pipeline/steps/landmarks.rs @@ -28,15 +28,8 @@ impl ProcessingStep for LandmarksStep { "Landmarks" } - async fn execute(&self, mut ctx: PipelineContext, config: &Config) -> StepOutcome { - // We always need landmarks if alignment is enabled, even if eye filter is disabled - let need_landmarks = - config.processing.alignment.enabled || config.processing.eye_filter.enabled; - - if !need_landmarks { - return StepOutcome::Continue(ctx); - } - + async fn execute(&self, mut ctx: PipelineContext, _config: &Config) -> StepOutcome { + // Landmarks are always needed for face alignment let image = match ctx.require_image("landmark detection") { Ok(img) => img, Err(e) => return StepOutcome::Error { ctx, error: e }, @@ -114,7 +107,6 @@ impl ProcessingStep for LandmarksStep { mod tests { use super::*; use crate::immich_api::FaceData; - use image::{DynamicImage, RgbImage}; fn make_test_ctx() -> PipelineContext { let face_data = FaceData { @@ -128,30 +120,11 @@ mod tests { PipelineContext::new("test".to_string(), "2024-01-01".to_string(), face_data) } - #[tokio::test] - async fn test_disabled_skips_check() { - let step = LandmarksStep; - let ctx = make_test_ctx(); - let mut config = Config::default(); - config.processing.alignment.enabled = false; - config.processing.eye_filter.enabled = false; - - // Create a dummy image - let img = DynamicImage::ImageRgb8(RgbImage::new(100, 100)); - let ctx = ctx.with_image(img); - - match step.execute(ctx, &config).await { - StepOutcome::Continue(_) => {} // Expected - other => panic!("Expected Continue when disabled, got {:?}", other), - } - } - #[tokio::test] async fn test_no_image_error() { let step = LandmarksStep; let ctx = make_test_ctx(); - let mut config = Config::default(); - config.processing.alignment.enabled = true; + let config = Config::default(); match step.execute(ctx, &config).await { StepOutcome::Error { error, .. } => {