Merge branch '7-async-image-loading' into 'master'

Resolve "Make image loading not blocking the programs execution."

Closes #7

See merge request alatiera/Hammond!29

CHANGELOG.md

@@ -6,6 +6,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased]
 
+* Downlaoding and loading images now is done asynchronously and is not blocking programs execution.
+[#7](https://gitlab.gnome.org/alatiera/Hammond/issues/7)
 
 ## [0.3.1] - 2018-03-28
 

hammond-data/src/models/new_podcast.rs

@@ -231,9 +231,9 @@ mod tests {
                          love, a crashed UFO, an alien body, and an impossible heist unlike any \
                          ever attempted - scripted by Mac Rogers, the award-winning playwright \
                          and writer of the multi-million download The Message and LifeAfter.</p>";
-            let img =  "https://dfkfj8j276wwv.cloudfront.net/images/2c/5f/a0/1a/2c5fa01a-ae78-4a8c-\
+            let img = "https://dfkfj8j276wwv.cloudfront.net/images/2c/5f/a0/1a/2c5fa01a-ae78-4a8c-\
-                        b183-7311d2e436c3/b3a4aa57a576bb662191f2a6bc2a436c8c4ae256ecffaff5c4c54fd42e\
+                       b183-7311d2e436c3/b3a4aa57a576bb662191f2a6bc2a436c8c4ae256ecffaff5c4c54fd42e\
-                        923914941c264d01efb1833234b52c9530e67d28a8cebbe3d11a4bc0fbbdf13ecdf1c3.jpeg";
+                       923914941c264d01efb1833234b52c9530e67d28a8cebbe3d11a4bc0fbbdf13ecdf1c3.jpeg";
 
             NewPodcastBuilder::default()
                 .title("Steal the Stars")

hammond-gtk/src/main.rs

@@ -26,15 +26,14 @@ extern crate hammond_downloader;
 extern crate humansize;
 extern crate loggerv;
 extern crate open;
+extern crate rayon;
 extern crate regex;
 extern crate reqwest;
 extern crate send_cell;
 extern crate serde_json;
 extern crate take_mut;
 extern crate url;
-// extern crate rayon;
 
-// use rayon::prelude::*;
 use log::Level;
 
 use gtk::prelude::*;

hammond-gtk/src/utils.rs

@@ -1,8 +1,13 @@
 #![cfg_attr(feature = "cargo-clippy", allow(type_complexity))]
 
-use failure::Error;
 use gdk_pixbuf::Pixbuf;
 use gio::{Settings, SettingsExt};
+use glib;
+use gtk;
+use gtk::prelude::*;
+
+use failure::Error;
+use rayon;
 use regex::Regex;
 use reqwest;
 use send_cell::SendCell;
@@ -18,7 +23,7 @@ use hammond_downloader::downloader;
 use std::collections::{HashMap, HashSet};
 use std::sync::{Mutex, RwLock};
 use std::sync::Arc;
-use std::sync::mpsc::Sender;
+use std::sync::mpsc::*;
 use std::thread;
 
 use app::Action;
@@ -128,6 +133,11 @@ fn refresh_feed(source: Option<Vec<Source>>, sender: Sender<Action>) -> Result<(
 lazy_static! {
     static ref CACHED_PIXBUFS: RwLock<HashMap<(i32, u32), Mutex<SendCell<Pixbuf>>>> =
         { RwLock::new(HashMap::new()) };
+    static ref COVER_DL_REGISTRY: RwLock<HashSet<i32>> = RwLock::new(HashSet::new());
+    static ref THREADPOOL: rayon::ThreadPool = rayon::ThreadPoolBuilder::new()
+        .breadth_first()
+        .build()
+        .unwrap();
 }
 
 // Since gdk_pixbuf::Pixbuf is refference counted and every episode,
@@ -137,25 +147,78 @@ lazy_static! {
 // GObjects do not implement Send trait, so SendCell is a way around that.
 // Also lazy_static requires Sync trait, so that's what the mutexes are.
 // TODO: maybe use something that would just scale to requested size?
-pub fn get_pixbuf_from_path(pd: &PodcastCoverQuery, size: u32) -> Result<Pixbuf, Error> {
+pub fn set_image_from_path(
+    image: &gtk::Image,
+    pd: Arc<PodcastCoverQuery>,
+    size: u32,
+) -> Result<(), Error> {
     {
-        let hashmap = CACHED_PIXBUFS
+        // Check if there's an active download about this show cover.
+        // If there is, a callback will be set so this function will be called again.
+        // If the download succedes, there should be a quick return from the pixbuf cache_image
+        // If it fails another download will be scheduled.
+        let reg_guard = COVER_DL_REGISTRY
             .read()
-            .map_err(|_| format_err!("Failed to get a lock on the pixbuf cache mutex."))?;
+            .map_err(|err| format_err!("Cover Registry: {}.", err))?;
-        if let Some(px) = hashmap.get(&(pd.id(), size)) {
+        if reg_guard.contains(&pd.id()) {
-            let m = px.lock()
+            let callback = clone!(image, pd => move || {
-                .map_err(|_| format_err!("Failed to lock pixbuf mutex."))?;
+                 let _ = set_image_from_path(&image, pd.clone(), size);
-            return Ok(m.clone().into_inner());
+                 glib::Continue(false)
+            });
+            gtk::timeout_add(250, callback);
+            return Ok(());
         }
     }
 
-    let img_path = downloader::cache_image(pd)?;
+    {
-    let px = Pixbuf::new_from_file_at_scale(&img_path, size as i32, size as i32, true)?;
+        let hashmap = CACHED_PIXBUFS
-    let mut hashmap = CACHED_PIXBUFS
+            .read()
-        .write()
+            .map_err(|err| format_err!("Pixbuf HashMap: {}", err))?;
-        .map_err(|_| format_err!("Failed to lock pixbuf mutex."))?;
+        if let Some(px) = hashmap.get(&(pd.id(), size)) {
-    hashmap.insert((pd.id(), size), Mutex::new(SendCell::new(px.clone())));
+            let m = px.lock()
-    Ok(px)
+                .map_err(|err| format_err!("SendCell Mutex: {}", err))?;
+            let px = m.try_get().ok_or_else(|| {
+                format_err!("Pixbuf was accessed from a different thread than created")
+            })?;
+            image.set_from_pixbuf(px);
+            return Ok(());
+        }
+    }
+
+    let (sender, receiver) = channel();
+    let pd_ = pd.clone();
+    THREADPOOL.spawn(move || {
+        {
+            if let Ok(mut guard) = COVER_DL_REGISTRY.write() {
+                guard.insert(pd_.id());
+            }
+        }
+        let _ = sender.send(downloader::cache_image(&pd_));
+        {
+            if let Ok(mut guard) = COVER_DL_REGISTRY.write() {
+                guard.remove(&pd_.id());
+            }
+        }
+    });
+
+    let image = image.clone();
+    let s = size as i32;
+    gtk::timeout_add(25, move || {
+        if let Ok(path) = receiver.try_recv() {
+            if let Ok(path) = path {
+                if let Ok(px) = Pixbuf::new_from_file_at_scale(&path, s, s, true) {
+                    if let Ok(mut hashmap) = CACHED_PIXBUFS.write() {
+                        hashmap.insert((pd.id(), size), Mutex::new(SendCell::new(px.clone())));
+                        image.set_from_pixbuf(&px);
+                    }
+                }
+            }
+            glib::Continue(false)
+        } else {
+            glib::Continue(true)
+        }
+    });
+    Ok(())
 }
 
 #[inline]
@@ -200,8 +263,8 @@ pub fn time_period_to_duration(time: i64, period: &str) -> Duration {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use hammond_data::Source;
+    // use hammond_data::Source;
-    use hammond_data::dbqueries;
+    // use hammond_data::dbqueries;
 
     #[test]
     fn test_time_period_to_duration() {
@@ -221,23 +284,25 @@ mod tests {
         assert_eq!(time, time_period_to_duration(time, "seconds").num_seconds());
     }
 
-    #[test]
+    // #[test]
     // This test inserts an rss feed to your `XDG_DATA/hammond/hammond.db` so we make it explicit
     // to run it.
-    #[ignore]
+    // #[ignore]
-    fn test_get_pixbuf_from_path() {
+    // Disabled till https://gitlab.gnome.org/alatiera/Hammond/issues/56
-        let url = "https://web.archive.org/web/20180120110727if_/https://rss.acast.com/thetipoff";
+    // fn test_set_image_from_path() {
-        // Create and index a source
+    //     let url = "https://web.archive.org/web/20180120110727if_/https://rss.acast.com/thetipoff";
-        let source = Source::from_url(url).unwrap();
+    // Create and index a source
-        // Copy it's id
+    //     let source = Source::from_url(url).unwrap();
-        let sid = source.id();
+    // Copy it's id
-        pipeline::run(vec![source], true).unwrap();
+    //     let sid = source.id();
+    //     pipeline::run(vec![source], true).unwrap();
 
-        // Get the Podcast
+    // Get the Podcast
-        let pd = dbqueries::get_podcast_from_source_id(sid).unwrap();
+    //     let img = gtk::Image::new();
-        let pxbuf = get_pixbuf_from_path(&pd.into(), 256);
+    //     let pd = dbqueries::get_podcast_from_source_id(sid).unwrap().into();
-        assert!(pxbuf.is_ok());
+    //     let pxbuf = set_image_from_path(&img, Arc::new(pd), 256);
-    }
+    //     assert!(pxbuf.is_ok());
+    // }
 
     #[test]
     fn test_itunes_to_rss() {

hammond-gtk/src/views/episodes.rs

@@ -7,9 +7,10 @@ use hammond_data::EpisodeWidgetQuery;
 use hammond_data::dbqueries;
 
 use app::Action;
-use utils::{get_ignored_shows, get_pixbuf_from_path};
+use utils::{get_ignored_shows, set_image_from_path};
 use widgets::EpisodeWidget;
 
+use std::sync::Arc;
 use std::sync::mpsc::Sender;
 
 #[derive(Debug, Clone)]
@@ -227,9 +228,7 @@ impl EpisodesViewWidget {
     }
 
     fn set_cover(&self, podcast_id: i32) -> Result<(), Error> {
-        let pd = dbqueries::get_podcast_cover_from_id(podcast_id)?;
+        let pd = Arc::new(dbqueries::get_podcast_cover_from_id(podcast_id)?);
-        let img = get_pixbuf_from_path(&pd, 64)?;
+        set_image_from_path(&self.image, pd, 64)
-        self.image.set_from_pixbuf(&img);
-        Ok(())
     }
 }

hammond-gtk/src/views/shows.rs

@@ -2,11 +2,11 @@ use failure::Error;
 use gtk;
 use gtk::prelude::*;
 
-use hammond_data::Podcast;
+use hammond_data::{Podcast, PodcastCoverQuery};
 use hammond_data::dbqueries;
 
 use app::Action;
-use utils::{get_ignored_shows, get_pixbuf_from_path};
+use utils::{get_ignored_shows, set_image_from_path};
 
 use std::sync::Arc;
 use std::sync::mpsc::Sender;
@@ -57,7 +57,7 @@ impl ShowsPopulated {
         let ignore = get_ignored_shows()?;
         let podcasts = dbqueries::get_podcasts_filter(&ignore)?;
 
-        podcasts.into_iter().map(Arc::new).for_each(|parent| {
+        podcasts.into_iter().for_each(|parent| {
             let flowbox_child = ShowsChild::new(parent);
             self.flowbox.add(&flowbox_child.child);
         });
@@ -116,25 +116,23 @@ impl Default for ShowsChild {
 }
 
 impl ShowsChild {
-    pub fn new(pd: Arc<Podcast>) -> ShowsChild {
+    pub fn new(pd: Podcast) -> ShowsChild {
         let child = ShowsChild::default();
         child.init(pd);
         child
     }
 
-    fn init(&self, pd: Arc<Podcast>) {
+    fn init(&self, pd: Podcast) {
         self.container.set_tooltip_text(pd.title());
+        WidgetExt::set_name(&self.child, &pd.id().to_string());
 
-        if let Err(err) = self.set_cover(pd.clone()) {
+        let pd = Arc::new(pd.into());
+        if let Err(err) = self.set_cover(pd) {
             error!("Failed to set a cover: {}", err)
         }
-
-        WidgetExt::set_name(&self.child, &pd.id().to_string());
     }
 
-    fn set_cover(&self, pd: Arc<Podcast>) -> Result<(), Error> {
+    fn set_cover(&self, pd: Arc<PodcastCoverQuery>) -> Result<(), Error> {
-        let image = get_pixbuf_from_path(&pd.clone().into(), 256)?;
+        set_image_from_path(&self.cover, pd, 256)
-        self.cover.set_from_pixbuf(&image);
-        Ok(())
     }
 }

hammond-gtk/src/widgets/show.rs

@@ -10,7 +10,7 @@ use hammond_data::dbqueries;
 use hammond_data::utils::replace_extra_spaces;
 
 use app::Action;
-use utils::get_pixbuf_from_path;
+use utils::set_image_from_path;
 use widgets::episode::episodes_listbox;
 
 use std::sync::Arc;
@@ -113,9 +113,7 @@ impl ShowWidget {
 
     /// Set the show cover.
     fn set_cover(&self, pd: Arc<Podcast>) -> Result<(), Error> {
-        let image = get_pixbuf_from_path(&pd.into(), 128)?;
+        set_image_from_path(&self.cover, Arc::new(pd.into()), 128)
-        self.cover.set_from_pixbuf(&image);
-        Ok(())
     }
 
     /// Set the descripton text.