Fix edge cases and consistency issues
This commit is contained in:
@@ -321,6 +321,79 @@ impl PipelineExecutor {
|
||||
.map(|m| m.len())
|
||||
.unwrap_or(0);
|
||||
|
||||
// Fast path: if no pixel processing needed (rename-only or rename+metadata),
|
||||
// just copy the file instead of decoding/re-encoding.
|
||||
if !job.needs_pixel_processing() {
|
||||
let output_path = if let Some(ref rename) = job.rename {
|
||||
let stem = source.path.file_stem().and_then(|s| s.to_str()).unwrap_or("output");
|
||||
let ext = source.path.extension().and_then(|e| e.to_str()).unwrap_or("jpg");
|
||||
if let Some(ref template) = rename.template {
|
||||
// Read dimensions without full decode for {width}/{height} templates
|
||||
let dims = image::ImageReader::open(&source.path)
|
||||
.ok()
|
||||
.and_then(|r| r.with_guessed_format().ok())
|
||||
.and_then(|r| r.into_dimensions().ok());
|
||||
let new_name = crate::operations::rename::apply_template_full(
|
||||
template, stem, ext,
|
||||
rename.counter_start.saturating_add(index as u32),
|
||||
dims, Some(ext), Some(&source.path), None,
|
||||
);
|
||||
let new_name = if rename.case_mode > 0 {
|
||||
if let Some(dot_pos) = new_name.rfind('.') {
|
||||
let (name_part, ext_part) = new_name.split_at(dot_pos);
|
||||
format!("{}{}", crate::operations::rename::apply_case_conversion(name_part, rename.case_mode), ext_part)
|
||||
} else {
|
||||
crate::operations::rename::apply_case_conversion(&new_name, rename.case_mode)
|
||||
}
|
||||
} else {
|
||||
new_name
|
||||
};
|
||||
job.output_dir.join(new_name)
|
||||
} else {
|
||||
let new_name = rename.apply_simple(stem, ext, (index as u32).saturating_add(1));
|
||||
job.output_dir.join(new_name)
|
||||
}
|
||||
} else {
|
||||
job.output_path_for(source, None)
|
||||
};
|
||||
|
||||
let output_path = match job.overwrite_behavior {
|
||||
crate::operations::OverwriteAction::Skip if output_path.exists() => {
|
||||
return Ok((input_size, 0));
|
||||
}
|
||||
crate::operations::OverwriteAction::AutoRename if output_path.exists() => {
|
||||
find_unique_path(&output_path)
|
||||
}
|
||||
_ => output_path,
|
||||
};
|
||||
|
||||
if let Some(parent) = output_path.parent() {
|
||||
std::fs::create_dir_all(parent).map_err(PixstripError::Io)?;
|
||||
}
|
||||
|
||||
std::fs::copy(&source.path, &output_path).map_err(PixstripError::Io)?;
|
||||
|
||||
// Metadata handling on the copy
|
||||
if let Some(ref meta_config) = job.metadata {
|
||||
match meta_config {
|
||||
crate::operations::MetadataConfig::KeepAll => {
|
||||
// Already a copy - metadata preserved
|
||||
}
|
||||
crate::operations::MetadataConfig::StripAll => {
|
||||
if !strip_all_metadata(&output_path) {
|
||||
eprintln!("Warning: failed to strip metadata from {}", output_path.display());
|
||||
}
|
||||
}
|
||||
_ => {
|
||||
strip_selective_metadata(&output_path, meta_config);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let output_size = std::fs::metadata(&output_path).map(|m| m.len()).unwrap_or(0);
|
||||
return Ok((input_size, output_size));
|
||||
}
|
||||
|
||||
// Load image
|
||||
let mut img = loader.load_pixels(&source.path)?;
|
||||
|
||||
@@ -404,7 +477,7 @@ impl PipelineExecutor {
|
||||
template,
|
||||
&working_stem,
|
||||
ext,
|
||||
rename.counter_start + index as u32,
|
||||
rename.counter_start.saturating_add(index as u32),
|
||||
dims,
|
||||
original_ext,
|
||||
Some(&source.path),
|
||||
@@ -423,7 +496,7 @@ impl PipelineExecutor {
|
||||
};
|
||||
job.output_dir.join(new_name)
|
||||
} else {
|
||||
let new_name = rename.apply_simple(stem, ext, index as u32 + 1);
|
||||
let new_name = rename.apply_simple(stem, ext, (index as u32).saturating_add(1));
|
||||
job.output_dir.join(new_name)
|
||||
}
|
||||
} else {
|
||||
@@ -432,21 +505,21 @@ impl PipelineExecutor {
|
||||
|
||||
// Handle overwrite behavior
|
||||
let output_path = match job.overwrite_behavior {
|
||||
crate::operations::OverwriteBehavior::Skip => {
|
||||
crate::operations::OverwriteAction::Skip => {
|
||||
if output_path.exists() {
|
||||
// Return 0 bytes written - file was skipped
|
||||
return Ok((input_size, 0));
|
||||
}
|
||||
output_path
|
||||
}
|
||||
crate::operations::OverwriteBehavior::AutoRename => {
|
||||
crate::operations::OverwriteAction::AutoRename => {
|
||||
if output_path.exists() {
|
||||
find_unique_path(&output_path)
|
||||
} else {
|
||||
output_path
|
||||
}
|
||||
}
|
||||
crate::operations::OverwriteBehavior::Overwrite => output_path,
|
||||
crate::operations::OverwriteAction::Overwrite => output_path,
|
||||
};
|
||||
|
||||
// Ensure output directory exists
|
||||
@@ -469,14 +542,18 @@ impl PipelineExecutor {
|
||||
if let Some(ref meta_config) = job.metadata {
|
||||
match meta_config {
|
||||
crate::operations::MetadataConfig::KeepAll => {
|
||||
copy_metadata_from_source(&source.path, &output_path);
|
||||
if !copy_metadata_from_source(&source.path, &output_path) {
|
||||
eprintln!("Warning: failed to copy metadata to {}", output_path.display());
|
||||
}
|
||||
}
|
||||
crate::operations::MetadataConfig::StripAll => {
|
||||
// Already stripped by re-encoding - nothing to do
|
||||
}
|
||||
_ => {
|
||||
// Privacy or Custom: copy all metadata back, then strip unwanted tags
|
||||
copy_metadata_from_source(&source.path, &output_path);
|
||||
if !copy_metadata_from_source(&source.path, &output_path) {
|
||||
eprintln!("Warning: failed to copy metadata to {}", output_path.display());
|
||||
}
|
||||
strip_selective_metadata(&output_path, meta_config);
|
||||
}
|
||||
}
|
||||
@@ -537,9 +614,9 @@ fn auto_orient_from_exif(
|
||||
2 => img.fliph(), // Flipped horizontal
|
||||
3 => img.rotate180(), // Rotated 180
|
||||
4 => img.flipv(), // Flipped vertical
|
||||
5 => img.fliph().rotate270(), // Transposed
|
||||
5 => img.rotate90().fliph(), // Transposed
|
||||
6 => img.rotate90(), // Rotated 90 CW
|
||||
7 => img.fliph().rotate90(), // Transverse
|
||||
7 => img.rotate270().fliph(), // Transverse
|
||||
8 => img.rotate270(), // Rotated 270 CW
|
||||
_ => img,
|
||||
}
|
||||
@@ -569,25 +646,28 @@ fn find_unique_path(path: &std::path::Path) -> std::path::PathBuf {
|
||||
.unwrap_or(0), ext))
|
||||
}
|
||||
|
||||
fn copy_metadata_from_source(source: &std::path::Path, output: &std::path::Path) {
|
||||
// Best-effort: try to copy EXIF from source to output using little_exif.
|
||||
// If it fails (e.g. non-JPEG, no EXIF), silently continue.
|
||||
fn copy_metadata_from_source(source: &std::path::Path, output: &std::path::Path) -> bool {
|
||||
let Ok(metadata) = little_exif::metadata::Metadata::new_from_path(source) else {
|
||||
return;
|
||||
return false;
|
||||
};
|
||||
let _: std::result::Result<(), std::io::Error> = metadata.write_to_file(output);
|
||||
metadata.write_to_file(output).is_ok()
|
||||
}
|
||||
|
||||
fn strip_all_metadata(path: &std::path::Path) -> bool {
|
||||
let empty = little_exif::metadata::Metadata::new();
|
||||
empty.write_to_file(path).is_ok()
|
||||
}
|
||||
|
||||
fn strip_selective_metadata(
|
||||
path: &std::path::Path,
|
||||
config: &crate::operations::MetadataConfig,
|
||||
) {
|
||||
) -> bool {
|
||||
use little_exif::exif_tag::ExifTag;
|
||||
use little_exif::metadata::Metadata;
|
||||
|
||||
// Read the metadata we just wrote back
|
||||
let Ok(source_meta) = Metadata::new_from_path(path) else {
|
||||
return;
|
||||
return false;
|
||||
};
|
||||
|
||||
// Build a set of tag IDs to strip
|
||||
@@ -639,5 +719,5 @@ fn strip_selective_metadata(
|
||||
}
|
||||
}
|
||||
|
||||
let _: std::result::Result<(), std::io::Error> = new_meta.write_to_file(path);
|
||||
new_meta.write_to_file(path).is_ok()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user