Add todo for performance improvement

src/cache.rs

@@ -51,25 +51,27 @@ impl PageCache {
         }
     }
 
-    pub fn cache_page(&mut self, page_number: PageNumber, height: i32) -> Option<CacheResponse> {
+    pub fn cache_page(&mut self, page_number: PageNumber, height: i32) -> Result<Option<CacheResponse>> {
         debug!("Caching page {}", page_number);
         if page_number.abs_diff(self.last_requested_page_number)
             > self.max_num_stored_pages.div_ceil(2)
         {
-            debug!("Page too far from reader, aborting caching call");
-            return None;
+            bail!("Page too far from reader, aborting caching call");
         }
-
-        let begin_of_cashing = Instant::now();
         if let Some(page) = self.pages.get(&page_number) {
             if page.height() >= height {
                 debug!("Page already in cache");
-                return None;
+                return Ok(None);
             }
         }
+        if page_number >= self.document.n_pages() as usize {
+            bail!("Requested page {} has too high number and is not in the document", page_number);
+        }
 
+        let begin_of_cashing = Instant::now();
         let mut response = None;
 
+        println!("Getting page form poppler: {}", page_number);
         if let Some(page) = self.document.page(page_number as i32) {
             let pages = vec![Rc::new(page)];
             let texture = draw::draw_pages_to_texture(&pages, height, &self.color_mode);
@@ -91,7 +93,7 @@ impl PageCache {
             page_number,
             begin_of_cashing.elapsed().as_millis()
         );
-        response
+        Ok(response)
     }
 
     fn remove_most_distant_page(&mut self) -> anyhow::Result<()> {
@@ -130,22 +132,24 @@ impl PageCache {
     fn process_command(&mut self, command: CacheCommand) -> Result<Option<CacheResponse>> {
         debug!("Processing command: {:?}...", command);
         match command {
-            CacheCommand::Cache(command) => Ok(self.cache_page(command.page, command.height)),
+            CacheCommand::Cache(command) => Ok(self.cache_page(command.page, command.height)?),
             CacheCommand::Retrieve(command) => match command {
                 RetrievePagesCommand::GetCurrentTwoPages { page_left_number } => {
                     let page_left = self.get_page_or_cache(page_left_number)?;
                     if let Ok(page_right) = self.get_page_or_cache(page_left_number + 1) {
                         Ok(Some(CacheResponse::TwoPagesRetrieved {
+                            page_number_left: page_left_number,
                             page_left,
+                            page_number_right: page_left_number + 1,
                             page_right,
                         }))
                     } else {
-                        Ok(Some(CacheResponse::SinglePageRetrieved { page: page_left }))
+                        Ok(Some(CacheResponse::SinglePageRetrieved { page_number: page_left_number, page: page_left }))
                     }
                 }
                 RetrievePagesCommand::GetCurrentPage { page_number } => {
                     let page = self.get_page_or_cache(page_number)?;
-                    Ok(Some(CacheResponse::SinglePageRetrieved { page }))
+                    Ok(Some(CacheResponse::SinglePageRetrieved { page_number, page }))
                 }
             },
         }
@@ -175,10 +179,13 @@ pub enum RetrievePagesCommand {
 
 pub enum CacheResponse {
     SinglePageRetrieved {
+        page_number: PageNumber,
         page: Rc<MyPageType>,
     },
     TwoPagesRetrieved {
+        page_number_left: PageNumber,
         page_left: Rc<MyPageType>,
+        page_number_right : PageNumber,
         page_right: Rc<MyPageType>,
     },
     PageResolutionUpgraded {
@@ -244,6 +251,7 @@ impl SyncCacheCommandSender {
     pub fn send_cache_commands(&self, pages: &[PageNumber], height: i32) {
         for &page in pages {
             // Make message in front the most important
+            // TODO: the low res cach will never be actually done
             self.channel
                 .borrow_mut()
                 .cache_commands

src/ui.rs

@@ -14,7 +14,7 @@ use gtk::{
 use log::debug;
 
 use crate::{
-    cache::{self, PageNumber, SyncCacheCommandSender},
+    cache::{self, CacheResponse, PageNumber, SyncCacheCommandSender},
     color_mode::{self, ColorMode},
 };
 use glib::clone;
@@ -445,38 +445,7 @@ pub fn load_document(file: impl AsRef<Path>, ui: Rc<RefCell<Ui>>, initial_page_n
     let sender = cache::spawn_sync_cache(
         document,
         color_mode,
-        clone!(@weak ui => move |cache_response| match cache_response {
-                cache::CacheResponse::SinglePageRetrieved { page } => {
-                    ui.borrow_mut().image_left.set_paintable(Some(page.as_ref()));
-                    if ui.borrow().document_canvas.as_ref().map(|canvas| canvas.num_pages.unwrap_or(0)).unwrap_or(0) > 1 {
-                        // Make image invisible but keep free space in layout
-                        ui.borrow_mut().image_right.set_opacity(0.0);
-                    } else {
-                        // Make image invisible and center left page in layout
-                        ui.borrow_mut().image_right.set_visible(false);
-                    }
-                    let area_height = ui.borrow().image_container.height();
-                    ui.borrow().document_canvas.as_ref().unwrap().cache_surrounding_pages(area_height);
-                }
-                cache::CacheResponse::TwoPagesRetrieved {
-                    page_left,
-                    page_right,
-                } => {
-                    ui.borrow_mut().image_left.set_paintable(Some(page_left.as_ref()));
-                    ui.borrow_mut().image_right.set_paintable(Some(page_right.as_ref()));
-                    ui.borrow_mut().image_right.set_visible(true);
-                    ui.borrow_mut().image_right.set_opacity(1.0);
-                    let area_height = ui.borrow().image_container.height();
-                    ui.borrow().document_canvas.as_ref().unwrap().cache_surrounding_pages(area_height);
-                },
-            cache::CacheResponse::PageResolutionUpgraded { page_number, page } => {
-                if ui.borrow().document_canvas.as_ref().unwrap().is_left_page(page_number){
-                    ui.borrow_mut().image_left.set_paintable(Some(page.as_ref()));
-                } else if ui.borrow().document_canvas.as_ref().unwrap().is_right_page(page_number){
-                    ui.borrow_mut().image_right.set_paintable(Some(page.as_ref()));
-                }
-            }
-        }),
+        clone!(@weak ui => move |cache_response| handle_cache_response(ui, cache_response)),
     );
 
     let mut document_canvas = DocumentCanvas::new(path, sender);
@@ -489,3 +458,56 @@ pub fn load_document(file: impl AsRef<Path>, ui: Rc<RefCell<Ui>>, initial_page_n
     update_page_status(&ui.borrow());
     debug!("finished loading document");
 }
+
+fn handle_cache_response(ui: Rc<RefCell<Ui>>, cache_response: CacheResponse) {
+    if let Some(canvas) = ui.borrow().document_canvas.as_ref() {
+        let image_left = &ui.borrow().image_left;
+        let image_right = &ui.borrow().image_right;
+        let area_height = ui.borrow().image_container.height();
+
+        match cache_response {
+            cache::CacheResponse::SinglePageRetrieved { page_number, page } => {
+                if !canvas.is_left_page(page_number) {
+                    debug!("Retrieved page with non up-to-date page_number from cache, ignoring.");
+                    return;
+                }
+
+                image_left.set_paintable(Some(page.as_ref()));
+                if canvas.num_pages.unwrap_or(0) > 1 {
+                    // Make image invisible but keep free space in layout
+                    image_right.set_opacity(0.0);
+                } else {
+                    // Make image invisible and center left page in layout
+                    image_right.set_visible(false);
+                }
+                canvas.cache_surrounding_pages(area_height);
+            }
+            cache::CacheResponse::TwoPagesRetrieved {
+                page_number_left,
+                page_left,
+                page_number_right,
+                page_right,
+            } => {
+                if !canvas.is_left_page(page_number_left)
+                    || !canvas.is_right_page(page_number_right)
+                {
+                    debug!("Retrieved pages with non up-to-date page_number from cache, ignoring.");
+                    return;
+                }
+
+                image_left.set_paintable(Some(page_left.as_ref()));
+                image_right.set_paintable(Some(page_right.as_ref()));
+                image_right.set_visible(true);
+                image_right.set_opacity(1.0);
+                canvas.cache_surrounding_pages(area_height);
+            }
+            cache::CacheResponse::PageResolutionUpgraded { page_number, page } => {
+                if canvas.is_left_page(page_number) {
+                    image_left.set_paintable(Some(page.as_ref()));
+                } else if canvas.is_right_page(page_number) {
+                    image_right.set_paintable(Some(page.as_ref()));
+                }
+            }
+        }
+    }
+}