ubuntu-touch-coreapps-reviewers team mailing list archive

[Roman Shchekin <mrqtros@xxxxxxxxx>]

Roman Shchekin has proposed merging lp:~mrqtros/ubuntu-docviewer-app/ubuntu-docviewer-app-render-engine-tuning into lp:ubuntu-docviewer-app/reboot.

Commit message:
Code of RenderEngine become more SOLID (still a lot of work to do).

Requested reviews:
  Ubuntu Document Viewer Developers (ubuntu-docviewer-dev)

For more details, see:
https://code.launchpad.net/~mrqtros/ubuntu-docviewer-app/ubuntu-docviewer-app-render-engine-tuning/+merge/276337

Code of RenderEngine become more SOLID (still a lot of work to do).
-- 
Your team Ubuntu Document Viewer Developers is requested to review the proposed merge of lp:~mrqtros/ubuntu-docviewer-app/ubuntu-docviewer-app-render-engine-tuning into lp:ubuntu-docviewer-app/reboot.

=== modified file 'po/com.ubuntu.docviewer.pot'
--- po/com.ubuntu.docviewer.pot	2015-10-20 18:22:16 +0000
+++ po/com.ubuntu.docviewer.pot	2015-10-31 20:38:57 +0000
@@ -8,7 +8,7 @@
 msgstr ""
 "Project-Id-Version: \n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2015-10-20 20:21+0200\n"
+"POT-Creation-Date: 2015-10-31 23:16+0300\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@xxxxxx>\n"
@@ -458,10 +458,10 @@
 msgid "copy %1"
 msgstr ""
 
-#: /tmp/build-reboot-uitk12-Desktop-Default/po/com.ubuntu.docviewer.desktop.in.in.h:1
+#: /home/qtros/dev/ubuntu-docviewer-app-render-engine-tuning-build/po/com.ubuntu.docviewer.desktop.in.in.h:1
 msgid "Document Viewer"
 msgstr ""
 
-#: /tmp/build-reboot-uitk12-Desktop-Default/po/com.ubuntu.docviewer.desktop.in.in.h:2
+#: /home/qtros/dev/ubuntu-docviewer-app-render-engine-tuning-build/po/com.ubuntu.docviewer.desktop.in.in.h:2
 msgid "documents;viewer;pdf;reader;"
 msgstr ""

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/renderengine.cpp'
--- src/plugin/libreofficetoolkit-qml-plugin/renderengine.cpp	2015-10-18 21:27:16 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/renderengine.cpp	2015-10-31 20:38:57 +0000
@@ -5,9 +5,9 @@
 RenderEngine* RenderEngine::s_instance = nullptr;
 
 RenderEngine::RenderEngine():
-    QObject(nullptr),
-    m_activeTaskCount(0),
-    m_lastPart(-1)
+    QObject(nullptr)
+    ,m_activeTaskCount(0)
+    ,m_lastTask(nullptr)
 {
     int itc = QThread::idealThreadCount();
     m_idealThreadCount = itc == -1 ? DefaultIdealThreadCount : itc;
@@ -55,7 +55,7 @@
         auto task = m_queue.at(i);
         if (task->id() == id) {
             m_queue.removeAt(i);
-            disposeTask(task);
+            disposeLater(task);
             break;
         }
     }
@@ -65,6 +65,11 @@
 {
     m_activeTaskCount--;
 
+    if (!m_activeTaskCount) {
+        m_lastTask = nullptr;
+        doDispose();
+    }
+
     switch (task->type())
     {
     case RttTile:
@@ -73,16 +78,27 @@
     case RttImpressThumbnail:
         Q_EMIT thumbnailRenderFinished(task->id(), img);
         break;
+    case RttPdfPage:
+    case RttUnknown:
+    default:
+        break;
     }
 
-    disposeTask(task);
-
     doNextTask();
-}
-
-void RenderEngine::disposeTask(AbstractRenderTask *task)
-{
-    delete task;
+
+    disposeLater(task);
+}
+
+void RenderEngine::disposeLater(AbstractRenderTask *task)
+{
+    m_disposedTasks.append(task);
+}
+
+void RenderEngine::doDispose()
+{
+    for (int i = 0; i < m_disposedTasks.size(); ++i)
+        delete m_disposedTasks.at(i);
+    m_disposedTasks.clear();
 }
 
 void RenderEngine::doNextTask()
@@ -97,20 +113,15 @@
 
     AbstractRenderTask* task = m_queue.head();
 
-    // LoRenderTask requires special check.
-    if (task->type() == RttTile || task->type() == RttImpressThumbnail) {
-        LoRenderTask* loTask = static_cast<LoRenderTask*>(task);
-
-        if (m_activeTaskCount && loTask->part() != m_lastPart)
-            return;
-
-        // Set correct part.
-        m_lastPart = loTask->part();
-        loTask->document()->setDocumentPart(m_lastPart);
-    }
+    // If some tasks already in progress, we should ask task about
+    // compatibility of parallel execution with last the task.
+    if (m_activeTaskCount && !task->canBeRunInParallel(m_lastTask))
+        return;
+
+    task->prepare();
 
     m_activeTaskCount++;
-    m_queue.dequeue();
+    m_lastTask = m_queue.dequeue();
 
     QtConcurrent::run( [=] {
         QImage img = task->doWork();

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/renderengine.h'
--- src/plugin/libreofficetoolkit-qml-plugin/renderengine.h	2015-10-18 21:27:16 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/renderengine.h	2015-10-31 20:38:57 +0000
@@ -7,6 +7,7 @@
 #include <QHash>
 #include <QQueue>
 #include <QAtomicInt>
+#include <QList>
 
 #include "lodocument.h"
 #include "rendertask.h"
@@ -45,14 +46,17 @@
 
 private:
     Q_INVOKABLE void internalRenderCallback(AbstractRenderTask* task, QImage img);
-    void disposeTask(AbstractRenderTask* task);
     void doNextTask();
+    void disposeLater(AbstractRenderTask* task); // Delayed deletion, must be used in pair with "doDispose".
+    void doDispose();                            // Deletes marked objects (disposeLater).
 
 private:
     QQueue<AbstractRenderTask*> m_queue;
     int m_activeTaskCount;
     int m_idealThreadCount;
-    int m_lastPart;
+
+    AbstractRenderTask* m_lastTask; // WARNING: valid only when: m_activeTaskCount > 0.
+    QList<AbstractRenderTask*> m_disposedTasks;
 };
 
 #endif // RENDERENGINE_H

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/rendertask.h'
--- src/plugin/libreofficetoolkit-qml-plugin/rendertask.h	2015-10-18 21:27:16 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/rendertask.h	2015-10-31 20:38:57 +0000
@@ -25,6 +25,8 @@
     virtual RenderTaskType type() { return RttUnknown; }
     virtual QImage doWork() = 0 ;
     virtual ~AbstractRenderTask() { }
+    virtual bool canBeRunInParallel(AbstractRenderTask*) { return true; }
+    virtual void prepare() = 0 ;
 
     int id() { return m_id; }
     void setId(int i) { m_id = i; }
@@ -37,6 +39,22 @@
 class LoRenderTask : public AbstractRenderTask
 {
 public:
+    virtual bool canBeRunInParallel(AbstractRenderTask* prevTask)
+    {
+        Q_ASSERT(prevTask != nullptr);
+        if (prevTask->type() == RttTile || prevTask->type() == RttImpressThumbnail) {
+            LoRenderTask* loTask = static_cast<LoRenderTask*>(prevTask);
+
+            // Another document or the same part in the same document can be run parallel.
+            return (loTask->document() != m_document ||
+                    loTask->part() == m_part);
+        }
+
+        return true;
+    }
+
+    virtual void prepare() { m_document->setDocumentPart(m_part); }
+
     int part() { return m_part; }
     void setPart(int p) { m_part = p; }