From 7151cf5d1d9f129b59fdc1788cb863e8dd0922a6 Mon Sep 17 00:00:00 2001 From: Sophie Herold Date: Mon, 22 Dec 2025 01:36:28 +0100 Subject: [PATCH] image_page: Specific error for file loading Give a specific error message if loading the source file fails. Closes #487 --- Cargo.lock | 9 +- Cargo.toml | 5 +- .../50.alpha/added-error-message-file-loading | 1 + src/decoder.rs | 57 ++++---- src/widgets/image.rs | 15 +- src/widgets/image/loading.rs | 24 ++-- src/widgets/image_page.rs | 129 +++++++++--------- src/widgets/image_view.rs | 5 +- 8 files changed, 123 insertions(+), 122 deletions(-) create mode 100644 news.d/50.alpha/added-error-message-file-loading diff --git a/Cargo.lock b/Cargo.lock index 8612a2c..e425196 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -881,8 +881,7 @@ dependencies = [ [[package]] name = "glycin" version = "3.0.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "502ac59dc785b95521e7deea20e292f3501a11752600c4f37cf34676f794b4e3" +source = "git+https://gitlab.gnome.org/GNOME/glycin#f34faa00c480b7809ceb8503ec85282446206277" dependencies = [ "async-fs", "async-global-executor", @@ -918,8 +917,7 @@ dependencies = [ [[package]] name = "glycin-common" version = "1.0.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6e9b31c7fa5ccb9d91317c956e767daef59be9fee3ae9df30468470368678482" +source = "git+https://gitlab.gnome.org/GNOME/glycin#f34faa00c480b7809ceb8503ec85282446206277" dependencies = [ "bitflags", "gufo-common", @@ -937,8 +935,7 @@ dependencies = [ [[package]] name = "glycin-utils" version = "4.0.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7dfec0cd29d5ecfb1dd723868e22e2c49441020f35d5f14307018ff0a1679e2f" +source = "git+https://gitlab.gnome.org/GNOME/glycin#f34faa00c480b7809ceb8503ec85282446206277" dependencies = [ "async-lock", "bitflags", diff --git a/Cargo.toml b/Cargo.toml index b95ba5e..816a9ec 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,7 +27,10 @@ chrono = { version = "0.4.38", default-features = false, features = ["std"] } gettext-rs = { version = "0.7.0", features = ["gettext-system"] } gettext-sys = { version = "0.26.0", features = ["gettext-system"] } gio = { version = "0.21.0", features = ["v2_74"] } -glycin = { version = "3.0.4", features = ["gdk4", "unstable-config"] } +glycin = { version = "3.0.4", features = [ + "gdk4", + "unstable-config", +], git = "https://gitlab.gnome.org/GNOME/glycin" } gtk = { package = "gtk4", version = "0.10.0", features = [ "v4_14", "xml_validation", diff --git a/news.d/50.alpha/added-error-message-file-loading b/news.d/50.alpha/added-error-message-file-loading new file mode 100644 index 0000000..4a96cf7 --- /dev/null +++ b/news.d/50.alpha/added-error-message-file-loading @@ -0,0 +1 @@ +Add a specific error message when loading the image file fails. Usually when the file does not exists or there are no rights to access the file. diff --git a/src/decoder.rs b/src/decoder.rs index 5562245..eb70027 100644 --- a/src/decoder.rs +++ b/src/decoder.rs @@ -59,32 +59,21 @@ pub enum DecoderUpdate { Redraw, /// Start animation Animated, - Error(DecoderError), + Error(Option), } -#[derive(Debug, Clone, Default, PartialEq, Eq)] +#[derive(Debug, Clone)] pub enum DecoderError { /// Image format not supported or unknown - UnsupportedFormat, + UnsupportedFormat(String, glycin::ErrorCtx), /// No glycin-loaders installed - NoLoadersConfigured, + NoLoadersConfigured(glycin::ErrorCtx), /// Memory limit exceeded - OutOfMemory, + OutOfMemory(glycin::ErrorCtx), + /// Failed to load image + ImageSource(glib::Error, glycin::ErrorCtx), - Generic(String), - //ImageSource(String), - #[default] - None, -} - -impl DecoderError { - pub fn is_err(&self) -> bool { - matches!(self, Self::None) - } - - pub fn is_ok(&self) -> bool { - !matches!(self, Self::None) - } + Generic(glycin::ErrorCtx), } impl UpdateSender { @@ -116,27 +105,34 @@ impl UpdateSender { } pub fn send_error(&self, err: glycin::ErrorCtx) { - dbg!(&err); if let Some(mime_type) = err.unsupported_format() { let mut metadata = Metadata::default(); - metadata.set_mime_type(mime_type); + metadata.set_mime_type(mime_type.clone()); self.send(DecoderUpdate::Metadata(Box::new(metadata))); - self.send(DecoderUpdate::Error(DecoderError::UnsupportedFormat)); - } - if err.is_out_of_memory() { - self.send(DecoderUpdate::Error(DecoderError::OutOfMemory)); + self.send(DecoderUpdate::Error(Some(DecoderError::UnsupportedFormat( + mime_type, err, + )))); + } else if err.is_out_of_memory() { + self.send(DecoderUpdate::Error(Some(DecoderError::OutOfMemory(err)))); } else if matches!(err.error(), glycin::Error::NoLoadersConfigured(_)) { - self.send(DecoderUpdate::Error(DecoderError::NoLoadersConfigured)); + self.send(DecoderUpdate::Error(Some( + DecoderError::NoLoadersConfigured(err), + ))); + } else if let glycin::Error::ImageSource(glib_err) = err.error() { + self.send(DecoderUpdate::Error(Some(DecoderError::ImageSource( + glib_err.clone(), + err, + )))); + } else { + self.send(DecoderUpdate::Error(Some(DecoderError::Generic(err)))); } - self.send(DecoderUpdate::Error(DecoderError::Generic(err.to_string()))); } } #[derive(Debug)] pub struct Decoder { decoder: FormatDecoder, - update_sender: UpdateSender, } #[derive(Debug)] @@ -171,7 +167,6 @@ impl Decoder { return ( Self { decoder: FormatDecoder::Failed, - update_sender, }, receiver, ); @@ -182,7 +177,6 @@ impl Decoder { let decoder = Self { decoder: format_decoder, - update_sender, }; (decoder, receiver) @@ -209,8 +203,7 @@ impl Decoder { pub fn request(&self, tile_request: TileRequest) { if let FormatDecoder::Svg(svg) = &self.decoder { if let Err(err) = svg.request(tile_request) { - self.update_sender - .send(DecoderUpdate::Error(DecoderError::Generic(err.to_string()))); + tracing::error!("Failed to request new SVG tiles: {err}"); } } } diff --git a/src/widgets/image.rs b/src/widgets/image.rs index 218e608..c25d253 100644 --- a/src/widgets/image.rs +++ b/src/widgets/image.rs @@ -54,6 +54,7 @@ use glib::subclass::Signal; use glib::{Properties, SignalGroup}; use gufo_common::orientation::Orientation; +use crate::decoder::tiling::SharedFrameBuffer; use crate::decoder::{self, Decoder, DecoderUpdate, tiling}; use crate::deps::*; use crate::metadata::Metadata; @@ -137,13 +138,11 @@ pub enum FitMode { ExactVolatile, } -mod imp { - use decoder::DecoderError; - use glycin::Operations; +use decoder::DecoderError; +use glycin::Operations; +mod imp { use super::*; - use crate::decoder::tiling::SharedFrameBuffer; - use crate::editing; #[derive(Debug, Default, Properties)] #[properties(wrapper_type = super::LpImage)] @@ -155,9 +154,9 @@ mod imp { #[property(get)] pub(super) is_deleted: Cell, /// Set if an error has occurred, shown on error_page - pub(super) specific_error: RefCell, + pub(super) specific_error: RefCell>, #[property(get=Self::is_error)] - pub(super) is_error: Cell, + pub(super) is_error: PhantomData, /// Set to true when image is ready for displaying #[property(get)] pub(super) is_loaded: Cell, @@ -257,7 +256,7 @@ mod imp { pub(super) nth_snapshot: Cell, /// Editing queue - pub(super) editing_queue: editing::Queue, + pub(super) editing_queue: crate::editing::Queue, pub(super) operations: RefCell>>, } diff --git a/src/widgets/image/loading.rs b/src/widgets/image/loading.rs index 632f06d..ea9c6c4 100644 --- a/src/widgets/image/loading.rs +++ b/src/widgets/image/loading.rs @@ -268,7 +268,7 @@ impl imp::LpImage { } pub(super) fn is_error(&self) -> bool { - self.specific_error.borrow().is_err() + self.specific_error.borrow().is_some() } fn set_loaded(&self, is_loaded: bool) { @@ -279,7 +279,7 @@ impl imp::LpImage { obj.notify_is_loaded(); if is_loaded { - obj.set_specific_error(DecoderError::None); + obj.set_specific_error(None); } } } @@ -357,26 +357,24 @@ impl LpImage { imp.decoder.replace(Some(Arc::new(decoder))); } - pub fn specific_error(&self) -> DecoderError { + pub fn specific_error(&self) -> Option { self.imp().specific_error.borrow().clone() } - pub fn set_specific_error(&self, error: DecoderError) { + pub fn set_specific_error(&self, error: Option) { let imp = self.imp(); - // Keeping first error instead of replacing with new error - if error.is_err() && self.is_error() { + // Keeping first error instead of replacing with new error or return + if error.is_some() == self.is_error() { return; } - if self.specific_error() != error { - let is_err = error.is_err(); - imp.specific_error.replace(error); - self.notify_is_error(); + let is_err = error.is_some(); + imp.specific_error.replace(error); + self.notify_is_error(); - if is_err { - imp.set_loaded(false); - } + if is_err { + imp.set_loaded(false); } } } diff --git a/src/widgets/image_page.rs b/src/widgets/image_page.rs index ad20018..eb00471 100644 --- a/src/widgets/image_page.rs +++ b/src/widgets/image_page.rs @@ -214,72 +214,79 @@ impl LpImagePage { let imp = self.imp(); let image = self.image(); - match image.specific_error() { - DecoderError::NoLoadersConfigured => { - // Translators: {} is replaced with a version number - let message = gettext_f( - "No image loaders available. Maybe the “glycin-loaders” package with compatibility version “{}” is not installed.", - [format!("{}+", glycin::COMPAT_VERSION)], - ); + if let Some(error) = image.specific_error() { + let details; + let description; - imp.error_page.set_description(Some(&message)); - imp.stack.set_visible_child(&*imp.error_page); - imp.error_more_info.set_visible(false); - } - DecoderError::UnsupportedFormat => { - let mime_type = image.metadata().mime_type().unwrap_or_default(); - let content_type = gio::content_type_from_mime_type(&mime_type).unwrap_or_default(); - let description = gio::content_type_get_description(&content_type).to_string(); + match error { + DecoderError::NoLoadersConfigured(err) => { + // Translators: {} is replaced with a version number + description = gettext_f( + "No image loaders available. Maybe the “glycin-loaders” package with compatibility version “{}” is not installed.", + [format!("{}+", glycin::COMPAT_VERSION)], + ); - let message = if glycin::Loader::DEFAULT_MIME_TYPES.contains(&mime_type.as_str()) { - // Translators: The first occurance of {} is replace with a description of the - // format and the second with an id (mime-type) of the format. - gettext_f( - "The image format “{} ({})” is known but not installed.", - [&description, &mime_type], - ) - } else { - // Translators: The first occurance of {} is replace with a description of the - // format and the second with an id (mime-type) of the format. - gettext_f( - "Unknown image format “{} ({}).”", - [&description, &mime_type], - ) - }; - - imp.error_more_info.connect_clicked(glib::clone!( - #[weak(rename_to = obj)] - self, - move |_| { - LpErrorDetails::new(&obj.root().unwrap(), &err, ErrorType::Loader); - } - )); - - imp.error_page.set_description(Some(&message)); - imp.stack.set_visible_child(&*imp.error_page); - } - DecoderError::Generic(err) => { - imp.error_more_info.connect_clicked(glib::clone!( - #[weak(rename_to = obj)] - self, - move |_| { - LpErrorDetails::new(&obj.root().unwrap(), &err, ErrorType::Loader); - } - )); - - if matches!(image.specific_error(), DecoderError::OutOfMemory) { - imp.error_page.set_description(Some(&gettext( - "There is not enough free memory available to load this image.", - ))); - } else { - imp.error_page.set_description(Some(&gettext( - "Either the image file is corrupted or it contains unsupported elements.", - ))); + details = err.to_string(); } + DecoderError::UnsupportedFormat(mime_type, err) => { + let content_type = + gio::content_type_from_mime_type(&mime_type).unwrap_or_default(); + let mime_type_description = + gio::content_type_get_description(&content_type).to_string(); - imp.error_more_info.set_visible(true); - imp.stack.set_visible_child(&*imp.error_page); + description = + if glycin::Loader::DEFAULT_MIME_TYPES.contains(&mime_type.as_str()) { + // Translators: The first occurance of {} is replace with a description + // of the format and the second with an id + // (mime-type) of the format. + gettext_f( + "The image format “{} ({})” is known but not installed.", + [&mime_type_description, &mime_type], + ) + } else { + // Translators: The first occurance of {} is replace with a description + // of the format and the second with an id + // (mime-type) of the format. + gettext_f( + "Unknown image format “{} ({}).”", + [&mime_type_description, &mime_type], + ) + }; + + details = err.to_string(); + } + DecoderError::OutOfMemory(err) => { + description = + gettext("There is not enough free memory available to load this image."); + details = err.to_string(); + } + DecoderError::ImageSource(glib_err, err) => { + // Translators: The first occurance of {} is replace with a + // translated, more detailled error message. + description = + gettext_f("Failed to load image file: {}", [&glib_err.to_string()]); + details = err.to_string(); + } + DecoderError::Generic(err) => { + description = gettext( + "Either the image file is corrupted or it contains unsupported elements.", + ); + details = err.to_string(); + } } + + imp.error_more_info.connect_clicked(glib::clone!( + #[weak(rename_to = obj)] + self, + move |_| { + LpErrorDetails::new(&obj.root().unwrap(), &details, ErrorType::Loader); + } + )); + + imp.error_page.set_description(Some(&description)); + + imp.error_more_info.set_visible(true); + imp.stack.set_visible_child(&*imp.error_page); } } } diff --git a/src/widgets/image_view.rs b/src/widgets/image_view.rs index 4010451..a9f48c9 100644 --- a/src/widgets/image_view.rs +++ b/src/widgets/image_view.rs @@ -650,7 +650,10 @@ impl LpImageView { #[weak(rename_to = obj)] self, move |image| { - if matches!(image.specific_error(), DecoderError::UnsupportedFormat) { + if matches!( + image.specific_error(), + Some(DecoderError::UnsupportedFormat(_, _)) + ) { if obj.current_image().as_ref() == Some(image) { tracing::debug!( "Image format unsupported but not removing since current image"