[Libreoffice-commits] core.git: libreofficekit/source

Pranav Kant pranavk at libreoffice.org
Wed Nov 4 02:33:16 PST 2015


 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(-)

New commits:
commit 93f98e98e42d75914a3e1d8f85bd3c6328d2e111
Author: Pranav Kant <pranavk at libreoffice.org>
Date:   Sun Nov 1 17:11:09 2015 +0530

    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 at collabora.co.uk>
    Tested-by: Miklos Vajna <vmiklos at collabora.co.uk>

diff --git a/libreofficekit/source/gtk/lokdocview.cxx b/libreofficekit/source/gtk/lokdocview.cxx
index 82d119c..dcf6d5f 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 6d447a8d..330b446 100644
--- a/libreofficekit/source/gtk/tilebuffer.cxx
+++ b/libreofficekit/source/gtk/tilebuffer.cxx
@@ -115,4 +115,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 e7a794a..7127e0b 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.
 */
@@ -153,6 +160,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.
 
@@ -198,6 +211,7 @@ struct LOEvent
     int m_nPaintTileX;
     int m_nPaintTileY;
     float m_fPaintTileZoom;
+    TileBuffer* m_pTileBuffer;
     ///@}
 
     /// @name postMouseEvent parameters
@@ -233,6 +247,7 @@ struct LOEvent
         , m_nPaintTileX(0)
         , m_nPaintTileY(0)
         , m_fPaintTileZoom(0)
+        , m_pTileBuffer(nullptr)
         , m_nPostMouseEventType(0)
         , m_nPostMouseEventX(0)
         , m_nPostMouseEventY(0)


More information about the Libreoffice-commits mailing list