Blob Blame History Raw
From 26a44f8b7e9ce4e5e9a097f5fedc04d4717fd51d Mon Sep 17 00:00:00 2001
From: Pranav Kant <pranavk@libreoffice.org>
Date: Sun, 1 Nov 2015 17:11:09 +0530
Subject: [PATCH 275/398] lokdocview: Don't render tiles while tile buffer has
 changed

This is common when widget gets a zoom request, resulting in a
new tile buffer, and the tiles from the old tile buffer are still
waiting to be processed in the LOK thread, for old tile buffer. If
we allow these useless operations to execute successfully, they
would end up writing in new tile buffer giving false results.

Lets tag every paint tile operations with their respective tile
buffer during `task` creation, and then check whether the tile
buffer has changed or not before writing to the tile buffer.

Change-Id: If784341a67ad430bc3415b765137badaad6b97f6
Reviewed-on: https://gerrit.libreoffice.org/19726
Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>
Tested-by: Miklos Vajna <vmiklos@collabora.co.uk>
(cherry picked from commit 93f98e98e42d75914a3e1d8f85bd3c6328d2e111)
---
 libreofficekit/source/gtk/lokdocview.cxx | 38 ++++++++++++++++++++++++++++++--
 libreofficekit/source/gtk/tilebuffer.cxx |  6 +++++
 libreofficekit/source/gtk/tilebuffer.hxx | 15 +++++++++++++
 3 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/libreofficekit/source/gtk/lokdocview.cxx b/libreofficekit/source/gtk/lokdocview.cxx
index 9cf78a3f5429..575116f4d028 100644
--- a/libreofficekit/source/gtk/lokdocview.cxx
+++ b/libreofficekit/source/gtk/lokdocview.cxx
@@ -912,7 +912,12 @@ paintTileCallback(GObject* sourceObject, GAsyncResult* res, gpointer userData)
     GdkPixbuf* pPixBuf = static_cast<GdkPixbuf*>(paintTileFinish(pDocView, res, &error));
     if (error != NULL)
     {
-        g_warning("Unable to get painted GdkPixbuf: %s", error->message);
+        if (error->domain == LOK_TILEBUFFER_ERROR &&
+            error->code == LOK_TILEBUFFER_CHANGED)
+            g_info("Skipping paint tile request because corresponding"
+                   "tile buffer has been destroyed");
+        else
+            g_warning("Unable to get painted GdkPixbuf: %s", error->message);
         g_error_free(error);
         return;
     }
@@ -977,6 +982,7 @@ renderDocument(LOKDocView* pDocView, cairo_t* pCairo)
                 pLOEvent->m_nPaintTileX = nRow;
                 pLOEvent->m_nPaintTileY = nColumn;
                 pLOEvent->m_fPaintTileZoom = priv->m_fZoom;
+                pLOEvent->m_pTileBuffer = &*priv->m_pTileBuffer;
                 GTask* task = g_task_new(pDocView, NULL, paintTileCallback, pLOEvent);
                 g_task_set_task_data(task, pLOEvent, LOEvent::destroy);
 
@@ -1541,6 +1547,17 @@ paintTileInThread (gpointer data)
     LOKDocView* pDocView = LOK_DOC_VIEW(g_task_get_source_object(task));
     LOKDocViewPrivate& priv = getPrivate(pDocView);
     LOEvent* pLOEvent = static_cast<LOEvent*>(g_task_get_task_data(task));
+
+    // check if "source" tile buffer is different from "current" tile buffer
+    if (pLOEvent->m_pTileBuffer != &*priv->m_pTileBuffer)
+    {
+        pLOEvent->m_pTileBuffer = nullptr;
+        g_task_return_new_error(task,
+                                LOK_TILEBUFFER_ERROR,
+                                LOK_TILEBUFFER_CHANGED,
+                                "TileBuffer has changed");
+        return;
+    }
     std::unique_ptr<TileBuffer>& buffer = priv->m_pTileBuffer;
     int index = pLOEvent->m_nPaintTileX * buffer->m_nWidth + pLOEvent->m_nPaintTileY;
     if (buffer->m_mTiles.find(index) != buffer->m_mTiles.end() &&
@@ -1550,7 +1567,10 @@ paintTileInThread (gpointer data)
     GdkPixbuf* pPixBuf = gdk_pixbuf_new(GDK_COLORSPACE_RGB, TRUE, 8, nTileSizePixels, nTileSizePixels);
     if (!pPixBuf)
     {
-        g_info ("Error allocating memory to pixbuf");
+        g_task_return_new_error(task,
+                                LOK_TILEBUFFER_ERROR,
+                                LOK_TILEBUFFER_MEMORY,
+                                "Error allocating memory to GdkPixbuf");
         return;
     }
 
@@ -1574,6 +1594,20 @@ paintTileInThread (gpointer data)
                                          pixelToTwip(nTileSizePixels, pLOEvent->m_fPaintTileZoom),
                                          pixelToTwip(nTileSizePixels, pLOEvent->m_fPaintTileZoom));
 
+    // Its likely that while the tilebuffer has changed, one of the paint tile
+    // requests has passed the previous check at start of this function, and has
+    // rendered the tile already. We want to stop such rendered tiles from being
+    // stored in new tile buffer.
+    if (pLOEvent->m_pTileBuffer != &*priv->m_pTileBuffer)
+    {
+        pLOEvent->m_pTileBuffer = nullptr;
+        g_task_return_new_error(task,
+                                LOK_TILEBUFFER_ERROR,
+                                LOK_TILEBUFFER_CHANGED,
+                                "TileBuffer has changed");
+        return;
+    }
+
     g_task_return_pointer(task, pPixBuf, g_object_unref);
 }
 
diff --git a/libreofficekit/source/gtk/tilebuffer.cxx b/libreofficekit/source/gtk/tilebuffer.cxx
index 1158209b2273..32a9534e884c 100644
--- a/libreofficekit/source/gtk/tilebuffer.cxx
+++ b/libreofficekit/source/gtk/tilebuffer.cxx
@@ -121,4 +121,10 @@ void LOEvent::destroy(void* pMemory)
     delete pLOEvent;
 }
 
+GQuark
+LOKTileBufferErrorQuark(void)
+{
+    return g_quark_from_static_string("lok-tilebuffer-error");
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/libreofficekit/source/gtk/tilebuffer.hxx b/libreofficekit/source/gtk/tilebuffer.hxx
index 8a8569eeb306..9407257e5fec 100644
--- a/libreofficekit/source/gtk/tilebuffer.hxx
+++ b/libreofficekit/source/gtk/tilebuffer.hxx
@@ -19,6 +19,8 @@
 #include <LibreOfficeKit/LibreOfficeKitEnums.h>
 #include <LibreOfficeKit/LibreOfficeKitGtk.h>
 
+#define LOK_TILEBUFFER_ERROR (LOKTileBufferErrorQuark())
+
 // We know that VirtualDevices use a DPI of 96.
 const int DPI = 96;
 // Lets use a square of side 256 pixels for each tile.
@@ -45,6 +47,11 @@ float pixelToTwip(float fInput, float zoom);
 float twipToPixel(float fInput, float zoom);
 
 /**
+   Gets GQuark identifying this tile buffer errors
+*/
+GQuark LOKTileBufferErrorQuark(void);
+
+/**
    This class represents a single tile in the tile buffer.
    It encloses a reference to GdkPixBuf containing the pixel data of the tile.
 */
@@ -155,6 +162,12 @@ enum
     LOK_SET_GRAPHIC_SELECTION
 };
 
+enum
+{
+    LOK_TILEBUFFER_CHANGED,
+    LOK_TILEBUFFER_MEMORY
+};
+
 /**
    A struct that we use to store the data about the LOK call.
 
@@ -200,6 +213,7 @@ struct LOEvent
     int m_nPaintTileX;
     int m_nPaintTileY;
     float m_fPaintTileZoom;
+    TileBuffer* m_pTileBuffer;
     ///@}
 
     /// @name postMouseEvent parameters
@@ -235,6 +249,7 @@ struct LOEvent
         , m_nPaintTileX(0)
         , m_nPaintTileY(0)
         , m_fPaintTileZoom(0)
+        , m_pTileBuffer(nullptr)
         , m_nPostMouseEventType(0)
         , m_nPostMouseEventX(0)
         , m_nPostMouseEventY(0)
-- 
2.12.0