diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md new file mode 100644 index 00000000..fe9b7239 --- /dev/null +++ b/.planning/ROADMAP.md @@ -0,0 +1,59 @@ +# Roadmap: Yazi Image Preview Quality Improvement + +## Overview + +Two phases deliver a surgical, single-file change to `yazi-adapter/src/image.rs`. Phase 1 +implements linear-light-correct downscaling using `fast_image_resize`, covering all resize and +compatibility requirements in one coherent pass (the three correctness invariants — gamma +linearization, alpha premultiplication, filter config mapping — cannot be split without creating +partial-correctness windows). Phase 2 adds the unit test for linear-light correctness and +validates the implementation against the full image-type smoke test matrix. + +## Phases + +**Phase Numbering:** +- Integer phases (1, 2, 3): Planned milestone work +- Decimal phases (2.1, 2.2): Urgent insertions (marked with INSERTED) + +Decimal phases appear between their surrounding integers in numeric order. + +- [ ] **Phase 1: Implementation** - Replace both resize call sites with linear-light-correct `fast_image_resize`, add dependency, map all config filter strings +- [ ] **Phase 2: Verification** - Add unit test for linear-light correctness and smoke-test across all image types and terminal protocols + +## Phase Details + +### Phase 1: Implementation +**Goal**: Image previews use linear-light-correct downscaling via `fast_image_resize` with all existing config options preserved +**Depends on**: Nothing (first phase) +**Requirements**: RESIZE-01, RESIZE-02, RESIZE-03, RESIZE-04, RESIZE-05, COMPAT-01, COMPAT-02, COMPAT-03, COMPAT-04 +**Success Criteria** (what must be TRUE): + 1. Opening an image in yazi produces a visibly sharper preview than before (no blurring of fine detail at default lanczos3 filter) + 2. All existing `image_filter` config values (`nearest`, `triangle`, `catmull-rom`, `gaussian`, `lanczos3`) continue to work without error or silent degradation + 3. All terminal protocol drivers (KGP, KGP-old, IIP, Sixel, Chafa, Ueberzug) display images without regressions + 4. `precache()` public signature is unchanged — Lua plugins that call it continue to function + 5. `cargo build --release` succeeds and `cargo clippy --all` produces no new warnings +**Plans:** 1 plan + +Plans: +- [ ] 01-01-PLAN.md — Replace resize pipeline with linear-light fast_image_resize (dependency + code + verification) + +### Phase 2: Verification +**Goal**: The implementation is confirmed correct across all image types, protocols, and edge cases, with at least one automated correctness test +**Depends on**: Phase 1 +**Requirements**: TEST-01 +**Success Criteria** (what must be TRUE): + 1. A unit test exists that downscales a 50% gray sRGB patch to 1x1 and asserts the result is within tolerance of the linear-light-correct value (not the sRGB-space-incorrect midpoint) + 2. RGBA PNG with transparency displays without color fringing on transparent edges in IIP/WezTerm + 3. EXIF-rotated JPEG (phone photo) previews with correct orientation and no quality regression + 4. `cargo test --workspace` passes with the new test included +**Plans**: TBD + +## Progress + +**Execution Order:** +Phases execute in numeric order: 1 -> 2 + +| Phase | Plans Complete | Status | Completed | +|-------|----------------|--------|-----------| +| 1. Implementation | 0/1 | Planned | - | +| 2. Verification | 0/TBD | Not started | - | diff --git a/.planning/phases/01-implementation/01-01-PLAN.md b/.planning/phases/01-implementation/01-01-PLAN.md new file mode 100644 index 00000000..4eaf82f3 --- /dev/null +++ b/.planning/phases/01-implementation/01-01-PLAN.md @@ -0,0 +1,417 @@ +--- +phase: 01-implementation +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - yazi-adapter/Cargo.toml + - yazi-adapter/src/image.rs +autonomous: true +requirements: + - RESIZE-01 + - RESIZE-02 + - RESIZE-03 + - RESIZE-04 + - RESIZE-05 + - COMPAT-01 + - COMPAT-02 + - COMPAT-03 + - COMPAT-04 + +must_haves: + truths: + - "Image downscaling uses linear-light pipeline (sRGB forward map before resize, backward map after)" + - "All 5 config filter strings map to correct fast_image_resize ResizeAlg variants" + - "Unknown filter names log a warning and fall back to lanczos3" + - "Nearest-neighbor resize skips linearization (no wasted buffer allocations)" + - "DynamicImage variant (Luma8, LumaA8, Rgb8, Rgba8) is preserved after resize" + - "Both precache() and downscale() use the new fir_resize path" + - "precache() public signature is unchanged" + - "All resize work remains inside spawn_blocking closures" + artifacts: + - path: "yazi-adapter/Cargo.toml" + provides: "fast_image_resize dependency" + contains: "fast_image_resize" + - path: "yazi-adapter/src/image.rs" + provides: "Linear-light resize implementation" + exports: ["Image"] + contains: "fir_resize" + key_links: + - from: "yazi-adapter/src/image.rs" + to: "fast_image_resize crate" + via: "use fast_image_resize::{...}" + pattern: "create_srgb_mapper|Resizer::new|ResizeOptions" + - from: "yazi-adapter/src/image.rs::precache()" + to: "fir_resize helper" + via: "Self::fir_resize call replacing img.resize()" + pattern: "Self::fir_resize" + - from: "yazi-adapter/src/image.rs::downscale()" + to: "fir_resize helper" + via: "Self::fir_resize call replacing img.resize()" + pattern: "Self::fir_resize" +--- + + +Replace both image resize call sites in yazi-adapter with linear-light-correct downscaling via fast_image_resize. + +Purpose: The current `image` crate `resize()` convolves in gamma-encoded sRGB space, producing visibly blurry thumbnails. Switching to `fast_image_resize` with explicit sRGB linearization (forward_map before resize, backward_map after) produces correct linear-light convolution with SIMD acceleration. + +Output: Modified `yazi-adapter/Cargo.toml` (new dependency) and `yazi-adapter/src/image.rs` (new resize pipeline replacing both call sites). + + + +@~/.claude/get-shit-done/workflows/execute-plan.md +@~/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/01-implementation/01-CONTEXT.md +@.planning/phases/01-implementation/01-RESEARCH.md + + + + +From yazi-adapter/src/image.rs (CURRENT — to be modified): +```rust +pub struct Image; + +impl Image { + // PUBLIC — signature MUST NOT change (called by Lua plugin layer) + pub async fn precache(src: PathBuf, cache: &Path) -> Result<()> + + // pub(super) — called by all protocol drivers + pub(super) async fn downscale(path: PathBuf, rect: Rect) -> Result + + // PRIVATE — to be replaced with resize_alg() + fn filter() -> FilterType + + // Unchanged helpers + pub(super) fn max_pixel(rect: Rect) -> (u16, u16) + pub(super) fn pixel_area(size: (u32, u32), rect: Rect) -> Rect + async fn decode_from(path: PathBuf) -> Result<(DynamicImage, Orientation)> + fn flip_size(orientation: Orientation, (w, h): (u16, u16)) -> (u32, u32) +} +``` + +From yazi-adapter/src/icc.rs (UNCHANGED — produces input to resize): +```rust +pub(super) struct Icc; +impl Icc { + // Can output: GrayImage (Luma8), GrayAlphaImage (LumaA8), RgbImage (Rgb8), RgbaImage (Rgba8) + pub(super) fn transform(mut decoder: impl ImageDecoder) -> anyhow::Result +} +``` + +From yazi-config/src/preview/preview.rs (UNCHANGED — config source): +```rust +pub struct Preview { + pub image_filter: String, // "nearest", "triangle", "catmull-rom", "gaussian", "lanczos3" + pub image_quality: u8, + pub max_width: u16, + pub max_height: u16, +} +``` + + + + + + + Task 1: Add fast_image_resize dependency + yazi-adapter/Cargo.toml + + - yazi-adapter/Cargo.toml + + +Add `fast_image_resize` to the `[dependencies]` section of `yazi-adapter/Cargo.toml`. + +Insert the following line in the external dependencies block (after the `image` line, maintaining alphabetical order among external deps): + +```toml +fast_image_resize = { version = "6.0.0", features = ["image"] } +``` + +The `image` feature enables zero-copy `IntoImageView`/`IntoImageViewMut` bridge for `DynamicImage` and all `ImageBuffer` variants. Do NOT add `rayon` feature (yazi uses `spawn_blocking` for parallelism). Do NOT add `only_u8x4` feature (it would disable Gray U8 and GrayAlpha U8x2 support). + +No workspace-level entry in root `Cargo.toml` is needed — only `yazi-adapter` uses this library. + + + cd /Users/D051699/git/playground/yazi && grep 'fast_image_resize' yazi-adapter/Cargo.toml && cargo check -p yazi-adapter 2>&1 | tail -5 + + + - yazi-adapter/Cargo.toml contains line: `fast_image_resize = { version = "6.0.0", features = ["image"] }` + - `cargo check -p yazi-adapter` exits 0 + + fast_image_resize 6.0.0 is declared as a dependency of yazi-adapter with the image feature enabled + + + + Task 2: Replace resize pipeline with linear-light fast_image_resize + yazi-adapter/src/image.rs + + - yazi-adapter/src/image.rs + - yazi-adapter/src/icc.rs + - .planning/phases/01-implementation/01-RESEARCH.md + + +Rewrite `yazi-adapter/src/image.rs` to replace the `image` crate's `img.resize()` with `fast_image_resize` using explicit sRGB linearization. The file has 128 lines currently. The changes are: + +**1. Update imports (line 4):** + +Remove `imageops::FilterType` from the `image` use statement. Add new imports for `fast_image_resize`: + +```rust +use fast_image_resize::{IntoImageView, ResizeAlg, ResizeOptions, Resizer, create_srgb_mapper}; +use fast_image_resize::images::Image as FirImage; +``` + +The full `image` import line becomes (remove `imageops::FilterType`): +```rust +use image::{DynamicImage, ImageDecoder, ImageError, ImageReader, Limits, codecs::{jpeg::JpegEncoder, png::PngEncoder}, metadata::Orientation}; +``` + +**2. Replace `fn filter()` (lines 86-95) with `fn resize_alg()`:** + +Per D-01 (default lanczos3), D-02 (same 5 filter names), D-03 (warn + lanczos3 fallback for unknown): + +```rust +fn resize_alg() -> ResizeAlg { + use fast_image_resize::FilterType; + match YAZI.preview.image_filter.as_str() { + "nearest" => ResizeAlg::Nearest, + "triangle" => ResizeAlg::Convolution(FilterType::Bilinear), + "catmull-rom" => ResizeAlg::Convolution(FilterType::CatmullRom), + "gaussian" => ResizeAlg::Convolution(FilterType::Gaussian), + "lanczos3" => ResizeAlg::Convolution(FilterType::Lanczos3), + other => { + tracing::warn!("unknown image_filter {other:?}, falling back to lanczos3"); + ResizeAlg::Convolution(FilterType::Lanczos3) + } + } +} +``` + +Note: `"triangle"` maps to `FilterType::Bilinear` (the `image` crate's `Triangle` IS bilinear interpolation; `fast_image_resize` uses the name `Bilinear`). + +**3. Add `fn fir_resize()` private helper:** + +This is the core of the change. It implements: sRGB linearize -> resize in linear light -> sRGB encode back. For `ResizeAlg::Nearest`, skip linearization (nearest doesn't convolve, so the mapper is wasted work). + +Handle `pixel_type()` returning `None` (exotic 16-bit/float variants from `DynamicImage::from_decoder()` fallback in icc.rs) by converting to `to_rgb8()` first with a `tracing::debug!` log. + +```rust +fn fir_resize(img: DynamicImage, w: u32, h: u32, alg: ResizeAlg) -> anyhow::Result { + let img = match img.pixel_type() { + Some(_) => img, + None => { + tracing::debug!("converting exotic pixel type to rgb8 for resize"); + img.to_rgb8().into() + } + }; + + let pixel_type = img + .pixel_type() + .ok_or_else(|| anyhow::anyhow!("unsupported pixel type for fast_image_resize"))?; + + if matches!(alg, ResizeAlg::Nearest) { + let mut dst = FirImage::new(w, h, pixel_type); + let mut resizer = Resizer::new(); + resizer.resize(&img, &mut dst, &ResizeOptions::new().resize_alg(alg))?; + return Self::reconstruct(w, h, pixel_type, dst.into_vec()); + } + + let mapper = create_srgb_mapper(); + + // sRGB -> linear + let mut linear_src = FirImage::new(img.width(), img.height(), pixel_type); + mapper.forward_map(&img, &mut linear_src)?; + + // Resize in linear light (mul_div_alpha=true by default handles alpha premult) + let mut dst = FirImage::new(w, h, pixel_type); + let mut resizer = Resizer::new(); + let opts = ResizeOptions::new().resize_alg(alg); + resizer.resize(&linear_src, &mut dst, &opts)?; + + // linear -> sRGB + let mut srgb_dst = FirImage::new(w, h, pixel_type); + mapper.backward_map(&dst, &mut srgb_dst)?; + + Self::reconstruct(w, h, pixel_type, srgb_dst.into_vec()) +} +``` + +**4. Add `fn reconstruct()` private helper:** + +Reconstructs the correct `DynamicImage` variant from the resized buffer bytes, matching the original pixel type. This preserves COMPAT-02 (buffer format preserved). + +```rust +fn reconstruct( + w: u32, + h: u32, + pixel_type: fast_image_resize::PixelType, + buf: Vec, +) -> anyhow::Result { + use anyhow::Context; + use fast_image_resize::PixelType; + use image::{GrayAlphaImage, GrayImage, RgbImage, RgbaImage}; + match pixel_type { + PixelType::U8 => { + Ok(GrayImage::from_raw(w, h, buf).context("gray reconstruct")?.into()) + } + PixelType::U8x2 => { + Ok(GrayAlphaImage::from_raw(w, h, buf).context("graya reconstruct")?.into()) + } + PixelType::U8x3 => { + Ok(RgbImage::from_raw(w, h, buf).context("rgb reconstruct")?.into()) + } + PixelType::U8x4 => { + Ok(RgbaImage::from_raw(w, h, buf).context("rgba reconstruct")?.into()) + } + _ => Err(anyhow::anyhow!("unexpected pixel type {pixel_type:?}")), + } +} +``` + +**5. Update `precache()` call site (line 21):** + +Replace: +```rust +img = img.resize(w, h, Self::filter()); +``` +With: +```rust +img = Self::fir_resize(img, w, h, Self::resize_alg())?; +``` + +The `?` operator works here because the closure already returns `Ok::<_, ImageError>(buf)` -- but note the error type changes. The closure return type must be updated from `Ok::<_, ImageError>(buf)` to `Ok::<_, anyhow::Error>(buf)` since `fir_resize` returns `anyhow::Result`. The outer `.await??` still works because `anyhow::Error` implements the required traits. + +Actually, looking more carefully: the spawn_blocking closure currently returns `Result, ImageError>`. Since `fir_resize` returns `anyhow::Result`, and the `?` operator on it needs the closure to return a compatible error type, change the closure to return `anyhow::Result>`: +- Change `Ok::<_, ImageError>(buf)` to `Ok::<_, anyhow::Error>(buf)` (or just `Ok(buf)` if the compiler can infer) +- The outer `precache` function already returns `anyhow::Result<()>`, and `.await??` will work with `anyhow::Error` from the inner closure + +**6. Update `downscale()` call site (line 54):** + +Replace: +```rust +img = img.resize(w, h, Self::filter()) +``` +With: +```rust +img = Self::fir_resize(img, w, h, Self::resize_alg())?; +``` + +The current `spawn_blocking` closure in `downscale()` returns `DynamicImage` directly (no Result). Since `fir_resize` returns `anyhow::Result`, update the closure to return `anyhow::Result`: +- Wrap the final `img` return in `Ok(img)` +- Change `.await?` to `.await??` (first `?` unwraps JoinError, second unwraps anyhow::Error) +- The outer function already returns `Result` so this works + +The updated `downscale` spawn_blocking block becomes: +```rust +let img = tokio::task::spawn_blocking(move || { + if img.width() > w || img.height() > h { + img = Self::fir_resize(img, w, h, Self::resize_alg())?; + } + if orientation != Orientation::NoTransforms { + img.apply_orientation(orientation); + } + Ok(img) +}) +.await??; +``` + +**Important constraints:** +- Keep all new code inside the existing `spawn_blocking` closures (COMPAT-03) +- Do NOT change `precache()` public signature `pub async fn precache(src: PathBuf, cache: &Path) -> Result<()>` (COMPAT-04) +- Use hard tabs for indentation per rustfmt.toml +- Follow import grouping: std -> external crates -> workspace crates + + + cd /Users/D051699/git/playground/yazi && cargo build -p yazi-adapter 2>&1 | tail -10 && cargo clippy -p yazi-adapter 2>&1 | tail -10 + + + - yazi-adapter/src/image.rs does NOT contain `imageops::FilterType` + - yazi-adapter/src/image.rs contains `fn resize_alg() -> ResizeAlg` + - yazi-adapter/src/image.rs contains `fn fir_resize(img: DynamicImage, w: u32, h: u32, alg: ResizeAlg)` + - yazi-adapter/src/image.rs contains `fn reconstruct(` + - yazi-adapter/src/image.rs contains `create_srgb_mapper()` + - yazi-adapter/src/image.rs contains `mapper.forward_map(` and `mapper.backward_map(` + - yazi-adapter/src/image.rs contains `matches!(alg, ResizeAlg::Nearest)` (skip linearization for nearest) + - yazi-adapter/src/image.rs contains `tracing::warn!("unknown image_filter` (D-03 fallback) + - yazi-adapter/src/image.rs contains `Self::fir_resize(img, w, h, Self::resize_alg())` in precache() + - yazi-adapter/src/image.rs contains `Self::fir_resize(img, w, h, Self::resize_alg())` in downscale() + - yazi-adapter/src/image.rs filter mapping contains `"triangle" => ResizeAlg::Convolution(FilterType::Bilinear)` + - precache() signature is still `pub async fn precache(src: PathBuf, cache: &Path) -> Result<()>` + - `cargo build -p yazi-adapter` exits 0 + - `cargo clippy -p yazi-adapter` produces no errors + + Both resize call sites use fir_resize with linear-light pipeline; all 5 filter names map correctly; unknown filters warn and fallback to lanczos3; DynamicImage variant is preserved; exotic pixel types handled gracefully + + + + Task 3: Full workspace build and lint verification + yazi-adapter/src/image.rs + + - yazi-adapter/src/image.rs + + +Run full workspace build and clippy to verify no regressions across the entire codebase. + +```bash +cargo build --release 2>&1 +cargo clippy --all 2>&1 +cargo test --workspace 2>&1 +``` + +If clippy produces warnings in the modified code, fix them: +- Common issues: unused imports (the old `FilterType` import), missing `use Self` pattern, format_push_string +- If `IntoImageView` import triggers an "unused import" warning because it is only used as a trait impl (not called directly), add `#[allow(unused_imports)]` or verify it is needed for the `.pixel_type()` method resolution + +If the release build fails, check for: +- Missing `Send` bounds on types used across `spawn_blocking` (all `fast_image_resize` types are `Send`) +- LTO compilation issues (unlikely with pure Rust dependency) + + + cd /Users/D051699/git/playground/yazi && cargo build --release 2>&1 | tail -5 && cargo clippy --all 2>&1 | tail -5 && cargo test --workspace 2>&1 | tail -10 + + + - `cargo build --release` exits 0 + - `cargo clippy --all` exits 0 with no new warnings in yazi-adapter + - `cargo test --workspace` exits 0 + + Full workspace builds and passes clippy without new warnings; all existing tests pass; release build with LTO succeeds + + + + + +After all tasks complete, verify the full implementation: + +1. **Dependency check:** `grep 'fast_image_resize' yazi-adapter/Cargo.toml` shows the dependency +2. **No old resize:** `grep 'img.resize(' yazi-adapter/src/image.rs` returns no matches (both call sites replaced) +3. **No old FilterType:** `grep 'imageops::FilterType' yazi-adapter/src/image.rs` returns no matches +4. **Linear pipeline present:** `grep 'forward_map\|backward_map' yazi-adapter/src/image.rs` shows both mapper calls +5. **Nearest optimization:** `grep 'Nearest' yazi-adapter/src/image.rs` shows the skip-linearization branch +6. **Filter mapping complete:** `grep -c 'ResizeAlg' yazi-adapter/src/image.rs` shows multiple usages +7. **Signature preserved:** `grep 'pub async fn precache' yazi-adapter/src/image.rs` shows unchanged signature +8. **Build clean:** `cargo build --release && cargo clippy --all && cargo test --workspace` all pass + + + +1. `yazi-adapter/src/image.rs` uses `fast_image_resize` with `create_srgb_mapper().forward_map()` before resize and `backward_map()` after (RESIZE-01, RESIZE-02) +2. Both `precache()` and `downscale()` call `fir_resize` instead of `img.resize()` (RESIZE-03) +3. All 5 filter strings correctly mapped to ResizeAlg variants (RESIZE-04) +4. Default path uses `Lanczos3` convolution in linear light (RESIZE-05) +5. No driver files modified (COMPAT-01) +6. `reconstruct()` preserves DynamicImage variant matching input pixel type (COMPAT-02) +7. All resize code inside `spawn_blocking` closures (COMPAT-03) +8. `precache()` public signature unchanged (COMPAT-04) +9. `cargo build --release` and `cargo clippy --all` pass clean + + + +After completion, create `.planning/phases/01-implementation/01-01-SUMMARY.md` +