Finish moving of the page cache

Add two more signals to ZathuraRenderRequest to notify pages if they are cached
or not. This allows us to move some logic away from
cb_view_vadjutment_value_changed to more appropriate places.

ZathuraPageWidget will now release the surface if
* it gets the signal that the page is no longer cached and the page is
  invisible,
* the page is not cached and the render request is aborted.

Signed-off-by: Sebastian Ramacher <sebastian+dev@ramacher.at>

callbacks.c

@@ -60,7 +60,7 @@ cb_view_vadjustment_value_changed(GtkAdjustment* GIRARA_UNUSED(adjustment), gpoi
   GtkAdjustment* view_hadjustment = gtk_scrolled_window_get_hadjustment(GTK_SCROLLED_WINDOW(zathura->ui.session->gtk.view));
 
   /* current adjustment values */
-  GdkRectangle view_rect = {
+  const GdkRectangle view_rect = {
     .x      = 0,
     .y      = 0,
     .width  = gtk_adjustment_get_page_size(view_hadjustment),
@@ -70,15 +70,15 @@ cb_view_vadjustment_value_changed(GtkAdjustment* GIRARA_UNUSED(adjustment), gpoi
   int page_padding = 1;
   girara_setting_get(zathura->ui.session, "page-padding", &page_padding);
 
-  GdkRectangle center = {
+  const GdkRectangle center = {
     .x      = (view_rect.width  + 1) / 2,
     .y      = (view_rect.height + 1) / 2,
     .width  = (2 * page_padding) + 1,
     .height = (2 * page_padding) + 1
   };
 
-  unsigned int number_of_pages = zathura_document_get_number_of_pages(zathura->document);
+  const unsigned int number_of_pages = zathura_document_get_number_of_pages(zathura->document);
-  double scale = zathura_document_get_scale(zathura->document);
+  const double scale = zathura_document_get_scale(zathura->document);
 
   bool updated = false;
   /* find page that fits */
@@ -91,8 +91,8 @@ cb_view_vadjustment_value_changed(GtkAdjustment* GIRARA_UNUSED(adjustment), gpoi
     };
     GtkWidget* page_widget = zathura_page_get_widget(zathura, page);
     ZathuraPage* zathura_page_widget = ZATHURA_PAGE(page_widget);
-    gtk_widget_translate_coordinates(page_widget,
+    gtk_widget_translate_coordinates(page_widget, zathura->ui.session->gtk.view,
-                                     zathura->ui.session->gtk.view, 0, 0, &page_rect.x, &page_rect.y);
+        0, 0, &page_rect.x, &page_rect.y);
 
     if (gdk_rectangle_intersect(&view_rect, &page_rect, NULL) == TRUE) {
       if (zathura_page_get_visibility(page) == false) {
@@ -106,14 +106,10 @@ cb_view_vadjustment_value_changed(GtkAdjustment* GIRARA_UNUSED(adjustment), gpoi
         updated = true;
       }
     } else {
+      if (zathura_page_get_visibility(page) == true) {
         zathura_page_set_visibility(page, false);
         /* If a page becomes invisible, abort the render request. */
         zathura_page_widget_abort_render_request(zathura_page_widget);
-      /* if the page is not visible and not cached, but still has a surface, we
-       * need to get rid of the surface */
-      if (zathura_page_widget_have_surface(zathura_page_widget) == true &&
-          zathura_page_cache_is_cached(zathura->sync.render_thread, zathura_page_get_index(page)) == false) {
-        zathura_page_widget_update_surface(zathura_page_widget, NULL);
       }
 
       girara_list_t* results = NULL;

page-widget.c

@@ -24,8 +24,8 @@ typedef struct zathura_page_widget_private_s {
   zathura_t* zathura; /**< Zathura object */
   cairo_surface_t* surface; /**< Cairo surface */
   ZathuraRenderRequest* render_request; /* Request object */
-  gint64 last_view; /**< Last time the page has been viewed */
   mutex lock; /**< Lock */
+  bool cached; /**< Cached state */
 
   struct {
     girara_list_t* list; /**< List of links on the page */
@@ -57,7 +57,8 @@ typedef struct zathura_page_widget_private_s {
 } zathura_page_widget_private_t;
 
 #define ZATHURA_PAGE_GET_PRIVATE(obj) \
-  (G_TYPE_INSTANCE_GET_PRIVATE ((obj), ZATHURA_TYPE_PAGE, zathura_page_widget_private_t))
+  (G_TYPE_INSTANCE_GET_PRIVATE((obj), ZATHURA_TYPE_PAGE, \
+                               zathura_page_widget_private_t))
 
 static gboolean zathura_page_widget_draw(GtkWidget* widget, cairo_t* cairo);
 #if GTK_MAJOR_VERSION == 2
@@ -77,6 +78,8 @@ static gboolean cb_zathura_page_widget_popup_menu(GtkWidget* widget);
 static void cb_menu_image_copy(GtkMenuItem* item, ZathuraPage* page);
 static void cb_menu_image_save(GtkMenuItem* item, ZathuraPage* page);
 static void cb_update_surface(ZathuraRenderRequest* request, cairo_surface_t* surface, void* data);
+static void cb_cache_added(ZathuraRenderRequest* request, void* data);
+static void cb_cache_invalidated(ZathuraRenderRequest* request, void* data);
 
 enum properties_e {
   PROP_0,
@@ -108,10 +111,10 @@ zathura_page_widget_class_init(ZathuraPageClass* class)
 
   /* overwrite methods */
   GtkWidgetClass* widget_class = GTK_WIDGET_CLASS(class);
-#if GTK_MAJOR_VERSION == 3
+#if GTK_MAJOR_VERSION == 2
-  widget_class->draw                 = zathura_page_widget_draw;
-#else
   widget_class->expose_event         = zathura_page_widget_expose;
+#else
+  widget_class->draw                 = zathura_page_widget_draw;
 #endif
   widget_class->size_allocate        = zathura_page_widget_size_allocate;
   widget_class->button_press_event   = cb_zathura_page_widget_button_press_event;
@@ -143,8 +146,6 @@ zathura_page_widget_class_init(ZathuraPageClass* class)
                                   g_param_spec_int("search-length", "search-length", "The number of search results", -1, INT_MAX, 0, G_PARAM_READABLE | G_PARAM_STATIC_STRINGS));
   g_object_class_install_property(object_class, PROP_DRAW_SEARCH_RESULTS,
                                   g_param_spec_boolean("draw-search-results", "draw-search-results", "Set to true if search results should be drawn", FALSE, G_PARAM_READABLE | G_PARAM_WRITABLE | G_PARAM_STATIC_STRINGS));
-  g_object_class_install_property(object_class, PROP_LAST_VIEW,
-                                  g_param_spec_int64("last-view", "last-view", "Last time the page has been viewed", -1, G_MAXINT64, 0, G_PARAM_READABLE | G_PARAM_STATIC_STRINGS));
 
   /* add signals */
   signals[TEXT_SELECTED] = g_signal_new("text-selected",
@@ -177,7 +178,7 @@ zathura_page_widget_init(ZathuraPage* widget)
   priv->page             = NULL;
   priv->surface          = NULL;
   priv->render_request   = NULL;
-  priv->last_view        = g_get_real_time();
+  priv->cached           = false;
 
   priv->links.list      = NULL;
   priv->links.retrieved = false;
@@ -220,6 +221,10 @@ zathura_page_widget_new(zathura_t* zathura, zathura_page_t* page)
   priv->render_request = zathura_render_request_new(zathura->sync.render_thread, page);
   g_signal_connect_object(priv->render_request, "completed",
       G_CALLBACK(cb_update_surface), widget, 0);
+  g_signal_connect_object(priv->render_request, "cache-added",
+      G_CALLBACK(cb_cache_added), widget, 0);
+  g_signal_connect_object(priv->render_request, "cache-invalidated",
+      G_CALLBACK(cb_cache_invalidated), widget, 0);
 
   return GTK_WIDGET(ret);
 }
@@ -355,9 +360,6 @@ zathura_page_widget_get_property(GObject* object, guint prop_id, GValue* value,
     case PROP_SEARCH_RESULTS:
       g_value_set_pointer(value, priv->search.list);
       break;
-    case PROP_LAST_VIEW:
-      g_value_set_int64(value, priv->last_view);
-      break;
     case PROP_DRAW_SEARCH_RESULTS:
       g_value_set_boolean(value, priv->search.draw);
       break;
@@ -533,7 +535,7 @@ zathura_page_widget_draw(GtkWidget* widget, cairo_t* cairo)
     }
 
     /* render real page */
-    zathura_render_request(priv->render_request, priv->last_view);
+    zathura_render_request(priv->render_request, g_get_real_time());
   }
   mutex_unlock(&(priv->lock));
   return FALSE;
@@ -556,13 +558,9 @@ zathura_page_widget_update_surface(ZathuraPage* widget, cairo_surface_t* surface
     priv->surface = NULL;
   }
   if (surface != NULL) {
-    /* if we're not visible or not cached, we don't care about the surface */
-    /*if (zathura_page_get_visibility(priv->page) == true ||
-        zathura_page_cache_is_cached(priv->zathura, zathura_page_get_index(priv->page)) == true) */ {
     priv->surface = surface;
     cairo_surface_reference(surface);
   }
-  }
   mutex_unlock(&(priv->lock));
   /* force a redraw here */
   if (priv->surface != NULL) {
@@ -571,13 +569,41 @@ zathura_page_widget_update_surface(ZathuraPage* widget, cairo_surface_t* surface
 }
 
 static void
-cb_update_surface(ZathuraRenderRequest* UNUSED(request), cairo_surface_t* surface, void* data)
+cb_update_surface(ZathuraRenderRequest* UNUSED(request),
+    cairo_surface_t* surface, void* data)
 {
   ZathuraPage* widget = data;
   g_return_if_fail(ZATHURA_IS_PAGE(widget));
   zathura_page_widget_update_surface(widget, surface);
 }
 
+static void
+cb_cache_added(ZathuraRenderRequest* UNUSED(request), void* data)
+{
+  ZathuraPage* widget = data;
+  g_return_if_fail(ZATHURA_IS_PAGE(widget));
+
+  zathura_page_widget_private_t* priv = ZATHURA_PAGE_GET_PRIVATE(widget);
+  priv->cached = true;
+}
+
+static void
+cb_cache_invalidated(ZathuraRenderRequest* UNUSED(request), void* data)
+{
+  ZathuraPage* widget = data;
+  g_return_if_fail(ZATHURA_IS_PAGE(widget));
+
+  zathura_page_widget_private_t* priv = ZATHURA_PAGE_GET_PRIVATE(widget);
+  if (zathura_page_widget_have_surface(widget) == true &&
+      priv->cached == true &&
+      zathura_page_get_visibility(priv->page) == false) {
+    /* The page was in the cache but got removed and is invisible, so get rid of
+     * the surface. */
+    zathura_page_widget_update_surface(widget, NULL);
+  }
+  priv->cached = false;
+}
+
 static void
 zathura_page_widget_size_allocate(GtkWidget* widget, GdkRectangle* allocation)
 {
@@ -594,10 +620,10 @@ redraw_rect(ZathuraPage* widget, zathura_rectangle_t* rectangle)
   grect.y = rectangle->y1;
   grect.width  = (rectangle->x2 + 1) - rectangle->x1;
   grect.height = (rectangle->y2 + 1) - rectangle->y1;
-#if (GTK_MAJOR_VERSION == 3)
+#if GTK_MAJOR_VERSION == 2
-  gtk_widget_queue_draw_area(GTK_WIDGET(widget), grect.x, grect.y, grect.width, grect.height);
-#else
   gdk_window_invalidate_rect(gtk_widget_get_window(GTK_WIDGET(widget)), &grect, TRUE);
+#else
+  gtk_widget_queue_draw_area(GTK_WIDGET(widget), grect.x, grect.y, grect.width, grect.height);
 #endif
 }
 
@@ -921,18 +947,18 @@ cb_menu_image_save(GtkMenuItem* item, ZathuraPage* page)
 void
 zathura_page_widget_update_view_time(ZathuraPage* widget)
 {
-  g_return_if_fail(ZATHURA_IS_PAGE(widget) == TRUE);
+  g_return_if_fail(ZATHURA_IS_PAGE(widget));
   zathura_page_widget_private_t* priv = ZATHURA_PAGE_GET_PRIVATE(widget);
 
   if (zathura_page_get_visibility(priv->page) == true) {
-    priv->last_view = g_get_real_time();
+    zathura_render_request_update_view_time(priv->render_request);
   }
 }
 
 bool
 zathura_page_widget_have_surface(ZathuraPage* widget)
 {
-  g_return_val_if_fail(ZATHURA_IS_PAGE(widget) == TRUE, false);
+  g_return_val_if_fail(ZATHURA_IS_PAGE(widget), false);
   zathura_page_widget_private_t* priv = ZATHURA_PAGE_GET_PRIVATE(widget);
   return priv->surface != NULL;
 }
@@ -943,5 +969,14 @@ zathura_page_widget_abort_render_request(ZathuraPage* widget)
   g_return_if_fail(ZATHURA_IS_PAGE(widget));
   zathura_page_widget_private_t* priv = ZATHURA_PAGE_GET_PRIVATE(widget);
   zathura_render_request_abort(priv->render_request);
+
+  /* Make sure that if we are not cached and invisible, that there is no
+   * surface.
+   *
+   * TODO: Maybe this should be moved somewhere else. */
+  if (zathura_page_widget_have_surface(widget) == true &&
+      priv->cached == false) {
+    zathura_page_widget_update_surface(widget, NULL);
+  }
 }
 

render.c

@@ -28,7 +28,6 @@ static void page_cache_invalidate_all(ZathuraRenderer* renderer);
 static bool page_cache_is_full(ZathuraRenderer* renderer, bool* result);
 
 
-
 /* private data for ZathuraRenderer */
 typedef struct private_s {
   GThreadPool* pool; /**< Pool of threads */
@@ -174,6 +173,8 @@ renderer_register_request(ZathuraRenderer* renderer,
 
 enum {
   REQUEST_COMPLETED,
+  REQUEST_CACHE_ADDED,
+  REQUEST_CACHE_INVALIDATED,
   REQUEST_LAST_SIGNAL
 };
 
@@ -199,6 +200,26 @@ zathura_render_request_class_init(ZathuraRenderRequestClass* class)
       G_TYPE_NONE,
       1,
       G_TYPE_POINTER);
+
+  request_signals[REQUEST_CACHE_ADDED] = g_signal_new("cache-added",
+      ZATHURA_TYPE_RENDER_REQUEST,
+      G_SIGNAL_RUN_LAST,
+      0,
+      NULL,
+      NULL,
+      g_cclosure_marshal_generic,
+      G_TYPE_NONE,
+      0);
+
+  request_signals[REQUEST_CACHE_INVALIDATED] = g_signal_new("cache-invalidated",
+      ZATHURA_TYPE_RENDER_REQUEST,
+      G_SIGNAL_RUN_LAST,
+      0,
+      NULL,
+      NULL,
+      g_cclosure_marshal_generic,
+      G_TYPE_NONE,
+      0);
 }
 
 static void
@@ -373,12 +394,12 @@ zathura_render_request(ZathuraRenderRequest* request, gint64 last_view_time)
   g_return_if_fail(ZATHURA_IS_RENDER_REQUEST(request));
 
   request_private_t* request_priv = REQUEST_GET_PRIVATE(request);
-  private_t* priv = GET_PRIVATE(request_priv->renderer);
   if (request_priv->requested == false) {
     request_priv->requested = true;
     request_priv->aborted = false;
     request_priv->last_view_time = last_view_time;
 
+    private_t* priv = GET_PRIVATE(request_priv->renderer);
     g_thread_pool_push(priv->pool, request, NULL);
   }
 }
@@ -394,6 +415,14 @@ zathura_render_request_abort(ZathuraRenderRequest* request)
   }
 }
 
+void
+zathura_render_request_update_view_time(ZathuraRenderRequest* request)
+{
+  g_return_if_fail(ZATHURA_IS_RENDER_REQUEST(request));
+
+  request_private_t* request_priv = REQUEST_GET_PRIVATE(request);
+  request_priv->last_view_time = g_get_real_time();
+}
 
 /* render job */
 
@@ -485,8 +514,10 @@ recolor(private_t* priv, unsigned int page_width, unsigned int page_height,
 {
   /* uses a representation of a rgb color as follows:
      - a lightness scalar (between 0,1), which is a weighted average of r, g, b,
-     - a hue vector, which indicates a radian direction from the grey axis, inside the equal lightness plane.
+     - a hue vector, which indicates a radian direction from the grey axis,
-     - a saturation scalar between 0,1. It is 0 when grey, 1 when the color is in the boundary of the rgb cube.
+       inside the equal lightness plane.
+     - a saturation scalar between 0,1. It is 0 when grey, 1 when the color is
+       in the boundary of the rgb cube.
   */
 
   const int rowstride  = cairo_image_surface_get_stride(surface);
@@ -569,9 +600,11 @@ render(ZathuraRenderRequest* request, ZathuraRenderer* renderer)
   /* create cairo surface */
   unsigned int page_width  = 0;
   unsigned int page_height = 0;
-  const double real_scale = page_calc_height_width(page, &page_height, &page_width, false);
+  const double real_scale = page_calc_height_width(page, &page_height,
+      &page_width, false);
 
-  cairo_surface_t* surface = cairo_image_surface_create(CAIRO_FORMAT_RGB24, page_width, page_height);
+  cairo_surface_t* surface = cairo_image_surface_create(CAIRO_FORMAT_RGB24,
+      page_width, page_height);
   if (surface == NULL) {
     return false;
   }
@@ -649,9 +682,11 @@ render_job(void* data, void* user_data)
     return;
   }
 
-  girara_debug("Rendering page %d ...", zathura_page_get_index(request_priv->page) + 1);
+  girara_debug("Rendering page %d ...",
+      zathura_page_get_index(request_priv->page) + 1);
   if (render(request, renderer) != true) {
-    girara_error("Rendering failed (page %d)\n", zathura_page_get_index(request_priv->page) + 1);
+    girara_error("Rendering failed (page %d)\n",
+        zathura_page_get_index(request_priv->page) + 1);
     request_priv->requested = false;
   }
 }
@@ -667,7 +702,8 @@ render_all(zathura_t* zathura)
   /* unmark all pages */
   unsigned int number_of_pages = zathura_document_get_number_of_pages(zathura->document);
   for (unsigned int page_id = 0; page_id < number_of_pages; page_id++) {
-    zathura_page_t* page = zathura_document_get_page(zathura->document, page_id);
+    zathura_page_t* page = zathura_document_get_page(zathura->document,
+        page_id);
     unsigned int page_height = 0, page_width = 0;
     page_calc_height_width(page, &page_height, &page_width, true);
 
@@ -700,8 +736,8 @@ render_thread_sort(gconstpointer a, gconstpointer b, gpointer UNUSED(data))
 
 /* cache functions */
 
-bool
+static bool
-zathura_page_cache_is_cached(ZathuraRenderer* renderer, unsigned int page_index)
+page_cache_is_cached(ZathuraRenderer* renderer, unsigned int page_index)
 {
   g_return_val_if_fail(renderer != NULL, false);
   private_t* priv = GET_PRIVATE(renderer);
@@ -742,25 +778,24 @@ page_cache_lru_invalidate(ZathuraRenderer* renderer)
 
   ssize_t lru_index = 0;
   gint64 lru_view_time = G_MAXINT64;
-
+  ZathuraRenderRequest* request = NULL;
   for (size_t i = 0; i < priv->page_cache.size; ++i) {
-    ZathuraRenderRequest* request = girara_list_find(priv->requests,
+    ZathuraRenderRequest* tmp_request = girara_list_find(priv->requests,
         find_request_by_page_index, &priv->page_cache.cache[i]);
-    g_return_val_if_fail(request != NULL, -1);
+    g_return_val_if_fail(tmp_request != NULL, -1);
-    request_private_t* request_priv = REQUEST_GET_PRIVATE(request);
+    request_private_t* request_priv = REQUEST_GET_PRIVATE(tmp_request);
 
     if (request_priv->last_view_time < lru_view_time) {
       lru_view_time = request_priv->last_view_time;
       lru_index = i;
+      request = tmp_request;
     }
   }
 
-  ZathuraRenderRequest* request = girara_list_find(priv->requests,
-      find_request_by_page_index, &priv->page_cache.cache[lru_index]);
   request_private_t* request_priv = REQUEST_GET_PRIVATE(request);
 
   /* emit the signal */
-  g_signal_emit(request, request_signals[REQUEST_COMPLETED], 0, NULL);
+  g_signal_emit(request, request_signals[REQUEST_CACHE_INVALIDATED], 0);
   girara_debug("Invalidated page %d at cache index %zd",
       zathura_page_get_index(request_priv->page) + 1, lru_index);
   priv->page_cache.cache[lru_index] = -1;
@@ -772,18 +807,18 @@ page_cache_lru_invalidate(ZathuraRenderer* renderer)
 static bool
 page_cache_is_full(ZathuraRenderer* renderer, bool* result)
 {
-  g_return_val_if_fail(renderer != NULL && result != NULL, false);
+  g_return_val_if_fail(ZATHURA_IS_RENDERER(renderer) && result != NULL, false);
-  private_t* priv = GET_PRIVATE(renderer);
 
+  private_t* priv = GET_PRIVATE(renderer);
   *result = priv->page_cache.num_cached_pages == priv->page_cache.size;
 
   return true;
 }
 
-void
+static void
 page_cache_invalidate_all(ZathuraRenderer* renderer)
 {
-  g_return_if_fail(renderer != NULL);
+  g_return_if_fail(ZATHURA_IS_RENDERER(renderer));
 
   private_t* priv = GET_PRIVATE(renderer);
   for (size_t i = 0; i < priv->page_cache.size; ++i) {
@@ -795,8 +830,8 @@ page_cache_invalidate_all(ZathuraRenderer* renderer)
 void
 zathura_page_cache_add(ZathuraRenderer* renderer, unsigned int page_index)
 {
-  g_return_if_fail(renderer != NULL);
+  g_return_if_fail(ZATHURA_IS_RENDERER(renderer));
-  if (zathura_page_cache_is_cached(renderer, page_index) == true) {
+  if (page_cache_is_cached(renderer, page_index) == true) {
     return;
   }
 
@@ -805,8 +840,7 @@ zathura_page_cache_add(ZathuraRenderer* renderer, unsigned int page_index)
   if (page_cache_is_full(renderer, &full) == false) {
     return;
   } else if (full == true) {
-    ssize_t idx = page_cache_lru_invalidate(renderer);
+    const ssize_t idx = page_cache_lru_invalidate(renderer);
-
     if (idx == -1) {
       return;
     }
@@ -814,11 +848,14 @@ zathura_page_cache_add(ZathuraRenderer* renderer, unsigned int page_index)
     priv->page_cache.cache[idx] = page_index;
     ++priv->page_cache.num_cached_pages;
     girara_debug("Page %d is cached at cache index %zd", page_index + 1, idx);
-    return;
+  } else {
-  }
-
     priv->page_cache.cache[priv->page_cache.num_cached_pages++] = page_index;
     girara_debug("Page %d is cached at cache index %zu", page_index + 1,
         priv->page_cache.num_cached_pages - 1);
-  return;
+  }
+
+  ZathuraRenderRequest* request = girara_list_find(priv->requests,
+        find_request_by_page_index, &page_index);
+  g_return_if_fail(request != NULL);
+  g_signal_emit(request, request_signals[REQUEST_CACHE_ADDED], 0);
 }

render.h

@@ -125,20 +125,6 @@ void zathura_renderer_unlock(ZathuraRenderer* renderer);
 void zathura_page_cache_add(ZathuraRenderer* renderer,
     unsigned int page_index);
 
-/**
- * Checks if the given page is cached
- *
- * @param zathura The zathura session
- * @param page_index The index of the page that may be cached
- *
- * @return true if page is cached otherwise false
- */
-bool zathura_page_cache_is_cached(ZathuraRenderer* renderer,
-    unsigned int page_index);
-
-
-
-
 
 typedef struct zathura_render_request_s ZathuraRenderRequest;
 typedef struct zathura_render_request_class_s ZathuraRenderRequestClass;
@@ -200,6 +186,8 @@ void zathura_render_request(ZathuraRenderRequest* request,
  */
 void zathura_render_request_abort(ZathuraRenderRequest* request);
 
+void zathura_render_request_update_view_time(ZathuraRenderRequest* request);
+
 /**
  * This function is used to unmark all pages as not rendered. This should
  * be used if all pages should be rendered again (e.g.: the zoom level or the