From 6ec80439345e63e764315a7c436dd6c701b006f0 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Fri, 16 Oct 2026 12:04:05 +0200 Subject: [PATCH] fix(gui): decode viewer photos and read their EXIF off the main thread The viewer decoded each full-size photo, read its EXIF and stat'ed it on the GTK main thread, freezing the app for a large photo. That now runs in gio::spawn_blocking, and a result for a photo the user has left is dropped (B242). --- crates/pdfs-gui/src/app/pages/photo_viewer.rs | 49 ++++++++++++++----- docs/BUGS.md | 22 +++++++++ 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/crates/pdfs-gui/src/app/pages/photo_viewer.rs b/crates/pdfs-gui/src/app/pages/photo_viewer.rs index fba9fe0..05460a6 100644 --- a/crates/pdfs-gui/src/app/pages/photo_viewer.rs +++ b/crates/pdfs-gui/src/app/pages/photo_viewer.rs @@ -275,14 +275,42 @@ pub(crate) fn load_photo(ui: &Rc, viewer: &Rc, uid: String) { viewer.status.set_label(message); viewer.status.set_visible(true); }; - match result { - Ok(Ok(Response::FilePath { path })) if is_video => { + let path = match result { + Ok(Ok(Response::FilePath { path })) => path, + Ok(Ok(Response::Error { message, .. })) => return fail(&message), + Ok(Ok(_)) => return fail(&gettext("Unexpected reply from the Proton Drive service.")), + Ok(Err(_)) | Err(_) => return fail(&gettext("Couldn't reach Proton Drive.")), + }; + // Decoding a full-size photo and reading its EXIF take a good part of + // a second for a large one, which froze the whole app on the main + // thread. Done on a worker; a reply for a photo the user has since + // left is dropped like the request's own. + let loaded = { + let path = path.clone(); + gio::spawn_blocking(move || { + let size = std::fs::metadata(&path).ok().map(|meta| meta.len()); + if is_video { + return (size, None, ExifInfo::default()); + } + let texture = gtk4::gdk::Texture::from_filename(&path); + (size, Some(texture), read_exif(&path)) + }) + .await + }; + if *viewer.uid.borrow() != uid { + return; + } + let Ok((size, texture, info)) = loaded else { + return fail(&gettext("Couldn't render this photo.")); + }; + match texture { + None => { viewer.video.set_filename(Some(&path)); viewer.media.set_visible_child_name("video"); *viewer.path.borrow_mut() = Some(path.clone()); - show_info(&viewer, &path, ExifInfo::default()); + show_info(&viewer, size, ExifInfo::default()); } - Ok(Ok(Response::FilePath { path })) => match gtk4::gdk::Texture::from_filename(&path) { + Some(texture) => match texture { Ok(texture) => { // Zoomed in on the thumbnail: keep the photo the size it is // on screen, now drawn from the full-size pixels. @@ -297,16 +325,13 @@ pub(crate) fn load_photo(ui: &Rc, viewer: &Rc, uid: String) { zoom_to(&viewer, Some(zoom), None); } *viewer.path.borrow_mut() = Some(path.clone()); - show_info(&viewer, &path, read_exif(&path)); + show_info(&viewer, size, info); } Err(e) => { tracing::error!("Failed to load texture for {path}: {e}"); fail(&gettext("Couldn't render this photo.")); } }, - Ok(Ok(Response::Error { message, .. })) => fail(&message), - Ok(Ok(_)) => fail(&gettext("Unexpected reply from the Proton Drive service.")), - Ok(Err(_)) | Err(_) => fail(&gettext("Couldn't reach Proton Drive.")), } }); } @@ -452,16 +477,16 @@ pub(crate) fn clear_info(viewer: &Rc) { } /// Fill the details drawer for the photo now on screen: its own file facts -/// (size, dimensions) plus whatever EXIF it carries. -pub(crate) fn show_info(viewer: &Rc, path: &str, info: ExifInfo) { +/// (`size`, dimensions) plus whatever EXIF it carries. +pub(crate) fn show_info(viewer: &Rc, size: Option, info: ExifInfo) { while let Some(row) = viewer.info_rows.first_child() { viewer.info_rows.remove(&row); } let mut fields: Vec<(String, String)> = Vec::new(); - if let Ok(meta) = std::fs::metadata(path) { + if let Some(size) = size { // Translators: label of a photo's file size in the details panel. - fields.push((pgettext("property", "Size"), human_bytes(meta.len()))); + fields.push((pgettext("property", "Size"), human_bytes(size))); } fields.extend(info.fields); diff --git a/docs/BUGS.md b/docs/BUGS.md index f66e334..784f9a9 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -100,6 +100,7 @@ a live account since. | Entry | Title | |---|---| +| [B242](#b242--opening-a-large-photo-freezes-the-app) | Opening a large photo freezes the app | | [B241](#b241--every-closed-photo-viewer-stays-in-memory-with-its-photo) | Every closed photo viewer stays in memory with its photo | | [B240](#b240--pdfs-mount-can-be-core-dumped-with-the-keys-in-memory) | pdfs mount can be core-dumped with the keys in memory | | [B239](#b239----wait-reports-success-for-imports-and-re-dates-that-failed) | --wait reports success for imports and re-dates that failed | @@ -344,6 +345,27 @@ a live account since. Newest first. +### B242 — Opening a large photo freezes the app + +**Status:** Fixed, not verified live +**Found:** 2026-10-16, architecture audit (`FINDINGS.md` 2.5.1). +**Where:** `load_photo` and `show_info` in `crates/pdfs-gui/src/app/pages/photo_viewer.rs` + +**Repro:** Open a 50-megapixel photo in the viewer and press → repeatedly while each one loads. + +**Cause.** When the full-size file arrived, the viewer decoded it with +`Texture::from_filename`, read its EXIF and stat'ed it on the GTK main thread. For a large photo +that took a good part of a second each time, and the whole app froze for it: no redraw, no +input, no other page. + +**Fix.** The decode, the EXIF read and the stat run on a worker (`gio::spawn_blocking`). The +result is used only if the viewer still shows that photo; a reply for one the user has left is +dropped. + +**Test:** none automated; the GUI tests run without a display. + +--- + ### B241 — Every closed photo viewer stays in memory with its photo **Status:** Fixed, not verified live