diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 5b9b45d..9c0f22a 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -61,6 +61,13 @@ The scaffold is functional with image display, async OCR pipeline, drag-select o `config.rs`): lang (precedence `--lang` > `QUICKVIEW_LANG` > config > `eng`) and `tessdata_dir` (`--tessdata-dir` > config); both live in `OcrOptions` and join the cache key +- OCR max-dimension guardrail (`max_dimension`/`--max-ocr-dim`, default 4000): + oversized images OCR a downscaled temp PNG made from the decoded texture + (`ocr_prep.rs`; main-thread download, worker-thread scale+encode), bboxes + map back to original space (`ocr/downscale.rs`), effective target joins the + cache key +- `quickview::perf` debug timing events (decode, cache hit, downscale prep, + OCR) — `RUST_LOG=quickview::perf=debug` - On-disk OCR cache (`~/.cache/quickview/ocr/`, keyed by path+lang+mtime+size; no eviction in v1 — see ADR-0009 implementation notes) - Drag-select overlay with word highlighting @@ -69,7 +76,7 @@ The scaffold is functional with image display, async OCR pipeline, drag-select o - Zoom & pan (Ctrl+scroll, pinch, +/- keys, middle-drag pan) via custom `ZoomableCanvas` widget ### What's not implemented yet: -- Performance benchmarks +- Memory usage limits (deferred to Phase 8) ## Development diff --git a/Cargo.lock b/Cargo.lock index a5775bc..5b3874a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1673,6 +1673,7 @@ dependencies = [ "gtk4-layer-shell", "libadwaita", "quickview-core", + "tempfile", "tracing", ] diff --git a/adrs/ADR-0009-Caching.md b/adrs/ADR-0009-Caching.md index 91f8143..d293c1d 100644 --- a/adrs/ADR-0009-Caching.md +++ b/adrs/ADR-0009-Caching.md @@ -53,10 +53,14 @@ The implementation went **straight to on-disk**, revising the decision above: derived from the lowercased app name, so the app-ID rename (done: io.github.Green2Grey2.QuickView) did not move it on Linux. - Tesseract is invoked with no psm/oem flags, so the OCR settings in the key - are `lang` and (since Phase 7's config work) the optional `tessdata_dir`, - hashed with a presence marker so `None` and empty stay distinct. **The rule - stands: any newly configurable OCR setting (psm/oem, the downscale target) - must join the key.** + are `lang`, the optional `tessdata_dir` (presence marker keeps `None` and + empty distinct), and the **planned** downscale target (`full` or + `WxH`) — the target rather than the configured `max_dimension` threshold, + so below-threshold images keep their entries across threshold edits, and + the entry path is derivable before any pixels are prepared (a cache probe + skips the texture download on reopens of oversized images). **The + rule stands: any newly configurable OCR setting (psm/oem) must join the + key.** - Writes are atomic (temp file + rename in the same directory): concurrent QuickView processes are a designed use case. - Entries are created `0600` in `0700` directories — they hold recognized diff --git a/crates/quickview-core/src/cache.rs b/crates/quickview-core/src/cache.rs index 76c7925..c053f65 100644 --- a/crates/quickview-core/src/cache.rs +++ b/crates/quickview-core/src/cache.rs @@ -1,10 +1,10 @@ //! On-disk OCR result cache. //! //! Entries are JSON files under `/ocr/`, keyed by a blake3 hash of -//! the image path, the OCR settings (language, tessdata dir), and file -//! mtime+size — so an edited file is simply a cache miss (no invalidation -//! logic needed). There is no eviction in -//! v1: entries are a few KB each, and users can clear the directory manually. +//! the image path, the OCR settings (language, tessdata dir), the effective +//! downscale target, and file mtime+size — so an edited file is simply a +//! cache miss (no invalidation logic needed). There is no eviction in v1: +//! entries are a few KB each, and users can clear the directory manually. //! Phase 8's persistent SQLite cache is the planned successor (ADR-0009). use std::path::{Path, PathBuf}; @@ -20,18 +20,60 @@ pub fn cache_dir() -> Option { Some(crate::config::project_dirs()?.cache_dir().to_path_buf()) } -pub fn ocr_cache_path(cache_root: &Path, file: &Path, opts: &OcrOptions) -> PathBuf { - // Include file metadata to avoid stale caches. Full nanosecond mtime: - // whole seconds would alias a same-second rewrite of the same path with - // an unchanged byte length (rapid screenshot/editor saves). - let meta = std::fs::metadata(file).ok(); - let mtime = meta - .as_ref() - .and_then(|m| m.modified().ok()) - .and_then(|t| t.duration_since(std::time::UNIX_EPOCH).ok()) - .map(|d| d.as_nanos()) - .unwrap_or(0); - let size = meta.as_ref().map(|m| m.len()).unwrap_or(0); +/// Snapshot of the file-identity fields that join the OCR cache key. +/// +/// Read the stamp **before** the image content is read (i.e. before decode +/// starts): if the file is replaced afterwards, the derived key belongs to +/// the old content and the edited file simply misses it. Stamping *after* +/// the content was read would let old pixels be stored under the live +/// file's key, which future opens would then wrongly hit. +/// +/// Full nanosecond mtime: whole seconds would alias a same-second rewrite +/// of the same path with an unchanged byte length (rapid screenshot/editor +/// saves). Unreadable metadata stamps as zeros. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct FileStamp { + mtime_nanos: u128, + size: u64, +} + +impl FileStamp { + pub fn read(file: &Path) -> Self { + let meta = std::fs::metadata(file).ok(); + let mtime_nanos = meta + .as_ref() + .and_then(|m| m.modified().ok()) + .and_then(|t| t.duration_since(std::time::UNIX_EPOCH).ok()) + .map(|d| d.as_nanos()) + .unwrap_or(0); + let size = meta.as_ref().map(|m| m.len()).unwrap_or(0); + Self { mtime_nanos, size } + } +} + +/// Compute the cache entry path for one OCR run. +/// +/// `stamp` must have been read before the image content (see [`FileStamp`]). +/// +/// `downscale_target` is the *planned* image size for tesseract: `None` for +/// a full-resolution run, `Some((w, h))` when the max-dimension guardrail +/// intends to feed it a downscaled copy. Hashing the target rather than the +/// configured threshold keeps below-threshold images' entries valid across +/// `max_dimension` edits — per ADR-0009, only inputs that change the +/// recognition output join the key. (A degraded run that falls back to full +/// resolution still stores under the planned key: strictly better content, +/// and it lets future opens hit the cache without re-preparing pixels.) +pub fn ocr_cache_path( + cache_root: &Path, + file: &Path, + opts: &OcrOptions, + downscale_target: Option<(u32, u32)>, + stamp: FileStamp, +) -> PathBuf { + let FileStamp { + mtime_nanos: mtime, + size, + } = stamp; let mut hasher = blake3::Hasher::new(); hasher.update(file.as_os_str().as_encoded_bytes()); @@ -51,6 +93,11 @@ pub fn ocr_cache_path(cache_root: &Path, file: &Path, opts: &OcrOptions) -> Path } } hasher.update(b"\0"); + match downscale_target { + Some((w, h)) => hasher.update(format!("{w}x{h}").as_bytes()), + None => hasher.update(b"full"), + }; + hasher.update(b"\0"); hasher.update(&mtime.to_le_bytes()); hasher.update(&size.to_le_bytes()); let key = hasher.finalize().to_hex().to_string(); @@ -157,41 +204,56 @@ mod tests { let img = dir.path().join("a.png"); std::fs::write(&img, b"xx").unwrap(); - let base = ocr_cache_path(root, &img, &eng()); + let base = ocr_cache_path(root, &img, &eng(), None, FileStamp::read(&img)); // Same inputs -> same key. - assert_eq!(base, ocr_cache_path(root, &img, &eng())); + assert_eq!( + base, + ocr_cache_path(root, &img, &eng(), None, FileStamp::read(&img)) + ); // Different language -> different key. let deu = OcrOptions { lang: "deu".into(), ..eng() }; - assert_ne!(base, ocr_cache_path(root, &img, &deu)); + assert_ne!( + base, + ocr_cache_path(root, &img, &deu, None, FileStamp::read(&img)) + ); // Different path -> different key. let img2 = dir.path().join("b.png"); std::fs::write(&img2, b"xx").unwrap(); - assert_ne!(base, ocr_cache_path(root, &img2, &eng())); + assert_ne!( + base, + ocr_cache_path(root, &img2, &eng(), None, FileStamp::read(&img2)) + ); // Different size -> different key. std::fs::write(&img, b"xxxx").unwrap(); - assert_ne!(base, ocr_cache_path(root, &img, &eng())); + assert_ne!( + base, + ocr_cache_path(root, &img, &eng(), None, FileStamp::read(&img)) + ); // Different mtime (same size) -> different key. std::fs::write(&img, b"xx").unwrap(); - let before = ocr_cache_path(root, &img, &eng()); + let before = ocr_cache_path(root, &img, &eng(), None, FileStamp::read(&img)); let old = std::time::SystemTime::UNIX_EPOCH + std::time::Duration::from_secs(1_000_000); std::fs::File::open(&img) .unwrap() .set_modified(old) .unwrap(); - assert_ne!(before, ocr_cache_path(root, &img, &eng())); + assert_ne!( + before, + ocr_cache_path(root, &img, &eng(), None, FileStamp::read(&img)) + ); // Subsecond mtime change (same second, same size) -> different key. let with_key = |t| { std::fs::File::open(&img).unwrap().set_modified(t).unwrap(); - ocr_cache_path(root, &img, &eng()) + ocr_cache_path(root, &img, &eng(), None, FileStamp::read(&img)) }; assert_ne!( with_key(old + std::time::Duration::from_nanos(1)), @@ -199,6 +261,31 @@ mod tests { ); } + #[test] + fn key_changes_with_downscale_target_only_when_downscaled() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let img = dir.path().join("a.png"); + std::fs::write(&img, b"xx").unwrap(); + + let with_target = |t| ocr_cache_path(root, &img, &eng(), t, FileStamp::read(&img)); + + // A downscaled run is keyed apart from full resolution, and targets + // are keyed apart from each other. + assert_ne!(with_target(None), with_target(Some((4000, 2000)))); + assert_ne!( + with_target(Some((4000, 2000))), + with_target(Some((2000, 1000))) + ); + // Same effective target -> same key (threshold edits don't invalidate + // below-threshold images, which always hash as full resolution). + assert_eq!(with_target(None), with_target(None)); + assert_eq!( + with_target(Some((4000, 2000))), + with_target(Some((4000, 2000))) + ); + } + #[test] fn key_changes_with_tessdata_dir() { let dir = tempfile::tempdir().unwrap(); @@ -211,7 +298,7 @@ mod tests { lang: "eng".into(), tessdata_dir: d.map(PathBuf::from), }; - ocr_cache_path(root, &img, &opts) + ocr_cache_path(root, &img, &opts, None, FileStamp::read(&img)) }; // Some(dir) differs from None, and dirs differ from each other. @@ -234,7 +321,7 @@ mod tests { let dir = tempfile::tempdir().unwrap(); let img = dir.path().join("a.png"); std::fs::write(&img, b"xx").unwrap(); - let entry = ocr_cache_path(dir.path(), &img, &eng()); + let entry = ocr_cache_path(dir.path(), &img, &eng(), None, FileStamp::read(&img)); let result = sample_result(); store_ocr(&entry, &result).unwrap(); @@ -267,7 +354,7 @@ mod tests { let img = dir.path().join("a.png"); std::fs::write(&img, b"xx").unwrap(); - let entry = ocr_cache_path(dir.path(), &img, &eng()); + let entry = ocr_cache_path(dir.path(), &img, &eng(), None, FileStamp::read(&img)); assert!(load_ocr(&entry).is_none()); } @@ -277,7 +364,7 @@ mod tests { let img = dir.path().join("a.png"); std::fs::write(&img, b"xx").unwrap(); - let entry = ocr_cache_path(dir.path(), &img, &eng()); + let entry = ocr_cache_path(dir.path(), &img, &eng(), None, FileStamp::read(&img)); std::fs::create_dir_all(entry.parent().unwrap()).unwrap(); std::fs::write(&entry, b"{not json").unwrap(); diff --git a/crates/quickview-core/src/config.rs b/crates/quickview-core/src/config.rs index 9314c6c..08f9dbf 100644 --- a/crates/quickview-core/src/config.rs +++ b/crates/quickview-core/src/config.rs @@ -43,8 +43,8 @@ pub struct OcrSection { /// Directory with `.traineddata` files, e.g. a `tessdata_fast` or /// `tessdata_best` checkout (overridden by `--tessdata-dir`). pub tessdata_dir: Option, - /// Maximum image dimension before OCR downscales (not consumed yet; - /// accepted so configs written for the guardrail don't error). + /// Maximum image dimension before OCR runs on a downscaled copy + /// (overridden by `--max-ocr-dim`; `0` disables the guardrail). pub max_dimension: Option, } diff --git a/crates/quickview-core/src/ocr/downscale.rs b/crates/quickview-core/src/ocr/downscale.rs new file mode 100644 index 0000000..63ca17c --- /dev/null +++ b/crates/quickview-core/src/ocr/downscale.rs @@ -0,0 +1,146 @@ +//! Downscale planning for the OCR max-dimension guardrail. +//! +//! Tesseract re-decodes the image file itself, so oversized images make OCR +//! slow and memory-hungry out of proportion to recognition quality. When an +//! image exceeds the configured maximum dimension, the UI feeds tesseract a +//! downscaled temporary copy instead and maps the resulting word boxes back +//! into original image space with [`upscale_bboxes`] — so everything +//! downstream (cache entries, the spatial index, selection) only ever sees +//! original-space coordinates. +//! +//! This module is pure math; producing the actual downscaled pixels is the +//! UI layer's job (it owns the decoded texture). + +use crate::ocr::models::OcrResult; + +/// Default for the `[ocr] max_dimension` config setting. `0` disables the +/// guardrail entirely. +pub const DEFAULT_MAX_OCR_DIMENSION: u32 = 4000; + +/// A decision to OCR a downscaled copy instead of the original. +#[derive(Debug, Clone, Copy, PartialEq)] +pub struct DownscalePlan { + pub target_w: u32, + pub target_h: u32, + /// `original / target` (> 1.0). Multiply target-space lengths by this to + /// get back to original image space. + pub factor: f64, +} + +/// Decide whether an image of `width` x `height` needs downscaling to fit +/// `max_dimension`. Returns `None` when it already fits (or the guardrail is +/// disabled with `max_dimension == 0`); aspect ratio is preserved and no +/// target dimension falls below 1. +pub fn plan_downscale(width: u32, height: u32, max_dimension: u32) -> Option { + if max_dimension == 0 || width == 0 || height == 0 { + return None; + } + let largest = width.max(height); + if largest <= max_dimension { + return None; + } + + let factor = f64::from(largest) / f64::from(max_dimension); + let scale = |dim: u32| ((f64::from(dim) / factor).round() as u32).max(1); + Some(DownscalePlan { + target_w: scale(width), + target_h: scale(height), + factor, + }) +} + +/// Map word bounding boxes recognized in downscaled space back to original +/// image space. Run this *before* the result is cached or indexed, so a +/// cache hit is indistinguishable from a full-resolution parse. +/// +/// Factors are per-axis (`original / actual target`, computed from the +/// dimensions of the image actually fed to tesseract): target sizes are +/// rounded independently per axis, so one uniform factor would drift boxes +/// on the minor axis. +pub fn upscale_bboxes(result: &mut OcrResult, factor_x: f64, factor_y: f64) { + for word in &mut result.words { + word.bbox.x *= factor_x; + word.bbox.y *= factor_y; + word.bbox.w *= factor_x; + word.bbox.h *= factor_y; + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::geometry::Rect; + use crate::ocr::models::OcrWord; + + #[test] + fn below_and_at_threshold_are_untouched() { + assert_eq!(plan_downscale(1000, 500, 4000), None); + assert_eq!(plan_downscale(4000, 3000, 4000), None); + assert_eq!(plan_downscale(3000, 4000, 4000), None); + } + + #[test] + fn zero_disables_and_degenerate_dims_are_ignored() { + assert_eq!(plan_downscale(100_000, 50, 0), None); + assert_eq!(plan_downscale(0, 5000, 4000), None); + assert_eq!(plan_downscale(5000, 0, 4000), None); + } + + #[test] + fn above_threshold_scales_and_preserves_aspect() { + let plan = plan_downscale(8000, 4000, 4000).unwrap(); + assert_eq!((plan.target_w, plan.target_h), (4000, 2000)); + assert_eq!(plan.factor, 2.0); + + // Portrait: the larger dimension drives the factor. + let plan = plan_downscale(3000, 6000, 4000).unwrap(); + assert_eq!((plan.target_w, plan.target_h), (2000, 4000)); + assert_eq!(plan.factor, 1.5); + } + + #[test] + fn extreme_aspect_never_reaches_zero() { + let plan = plan_downscale(100_000, 50, 4000).unwrap(); + assert_eq!(plan.target_w, 4000); + assert!(plan.target_h >= 1); + assert_eq!(plan.target_h, 2); // 50 / 25 = 2 + + let plan = plan_downscale(1_000_000, 1, 4000).unwrap(); + assert_eq!(plan.target_h, 1); + } + + #[test] + fn upscale_round_trips_within_epsilon() { + let factor = 8000.0 / 4000.0; + let original = Rect { + x: 123.0, + y: 456.0, + w: 78.0, + h: 90.0, + }; + // What tesseract would report in downscaled space. + let mut result = OcrResult { + words: vec![OcrWord { + text: "hi".into(), + confidence: 90.0, + bbox: Rect { + x: original.x / factor, + y: original.y / factor, + w: original.w / factor, + h: original.h / factor, + }, + order: 0, + }], + }; + upscale_bboxes(&mut result, factor, factor); + let got = &result.words[0].bbox; + for (a, b) in [ + (got.x, original.x), + (got.y, original.y), + (got.w, original.w), + (got.h, original.h), + ] { + assert!((a - b).abs() < 1e-9, "{a} vs {b}"); + } + } +} diff --git a/crates/quickview-core/src/ocr/mod.rs b/crates/quickview-core/src/ocr/mod.rs index 2552ffd..85c1608 100644 --- a/crates/quickview-core/src/ocr/mod.rs +++ b/crates/quickview-core/src/ocr/mod.rs @@ -1,5 +1,6 @@ //! OCR-related types and helpers. +pub mod downscale; pub mod index; pub mod models; pub mod select; diff --git a/crates/quickview-ui/Cargo.toml b/crates/quickview-ui/Cargo.toml index 091c78b..d104180 100644 --- a/crates/quickview-ui/Cargo.toml +++ b/crates/quickview-ui/Cargo.toml @@ -22,3 +22,4 @@ async-channel = "2" # runtime by probing installed loaders. Links libseccomp, lcms2, and # fontconfig (see docs/DEPENDENCIES.md). glycin = { version = "3", features = ["gdk4"] } +tempfile = "3" diff --git a/crates/quickview-ui/src/ipc.rs b/crates/quickview-ui/src/ipc.rs index 803d984..e908fb1 100644 --- a/crates/quickview-ui/src/ipc.rs +++ b/crates/quickview-ui/src/ipc.rs @@ -7,7 +7,7 @@ //! //! ```text //! quickview --mode= --lang= \ -//! [--tessdata-dir=] --file= +//! [--tessdata-dir=] --max-ocr-dim= --file= //! ``` //! //! Every value is glued to its key in a single `--key=value` token: GLib's @@ -46,6 +46,7 @@ pub(crate) fn to_argv(opts: &LaunchOptions) -> Vec { if let Some(dir) = &opts.ocr.tessdata_dir { argv.push(format!("--tessdata-dir={}", dir.to_string_lossy())); } + argv.push(format!("--max-ocr-dim={}", opts.max_ocr_dimension)); argv.push(format!("--file={}", opts.file.to_string_lossy())); argv } @@ -54,6 +55,7 @@ pub(crate) fn from_argv(argv: &[OsString]) -> Result { let mut mode = None; let mut lang = None; let mut tessdata_dir: Option = None; + let mut max_ocr_dimension = None; let mut file: Option = None; for arg in argv.iter().skip(1) { @@ -71,6 +73,12 @@ pub(crate) fn from_argv(argv: &[OsString]) -> Result { lang = Some(value.to_owned()); } else if let Some(value) = arg.strip_prefix("--tessdata-dir=") { tessdata_dir = Some(PathBuf::from(value)); + } else if let Some(value) = arg.strip_prefix("--max-ocr-dim=") { + max_ocr_dimension = Some( + value + .parse::() + .map_err(|err| anyhow!("bad --max-ocr-dim {value:?}: {err}"))?, + ); } else if let Some(value) = arg.strip_prefix("--file=") { file = Some(PathBuf::from(value)); } else { @@ -84,6 +92,7 @@ pub(crate) fn from_argv(argv: &[OsString]) -> Result { lang: lang.ok_or_else(|| anyhow!("missing --lang"))?, tessdata_dir, }, + max_ocr_dimension: max_ocr_dimension.ok_or_else(|| anyhow!("missing --max-ocr-dim"))?, file: file.ok_or_else(|| anyhow!("missing --file"))?, }) } @@ -100,6 +109,7 @@ mod tests { lang: lang.to_owned(), tessdata_dir: None, }, + max_ocr_dimension: 4000, } } @@ -138,6 +148,11 @@ mod tests { original.ocr.tessdata_dir = Some(PathBuf::from("/opt/tess data/fast")); assert_eq!(round_trip(&original), original); + // Guardrail disabled round-trips too. + let mut disabled = opts(Mode::QuickPreview, "eng", "/tmp/a.png"); + disabled.max_ocr_dimension = 0; + assert_eq!(round_trip(&disabled), disabled); + // Absent stays absent (no --tessdata-dir token emitted at all). let without = opts(Mode::QuickPreview, "eng", "/tmp/a.png"); assert!(!to_argv(&without) @@ -168,6 +183,14 @@ mod tests { #[test] fn rejects_garbage() { assert!(from_argv(&os_argv(&["quickview", "--bogus=x"])).is_err()); + assert!(from_argv(&os_argv(&[ + "quickview", + "--mode=full-viewer", + "--lang=eng", + "--max-ocr-dim=lots", + "--file=/a" + ])) + .is_err()); assert!(from_argv(&os_argv(&[ "quickview", "--mode=sideways", diff --git a/crates/quickview-ui/src/lib.rs b/crates/quickview-ui/src/lib.rs index 9d08d69..66b6781 100644 --- a/crates/quickview-ui/src/lib.rs +++ b/crates/quickview-ui/src/lib.rs @@ -11,6 +11,7 @@ use gtk::gio; mod decode; mod ipc; +mod ocr_prep; pub mod widgets; pub mod windows; @@ -30,6 +31,8 @@ pub struct LaunchOptions { /// Fully resolved OCR settings: CLI/env/config precedence is applied in /// the invoking process before these cross the instance boundary. pub ocr: OcrOptions, + /// Max image dimension before OCR downscales (0 disables the guardrail). + pub max_ocr_dimension: u32, } /// Windows the primary instance manages across invocations. @@ -104,6 +107,7 @@ fn dispatch(app: &adw::Application, state: &Rc, opts: &LaunchOptions) // Explicit request for a different file: show it, with // the OCR settings this invocation asked for. controller.set_ocr_options(opts.ocr.clone()); + controller.set_max_ocr_dimension(opts.max_ocr_dimension); controller.load_file(&opts.file); window.present(); } diff --git a/crates/quickview-ui/src/ocr_prep.rs b/crates/quickview-ui/src/ocr_prep.rs new file mode 100644 index 0000000..9bb6b7c --- /dev/null +++ b/crates/quickview-ui/src/ocr_prep.rs @@ -0,0 +1,151 @@ +//! Producing downscaled pixels for the OCR max-dimension guardrail. +//! +//! Tesseract reads a file path, so downscaling means materializing a smaller +//! temporary image. The pixels come from the already-decoded `gdk::Texture` +//! — never from re-decoding the untrusted file in-process, which would +//! bypass the glycin sandbox (NFR-002) and miss formats gdk-pixbuf has no +//! loader for. +//! +//! Everything here runs on the OCR worker thread, only after the cache +//! misses: GdkTexture is immutable and threadsafe, and both decode backends +//! (glycin, GDK fallback) produce memory textures whose download is a plain +//! copy with no GL involvement. + +use std::path::PathBuf; + +use anyhow::{Context, Result}; +use gtk4 as gtk; + +use gtk::prelude::*; +use gtk::{gdk, gdk_pixbuf}; + +use quickview_core::ocr::downscale::DownscalePlan; + +/// RGBA pixels downloaded from a texture, ready to cross to a worker thread +/// (`glib::Bytes` is `Send + Sync`; no GObject travels with it). +pub struct RgbaPixels { + bytes: glib::Bytes, + width: i32, + height: i32, + stride: i32, +} + +/// Download `texture` as unpremultiplied RGBA. +pub fn download_rgba(texture: &gdk::Texture) -> Result { + let width = texture.width(); + let height = texture.height(); + let stride = usize::try_from(width) + .ok() + .and_then(|w| w.checked_mul(4)) + .context("image width overflows RGBA stride")?; + let len = usize::try_from(height) + .ok() + .and_then(|h| h.checked_mul(stride)) + .context("image size overflows RGBA buffer")?; + + // Checked allocation first: `download_bytes()` would g_malloc the same + // buffer and abort the whole process on OOM. A failed allocation must + // instead surface as an Err so the caller degrades to full-resolution + // OCR. + let mut buf: Vec = Vec::new(); + buf.try_reserve_exact(len) + .with_context(|| format!("cannot allocate {len} bytes for the RGBA copy"))?; + buf.resize(len, 0); + + let mut downloader = gdk::TextureDownloader::new(texture); + downloader.set_format(gdk::MemoryFormat::R8g8b8a8); + // SAFETY: gdk4-rs 0.10 does not bind download_into (only the aborting + // download_bytes). The buffer is exactly `stride * height` bytes with + // `stride == width * 4`, which is what an R8g8b8a8 download writes, and + // the downloader stays alive for the duration of the call. + unsafe { + let ptr: *const gdk::ffi::GdkTextureDownloader = glib::translate::ToGlibPtr::< + *const gdk::ffi::GdkTextureDownloader, + >::to_glib_none(&downloader) + .0; + gdk::ffi::gdk_texture_downloader_download_into(ptr, buf.as_mut_ptr(), stride); + } + + Ok(RgbaPixels { + bytes: glib::Bytes::from_owned(buf), + width, + height, + stride: i32::try_from(stride).context("texture stride exceeds i32")?, + }) +} + +/// The downscaled temp image tesseract will read. +/// +/// The temp file is deleted when this guard drops (including on every error +/// path), so keep it alive across the tesseract run. +pub struct DownscaledImage { + file: tempfile::NamedTempFile, + /// Per-axis `original / actual` factors for mapping OCR bboxes back to + /// original image space, computed from the scaled image's real + /// dimensions (per-axis rounding makes one uniform factor drift). + pub factor_x: f64, + pub factor_y: f64, + /// Actual dimensions fed to tesseract (equals the plan's target). + pub target: (u32, u32), +} + +impl DownscaledImage { + pub fn path(&self) -> &std::path::Path { + self.file.path() + } +} + +/// Scale `pixels` to `plan`'s target and write a private temp PNG. +/// Worker-thread safe: the pixbuf is created and dropped here. +pub fn write_downscaled_png(pixels: &RgbaPixels, plan: &DownscalePlan) -> Result { + let src = gdk_pixbuf::Pixbuf::from_bytes( + &pixels.bytes, + gdk_pixbuf::Colorspace::Rgb, + true, // has_alpha (R8g8b8a8, unpremultiplied) + 8, + pixels.width, + pixels.height, + pixels.stride, + ); + let target_w = i32::try_from(plan.target_w).context("target width exceeds i32")?; + let target_h = i32::try_from(plan.target_h).context("target height exceeds i32")?; + let scaled = src + .scale_simple(target_w, target_h, gdk_pixbuf::InterpType::Bilinear) + .context("pixbuf scaling failed (out of memory?)")?; + drop(src); + + // NamedTempFile is created 0600, matching the OCR cache's posture: the + // downscaled copy holds the same sensitive content as the screenshot. + let file = tempfile::Builder::new() + .prefix("quickview-ocr-") + .suffix(".png") + .tempfile_in(temp_dir()) + .context("failed to create OCR temp file")?; + scaled + .savev(file.path(), "png", &[]) + .context("failed to encode downscaled PNG")?; + + Ok(DownscaledImage { + file, + factor_x: f64::from(pixels.width) / f64::from(scaled.width()), + factor_y: f64::from(pixels.height) / f64::from(scaled.height()), + target: (scaled.width() as u32, scaled.height() as u32), + }) +} + +/// Where the temp image lives: `$XDG_RUNTIME_DIR` first (0700 tmpfs — same +/// privacy expectations as the OCR cache), then the cache dir, then the +/// system temp dir. +fn temp_dir() -> PathBuf { + if let Some(dir) = std::env::var_os("XDG_RUNTIME_DIR").map(PathBuf::from) { + if dir.is_dir() { + return dir; + } + } + if let Some(dir) = quickview_core::cache::cache_dir() { + if std::fs::create_dir_all(&dir).is_ok() { + return dir; + } + } + std::env::temp_dir() +} diff --git a/crates/quickview-ui/src/windows/full_viewer.rs b/crates/quickview-ui/src/windows/full_viewer.rs index 853f5ad..6101c08 100644 --- a/crates/quickview-ui/src/windows/full_viewer.rs +++ b/crates/quickview-ui/src/windows/full_viewer.rs @@ -20,7 +20,7 @@ pub fn present(app: &adw::Application, opts: &LaunchOptions) { let header = adw::HeaderBar::new(); header.set_title_widget(Some(&title)); - let viewer = ViewerController::new(opts.file.clone(), opts.ocr.clone()); + let viewer = ViewerController::new(opts.file.clone(), opts.ocr.clone(), opts.max_ocr_dimension); { let title = title.clone(); diff --git a/crates/quickview-ui/src/windows/quick_preview.rs b/crates/quickview-ui/src/windows/quick_preview.rs index 6a87ed1..76aa02a 100644 --- a/crates/quickview-ui/src/windows/quick_preview.rs +++ b/crates/quickview-ui/src/windows/quick_preview.rs @@ -27,7 +27,7 @@ pub fn present( .default_height(PANEL_HEIGHT) .build(); - let viewer = ViewerController::new(opts.file.clone(), opts.ocr.clone()); + let viewer = ViewerController::new(opts.file.clone(), opts.ocr.clone(), opts.max_ocr_dimension); // Layer shell if supported. if gtk4_layer_shell::is_supported() { diff --git a/crates/quickview-ui/src/windows/shared.rs b/crates/quickview-ui/src/windows/shared.rs index 3ce1f37..261d07f 100644 --- a/crates/quickview-ui/src/windows/shared.rs +++ b/crates/quickview-ui/src/windows/shared.rs @@ -9,7 +9,7 @@ use gtk4 as gtk; use quickview_core::{ cache, fs, - ocr::{tesseract, tesseract::OcrOptions, tsv}, + ocr::{downscale, tesseract, tesseract::OcrOptions, tsv}, }; use crate::widgets::image_overlay::ImageOverlayWidget; @@ -40,6 +40,7 @@ pub struct ViewerController { dir_index: Rc>, ocr: Rc>, + max_ocr_dimension: Rc>, // Monotonic ids to ignore late results from superseded jobs. decode_job_id: Rc>, @@ -50,7 +51,7 @@ pub struct ViewerController { } impl ViewerController { - pub fn new(initial_file: PathBuf, ocr: OcrOptions) -> Self { + pub fn new(initial_file: PathBuf, ocr: OcrOptions, max_ocr_dimension: u32) -> Self { let overlay = ImageOverlayWidget::new(); let current_file = Rc::new(RefCell::new(initial_file.clone())); @@ -62,6 +63,7 @@ impl ViewerController { dir_images: Rc::new(RefCell::new(dir_images)), dir_index: Rc::new(Cell::new(dir_index)), ocr: Rc::new(RefCell::new(ocr)), + max_ocr_dimension: Rc::new(Cell::new(max_ocr_dimension)), decode_job_id: Rc::new(Cell::new(0)), ocr_job_id: Rc::new(Cell::new(0)), on_file_loaded: Rc::new(RefCell::new(None)), @@ -92,6 +94,11 @@ impl ViewerController { *self.ocr.borrow_mut() = ocr; } + /// Change the OCR max-dimension guardrail for subsequent loads. + pub fn set_max_ocr_dimension(&self, max: u32) { + self.max_ocr_dimension.set(max); + } + /// Register a callback fired whenever a file finishes loading (or fails). /// /// If a file has already been loaded, the callback is invoked immediately @@ -118,6 +125,11 @@ impl ViewerController { .map(|s| s.to_string_lossy().into_owned()) .unwrap_or_else(|| path.display().to_string()); let size_bytes = std::fs::metadata(path).ok().map(|m| m.len()); + // Stamp the file identity before the decode reads its content: the + // OCR cache key must describe the bytes that were actually decoded, + // not whatever the file becomes while the async decode runs (see + // cache::FileStamp). + let stamp = cache::FileStamp::read(path); // Supersede any in-flight decode. let decode_id = self.decode_job_id.get().wrapping_add(1); @@ -133,13 +145,15 @@ impl ViewerController { let this = self.clone(); let path = path.to_path_buf(); + let started = std::time::Instant::now(); glib::MainContext::default().spawn_local(async move { let result = crate::decode::decode_texture(&path).await; - this.finish_decode(decode_id, path, name, size_bytes, result); + this.finish_decode(decode_id, path, name, size_bytes, result, started, stamp); }); } /// Apply a finished decode, unless a newer `load_file` superseded it. + #[allow(clippy::too_many_arguments)] fn finish_decode( &self, job_id: u64, @@ -147,6 +161,8 @@ impl ViewerController { name: String, size_bytes: Option, result: anyhow::Result, + started: std::time::Instant, + stamp: cache::FileStamp, ) { if job_id != self.decode_job_id.get() { // Late result for a file the user already navigated away from. @@ -155,6 +171,14 @@ impl ViewerController { match result { Ok(texture) => { + tracing::debug!( + target: "quickview::perf", + elapsed_ms = started.elapsed().as_millis() as u64, + width = texture.width(), + height = texture.height(), + path = %path.display(), + "decode" + ); let info = FileInfo { name, width: texture.width(), @@ -162,9 +186,25 @@ impl ViewerController { size_bytes, load_failed: false, }; - self.overlay.set_texture(texture); + self.overlay.set_texture(texture.clone()); self.emit_file_loaded(info); - self.start_ocr(path); + // Re-stat now that the decode is done: if the file changed + // while the decoder had it open, the texture's provenance is + // ambiguous (old or new bytes), so the cache must sit this + // load out — a hit could paint stale boxes over new pixels + // and a store could file this OCR under a mismatched key. + // Matching nanosecond stamps on both sides of the decode pin + // the decoded version to the key. + let stamp = { + let after = cache::FileStamp::read(&path); + if after != stamp { + tracing::debug!( + "file changed during decode; skipping OCR cache for this load" + ); + } + (after == stamp).then_some(stamp) + }; + self.start_ocr(path, &texture, stamp); } Err(err) => { tracing::error!("Failed to load image: {err:#}"); @@ -216,12 +256,41 @@ impl ViewerController { self.overlay.copy_selection_to_clipboard(); } - fn start_ocr(&self, path: PathBuf) { + fn start_ocr( + &self, + path: PathBuf, + texture: >k::gdk::Texture, + stamp: Option, + ) { self.overlay.set_ocr_busy(true); self.overlay.set_ocr_result(None); let ocr_opts = self.ocr.borrow().clone(); + // Max-dimension guardrail: oversized images are fed to tesseract as + // a downscaled temp copy made from the decoded texture (see + // ocr_prep). Any prep failure degrades to full-resolution OCR — the + // guardrail is a performance measure, not a correctness one. + let plan = downscale::plan_downscale( + texture.width().max(0) as u32, + texture.height().max(0) as u32, + self.max_ocr_dimension.get(), + ); + + // The cache key uses the *planned* downscale target — the size the + // guardrail intends tesseract to see — so it is derivable before any + // pixels are prepared, and a cache hit never touches the pixels at + // all. Degraded runs (failed download or scale) store their + // full-resolution result under the same planned key: strictly better + // content, and future opens then hit it like any other entry. + let downscale_target = plan.map(|p| (p.target_w, p.target_h)); + // No stamp (file changed during decode) means no entry: the worker + // neither reads nor writes the cache for this load. + let entry = stamp.and_then(|stamp| { + cache::cache_dir() + .map(|root| cache::ocr_cache_path(&root, &path, &ocr_opts, downscale_target, stamp)) + }); + let (sender, receiver) = async_channel::bounded::<( u64, anyhow::Result, @@ -231,23 +300,113 @@ impl ViewerController { let new_id = self.ocr_job_id.get().wrapping_add(1); self.ocr_job_id.set(new_id); - // All cache I/O stays on the worker thread; hits flow through the same - // channel as fresh results, so the job-id guard applies unchanged. - let cache_root = cache::cache_dir(); + // All cache I/O and all pixel work stays on the worker thread; hits + // flow through the same channel as fresh results, so the job-id + // guard applies unchanged. The texture travels with the closure: + // GdkTexture is immutable and threadsafe (decode.rs already sends + // textures across threads), so the download happens only after the + // authoritative cache read misses — never for a hit, and never on + // the main thread. + let texture = texture.clone(); std::thread::spawn(move || { let r = (|| { - // Snapshot the cache key before OCR runs: if the file is + // The entry was snapshotted before OCR runs: if the file is // edited mid-OCR, the stale result lands under the old key, // which the edited file then correctly misses. - let entry = cache_root - .as_deref() - .map(|root| cache::ocr_cache_path(root, &path, &ocr_opts)); if let Some(cached) = entry.as_deref().and_then(cache::load_ocr) { tracing::debug!("OCR cache hit for {}", path.display()); return Ok(cached); } - let tsv_out = tesseract::run_tesseract_tsv(&path, &ocr_opts)?; - let parsed = tsv::parse_tesseract_tsv(&tsv_out)?; + + // Materialize the temp copy tesseract will read (cache + // misses only — a hit never needs the pixels). The temp file + // guard must outlive the tesseract run; drop deletes it on + // every path. + // + // With an unknown stamp (the file changed during decode) the + // live path cannot be trusted to match the displayed texture, + // so the decoded pixels are the only safe OCR input: OCR them + // at original size (factor 1.0) and never fall back to the + // live file. With a known stamp, a prep failure degrades to + // full resolution, which stores a (strictly better) full-res + // result under the downscaled key. + let pixels_only = stamp.is_none(); + let prep_plan = plan.or_else(|| { + pixels_only.then(|| downscale::DownscalePlan { + target_w: texture.width().max(1) as u32, + target_h: texture.height().max(1) as u32, + factor: 1.0, + }) + }); + let mut ocr_input = path.clone(); + let mut tmp_guard = None; + let mut factors = None; + if let Some(plan) = &prep_plan { + let prep_started = std::time::Instant::now(); + let downscaled = crate::ocr_prep::download_rgba(&texture) + .and_then(|pixels| crate::ocr_prep::write_downscaled_png(&pixels, plan)); + match downscaled { + Ok(downscaled) => { + tracing::debug!( + target: "quickview::perf", + elapsed_ms = prep_started.elapsed().as_millis() as u64, + factor = plan.factor, + target_w = downscaled.target.0, + target_h = downscaled.target.1, + "downscale prep" + ); + ocr_input = downscaled.path().to_path_buf(); + factors = Some((downscaled.factor_x, downscaled.factor_y)); + tmp_guard = Some(downscaled); + } + Err(err) if pixels_only => { + return Err(err.context( + "cannot OCR from decoded pixels and the live file is untrusted", + )); + } + Err(err) => { + tracing::warn!("downscale failed; OCR at full resolution: {err:#}"); + } + } + } + + let ocr_started = std::time::Instant::now(); + let tsv_out = tesseract::run_tesseract_tsv(&ocr_input, &ocr_opts)?; + let mut parsed = tsv::parse_tesseract_tsv(&tsv_out)?; + tracing::debug!( + target: "quickview::perf", + elapsed_ms = ocr_started.elapsed().as_millis() as u64, + words = parsed.words.len(), + lang = %ocr_opts.lang, + downscaled = tmp_guard.is_some(), + "ocr" + ); + + // A full-resolution run had tesseract read the *live* file, + // which may have been replaced since the decode; a stamp + // mismatch means these words describe different bytes than + // the displayed texture (and than the entry's key), so the + // result is dropped — no overlay, nothing stored. Pixel-fed + // runs (downscaled, or pixels_only above) OCR the decoded + // texture and are immune, so only the stamped full-resolution + // path needs re-verification. + if tmp_guard.is_none() { + if let Some(stamp) = stamp { + if cache::FileStamp::read(&path) != stamp { + anyhow::bail!("file changed during OCR; discarding mismatched result"); + } + } + } + drop(tmp_guard); + + // Bboxes go back to original image space before caching and + // indexing: everything downstream only sees original-space + // coordinates, so cache hits are indistinguishable from + // full-resolution parses. + if let Some((fx, fy)) = factors { + downscale::upscale_bboxes(&mut parsed, fx, fy); + } + // Empty results are cached too (text-free images shouldn't // re-run tesseract); failures are not, so transient errors // retry on the next open. diff --git a/crates/quickview/src/main.rs b/crates/quickview/src/main.rs index beb1c5d..6064b49 100644 --- a/crates/quickview/src/main.rs +++ b/crates/quickview/src/main.rs @@ -27,6 +27,11 @@ struct Cli { #[arg(long)] tessdata_dir: Option, + /// Max image dimension (px) before OCR works on a downscaled copy; + /// 0 disables. Overrides the config file; defaults to 4000. + #[arg(long)] + max_ocr_dim: Option, + /// Image file path. Use '-' (or omit) to read a path from stdin. file: Option, } @@ -53,12 +58,13 @@ fn main() -> Result<()> { // single-instance app, the primary must never re-resolve a remote // invocation's environment or config — only fully resolved values cross // the process boundary (see quickview-ui's ipc module). - let ocr = resolve_ocr_options(cli.lang, cli.tessdata_dir); + let (ocr, max_ocr_dimension) = resolve_ocr_options(cli.lang, cli.tessdata_dir, cli.max_ocr_dim); let code = quickview_ui::run(quickview_ui::LaunchOptions { mode, file: file_path, ocr, + max_ocr_dimension, })?; std::process::exit(code); @@ -72,8 +78,10 @@ fn main() -> Result<()> { fn resolve_ocr_options( cli_lang: Option, cli_tessdata_dir: Option, -) -> quickview_core::ocr::tesseract::OcrOptions { + cli_max_ocr_dim: Option, +) -> (quickview_core::ocr::tesseract::OcrOptions, u32) { use quickview_core::config; + use quickview_core::ocr::downscale::DEFAULT_MAX_OCR_DIMENSION; let cfg = config::config_path() .map(|path| { @@ -85,12 +93,16 @@ fn resolve_ocr_options( .unwrap_or_default(); let env_lang = std::env::var("QUICKVIEW_LANG").ok(); - quickview_core::ocr::tesseract::OcrOptions { + let opts = quickview_core::ocr::tesseract::OcrOptions { lang: config::resolve_lang(cli_lang.as_deref(), env_lang.as_deref(), &cfg), tessdata_dir: non_blank(cli_tessdata_dir) .or_else(|| non_blank(cfg.ocr.tessdata_dir.clone())) .map(absolutize), - } + }; + let max_ocr_dimension = cli_max_ocr_dim + .or(cfg.ocr.max_dimension) + .unwrap_or(DEFAULT_MAX_OCR_DIMENSION); + (opts, max_ocr_dimension) } /// Blank is unset, at each precedence level — same rule as `resolve_lang`. diff --git a/docs/PHASED_PLAN.md b/docs/PHASED_PLAN.md index 32404db..e56c155 100644 --- a/docs/PHASED_PLAN.md +++ b/docs/PHASED_PLAN.md @@ -170,7 +170,9 @@ If priorities change, you can reshuffle phases, but try to keep the “render fi `~/.cache/quickview/ocr/`, checked/written on the OCR worker thread, atomic writes, no eviction in v1. Survives restarts, so Quick Preview's process-per-invocation benefits too. -- Add basic benchmarking hooks (decode + OCR timing) +- Add basic benchmarking hooks ✅ — `tracing` debug events under target + `quickview::perf` (decode, OCR cache hit, downscale prep, tesseract+parse); + opt in with `RUST_LOG=quickview::perf=debug` - Improve OCR accuracy options: - language selection via config file / env var ✅ — precedence `--lang` > `QUICKVIEW_LANG` > `~/.config/quickview/config.toml` > `eng` @@ -180,8 +182,15 @@ If priorities change, you can reshuffle phases, but try to keep the “render fi the fast/best sets have no conventional install location, users clone them anywhere. Joins the OCR cache key per ADR-0009. - Add guardrails: - - maximum image dimensions for OCR (downscale) - - memory usage limits (where feasible) + - maximum image dimensions for OCR ✅ — `[ocr] max_dimension` (default + 4000, 0 disables, `--max-ocr-dim` overrides): oversized images are fed to + tesseract as a downscaled temp PNG produced from the already-decoded + texture (never an in-process re-decode of the untrusted file); word boxes + are mapped back to original image space before caching/indexing, and the + effective target joins the cache key + - memory usage limits — deferred to Phase 8 (the transient RGBA copy during + downscale prep is the only known spike, bounded and measured by the perf + hooks) **Definition of done** - Opening a large image does not freeze or hitch the UI diff --git a/templates/config.example.toml b/templates/config.example.toml index 8aa2ad3..29c4b11 100644 --- a/templates/config.example.toml +++ b/templates/config.example.toml @@ -18,6 +18,7 @@ # Changing this re-runs OCR (it is part of the cache key). #tessdata_dir = "/home/you/src/tessdata_fast" -# Maximum image dimension (px) before OCR works on a downscaled copy. -# Accepted but not consumed yet; the guardrail lands in a follow-up. +# Maximum image dimension (px) before OCR works on a downscaled temporary +# copy (word positions are mapped back to the full-size image). 0 disables +# the guardrail. Overridden by --max-ocr-dim. Default: 4000. #max_dimension = 4000