From 01bb705b7b676abebef7aa289c00a003ae430f56 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 02:18:54 +0000 Subject: [PATCH] chore: resolve SonarCloud maintainability issues --- apps/photocraft-cli/tests/it/cli.rs | 2 +- apps/photocraft-web/src/web.rs | 10 +- crates/algo/src/selection/lasso.rs | 2 +- crates/algo/src/smart_blur_tests.rs | 2 +- crates/cms/src/transform.rs | 6 +- crates/codecs/src/codecs/png.rs | 4 +- crates/compose/src/lib.rs | 70 +++++++------ .../engine/src/atn_cmds/layer_props_tests.rs | 5 +- crates/engine/src/edit_menu_cmds.rs | 2 +- crates/format/src/store/durability_tests.rs | 12 ++- crates/io/src/pixels.rs | 2 +- crates/io/tests/it/psd_kerning.rs | 4 +- crates/raster/src/lib.rs | 97 ++++++++++--------- crates/raw/src/tiff.rs | 32 +++--- .../ui-egui/src/camera_raw_identity_tests.rs | 4 +- crates/ui-egui/src/layer_style.rs | 95 +++++++++--------- crates/ui-egui/src/liquify_ui.rs | 2 +- crates/ui-egui/src/swatches_ui.rs | 3 +- xtask/src/pinned.rs | 3 +- 19 files changed, 193 insertions(+), 164 deletions(-) diff --git a/apps/photocraft-cli/tests/it/cli.rs b/apps/photocraft-cli/tests/it/cli.rs index 61778d0fd3..7b8dcd9ab7 100644 --- a/apps/photocraft-cli/tests/it/cli.rs +++ b/apps/photocraft-cli/tests/it/cli.rs @@ -1000,7 +1000,7 @@ fn batch_plays_a_photoshop_actions_file() { /// `photocraft_cli::run` in-process: `(exit code, stdout, stderr)`. fn invoke(args: &[&str]) -> (i32, String, String) { let (mut out, mut err) = (Vec::new(), Vec::new()); - let args: Vec = args.iter().map(|s| s.to_string()).collect(); + let args: Vec = args.iter().map(ToString::to_string).collect(); let code = photocraft_cli::run(&args, &mut out, &mut err); (code, String::from_utf8(out).unwrap(), String::from_utf8(err).unwrap()) } diff --git a/apps/photocraft-web/src/web.rs b/apps/photocraft-web/src/web.rs index 20e64fa046..355ff9e534 100644 --- a/apps/photocraft-web/src/web.rs +++ b/apps/photocraft-web/src/web.rs @@ -4,7 +4,7 @@ use std::cell::Cell; use std::collections::{HashMap, HashSet}; use std::rc::Rc; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, PoisonError}; use photocraft_codecs::{ChannelLayout, EncodeOptions, Image}; use photocraft_doc::Document; @@ -218,7 +218,7 @@ fn load_font_manifest(served: Served, ctx: egui::Context) { files.entry(f.family.clone()).or_default().push((f.file.clone(), f.integrity.clone())); } log::info!("photocraft-web: {} served font families", files.len()); - served.lock().unwrap_or_else(|e| e.into_inner()).files = files; + served.lock().unwrap_or_else(PoisonError::into_inner).files = files; // The families for the font menus, and the script fallbacks (the manifest's `scripts`). served_fonts::add_fonts(&fonts); // Fetch now the fallbacks of the scripts the browser's languages use (an Arabic reader gets @@ -295,7 +295,7 @@ impl WebShell { fn serve_fonts(&mut self, ctx: &egui::Context) { let requested = served_fonts::take_requests(); let (fetch, arrived) = { - let mut s = self.served.lock().unwrap_or_else(|e| e.into_inner()); + let mut s = self.served.lock().unwrap_or_else(PoisonError::into_inner); let mut fetch = Vec::new(); for family in requested { if let Some(files) = s.files.get(&family).cloned() @@ -312,7 +312,7 @@ impl WebShell { wasm_bindgen_futures::spawn_local(async move { match fetch_bytes(&file, integrity.as_deref()).await { Ok(bytes) => { - served.lock().unwrap_or_else(|e| e.into_inner()).arrived.push((family, bytes)); + served.lock().unwrap_or_else(PoisonError::into_inner).arrived.push((family, bytes)); ctx.request_repaint(); } Err(e) => log::error!("couldn't fetch served font {file}: {e}"), @@ -347,7 +347,7 @@ impl eframe::App for WebShell { match f.bytes_async().await { Ok(bytes) => { let queue = if place { &place_inbox } else { &inbox }; - queue.lock().unwrap_or_else(|e| e.into_inner()).push((name, bytes)); + queue.lock().unwrap_or_else(PoisonError::into_inner).push((name, bytes)); ctx.request_repaint(); } Err(e) => log::error!("couldn't read dropped file {name}: {e}"), diff --git a/crates/algo/src/selection/lasso.rs b/crates/algo/src/selection/lasso.rs index 3ca4eacd52..2471a379b5 100644 --- a/crates/algo/src/selection/lasso.rs +++ b/crates/algo/src/selection/lasso.rs @@ -123,7 +123,7 @@ pub fn polygon_surface(points: &[(f32, f32)], clip: Rect) -> Option { let crossings = |e: &[&Edge], y: f32, xs: &mut Vec| { xs.clear(); xs.extend(e.iter().map(|&&Edge { a, b, .. }| a.0 + (y - a.1) / (b.1 - a.1) * (b.0 - a.0))); - xs.sort_by(|a, b| a.total_cmp(b)); + xs.sort_by(f32::total_cmp); }; let full = s.solid_tile(&[1.0]); let blank = s.solid_tile(&[0.0]); diff --git a/crates/algo/src/smart_blur_tests.rs b/crates/algo/src/smart_blur_tests.rs index 47290e2425..b1f37b1f7e 100644 --- a/crates/algo/src/smart_blur_tests.rs +++ b/crates/algo/src/smart_blur_tests.rs @@ -1,6 +1,6 @@ //! The pre-#7921 Smart Blur (every neighbour read per channel through `Image::get`) stays as an //! exact oracle: the offset fast path must give the same bits for every mode, depth and layout. -use super::*; +use super::{native, ncol, smart_blur, unpremul_px, xy, BlurQuality, Ctx, Image, Rect, SmartBlurMode, MAXC}; use photocraft_color::ColorMode; #[allow(clippy::too_many_arguments)] diff --git a/crates/cms/src/transform.rs b/crates/cms/src/transform.rs index 3ce59687a7..5cd6c65c57 100644 --- a/crates/cms/src/transform.rs +++ b/crates/cms/src/transform.rs @@ -2,7 +2,7 @@ //! compensation, soft-proofing chains, and fast lookup-table evaluation of pixel buffers. use std::collections::HashMap; -use std::sync::{Arc, Mutex, OnceLock}; +use std::sync::{Arc, Mutex, OnceLock, PoisonError}; use crate::clut::{Clut, MAX_CHANNELS}; use crate::curve::Curve; @@ -718,14 +718,14 @@ pub fn cached(src: &Profile, dst: &Profile, opts: TransformOptions) -> Result return Ok(t), Some(None) => return Ok(Arc::new(Transform::with_options(src, dst, opts)?)), None => {} } let t = Arc::new(Transform::with_options(src, dst, opts)?); - let mut c = cache.lock().unwrap_or_else(|e| e.into_inner()); + let mut c = cache.lock().unwrap_or_else(PoisonError::into_inner); if c.len() > 64 { c.clear(); } diff --git a/crates/codecs/src/codecs/png.rs b/crates/codecs/src/codecs/png.rs index 085246b3f1..7fbbbc69db 100644 --- a/crates/codecs/src/codecs/png.rs +++ b/crates/codecs/src/codecs/png.rs @@ -140,7 +140,7 @@ pub fn stereo(bytes: &[u8]) -> Option { } let Ok(length) = usize::try_from(length) else { break }; let Some(data) = tail.get(..length) else { break }; - let Some((checksum, next)) = tail.get(length..).and_then(|t| t.split_first_chunk::<4>()) else { break }; + let Some((checksum, next)) = tail.get(length..).and_then(<[u8]>::split_first_chunk::<4>) else { break }; rest = next; if kind == b"IEND" { break; @@ -194,7 +194,7 @@ fn read_text_metadata(bytes: &[u8], mut remaining: usize, meta: &mut Metadata) - } let Ok(length) = usize::try_from(length) else { break }; let Some(data) = tail.get(..length) else { break }; - let Some((checksum, next)) = tail.get(length..).and_then(|t| t.split_first_chunk::<4>()) else { break }; + let Some((checksum, next)) = tail.get(length..).and_then(<[u8]>::split_first_chunk::<4>) else { break }; rest = next; if kind == b"IEND" { break; diff --git a/crates/compose/src/lib.rs b/crates/compose/src/lib.rs index 4786bc1912..21aceac983 100644 --- a/crates/compose/src/lib.rs +++ b/crates/compose/src/lib.rs @@ -2334,6 +2334,42 @@ fn effect_maps(layer: &Layer, rect: Rect, cx: &Ctx) -> std::sync::Arc std::sync::Arc { + let shape: Vec = if region.is_empty() { Vec::new() } else { effect_shape(layer, region, cx) }; + let maps = effects::build_maps_prepared(layer, shape, region, &cx.light, &texture_ctx(layer, region, cx), cx.patterns); + let mut pin = Vec::new(); + layer_identity(layer, &mut std::collections::hash_map::DefaultHasher::new(), &mut |t| pin.push(std::sync::Arc::downgrade(t))); + // The maps, plus what each reserved tile address keeps allocated. + let bytes = + maps.bytes().saturating_add(pin.len().saturating_mul(std::mem::size_of::() + 2 * std::mem::size_of::())); + let maps = std::sync::Arc::new(maps); + if slot.set(FxEntry { maps: maps.clone(), _pin: pin, bytes }).is_ok() { + // Counted exactly once, by the build that filled the slot, and only while the + // slot is still cached (eviction may have dropped it meanwhile). + let mut c = fx_cache().lock().unwrap_or_else(|e| e.into_inner()); + if c.map.get(&key).is_some_and(|s| std::sync::Arc::ptr_eq(s, slot)) { + c.bytes += bytes; + } + } + // A concurrent build that finished first wins, so every tile shares one map. + slot.get().map_or(maps, |e| e.maps.clone()) +} + +fn evict_fx_cache_over_budget(key: u64) { + let mut c = fx_cache().lock().unwrap_or_else(|e| e.into_inner()); + let budget = effect_cache_budget(); + while c.bytes > budget && c.order.len() > 1 { + let Some(old) = c.order.pop_front() else { break }; + if old == key { + c.order.push_back(old); + continue; + } + if let Some(s) = c.map.remove(&old) { + c.bytes = c.bytes.saturating_sub(s.get().map_or(0, |e| e.bytes)); + } + } +} + /// The layer's effect maps from the global cache, built on a miss. /// /// Never waits for another thread's build (#276): this can run on a Rayon worker inside a tile, @@ -2355,40 +2391,10 @@ fn cached_effect_maps(layer: &Layer, region: Rect, key: u64, cx: &Ctx) -> std::s }; let maps = match slot.get() { Some(e) => e.maps.clone(), - None => { - let shape: Vec = if region.is_empty() { Vec::new() } else { effect_shape(layer, region, cx) }; - let maps = effects::build_maps_prepared(layer, shape, region, &cx.light, &texture_ctx(layer, region, cx), cx.patterns); - let mut pin = Vec::new(); - layer_identity(layer, &mut std::collections::hash_map::DefaultHasher::new(), &mut |t| pin.push(std::sync::Arc::downgrade(t))); - // The maps, plus what each reserved tile address keeps allocated. - let bytes = - maps.bytes().saturating_add(pin.len().saturating_mul(std::mem::size_of::() + 2 * std::mem::size_of::())); - let maps = std::sync::Arc::new(maps); - if slot.set(FxEntry { maps: maps.clone(), _pin: pin, bytes }).is_ok() { - // Counted exactly once, by the build that filled the slot, and only while the - // slot is still cached (eviction may have dropped it meanwhile). - let mut c = fx_cache().lock().unwrap_or_else(|e| e.into_inner()); - if c.map.get(&key).is_some_and(|s| std::sync::Arc::ptr_eq(s, &slot)) { - c.bytes += bytes; - } - } - // A concurrent build that finished first wins, so every tile shares one map. - slot.get().map_or(maps, |e| e.maps.clone()) - } + None => build_fx_slot_maps(layer, region, key, cx, &slot), }; // Evict the oldest entries over budget (never the one just used). - let mut c = fx_cache().lock().unwrap_or_else(|e| e.into_inner()); - let budget = effect_cache_budget(); - while c.bytes > budget && c.order.len() > 1 { - let Some(old) = c.order.pop_front() else { break }; - if old == key { - c.order.push_back(old); - continue; - } - if let Some(s) = c.map.remove(&old) { - c.bytes = c.bytes.saturating_sub(s.get().map_or(0, |e| e.bytes)); - } - } + evict_fx_cache_over_budget(key); maps } diff --git a/crates/engine/src/atn_cmds/layer_props_tests.rs b/crates/engine/src/atn_cmds/layer_props_tests.rs index 155729923a..4743163340 100644 --- a/crates/engine/src/atn_cmds/layer_props_tests.rs +++ b/crates/engine/src/atn_cmds/layer_props_tests.rs @@ -1,4 +1,7 @@ -use super::*; +use photocraft_psd::atn; +use serde_json::json; + +use super::{enm, export, export_item, get_obj, layer_ref, pct, text, trimmed, Action, Session, Skipped}; /// PhotoCraft's own roundtrip accepts a combined Set, but Photoshop applies only its rename. #[test] diff --git a/crates/engine/src/edit_menu_cmds.rs b/crates/engine/src/edit_menu_cmds.rs index 997c0daacd..0ef173d242 100644 --- a/crates/engine/src/edit_menu_cmds.rs +++ b/crates/engine/src/edit_menu_cmds.rs @@ -1408,7 +1408,7 @@ fn export_import(s: &mut Session, p: &Value) -> Result { .get("kinds") .and_then(Value::as_array) .map(|a| a.iter().filter_map(Value::as_str).map(str::to_string).collect()) - .unwrap_or_else(|| ["brushes", "customShapes", "tools"].iter().map(|k| k.to_string()).collect()); + .unwrap_or_else(|| ["brushes", "customShapes", "tools"].iter().map(ToString::to_string).collect()); let want = |k: &str| kinds.iter().any(|x| x == k); match str_or(p, "action", "export") { "export" => { diff --git a/crates/format/src/store/durability_tests.rs b/crates/format/src/store/durability_tests.rs index 5f60db2948..cf75e25963 100644 --- a/crates/format/src/store/durability_tests.rs +++ b/crates/format/src/store/durability_tests.rs @@ -1,10 +1,14 @@ //! Exercise the real directory save, including cached-object reuse and garbage collection. -use super::*; -use crate::atomic::{AtomicFs, RealFs}; -use photocraft_color::{ColorMode, SampleType}; -use photocraft_doc::{Layer, Size}; use std::cell::{Cell, RefCell}; use std::io; +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +use photocraft_color::{ColorMode, SampleType}; +use photocraft_doc::{Document, Layer, Size}; + +use super::{list_objects, PcraftWriter, SaveOptions, MANIFEST}; +use crate::atomic::{AtomicFs, RealFs}; /// The object or manifest directory barrier at which to inject one publication failure. #[derive(Clone, Copy, Debug)] diff --git a/crates/io/src/pixels.rs b/crates/io/src/pixels.rs index 0251f83755..810d1e355f 100644 --- a/crates/io/src/pixels.rs +++ b/crates/io/src/pixels.rs @@ -47,7 +47,7 @@ pub fn interleave(planes: &[Option<&[u8]>], fill: &[Vec], n: usize, s: Sampl return out; } let bs = bands(n); - let step = bs.first().map_or(n, |b| b.len()) * row; + let step = bs.first().map_or(n, ExactSizeIterator::len) * row; par_map(bs.into_iter().zip(out.chunks_mut(step)).collect(), |(band, part)| { for (c, plane) in planes.iter().enumerate() { let src = plane.and_then(|p| p.get(band.start * bps..)).unwrap_or_default(); diff --git a/crates/io/tests/it/psd_kerning.rs b/crates/io/tests/it/psd_kerning.rs index ad492fce25..d9fb3be6fd 100644 --- a/crates/io/tests/it/psd_kerning.rs +++ b/crates/io/tests/it/psd_kerning.rs @@ -224,11 +224,11 @@ fn add_type_layer(objects: &[(&str, &str)], kept: &[(&str, i32)], at: usize) -> .iter() .map(|l| { let LayerContent::Text(t) = &l.content else { panic!("not text") }; - let index = t.psd_raw.as_deref().and_then(|d| photocraft_text::psd::text_index(d)); + let index = t.psd_raw.as_deref().and_then(photocraft_text::psd::text_index); (l.name.clone(), index, t.char_runs().iter().flat_map(|r| r.style.features.iter().filter(|f| f.tag != "calt").map(|f| f.tag.clone())).collect()) }) .collect(); - let hi = layers.iter().find(|l| l.0 == "Hi").and_then(|l| l.1).unwrap() as usize; + let hi = usize::try_from(layers.iter().find(|l| l.0 == "Hi").and_then(|l| l.1).unwrap()).unwrap(); let run = txt2.path(&["1", "1"]).and_then(E::as_array).unwrap()[hi].path(&["0", "6", "0"]).and_then(E::as_array).unwrap()[0].clone(); (layers, run) } diff --git a/crates/raster/src/lib.rs b/crates/raster/src/lib.rs index fb6a6fd388..12b9903701 100644 --- a/crates/raster/src/lib.rs +++ b/crates/raster/src/lib.rs @@ -245,6 +245,56 @@ fn convert_tiles(tiles: &mut [(TileCoord, Arc)], r: Rect, data: &[f32], w: } } +fn decode_tile_row_rgba(fmt: &PixelFormat, t: &Tile, base: usize, dst: &mut [[f32; 4]], row: &mut Vec) { + let n = fmt.channels(); + match (fmt.mode, fmt.sample, fmt.alpha) { + (ColorMode::Rgb, SampleType::U8, true) => { + let src = &t.data[base..base + dst.len() * 4]; + for (d, s) in dst.iter_mut().zip(src.as_chunks::<4>().0) { + *d = [s[0] as f32 / 255.0, s[1] as f32 / 255.0, s[2] as f32 / 255.0, s[3] as f32 / 255.0]; + } + } + (ColorMode::Grayscale, SampleType::U8, true) => { + let src = &t.data[base..base + dst.len() * 2]; + for (d, s) in dst.iter_mut().zip(src.as_chunks::<2>().0) { + let g = s[0] as f32 / 255.0; + *d = [g, g, g, s[1] as f32 / 255.0]; + } + } + (ColorMode::Cmyk, SampleType::U8, _) => { + // Identical ink bytes share the exact ICC result; alpha is independent. + let mut previous: Option<([u8; 4], [f32; 3])> = None; + let src = &t.data[base..base + dst.len() * n]; + for (d, s) in dst.iter_mut().zip(src.chunks_exact(n)) { + let ink = [s[0], s[1], s[2], s[3]]; + let rgb = match previous { + Some((key, rgb)) if key == ink => rgb, + _ => { + let rgb = photocraft_color::convert::cmyk_to_rgb(ink.map(|v| v as f32 / 255.0)); + previous = Some((ink, rgb)); + rgb + } + }; + *d = [rgb[0], rgb[1], rgb[2], s.get(4).map_or(1.0, |v| *v as f32 / 255.0)]; + } + } + _ => { + // The row's samples in one pass, then its colour model resolved once: + // RGB and grey take their channels as `to_rgba` does, other models go + // through it. + row.resize(dst.len() * n, 0.0); + read_samples(&t.data, fmt.sample, base, row); + let a = |s: &[f32], i: usize| if fmt.alpha { s[i] } else { 1.0 }; + let pixels = dst.iter_mut().zip(row.chunks_exact(n)); + match fmt.mode { + ColorMode::Rgb => pixels.for_each(|(d, s)| *d = [s[0], s[1], s[2], a(s, 3)]), + ColorMode::Grayscale | ColorMode::Bitmap | ColorMode::Duotone => pixels.for_each(|(d, s)| *d = [s[0], s[0], s[0], a(s, 1)]), + _ => pixels.for_each(|(d, s)| *d = to_rgba(fmt, s)), + } + } + } +} + impl Surface { /// Reads a region into a newly allocated buffer, returning an error instead of aborting if /// the requested pixel buffer cannot be represented or reserved. @@ -577,52 +627,7 @@ impl Surface { continue; }; let base = (((y - tc.ty * TILE_SIZE) as usize) * TILE_SIZE as usize + (tr.x0 - tc.tx * TILE_SIZE) as usize) * n; - match (fmt.mode, fmt.sample, fmt.alpha) { - (ColorMode::Rgb, SampleType::U8, true) => { - let src = &t.data[base..base + dst.len() * 4]; - for (d, s) in dst.iter_mut().zip(src.as_chunks::<4>().0) { - *d = [s[0] as f32 / 255.0, s[1] as f32 / 255.0, s[2] as f32 / 255.0, s[3] as f32 / 255.0]; - } - } - (ColorMode::Grayscale, SampleType::U8, true) => { - let src = &t.data[base..base + dst.len() * 2]; - for (d, s) in dst.iter_mut().zip(src.as_chunks::<2>().0) { - let g = s[0] as f32 / 255.0; - *d = [g, g, g, s[1] as f32 / 255.0]; - } - } - (ColorMode::Cmyk, SampleType::U8, _) => { - // Identical ink bytes share the exact ICC result; alpha is independent. - let mut previous: Option<([u8; 4], [f32; 3])> = None; - let src = &t.data[base..base + dst.len() * n]; - for (d, s) in dst.iter_mut().zip(src.chunks_exact(n)) { - let ink = [s[0], s[1], s[2], s[3]]; - let rgb = match previous { - Some((key, rgb)) if key == ink => rgb, - _ => { - let rgb = photocraft_color::convert::cmyk_to_rgb(ink.map(|v| v as f32 / 255.0)); - previous = Some((ink, rgb)); - rgb - } - }; - *d = [rgb[0], rgb[1], rgb[2], s.get(4).map_or(1.0, |v| *v as f32 / 255.0)]; - } - } - _ => { - // The row's samples in one pass, then its colour model resolved once: - // RGB and grey take their channels as `to_rgba` does, other models go - // through it. - row.resize(dst.len() * n, 0.0); - read_samples(&t.data, fmt.sample, base, &mut row); - let a = |s: &[f32], i: usize| if fmt.alpha { s[i] } else { 1.0 }; - let pixels = dst.iter_mut().zip(row.chunks_exact(n)); - match fmt.mode { - ColorMode::Rgb => pixels.for_each(|(d, s)| *d = [s[0], s[1], s[2], a(s, 3)]), - ColorMode::Grayscale | ColorMode::Bitmap | ColorMode::Duotone => pixels.for_each(|(d, s)| *d = [s[0], s[0], s[0], a(s, 1)]), - _ => pixels.for_each(|(d, s)| *d = to_rgba(&fmt, s)), - } - } - } + decode_tile_row_rgba(&fmt, t, base, dst, &mut row); } } } diff --git a/crates/raw/src/tiff.rs b/crates/raw/src/tiff.rs index df695e3346..6b5c44d15d 100644 --- a/crates/raw/src/tiff.rs +++ b/crates/raw/src/tiff.rs @@ -480,21 +480,21 @@ mod tests { assert!(out.len() < 2 << 20, "dump output was {} bytes", out.len()); assert!(out.contains("output truncated") || out.len() < 1 << 20); } -} -#[test] -fn linked_directory_values_are_read_only_as_needed() { - // The traversal limit is independent of the TIFF entry count: callers should - // never need a complete attacker-controlled offset array to visit SubIFDs. - let mut b = b"II*\0\x08\0\0\0".to_vec(); - b.extend_from_slice(&1u16.to_le_bytes()); - b.extend_from_slice(&330u16.to_le_bytes()); - b.extend_from_slice(&4u16.to_le_bytes()); - b.extend_from_slice(&(1_000_000u32).to_le_bytes()); - b.extend_from_slice(&26u32.to_le_bytes()); - b.extend_from_slice(&0u32.to_le_bytes()); - b.extend_from_slice(&0u32.to_le_bytes()); - b.resize(26 + 16 * 4, 0); - let t = Tiff::new(&b).unwrap(); - assert!(t.all_ifds().len() <= 17); + #[test] + fn linked_directory_values_are_read_only_as_needed() { + // The traversal limit is independent of the TIFF entry count: callers should + // never need a complete attacker-controlled offset array to visit SubIFDs. + let mut b = b"II*\0\x08\0\0\0".to_vec(); + b.extend_from_slice(&1u16.to_le_bytes()); + b.extend_from_slice(&330u16.to_le_bytes()); + b.extend_from_slice(&4u16.to_le_bytes()); + b.extend_from_slice(&(1_000_000u32).to_le_bytes()); + b.extend_from_slice(&26u32.to_le_bytes()); + b.extend_from_slice(&0u32.to_le_bytes()); + b.extend_from_slice(&0u32.to_le_bytes()); + b.resize(26 + 16 * 4, 0); + let t = Tiff::new(&b).unwrap(); + assert!(t.all_ifds().len() <= 17); + } } diff --git a/crates/ui-egui/src/camera_raw_identity_tests.rs b/crates/ui-egui/src/camera_raw_identity_tests.rs index 17c4967ba6..c41213a075 100644 --- a/crates/ui-egui/src/camera_raw_identity_tests.rs +++ b/crates/ui-egui/src/camera_raw_identity_tests.rs @@ -2,7 +2,9 @@ use std::sync::Arc; -use super::*; +use serde_json::{json, Value}; + +use super::{commit_open_raw, finish_redevelop, queue_open_dialog, replace_raw_document, CameraRaw, PhotocraftApp, RawOpen}; fn synthetic_raw() -> Vec { use photocraft_raw::testgen::{DngSpec, mosaic, scene}; diff --git a/crates/ui-egui/src/layer_style.rs b/crates/ui-egui/src/layer_style.rs index 084a48ffa4..37546d772d 100644 --- a/crates/ui-egui/src/layer_style.rs +++ b/crates/ui-egui/src/layer_style.rs @@ -643,6 +643,54 @@ fn validate_params(f: &Map) -> Result<(), String> { Ok(()) } +fn effect_entry(e: &Map, kind: &str, mut params: Value, initial_light_angle: Option) -> Value { + match e.get("fx").filter(|fx| fx.is_object()) { + Some(fx) => { + if kind == "bevelEmboss" + && let Ok(original) = serde_json::from_value::(fx.clone()) + && let Some(edits) = params.as_object_mut() + { + let shown = values_of(&original, initial_light_angle.unwrap_or(120.0) as f32); + // Unedited display values must not quantize imported shading in the snapshot. + for key in ["highlightBlend", "highlightColor", "highlightOpacity", "shadowBlend", "shadowColor", "shadowOpacity"] { + if edits.get(key) == shown.get(key) { + edits.remove(key); + } + } + } + json!({"kind": kind, "fx": fx, "params": params}) + } + None => { + // A new instance: the page shows the factory defaults merged with + // the edits, and the engine builds from the full set. + let mut p = defaults(kind); + if let (Some(o), Some(pv)) = (p.as_object_mut(), params.as_object()) { + for (k, v) in pv { + o.insert(k.clone(), v.clone()); + } + } + json!({"kind": kind, "params": p}) + } + } +} + +fn find_light_angle(f: &Map, initial_light_angle: Option) -> Option { + let mut light_angle: Option = None; + for e in effects_of(f) { + if e.get("on").and_then(Value::as_bool) != Some(true) { + continue; + } + let params = e.get("params").cloned().unwrap_or_else(|| json!({})); + if params.get("useGlobalLight").and_then(Value::as_bool) == Some(true) + && let Some(angle) = params.get("angle").and_then(Value::as_f64) + && initial_light_angle.is_none_or(|initial| angle != initial) + { + light_angle = Some(angle); + } + } + light_angle +} + /// Runs the dialog's commands through `run`: blending options, then the whole /// effect list in one `layer.layerStyle.replace` (instances keep their order; /// their snapshots carry what the dialog doesn't model). @@ -662,55 +710,14 @@ fn apply(f: &Map, mut run: impl FnMut(&str, Value) -> Result { - if kind == "bevelEmboss" - && let Ok(original) = serde_json::from_value::(fx.clone()) - && let Some(edits) = params.as_object_mut() - { - let shown = values_of(&original, initial_light_angle.unwrap_or(120.0) as f32); - // Unedited display values must not quantize imported shading in the snapshot. - for key in ["highlightBlend", "highlightColor", "highlightOpacity", "shadowBlend", "shadowColor", "shadowOpacity"] { - if edits.get(key) == shown.get(key) { - edits.remove(key); - } - } - } - json!({"kind": kind, "fx": fx, "params": params}) - } - None => { - // A new instance: the page shows the factory defaults merged with - // the edits, and the engine builds from the full set. - let mut p = defaults(kind); - if let (Some(o), Some(pv)) = (p.as_object_mut(), params.as_object()) { - for (k, v) in pv { - o.insert(k.clone(), v.clone()); - } - } - json!({"kind": kind, "params": p}) - } - }; - entries.push(entry); + entries.push(effect_entry(e, kind, params, initial_light_angle)); } run("layer.layerStyle.replace", json!({"layer": layer, "effects": entries}))?; // An effect lit by the global light follows the document's light angle, so the Angle slider // drives that shared angle rather than the per-effect angle the compositor ignores. - let mut light_angle: Option = None; - for e in effects_of(f) { - if e.get("on").and_then(Value::as_bool) != Some(true) { - continue; - } - let params = e.get("params").cloned().unwrap_or_else(|| json!({})); - if params.get("useGlobalLight").and_then(Value::as_bool) == Some(true) - && let Some(angle) = params.get("angle").and_then(Value::as_f64) - && initial_light_angle.is_none_or(|initial| angle != initial) - { - light_angle = Some(angle); - } - } + let light_angle = find_light_angle(f, initial_light_angle); if let Some(angle) = f.get("pendingGlobalLight").and_then(Value::as_f64).or(light_angle) { run("layer.layerStyle.globalLight", json!({"angle": angle}))?; } diff --git a/crates/ui-egui/src/liquify_ui.rs b/crates/ui-egui/src/liquify_ui.rs index 43a792d3ff..4ebfc84316 100644 --- a/crates/ui-egui/src/liquify_ui.rs +++ b/crates/ui-egui/src/liquify_ui.rs @@ -59,7 +59,7 @@ pub enum BackdropMode { } impl BackdropMode { - const ALL: [(BackdropMode, &'static str); 3] = [(BackdropMode::InFront, "inFront"), (BackdropMode::Behind, "behind"), (BackdropMode::Blend, "blend")]; + const ALL: [(BackdropMode, &str); 3] = [(BackdropMode::InFront, "inFront"), (BackdropMode::Behind, "behind"), (BackdropMode::Blend, "blend")]; fn key(self) -> &'static str { Self::ALL.iter().find(|(m, _)| *m == self).map_or("inFront", |(_, k)| k) diff --git a/crates/ui-egui/src/swatches_ui.rs b/crates/ui-egui/src/swatches_ui.rs index 81167d6836..83a4dc6174 100644 --- a/crates/ui-egui/src/swatches_ui.rs +++ b/crates/ui-egui/src/swatches_ui.rs @@ -10,6 +10,7 @@ //! `swatches.*` engine command; this module only draws and dispatches. Colours are shown in the //! RGB working space through `photocraft-cms` (cached per library revision and colour settings). +use std::ffi::OsStr; use std::sync::Arc; use egui::{Color32, CornerRadius, Rect}; @@ -157,7 +158,7 @@ pub fn import_bytes(app: &mut PhotocraftApp, name: &str, bytes: &[u8], replace: p["mode"] = json!(if replace { "replace" } else { "append" }); // Bytes without a path (the browser, drag and drop) are sniffed by content, and `.act` has // no signature: one starting 00 01 or 00 02 reads as `.aco`. Its file name decides. - if std::path::Path::new(name).extension().and_then(|e| e.to_str()).is_some_and(|e| e.eq_ignore_ascii_case("act")) { + if std::path::Path::new(name).extension().and_then(OsStr::to_str).is_some_and(|e| e.eq_ignore_ascii_case("act")) { p["format"] = json!("act"); } if let Some(stem) = stem { diff --git a/xtask/src/pinned.rs b/xtask/src/pinned.rs index baf798f1de..ddca8a6163 100644 --- a/xtask/src/pinned.rs +++ b/xtask/src/pinned.rs @@ -9,6 +9,7 @@ //! `--update-manifest` rewrites the manifest from the download (after moving a pin). //! Local mode copies from a working clone instead of downloading (see `fetch_local`). +use std::ffi::OsStr; use std::path::{Path, PathBuf}; use std::process::Command; @@ -117,7 +118,7 @@ impl PinnedCorpus { } if ft.is_dir() { self.collect_inner(&p, base, out, visited, dirs); - } else if ft.is_file() && p.extension().and_then(|e| e.to_str()).is_some_and(|e| self.exts.iter().any(|x| e.eq_ignore_ascii_case(x))) { + } else if ft.is_file() && p.extension().and_then(OsStr::to_str).is_some_and(|e| self.exts.iter().any(|x| e.eq_ignore_ascii_case(x))) { out.push(p.strip_prefix(base).unwrap_or(&p).to_path_buf()); } }