From da321c76a47c84718e6a6b34f9529d091c5209ab Mon Sep 17 00:00:00 2001 From: Sophie Herold Date: Sat, 26 Nov 2022 00:02:56 +0100 Subject: [PATCH 1/2] file_model: Load opened image first add dir later - Fixes image cannot be opened when directory unreadable - Avoids bugs with wrong images being selected for an instant. Mostly important for adjusting the window size to image. - Basis for prioritizing loading the important image first --- src/file_model.rs | 33 ++++++++++++++++----- src/widgets/image_view.rs | 62 ++++++++++++++++++++++----------------- 2 files changed, 61 insertions(+), 34 deletions(-) diff --git a/src/file_model.rs b/src/file_model.rs index 84e1f0d1..394d7359 100644 --- a/src/file_model.rs +++ b/src/file_model.rs @@ -70,12 +70,30 @@ glib::wrapper! { } impl LpFileModel { - pub fn from_directory(directory: &gio::File) -> anyhow::Result { + pub fn from_file(file: &gio::File) -> anyhow::Result { let model = glib::Object::new::(&[]); + let directory = file.parent().context("File has not parent")?; + let file = + directory.resolve_relative_path(file.basename().context("File has no basename")?); { - // Here we use a nested scope so that the mutable borrow only lasts as long as we need it let mut vec = model.imp().inner.borrow_mut(); + vec.push(file); + } + + model.imp().directory.set(directory.clone()).unwrap(); + + Ok(model) + } + + pub fn load_directory(&self) -> anyhow::Result<()> { + let directory = self.imp().directory.get().unwrap(); + + { + // Here we use a nested scope so that the mutable borrow only lasts as long as we need it + let mut vec = self.imp().inner.borrow_mut(); + let original_vec: Vec> = + vec.iter().map(|x| x.path()).collect(); let enumerator = directory .enumerate_children( @@ -95,8 +113,11 @@ impl LpFileModel { if let Some(content_type) = info.content_type().map(|t| t.to_string()) { if content_type.starts_with("image/") { let name = info.name(); - log::debug!("{:?} is an image, adding to the list", name); - vec.push(directory.resolve_relative_path(&name)); + let relative_path = directory.resolve_relative_path(&name); + if !original_vec.contains(&relative_path.path()) { + log::debug!("{:?} is an image, adding to the list", name); + vec.push(relative_path); + } } } } @@ -104,11 +125,9 @@ impl LpFileModel { // Then sort by name. vec.sort_by(util::compare_by_name); - - model.imp().directory.set(directory.clone()).unwrap(); } - Ok(model) + Ok(()) } pub fn directory(&self) -> Option { diff --git a/src/widgets/image_view.rs b/src/widgets/image_view.rs index af360791..dd39dc3e 100644 --- a/src/widgets/image_view.rs +++ b/src/widgets/image_view.rs @@ -182,38 +182,46 @@ impl LpImageView { let imp = self.imp(); let carousel = &imp.carousel; - { + let model = { // Here we use a nested scope so that the mutable borrow only lasts as long as we need it - let mut model = imp.model.borrow_mut(); - - if let Some(ref parent) = file.parent() { - if let Some(ref m) = *model { - if m.directory().map_or(false, |f| !f.equal(parent)) { - // Clear the carousel before creating the new model - self.clear_carousel(false); - *model = Some(LpFileModel::from_directory(parent)?); - log::debug!("new model created"); - } else { - log::debug!("Re-using old model and navigating to the current file"); - self.navigate_to_file(m, file); - return Ok(()); - } - } else { - *model = Some(LpFileModel::from_directory(parent)?); + let mut model_store = imp.model.borrow_mut(); + + if let Some(ref m) = *model_store { + let is_same_directory = file + .parent() + .map_or(false, |p| m.directory().map_or(false, |f| !f.equal(&p))); + + if is_same_directory { + // Clear the carousel before creating the new model + self.clear_carousel(false); + let model = LpFileModel::from_file(file)?; + *model_store = Some(model.clone()); log::debug!("new model created"); + model + } else { + log::debug!("Re-using old model and navigating to the current file"); + self.navigate_to_file(m, file); + return Ok(()); } + } else { + let model = LpFileModel::from_file(file)?; + *model_store = Some(model.clone()); + log::debug!("new model created"); + model } - } + }; - if let Some(model) = imp.model.borrow().as_ref() { - let index = model - .index_of(file) - .context(i18n("File not found in model. (TODO: bad message)"))?; - log::debug!("Currently at file {index} in the directory"); - carousel.append(&LpImagePage::from_file(file)); - self.fill_carousel(model, index); - self.update_action_state(model, index); - } + carousel.append(&LpImagePage::from_file(file)); + self.update_action_state(&model, 0); + + model.load_directory()?; + let index = model + .index_of(file) + .context(i18n("File not found in model. (TODO: bad message)"))?; + log::debug!("Currently at file {index} in the directory"); + + self.fill_carousel(&model, index); + self.update_action_state(&model, index); Ok(()) } -- GitLab From 28d068ea0a4afde5ef3197ee567adcd8f55385c1 Mon Sep 17 00:00:00 2001 From: Sophie Herold Date: Tue, 29 Nov 2022 04:08:06 +0100 Subject: [PATCH 2/2] imageView: Work around AdwCarousel bug --- src/widgets/image.rs | 4 ++-- src/widgets/image_view.rs | 21 +++++++++++++++------ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/widgets/image.rs b/src/widgets/image.rs index e1f7315e..646f444f 100644 --- a/src/widgets/image.rs +++ b/src/widgets/image.rs @@ -808,7 +808,7 @@ impl LpImage { if let Some(adj) = self.imp().hadjustment.borrow().as_ref() { adj.clone() } else { - log::debug!("Hadjustment not set yet: Using fake object"); + log::trace!("Hadjustment not set yet: Using fake object"); gtk::Adjustment::default() } } @@ -830,7 +830,7 @@ impl LpImage { if let Some(adj) = self.imp().vadjustment.borrow().as_ref() { adj.clone() } else { - log::debug!("Vadjustment not set yet: Using fake object"); + log::trace!("Vadjustment not set yet: Using fake object"); gtk::Adjustment::default() } } diff --git a/src/widgets/image_view.rs b/src/widgets/image_view.rs index dd39dc3e..b7a864cb 100644 --- a/src/widgets/image_view.rs +++ b/src/widgets/image_view.rs @@ -180,18 +180,18 @@ impl LpImageView { // and in turn we'll update our `adw::Carousel`. fn build_model_from_file(&self, file: &gio::File) -> anyhow::Result<()> { let imp = self.imp(); - let carousel = &imp.carousel; + let carousel = imp.carousel.clone(); let model = { // Here we use a nested scope so that the mutable borrow only lasts as long as we need it let mut model_store = imp.model.borrow_mut(); if let Some(ref m) = *model_store { - let is_same_directory = file + let is_new_directory = file .parent() .map_or(false, |p| m.directory().map_or(false, |f| !f.equal(&p))); - if is_same_directory { + if is_new_directory { // Clear the carousel before creating the new model self.clear_carousel(false); let model = LpFileModel::from_file(file)?; @@ -206,12 +206,19 @@ impl LpImageView { } else { let model = LpFileModel::from_file(file)?; *model_store = Some(model.clone()); - log::debug!("new model created"); + log::debug!("first model created"); model } }; - carousel.append(&LpImagePage::from_file(file)); + let page = LpImagePage::from_file(file); + carousel.append(&page); + adw::glib::timeout_add_local_once( + std::time::Duration::from_millis(20), + glib::clone!(@weak carousel => move || { + carousel.scroll_to(&page, false); + }), + ); self.update_action_state(&model, 0); model.load_directory()?; @@ -268,7 +275,9 @@ impl LpImageView { // new page. log::debug!("Scrolling to page for {new_index}"); imp.scrolling.set(true); - carousel.scroll_to(&page, true); + adw::glib::timeout_add_local_once(std::time::Duration::from_millis(20), move || { + carousel.scroll_to(&page, true) + }); } // Fills the carousel with items on either side of the given `index` of `model` -- GitLab