From 0a3a6efa02825532d1cd2ef1befe64976b05fba6 Mon Sep 17 00:00:00 2001 From: Sophie Herold Date: Thu, 16 Jan 2025 12:18:52 +0100 Subject: [PATCH 1/2] edit_window: Always use .suggested-action style --- data/resources/icons/scalable/actions/crop-symbolic.svg | 2 ++ data/resources/style.css | 5 +++++ src/widgets/edit_window.rs | 5 ----- src/widgets/edit_window.ui | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) create mode 100644 data/resources/icons/scalable/actions/crop-symbolic.svg diff --git a/data/resources/icons/scalable/actions/crop-symbolic.svg b/data/resources/icons/scalable/actions/crop-symbolic.svg new file mode 100644 index 00000000..422c9001 --- /dev/null +++ b/data/resources/icons/scalable/actions/crop-symbolic.svg @@ -0,0 +1,2 @@ + + diff --git a/data/resources/style.css b/data/resources/style.css index 04376854..d3af6240 100644 --- a/data/resources/style.css +++ b/data/resources/style.css @@ -85,4 +85,9 @@ dialog.error-details textview { .error-message { padding: 12px; font-size: 0.95em; +} + +menubutton.suggested-action button { + padding-left: 17px; + padding-right: 17px; } \ No newline at end of file diff --git a/src/widgets/edit_window.rs b/src/widgets/edit_window.rs index 9577a7ff..12c46079 100644 --- a/src/widgets/edit_window.rs +++ b/src/widgets/edit_window.rs @@ -266,11 +266,6 @@ impl LpEditWindow { let done_sensitive = operations .as_ref() .is_some_and(|x| !x.operations().is_empty()); - if done_sensitive { - imp.done.add_css_class("suggested-action"); - } else { - imp.done.remove_css_class("suggested-action"); - } imp.done.set_sensitive(done_sensitive); imp.operations.replace(operations); diff --git a/src/widgets/edit_window.ui b/src/widgets/edit_window.ui index 6cf5c9b9..3c3422a7 100644 --- a/src/widgets/edit_window.ui +++ b/src/widgets/edit_window.ui @@ -20,7 +20,7 @@ true false -- GitLab From 8d3c9d70e7a06888174d3d0b9dec894145ebf3f7 Mon Sep 17 00:00:00 2001 From: Sophie Herold Date: Fri, 17 Jan 2025 01:30:41 +0100 Subject: [PATCH 2/2] image_view: Don't rely on LpImage signals --- src/file_model.rs | 124 +++++++++++++++++++++++++--------- src/widgets/edit_window.rs | 2 +- src/widgets/image/loading.rs | 24 +++++-- src/widgets/image/metadata.rs | 2 +- src/widgets/image_view.rs | 75 ++++++-------------- 5 files changed, 134 insertions(+), 93 deletions(-) diff --git a/src/file_model.rs b/src/file_model.rs index 09f25808..e79e9708 100644 --- a/src/file_model.rs +++ b/src/file_model.rs @@ -1,4 +1,4 @@ -// Copyright (c) 2022-2024 Sophie Herold +// Copyright (c) 2022-2025 Sophie Herold // Copyright (c) 2022 Christopher Davis // // This program is free software: you can redistribute it and/or modify @@ -99,8 +99,11 @@ mod imp { impl ObjectImpl for LpFileModel { fn signals() -> &'static [Signal] { - static SIGNALS: LazyLock> = - LazyLock::new(|| vec![Signal::builder("changed").build()]); + static SIGNALS: LazyLock> = LazyLock::new(|| { + vec![Signal::builder("changed") + .param_types([glib::Type::VARIANT]) + .build()] + }); SIGNALS.as_ref() } } @@ -243,7 +246,7 @@ impl LpFileModel { self.imp().files.borrow().len() } - pub fn contains(&self, file: &gio::File) -> bool { + pub fn contains_file(&self, file: &gio::File) -> bool { self.imp().files.borrow().contains_key(&file.uri()) } @@ -336,30 +339,37 @@ impl LpFileModel { } /// Signal that notifies about added/removed files - pub fn connect_changed(&self, f: impl Fn() + 'static) { - self.connect_local("changed", false, move |_| { - f(); + pub fn connect_changed(&self, f: impl Fn(&FileEvent) + 'static) { + self.connect_local("changed", false, move |args| { + if let Some(file_event) = args + .get(1) + .and_then(|x| x.get().ok()) + .and_then(|x| FileEvent::from_variant(&x)) + { + f(&file_event); + } else { + log::error!("Failed to read arguments from 'changed' signal."); + } + None }); } - /// Insert a file and signal changes - /// - /// Setting `changed` to true will always trigger a `notify::changed`` - /// notification. Otherwise, a change signal is only sent if no file with - /// the same URI existed before. - pub fn insert(&self, file: gio::File, mut changed: bool) { + pub fn emmit_changed(&self, file_event: &FileEvent) { + self.emit_by_name::<()>("changed", &[&file_event.to_variant()]); + } + + /// Insert a file + async fn insert(&self, file: gio::File) -> bool { let obj = self.clone(); - glib::spawn_future_local(async move { - let entry = Entry::new(file.clone()).await; - let mut files = obj.imp().files.borrow_mut(); - changed |= files.insert(file.uri(), entry).is_none(); - if changed { - Self::sort(&mut files); - drop(files); - obj.emit_by_name::<()>("changed", &[]); - } - }); + let entry = Entry::new(file.clone()).await; + let mut files = obj.imp().files.borrow_mut(); + let changed = files.insert(file.uri(), entry).is_none(); + if changed { + Self::sort(&mut files); + drop(files); + } + changed } fn file_monitor_cb( @@ -368,33 +378,83 @@ impl LpFileModel { file_a: &gio::File, file_b: Option<&gio::File>, ) { + let uri_a = file_a.uri(); match event { gio::FileMonitorEvent::Created | gio::FileMonitorEvent::MovedIn - // Changing file content could theoretically make it an image - // by adding a magic byte | gio::FileMonitorEvent::ChangesDoneHint if Self::is_image_file(file_a) => { - self.insert(file_a.clone(), false); + // ^^^^ + // Changing file content could theoretically make it an image + // by adding a magic byte + + log::debug!("File added: {uri_a}"); + + glib::spawn_future_local(glib::clone!( + #[strong(rename_to=obj)] + self, + #[strong] + file_a, + async move { + let changed = obj.insert(file_a.clone()).await; + if changed { + obj.emmit_changed(&FileEvent::New(uri_a.to_string())); + } + } + )); } - gio::FileMonitorEvent::Deleted | gio::FileMonitorEvent::MovedOut | gio::FileMonitorEvent::Unmounted => { - let removed = self.imp().files.borrow_mut().shift_remove(&file_a.uri()).is_some(); + gio::FileMonitorEvent::Deleted + | gio::FileMonitorEvent::MovedOut + | gio::FileMonitorEvent::Unmounted => { + log::debug!("File removed: {}", file_a.uri()); + let removed = self + .imp() + .files + .borrow_mut() + .shift_remove(&file_a.uri()) + .is_some(); if removed { - self.emit_by_name::<()>("changed", &[]); + self.emmit_changed(&FileEvent::Removed(file_a.uri().to_string())); } } gio::FileMonitorEvent::Renamed => { if let Some(file_b) = file_b { { - let changed = self.imp().files.borrow_mut().shift_remove(&file_a.uri()).is_some(); + let uri_b = file_b.uri(); + log::debug!("File moved from '{uri_a}' to '{uri_b}'"); + let mut changed = + self.imp().files.borrow_mut().shift_remove(&uri_a).is_some(); + if Self::is_image_file(file_b) { - self.insert(file_b.clone(), changed); + glib::spawn_future_local(glib::clone!( + #[strong(rename_to=obj)] + self, + #[strong] + file_b, + async move { + changed |= obj.insert(file_b.clone()).await; + + if changed { + obj.emmit_changed(&FileEvent::Moved( + uri_a.to_string(), + uri_b.to_string(), + )); + } + } + )); } } } } - _ => {}, + _ => {} } } } + +#[derive(Debug, glib::Variant)] +pub enum FileEvent { + New(String), + Removed(String), + Moved(String, String), +} diff --git a/src/widgets/edit_window.rs b/src/widgets/edit_window.rs index 12c46079..cd9b01b5 100644 --- a/src/widgets/edit_window.rs +++ b/src/widgets/edit_window.rs @@ -1,4 +1,4 @@ -// Copyright (c) 2024 Sophie Herold +// Copyright (c) 2024-2025 Sophie Herold // // This program is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by diff --git a/src/widgets/image/loading.rs b/src/widgets/image/loading.rs index fbda2f1c..1bd2d38e 100644 --- a/src/widgets/image/loading.rs +++ b/src/widgets/image/loading.rs @@ -203,32 +203,48 @@ impl imp::LpImage { file_a: &gio::File, file_b: Option<&gio::File>, ) { + let obj = self.obj().to_owned(); + match event { gio::FileMonitorEvent::Renamed => { if let Some(file) = file_b.cloned() { log::debug!("Moved to {}", file.uri()); - let obj = self.obj().to_owned(); // current file got replaced with a new one let file_replace = obj.file().is_some_and(|x| x.equal(&file)); if file_replace { log::debug!("Image got replaced {}", file.uri()); - // TODO: error handling is missing glib::spawn_future_local(async move { obj.load(&file).await; }); } else { glib::spawn_future_local(async move { - obj.imp().set_file_changed(&file).await; + if obj.error().is_some() { + // 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. + obj.load(&file).await; + } else { + obj.imp().set_file_changed(&file).await; + } }); } } } + gio::FileMonitorEvent::MovedIn => { + log::debug!("Image got replaced by moving into dir {}", file_a.uri()); + glib::spawn_future_local(glib::clone!( + #[strong] + file_a, + async move { + obj.load(&file_a).await; + } + )); + } gio::FileMonitorEvent::ChangesDoneHint => { let obj = self.obj().to_owned(); let file = file_a.clone(); log::debug!("Image was edited {}", file.uri()); - // TODO: error handling is missing glib::spawn_future_local(async move { obj.load(&file).await; }); diff --git a/src/widgets/image/metadata.rs b/src/widgets/image/metadata.rs index f1dcc3f5..f141dde5 100644 --- a/src/widgets/image/metadata.rs +++ b/src/widgets/image/metadata.rs @@ -1,4 +1,4 @@ -// Copyright (c) 2023-2024 Sophie Herold +// Copyright (c) 2023-2025 Sophie Herold // // This program is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by diff --git a/src/widgets/image_view.rs b/src/widgets/image_view.rs index bd8daeea..ea1c9c0e 100644 --- a/src/widgets/image_view.rs +++ b/src/widgets/image_view.rs @@ -1,5 +1,5 @@ // Copyright (c) 2020-2023 Christopher Davis -// Copyright (c) 2022-2024 Sophie Herold +// Copyright (c) 2022-2025 Sophie Herold // Copyright (c) 2022 Maximiliano Sandoval R // Copyright (c) 2023 FineFindus // Copyright (c) 2023 Huan Nguyen @@ -43,7 +43,7 @@ use gtk::CompositeTemplate; use crate::decoder::DecoderError; use crate::deps::*; -use crate::file_model::LpFileModel; +use crate::file_model::{FileEvent, LpFileModel}; use crate::util::gettext::*; use crate::util::{self, Direction, Position}; use crate::widgets::{LpImage, LpImagePage, LpPrint, LpSlidingView}; @@ -174,29 +174,6 @@ mod imp { let current_image_signal_group = obj.current_image_signals(); let view = &*obj; - current_image_signal_group.connect_closure( - "notify::file", - true, - glib::closure_local!( - #[watch] - view, - move |_: LpImage, _: glib::ParamSpec| { - view.current_image_path_changed(); - } - ), - ); - - current_image_signal_group.connect_closure( - "notify::is-deleted", - true, - glib::closure_local!( - #[watch] - view, - move |_: LpImage, _: glib::ParamSpec| { - view.current_image_path_changed(); - } - ), - ); current_image_signal_group.connect_closure( "notify::zoom-target", @@ -687,16 +664,16 @@ impl LpImageView { model.connect_changed(glib::clone!( #[weak(rename_to = obj)] self, - move || obj.model_content_changed_cb() + move |change| obj.model_content_changed_cb(change) )); self.imp().model.replace(model); } /// Handle files are added or removed from directory - fn model_content_changed_cb(&self) { + fn model_content_changed_cb(&self, file_event: &FileEvent) { // Animate to image that was restored from trash if let Some(trash_restore) = self.trash_restore() { - if self.model().contains(&trash_restore) { + if self.model().contains_file(&trash_restore) { self.set_trash_restore(gio::File::NONE); self.update_sliding_view(&trash_restore); self.scroll_sliding_view(&trash_restore, true); @@ -708,30 +685,21 @@ impl LpImageView { return; }; - // LpImage did not get the update yet - // Update will be handled by current_image_path_changed - if !self.model().contains(¤t_file) { - return; - } - - self.update_sliding_view(¤t_file); - } - - /// Handle current image being moved or deleted - /// - /// This is handled separately since we want to delete animations and - /// want to still show the same image if renamed. - fn current_image_path_changed(&self) { - if let Some(image) = self.current_page().map(|x| x.image()) { - if image.is_deleted() { + match file_event { + FileEvent::Removed(removed) if removed == ¤t_file.uri() => { self.sliding_view().scroll_to_neighbor(); + return; + } + FileEvent::Moved(src, target) => { + if let Some(page) = self.sliding_view().get(&gio::File::for_uri(src)) { + page.image().init(&gio::File::for_uri(target)); + } } + _ => {} } - if let Some(current_file) = self.current_file() { - if self.model().contains(¤t_file) && !self.imp().preserve_content.get() { - self.update_sliding_view(¤t_file); - } + if !self.imp().preserve_content.get() { + self.update_sliding_view(¤t_file); } } @@ -757,14 +725,11 @@ impl LpImageView { self.imp() .sliding_view .current_page() - .map(|x| x.file().uri()) + .map(|x: LpImagePage| x.file().uri()) } pub fn current_file(&self) -> Option { - self.imp() - .sliding_view - .current_page() - .and_then(|x| x.image().file()) + self.current_uri().map(|x| gio::File::for_uri(&x)) } pub fn drag_source(&self) -> gtk::DragSource { @@ -819,8 +784,8 @@ impl LpImageView { pub async fn set_background(&self) -> anyhow::Result<()> { let uri = self - .current_file() - .and_then(|f| url::Url::parse(f.uri().as_str()).ok()) + .current_uri() + .and_then(|x| url::Url::parse(x.as_str()).ok()) .context("Invalid URL for background image")?; let native = self.native().context("View should have a GtkNative")?; let id = WindowIdentifier::from_native(&native).await; -- GitLab