Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .claude/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ The scaffold is functional with image display, async OCR pipeline, drag-select o
- Drag-select overlay with word highlighting
- Ctrl+C clipboard copy
- Stale OCR result cancellation via monotonic job IDs
- Zoom & pan (Ctrl+scroll, pinch, +/- keys, middle-drag pan) via custom `ZoomableCanvas` widget

### What's not implemented yet:
- Zoom and pan
- Info panel (filename, dimensions, file size)
- Context menu (right-click copy)
- OCR caching (cache module exists but is not wired up)
Expand Down Expand Up @@ -81,5 +81,6 @@ cargo test --all
- Never block the GTK main thread — all OCR and I/O runs on background threads
- Use async-channel to send results back to the UI thread
- quickview-core must have zero GTK dependencies (keeps it testable without a display server)
- Coordinate transforms go through `compute_contain_transform()` — image coords vs widget coords
- Coordinate transforms go through `ViewTransform::from_center()` and related methods in `geometry.rs` — image coords vs widget coords. Fields are private; use `.scale()`, `.offset_x()`, `.offset_y()` getters. `contain()` returns `ContainResult`.
- OCR results use image-space coordinates; convert to widget-space only for rendering
- OCR hit-testing uses `OcrWordIndex` spatial index (`ocr/index.rs`) for efficient drag-select queries
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ jobs:
gtk4 libadwaita \
tesseract tesseract-data-eng \
gtk4-layer-shell
pkg-config --atleast-version=4.10 gtk4
rustup default stable

- name: Cache cargo registry and build artifacts
Expand Down Expand Up @@ -74,6 +75,7 @@ jobs:
gtk4 libadwaita \
tesseract tesseract-data-eng \
gtk4-layer-shell
pkg-config --atleast-version=4.10 gtk4
rustup default stable

- name: Cache cargo registry, build artifacts, and tools
Expand Down
4 changes: 2 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Primary target is Arch Linux + Wayland (wlroots compositors like Sway/Hyprland/n
## Repo Layout

- `crates/quickview/`: CLI entrypoint (`quickview` binary).
- `crates/quickview-core/`: non-GTK core (OCR parsing, geometry, selection logic, cache helpers).
- `crates/quickview-core/`: non-GTK core (OCR parsing, geometry/ViewTransform, spatial index, selection logic, cache helpers).
- `crates/quickview-ui/`: GTK4/libadwaita UI (full viewer + quick preview windows, overlay widget).
- `docs/`: phased plan, architecture, decisions, development notes.
- `adrs/`: deeper architecture decisions.
Expand Down Expand Up @@ -83,7 +83,7 @@ GitHub Actions runs in an `archlinux:latest` container and installs system packa
- Quick Preview window: `crates/quickview-ui/src/windows/quick_preview.rs`
- Full viewer window: `crates/quickview-ui/src/windows/full_viewer.rs`
- Viewer controller (loads images, kicks OCR, ignores late results): `crates/quickview-ui/src/windows/shared.rs`
- Overlay + drag selection rendering: `crates/quickview-ui/src/widgets/image_overlay.rs`
- Image rendering, zoom/pan, drag selection, OCR overlay: `crates/quickview-ui/src/widgets/image_overlay.rs` (contains `ImageOverlayWidget` wrapper + `ZoomableCanvas` custom widget subclass)
- Tesseract invocation + TSV parsing: `crates/quickview-core/src/ocr/`

## Project Invariants (Don't Break These)
Expand Down
12 changes: 11 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,15 @@

## Unreleased

- Initial scaffold.
### Added
- **Zoom & pan** — Ctrl+scroll zoom (anchored at cursor), pinch-to-zoom,
`+`/`-` keyboard zoom, `0`/`Home` reset to fit-to-window. Middle-click drag
or Ctrl+left-drag to pan. Works in both Full Viewer and Quick Preview.
Selection and OCR highlights stay aligned at all zoom levels.
- **Spatial index for OCR hit-testing** — `OcrWordIndex` uniform-grid index
replaces linear scan during drag-select for faster word lookup.
- **ViewTransform hardening** — validated constructor rejects non-finite and
non-positive scale values; fields are now private with getters.
- **CI GTK4 version check** — `pkg-config --atleast-version=4.10 gtk4` in CI.

- Initial scaffold.
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,13 @@ quickview --quick-preview photo.png
quickview photo.png
```

**OCR Text Selection** — Tesseract runs asynchronously after the image loads. Drag to select recognized words, `Ctrl+C` to copy.
**Zoom & Pan** — `Ctrl+scroll` to zoom at cursor, pinch-to-zoom on touchpad, `+`/`-` keys, `0` to reset. Middle-click drag or `Ctrl+left-drag` to pan.

**OCR Text Selection** — Tesseract runs asynchronously after the image loads. Drag to select recognized words, `Ctrl+C` to copy. Selection stays aligned at any zoom level.
Comment on lines +38 to +40
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add Home to the documented reset shortcuts.

This line currently omits Home, which is documented elsewhere in this PR as a reset keybinding.

📝 Suggested doc patch
-**Zoom & Pan** — `Ctrl+scroll` to zoom at cursor, pinch-to-zoom on touchpad, `+`/`-` keys, `0` to reset. Middle-click drag or `Ctrl+left-drag` to pan.
+**Zoom & Pan** — `Ctrl+scroll` to zoom at cursor, pinch-to-zoom on touchpad, `+`/`-` keys, `0`/`Home` to reset. Middle-click drag or `Ctrl+left-drag` to pan.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
**Zoom & Pan**`Ctrl+scroll` to zoom at cursor, pinch-to-zoom on touchpad, `+`/`-` keys, `0` to reset. Middle-click drag or `Ctrl+left-drag` to pan.
**OCR Text Selection** — Tesseract runs asynchronously after the image loads. Drag to select recognized words, `Ctrl+C` to copy. Selection stays aligned at any zoom level.
**Zoom & Pan**`Ctrl+scroll` to zoom at cursor, pinch-to-zoom on touchpad, `+`/`-` keys, `0`/`Home` to reset. Middle-click drag or `Ctrl+left-drag` to pan.
**OCR Text Selection** — Tesseract runs asynchronously after the image loads. Drag to select recognized words, `Ctrl+C` to copy. Selection stays aligned at any zoom level.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 38 - 40, Update the "Zoom & Pan" README entry to
include the Home key as a reset shortcut by appending "or `Home`" to the reset
list (the line under the "Zoom & Pan" heading that currently reads "`0` to
reset"), so the reset shortcuts mention `0` and `Home` consistently with other
documentation in this PR.


## Requirements

Arch Linux (primary target):
Arch Linux (primary target, requires GTK4 >= 4.10):

```bash
sudo pacman -S --needed \
Expand Down
294 changes: 294 additions & 0 deletions crates/quickview-core/src/geometry.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use serde::{Deserialize, Serialize};

use std::fmt;

#[derive(Debug, Default, Clone, Copy, PartialEq, Serialize, Deserialize)]
pub struct Point {
pub x: f64,
Expand Down Expand Up @@ -41,3 +43,295 @@ impl Rect {
self.x < bx2 && ax2 > other.x && self.y < by2 && ay2 > other.y
}
}

/// Result of `ViewTransform::contain()`.
///
/// This represents the baseline "fit to widget" (contain) scale and the widget-space
/// center point used by `ViewTransform::from_center()`.
#[derive(Debug, Clone, Copy, PartialEq)]
pub struct ContainResult {
/// Uniform scale that fits the entire image inside the widget.
pub contain_scale: f64,

/// Center of the widget in widget coordinates (pixels).
pub widget_center: Point,
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ViewTransformError {
NonFinite,
NonPositiveScale,
}

impl fmt::Display for ViewTransformError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
ViewTransformError::NonFinite => write!(f, "non-finite view transform value"),
ViewTransformError::NonPositiveScale => write!(f, "scale must be > 0"),
}
}
}

impl std::error::Error for ViewTransformError {}

#[derive(Debug, Clone, Copy, PartialEq)]
pub struct ViewTransform {
scale: f64,
offset_x: f64,
offset_y: f64,
}

impl ViewTransform {
pub fn new(scale: f64, offset_x: f64, offset_y: f64) -> Result<Self, ViewTransformError> {
if !scale.is_finite() || !offset_x.is_finite() || !offset_y.is_finite() {
return Err(ViewTransformError::NonFinite);
}
if scale <= 0.0 {
return Err(ViewTransformError::NonPositiveScale);
}
Ok(Self {
scale,
offset_x,
offset_y,
})
}

pub fn scale(&self) -> f64 {
self.scale
}

pub fn offset_x(&self) -> f64 {
self.offset_x
}

pub fn offset_y(&self) -> f64 {
self.offset_y
}

/// Compute the baseline "contain" (fit-to-widget) scale.
///
/// The returned `widget_center` is in widget coordinates (pixels) and is the point
/// that `from_center()` treats as the widget's visual center anchor.
pub fn contain(widget_w: f64, widget_h: f64, image_w: f64, image_h: f64) -> ContainResult {
let widget_center = Point {
x: widget_w.max(0.0) * 0.5,
y: widget_h.max(0.0) * 0.5,
};

if widget_w <= 0.0 || widget_h <= 0.0 || image_w <= 0.0 || image_h <= 0.0 {
return ContainResult {
contain_scale: 1.0,
widget_center,
};
}

let contain_scale = (widget_w / image_w)
.min(widget_h / image_h)
.max(f64::MIN_POSITIVE);
ContainResult {
contain_scale,
widget_center,
}
}

/// Construct a `ViewTransform` from canonical view state.
///
/// `center_img.x` and `center_img.y` must be finite. This function delegates validation
/// to `ViewTransform::new` and will panic if invariants are violated.
pub fn from_center(
widget_w: f64,
widget_h: f64,
image_w: f64,
image_h: f64,
zoom_factor: f64,
center_img: Point,
) -> Self {
// `from_center()` delegates invariants to `ViewTransform::new`.
// `center_img.x` / `center_img.y` must be finite or `ViewTransform::new` will error.
debug_assert!(
center_img.x.is_finite() && center_img.y.is_finite(),
"from_center: center_img must be finite (x={}, y={})",
center_img.x,
center_img.y
);

let contain = Self::contain(widget_w, widget_h, image_w, image_h);
let scale =
(contain.contain_scale * zoom_factor.max(f64::MIN_POSITIVE)).max(f64::MIN_POSITIVE);

let offset_x = contain.widget_center.x - center_img.x * scale;
let offset_y = contain.widget_center.y - center_img.y * scale;
Self::new(scale, offset_x, offset_y).expect("ViewTransform invariants violated")
}
Comment on lines +141 to +165
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find crates -name "geometry.rs" -type f

Repository: Green2Grey2/QuickView

Length of output: 102


🏁 Script executed:

head -20 crates/quickview-core/src/geometry.rs

Repository: Green2Grey2/QuickView

Length of output: 460


🏁 Script executed:

sed -n '141,165p' crates/quickview-core/src/geometry.rs

Repository: Green2Grey2/QuickView

Length of output: 1063


🏁 Script executed:

rg -A 5 "ViewTransformError" crates/quickview-core/src/

Repository: Green2Grey2/QuickView

Length of output: 2210


🏁 Script executed:

rg -B 2 -A 10 "impl ViewTransform" crates/quickview-core/src/geometry.rs

Repository: Green2Grey2/QuickView

Length of output: 487


🏁 Script executed:

rg "from_center" crates/quickview-core/src/ -B 2 -A 5

Repository: Green2Grey2/QuickView

Length of output: 5154


Avoid panicking from public from_center() on invalid runtime input.

from_center() currently calls .expect(...) which will panic if invariants are violated. Non-finite inputs to center_img, or NaN/Inf results from intermediate calculations, bypass the debug_assert! in release builds and reach ViewTransform::new(), which returns an error. Prefer returning Result<Self, ViewTransformError> to allow proper error propagation.

🔧 Proposed change
-    pub fn from_center(
+    pub fn from_center(
         widget_w: f64,
         widget_h: f64,
         image_w: f64,
         image_h: f64,
         zoom_factor: f64,
         center_img: Point,
-    ) -> Self {
+    ) -> Result<Self, ViewTransformError> {
         // `from_center()` delegates invariants to `ViewTransform::new`.
         // `center_img.x` / `center_img.y` must be finite or `ViewTransform::new` will error.
         debug_assert!(
             center_img.x.is_finite() && center_img.y.is_finite(),
             "from_center: center_img must be finite (x={}, y={})",
             center_img.x,
             center_img.y
         );

         let contain = Self::contain(widget_w, widget_h, image_w, image_h);
         let scale =
             (contain.contain_scale * zoom_factor.max(f64::MIN_POSITIVE)).max(f64::MIN_POSITIVE);

         let offset_x = contain.widget_center.x - center_img.x * scale;
         let offset_y = contain.widget_center.y - center_img.y * scale;
-        Self::new(scale, offset_x, offset_y).expect("ViewTransform invariants violated")
+        Self::new(scale, offset_x, offset_y)
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pub fn from_center(
widget_w: f64,
widget_h: f64,
image_w: f64,
image_h: f64,
zoom_factor: f64,
center_img: Point,
) -> Self {
// `from_center()` delegates invariants to `ViewTransform::new`.
// `center_img.x` / `center_img.y` must be finite or `ViewTransform::new` will error.
debug_assert!(
center_img.x.is_finite() && center_img.y.is_finite(),
"from_center: center_img must be finite (x={}, y={})",
center_img.x,
center_img.y
);
let contain = Self::contain(widget_w, widget_h, image_w, image_h);
let scale =
(contain.contain_scale * zoom_factor.max(f64::MIN_POSITIVE)).max(f64::MIN_POSITIVE);
let offset_x = contain.widget_center.x - center_img.x * scale;
let offset_y = contain.widget_center.y - center_img.y * scale;
Self::new(scale, offset_x, offset_y).expect("ViewTransform invariants violated")
}
pub fn from_center(
widget_w: f64,
widget_h: f64,
image_w: f64,
image_h: f64,
zoom_factor: f64,
center_img: Point,
) -> Result<Self, ViewTransformError> {
// `from_center()` delegates invariants to `ViewTransform::new`.
// `center_img.x` / `center_img.y` must be finite or `ViewTransform::new` will error.
debug_assert!(
center_img.x.is_finite() && center_img.y.is_finite(),
"from_center: center_img must be finite (x={}, y={})",
center_img.x,
center_img.y
);
let contain = Self::contain(widget_w, widget_h, image_w, image_h);
let scale =
(contain.contain_scale * zoom_factor.max(f64::MIN_POSITIVE)).max(f64::MIN_POSITIVE);
let offset_x = contain.widget_center.x - center_img.x * scale;
let offset_y = contain.widget_center.y - center_img.y * scale;
Self::new(scale, offset_x, offset_y)
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/quickview-core/src/geometry.rs` around lines 141 - 165, Change
from_center to return Result<Self, ViewTransformError> instead of panicking:
validate inputs (e.g., ensure center_img.x/y are finite and computed
scale/offsets are finite) and propagate the error from ViewTransform::new rather
than calling .expect(...). Locate from_center which calls Self::contain(...)
(use contain.contain_scale and contain.widget_center) and replace the final
Self::new(scale, offset_x, offset_y).expect(...) with returning the Result
produced by ViewTransform::new (or map/convert its error to ViewTransformError),
returning Err early on invalid inputs.


pub fn image_to_widget(&self, point: Point) -> Point {
Point {
x: self.offset_x + point.x * self.scale,
y: self.offset_y + point.y * self.scale,
}
}

pub fn widget_to_image(&self, point: Point) -> Point {
Point {
x: (point.x - self.offset_x) / self.scale,
y: (point.y - self.offset_y) / self.scale,
}
}

pub fn image_rect_to_widget(&self, rect: Rect) -> Rect {
Rect {
x: self.offset_x + rect.x * self.scale,
y: self.offset_y + rect.y * self.scale,
w: rect.w * self.scale,
h: rect.h * self.scale,
}
}

pub fn widget_rect_to_image(&self, rect: Rect) -> Rect {
Rect {
x: (rect.x - self.offset_x) / self.scale,
y: (rect.y - self.offset_y) / self.scale,
w: rect.w / self.scale,
h: rect.h / self.scale,
}
}

pub fn clamp_center(
widget_w: f64,
widget_h: f64,
image_w: f64,
image_h: f64,
scale: f64,
center_img: Point,
) -> Point {
if widget_w <= 0.0 || widget_h <= 0.0 || image_w <= 0.0 || image_h <= 0.0 || scale <= 0.0 {
return center_img;
}

let half_view_w = widget_w / (2.0 * scale);
let half_view_h = widget_h / (2.0 * scale);

let center_x = if image_w * scale <= widget_w {
image_w * 0.5
} else {
center_img.x.clamp(half_view_w, image_w - half_view_w)
};

let center_y = if image_h * scale <= widget_h {
image_h * 0.5
} else {
center_img.y.clamp(half_view_h, image_h - half_view_h)
Comment on lines +207 to +223
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "geometry.rs" -path "*/crates/quickview-core/*" 2>/dev/null

Repository: Green2Grey2/QuickView

Length of output: 104


🏁 Script executed:

cat -n crates/quickview-core/src/geometry.rs | head -250 | tail -100

Repository: Green2Grey2/QuickView

Length of output: 3659


🏁 Script executed:

wc -l crates/quickview-core/src/geometry.rs

Repository: Green2Grey2/QuickView

Length of output: 106


🏁 Script executed:

cat -n crates/quickview-core/src/geometry.rs | sed -n '180,230p'

Repository: Green2Grey2/QuickView

Length of output: 1828


🏁 Script executed:

rg -l "clamp_center" -t rust

Repository: Green2Grey2/QuickView

Length of output: 151


🏁 Script executed:

rg "clamp_center" -B 5 -A 30

Repository: Green2Grey2/QuickView

Length of output: 21460


🏁 Script executed:

ls -la crates/quickview-core/src/

Repository: Green2Grey2/QuickView

Length of output: 459


🏁 Script executed:

find . -path "*/crates/quickview-core/src/geometry.rs"

Repository: Green2Grey2/QuickView

Length of output: 104


🏁 Script executed:

git ls-files | grep geometry

Repository: Green2Grey2/QuickView

Length of output: 102


🏁 Script executed:

find . -name "*.rs" -type f | head -20

Repository: Green2Grey2/QuickView

Length of output: 799


🌐 Web query:

Rust f64 clamp NaN infinity behavior documentation

💡 Result:

f64::clamp(min, max) is documented to behave as follows: it returns max if self > max, min if self < min, otherwise returns selfexcept it “restrict[s] a value to a certain interval unless it is NaN.” In particular: if self is NaN, the result is NaN; if min or max is NaN (or min > max), it panics; and if the clamped result is zero with mixed-sign zeros among self/min/max, the sign of the returned zero (0.0 vs -0.0) is non-deterministic. This definition also implies that ±∞ will clamp to the nearest bound when it lies outside [min, max] (e.g., INFINITY.clamp(min, max_finite) == max_finite). [1]

Source: Rust core primitive f64 docs (f64::clamp). [1]


🏁 Script executed:

rg "widget_w.*widget_h.*image_w" -A 20

Repository: Green2Grey2/QuickView

Length of output: 18286


🏁 Script executed:

rg "let half_view_w = widget_w" -B 10 -A 15

Repository: Green2Grey2/QuickView

Length of output: 1802


🏁 Script executed:

git log --oneline -5 2>/dev/null

Repository: Green2Grey2/QuickView

Length of output: 146


🏁 Script executed:

ls -la .git 2>/dev/null | head -5

Repository: Green2Grey2/QuickView

Length of output: 294


🏁 Script executed:

find . -type f -name "*.rs" 2>/dev/null | wc -l

Repository: Green2Grey2/QuickView

Length of output: 67


🏁 Script executed:

find . -type f -name "*.rs" 2>/dev/null | head -20

Repository: Green2Grey2/QuickView

Length of output: 799


🏁 Script executed:

find . -maxdepth 3 -type f -name "*.rs" 2>/dev/null | head -10

Repository: Green2Grey2/QuickView

Length of output: 47


🌐 Web query:

Rust f64::clamp behavior with NaN infinity non-finite values

💡 Result:

f64::clamp(self, min, max) clamps using IEEE-754 comparisons, except it has special rules for NaN and may be nondeterministic for signed zero:

  • If self is NaN: returns NaN (it “doesn’t clamp NaN away”). [1]
  • If min is NaN or max is NaN: panics. [1]
  • If min > max: panics. [1]
  • If values are ±infinity (and bounds are not NaN):
    • self = +∞ → returns max (unless max is also +∞, then returns +∞).
    • self = -∞ → returns min (unless min is also -∞, then returns -∞).
    • Infinite bounds are allowed; they just affect the comparisons normally. [1]
  • Signed zero corner case: if the clamped result is zero and the inputs contain both 0.0 and -0.0, the function may return either sign non-deterministically. [1]

Examples (from the docs): NaN stays NaN; and signed-zero sign may vary. [1]

Source: Rust f64::clamp documentation. [1]

[1] (doc.rust-lang.org)

Citations:


🏁 Script executed:

rg "clamp.*NaN\|NaN.*clamp\|is_finite" -t rust

Repository: Green2Grey2/QuickView

Length of output: 47


🏁 Script executed:

rg "fn clamp_center\|pub fn clamp_center" -B 5 -A 30

Repository: Green2Grey2/QuickView

Length of output: 47


🏁 Script executed:

rg "clamp_center|center_img" -t rust | head -30

Repository: Green2Grey2/QuickView

Length of output: 3056


🏁 Script executed:

python3 << 'EOF'
import math

# Simulating the exact code from the review
widget_w = 100.0
widget_h = 100.0
image_w = 500.0
image_h = 500.0
scale = 1.0
center_img_x = float('nan')
center_img_y = float('nan')

# Current guard
guard_check = (widget_w <= 0.0 or widget_h <= 0.0 or image_w <= 0.0 or 
               image_h <= 0.0 or scale <= 0.0)
print(f"Current guard catches NaN inputs: {guard_check}")
print(f"  (All checks are False, so guard doesn't catch NaN)\n")

# Proposed guard with is_finite()
finite_check = (math.isfinite(widget_w) and math.isfinite(widget_h) and 
                math.isfinite(image_w) and math.isfinite(image_h) and 
                math.isfinite(scale) and math.isfinite(center_img_x) and 
                math.isfinite(center_img_y))
print(f"Proposed guard with is_finite() catches NaN inputs: {not finite_check}")
print(f"  (center_img values are NaN, so guard would catch it)\n")

# What happens without the fix
def clamp(v, lo, hi):
    return min(max(v, lo), hi)

half_view_w = widget_w / (2.0 * scale)
image_w_minus_half = image_w - half_view_w

result = clamp(center_img_x, half_view_w, image_w_minus_half)
print(f"Result of clamp(NaN, {half_view_w}, {image_w_minus_half}) = {result}")
print(f"  Is NaN: {math.isnan(result)}")
print(f"  Problem: Function would return center_img which is (NaN, NaN)")
EOF

Repository: Green2Grey2/QuickView

Length of output: 390


🏁 Script executed:

rg "fn clamp_center" -A 50

Repository: Green2Grey2/QuickView

Length of output: 5159


🏁 Script executed:

find . -type f -name "*.rs" 2>/dev/null | xargs grep -l "clamp_center\|half_view_w" | head -3

Repository: Green2Grey2/QuickView

Length of output: 155


🏁 Script executed:

rg "let half_view_w" -B 20 -A 20

Repository: Green2Grey2/QuickView

Length of output: 2700


clamp_center() must guard against non-finite inputs before arithmetic operations.

Current guard only checks <= 0.0, which cannot catch NaN or Inf values. This creates two failure modes:

  1. Panic risk: If any dimension parameter is NaN, computing half_view_w = widget_w / (2.0 * scale) produces NaN. Passing NaN as a clamp bound causes a runtime panic.
  2. Silent corruption: If center_img.x or center_img.y are NaN, they bypass the guard and propagate through clamp as NaN (which returns NaN unchanged), corrupting output.

Add is_finite() checks for all float parameters before any arithmetic:

Proposed fix
-        if widget_w <= 0.0 || widget_h <= 0.0 || image_w <= 0.0 || image_h <= 0.0 || scale <= 0.0 {
+        if !widget_w.is_finite()
+            || !widget_h.is_finite()
+            || !image_w.is_finite()
+            || !image_h.is_finite()
+            || !scale.is_finite()
+            || !center_img.x.is_finite()
+            || !center_img.y.is_finite()
+            || widget_w <= 0.0
+            || widget_h <= 0.0
+            || image_w <= 0.0
+            || image_h <= 0.0
+            || scale <= 0.0
+        {
             return center_img;
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if widget_w <= 0.0 || widget_h <= 0.0 || image_w <= 0.0 || image_h <= 0.0 || scale <= 0.0 {
return center_img;
}
let half_view_w = widget_w / (2.0 * scale);
let half_view_h = widget_h / (2.0 * scale);
let center_x = if image_w * scale <= widget_w {
image_w * 0.5
} else {
center_img.x.clamp(half_view_w, image_w - half_view_w)
};
let center_y = if image_h * scale <= widget_h {
image_h * 0.5
} else {
center_img.y.clamp(half_view_h, image_h - half_view_h)
if !widget_w.is_finite()
|| !widget_h.is_finite()
|| !image_w.is_finite()
|| !image_h.is_finite()
|| !scale.is_finite()
|| !center_img.x.is_finite()
|| !center_img.y.is_finite()
|| widget_w <= 0.0
|| widget_h <= 0.0
|| image_w <= 0.0
|| image_h <= 0.0
|| scale <= 0.0
{
return center_img;
}
let half_view_w = widget_w / (2.0 * scale);
let half_view_h = widget_h / (2.0 * scale);
let center_x = if image_w * scale <= widget_w {
image_w * 0.5
} else {
center_img.x.clamp(half_view_w, image_w - half_view_w)
};
let center_y = if image_h * scale <= widget_h {
image_h * 0.5
} else {
center_img.y.clamp(half_view_h, image_h - half_view_h)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/quickview-core/src/geometry.rs` around lines 207 - 223, The
clamp_center function currently only guards values with <= 0.0, which misses
NaN/Inf; update clamp_center to first verify all float inputs are finite
(widget_w, widget_h, image_w, image_h, scale, and center_img.x/center_img.y)
using is_finite() and return center_img early if any are not finite, before
performing arithmetic like half_view_w = widget_w / (2.0 * scale) or using
clamp; ensure the same finite checks cover both the branch conditions (image_w *
scale <= widget_w / image_h * scale <= widget_h) and the final clamp calls so no
NaN/Inf can be used as bounds or propagated.

};

Point {
x: center_x,
y: center_y,
}
}
}

#[cfg(test)]
mod tests {
use super::{Point, ViewTransform};

fn approx_eq(a: f64, b: f64, eps: f64) {
assert!((a - b).abs() <= eps, "{a} != {b} (eps={eps})");
}

#[test]
fn image_and_widget_mapping_are_inverse() {
let t = ViewTransform::from_center(
1200.0,
800.0,
2400.0,
1600.0,
2.25,
Point { x: 900.0, y: 600.0 },
);

let img = Point {
x: 1234.5,
y: 345.25,
};
let widget = t.image_to_widget(img);
let roundtrip = t.widget_to_image(widget);

approx_eq(roundtrip.x, img.x, 1e-9);
approx_eq(roundtrip.y, img.y, 1e-9);
}

#[test]
fn anchor_preserving_zoom_keeps_widget_anchor_fixed() {
let widget_w = 1000.0;
let widget_h = 700.0;
let image_w = 3000.0;
let image_h = 2000.0;

let center_start = Point {
x: 1300.0,
y: 900.0,
};
let zoom_start = 1.3;
let zoom_new = 2.1;
let anchor_widget = Point { x: 120.0, y: 520.0 };

let t_start = ViewTransform::from_center(
widget_w,
widget_h,
image_w,
image_h,
zoom_start,
center_start,
);
let anchor_img = t_start.widget_to_image(anchor_widget);

let t_new_unclamped = ViewTransform::from_center(
widget_w,
widget_h,
image_w,
image_h,
zoom_new,
center_start,
);
let contain = ViewTransform::contain(widget_w, widget_h, image_w, image_h);
let widget_center = contain.widget_center;
let center_new = Point {
x: anchor_img.x - (anchor_widget.x - widget_center.x) / t_new_unclamped.scale(),
y: anchor_img.y - (anchor_widget.y - widget_center.y) / t_new_unclamped.scale(),
};
let t_new =
ViewTransform::from_center(widget_w, widget_h, image_w, image_h, zoom_new, center_new);

let mapped_anchor = t_new.image_to_widget(anchor_img);
approx_eq(mapped_anchor.x, anchor_widget.x, 1e-9);
approx_eq(mapped_anchor.y, anchor_widget.y, 1e-9);
}

#[test]
fn clamp_center_forces_image_center_when_scaled_image_fits() {
let center =
ViewTransform::clamp_center(1000.0, 800.0, 300.0, 200.0, 2.0, Point { x: 0.0, y: 0.0 });
approx_eq(center.x, 150.0, 1e-9);
approx_eq(center.y, 100.0, 1e-9);
}

#[test]
fn clamp_center_limits_pan_when_scaled_image_exceeds_viewport() {
let center = ViewTransform::clamp_center(
1000.0,
700.0,
3000.0,
2000.0,
0.6,
Point {
x: -5000.0,
y: 5000.0,
},
);

// half_view_w = 1000 / (2 * 0.6) = 833.333...
// half_view_h = 700 / (2 * 0.6) = 583.333...
approx_eq(center.x, 833.3333333333334, 1e-9);
approx_eq(center.y, 1416.6666666666667, 1e-9);
}
}
Loading