Add image, SVG, and font previews to the file viewer#129
Open
jonasnobile wants to merge 6 commits into
Open
Conversation
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 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
…r scales - 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 <noreply@anthropic.com>
jonasnobile
force-pushed
the
feat/file-viewer-asset-previews
branch
from
2b7f0d3 to
deb881b
Compare
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 <noreply@anthropic.com>
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<RenderImage>` 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<u64>` 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 <noreply@anthropic.com>
…omplexity CI's clippy at rust 1.93 flagged the inline `Box<dyn Fn(&mut FileViewer, &mut Context<FileViewer>)>` parameter in image_zoom_controls. Pull it out as `ZoomButtonHandler` so the helper's signature stays readable. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends the file viewer beyond text to handle three new asset kinds, with a Preview/Source toggle for SVG and a dedicated preview pane for fonts.
Images
gpui::img()withObjectFit::Contain. A.with_fallback(...)element surfaces decode failures (corrupt, truncated, 0-byte, mis-extensioned) instead of the silent blank pane GPUI defaults to.cx.svg_renderer().render_single_frame(.., to_bgra=true). GPUI's built-inImage::from_bytes(ImageFormat::Svg, _)path calls the renderer withto_bgra=false, leaving R/B swapped and the preview inverted — Zed itself never uses that path for SVG either.Fonts
ttf-parser(family, full name, style, weight class, italic, glyph count, units per em, version).woff2-patched— upstreamwoff20.3 fails to compile against currentsafer-bytes/bytesversions.cx.text_system().add_fonts(..)so the preview pane renders an actual pangram in the font across 5 sizes (48/32/24/18/14 px) above a metadata table.Wire-level additions
ProjectFs::read_file_bytes(raw bytes alongsideread_file's UTF-8 path).LocalProjectFs::resolve()canonicalizes and rejects path-traversal, matching the server-sideresolve_project_file.ActionRequest::ReadFileBytesreturns base64 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 so large images on slow links surface as size/timeout, not a generic transport error.Lifecycle hardening
A multi-angle code review surfaced 15 issues; all fixed:
FreshnessOutcome::Failedcarriesnew_mtimeso a failing reload doesn't get re-attempted every throttle tick.apply_loaded_contentreads mtime only whenfile_path.is_absolute()— for remote tabs that path is synthetic, sostd::fs::metadataagainst it would either fail or coincidentally match an unrelated local file.spawn_tab_loadstamps each load with a viewer-level generation counter; stale results from earlier loads for the samerelative_pathare dropped on apply.load_filecommitsmodified_atonly after a successful decode and resetsimage_dataon every error path.apply_freshness_reloadalways overwrites the source fields so asource=Nonereload (SVG with non-UTF-8 bytes) doesn't leave stale XML visible in Source mode.Test plan
cargo build— green on macOS../../etc/passwdvia any caller of LocalProjectFs returns "path traversal not allowed"🤖 Generated with Claude Code