plan(01): create phase 1 implementation plan

2026-07-25 08:41:05 +00:00 · 2026-03-21 12:46:48 +01:00 · 2026-03-21 12:46:48 +01:00 · 6a199a81c6
commit 6a199a81c6
parent 96f443baa0
2 changed files with 476 additions and 0 deletions
--- a/.planning/ROADMAP.md
+++ 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 | - |
--- a/.planning/phases/01-implementation/01-01-PLAN.md
+++ 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"
+---
+
+<objective>
+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).
+</objective>
+
+<execution_context>
+@~/.claude/get-shit-done/workflows/execute-plan.md
+@~/.claude/get-shit-done/templates/summary.md
+</execution_context>
+
+<context>
+@.planning/PROJECT.md
+@.planning/ROADMAP.md
+@.planning/STATE.md
+@.planning/phases/01-implementation/01-CONTEXT.md
+@.planning/phases/01-implementation/01-RESEARCH.md
+
+<interfaces>
+<!-- Key types and contracts the executor needs. Extracted from codebase. -->
+
+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<DynamicImage>
+
+    // 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<DynamicImage>
+}
+```
+
+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,
+}
+```
+</interfaces>
+</context>
+
+<tasks>
+
+<task type="auto">
+  <name>Task 1: Add fast_image_resize dependency</name>
+  <files>yazi-adapter/Cargo.toml</files>
+  <read_first>
+    - yazi-adapter/Cargo.toml
+  </read_first>
+  <action>
+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.
+  </action>
+  <verify>
+    <automated>cd /Users/D051699/git/playground/yazi && grep 'fast_image_resize' yazi-adapter/Cargo.toml && cargo check -p yazi-adapter 2>&1 | tail -5</automated>
+  </verify>
+  <acceptance_criteria>
+    - yazi-adapter/Cargo.toml contains line: `fast_image_resize = { version = "6.0.0", features = ["image"] }`
+    - `cargo check -p yazi-adapter` exits 0
+  </acceptance_criteria>
+  <done>fast_image_resize 6.0.0 is declared as a dependency of yazi-adapter with the image feature enabled</done>
+</task>
+
+<task type="auto">
+  <name>Task 2: Replace resize pipeline with linear-light fast_image_resize</name>
+  <files>yazi-adapter/src/image.rs</files>
+  <read_first>
+    - yazi-adapter/src/image.rs
+    - yazi-adapter/src/icc.rs
+    - .planning/phases/01-implementation/01-RESEARCH.md
+  </read_first>
+  <action>
+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<DynamicImage> {
+    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<u8>,
+) -> anyhow::Result<DynamicImage> {
+    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<Vec<u8>, ImageError>`. Since `fir_resize` returns `anyhow::Result<DynamicImage>`, and the `?` operator on it needs the closure to return a compatible error type, change the closure to return `anyhow::Result<Vec<u8>>`:
+- 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<DynamicImage>`, update the closure to return `anyhow::Result<DynamicImage>`:
+- 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<DynamicImage>` 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
+  </action>
+  <verify>
+    <automated>cd /Users/D051699/git/playground/yazi && cargo build -p yazi-adapter 2>&1 | tail -10 && cargo clippy -p yazi-adapter 2>&1 | tail -10</automated>
+  </verify>
+  <acceptance_criteria>
+    - 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
+  </acceptance_criteria>
+  <done>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</done>
+</task>
+
+<task type="auto">
+  <name>Task 3: Full workspace build and lint verification</name>
+  <files>yazi-adapter/src/image.rs</files>
+  <read_first>
+    - yazi-adapter/src/image.rs
+  </read_first>
+  <action>
+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)
+  </action>
+  <verify>
+    <automated>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</automated>
+  </verify>
+  <acceptance_criteria>
+    - `cargo build --release` exits 0
+    - `cargo clippy --all` exits 0 with no new warnings in yazi-adapter
+    - `cargo test --workspace` exits 0
+  </acceptance_criteria>
+  <done>Full workspace builds and passes clippy without new warnings; all existing tests pass; release build with LTO succeeds</done>
+</task>
+
+</tasks>
+
+<verification>
+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
+</verification>
+
+<success_criteria>
+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
+</success_criteria>
+
+<output>
+After completion, create `.planning/phases/01-implementation/01-01-SUMMARY.md`
+</output>