From b9668c72629a0d04b3956ab39c40d74be3eee03f Mon Sep 17 00:00:00 2001 From: Sophie Herold Date: Sun, 21 Dec 2025 15:16:11 +0100 Subject: [PATCH] image: Unify error handling in one type --- src/application.rs | 2 +- src/decoder.rs | 38 +++++-------- src/file_model.rs | 5 +- src/widgets/image.rs | 9 ++- src/widgets/image/loading.rs | 69 ++++++++++------------- src/widgets/image_page.rs | 104 ++++++++++++++++++----------------- src/widgets/image_view.rs | 4 +- src/widgets/image_window.rs | 6 +- 8 files changed, 108 insertions(+), 129 deletions(-) diff --git a/src/application.rs b/src/application.rs index 6fbdb62..526295f 100644 --- a/src/application.rs +++ b/src/application.rs @@ -90,7 +90,7 @@ mod imp { // Handles opening files from the command line or other applications fn open(&self, files: &[gio::File], _hint: &str) { - tracing::trace!("Open {} file(s)", files.len()); + tracing::debug!("Open {} file(s)", files.len()); let application = self.obj(); let win = LpWindow::new(&*application); win.image_view().set_images_from_files(files.to_vec()); diff --git a/src/decoder.rs b/src/decoder.rs index 062b535..5562245 100644 --- a/src/decoder.rs +++ b/src/decoder.rs @@ -59,15 +59,10 @@ pub enum DecoderUpdate { Redraw, /// Start animation Animated, - SpecificError(DecoderError), - /// And error occurred during decoding - GenericError(anyhow::Error), + Error(DecoderError), } -#[derive(glib::Enum, Debug, Clone, Copy, Default, PartialEq, Eq)] -#[enum_type(name = "LpDecoderError")] -//#[enum_dynamic(lazy_registration = true)] -#[repr(i32)] +#[derive(Debug, Clone, Default, PartialEq, Eq)] pub enum DecoderError { /// Image format not supported or unknown UnsupportedFormat, @@ -75,26 +70,23 @@ pub enum DecoderError { NoLoadersConfigured, /// Memory limit exceeded OutOfMemory, + + Generic(String), + //ImageSource(String), #[default] None, } impl DecoderError { - pub fn is_err(self) -> bool { + pub fn is_err(&self) -> bool { matches!(self, Self::None) } - pub fn is_ok(self) -> bool { + pub fn is_ok(&self) -> bool { !matches!(self, Self::None) } } -impl glib::value::ToValueOptional for DecoderError { - fn to_value_optional(s: Option<&Self>) -> glib::Value { - s.unwrap_or(&Self::None).into() - } -} - impl UpdateSender { pub fn send(&self, update: DecoderUpdate) { let result = self.sender.force_send(update); @@ -124,23 +116,20 @@ 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); self.send(DecoderUpdate::Metadata(Box::new(metadata))); - self.send(DecoderUpdate::SpecificError( - DecoderError::UnsupportedFormat, - )); + self.send(DecoderUpdate::Error(DecoderError::UnsupportedFormat)); } if err.is_out_of_memory() { - self.send(DecoderUpdate::SpecificError(DecoderError::OutOfMemory)); + self.send(DecoderUpdate::Error(DecoderError::OutOfMemory)); } else if matches!(err.error(), glycin::Error::NoLoadersConfigured(_)) { - self.send(DecoderUpdate::SpecificError( - DecoderError::NoLoadersConfigured, - )); + self.send(DecoderUpdate::Error(DecoderError::NoLoadersConfigured)); } - self.send(DecoderUpdate::GenericError(err.into())); + self.send(DecoderUpdate::Error(DecoderError::Generic(err.to_string()))); } } @@ -220,7 +209,8 @@ 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::GenericError(err)); + self.update_sender + .send(DecoderUpdate::Error(DecoderError::Generic(err.to_string()))); } } } diff --git a/src/file_model.rs b/src/file_model.rs index 43ce393..93a7a83 100644 --- a/src/file_model.rs +++ b/src/file_model.rs @@ -82,10 +82,7 @@ impl Entry { } Err(err) => { - tracing::error!( - "Failed to obtain information for sorting '{}': {err}", - uri - ); + tracing::error!("Failed to obtain information for sorting '{uri}': {err}",); break; } } diff --git a/src/widgets/image.rs b/src/widgets/image.rs index b85c6ba..218e608 100644 --- a/src/widgets/image.rs +++ b/src/widgets/image.rs @@ -154,14 +154,13 @@ mod imp { pub(super) queued_reload: RefCell>, #[property(get)] pub(super) is_deleted: Cell, - #[property(get, builder(DecoderError::None))] - pub(super) specific_error: Cell, + /// Set if an error has occurred, shown on error_page + pub(super) specific_error: RefCell, + #[property(get=Self::is_error)] + pub(super) is_error: Cell, /// Set to true when image is ready for displaying #[property(get)] pub(super) is_loaded: Cell, - /// Set if an error has occurred, shown on error_page - #[property(get)] - pub(super) error: RefCell>, pub(super) background_color: RefCell>, pub(super) fixed_background_color: RefCell>, /// Animations disabled diff --git a/src/widgets/image/loading.rs b/src/widgets/image/loading.rs index 8a48e66..632f06d 100644 --- a/src/widgets/image/loading.rs +++ b/src/widgets/image/loading.rs @@ -142,18 +142,11 @@ impl imp::LpImage { self.tick_callback.replace(Some(callback_id)); } } - DecoderUpdate::SpecificError(err) => { - if self.obj().is_loaded() { + DecoderUpdate::Error(err) => { + if obj.is_loaded() { tracing::warn!("Error occured while loading additional data: {err:?}"); } else { - self.set_specific_error(err); - } - } - DecoderUpdate::GenericError(err) => { - if self.obj().is_loaded() { - tracing::warn!("Error occured while loading additional data: {err:?}"); - } else { - self.set_error(Some(err)); + obj.set_specific_error(err); } } } @@ -233,7 +226,7 @@ impl imp::LpImage { } else { let old_file = file_a.clone(); glib::spawn_future_local(async move { - if obj.error().is_some() { + if obj.is_error() { // Tmp files might be renamed quickly after creation. In this case // the original load fails, because the filename is already invalid. // Therefore reload on name change loading caused an error. @@ -274,32 +267,8 @@ impl imp::LpImage { } } - fn set_error(&self, err: Option) { - tracing::debug!("Decoding error: {err:?}"); - - // Keeping first error instead of replacing with new error - if err.is_some() && self.error.borrow().is_some() { - return; - } - - self.error.replace(err.as_ref().map(|x| x.to_string())); - self.obj().notify_error(); - - if err.is_some() { - self.set_loaded(false); - } - } - - fn set_specific_error(&self, error: DecoderError) { - let obj = self.obj(); - if obj.specific_error() != error { - self.specific_error.set(error); - obj.notify_specific_error(); - - if error.is_err() { - self.set_loaded(false); - } - } + pub(super) fn is_error(&self) -> bool { + self.specific_error.borrow().is_err() } fn set_loaded(&self, is_loaded: bool) { @@ -310,8 +279,7 @@ impl imp::LpImage { obj.notify_is_loaded(); if is_loaded { - self.set_error(None); - self.set_specific_error(DecoderError::None); + obj.set_specific_error(DecoderError::None); } } } @@ -388,4 +356,27 @@ impl LpImage { imp.decoder.replace(Some(Arc::new(decoder))); } + + pub fn specific_error(&self) -> DecoderError { + self.imp().specific_error.borrow().clone() + } + + pub fn set_specific_error(&self, error: DecoderError) { + let imp = self.imp(); + + // Keeping first error instead of replacing with new error + if error.is_err() && self.is_error() { + return; + } + + if self.specific_error() != error { + let is_err = error.is_err(); + imp.specific_error.replace(error); + self.notify_is_error(); + + if is_err { + imp.set_loaded(false); + } + } + } } diff --git a/src/widgets/image_page.rs b/src/widgets/image_page.rs index d8d4678..ad20018 100644 --- a/src/widgets/image_page.rs +++ b/src/widgets/image_page.rs @@ -136,7 +136,7 @@ mod imp { } )); - obj.image().connect_error_notify(clone!( + obj.image().connect_is_error_notify(clone!( #[weak] obj, move |_| { @@ -214,38 +214,39 @@ impl LpImagePage { let imp = self.imp(); let image = self.image(); - if 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)], - ); + 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)], + ); - imp.error_page.set_description(Some(&message)); - imp.stack.set_visible_child(&*imp.error_page); - imp.error_more_info.set_visible(false); - } else if image.specific_error() == 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(); + 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(); - 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], - ) - }; + 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], + ) + }; - if let Some(err) = image.error() { imp.error_more_info.connect_clicked(glib::clone!( #[weak(rename_to = obj)] self, @@ -253,31 +254,32 @@ impl LpImagePage { LpErrorDetails::new(&obj.root().unwrap(), &err, ErrorType::Loader); } )); - } - imp.error_page.set_description(Some(&message)); - imp.stack.set_visible_child(&*imp.error_page); - } else if let Some(err) = image.error() { - 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.", + ))); } - )); - if 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.", - ))); + imp.error_more_info.set_visible(true); + imp.stack.set_visible_child(&*imp.error_page); } - - 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 394fe76..4010451 100644 --- a/src/widgets/image_view.rs +++ b/src/widgets/image_view.rs @@ -646,11 +646,11 @@ impl LpImageView { fn new_image_page(&self, file: &gio::File) -> LpImagePage { let page = LpImagePage::from_file(file); - page.image().connect_specific_error_notify(glib::clone!( + page.image().connect_is_error_notify(glib::clone!( #[weak(rename_to = obj)] self, move |image| { - if image.specific_error() == DecoderError::UnsupportedFormat { + if matches!(image.specific_error(), DecoderError::UnsupportedFormat) { if obj.current_image().as_ref() == Some(image) { tracing::debug!( "Image format unsupported but not removing since current image" diff --git a/src/widgets/image_window.rs b/src/widgets/image_window.rs index beb1838..3eb2665 100644 --- a/src/widgets/image_window.rs +++ b/src/widgets/image_window.rs @@ -452,7 +452,7 @@ mod imp { self.image_view.try_get().is_some_and(|image_view| { image_view .current_page() - .is_some_and(|page| page.image().error().is_none()) + .is_some_and(|page| !page.image().is_error()) }) } } @@ -748,7 +748,7 @@ impl LpImageWindow { .imp() .image_view .current_page() - .is_some_and(|x| x.image().is_loaded() && x.image().error().is_none()); + .is_some_and(|x| x.image().is_loaded() && !x.image().is_error()); for action in ACTIONS_SHOWN { self.action_set_enabled(action, enabled_shown); @@ -914,7 +914,7 @@ impl LpImageWindow { let current_page = self.imp().image_view.current_page(); - if current_page.is_some_and(|page| page.image().error().is_some()) { + if current_page.is_some_and(|page| page.image().is_error()) { tracing::debug!("Showing window because loading image failed"); self.window_inspect(|w| w.present()); }