From d379e513fd2f241f028e90be85a0d256c22ef567 Mon Sep 17 00:00:00 2001 From: jonasnobile Date: Mon, 25 May 2026 14:10:07 +0200 Subject: [PATCH 1/8] feat(files): add image, SVG, and font previews to the file viewer Extends the file viewer beyond text: - Images: PNG, JPEG, WebP, GIF, BMP, TIFF, ICO render via gpui::img() with ObjectFit::Contain. Raster decode errors surface through a fallback element rather than a silent blank pane. - SVG: pre-rasterized through cx.svg_renderer() (with the BGRA swap GPUI's built-in Image::from_bytes path skips for SVG, producing inverted colors). Source/Preview toggle exposes the raw XML with xml syntax highlighting, mirroring the markdown toggle. Input bytes capped before rasterization; intrinsic SVG dimensions capped at 64 megapixels via usvg pre-parse to refuse pathological viewBoxes. - Fonts: TTF and OTF parsed via ttf-parser; WOFF2 decompressed first via woff2-patched (the upstream woff2 0.3 fails to compile against current safer-bytes; WOFF v1 is rejected with a clear message). The font bytes are registered with the platform text system so the preview pane renders a pangram across multiple sizes in the actual font, plus a metadata table (family, full name, style, weight class, italic, glyph count, units per em, version). Wire-level additions: - ProjectFs gains read_file_bytes (raw bytes alongside read_file's UTF-8 text path). LocalProjectFs gains a resolve() helper that canonicalizes and rejects path-traversal, matching the server-side resolve_project_file. - New ActionRequest::ReadFileBytes returns base64-encoded content with a 20 MB server-side cap. The remote_action client adds a 32 MB response body ceiling and a separate 90 s timeout client for byte payloads so large images on slow links don't surface as generic transport errors. Lifecycle hardening (from internal review): - FreshnessOutcome carries new_mtime in the Err arm so a failing read isn't repeated every throttle tick. - apply_loaded_content reads mtime only when file_path is absolute, preventing remote tabs from accidentally picking up an unrelated local file's mtime via std::fs::metadata. - spawn_tab_load stamps each load with a generation counter; stale results from earlier loads for the same tab are dropped. - load_file commits modified_at only after a successful image decode and resets stale image_data on every error path. - Image branch in apply_freshness_reload always overwrites the source fields so a source=None reload (e.g. SVG with non-UTF-8 bytes) doesn't show stale XML in Source mode. - Theme rehighlight covers SVG tabs (they carry XML in highlighted_lines) while skipping raster and font tabs. Co-Authored-By: Claude Opus 4.7 --- Cargo.lock | 110 ++++ crates/okena-core/src/api.rs | 4 + crates/okena-core/src/remote_action.rs | 87 ++- crates/okena-files/Cargo.toml | 4 + crates/okena-files/src/file_viewer/loading.rs | 556 ++++++++++++++++-- crates/okena-files/src/file_viewer/mod.rs | 254 +++++++- crates/okena-files/src/file_viewer/render.rs | 215 ++++++- .../okena-files/src/file_viewer/selection.rs | 6 +- crates/okena-files/src/project_fs.rs | 51 +- src/action_dispatch.rs | 4 + src/workspace/actions/execute/files.rs | 48 ++ src/workspace/actions/execute/mod.rs | 3 + 12 files changed, 1237 insertions(+), 105 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c5e9d2c7..5315f68d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -105,6 +105,21 @@ dependencies = [ "backtrace", ] +[[package]] +name = "alloc-no-stdlib" +version = "2.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cc7bb162ec39d46ab1ca8c77bf72e890535becd1751bb45f64c597edb4c8c6b3" + +[[package]] +name = "alloc-stdlib" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94fb8275041c72129eb51b7d0322c29b8387a0386127718b096429201a5d6ece" +dependencies = [ + "alloc-no-stdlib", +] + [[package]] name = "allocator-api2" version = "0.2.21" @@ -759,6 +774,18 @@ dependencies = [ "core2", ] +[[package]] +name = "bitvec" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1bc2832c24239b0141d5674bb9174f9d68a8b5b3f2753311927c172ca46f7e9c" +dependencies = [ + "funty", + "radium", + "tap", + "wyz", +] + [[package]] name = "block" version = "0.1.6" @@ -805,6 +832,27 @@ dependencies = [ "piper", ] +[[package]] +name = "brotli" +version = "7.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cc97b8f16f944bba54f0433f07e30be199b6dc2bd25937444bbad560bcea29bd" +dependencies = [ + "alloc-no-stdlib", + "alloc-stdlib", + "brotli-decompressor", +] + +[[package]] +name = "brotli-decompressor" +version = "4.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a334ef7c9e23abf0ce748e8cd309037da93e606ad52eb372e4ce327a0dcfbdfd" +dependencies = [ + "alloc-no-stdlib", + "alloc-stdlib", +] + [[package]] name = "bstr" version = "1.12.1" @@ -2228,6 +2276,12 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "four-cc" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "795cbfc56d419a7ce47ccbb7504dd9a5b7c484c083c356e797de08bd988d9629" + [[package]] name = "freetype-sys" version = "0.20.1" @@ -2248,6 +2302,12 @@ dependencies = [ "libc", ] +[[package]] +name = "funty" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6d5a32815ae3f33302d95fdcb2ce17862f8c65363dcfd29360480ba1001fc9c" + [[package]] name = "futf" version = "0.1.5" @@ -5634,6 +5694,7 @@ dependencies = [ name = "okena-files" version = "0.1.0" dependencies = [ + "base64", "gpui", "gpui-component", "grep-matcher", @@ -5649,7 +5710,10 @@ dependencies = [ "serde_json", "syntect", "tempfile", + "ttf-parser", "two-face", + "usvg", + "woff2-patched", ] [[package]] @@ -6659,6 +6723,12 @@ version = "6.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" +[[package]] +name = "radium" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc33ff2d4973d518d823d61aa239014831e521c75da58e3df4840d3f47749d09" + [[package]] name = "rand" version = "0.8.5" @@ -7283,6 +7353,17 @@ version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9774ba4a74de5f7b1c1451ed6cd5285a32eddb5cccb8cc655a4e50009e06477f" +[[package]] +name = "safer-bytes" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9814c78d534f27a438fcb57091d6deed0634b60e4e500fee28fe5990adf5ea54" +dependencies = [ + "bytes", + "paste", + "thiserror 1.0.69", +] + [[package]] name = "same-file" version = "1.0.6" @@ -8161,6 +8242,12 @@ dependencies = [ "objc", ] +[[package]] +name = "tap" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" + [[package]] name = "tempfile" version = "3.27.0" @@ -10257,6 +10344,20 @@ dependencies = [ "wasmparser", ] +[[package]] +name = "woff2-patched" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb9e0d17aedee2afac15749407e107edce8485f57b7ad9fb929f0bbdf2c28f24" +dependencies = [ + "bitvec", + "brotli", + "bytes", + "four-cc", + "safer-bytes", + "thiserror 1.0.69", +] + [[package]] name = "workspace-hack" version = "0.1.0" @@ -10269,6 +10370,15 @@ version = "0.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9edde0db4769d2dc68579893f2306b26c6ecfbe0ef499b013d731b7b9247e0b9" +[[package]] +name = "wyz" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05f360fc0b24296329c78fda852a1e9ae82de9cf7b27dae4b7f62f118f77b9ed" +dependencies = [ + "tap", +] + [[package]] name = "x11" version = "2.21.0" diff --git a/crates/okena-core/src/api.rs b/crates/okena-core/src/api.rs index e33656ce..90eebfc2 100644 --- a/crates/okena-core/src/api.rs +++ b/crates/okena-core/src/api.rs @@ -451,6 +451,10 @@ pub enum ActionRequest { project_id: String, relative_path: String, }, + ReadFileBytes { + project_id: String, + relative_path: String, + }, FileSize { project_id: String, relative_path: String, diff --git a/crates/okena-core/src/remote_action.rs b/crates/okena-core/src/remote_action.rs index 3ab22369..fb0ee359 100644 --- a/crates/okena-core/src/remote_action.rs +++ b/crates/okena-core/src/remote_action.rs @@ -2,21 +2,56 @@ use crate::api::ActionRequest; -/// A shared blocking HTTP client (connection pooling across all calls). +/// Total request timeout for "fast" actions (terminal control, listings, +/// metadata). 10 s is generous for these; longer would mask real failures. +const FAST_TIMEOUT_SECS: u64 = 10; + +/// Total request timeout for byte-payload reads (ReadFileBytes). A 20 MB +/// image base64-encodes to ~27 MB; over a 5 Mbit/s link that's ~45 s on the +/// wire alone, which would time out the fast client with no useful signal. +const BYTES_TIMEOUT_SECS: u64 = 90; + +/// Hard ceiling on response body size accepted by the remote bridge. Cuts +/// off arbitrarily large or runaway responses before they're buffered into +/// memory (peak resident is ~4× the file size while the base64 + JSON + +/// decoded Vec all co-exist). Mirrors the server-side cap in +/// `src/workspace/actions/execute/files.rs`. +const MAX_RESPONSE_BYTES: u64 = 32 * 1024 * 1024; + +fn build_client(timeout_secs: u64) -> reqwest::blocking::Client { + #[allow( + clippy::expect_used, + reason = "reqwest client build only fails on TLS backend init — nothing recoverable at this call site" + )] + reqwest::blocking::Client::builder() + .timeout(std::time::Duration::from_secs(timeout_secs)) + .connect_timeout(std::time::Duration::from_secs(5)) + .build() + .expect("Failed to build HTTP client") +} + +/// Fast-action client (terminal control, listings, metadata). fn shared_client() -> &'static reqwest::blocking::Client { use std::sync::OnceLock; static CLIENT: OnceLock = OnceLock::new(); - CLIENT.get_or_init(|| { - #[allow( - clippy::expect_used, - reason = "reqwest client build only fails on TLS backend init — nothing recoverable at this call site" - )] - reqwest::blocking::Client::builder() - .timeout(std::time::Duration::from_secs(10)) - .connect_timeout(std::time::Duration::from_secs(5)) - .build() - .expect("Failed to build HTTP client") - }) + CLIENT.get_or_init(|| build_client(FAST_TIMEOUT_SECS)) +} + +/// Bytes-action client with a longer timeout, for ReadFileBytes specifically. +fn bytes_client() -> &'static reqwest::blocking::Client { + use std::sync::OnceLock; + static CLIENT: OnceLock = OnceLock::new(); + CLIENT.get_or_init(|| build_client(BYTES_TIMEOUT_SECS)) +} + +/// Pick the right client for the action. Bytes-bearing actions get the +/// larger timeout so a slow remote doesn't surface as a generic transport +/// error mid-download. +fn client_for(action: &ActionRequest) -> &'static reqwest::blocking::Client { + match action { + ActionRequest::ReadFileBytes { .. } => bytes_client(), + _ => shared_client(), + } } /// Post an action request to a remote server and return the JSON response body. @@ -27,7 +62,7 @@ pub fn post_action( action: ActionRequest, ) -> Result, String> { let url = format!("http://{}:{}/v1/actions", host, port); - let client = shared_client(); + let client = client_for(&action); let resp = client .post(&url) .bearer_auth(token) @@ -41,8 +76,30 @@ pub fn post_action( return Err(format!("Server returned {}: {}", status, body)); } - let body: serde_json::Value = - resp.json().map_err(|e| format!("Failed to parse response: {}", e))?; + // Cap how much body we buffer. A hostile / older / desync'd server can't + // make us swallow a multi-GB JSON+base64 stream into memory. Content-Length + // is only a hint; we still bound the actual read. + if let Some(len) = resp.content_length() + && len > MAX_RESPONSE_BYTES { + return Err(format!( + "Response too large ({:.1} MB). Max {} MB.", + len as f64 / 1024.0 / 1024.0, + MAX_RESPONSE_BYTES / 1024 / 1024 + )); + } + use std::io::Read as _; + let mut body_bytes = Vec::new(); + resp.take(MAX_RESPONSE_BYTES + 1) + .read_to_end(&mut body_bytes) + .map_err(|e| format!("Failed to read response: {}", e))?; + if body_bytes.len() as u64 > MAX_RESPONSE_BYTES { + return Err(format!( + "Response too large (>{} MB).", + MAX_RESPONSE_BYTES / 1024 / 1024 + )); + } + let body: serde_json::Value = serde_json::from_slice(&body_bytes) + .map_err(|e| format!("Failed to parse response: {}", e))?; if let Some(error) = body.get("error").and_then(|e| e.as_str()) { return Err(error.to_string()); diff --git a/crates/okena-files/Cargo.toml b/crates/okena-files/Cargo.toml index c5761ae7..d8c6a80d 100644 --- a/crates/okena-files/Cargo.toml +++ b/crates/okena-files/Cargo.toml @@ -23,6 +23,10 @@ nucleo-matcher = "0.3" serde = { version = "1", features = ["derive"] } serde_json = "1" log = "0.4" +base64 = "0.22" +usvg = { version = "0.45", default-features = false } +ttf-parser = "0.25" +woff2 = { package = "woff2-patched", version = "0.4" } [dev-dependencies] tempfile = "3" diff --git a/crates/okena-files/src/file_viewer/loading.rs b/crates/okena-files/src/file_viewer/loading.rs index 5cd03d1c..fc02e892 100644 --- a/crates/okena-files/src/file_viewer/loading.rs +++ b/crates/okena-files/src/file_viewer/loading.rs @@ -1,23 +1,85 @@ //! File loading and syntax highlighting for the file viewer. -use super::{FileViewerTab, MAX_FILE_SIZE, MAX_LINES}; +use super::{ + font_format_for_path, image_format_for_path, DecodedImage, FileViewerTab, FontData, + FontFormat, MAX_FILE_SIZE, MAX_IMAGE_FILE_SIZE, MAX_LINES, +}; use crate::syntax::{highlight_content, HighlightedLine}; +use gpui::{Image, ImageFormat, SvgRenderer}; use okena_markdown::MarkdownDocument; use std::path::{Path, PathBuf}; +use std::sync::Arc; use std::time::SystemTime; use syntect::parsing::SyntaxSet; -/// Result of a background freshness reload: the file changed on disk and was -/// re-read and re-highlighted off the UI thread. All the heavy work (stat, -/// read, syntax highlighting, markdown parse) has already happened; applying -/// this back on the UI thread is just a set of field assignments. +/// Max font file size (in source-on-disk bytes). Fonts are typically much +/// smaller than images; 20 MB is comfortably above any realistic OpenType / +/// WOFF2 file and stops us hammering ttf-parser / woff2 with multi-GB +/// inputs. +pub(super) const MAX_FONT_FILE_SIZE: u64 = 20 * 1024 * 1024; + +/// Content produced by the async loader. Text, image, and font tabs share +/// the same plumbing (apply_loaded_content / apply_freshness_reload) but +/// carry different payloads. +pub(super) enum LoadedContent { + Text(String), + Image { + decoded: DecodedImage, + /// For SVG, the raw XML so the user can toggle into source view. + /// `None` for raster formats. + source: Option, + }, + Font { + data: Arc, + /// OpenType bytes ready for `text_system.add_fonts`. After WOFF2 + /// decompression, this is the underlying TTF/OTF payload. + ttf_bytes: Arc>, + }, +} + +/// Result of a background freshness check. Carries enough info that the UI +/// thread can apply field assignments without doing any blocking I/O. +pub(super) enum FreshnessOutcome { + /// File unchanged (or couldn't be stat'd) — nothing to do. + Unchanged, + /// File changed and was successfully re-read. + Reloaded(FreshnessReload), + /// File changed but re-read/decode failed. `new_mtime` is the file's + /// current mtime so apply can pin it and avoid re-reading the bad file + /// on every throttle tick. + Failed { + message: String, + new_mtime: Option, + }, +} + +/// All the heavy work (stat, read, syntax highlighting, markdown parse) has +/// already happened on the background executor; applying this back on the +/// UI thread is just a set of field assignments. pub(super) struct FreshnessReload { - pub content: String, - pub highlighted_lines: Vec, - pub markdown_doc: Option, + pub kind: FreshnessKind, pub modified_at: Option, } +pub(super) enum FreshnessKind { + Text { + content: String, + highlighted_lines: Vec, + markdown_doc: Option, + }, + Image { + decoded: DecodedImage, + /// For SVG, the raw XML plus its pre-highlighted lines so the user + /// keeps their source view in sync across mtime reloads. `None` + /// for raster, or for SVG whose bytes failed UTF-8 decode. + source: Option<(String, Vec)>, + }, + Font { + data: Arc, + ttf_bytes: Arc>, + }, +} + /// Stat `path` and, if its mtime differs from `old_mtime`, read and /// re-highlight it. Returns `Ok(None)` when the file is unchanged (or can't be /// stat'd), `Ok(Some(..))` with the recomputed content when it changed, and @@ -31,45 +93,247 @@ pub(super) fn compute_freshness_reload( is_markdown: bool, syntax_set: &SyntaxSet, is_dark: bool, -) -> Result, String> { + svg_renderer: &SvgRenderer, +) -> FreshnessOutcome { let Some(old_mtime) = old_mtime else { - return Ok(None); + return FreshnessOutcome::Unchanged; }; let Ok(metadata) = std::fs::metadata(path) else { - return Ok(None); + return FreshnessOutcome::Unchanged; }; let Ok(new_mtime) = metadata.modified() else { - return Ok(None); + return FreshnessOutcome::Unchanged; }; if new_mtime == old_mtime { - return Ok(None); + return FreshnessOutcome::Unchanged; + } + // Inline helper so every Err branch carries new_mtime — the UI thread + // pins this so the throttle loop doesn't re-read the bad file every + // second. + let failed = |message: String| FreshnessOutcome::Failed { + message, + new_mtime: Some(new_mtime), + }; + if font_format_for_path(path).is_some() { + if metadata.len() > MAX_FONT_FILE_SIZE { + return failed(format!( + "Font too large ({:.1} MB). Maximum size is 20 MB.", + metadata.len() as f64 / 1024.0 / 1024.0 + )); + } + let bytes = match std::fs::read(path) { + Ok(b) => b, + Err(e) => return failed(format!("Cannot read file: {}", e)), + }; + let (data, ttf_bytes) = match build_font_content(path, bytes) { + Ok(LoadedContent::Font { data, ttf_bytes }) => (data, ttf_bytes), + Ok(_) => return failed("Internal error decoding font".to_string()), + Err(e) => return failed(e), + }; + return FreshnessOutcome::Reloaded(FreshnessReload { + kind: FreshnessKind::Font { data, ttf_bytes }, + modified_at: Some(new_mtime), + }); + } + if image_format_for_path(path).is_some() { + if metadata.len() > MAX_IMAGE_FILE_SIZE { + return failed(format!( + "Image too large ({:.1} MB). Maximum size is 20 MB.", + metadata.len() as f64 / 1024.0 / 1024.0 + )); + } + let bytes = match std::fs::read(path) { + Ok(b) => b, + Err(e) => return failed(format!("Cannot read file: {}", e)), + }; + // Defense in depth: TOCTOU could grow the file between stat and + // read. The build below has its own cost, so reject before paying it. + if bytes.len() as u64 > MAX_IMAGE_FILE_SIZE { + return failed(format!( + "Image too large ({:.1} MB). Maximum size is 20 MB.", + bytes.len() as f64 / 1024.0 / 1024.0 + )); + } + let (decoded, source) = match build_image_content(path, bytes, svg_renderer) { + Ok(LoadedContent::Image { decoded, source }) => (decoded, source), + Ok(_) => return failed("Internal error decoding image".to_string()), + Err(e) => return failed(e), + }; + let source = source.map(|content| { + let highlighted = + highlight_content(&content, path, syntax_set, MAX_LINES, is_dark); + (content, highlighted) + }); + return FreshnessOutcome::Reloaded(FreshnessReload { + kind: FreshnessKind::Image { decoded, source }, + modified_at: Some(new_mtime), + }); } if metadata.len() > MAX_FILE_SIZE { - return Err(format!( + return failed(format!( "File too large ({:.1} MB). Maximum size is 5 MB.", metadata.len() as f64 / 1024.0 / 1024.0 )); } - let content = std::fs::read_to_string(path).map_err(|e| { - // Distinguish binary files from other read errors, matching load_file. - if let Ok(bytes) = std::fs::read(path) - && bytes.iter().take(1024).any(|&b| b == 0) { - return "Cannot display binary file".to_string(); - } - format!("Cannot read file: {}", e) - })?; + let content = match std::fs::read_to_string(path) { + Ok(c) => c, + Err(e) => { + // Distinguish binary files from other read errors, matching load_file. + let message = if let Ok(bytes) = std::fs::read(path) + && bytes.iter().take(1024).any(|&b| b == 0) + { + "Cannot display binary file".to_string() + } else { + format!("Cannot read file: {}", e) + }; + return failed(message); + } + }; let highlighted_lines = highlight_content(&content, path, syntax_set, MAX_LINES, is_dark); let markdown_doc = if is_markdown { Some(MarkdownDocument::parse(&content)) } else { None }; - Ok(Some(FreshnessReload { - content, - highlighted_lines, - markdown_doc, + FreshnessOutcome::Reloaded(FreshnessReload { + kind: FreshnessKind::Text { + content, + highlighted_lines, + markdown_doc, + }, modified_at: Some(new_mtime), - })) + }) +} + +/// Decode raw image bytes into a `DecodedImage` based on file extension. +/// Used by both the initial async load and freshness reloads for image tabs. +/// +/// Megapixel budget for a single rasterized SVG. tiny-skia's `Pixmap::new` +/// allocates `width * height * 4` bytes (RGBA), and `SMOOTH_SVG_SCALE_FACTOR` +/// inside GPUI doubles that. A hostile or accidentally-huge `viewBox` would +/// otherwise let one preview commit hundreds of MB / many GB before the +/// allocator complains. 64 MP ≈ 256 MB at 1× scale (~1 GB at 2×) — big +/// enough for any real-world icon or illustration, small enough to refuse +/// pathological inputs. +const MAX_SVG_PIXELS: u64 = 64 * 1024 * 1024; + +/// SVGs are pre-rasterized via the supplied `SvgRenderer` (with the BGRA +/// channel swap GPUI's built-in decoder skips for SVG) and the raw XML is +/// returned as `source` so the user can flip to a highlighted source view. +/// Raster formats are wrapped as `Image::from_bytes` and lean on GPUI's +/// asset cache to decode lazily on the UI thread. +pub(super) fn build_image_content( + path: &Path, + bytes: Vec, + svg_renderer: &SvgRenderer, +) -> Result { + let format = image_format_for_path(path).ok_or_else(|| { + format!( + "Unsupported image extension: {}", + path.extension() + .and_then(|e| e.to_str()) + .unwrap_or("(none)") + ) + })?; + match format { + ImageFormat::Svg => { + // Pre-parse with usvg so we can refuse pathological dimensions + // before SvgRenderer tries to allocate the pixmap. usvg::Tree + // parsing is cheap relative to rasterization. + let tree = usvg::Tree::from_data(&bytes, &usvg::Options::default()) + .map_err(|e| format!("Cannot decode SVG: {}", e))?; + let svg_size = tree.size(); + let w = svg_size.width().ceil() as u64; + let h = svg_size.height().ceil() as u64; + let pixels = w.saturating_mul(h); + if pixels == 0 || pixels > MAX_SVG_PIXELS { + return Err(format!( + "SVG dimensions out of range ({}×{}). Max {} megapixels.", + w, h, MAX_SVG_PIXELS / 1024 / 1024 + )); + } + let rendered = svg_renderer + .render_single_frame(&bytes, 1.0, true) + .map_err(|e| format!("Cannot decode SVG: {}", e))?; + // SVG is XML — UTF-8 unless someone hand-saved it weird. If + // decoding fails we still surface the preview without source. + let source = String::from_utf8(bytes).ok(); + Ok(LoadedContent::Image { + decoded: DecodedImage::Rendered(rendered), + source, + }) + } + _ => Ok(LoadedContent::Image { + decoded: DecodedImage::Raster(Arc::new(Image::from_bytes(format, bytes))), + source: None, + }), + } +} + +/// Parse a font file and return the metadata + OpenType bytes ready for +/// GPUI's text-system registration. WOFF2 input is decompressed first; +/// WOFF1 is currently rejected with a user-visible error. +pub(super) fn build_font_content( + path: &Path, + bytes: Vec, +) -> Result { + let format = font_format_for_path(path).ok_or_else(|| { + format!( + "Unsupported font extension: {}", + path.extension().and_then(|e| e.to_str()).unwrap_or("(none)") + ) + })?; + let ttf_bytes: Vec = match format { + FontFormat::OpenType => bytes, + FontFormat::Woff2 => { + // woff2 0.3 takes `&mut impl bytes::Buf`. `&[u8]` already + // implements `Buf`, so a mutable reference to a borrowed slice + // is the cheapest reader. + let mut slice: &[u8] = &bytes; + woff2::convert_woff2_to_ttf(&mut slice) + .map_err(|e| format!("Cannot decompress WOFF2: {:?}", e))? + } + FontFormat::Woff => { + return Err( + "WOFF preview is not supported yet — only WOFF2, OTF, and TTF are." + .to_string(), + ); + } + }; + let face = ttf_parser::Face::parse(&ttf_bytes, 0) + .map_err(|e| format!("Cannot parse font: {}", e))?; + let read_name = |name_id: u16| -> Option { + face.names() + .into_iter() + .find(|n| n.name_id == name_id && n.to_string().is_some()) + .and_then(|n| n.to_string()) + }; + let family_name = read_name(ttf_parser::name_id::FAMILY) + .unwrap_or_else(|| { + path.file_stem() + .and_then(|s| s.to_str()) + .unwrap_or("Unknown") + .to_string() + }); + let full_name = read_name(ttf_parser::name_id::FULL_NAME) + .unwrap_or_else(|| family_name.clone()); + let style = read_name(ttf_parser::name_id::SUBFAMILY) + .unwrap_or_else(|| if face.is_italic() { "Italic" } else { "Regular" }.to_string()); + let version = read_name(ttf_parser::name_id::VERSION).unwrap_or_default(); + let data = Arc::new(FontData { + family_name, + full_name, + style, + version, + num_glyphs: face.number_of_glyphs(), + units_per_em: face.units_per_em(), + weight_class: face.weight().to_number(), + is_italic: face.is_italic(), + }); + Ok(LoadedContent::Font { + data, + ttf_bytes: Arc::new(ttf_bytes), + }) } impl FileViewerTab { @@ -85,25 +349,121 @@ impl FileViewerTab { } /// Load file content and apply syntax highlighting. - pub(super) fn load_file(&mut self, path: &PathBuf, syntax_set: &SyntaxSet, is_dark: bool) { + pub(super) fn load_file( + &mut self, + path: &PathBuf, + syntax_set: &SyntaxSet, + is_dark: bool, + svg_renderer: &SvgRenderer, + ) { // Check file size first - match std::fs::metadata(path) { - Ok(metadata) => { - if metadata.len() > MAX_FILE_SIZE { - self.error_message = Some(format!( - "File too large ({:.1} MB). Maximum size is 5 MB.", - metadata.len() as f64 / 1024.0 / 1024.0 - )); - return; - } - self.modified_at = metadata.modified().ok(); - } + let metadata = match std::fs::metadata(path) { + Ok(m) => m, Err(e) => { self.error_message = Some(format!("Cannot read file: {}", e)); return; } + }; + + if self.is_image { + if metadata.len() > MAX_IMAGE_FILE_SIZE { + self.error_message = Some(format!( + "Image too large ({:.1} MB). Maximum size is 20 MB.", + metadata.len() as f64 / 1024.0 / 1024.0 + )); + self.image_data = None; + return; + } + match std::fs::read(path) { + Ok(bytes) => match build_image_content(path, bytes, svg_renderer) { + Ok(LoadedContent::Image { decoded, source }) => { + self.image_data = Some(decoded); + if let Some(content) = source { + self.content = content; + self.do_highlight_content(path, syntax_set, is_dark); + } else { + self.content.clear(); + self.highlighted_lines.clear(); + self.line_count = 0; + self.line_num_width = 3; + } + // Only commit the new mtime after a successful + // decode. If decode fails we leave modified_at at + // its previous value so a subsequent retry (e.g. + // file fixed in place without mtime change) still + // re-runs through reload_if_changed. + self.modified_at = metadata.modified().ok(); + } + Ok(_) => { + // Unreachable: build_image_content only returns Image. + self.error_message = Some("Internal error decoding image".to_string()); + self.image_data = None; + } + Err(e) => { + self.error_message = Some(e); + self.image_data = None; + } + }, + Err(e) => { + self.error_message = Some(format!("Cannot read file: {}", e)); + self.image_data = None; + } + } + return; + } + + if self.is_font { + if metadata.len() > MAX_FONT_FILE_SIZE { + self.error_message = Some(format!( + "Font too large ({:.1} MB). Maximum size is 20 MB.", + metadata.len() as f64 / 1024.0 / 1024.0 + )); + self.font_data = None; + return; + } + match std::fs::read(path) { + Ok(bytes) => match build_font_content(path, bytes) { + Ok(LoadedContent::Font { data, .. }) => { + self.font_data = Some(data); + self.image_data = None; + self.content.clear(); + self.highlighted_lines.clear(); + self.line_count = 0; + self.line_num_width = 3; + // Note: bytes are not re-registered with the text + // system here. Sync reload via reload_if_changed + // updates the displayed metadata; the sample text + // keeps rendering with the originally-registered + // font (same family name resolves), which is + // acceptable until the user reopens the tab. + self.modified_at = metadata.modified().ok(); + } + Ok(_) => { + self.error_message = Some("Internal error decoding font".to_string()); + self.font_data = None; + } + Err(e) => { + self.error_message = Some(e); + self.font_data = None; + } + }, + Err(e) => { + self.error_message = Some(format!("Cannot read file: {}", e)); + self.font_data = None; + } + } + return; } + if metadata.len() > MAX_FILE_SIZE { + self.error_message = Some(format!( + "File too large ({:.1} MB). Maximum size is 5 MB.", + metadata.len() as f64 / 1024.0 / 1024.0 + )); + return; + } + self.modified_at = metadata.modified().ok(); + // Read file content match std::fs::read_to_string(path) { Ok(content) => { @@ -135,22 +495,47 @@ impl FileViewerTab { /// Apply content that was loaded asynchronously in the background. pub(super) fn apply_loaded_content( &mut self, - result: Result, + result: Result, syntax_set: &SyntaxSet, is_dark: bool, ) { self.loading = false; match result { - Ok(content) => { + Ok(LoadedContent::Text(content)) => { self.content = content; self.do_highlight_content(&self.file_path.clone(), syntax_set, is_dark); if self.is_markdown { self.markdown_doc = Some(MarkdownDocument::parse(&self.content)); } - // Try to get mtime for local files; harmlessly fails for remote files. - self.modified_at = std::fs::metadata(&self.file_path) - .ok() - .and_then(|m| m.modified().ok()); + self.modified_at = self.local_mtime(); + } + Ok(LoadedContent::Image { decoded, source }) => { + self.image_data = Some(decoded); + self.font_data = None; + if let Some(content) = source { + self.content = content; + self.do_highlight_content(&self.file_path.clone(), syntax_set, is_dark); + } else { + // Raster image or SVG with non-UTF-8 bytes — make sure + // we don't keep a stale source view alive from a + // previously-loaded text/SVG tab. + self.content.clear(); + self.highlighted_lines.clear(); + self.line_count = 0; + self.line_num_width = 3; + } + self.modified_at = self.local_mtime(); + } + Ok(LoadedContent::Font { data, .. }) => { + self.font_data = Some(data); + self.image_data = None; + // Font tabs have no source view; clear text fields so a + // previously-loaded text/SVG doesn't leak through. + self.content.clear(); + self.highlighted_lines.clear(); + self.line_count = 0; + self.line_num_width = 3; + self.modified_at = self.local_mtime(); } Err(e) => { self.error_message = Some(e); @@ -158,23 +543,77 @@ impl FileViewerTab { } } - /// Apply the result of a background freshness reload computed by + /// Read mtime for the active file, but only when `file_path` looks like + /// an absolute on-disk path (local project). For remote projects the + /// loader synthesises a relative path placeholder; stat'ing that on the + /// UI host either fails or — worse — coincidentally matches an + /// unrelated local file. Returning None there keeps the freshness loop + /// dormant rather than poisoned. + fn local_mtime(&self) -> Option { + if !self.file_path.is_absolute() { + return None; + } + std::fs::metadata(&self.file_path) + .ok() + .and_then(|m| m.modified().ok()) + } + + /// Apply the result of a background freshness check computed by /// `compute_freshness_reload`. All heavy work (stat/read/highlight) already /// happened off-thread; this is just field assignment on the UI thread. - pub(super) fn apply_freshness_reload(&mut self, reload: Result, String>) { - match reload { - Ok(Some(reload)) => { + pub(super) fn apply_freshness_reload(&mut self, outcome: FreshnessOutcome) { + match outcome { + FreshnessOutcome::Reloaded(reload) => { self.error_message = None; - self.content = reload.content; - self.line_count = reload.highlighted_lines.len(); - self.line_num_width = self.line_count.to_string().len().max(3); - self.highlighted_lines = reload.highlighted_lines; - self.markdown_doc = reload.markdown_doc; + match reload.kind { + FreshnessKind::Text { content, highlighted_lines, markdown_doc } => { + self.content = content; + self.line_count = highlighted_lines.len(); + self.line_num_width = self.line_count.to_string().len().max(3); + self.highlighted_lines = highlighted_lines; + self.markdown_doc = markdown_doc; + // A text reload replaces a previous image preview; + // drop the old bitmap so we don't keep it around. + self.image_data = None; + } + FreshnessKind::Image { decoded, source } => { + self.image_data = Some(decoded); + self.font_data = None; + // Always overwrite the source-view fields so a + // source=None reload doesn't leave stale XML from + // the previous revision visible in Source mode. + if let Some((content, highlighted)) = source { + self.content = content; + self.line_count = highlighted.len(); + self.line_num_width = self.line_count.to_string().len().max(3); + self.highlighted_lines = highlighted; + } else { + self.content.clear(); + self.highlighted_lines.clear(); + self.line_count = 0; + self.line_num_width = 3; + } + } + FreshnessKind::Font { data, .. } => { + self.font_data = Some(data); + self.image_data = None; + self.content.clear(); + self.highlighted_lines.clear(); + self.line_count = 0; + self.line_num_width = 3; + } + } self.modified_at = reload.modified_at; } - Ok(None) => {} - Err(e) => { - self.error_message = Some(e); + FreshnessOutcome::Unchanged => {} + FreshnessOutcome::Failed { message, new_mtime } => { + self.error_message = Some(message); + // Pin the mtime so we don't re-attempt the same expensive + // read on every throttle tick. The next change to the file + // bumps the mtime again and re-triggers a real reload. + if let Some(mtime) = new_mtime { + self.modified_at = Some(mtime); + } } } } @@ -185,6 +624,7 @@ impl FileViewerTab { &mut self, syntax_set: &SyntaxSet, is_dark: bool, + svg_renderer: &SvgRenderer, ) -> bool { let Some(old_mtime) = self.modified_at else { return false; @@ -200,7 +640,7 @@ impl FileViewerTab { } let path = self.file_path.clone(); self.error_message = None; - self.load_file(&path, syntax_set, is_dark); + self.load_file(&path, syntax_set, is_dark, svg_renderer); true } diff --git a/crates/okena-files/src/file_viewer/mod.rs b/crates/okena-files/src/file_viewer/mod.rs index 223dbe7f..241d6592 100644 --- a/crates/okena-files/src/file_viewer/mod.rs +++ b/crates/okena-files/src/file_viewer/mod.rs @@ -20,13 +20,18 @@ use context_menu::{DeleteConfirmState, FileRenameState, FileTreeContextMenu, Tab use gpui::*; use okena_markdown::{MarkdownDocument, MarkdownSelection}; use std::collections::{HashMap, HashSet}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; +use std::sync::Arc; use std::time::SystemTime; use syntect::parsing::SyntaxSet; -/// Maximum file size to load (5MB) +/// Maximum file size to load for text/markdown (5MB) const MAX_FILE_SIZE: u64 = 5 * 1024 * 1024; +/// Maximum file size to load for image previews (20MB — image decoders +/// handle larger files comfortably than the text/syntect path). +const MAX_IMAGE_FILE_SIZE: u64 = 20 * 1024 * 1024; + /// Maximum number of lines to display const MAX_LINES: usize = 10000; @@ -86,6 +91,53 @@ pub(super) struct FileViewerTab { /// Per-line git blame for this file. Lazy-loaded when the user toggles /// the blame gutter on. pub blame: BlameLoadState, + /// Monotonic counter bumped each time `spawn_tab_load` schedules a fresh + /// async load for this tab. The bg task captures the generation it was + /// scheduled at and `apply_loaded_content` is skipped if a newer load + /// has been queued in the meantime, so a slow earlier load can't + /// clobber a faster later one with stale content. + pub load_generation: u64, + /// True for files previewed as images (png/jpg/gif/webp/svg/...). When + /// set, `image_data` carries the bytes and the source/markdown rendering + /// paths are bypassed. + pub is_image: bool, + /// True for SVG files specifically. SVG is the one image format that + /// also has a meaningful source view — the loader keeps the raw XML in + /// `content`/`highlighted_lines` so the user can flip between Preview + /// (rendered) and Source (highlighted XML) via the same toggle markdown + /// uses. + pub is_svg: bool, + /// Decoded image data wrapped for GPUI's `img()` element. Populated by + /// the async loader for image tabs. + pub image_data: Option, + /// True for font files (otf/ttf/woff/woff2). When set, `font_data` + /// carries parsed metadata and the source/image rendering paths are + /// bypassed in favour of the font-preview branch. + pub is_font: bool, + /// Parsed font metadata + family name. The TTF bytes themselves are + /// registered with GPUI's text system on apply (kept inside the system, + /// not on the tab) so the preview pane can render sample text in the + /// font. + pub font_data: Option>, +} + +/// Decoded image payload. Raster formats let GPUI's asset cache handle the +/// RGBA→BGRA swap; SVGs are pre-rasterized to BGRA ourselves because GPUI's +/// built-in `ImageDecoder` calls `render_single_frame(.., to_bgra=false)` for +/// SVG, leaving R/B swapped and the preview inverted. +#[derive(Clone)] +pub enum DecodedImage { + Raster(Arc), + Rendered(Arc), +} + +impl From for ImageSource { + fn from(value: DecodedImage) -> Self { + match value { + DecodedImage::Raster(image) => ImageSource::Image(image), + DecodedImage::Rendered(rendered) => ImageSource::Render(rendered), + } + } } /// Lifecycle of a tab's blame data. @@ -98,6 +150,72 @@ pub enum BlameLoadState { Error(BlameError), } +/// Map an extension to a `gpui::ImageFormat` for files we can preview as +/// images. Returns `None` for non-image extensions. +pub(super) fn image_format_for_path(path: &Path) -> Option { + let ext = path.extension()?.to_str()?.to_ascii_lowercase(); + Some(match ext.as_str() { + "png" => ImageFormat::Png, + "jpg" | "jpeg" => ImageFormat::Jpeg, + "gif" => ImageFormat::Gif, + "webp" => ImageFormat::Webp, + "bmp" => ImageFormat::Bmp, + "tif" | "tiff" => ImageFormat::Tiff, + "ico" => ImageFormat::Ico, + "svg" => ImageFormat::Svg, + _ => return None, + }) +} + +/// Font container format we know how to load. +#[derive(Clone, Copy, PartialEq, Eq)] +pub(super) enum FontFormat { + /// OpenType / TrueType — fed straight to ttf-parser and registered as-is. + OpenType, + /// WOFF2 — Brotli-compressed OpenType, decompressed back to TTF before + /// parsing and font-system registration. + Woff2, + /// WOFF (v1) — zlib-compressed per-table OpenType. We surface the file + /// but reject decoding for now (rare in practice; adding decoder is a + /// follow-up). + Woff, +} + +/// Detect whether `path`'s extension is one of the font formats we preview. +pub(super) fn font_format_for_path(path: &Path) -> Option { + let ext = path.extension()?.to_str()?.to_ascii_lowercase(); + Some(match ext.as_str() { + "ttf" | "otf" => FontFormat::OpenType, + "woff2" => FontFormat::Woff2, + "woff" => FontFormat::Woff, + _ => return None, + }) +} + +/// Parsed metadata for a font preview, plus the OpenType bytes that GPUI's +/// text system needs to actually render text in the font. Wrapped in `Arc` +/// on the tab so freshness reloads can swap it cheaply. +#[derive(Clone)] +pub struct FontData { + /// Family name from the OpenType `name` table (e.g. "JetBrains Mono"), + /// used as the `Font::family` for sample-text rendering. + pub family_name: String, + /// Full font name from the `name` table (e.g. "JetBrains Mono Bold Italic"). + pub full_name: String, + /// Subfamily / style description ("Regular", "Bold", "Italic", ...). + pub style: String, + /// Font version string from the `name` table. + pub version: String, + /// Number of glyphs in the font. + pub num_glyphs: u16, + /// EM square units (typically 1000 for OTF, 2048 for TTF). + pub units_per_em: u16, + /// OS/2 `usWeightClass` — 100 (Thin) through 900 (Black). + pub weight_class: u16, + /// Whether the font advertises italic style. + pub is_italic: bool, +} + impl FileViewerTab { /// Create a new tab for browsing (no file loaded). pub(super) fn new_empty() -> Self { @@ -122,12 +240,22 @@ impl FileViewerTab { modified_at: None, loading: false, blame: BlameLoadState::NotLoaded, + load_generation: 0, + is_image: false, + is_svg: false, + image_data: None, + is_font: false, + font_data: None, } } /// Create a tab in loading state (content will be filled asynchronously). fn new_loading(relative_path: String, file_path: PathBuf) -> Self { - let is_markdown = Self::is_markdown_file(&file_path); + let image_format = image_format_for_path(&file_path); + let is_image = image_format.is_some(); + let is_svg = image_format == Some(ImageFormat::Svg); + let is_font = !is_image && font_format_for_path(&file_path).is_some(); + let is_markdown = !is_image && !is_font && Self::is_markdown_file(&file_path); Self { file_path, relative_path, @@ -137,7 +265,7 @@ impl FileViewerTab { line_num_width: 3, error_message: None, selection: Selection::default(), - display_mode: if is_markdown { + display_mode: if is_markdown || is_svg { DisplayMode::Preview } else { DisplayMode::Source @@ -153,6 +281,12 @@ impl FileViewerTab { modified_at: None, loading: true, blame: BlameLoadState::NotLoaded, + load_generation: 0, + is_image, + is_svg, + image_data: None, + is_font, + font_data: None, } } @@ -302,6 +436,11 @@ pub struct FileViewer { pub(super) blame_visible: bool, /// Right-click context menu over a non-empty text selection. pub(super) selection_context_menu: Option>, + /// Monotonic counter used to stamp each `spawn_tab_load` invocation. + /// Each background task captures the value it was scheduled at and only + /// applies its result if the tab's recorded generation still matches, + /// so a slow earlier load can't overwrite a faster later one. + next_load_generation: u64, } impl FileViewer { @@ -366,6 +505,7 @@ impl FileViewer { blame_provider, blame_visible, selection_context_menu: None, + next_load_generation: 0, }; // Kick off the root directory listing and any expanded ancestors so @@ -421,6 +561,7 @@ impl FileViewer { blame_provider, blame_visible, selection_context_menu: None, + next_load_generation: 0, }; viewer.fetch_initial_dirs(cx); viewer @@ -455,17 +596,20 @@ impl FileViewer { // Re-fetch directory listings so the sidebar reflects added/removed files self.refresh_file_tree_async(cx); + let svg_renderer = cx.svg_renderer(); for tab in &mut self.tabs { if tab.is_empty() { continue; } // Reload externally modified files (also re-highlights) - if tab.reload_if_changed(&self.syntax_set, self.is_dark) { + if tab.reload_if_changed(&self.syntax_set, self.is_dark, &svg_renderer) { tab.blame = BlameLoadState::NotLoaded; continue; } - // Theme changed — re-highlight without reloading - if rehighlight { + // Theme changed — re-highlight without reloading. Raster image + // and font tabs have no highlighted content; SVG tabs do (the + // source-view XML), so they need the rehighlight too. + if rehighlight && !tab.is_font && (!tab.is_image || tab.is_svg) { tab.do_highlight_content( &tab.file_path.clone(), &self.syntax_set, @@ -600,6 +744,7 @@ impl FileViewer { let is_markdown = tab.is_markdown; let syntax_set = self.syntax_set.clone(); let is_dark = self.is_dark; + let svg_renderer = cx.svg_renderer(); self.freshness_check_in_flight = true; cx.spawn(async move |entity: WeakEntity, cx| { @@ -612,12 +757,21 @@ impl FileViewer { is_markdown, &syntax_set, is_dark, + &svg_renderer, ) }) .await; let _ = entity.update(cx, |this, cx| { this.freshness_check_in_flight = false; - let reloaded = matches!(result, Ok(Some(_))); + let reloaded = matches!(result, loading::FreshnessOutcome::Reloaded(_)); + // Register fresh font bytes with the platform text system + // before apply, mirroring spawn_tab_load. add_fonts is + // idempotent so a no-change font reload is a cheap no-op. + if let loading::FreshnessOutcome::Reloaded(reload) = &result + && let loading::FreshnessKind::Font { ttf_bytes, .. } = &reload.kind + { + register_font_bytes(cx, ttf_bytes); + } // Re-find the tab by relative_path; concurrent reorders/closes // mean the index may have shifted since we scheduled the check. if let Some(tab) = @@ -820,28 +974,81 @@ impl FileViewer { /// Spawn a background task to load file content for a tab. The tab is /// identified by `relative_path` so concurrent reorders don't bind us to - /// a stale index. - fn spawn_tab_load(&self, relative_path: String, cx: &mut Context) { + /// a stale index, AND by a per-load generation token so a slow earlier + /// load can't overwrite a faster later one for the same path. + fn spawn_tab_load(&mut self, relative_path: String, cx: &mut Context) { + self.next_load_generation = self.next_load_generation.wrapping_add(1); + let generation = self.next_load_generation; + if let Some(tab) = self.tabs.iter_mut().find(|t| t.relative_path == relative_path) + { + tab.load_generation = generation; + } let fs = self.project_fs.clone(); let rel = relative_path.clone(); + // Image / font detection is driven purely by extension, so we can + // decide the load strategy off-thread without holding the tab borrow. + let asset_path = PathBuf::from(&relative_path); + let is_image = image_format_for_path(&asset_path).is_some(); + let is_font = !is_image && font_format_for_path(&asset_path).is_some(); + let svg_renderer = cx.svg_renderer(); cx.spawn(async move |entity: WeakEntity, cx| { - let result: Result = cx + let result: Result = cx .background_executor() .spawn(async move { - let size = fs.file_size(&rel)?; - if size > MAX_FILE_SIZE { - return Err(format!( - "File too large ({:.1} MB). Maximum size is 5 MB.", - size as f64 / 1024.0 / 1024.0 - )); + if is_image { + // Don't do a separate file_size round-trip — the + // server enforces MAX_IMAGE_FILE_SIZE inside + // read_file_bytes (TOCTOU close), and for local + // projects build_image_content rejects oversize + // bytes too. + let bytes = fs.read_file_bytes(&rel)?; + if bytes.len() as u64 > MAX_IMAGE_FILE_SIZE { + return Err(format!( + "Image too large ({:.1} MB). Maximum size is 20 MB.", + bytes.len() as f64 / 1024.0 / 1024.0 + )); + } + loading::build_image_content(&asset_path, bytes, &svg_renderer) + } else if is_font { + let bytes = fs.read_file_bytes(&rel)?; + if bytes.len() as u64 > loading::MAX_FONT_FILE_SIZE { + return Err(format!( + "Font too large ({:.1} MB). Maximum size is 20 MB.", + bytes.len() as f64 / 1024.0 / 1024.0 + )); + } + loading::build_font_content(&asset_path, bytes) + } else { + let size = fs.file_size(&rel)?; + if size > MAX_FILE_SIZE { + return Err(format!( + "File too large ({:.1} MB). Maximum size is 5 MB.", + size as f64 / 1024.0 / 1024.0 + )); + } + fs.read_file(&rel).map(loading::LoadedContent::Text) } - fs.read_file(&rel) }) .await; let _ = entity.update(cx, |this, cx| { if let Some(tab) = this.tabs.iter_mut().find(|t| t.relative_path == relative_path) { + // Drop stale results: a newer spawn_tab_load has been + // queued for this tab (closed-and-reopened, navigated + // away and back, etc.) and its result is what the user + // is waiting for. + if tab.load_generation != generation { + return; + } + // Register font bytes with the platform text system + // BEFORE applying, so the family name is resolvable by + // the time render runs. add_fonts is idempotent on + // re-registration (same internal name resolves), so + // closing and reopening the same font is cheap. + if let Ok(loading::LoadedContent::Font { ttf_bytes, .. }) = &result { + register_font_bytes(cx, ttf_bytes); + } tab.apply_loaded_content(result, &this.syntax_set, this.is_dark); tab.blame = BlameLoadState::NotLoaded; cx.notify(); @@ -887,6 +1094,17 @@ pub enum FileViewerEvent { SendToTerminal(okena_core::send_payload::SendPayload), } +/// Register a font's OpenType bytes with the active text system so any +/// subsequent render of `Font { family: family_name, .. }` resolves to it. +/// GPUI's `add_fonts` is idempotent on a re-registration (same internal +/// name resolves), so this is safe to call on every freshness reload. +fn register_font_bytes(cx: &mut App, ttf_bytes: &Arc>) { + let bytes: Vec = ttf_bytes.as_ref().clone(); + if let Err(e) = cx.text_system().add_fonts(vec![std::borrow::Cow::Owned(bytes)]) { + log::warn!("Failed to register font with text system: {}", e); + } +} + impl EventEmitter for FileViewer {} impl okena_ui::overlay::CloseEvent for FileViewerEvent { diff --git a/crates/okena-files/src/file_viewer/render.rs b/crates/okena-files/src/file_viewer/render.rs index f741bb86..cd77a39c 100644 --- a/crates/okena-files/src/file_viewer/render.rs +++ b/crates/okena-files/src/file_viewer/render.rs @@ -24,7 +24,7 @@ use okena_ui::tokens::{ui_text, ui_text_md, ui_text_ms, ui_text_sm, ui_text_xl}; use std::sync::Arc; use super::context_menu::TreeNodeTarget; -use super::{DisplayMode, FileViewer, SIDEBAR_WIDTH}; +use super::{DisplayMode, FileViewer, FontData, SIDEBAR_WIDTH}; /// Helper to create rgba from u32 color and alpha. fn rgba(color: u32, alpha: f32) -> Rgba { @@ -786,8 +786,26 @@ impl Render for FileViewer { let has_error = tab.error_message.is_some(); let error_message = tab.error_message.clone(); let is_markdown = tab.is_markdown; + let is_image = tab.is_image; + let is_svg = tab.is_svg; + let is_font = tab.is_font; + let image_data = tab.image_data.clone(); + let font_data = tab.font_data.clone(); let display_mode = tab.display_mode; let is_preview_mode = display_mode == DisplayMode::Preview; + // Body view selectors. Each tab renders exactly one of these branches: + // * show_image — raster image, or SVG in Preview mode + // * show_font — font preview (sample text + metadata) + // * show_md_preview — markdown rendered preview + // * show_source — text source / markdown source / SVG XML source + // Markdown / image / font are mutually exclusive (the loader picks + // one), so these four are also mutually exclusive in practice. + let show_image = is_image && (!is_svg || is_preview_mode); + let show_font = is_font; + let show_md_preview = is_markdown && is_preview_mode; + let show_source = !show_image && !show_font && !show_md_preview; + // Whether the header should expose the Preview/Source toggle. + let supports_view_toggle = is_markdown || is_svg; let sidebar_visible = self.sidebar_visible; let show_tabs = self.tabs.len() > 1; @@ -871,7 +889,7 @@ impl Render for FileViewer { outer .track_focus(&focus_handle) .key_context("FileViewer") - .when(!is_preview_mode, |d| d.cursor(CursorStyle::IBeam)) + .when(show_source, |d| d.cursor(CursorStyle::IBeam)) .on_action(cx.listener(|this, _: &Cancel, window, cx| { // Dismiss overlays in priority order before default close behavior if this.selection_context_menu.is_some() { @@ -925,14 +943,21 @@ impl Render for FileViewer { let tab = this.active_tab(); let is_preview = tab.display_mode == DisplayMode::Preview; let is_md = tab.is_markdown; + let is_img = tab.is_image; + let is_svg_tab = tab.is_svg; + let is_font_tab = tab.is_font; + // SVG in preview mode behaves like a raster image (no source). + // In source mode the highlighted XML is shown, so selection + // / search / copy / select-all all work normally. + let is_img_view = is_img && (!is_svg_tab || is_preview); match key { "f" if modifiers.platform || modifiers.control => { - if !is_preview { + if !is_preview && !is_img_view && !is_font_tab { this.open_search(window, cx); } } - "tab" if is_md && !modifiers.control && !modifiers.shift => { + "tab" if (is_md || is_svg_tab) && !modifiers.control && !modifiers.shift => { this.toggle_display_mode(cx); } "tab" if modifiers.control && modifiers.shift => { @@ -952,14 +977,18 @@ impl Render for FileViewer { this.toggle_sidebar(cx); } "c" if modifiers.platform || modifiers.control => { - if is_preview { + if is_img_view || is_font_tab { + // No text selection while previewing an image or font. + } else if is_preview { this.copy_markdown_selection(cx); } else { this.copy_selection(cx); } } "a" if modifiers.platform || modifiers.control => { - if is_preview { + if is_img_view || is_font_tab { + // Nothing to select. + } else if is_preview { this.select_all_markdown(cx); } else { this.select_all(cx); @@ -1085,7 +1114,7 @@ impl Render for FileViewer { .child( h_flex() .gap(px(12.0)) - .when(self.blame_provider.is_some(), |d| { + .when(self.blame_provider.is_some() && !is_image && !is_font, |d| { let on = self.blame_visible; d.child( div() @@ -1116,7 +1145,7 @@ impl Render for FileViewer { ), ) }) - .when(is_markdown, |d| { + .when(supports_view_toggle, |d| { d.child( div() .id("display-mode-toggle") @@ -1230,7 +1259,80 @@ impl Render for FileViewer { ), ) }) - .when(!tab_loading && !has_error && !is_preview_mode, |d| { + .when(!tab_loading && !has_error && show_image, |d| { + let body = div() + .id("file-image") + .flex_1() + .min_h_0() + .flex() + .items_center() + .justify_center() + .overflow_hidden() + .bg(rgb(t.bg_secondary)) + .p(px(16.0)); + let muted = t.text_muted; + let body = match image_data.clone() { + Some(image) => body.child( + img(image) + .object_fit(ObjectFit::Contain) + .max_w_full() + .max_h_full() + // Without this, GPUI silently + // renders an empty box when the + // raster decoder rejects the + // bytes (corrupt, truncated, + // 0-byte, or mis-extensioned + // file). SVG goes through a + // separate pre-rasterizer that + // surfaces errors via error_message; + // raster decodes happen lazily + // inside GPUI's asset cache. + .with_fallback(move || { + div() + .flex() + .items_center() + .justify_center() + .text_size(px(14.0)) + .text_color(rgb(muted)) + .child("Cannot decode image") + .into_any_element() + }), + ), + None => body.child( + div() + .text_size(ui_text_sm(cx)) + .text_color(rgb(t.text_muted)) + .child("Image not available"), + ), + }; + d.child(body) + }) + .when(!tab_loading && !has_error && show_font, |d| { + d.child(render_font_preview(font_data.clone(), &t, cx)) + }) + .when(!tab_loading && !has_error && show_source, |d| { + // SVG in Source mode with no decoded XML (loader + // got non-UTF-8 bytes) would render as a blank + // pane with zero lines — surface a placeholder + // instead of a silent empty box. + if is_svg && line_count == 0 { + return d.child( + div() + .flex_1() + .flex() + .items_center() + .justify_center() + .bg(rgb(t.bg_secondary)) + .child( + div() + .text_size(ui_text_sm(cx)) + .text_color(rgb(t.text_muted)) + .child( + "SVG source is not valid UTF-8 — switch back to Preview.", + ), + ), + ); + } let tc = theme_colors.clone(); let view_clone = view.clone(); d.child( @@ -1271,7 +1373,7 @@ impl Render for FileViewer { ), ) }) - .when(!tab_loading && !has_error && is_preview_mode, |d| { + .when(!tab_loading && !has_error && show_md_preview, |d| { let Some(list_state) = markdown_list_state.clone() else { return d; }; @@ -1763,3 +1865,96 @@ impl FileViewer { ) } } + +/// Render the font-preview body: a pangram in descending sizes followed by +/// a metadata table. `data` is None while the loader is still running or +/// if the font failed to parse (caller already gates on error state, so +/// None here means the font Arc was somehow dropped — render a placeholder). +fn render_font_preview( + data: Option>, + t: &ThemeColors, + cx: &mut Context, +) -> impl IntoElement { + let Some(data) = data else { + return div() + .id("font-empty") + .flex_1() + .flex() + .items_center() + .justify_center() + .bg(rgb(t.bg_secondary)) + .child( + div() + .text_size(ui_text_sm(cx)) + .text_color(rgb(t.text_muted)) + .child("Font not available"), + ) + .into_any_element(); + }; + + let family: SharedString = data.family_name.clone().into(); + let sample = "The quick brown fox jumps over the lazy dog"; + let sample_sizes_px: [f32; 5] = [48.0, 32.0, 24.0, 18.0, 14.0]; + + let sample_block = v_flex() + .gap(px(20.0)) + .children(sample_sizes_px.iter().map(|&size| { + div() + .text_size(px(size)) + .text_color(rgb(t.text_primary)) + .font_family(family.clone()) + .child(sample.to_string()) + })); + + let metadata_rows: Vec<(&str, String)> = vec![ + ("Family", data.family_name.clone()), + ("Full name", data.full_name.clone()), + ("Style", data.style.clone()), + ("Weight class", data.weight_class.to_string()), + ("Italic", if data.is_italic { "yes" } else { "no" }.to_string()), + ("Glyphs", data.num_glyphs.to_string()), + ("Units per em", data.units_per_em.to_string()), + ("Version", if data.version.is_empty() { "—".to_string() } else { data.version.clone() }), + ]; + + let metadata_block = v_flex() + .gap(px(6.0)) + .children(metadata_rows.into_iter().map(|(label, value)| { + h_flex() + .gap(px(16.0)) + .child( + div() + .w(px(120.0)) + .text_size(ui_text_sm(cx)) + .text_color(rgb(t.text_muted)) + .child(label), + ) + .child( + div() + .text_size(ui_text_sm(cx)) + .text_color(rgb(t.text_primary)) + .child(value), + ) + })); + + div() + .id("font-preview") + .flex_1() + .min_h_0() + .overflow_y_scroll() + .bg(rgb(t.bg_secondary)) + .child( + v_flex() + .gap(px(32.0)) + .p(px(32.0)) + .child(sample_block) + .child( + div() + .h(px(1.0)) + .bg(rgb(t.border)) + .w_full(), + ) + .child(metadata_block), + ) + .into_any_element() +} diff --git a/crates/okena-files/src/file_viewer/selection.rs b/crates/okena-files/src/file_viewer/selection.rs index 20501250..242fca58 100644 --- a/crates/okena-files/src/file_viewer/selection.rs +++ b/crates/okena-files/src/file_viewer/selection.rs @@ -8,10 +8,12 @@ use okena_core::send_payload::{CodeBlock, SendPayload}; use super::{DisplayMode, FileViewer, FileViewerEvent}; impl FileViewer { - /// Toggle between source and preview display modes. + /// Toggle between source and preview display modes. Only meaningful for + /// tabs that actually have both views — markdown (rendered ↔ source) and + /// SVG (rasterized image ↔ XML). pub(super) fn toggle_display_mode(&mut self, cx: &mut Context) { let tab = self.active_tab_mut(); - if !tab.is_markdown { + if !tab.is_markdown && !tab.is_svg { return; } tab.display_mode = match tab.display_mode { diff --git a/crates/okena-files/src/project_fs.rs b/crates/okena-files/src/project_fs.rs index fb230a04..3779d8b9 100644 --- a/crates/okena-files/src/project_fs.rs +++ b/crates/okena-files/src/project_fs.rs @@ -22,6 +22,9 @@ pub trait ProjectFs: Send + Sync + 'static { /// Read file content as UTF-8 string. fn read_file(&self, relative_path: &str) -> Result; + /// Read file content as raw bytes. Used for binary previews (images). + fn read_file_bytes(&self, relative_path: &str) -> Result, String>; + /// Get file size in bytes. fn file_size(&self, relative_path: &str) -> Result; @@ -56,6 +59,25 @@ impl LocalProjectFs { pub fn new(path: impl Into) -> Self { Self { path: path.into() } } + + /// Canonicalize `relative_path` inside the project root and reject any + /// path that escapes via `..` or absolute-path replacement. Mirrors the + /// server-side `resolve_project_file` so the trait has the same + /// containment guarantee on both implementations. + fn resolve(&self, relative_path: &str) -> Result { + let joined = self.path.join(relative_path); + let canonical = joined + .canonicalize() + .map_err(|e| format!("Cannot read file: {}", e))?; + let root = self + .path + .canonicalize() + .map_err(|e| format!("Cannot resolve project path: {}", e))?; + if !canonical.starts_with(&root) { + return Err("path traversal not allowed".to_string()); + } + Ok(canonical) + } } impl ProjectFs for LocalProjectFs { @@ -72,12 +94,17 @@ impl ProjectFs for LocalProjectFs { } fn read_file(&self, relative_path: &str) -> Result { - let full = self.path.join(relative_path); + let full = self.resolve(relative_path)?; std::fs::read_to_string(&full).map_err(|e| format!("Cannot read file: {}", e)) } + fn read_file_bytes(&self, relative_path: &str) -> Result, String> { + let full = self.resolve(relative_path)?; + std::fs::read(&full).map_err(|e| format!("Cannot read file: {}", e)) + } + fn file_size(&self, relative_path: &str) -> Result { - let full = self.path.join(relative_path); + let full = self.resolve(relative_path)?; std::fs::metadata(&full) .map(|m| m.len()) .map_err(|e| format!("Cannot read file: {}", e)) @@ -176,6 +203,26 @@ impl ProjectFs for RemoteProjectFs { } } + fn read_file_bytes(&self, relative_path: &str) -> Result, String> { + use base64::Engine as _; + let action = okena_core::api::ActionRequest::ReadFileBytes { + project_id: self.project_id.clone(), + relative_path: relative_path.to_string(), + }; + match self.post_action(action)? { + Some(value) => { + let encoded = value + .get("content_b64") + .and_then(|v| v.as_str()) + .ok_or_else(|| "Missing content_b64 in response".to_string())?; + base64::engine::general_purpose::STANDARD + .decode(encoded) + .map_err(|e| format!("Invalid base64 in response: {}", e)) + } + None => Err("Empty response".to_string()), + } + } + fn file_size(&self, relative_path: &str) -> Result { let action = okena_core::api::ActionRequest::FileSize { project_id: self.project_id.clone(), diff --git a/src/action_dispatch.rs b/src/action_dispatch.rs index c184a90c..eb9c9d47 100644 --- a/src/action_dispatch.rs +++ b/src/action_dispatch.rs @@ -675,6 +675,10 @@ fn strip_remote_ids(action: ActionRequest, connection_id: &str) -> ActionRequest project_id: s(&project_id), relative_path, }, + ActionRequest::ReadFileBytes { project_id, relative_path } => ActionRequest::ReadFileBytes { + project_id: s(&project_id), + relative_path, + }, ActionRequest::FileSize { project_id, relative_path } => ActionRequest::FileSize { project_id: s(&project_id), relative_path, diff --git a/src/workspace/actions/execute/files.rs b/src/workspace/actions/execute/files.rs index 912cdd12..3f4aafb1 100644 --- a/src/workspace/actions/execute/files.rs +++ b/src/workspace/actions/execute/files.rs @@ -52,6 +52,54 @@ pub(super) fn read_file(ws: &Workspace, project_id: String, relative_path: Strin } } +/// Server-side ceiling on bytes returned from ReadFileBytes. Mirrors the +/// client's MAX_IMAGE_FILE_SIZE so a misbehaving or older client can't trick +/// the server into reading and base64-encoding arbitrarily large files +/// (each request transiently holds raw + base64 + JSON copies, so the +/// resident multiple is roughly 3-4× the file size). +const MAX_READ_FILE_BYTES: u64 = 20 * 1024 * 1024; + +pub(super) fn read_file_bytes(ws: &Workspace, project_id: String, relative_path: String) -> ActionResult { + use base64::Engine as _; + match ws.project(&project_id) { + Some(p) => { + let canonical = match resolve_project_file(&p.path, &relative_path) { + Ok(c) => c, + Err(e) => return ActionResult::Err(e), + }; + // Enforce the cap from metadata before allocating; std::fs::read + // alone would happily pull a multi-GB file into memory. + match std::fs::metadata(&canonical) { + Ok(m) if m.len() > MAX_READ_FILE_BYTES => { + return ActionResult::Err(format!( + "File too large ({:.1} MB). Maximum is {} MB.", + m.len() as f64 / 1024.0 / 1024.0, + MAX_READ_FILE_BYTES / 1024 / 1024 + )); + } + Ok(_) => {} + Err(e) => return ActionResult::Err(format!("Cannot read file: {}", e)), + } + match std::fs::read(&canonical) { + Ok(bytes) => { + if bytes.len() as u64 > MAX_READ_FILE_BYTES { + // TOCTOU: file grew between stat and read. + return ActionResult::Err(format!( + "File too large ({:.1} MB). Maximum is {} MB.", + bytes.len() as f64 / 1024.0 / 1024.0, + MAX_READ_FILE_BYTES / 1024 / 1024 + )); + } + let encoded = base64::engine::general_purpose::STANDARD.encode(&bytes); + ActionResult::Ok(Some(serde_json::json!({ "content_b64": encoded }))) + } + Err(e) => ActionResult::Err(format!("Cannot read file: {}", e)), + } + } + None => ActionResult::Err(format!("project not found: {}", project_id)), + } +} + pub(super) fn file_size(ws: &Workspace, project_id: String, relative_path: String) -> ActionResult { match ws.project(&project_id) { Some(p) => { diff --git a/src/workspace/actions/execute/mod.rs b/src/workspace/actions/execute/mod.rs index b4f4a799..437916ba 100644 --- a/src/workspace/actions/execute/mod.rs +++ b/src/workspace/actions/execute/mod.rs @@ -157,6 +157,9 @@ pub fn execute_action( ActionRequest::ReadFile { project_id, relative_path } => { files::read_file(ws, project_id, relative_path) } + ActionRequest::ReadFileBytes { project_id, relative_path } => { + files::read_file_bytes(ws, project_id, relative_path) + } ActionRequest::FileSize { project_id, relative_path } => { files::file_size(ws, project_id, relative_path) } From ded3fa80d8df230d566f1592fa495961232394f1 Mon Sep 17 00:00:00 2001 From: jonasnobile Date: Mon, 25 May 2026 14:34:01 +0200 Subject: [PATCH 2/8] feat(files): zoom/pan + background toggle for image previews MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Scroll wheel zooms exponentially; drag pans; double-click resets to fit; Cmd/Ctrl+0/+/- keybinds. Default is auto-fit (ObjectFit::Contain); zooming or panning promotes the view to natural-size × zoom with manual pan offset. - Header chiclet shows "− Fit | 100% +"; clicking the label toggles between fit and 100%. Zoom clamped to [0.1, 10]. - Background segmented toggle (Checker / Light / Dark) solves the "black SVG invisible on dark theme" / "white SVG invisible on light theme" problem. Checker is the default — a 12 px two-tone grid painted procedurally via gpui::canvas + paint_quad. A single-colour icon stays visible against at least half the tiles regardless of fill. - DecodedImage now carries intrinsic pixel dimensions (probed via image::ImageReader header read for raster, taken from usvg::Tree for SVG) so zoom can render at natural × factor without re-decoding. Co-Authored-By: Claude Opus 4.7 --- Cargo.lock | 1 + crates/okena-files/Cargo.toml | 1 + crates/okena-files/src/file_viewer/loading.rs | 27 +- crates/okena-files/src/file_viewer/mod.rs | 115 +++++- crates/okena-files/src/file_viewer/render.rs | 365 +++++++++++++++--- .../okena-files/src/file_viewer/selection.rs | 102 ++++- 6 files changed, 552 insertions(+), 59 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5315f68d..1f5de1d6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5701,6 +5701,7 @@ dependencies = [ "grep-regex", "grep-searcher", "ignore", + "image", "log", "nucleo-matcher", "okena-core", diff --git a/crates/okena-files/Cargo.toml b/crates/okena-files/Cargo.toml index d8c6a80d..562fadce 100644 --- a/crates/okena-files/Cargo.toml +++ b/crates/okena-files/Cargo.toml @@ -27,6 +27,7 @@ base64 = "0.22" usvg = { version = "0.45", default-features = false } ttf-parser = "0.25" woff2 = { package = "woff2-patched", version = "0.4" } +image = { version = "0.25", default-features = false } [dev-dependencies] tempfile = "3" diff --git a/crates/okena-files/src/file_viewer/loading.rs b/crates/okena-files/src/file_viewer/loading.rs index fc02e892..6643cfec 100644 --- a/crates/okena-files/src/file_viewer/loading.rs +++ b/crates/okena-files/src/file_viewer/loading.rs @@ -259,14 +259,31 @@ pub(super) fn build_image_content( // decoding fails we still surface the preview without source. let source = String::from_utf8(bytes).ok(); Ok(LoadedContent::Image { - decoded: DecodedImage::Rendered(rendered), + decoded: DecodedImage::Rendered { + image: rendered, + width: w as u32, + height: h as u32, + }, source, }) } - _ => Ok(LoadedContent::Image { - decoded: DecodedImage::Raster(Arc::new(Image::from_bytes(format, bytes))), - source: None, - }), + _ => { + // Probe intrinsic dimensions without decoding the full pixel + // buffer; image::ImageReader reads only the header. + let (width, height) = image::ImageReader::new(std::io::Cursor::new(&bytes)) + .with_guessed_format() + .map_err(|e| format!("Cannot read image header: {}", e))? + .into_dimensions() + .map_err(|e| format!("Cannot read image dimensions: {}", e))?; + Ok(LoadedContent::Image { + decoded: DecodedImage::Raster { + image: Arc::new(Image::from_bytes(format, bytes)), + width, + height, + }, + source: None, + }) + } } } diff --git a/crates/okena-files/src/file_viewer/mod.rs b/crates/okena-files/src/file_viewer/mod.rs index 241d6592..483d9440 100644 --- a/crates/okena-files/src/file_viewer/mod.rs +++ b/crates/okena-files/src/file_viewer/mod.rs @@ -49,6 +49,86 @@ pub(super) enum DisplayMode { Preview, } +/// Background fill behind an image / SVG preview. Single-colour SVGs (a +/// black icon) become invisible on a matching pane background, so the user +/// can flip between a checkerboard (default — shows both extremes) and +/// explicit Light / Dark fills. +#[derive(Clone, Copy, PartialEq, Eq, Default)] +pub enum PreviewBackground { + #[default] + Checker, + Light, + Dark, +} + +/// Per-tab zoom / pan / background state for the image preview. +/// +/// `auto_fit` is the default: image renders with `ObjectFit::Contain` to +/// fill the pane, `zoom` and `pan` are ignored. Once the user wheel-zooms, +/// drags, or clicks 100%, `auto_fit` flips off and the view renders at +/// natural-size × `zoom` with `pan` applied. +#[derive(Clone)] +pub struct ImageViewState { + /// Fit-to-pane mode (ObjectFit::Contain). Reset via Ctrl+0 or the Fit + /// header button. + pub auto_fit: bool, + /// Scale factor: 1.0 = 100% natural pixel size. Ignored when `auto_fit`. + pub zoom: f32, + /// Pan offset (image translates by this). Ignored when `auto_fit`. + pub pan: gpui::Point, + /// True while a pan drag is in progress. + pub is_panning: bool, + /// Mouse position at drag start, plus the pan offset captured then, so + /// we can compute pan = offset + (mouse - anchor) without drift. + pub pan_anchor: Option>, + pub pan_anchor_offset: gpui::Point, + pub background: PreviewBackground, +} + +impl Default for ImageViewState { + fn default() -> Self { + Self { + auto_fit: true, + zoom: 1.0, + pan: gpui::Point::default(), + is_panning: false, + pan_anchor: None, + pan_anchor_offset: gpui::Point::default(), + background: PreviewBackground::Checker, + } + } +} + +impl ImageViewState { + /// Clamp the zoom factor to a sane range. Below 0.1× the image + /// disappears; above 10× the rendered surface dwarfs any reasonable + /// display and pan becomes unusable. + pub const MIN_ZOOM: f32 = 0.1; + pub const MAX_ZOOM: f32 = 10.0; + + /// Set zoom and leave auto-fit mode. Pan stays as-is. + pub fn set_zoom(&mut self, zoom: f32) { + self.zoom = zoom.clamp(Self::MIN_ZOOM, Self::MAX_ZOOM); + self.auto_fit = false; + } + + /// Multiply current zoom by `factor` and leave auto-fit mode. + pub fn zoom_by(&mut self, factor: f32) { + let current = if self.auto_fit { 1.0 } else { self.zoom }; + self.set_zoom(current * factor); + } + + /// Reset to fit-to-pane. + pub fn reset_to_fit(&mut self) { + self.auto_fit = true; + self.zoom = 1.0; + self.pan = gpui::Point::default(); + self.is_panning = false; + self.pan_anchor = None; + self.pan_anchor_offset = gpui::Point::default(); + } +} + /// Type alias for source view selection (line, column). type Selection = SelectionState<(usize, usize)>; @@ -110,6 +190,10 @@ pub(super) struct FileViewerTab { /// Decoded image data wrapped for GPUI's `img()` element. Populated by /// the async loader for image tabs. pub image_data: Option, + /// Pan / zoom / background state for image and SVG-Preview tabs. + /// Persists across freshness reloads so the user keeps their view as + /// the file changes on disk. + pub image_view: ImageViewState, /// True for font files (otf/ttf/woff/woff2). When set, `font_data` /// carries parsed metadata and the source/image rendering paths are /// bypassed in favour of the font-preview branch. @@ -124,18 +208,37 @@ pub(super) struct FileViewerTab { /// Decoded image payload. Raster formats let GPUI's asset cache handle the /// RGBA→BGRA swap; SVGs are pre-rasterized to BGRA ourselves because GPUI's /// built-in `ImageDecoder` calls `render_single_frame(.., to_bgra=false)` for -/// SVG, leaving R/B swapped and the preview inverted. +/// SVG, leaving R/B swapped and the preview inverted. Each variant carries +/// the intrinsic pixel dimensions so the zoom UI can compute pan bounds +/// and a "100%" / "fit" mode without re-decoding. #[derive(Clone)] pub enum DecodedImage { - Raster(Arc), - Rendered(Arc), + Raster { + image: Arc, + width: u32, + height: u32, + }, + Rendered { + image: Arc, + width: u32, + height: u32, + }, +} + +impl DecodedImage { + pub fn dimensions(&self) -> (u32, u32) { + match self { + DecodedImage::Raster { width, height, .. } => (*width, *height), + DecodedImage::Rendered { width, height, .. } => (*width, *height), + } + } } impl From for ImageSource { fn from(value: DecodedImage) -> Self { match value { - DecodedImage::Raster(image) => ImageSource::Image(image), - DecodedImage::Rendered(rendered) => ImageSource::Render(rendered), + DecodedImage::Raster { image, .. } => ImageSource::Image(image), + DecodedImage::Rendered { image, .. } => ImageSource::Render(image), } } } @@ -244,6 +347,7 @@ impl FileViewerTab { is_image: false, is_svg: false, image_data: None, + image_view: ImageViewState::default(), is_font: false, font_data: None, } @@ -285,6 +389,7 @@ impl FileViewerTab { is_image, is_svg, image_data: None, + image_view: ImageViewState::default(), is_font, font_data: None, } diff --git a/crates/okena-files/src/file_viewer/render.rs b/crates/okena-files/src/file_viewer/render.rs index cd77a39c..c724c745 100644 --- a/crates/okena-files/src/file_viewer/render.rs +++ b/crates/okena-files/src/file_viewer/render.rs @@ -24,7 +24,7 @@ use okena_ui::tokens::{ui_text, ui_text_md, ui_text_ms, ui_text_sm, ui_text_xl}; use std::sync::Arc; use super::context_menu::TreeNodeTarget; -use super::{DisplayMode, FileViewer, FontData, SIDEBAR_WIDTH}; +use super::{DisplayMode, FileViewer, FontData, PreviewBackground, SIDEBAR_WIDTH}; /// Helper to create rgba from u32 color and alpha. fn rgba(color: u32, alpha: f32) -> Rgba { @@ -770,6 +770,301 @@ impl FileViewer { tab.markdown_list_state.clone() } + + /// Render the image / SVG-Preview pane with zoom / pan / background + /// support. Default is `auto_fit` (ObjectFit::Contain); once the user + /// wheel-zooms or drags, the view switches to natural-size × zoom with + /// manual pan. Double-click resets to fit. + pub(super) fn render_image_preview( + &self, + t: &ThemeColors, + cx: &mut Context, + ) -> impl IntoElement { + let tab = self.active_tab(); + let image_data = tab.image_data.clone(); + let view = tab.image_view.clone(); + let muted = t.text_muted; + let bg_secondary = t.bg_secondary; + + let Some(image) = image_data else { + return div() + .id("file-image-empty") + .flex_1() + .min_h_0() + .flex() + .items_center() + .justify_center() + .bg(rgb(bg_secondary)) + .child( + div() + .text_size(ui_text_sm(cx)) + .text_color(rgb(muted)) + .child("Image not available"), + ) + .into_any_element(); + }; + + let (nat_w, nat_h) = image.dimensions(); + let zoom = view.zoom; + let pan = view.pan; + let auto_fit = view.auto_fit; + + // Background fill: explicit color for Light/Dark, checkerboard + // canvas for Checker. The canvas is painted into the pane via + // paint_quad in tiles small enough to read against both extremes. + let (bg_solid, show_checker) = match view.background { + PreviewBackground::Light => (0xFFFFFFu32, false), + PreviewBackground::Dark => (0x111111u32, false), + PreviewBackground::Checker => (0x808080u32, true), + }; + + let fallback_muted = muted; + let img_element = img(image) + .with_fallback(move || { + div() + .flex() + .items_center() + .justify_center() + .text_size(px(14.0)) + .text_color(rgb(fallback_muted)) + .child("Cannot decode image") + .into_any_element() + }); + + let img_styled: AnyElement = if auto_fit { + img_element + .object_fit(ObjectFit::Contain) + .max_w_full() + .max_h_full() + .into_any_element() + } else { + img_element + .object_fit(ObjectFit::Fill) + .w(px((nat_w as f32) * zoom)) + .h(px((nat_h as f32) * zoom)) + .ml(pan.x) + .mt(pan.y) + .into_any_element() + }; + + let cursor = if auto_fit { + CursorStyle::Arrow + } else if view.is_panning { + CursorStyle::ClosedHand + } else { + CursorStyle::OpenHand + }; + + let mut container = div() + .id("file-image") + .flex_1() + .min_h_0() + .relative() + .overflow_hidden() + .bg(rgb(bg_solid)) + .cursor(cursor); + + if show_checker { + container = container.child( + canvas( + |_, _, _| (), + |bounds, _, window, _| paint_checkerboard(bounds, window), + ) + .absolute() + .inset_0() + .size_full(), + ); + } + + container + .on_scroll_wheel(cx.listener(|this, event: &ScrollWheelEvent, _, cx| { + let delta = event.delta.pixel_delta(px(17.0)); + let dy = f32::from(delta.y); + if dy.abs() < f32::EPSILON { + return; + } + // Exponential: positive dy zooms in, negative out. ±100 px ≈ 1.2× / 0.83×. + let factor = (dy / 250.0).exp(); + this.image_zoom_by(factor, cx); + })) + .on_mouse_down( + MouseButton::Left, + cx.listener(|this, event: &MouseDownEvent, _, cx| { + if event.click_count >= 2 { + this.image_fit(cx); + } else { + this.image_start_pan(event.position, cx); + } + }), + ) + .on_mouse_move(cx.listener(|this, event: &MouseMoveEvent, _, cx| { + this.image_update_pan(event.position, cx); + })) + .on_mouse_up( + MouseButton::Left, + cx.listener(|this, _, _, cx| { + this.image_end_pan(cx); + }), + ) + .child( + div() + .size_full() + .flex() + .items_center() + .justify_center() + .child(img_styled), + ) + .into_any_element() + } +} + +/// Header chiclet: `−` / `Fit | 100%` / `+`. Click on the label toggles +/// between Fit and 100%. Buttons step zoom by 1.25× / 0.8×. +fn image_zoom_controls( + zoom_label: String, + t: &ThemeColors, + cx: &mut Context, +) -> impl IntoElement { + let button = move |id: &'static str, glyph: &'static str, t: &ThemeColors, cx: &mut Context, on_click: Box)>| { + let t_text_muted = t.text_muted; + let t_bg_hover = t.bg_hover; + div() + .id(id) + .cursor_pointer() + .w(px(24.0)) + .h(px(24.0)) + .flex() + .items_center() + .justify_center() + .rounded(px(4.0)) + .hover(|s| s.bg(rgb(t_bg_hover))) + .text_size(px(13.0)) + .text_color(rgb(t_text_muted)) + .on_click(cx.listener(move |this, _, _, cx| on_click(this, cx))) + .child(glyph) + }; + h_flex() + .gap(px(2.0)) + .px(px(4.0)) + .py(px(2.0)) + .rounded(px(4.0)) + .bg(rgb(t.bg_secondary)) + .child(button( + "zoom-out", + "−", + t, + cx, + Box::new(|this, cx| this.image_zoom_by(1.0 / 1.25, cx)), + )) + .child( + div() + .id("zoom-label") + .cursor_pointer() + .min_w(px(48.0)) + .text_align(TextAlign::Center) + .text_size(px(12.0)) + .text_color(rgb(t.text_muted)) + .rounded(px(4.0)) + .hover(|s| s.bg(rgb(t.bg_hover))) + .px(px(4.0)) + .py(px(2.0)) + .on_click(cx.listener(|this, _, _, cx| { + if this.active_tab().image_view.auto_fit { + this.image_set_zoom(1.0, cx); + } else { + this.image_fit(cx); + } + })) + .child(zoom_label), + ) + .child(button( + "zoom-in", + "+", + t, + cx, + Box::new(|this, cx| this.image_zoom_by(1.25, cx)), + )) +} + +/// Header chiclet that cycles the preview background (Checker / Light / +/// Dark). The current option is highlighted; clicking advances to the +/// next. Solves the "black SVG on dark theme" / "white SVG on light +/// theme" invisibility problem. +fn image_background_toggle( + current: PreviewBackground, + t: &ThemeColors, + cx: &mut Context, +) -> impl IntoElement { + let labels = [ + (PreviewBackground::Checker, "Checker"), + (PreviewBackground::Light, "Light"), + (PreviewBackground::Dark, "Dark"), + ]; + h_flex() + .gap(px(2.0)) + .px(px(2.0)) + .py(px(2.0)) + .rounded(px(4.0)) + .bg(rgb(t.bg_secondary)) + .children(labels.into_iter().map(|(bg, label)| { + let is_active = current == bg; + let t_active = t.bg_selection; + let t_hover = t.bg_hover; + let t_text_primary = t.text_primary; + let t_text_muted = t.text_muted; + div() + .id(ElementId::Name(format!("bg-{}", label).into())) + .cursor_pointer() + .px(px(8.0)) + .py(px(2.0)) + .rounded(px(3.0)) + .when(is_active, |d| d.bg(rgb(t_active))) + .when(!is_active, |d| d.hover(|s| s.bg(rgb(t_hover)))) + .text_size(px(12.0)) + .text_color(rgb(if is_active { t_text_primary } else { t_text_muted })) + .on_click(cx.listener(move |this, _, _, cx| { + this.image_set_background(bg, cx); + })) + .child(label) + })) +} + +/// Paint a procedural 12 px checkerboard inside `bounds`. Used as the +/// default image-preview background — a black SVG icon stays visible +/// against the light tiles and a white one against the dark tiles, so the +/// user always sees at least half the artwork regardless of fill color. +fn paint_checkerboard(bounds: gpui::Bounds, window: &mut gpui::Window) { + const TILE: f32 = 12.0; + let light: gpui::Rgba = gpui::Rgba { + r: 0.62, + g: 0.62, + b: 0.62, + a: 1.0, + }; + let dark: gpui::Rgba = gpui::Rgba { + r: 0.42, + g: 0.42, + b: 0.42, + a: 1.0, + }; + // Base fill — every "even" tile inherits this. + window.paint_quad(fill(bounds, light)); + let cols = (f32::from(bounds.size.width) / TILE).ceil() as usize + 1; + let rows = (f32::from(bounds.size.height) / TILE).ceil() as usize + 1; + for row in 0..rows { + for col in 0..cols { + if (row + col) % 2 == 0 { + continue; + } + let x = f32::from(bounds.origin.x) + (col as f32) * TILE; + let y = f32::from(bounds.origin.y) + (row as f32) * TILE; + let tile_bounds = gpui::Bounds { + origin: gpui::point(px(x), px(y)), + size: gpui::size(px(TILE), px(TILE)), + }; + window.paint_quad(fill(tile_bounds, dark)); + } + } } impl Render for FileViewer { @@ -789,7 +1084,6 @@ impl Render for FileViewer { let is_image = tab.is_image; let is_svg = tab.is_svg; let is_font = tab.is_font; - let image_data = tab.image_data.clone(); let font_data = tab.font_data.clone(); let display_mode = tab.display_mode; let is_preview_mode = display_mode == DisplayMode::Preview; @@ -997,6 +1291,15 @@ impl Render for FileViewer { "w" if modifiers.platform || modifiers.control => { this.close_active_tab(cx); } + "0" if (modifiers.platform || modifiers.control) && is_img => { + this.image_fit(cx); + } + "=" | "+" if (modifiers.platform || modifiers.control) && is_img => { + this.image_zoom_by(1.25, cx); + } + "-" if (modifiers.platform || modifiers.control) && is_img => { + this.image_zoom_by(1.0 / 1.25, cx); + } "r" if !modifiers.platform && !modifiers.control => { this.refresh_file_tree_async(cx); } @@ -1162,6 +1465,17 @@ impl Render for FileViewer { )), ) }) + .when(show_image, |d| { + let view = self.active_tab().image_view.clone(); + let zoom_label = if view.auto_fit { + "Fit".to_string() + } else { + format!("{}%", (view.zoom * 100.0).round() as i32) + }; + let bg = view.background; + d.child(image_zoom_controls(zoom_label, &t, cx)) + .child(image_background_toggle(bg, &t, cx)) + }) .when(!self.is_detached, |d| { d.child( div() @@ -1260,52 +1574,7 @@ impl Render for FileViewer { ) }) .when(!tab_loading && !has_error && show_image, |d| { - let body = div() - .id("file-image") - .flex_1() - .min_h_0() - .flex() - .items_center() - .justify_center() - .overflow_hidden() - .bg(rgb(t.bg_secondary)) - .p(px(16.0)); - let muted = t.text_muted; - let body = match image_data.clone() { - Some(image) => body.child( - img(image) - .object_fit(ObjectFit::Contain) - .max_w_full() - .max_h_full() - // Without this, GPUI silently - // renders an empty box when the - // raster decoder rejects the - // bytes (corrupt, truncated, - // 0-byte, or mis-extensioned - // file). SVG goes through a - // separate pre-rasterizer that - // surfaces errors via error_message; - // raster decodes happen lazily - // inside GPUI's asset cache. - .with_fallback(move || { - div() - .flex() - .items_center() - .justify_center() - .text_size(px(14.0)) - .text_color(rgb(muted)) - .child("Cannot decode image") - .into_any_element() - }), - ), - None => body.child( - div() - .text_size(ui_text_sm(cx)) - .text_color(rgb(t.text_muted)) - .child("Image not available"), - ), - }; - d.child(body) + d.child(self.render_image_preview(&t, cx)) }) .when(!tab_loading && !has_error && show_font, |d| { d.child(render_font_preview(font_data.clone(), &t, cx)) diff --git a/crates/okena-files/src/file_viewer/selection.rs b/crates/okena-files/src/file_viewer/selection.rs index 242fca58..b6e7da2f 100644 --- a/crates/okena-files/src/file_viewer/selection.rs +++ b/crates/okena-files/src/file_viewer/selection.rs @@ -5,7 +5,7 @@ use crate::selection::{copy_to_clipboard, Selection1DExtension, Selection2DNonEm use gpui::*; use okena_core::send_payload::{CodeBlock, SendPayload}; -use super::{DisplayMode, FileViewer, FileViewerEvent}; +use super::{DisplayMode, FileViewer, FileViewerEvent, PreviewBackground}; impl FileViewer { /// Toggle between source and preview display modes. Only meaningful for @@ -209,4 +209,104 @@ impl FileViewer { self.active_tab_mut().scrollbar_drag = None; cx.notify(); } + + // ── Image zoom / pan / background ──────────────────────────────────── + + /// Multiply the active tab's image zoom by `factor` (e.g. 1.25 in, + /// 1/1.25 out). Leaves auto-fit mode. + pub(super) fn image_zoom_by(&mut self, factor: f32, cx: &mut Context) { + let tab = self.active_tab_mut(); + if !tab.is_image { + return; + } + tab.image_view.zoom_by(factor); + cx.notify(); + } + + /// Set the active tab's image zoom to an explicit factor (1.0 = 100%). + pub(super) fn image_set_zoom(&mut self, zoom: f32, cx: &mut Context) { + let tab = self.active_tab_mut(); + if !tab.is_image { + return; + } + tab.image_view.set_zoom(zoom); + cx.notify(); + } + + /// Reset the active tab's image view to fit-to-pane. + pub(super) fn image_fit(&mut self, cx: &mut Context) { + let tab = self.active_tab_mut(); + if !tab.is_image { + return; + } + tab.image_view.reset_to_fit(); + cx.notify(); + } + + /// Begin a pan drag at the given screen-space position. + pub(super) fn image_start_pan( + &mut self, + position: gpui::Point, + cx: &mut Context, + ) { + let tab = self.active_tab_mut(); + if !tab.is_image { + return; + } + // Panning only matters once we've left auto-fit, so promote to + // explicit zoom on first drag. + if tab.image_view.auto_fit { + tab.image_view.auto_fit = false; + tab.image_view.zoom = 1.0; + } + tab.image_view.is_panning = true; + tab.image_view.pan_anchor = Some(position); + tab.image_view.pan_anchor_offset = tab.image_view.pan; + cx.notify(); + } + + /// Continue a pan drag — translate by (current - anchor). + pub(super) fn image_update_pan( + &mut self, + position: gpui::Point, + cx: &mut Context, + ) { + let tab = self.active_tab_mut(); + if !tab.is_image || !tab.image_view.is_panning { + return; + } + let Some(anchor) = tab.image_view.pan_anchor else { + return; + }; + tab.image_view.pan = gpui::Point::new( + tab.image_view.pan_anchor_offset.x + (position.x - anchor.x), + tab.image_view.pan_anchor_offset.y + (position.y - anchor.y), + ); + cx.notify(); + } + + /// End a pan drag. + pub(super) fn image_end_pan(&mut self, cx: &mut Context) { + let tab = self.active_tab_mut(); + if !tab.is_image { + return; + } + tab.image_view.is_panning = false; + tab.image_view.pan_anchor = None; + cx.notify(); + } + + /// Set the preview background explicitly (header button click). + pub(super) fn image_set_background( + &mut self, + background: PreviewBackground, + cx: &mut Context, + ) { + let tab = self.active_tab_mut(); + if !tab.is_image { + return; + } + tab.image_view.background = background; + cx.notify(); + } } From d4fa47b67c44da416f98c3f47146296cae779957 Mon Sep 17 00:00:00 2001 From: jonasnobile Date: Mon, 25 May 2026 14:52:11 +0200 Subject: [PATCH 3/8] fix(files): preserve image aspect on zoom + re-rasterize SVG at higher scales MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Image element kept being squished by the surrounding flex layout once its explicit size grew past the viewport, which combined with ObjectFit::Fill produced a stretched preview and made only one axis appear to "zoom past" the pane. Pin with flex_shrink_0 and switch to ObjectFit::Contain so a fixed-aspect element is always sized exactly natural × zoom. - Pre-rasterized SVGs were locked to the load-time bitmap (intrinsic × GPUI's SMOOTH_SVG_SCALE_FACTOR = 2×), so zooming past ~100% revealed the bitmap's pixels instead of staying crisp. DecodedImage::Rendered now carries the source SVG bytes and the scale_factor we rasterized at; image_zoom_by/set_zoom call maybe_rerender_svg, which spawns a background SvgRenderer task at zoom × 1.25 (capped at 16×) whenever the current zoom exceeds the existing render. A per-tab in-flight flag coalesces a wheel-zoom storm into a single follow-up raster. Co-Authored-By: Claude Opus 4.7 --- crates/okena-files/src/file_viewer/loading.rs | 8 +- crates/okena-files/src/file_viewer/mod.rs | 115 ++++++++++++++++++ crates/okena-files/src/file_viewer/render.rs | 9 +- .../okena-files/src/file_viewer/selection.rs | 5 +- 4 files changed, 133 insertions(+), 4 deletions(-) diff --git a/crates/okena-files/src/file_viewer/loading.rs b/crates/okena-files/src/file_viewer/loading.rs index 6643cfec..2b36f753 100644 --- a/crates/okena-files/src/file_viewer/loading.rs +++ b/crates/okena-files/src/file_viewer/loading.rs @@ -252,17 +252,21 @@ pub(super) fn build_image_content( w, h, MAX_SVG_PIXELS / 1024 / 1024 )); } + let initial_scale: f32 = 1.0; let rendered = svg_renderer - .render_single_frame(&bytes, 1.0, true) + .render_single_frame(&bytes, initial_scale, true) .map_err(|e| format!("Cannot decode SVG: {}", e))?; // SVG is XML — UTF-8 unless someone hand-saved it weird. If // decoding fails we still surface the preview without source. - let source = String::from_utf8(bytes).ok(); + let svg_bytes = Arc::new(bytes); + let source = String::from_utf8(svg_bytes.as_ref().clone()).ok(); Ok(LoadedContent::Image { decoded: DecodedImage::Rendered { image: rendered, width: w as u32, height: h as u32, + svg_bytes, + rendered_scale: initial_scale, }, source, }) diff --git a/crates/okena-files/src/file_viewer/mod.rs b/crates/okena-files/src/file_viewer/mod.rs index 483d9440..f20ce582 100644 --- a/crates/okena-files/src/file_viewer/mod.rs +++ b/crates/okena-files/src/file_viewer/mod.rs @@ -83,6 +83,11 @@ pub struct ImageViewState { pub pan_anchor: Option>, pub pan_anchor_offset: gpui::Point, pub background: PreviewBackground, + /// True while a background SVG re-rasterization is in flight. Set on + /// `maybe_rerender_svg` dispatch and cleared on apply, so concurrent + /// zoom changes coalesce into a single follow-up raster instead of + /// spamming the background executor. + pub svg_rerender_in_flight: bool, } impl Default for ImageViewState { @@ -95,6 +100,7 @@ impl Default for ImageViewState { pan_anchor: None, pan_anchor_offset: gpui::Point::default(), background: PreviewBackground::Checker, + svg_rerender_in_flight: false, } } } @@ -126,6 +132,8 @@ impl ImageViewState { self.is_panning = false; self.pan_anchor = None; self.pan_anchor_offset = gpui::Point::default(); + // Keep svg_rerender_in_flight as-is; the in-flight task will + // resolve and we'll just discard its result via the apply check. } } @@ -222,6 +230,16 @@ pub enum DecodedImage { image: Arc, width: u32, height: u32, + /// Raw SVG bytes kept for re-rasterization at higher resolutions + /// when the user zooms in. Without this, `image` (a fixed bitmap) + /// would visibly pixelate past the rasterized scale. + svg_bytes: Arc>, + /// The `scale_factor` argument that was passed to + /// `SvgRenderer::render_single_frame` when `image` was produced. + /// GPUI multiplies this by `SMOOTH_SVG_SCALE_FACTOR (= 2)` + /// internally, so the actual pixmap is `intrinsic × scale_factor × 2`. + /// Used to decide when a re-raster is needed (zoom > rendered_scale). + rendered_scale: f32, }, } @@ -898,6 +916,103 @@ impl FileViewer { .detach(); } + /// If the active tab is an SVG and the current zoom exceeds the + /// rasterized bitmap's resolution, kick off a background re-raster at + /// a higher scale so the preview stays crisp. + /// + /// Coalescing: at most one re-raster is in flight per tab. While one + /// is running, further zoom changes are silently absorbed; when the + /// result arrives we re-check the current zoom and schedule another + /// raster if the user has zoomed further in the meantime. + pub(super) fn maybe_rerender_svg(&mut self, cx: &mut Context) { + let relative_path = self.active_tab().relative_path.clone(); + let svg_renderer = cx.svg_renderer(); + let Some((bytes, target_scale)) = self.compute_rerender_target(&relative_path) else { + return; + }; + + if let Some(tab) = self + .tabs + .iter_mut() + .find(|t| t.relative_path == relative_path) + { + tab.image_view.svg_rerender_in_flight = true; + } + + cx.spawn(async move |entity, cx| { + let result = cx + .background_executor() + .spawn(async move { + svg_renderer + .render_single_frame(&bytes, target_scale, true) + .map_err(|e| format!("Cannot re-rasterize SVG: {}", e)) + }) + .await; + let _ = entity.update(cx, |this, cx| { + if let Some(tab) = this + .tabs + .iter_mut() + .find(|t| t.relative_path == relative_path) + { + tab.image_view.svg_rerender_in_flight = false; + if let Ok(new_image) = result + && let Some(DecodedImage::Rendered { + image, + rendered_scale, + .. + }) = &mut tab.image_data + { + *image = new_image; + *rendered_scale = target_scale; + cx.notify(); + } + } + // The user may have kept zooming while we were rasterizing + // — check whether we still need to chase a higher target. + this.maybe_rerender_svg(cx); + }); + }) + .detach(); + } + + /// Return `(svg_bytes, target_scale)` if a re-raster of the named tab + /// would meaningfully sharpen the preview. Returns `None` when the tab + /// isn't an SVG, isn't zoomed in past its rendered scale, or already + /// has a re-raster in flight. + fn compute_rerender_target( + &self, + relative_path: &str, + ) -> Option<(Arc>, f32)> { + let tab = self.tabs.iter().find(|t| t.relative_path == relative_path)?; + if !tab.is_svg || tab.image_view.svg_rerender_in_flight { + return None; + } + // Fit mode renders via ObjectFit::Contain so the existing 2× pixmap + // is plenty — no need to re-raster. + if tab.image_view.auto_fit { + return None; + } + let DecodedImage::Rendered { + svg_bytes, + rendered_scale, + .. + } = tab.image_data.as_ref()? + else { + return None; + }; + let zoom = tab.image_view.zoom; + // Small threshold avoids re-rastering on micro-zoom adjustments + // and on the way back down from a previously-rendered high scale. + if zoom <= *rendered_scale * 1.1 { + return None; + } + // Cap to keep memory bounded. At 16× a 100×100 SVG produces a + // 3200×3200 RGBA bitmap (~40 MB) which is the absolute ceiling + // before the user notices the allocator. + let target = (zoom * 1.25).clamp(1.0, 16.0); + Some((svg_bytes.clone(), target)) + } + /// Get the active tab. pub(super) fn active_tab(&self) -> &FileViewerTab { &self.tabs[self.active_tab] diff --git a/crates/okena-files/src/file_viewer/render.rs b/crates/okena-files/src/file_viewer/render.rs index c724c745..3aa3be73 100644 --- a/crates/okena-files/src/file_viewer/render.rs +++ b/crates/okena-files/src/file_viewer/render.rs @@ -838,10 +838,17 @@ impl FileViewer { .max_h_full() .into_any_element() } else { + // flex_shrink_0 is required: without it, the flex container at + // the next layer ("size_full .flex .items_center .justify_center") + // squishes one axis once the image's explicit size exceeds the + // pane, which combined with ObjectFit::Fill produced a stretched + // preview and made only one axis appear to "zoom past" the + // viewport. Contain keeps aspect inside the (now-fixed) box. img_element - .object_fit(ObjectFit::Fill) + .object_fit(ObjectFit::Contain) .w(px((nat_w as f32) * zoom)) .h(px((nat_h as f32) * zoom)) + .flex_shrink_0() .ml(pan.x) .mt(pan.y) .into_any_element() diff --git a/crates/okena-files/src/file_viewer/selection.rs b/crates/okena-files/src/file_viewer/selection.rs index b6e7da2f..c968ca62 100644 --- a/crates/okena-files/src/file_viewer/selection.rs +++ b/crates/okena-files/src/file_viewer/selection.rs @@ -213,7 +213,8 @@ impl FileViewer { // ── Image zoom / pan / background ──────────────────────────────────── /// Multiply the active tab's image zoom by `factor` (e.g. 1.25 in, - /// 1/1.25 out). Leaves auto-fit mode. + /// 1/1.25 out). Leaves auto-fit mode and, for SVG tabs, schedules a + /// fresh raster at the new scale so the preview stays crisp. pub(super) fn image_zoom_by(&mut self, factor: f32, cx: &mut Context) { let tab = self.active_tab_mut(); if !tab.is_image { @@ -221,6 +222,7 @@ impl FileViewer { } tab.image_view.zoom_by(factor); cx.notify(); + self.maybe_rerender_svg(cx); } /// Set the active tab's image zoom to an explicit factor (1.0 = 100%). @@ -231,6 +233,7 @@ impl FileViewer { } tab.image_view.set_zoom(zoom); cx.notify(); + self.maybe_rerender_svg(cx); } /// Reset the active tab's image view to fit-to-pane. From 149db5e28fa5fd2e377c437f5376253eee39f274 Mon Sep 17 00:00:00 2001 From: jonasnobile Date: Mon, 25 May 2026 15:34:22 +0200 Subject: [PATCH 4/8] fix(files): address third-pass review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lifecycle + race fixes: - maybe_rerender_svg now takes a captured relative_path so the chase loop re-enters against the tab whose raster just completed, not self.active_tab() — a tab switch mid-raster no longer strands the loop on the wrong tab. - On apply, compare Arc::as_ptr(&svg_bytes) to the bytes captured at dispatch; if a freshness reload swapped image_data mid-flight, discard the stale raster instead of overwriting the fresh bitmap with old pixels. - On Err, pin rendered_scale to target_scale so the chase loop stops re-requesting the same failing raster every tick (was an infinite serial respawn loop). - check_active_tab_freshness now kicks maybe_rerender_svg_for after a successful SVG reload — without this, an external save reset the bitmap to 1x while the persisted zoom stayed at e.g. 8x, leaving a pixelated preview until the user wheels. Input hardening: - image_start_pan no longer promotes auto_fit -> false on mouse-down; it only records the anchor. image_update_pan promotes on the first non-zero delta, so a stationary click (focus, accidental tap, or the first half of a double-click) no longer snaps Fit -> 100%. Also kills the previous "double-click flickers Fit -> 100% -> Fit" bug. - on_mouse_move clears is_panning when the Left button isn't pressed, so a release outside the container no longer leaves the image tracking the cursor with no button held. - on_scroll_wheel rejects non-finite dy (NaN/inf no longer poisons zoom) and uses a 0.5px threshold instead of f32::EPSILON so kinetic- momentum tail events stop nudging zoom for ~1s after the user stops. - Cmd/Ctrl+0/+/- gated on is_img_view (excludes SVG-in-Source mode) so zoom chords don't silently mutate a hidden image_view while the user is editing XML. - Pan offset hard-capped at +-10000px per axis so a runaway drag can't send the image into the void where the only recovery is the Fit chiclet. Security + robustness: - LocalProjectFs::list_directory was unguarded; route the relative path through the same canonicalize + starts_with(project_root) check the read_file_bytes path already uses. Closes the path-traversal hole the previous round opened on the bytes side but missed on listings. - remote_action's shared_client / bytes_client switched from .expect() to a Result-bearing OnceLock — TLS backend init failures (corrupt cert store, sandboxed Keychain denial, FIPS mode) now surface as a recoverable error rather than panicking inside OnceLock and poisoning every retry. Correctness: - build_image_content for raster formats now uses image::ImageReader's content-sniffed format (not the extension-derived one) for both the dimensions probe AND Image::from_bytes, so an extension/content mismatch (e.g. a .png that's really JPEG) decodes through the right codec instead of dead-ending at "Cannot decode image". Co-Authored-By: Claude Opus 4.7 --- crates/okena-core/src/remote_action.rs | 42 +++++---- crates/okena-files/src/file_viewer/loading.rs | 35 +++++++- crates/okena-files/src/file_viewer/mod.rs | 86 ++++++++++++++++--- crates/okena-files/src/file_viewer/render.rs | 34 +++++++- .../okena-files/src/file_viewer/selection.rs | 40 ++++++--- crates/okena-files/src/list_directory.rs | 24 +++++- 6 files changed, 215 insertions(+), 46 deletions(-) diff --git a/crates/okena-core/src/remote_action.rs b/crates/okena-core/src/remote_action.rs index fb0ee359..09ef7941 100644 --- a/crates/okena-core/src/remote_action.rs +++ b/crates/okena-core/src/remote_action.rs @@ -18,36 +18,46 @@ const BYTES_TIMEOUT_SECS: u64 = 90; /// `src/workspace/actions/execute/files.rs`. const MAX_RESPONSE_BYTES: u64 = 32 * 1024 * 1024; -fn build_client(timeout_secs: u64) -> reqwest::blocking::Client { - #[allow( - clippy::expect_used, - reason = "reqwest client build only fails on TLS backend init — nothing recoverable at this call site" - )] +/// Build a reqwest blocking client. Returns Err on TLS backend init +/// failure (corrupt cert store, sandboxed Keychain denial, FIPS mode +/// rejecting default ciphers, container without ca-certificates). Caller +/// is expected to surface the error to the user — a panic here would +/// abort the whole app at first remote probe and OnceLock-poison every +/// retry. +fn build_client(timeout_secs: u64) -> Result { reqwest::blocking::Client::builder() .timeout(std::time::Duration::from_secs(timeout_secs)) .connect_timeout(std::time::Duration::from_secs(5)) .build() - .expect("Failed to build HTTP client") + .map_err(|e| format!("Cannot initialise HTTP client: {}", e)) } -/// Fast-action client (terminal control, listings, metadata). -fn shared_client() -> &'static reqwest::blocking::Client { +/// Cached fast-action client. We store the build result so a TLS init +/// failure surfaces as a recoverable error rather than poisoning the +/// OnceLock and panicking every retry. +fn shared_client() -> Result<&'static reqwest::blocking::Client, String> { use std::sync::OnceLock; - static CLIENT: OnceLock = OnceLock::new(); - CLIENT.get_or_init(|| build_client(FAST_TIMEOUT_SECS)) + static CLIENT: OnceLock> = OnceLock::new(); + match CLIENT.get_or_init(|| build_client(FAST_TIMEOUT_SECS)) { + Ok(c) => Ok(c), + Err(e) => Err(e.clone()), + } } -/// Bytes-action client with a longer timeout, for ReadFileBytes specifically. -fn bytes_client() -> &'static reqwest::blocking::Client { +/// Cached bytes-action client with a longer timeout, for ReadFileBytes. +fn bytes_client() -> Result<&'static reqwest::blocking::Client, String> { use std::sync::OnceLock; - static CLIENT: OnceLock = OnceLock::new(); - CLIENT.get_or_init(|| build_client(BYTES_TIMEOUT_SECS)) + static CLIENT: OnceLock> = OnceLock::new(); + match CLIENT.get_or_init(|| build_client(BYTES_TIMEOUT_SECS)) { + Ok(c) => Ok(c), + Err(e) => Err(e.clone()), + } } /// Pick the right client for the action. Bytes-bearing actions get the /// larger timeout so a slow remote doesn't surface as a generic transport /// error mid-download. -fn client_for(action: &ActionRequest) -> &'static reqwest::blocking::Client { +fn client_for(action: &ActionRequest) -> Result<&'static reqwest::blocking::Client, String> { match action { ActionRequest::ReadFileBytes { .. } => bytes_client(), _ => shared_client(), @@ -62,7 +72,7 @@ pub fn post_action( action: ActionRequest, ) -> Result, String> { let url = format!("http://{}:{}/v1/actions", host, port); - let client = client_for(&action); + let client = client_for(&action)?; let resp = client .post(&url) .bearer_auth(token) diff --git a/crates/okena-files/src/file_viewer/loading.rs b/crates/okena-files/src/file_viewer/loading.rs index 2b36f753..66a1eb39 100644 --- a/crates/okena-files/src/file_viewer/loading.rs +++ b/crates/okena-files/src/file_viewer/loading.rs @@ -205,6 +205,23 @@ pub(super) fn compute_freshness_reload( }) } +/// Map the `image` crate's content-sniffed format onto GPUI's `ImageFormat`. +/// `image::ImageFormat` covers more formats than GPUI knows; we return +/// `None` for anything GPUI can't render so the caller can fall back to +/// the extension-derived format. +fn image_format_from_image_crate(format: image::ImageFormat) -> Option { + Some(match format { + image::ImageFormat::Png => ImageFormat::Png, + image::ImageFormat::Jpeg => ImageFormat::Jpeg, + image::ImageFormat::Gif => ImageFormat::Gif, + image::ImageFormat::WebP => ImageFormat::Webp, + image::ImageFormat::Bmp => ImageFormat::Bmp, + image::ImageFormat::Tiff => ImageFormat::Tiff, + image::ImageFormat::Ico => ImageFormat::Ico, + _ => return None, + }) +} + /// Decode raw image bytes into a `DecodedImage` based on file extension. /// Used by both the initial async load and freshness reloads for image tabs. /// @@ -273,15 +290,25 @@ pub(super) fn build_image_content( } _ => { // Probe intrinsic dimensions without decoding the full pixel - // buffer; image::ImageReader reads only the header. - let (width, height) = image::ImageReader::new(std::io::Cursor::new(&bytes)) + // buffer; image::ImageReader reads only the header. Trust the + // content-derived format over the extension-derived one so a + // `.png` that's actually JPEG bytes (common after "Save As") + // decodes through the right codec rather than failing silently + // inside GPUI's lazy decoder with the user looking at a sized + // but blank "Cannot decode image" box. + let reader = image::ImageReader::new(std::io::Cursor::new(&bytes)) .with_guessed_format() - .map_err(|e| format!("Cannot read image header: {}", e))? + .map_err(|e| format!("Cannot read image header: {}", e))?; + let guessed = reader.format(); + let (width, height) = reader .into_dimensions() .map_err(|e| format!("Cannot read image dimensions: {}", e))?; + let effective_format = guessed + .and_then(image_format_from_image_crate) + .unwrap_or(format); Ok(LoadedContent::Image { decoded: DecodedImage::Raster { - image: Arc::new(Image::from_bytes(format, bytes)), + image: Arc::new(Image::from_bytes(effective_format, bytes)), width, height, }, diff --git a/crates/okena-files/src/file_viewer/mod.rs b/crates/okena-files/src/file_viewer/mod.rs index f20ce582..66f83bac 100644 --- a/crates/okena-files/src/file_viewer/mod.rs +++ b/crates/okena-files/src/file_viewer/mod.rs @@ -909,6 +909,19 @@ impl FileViewer { if this.blame_visible { this.spawn_blame_load_for_active(cx); } + // The freshness reload of an SVG always rebuilds the + // bitmap at rendered_scale=1.0, but image_view.zoom + // persists across the reload. Without this kick the + // user's previously-tuned high-zoom view stays + // pixelated until they touch the wheel. + if let Some(tab) = this + .tabs + .iter() + .find(|t| t.relative_path == relative_path) + && tab.is_svg + { + this.maybe_rerender_svg_for(relative_path.clone(), cx); + } cx.notify(); } }); @@ -926,10 +939,30 @@ impl FileViewer { /// raster if the user has zoomed further in the meantime. pub(super) fn maybe_rerender_svg(&mut self, cx: &mut Context) { let relative_path = self.active_tab().relative_path.clone(); + self.maybe_rerender_svg_for(relative_path, cx); + } + + /// Re-raster the SVG on the tab identified by `relative_path` (not + /// necessarily the active tab). Used both as the entry point from + /// zoom actions (which pass the active tab) and from the apply + /// callback's chase-loop recursion, where the user may have switched + /// tabs mid-raster and `self.active_tab()` would target the wrong + /// path. + pub(super) fn maybe_rerender_svg_for( + &mut self, + relative_path: String, + cx: &mut Context, + ) { let svg_renderer = cx.svg_renderer(); let Some((bytes, target_scale)) = self.compute_rerender_target(&relative_path) else { return; }; + // Capture the source bytes' Arc identity so we can detect — at + // apply time — that image_data was swapped to a different SVG + // (freshness reload, tab-replace) while our raster was in flight. + // Without this guard the stale bitmap would overwrite the fresh + // image, showing pre-edit pixels for the post-edit file. + let dispatched_bytes_id = Arc::as_ptr(&bytes) as usize; if let Some(tab) = self .tabs @@ -948,28 +981,61 @@ impl FileViewer { .map_err(|e| format!("Cannot re-rasterize SVG: {}", e)) }) .await; + let path_for_callback = relative_path.clone(); let _ = entity.update(cx, |this, cx| { + let mut should_chase = false; if let Some(tab) = this .tabs .iter_mut() .find(|t| t.relative_path == relative_path) { tab.image_view.svg_rerender_in_flight = false; - if let Ok(new_image) = result - && let Some(DecodedImage::Rendered { + match (result, tab.image_data.as_mut()) { + (Ok(new_image), Some(DecodedImage::Rendered { image, rendered_scale, + svg_bytes, .. - }) = &mut tab.image_data - { - *image = new_image; - *rendered_scale = target_scale; - cx.notify(); + })) => { + // Discard the result if image_data was + // replaced (different SVG bytes) while we + // were rasterizing — otherwise the stale + // raster would overwrite the new bitmap. + if Arc::as_ptr(svg_bytes) as usize == dispatched_bytes_id { + *image = new_image; + *rendered_scale = target_scale; + cx.notify(); + should_chase = true; + } + // Bytes mismatch — the freshness reload's own + // 1× bitmap is what's now displayed; let any + // subsequent zoom action kick a fresh raster. + } + (Err(_), Some(DecodedImage::Rendered { + rendered_scale, + svg_bytes, + .. + })) => { + // Pin rendered_scale to the failed target so + // compute_rerender_target stops requesting + // the same raster on every chase tick. The + // bitmap stays at its previous resolution + // (the user keeps seeing whatever crisp / + // soft state they had before). Only apply + // this pin to the SVG we dispatched against. + if Arc::as_ptr(svg_bytes) as usize == dispatched_bytes_id { + *rendered_scale = target_scale; + } + } + _ => {} } } - // The user may have kept zooming while we were rasterizing - // — check whether we still need to chase a higher target. - this.maybe_rerender_svg(cx); + // Recurse against the captured relative_path, not the + // active tab — the user may have switched tabs while the + // raster ran. + if should_chase { + this.maybe_rerender_svg_for(path_for_callback, cx); + } }); }) .detach(); diff --git a/crates/okena-files/src/file_viewer/render.rs b/crates/okena-files/src/file_viewer/render.rs index 3aa3be73..84b86368 100644 --- a/crates/okena-files/src/file_viewer/render.rs +++ b/crates/okena-files/src/file_viewer/render.rs @@ -887,7 +887,18 @@ impl FileViewer { .on_scroll_wheel(cx.listener(|this, event: &ScrollWheelEvent, _, cx| { let delta = event.delta.pixel_delta(px(17.0)); let dy = f32::from(delta.y); - if dy.abs() < f32::EPSILON { + // Reject non-finite deltas (a hostile remote bridge or + // momentum-extrapolation overflow can hand us NaN/inf; + // `NaN < EPSILON` is false so the old guard let them + // through and poisoned zoom into NaN). + if !dy.is_finite() { + return; + } + // Filter the macOS kinetic-momentum tail (deltas in the + // 0.01–0.5 range continuing for ~1 s after the user + // stops scrolling — they would otherwise keep nudging + // zoom and respawning the SVG re-raster pipeline). + if dy.abs() < 0.5 { return; } // Exponential: positive dy zooms in, negative out. ±100 px ≈ 1.2× / 0.83×. @@ -905,6 +916,17 @@ impl FileViewer { }), ) .on_mouse_move(cx.listener(|this, event: &MouseMoveEvent, _, cx| { + // If the user released the button outside the container + // we never saw the on_mouse_up — clear sticky pan state + // on the next move that arrives without a pressed Left + // button so the image doesn't follow the cursor with no + // button held. + if event.pressed_button != Some(MouseButton::Left) { + if this.active_tab().image_view.is_panning { + this.image_end_pan(cx); + } + return; + } this.image_update_pan(event.position, cx); })) .on_mouse_up( @@ -1251,6 +1273,10 @@ impl Render for FileViewer { // In source mode the highlighted XML is shown, so selection // / search / copy / select-all all work normally. let is_img_view = is_img && (!is_svg_tab || is_preview); + // Image-zoom chords only apply when the user is actually + // looking at the image (SVG in Source mode shows XML; we + // don't want Cmd+= to silently zoom a hidden preview). + let img_zoom_chord = is_img_view; match key { "f" if modifiers.platform || modifiers.control => { @@ -1298,13 +1324,13 @@ impl Render for FileViewer { "w" if modifiers.platform || modifiers.control => { this.close_active_tab(cx); } - "0" if (modifiers.platform || modifiers.control) && is_img => { + "0" if (modifiers.platform || modifiers.control) && img_zoom_chord => { this.image_fit(cx); } - "=" | "+" if (modifiers.platform || modifiers.control) && is_img => { + "=" | "+" if (modifiers.platform || modifiers.control) && img_zoom_chord => { this.image_zoom_by(1.25, cx); } - "-" if (modifiers.platform || modifiers.control) && is_img => { + "-" if (modifiers.platform || modifiers.control) && img_zoom_chord => { this.image_zoom_by(1.0 / 1.25, cx); } "r" if !modifiers.platform && !modifiers.control => { diff --git a/crates/okena-files/src/file_viewer/selection.rs b/crates/okena-files/src/file_viewer/selection.rs index c968ca62..45b80948 100644 --- a/crates/okena-files/src/file_viewer/selection.rs +++ b/crates/okena-files/src/file_viewer/selection.rs @@ -246,7 +246,12 @@ impl FileViewer { cx.notify(); } - /// Begin a pan drag at the given screen-space position. + /// Record a potential pan drag at `position`. Does NOT promote the + /// view out of auto-fit yet — a single click anywhere on a fit-mode + /// image (e.g. to focus the pane, or the first half of a double-click + /// reset) would otherwise snap the image from Fit to 100% before the + /// user has even moved the mouse. Promotion happens in + /// `image_update_pan` on the first non-zero movement. pub(super) fn image_start_pan( &mut self, position: gpui::Point, @@ -256,19 +261,16 @@ impl FileViewer { if !tab.is_image { return; } - // Panning only matters once we've left auto-fit, so promote to - // explicit zoom on first drag. - if tab.image_view.auto_fit { - tab.image_view.auto_fit = false; - tab.image_view.zoom = 1.0; - } tab.image_view.is_panning = true; tab.image_view.pan_anchor = Some(position); tab.image_view.pan_anchor_offset = tab.image_view.pan; cx.notify(); } - /// Continue a pan drag — translate by (current - anchor). + /// Continue a pan drag — translate by (current - anchor). On the + /// first frame with a non-zero delta we also promote the view out of + /// auto-fit (and zoom to 1.0 if we hadn't left fit yet) so a stationary + /// click that never moved leaves the view untouched. pub(super) fn image_update_pan( &mut self, position: gpui::Point, @@ -281,9 +283,27 @@ impl FileViewer { let Some(anchor) = tab.image_view.pan_anchor else { return; }; + let dx = position.x - anchor.x; + let dy = position.y - anchor.y; + if f32::from(dx) == 0.0 && f32::from(dy) == 0.0 { + return; + } + if tab.image_view.auto_fit { + tab.image_view.auto_fit = false; + tab.image_view.zoom = 1.0; + tab.image_view.pan_anchor_offset = gpui::Point::default(); + } + // Clamp pan magnitude. Without this the user can drag the image + // arbitrarily far off-pane (the container is overflow_hidden, so + // it just vanishes) with no obvious recovery affordance. ±10000 px + // per axis is generous for any realistic display while preventing + // a runaway momentum drift from sending the image to infinity. + const PAN_HARDCAP: f32 = 10_000.0; + let raw_x = f32::from(tab.image_view.pan_anchor_offset.x + dx); + let raw_y = f32::from(tab.image_view.pan_anchor_offset.y + dy); tab.image_view.pan = gpui::Point::new( - tab.image_view.pan_anchor_offset.x + (position.x - anchor.x), - tab.image_view.pan_anchor_offset.y + (position.y - anchor.y), + gpui::px(raw_x.clamp(-PAN_HARDCAP, PAN_HARDCAP)), + gpui::px(raw_y.clamp(-PAN_HARDCAP, PAN_HARDCAP)), ); cx.notify(); } diff --git a/crates/okena-files/src/list_directory.rs b/crates/okena-files/src/list_directory.rs index 3deb1f0d..11cdc58f 100644 --- a/crates/okena-files/src/list_directory.rs +++ b/crates/okena-files/src/list_directory.rs @@ -28,10 +28,26 @@ pub fn list_directory( relative_path: &str, show_ignored: bool, ) -> Result, String> { + // Canonicalize the join target and verify it stays inside the project + // root — `relative_path` may carry `..` segments or even be absolute + // (Path::join with an absolute argument replaces). Without this guard + // a caller passing `../../etc` would list arbitrary directories. The + // read_file / read_file_bytes path enforces the same invariant via + // LocalProjectFs::resolve; this is the listing-side mirror. + let canonical_root = project_root + .canonicalize() + .map_err(|e| format!("Cannot resolve project path: {}", e))?; let target = if relative_path.is_empty() { - project_root.to_path_buf() + canonical_root.clone() } else { - project_root.join(relative_path) + let joined = canonical_root.join(relative_path); + let canonical = joined + .canonicalize() + .map_err(|e| format!("Cannot read directory: {}", e))?; + if !canonical.starts_with(&canonical_root) { + return Err("path traversal not allowed".to_string()); + } + canonical }; let metadata = std::fs::metadata(&target) @@ -39,6 +55,10 @@ pub fn list_directory( if !metadata.is_dir() { return Err(format!("Not a directory: {}", target.display())); } + // Subsequent code references `project_root` — keep using the + // canonical root for ignore overrides so anchored patterns resolve + // the same way as inside resolve(). + let project_root = canonical_root.as_path(); let mut walk_builder = WalkBuilder::new(&target); walk_builder From 762bcfe943ca540c3f14e9a03b5a38138b8211f0 Mon Sep 17 00:00:00 2001 From: jonasnobile Date: Mon, 25 May 2026 17:00:10 +0200 Subject: [PATCH 5/8] fix(files): plug memory leaks in image / SVG / font previews MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GPU and platform-text-system resources allocated by the preview loaders were never released, so a long session of zooming SVGs, reloading externally-edited assets, or iterating in a font editor would accumulate hundreds of MB of memory the user couldn't free without restarting okena. 1. SVG re-raster and freshness reload now call `cx.drop_image(old)` on the previous `Arc` before installing the new one. Each raster tile lives in the GPU sprite atlas and is only removable via that API — dropping the `Arc` alone leaves the tile resident. 2. Raster preview cleanup: the `release_image_assets` helper calls `Image::remove_asset(cx)` for `DecodedImage::Raster` and `cx.drop_image(image, None)` for `DecodedImage::Rendered`. It runs on every replacement site: - apply_loaded_content (spawn_tab_load): captures the previous image and releases it after apply assigns the new one. - apply_freshness_reload (check_active_tab_freshness): same pattern. - close_tab / close_other_tabs / close_all_tabs. - In-place tab replace in open_file_in_tab (both the empty-tab and at-MAX_TABS branches) and navigate_to_file_no_history. 3. Font registration was unbounded: GPUI's `add_fonts` doesn't dedup internally, so every freshness reload of a font tab (and every reopen of the same file) appended another full copy of the bytes to the platform text system's font source. register_font_bytes now gates on a process-wide `HashSet` of byte hashes and short-circuits a re-registration; if the underlying call fails we roll the hash back so a transient error doesn't permanently block a retry. The discarded-stale-raster path in maybe_rerender_svg_for now also drops its own RenderImage instead of leaking it after the bytes-id mismatch check fires. Co-Authored-By: Claude Opus 4.7 --- crates/okena-files/src/file_viewer/mod.rs | 179 +++++++++++++++++++--- 1 file changed, 154 insertions(+), 25 deletions(-) diff --git a/crates/okena-files/src/file_viewer/mod.rs b/crates/okena-files/src/file_viewer/mod.rs index 66f83bac..b6cb2f9c 100644 --- a/crates/okena-files/src/file_viewer/mod.rs +++ b/crates/okena-files/src/file_viewer/mod.rs @@ -897,14 +897,25 @@ impl FileViewer { } // Re-find the tab by relative_path; concurrent reorders/closes // mean the index may have shifted since we scheduled the check. + // Capture the previously-installed image so we can evict its + // sprite-atlas tile / asset-cache entry AFTER apply assigns + // the new one — apply runs on `&mut FileViewerTab` and can't + // see `cx: &mut App`. + let mut old_image: Option = None; if let Some(tab) = this.tabs.iter_mut().find(|t| t.relative_path == relative_path) { + if reloaded { + old_image = tab.image_data.take(); + } tab.apply_freshness_reload(result); if reloaded { tab.blame = BlameLoadState::NotLoaded; } } + if let Some(decoded) = old_image { + release_image_assets(decoded, cx); + } if reloaded { if this.blame_visible { this.spawn_blame_load_for_active(cx); @@ -1002,14 +1013,23 @@ impl FileViewer { // were rasterizing — otherwise the stale // raster would overwrite the new bitmap. if Arc::as_ptr(svg_bytes) as usize == dispatched_bytes_id { - *image = new_image; + // Replace the bitmap, then evict the old + // sprite-atlas tile so the GPU memory + // gets reclaimed (cx.drop_image is the + // only path; the Arc drop on its own + // leaves the tile resident). + let old_image = std::mem::replace(image, new_image); *rendered_scale = target_scale; + cx.drop_image(old_image, None); cx.notify(); should_chase = true; + } else { + // Bytes mismatch — image_data was replaced + // by a freshness reload. The new bitmap + // (the freshness reload's 1× raster) is + // already on the tab; drop our raster. + cx.drop_image(new_image, None); } - // Bytes mismatch — the freshness reload's own - // 1× bitmap is what's now displayed; let any - // subsequent zoom action kick a fresh raster. } (Err(_), Some(DecodedImage::Rendered { rendered_scale, @@ -1113,7 +1133,11 @@ impl FileViewer { // If current tab is empty (no file loaded), replace it if self.active_tab().is_empty() { + let old_image = self.tabs[self.active_tab].image_data.take(); self.tabs[self.active_tab] = new_tab; + if let Some(decoded) = old_image { + release_image_assets(decoded, cx); + } self.spawn_tab_load(relative_path, cx); cx.notify(); return; @@ -1123,34 +1147,49 @@ impl FileViewer { let current = self.active_tab().relative_path.clone(); self.history.push(¤t); - self.active_tab = Self::insert_tab_after_active(&mut self.tabs, self.active_tab, new_tab); + let (new_active, evicted) = + Self::insert_tab_after_active(&mut self.tabs, self.active_tab, new_tab); + self.active_tab = new_active; + // Release any image assets owned by the evicted tab — otherwise + // its sprite-atlas tile / decoded asset cache entry would linger + // after the tab is gone (an SVG that had been zoomed to 16× is + // tens of MB of GPU memory). + if let Some(mut tab) = evicted + && let Some(decoded) = tab.image_data.take() + { + release_image_assets(decoded, cx); + } self.spawn_tab_load(relative_path, cx); cx.notify(); } - /// Insert `new_tab` directly after the active tab and return the new - /// active index (the inserted tab). When already at `MAX_TABS`, the oldest - /// tab is evicted first to make room — never the active tab, so the file - /// the user is looking at is preserved. Evicting a tab before the active - /// one shifts the active index left by one. + /// Insert `new_tab` directly after the active tab and return + /// `(new_active_index, evicted_tab)`. When already at `MAX_TABS`, the + /// oldest tab is evicted first to make room — never the active tab, + /// so the file the user is looking at is preserved. Evicting a tab + /// before the active one shifts the active index left by one. + /// + /// The evicted tab is returned (not dropped here) so the caller can + /// release any GPU-side image assets it owned before letting it drop. fn insert_tab_after_active( tabs: &mut Vec, active: usize, new_tab: FileViewerTab, - ) -> usize { + ) -> (usize, Option) { let mut active = active; + let mut evicted = None; if tabs.len() >= MAX_TABS { // Oldest tab is index 0; skip it only if it's the active tab. let evict = if active == 0 { 1 } else { 0 }; - tabs.remove(evict); + evicted = Some(tabs.remove(evict)); if evict < active { active -= 1; } } let insert_at = active + 1; tabs.insert(insert_at, new_tab); - insert_at + (insert_at, evicted) } /// Mark all ancestor folders of `relative_path` as expanded and ensure @@ -1170,7 +1209,10 @@ impl FileViewer { return; } - self.tabs.remove(index); + let mut removed = self.tabs.remove(index); + if let Some(decoded) = removed.image_data.take() { + release_image_assets(decoded, cx); + } if index == self.active_tab { // Closed the active tab: prefer the tab to the right (same index), @@ -1189,18 +1231,28 @@ impl FileViewer { pub(super) fn close_other_tabs(&mut self, index: usize, cx: &mut Context) { if index < self.tabs.len() { let kept = self.tabs.remove(index); - self.tabs.clear(); + let mut dropped: Vec = self.tabs.drain(..).collect(); self.tabs.push(kept); self.active_tab = 0; + for mut tab in dropped.drain(..) { + if let Some(decoded) = tab.image_data.take() { + release_image_assets(decoded, cx); + } + } cx.notify(); } } /// Close all tabs, leaving an empty viewer state. pub(super) fn close_all_tabs(&mut self, cx: &mut Context) { - self.tabs.clear(); + let mut dropped: Vec = self.tabs.drain(..).collect(); self.tabs.push(FileViewerTab::new_empty()); self.active_tab = 0; + for mut tab in dropped.drain(..) { + if let Some(decoded) = tab.image_data.take() { + release_image_assets(decoded, cx); + } + } cx.notify(); } @@ -1253,7 +1305,11 @@ impl FileViewer { let file_path = Self::resolve_absolute(&self.project_fs, &relative_path); let new_tab = FileViewerTab::new_loading(relative_path.clone(), file_path); + let old_image = self.tabs[self.active_tab].image_data.take(); self.tabs[self.active_tab] = new_tab; + if let Some(decoded) = old_image { + release_image_assets(decoded, cx); + } self.spawn_tab_load(relative_path, cx); cx.notify(); } @@ -1317,6 +1373,7 @@ impl FileViewer { }) .await; let _ = entity.update(cx, |this, cx| { + let mut old_image: Option = None; if let Some(tab) = this.tabs.iter_mut().find(|t| t.relative_path == relative_path) { @@ -1328,17 +1385,26 @@ impl FileViewer { return; } // Register font bytes with the platform text system - // BEFORE applying, so the family name is resolvable by - // the time render runs. add_fonts is idempotent on - // re-registration (same internal name resolves), so - // closing and reopening the same font is cheap. + // BEFORE applying so the family name is resolvable by + // the time render runs. register_font_bytes dedups on + // a byte hash so a re-register of the same font is a + // no-op (without that, GPUI's add_fonts pushes every + // call into the platform font source and never frees). if let Ok(loading::LoadedContent::Font { ttf_bytes, .. }) = &result { register_font_bytes(cx, ttf_bytes); } + // Capture the previously-installed image so we can + // evict its sprite-atlas tile after the new one is in + // place — apply_loaded_content runs on `&mut tab` + // alone and can't see `cx: &mut App`. + old_image = tab.image_data.take(); tab.apply_loaded_content(result, &this.syntax_set, this.is_dark); tab.blame = BlameLoadState::NotLoaded; cx.notify(); } + if let Some(decoded) = old_image { + release_image_assets(decoded, cx); + } if this.blame_visible { this.spawn_blame_load_for_active(cx); } @@ -1380,14 +1446,71 @@ pub enum FileViewerEvent { SendToTerminal(okena_core::send_payload::SendPayload), } +/// Release the GPU-side cache entries backing a `DecodedImage` before the +/// owning `Arc` is dropped. +/// +/// Without this, replacing `image_data` (on tab close, freshness reload, +/// or an SVG re-raster) only drops the CPU-side `Arc`; the corresponding +/// sprite-atlas tile / asset-cache entry stays resident in GPU memory. +/// Repeatedly zooming an SVG (which kicks a fresh rasterization per 1.1× +/// past the rendered scale) and external-edit cycles can each leak tens +/// of MB of GPU memory over a session if this isn't called. +pub(super) fn release_image_assets(decoded: DecodedImage, cx: &mut App) { + match decoded { + DecodedImage::Raster { image, .. } => { + // `Image::remove_asset` removes the decoded `RenderImage` + // produced by `ImageDecoder` from the asset cache; the + // underlying sprite-atlas tile is dropped along with it. + image.remove_asset(cx); + } + DecodedImage::Rendered { image, .. } => { + // Rendered SVG bitmaps live as `Arc` directly in + // the sprite atlas; `cx.drop_image` is the only path that + // removes them across all windows. + cx.drop_image(image, None); + } + } +} + /// Register a font's OpenType bytes with the active text system so any /// subsequent render of `Font { family: family_name, .. }` resolves to it. -/// GPUI's `add_fonts` is idempotent on a re-registration (same internal -/// name resolves), so this is safe to call on every freshness reload. +/// +/// GPUI's `add_fonts` does NOT dedup internally — every call pushes a +/// fresh `Handle::from_memory` into the platform text system's font +/// source. Without our own gate, every freshness reload of a font tab +/// (and every reopen of the same file) leaks the full payload again. +/// We hash the bytes and short-circuit if we've already registered an +/// identical font this session. fn register_font_bytes(cx: &mut App, ttf_bytes: &Arc>) { + use std::collections::HashSet; + use std::hash::{Hash, Hasher}; + use std::sync::{Mutex, OnceLock}; + static REGISTERED: OnceLock>> = OnceLock::new(); + + let registered = REGISTERED.get_or_init(|| Mutex::new(HashSet::new())); + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + ttf_bytes.as_ref().hash(&mut hasher); + let hash = hasher.finish(); + { + let mut guard = match registered.lock() { + Ok(g) => g, + Err(p) => p.into_inner(), + }; + if !guard.insert(hash) { + return; + } + } + let bytes: Vec = ttf_bytes.as_ref().clone(); if let Err(e) = cx.text_system().add_fonts(vec![std::borrow::Cow::Owned(bytes)]) { log::warn!("Failed to register font with text system: {}", e); + // Roll back the dedup entry so a transient registration failure + // doesn't permanently block a retry from re-attempting it. + let mut guard = match registered.lock() { + Ok(g) => g, + Err(p) => p.into_inner(), + }; + guard.remove(&hash); } } @@ -1420,8 +1543,10 @@ mod tests { #[::core::prelude::v1::test] fn insert_tab_below_limit_inserts_after_active() { let mut tabs = vec![tab("a"), tab("b"), tab("c")]; - let active = FileViewer::insert_tab_after_active(&mut tabs, 0, tab("new")); + let (active, evicted) = + FileViewer::insert_tab_after_active(&mut tabs, 0, tab("new")); assert_eq!(active, 1); + assert!(evicted.is_none()); assert_eq!(paths(&tabs), ["a", "new", "b", "c"]); } @@ -1430,10 +1555,12 @@ mod tests { let mut tabs: Vec = (0..MAX_TABS).map(|i| tab(&format!("f{i}"))).collect(); // Active is somewhere in the middle. - let active = FileViewer::insert_tab_after_active(&mut tabs, 10, tab("new")); + let (active, evicted) = + FileViewer::insert_tab_after_active(&mut tabs, 10, tab("new")); assert_eq!(tabs.len(), MAX_TABS); // Oldest (index 0) was evicted; everything shifted left by one, so the // active file f10 stays active and the new tab lands right after it. + assert_eq!(evicted.expect("oldest tab returned").relative_path, "f0"); assert_eq!(tabs[0].relative_path, "f1"); assert_eq!(tabs[active - 1].relative_path, "f10"); assert_eq!(tabs[active].relative_path, "new"); @@ -1444,10 +1571,12 @@ mod tests { let mut tabs: Vec = (0..MAX_TABS).map(|i| tab(&format!("f{i}"))).collect(); // Active IS the oldest tab — must not evict it. - let active = FileViewer::insert_tab_after_active(&mut tabs, 0, tab("new")); + let (active, evicted) = + FileViewer::insert_tab_after_active(&mut tabs, 0, tab("new")); assert_eq!(tabs.len(), MAX_TABS); assert_eq!(active, 1); // f0 (active) preserved at index 0; f1 (next oldest) evicted. + assert_eq!(evicted.expect("next-oldest tab returned").relative_path, "f1"); assert_eq!(tabs[0].relative_path, "f0"); assert_eq!(tabs[1].relative_path, "new"); assert_eq!(tabs[2].relative_path, "f2"); From d88029c8b286bdbe2801b9c39a78582661df054f Mon Sep 17 00:00:00 2001 From: jonasnobile Date: Mon, 25 May 2026 17:25:57 +0200 Subject: [PATCH 6/8] fix(files): factor boxed Fn into type alias to satisfy clippy::type_complexity CI's clippy at rust 1.93 flagged the inline `Box)>` parameter in image_zoom_controls. Pull it out as `ZoomButtonHandler` so the helper's signature stays readable. Co-Authored-By: Claude Opus 4.7 --- crates/okena-files/src/file_viewer/render.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/okena-files/src/file_viewer/render.rs b/crates/okena-files/src/file_viewer/render.rs index 84b86368..2d59da44 100644 --- a/crates/okena-files/src/file_viewer/render.rs +++ b/crates/okena-files/src/file_viewer/render.rs @@ -947,6 +947,10 @@ impl FileViewer { } } +/// Click handler captured by the small `−` / `+` chiclets — boxed so the +/// helper can take heterogeneous closures by value. +type ZoomButtonHandler = Box)>; + /// Header chiclet: `−` / `Fit | 100%` / `+`. Click on the label toggles /// between Fit and 100%. Buttons step zoom by 1.25× / 0.8×. fn image_zoom_controls( @@ -954,7 +958,11 @@ fn image_zoom_controls( t: &ThemeColors, cx: &mut Context, ) -> impl IntoElement { - let button = move |id: &'static str, glyph: &'static str, t: &ThemeColors, cx: &mut Context, on_click: Box)>| { + let button = move |id: &'static str, + glyph: &'static str, + t: &ThemeColors, + cx: &mut Context, + on_click: ZoomButtonHandler| { let t_text_muted = t.text_muted; let t_bg_hover = t.bg_hover; div() From 2491ecd9c17d6c43e94b079ad3b426018dc86acc Mon Sep 17 00:00:00 2001 From: jonasnobile Date: Wed, 27 May 2026 10:18:18 +0200 Subject: [PATCH 7/8] =?UTF-8?q?feat(files):=20split=20image-preview=20whee?= =?UTF-8?q?l=20=E2=80=94=20Cmd/Ctrl=20scrolls=20zoom,=20plain=20scroll=20p?= =?UTF-8?q?ans?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plain wheel scroll now pans the image (matching how every other image viewer behaves) and Cmd/Ctrl+wheel keeps the zoom behaviour. Previously the wheel always zoomed, which was discoverable but surprising — especially with momentum trackpads where a stray gesture would jump the zoom level. Auto-fit mode short-circuits plain scroll (the image already fills the pane, nothing to pan to); the Cmd/Ctrl zoom path still promotes out of fit normally. The 0.5 px momentum-tail filter applies to both branches. New `image_pan_by(delta)` honours the same ±10 000 px hard-cap as the drag-pan path so a runaway momentum scroll can't send the image off-pane. Co-Authored-By: Claude Opus 4.7 --- crates/okena-files/src/file_viewer/render.rs | 40 +++++++++++++------ .../okena-files/src/file_viewer/selection.rs | 23 +++++++++++ 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/crates/okena-files/src/file_viewer/render.rs b/crates/okena-files/src/file_viewer/render.rs index 2d59da44..f87e3e8c 100644 --- a/crates/okena-files/src/file_viewer/render.rs +++ b/crates/okena-files/src/file_viewer/render.rs @@ -886,24 +886,40 @@ impl FileViewer { container .on_scroll_wheel(cx.listener(|this, event: &ScrollWheelEvent, _, cx| { let delta = event.delta.pixel_delta(px(17.0)); + let dx = f32::from(delta.x); let dy = f32::from(delta.y); // Reject non-finite deltas (a hostile remote bridge or // momentum-extrapolation overflow can hand us NaN/inf; - // `NaN < EPSILON` is false so the old guard let them - // through and poisoned zoom into NaN). - if !dy.is_finite() { + // those would otherwise poison zoom into NaN). + if !dx.is_finite() || !dy.is_finite() { return; } - // Filter the macOS kinetic-momentum tail (deltas in the - // 0.01–0.5 range continuing for ~1 s after the user - // stops scrolling — they would otherwise keep nudging - // zoom and respawning the SVG re-raster pipeline). - if dy.abs() < 0.5 { - return; + let zoom_modifier = event.modifiers.platform || event.modifiers.control; + if zoom_modifier { + // Filter the kinetic-momentum tail (sub-pixel deltas + // continuing for ~1 s after the user stopped) — they + // would otherwise keep nudging zoom and respawning + // the SVG re-raster pipeline. + if dy.abs() < 0.5 { + return; + } + // Exponential: positive dy zooms in, negative out. + // ±100 px ≈ 1.2× / 0.83×. + let factor = (dy / 250.0).exp(); + this.image_zoom_by(factor, cx); + } else { + // Plain scroll = classic pan. Skip on auto-fit (the + // image already fills the pane and there's nothing + // to reveal) so a stray trackpad gesture doesn't + // accidentally promote out of fit mode. + if this.active_tab().image_view.auto_fit { + return; + } + if dx.abs() < 0.5 && dy.abs() < 0.5 { + return; + } + this.image_pan_by(gpui::point(px(dx), px(dy)), cx); } - // Exponential: positive dy zooms in, negative out. ±100 px ≈ 1.2× / 0.83×. - let factor = (dy / 250.0).exp(); - this.image_zoom_by(factor, cx); })) .on_mouse_down( MouseButton::Left, diff --git a/crates/okena-files/src/file_viewer/selection.rs b/crates/okena-files/src/file_viewer/selection.rs index 45b80948..827fdd59 100644 --- a/crates/okena-files/src/file_viewer/selection.rs +++ b/crates/okena-files/src/file_viewer/selection.rs @@ -308,6 +308,29 @@ impl FileViewer { cx.notify(); } + /// Translate the image by `delta` pixels — used by classic wheel-scroll + /// (without the Cmd/Ctrl zoom modifier). Honors the same hard-cap that + /// drag-pan does so a runaway momentum scroll can't send the image off + /// to infinity. + pub(super) fn image_pan_by( + &mut self, + delta: gpui::Point, + cx: &mut Context, + ) { + let tab = self.active_tab_mut(); + if !tab.is_image { + return; + } + const PAN_HARDCAP: f32 = 10_000.0; + let raw_x = f32::from(tab.image_view.pan.x + delta.x); + let raw_y = f32::from(tab.image_view.pan.y + delta.y); + tab.image_view.pan = gpui::Point::new( + gpui::px(raw_x.clamp(-PAN_HARDCAP, PAN_HARDCAP)), + gpui::px(raw_y.clamp(-PAN_HARDCAP, PAN_HARDCAP)), + ); + cx.notify(); + } + /// End a pan drag. pub(super) fn image_end_pan(&mut self, cx: &mut Context) { let tab = self.active_tab_mut(); From 16ca16e95480b85178cd5c1ff6c8cc1449dd753c Mon Sep 17 00:00:00 2001 From: jonasnobile Date: Wed, 27 May 2026 10:39:49 +0200 Subject: [PATCH 8/8] feat(files): shift+scroll pans the image horizontally Standard mouse-wheel convention for users without a tilt wheel or horizontal trackpad gesture. Trackpads that already report dx natively (macOS, modern Linux) take the dx path; only when shift is held AND dx is sub-pixel do we remap vertical wheel input to horizontal pan. Co-Authored-By: Claude Opus 4.7 --- crates/okena-files/src/file_viewer/render.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/okena-files/src/file_viewer/render.rs b/crates/okena-files/src/file_viewer/render.rs index f87e3e8c..19207074 100644 --- a/crates/okena-files/src/file_viewer/render.rs +++ b/crates/okena-files/src/file_viewer/render.rs @@ -915,10 +915,18 @@ impl FileViewer { if this.active_tab().image_view.auto_fit { return; } - if dx.abs() < 0.5 && dy.abs() < 0.5 { + // Shift+scroll maps vertical wheel to horizontal pan — + // standard mouse-wheel convention for users without a + // tilt wheel or horizontal trackpad gesture. + let (pan_dx, pan_dy) = if event.modifiers.shift && dx.abs() < 0.5 { + (dy, 0.0) + } else { + (dx, dy) + }; + if pan_dx.abs() < 0.5 && pan_dy.abs() < 0.5 { return; } - this.image_pan_by(gpui::point(px(dx), px(dy)), cx); + this.image_pan_by(gpui::point(px(pan_dx), px(pan_dy)), cx); } })) .on_mouse_down(