Issue #1: Optimize page rendering thread management

Pdf4QtLib/sources/pdfcompiler.cpp

@@ -28,6 +28,87 @@
 namespace pdf
 {
 
+PDFAsynchronousPageCompilerWorkerThread::PDFAsynchronousPageCompilerWorkerThread(PDFAsynchronousPageCompiler* parent) :
+    QThread(parent),
+    m_compiler(parent),
+    m_mutex(&m_compiler->m_mutex),
+    m_waitCondition(&m_compiler->m_waitCondition)
+{
+
+}
+
+void PDFAsynchronousPageCompilerWorkerThread::run()
+{
+    QMutexLocker locker(m_mutex);
+    while (!isInterruptionRequested())
+    {
+        if (m_waitCondition->wait(locker.mutex(), QDeadlineTimer(QDeadlineTimer::Forever)))
+        {
+            while (!isInterruptionRequested())
+            {
+                std::vector<PDFAsynchronousPageCompiler::CompileTask> tasks;
+                for (auto& task : m_compiler->m_tasks)
+                {
+                    if (!task.second.finished)
+                    {
+                        tasks.push_back(task.second);
+                    }
+                }
+
+                if (!tasks.empty())
+                {
+                    locker.unlock();
+
+                    // Perform page compilation
+                    auto proxy = m_compiler->getProxy();
+                    proxy->getFontCache()->setCacheShrinkEnabled(this, false);
+
+                    auto compilePage = [proxy](PDFAsynchronousPageCompiler::CompileTask& task) -> PDFPrecompiledPage
+                    {
+                        PDFPrecompiledPage compiledPage;
+                        PDFCMSPointer cms = proxy->getCMSManager()->getCurrentCMS();
+                        PDFRenderer renderer(proxy->getDocument(), proxy->getFontCache(), cms.data(), proxy->getOptionalContentActivity(), proxy->getFeatures(), proxy->getMeshQualitySettings());
+                        renderer.compile(&task.precompiledPage, task.pageIndex);
+                        task.finished = true;
+                        return compiledPage;
+                    };
+                    PDFExecutionPolicy::execute(PDFExecutionPolicy::Scope::Page, tasks.begin(), tasks.end(), compilePage);
+
+                    proxy->getFontCache()->setCacheShrinkEnabled(this, true);
+
+                    // Relock the mutex to write the tasks
+                    locker.relock();
+
+                    // Now, write compiled pages
+                    bool isSomethingWritten = false;
+                    for (auto& task : tasks)
+                    {
+                        if (task.finished)
+                        {
+                            isSomethingWritten = true;
+                            m_compiler->m_tasks[task.pageIndex] = std::move(task);
+                        }
+                    }
+
+                    if (isSomethingWritten)
+                    {
+                        // Why we are unlocking the mutex? Because
+                        // we do not want to emit signals with locked mutexes.
+                        // If direct connection is applied, this can lead to deadlock.
+                        locker.unlock();
+                        emit pageCompiled();
+                        locker.relock();
+                    }
+                }
+                else
+                {
+                    break;
+                }
+            }
+        }
+    }
+}
+
 PDFAsynchronousPageCompiler::PDFAsynchronousPageCompiler(PDFDrawWidgetProxy* proxy) :
     BaseClass(proxy),
     m_proxy(proxy)
@@ -35,13 +116,22 @@ PDFAsynchronousPageCompiler::PDFAsynchronousPageCompiler(PDFDrawWidgetProxy* pro
     m_cache.setMaxCost(128 * 1024 * 1024);
 }
 
+PDFAsynchronousPageCompiler::~PDFAsynchronousPageCompiler()
+{
+    stop(true);
+}
+
 void PDFAsynchronousPageCompiler::start()
 {
     switch (m_state)
     {
         case State::Inactive:
         {
+            Q_ASSERT(!m_thread);
             m_state = State::Active;
+            m_thread = new PDFAsynchronousPageCompilerWorkerThread(this);
+            connect(m_thread, &PDFAsynchronousPageCompilerWorkerThread::pageCompiled, this, &PDFAsynchronousPageCompiler::onPageCompiled);
+            m_thread->start();
             break;
         }
 
@@ -62,18 +152,25 @@ void PDFAsynchronousPageCompiler::stop(bool clearCache)
     switch (m_state)
     {
         case State::Inactive:
+        {
+            Q_ASSERT(!m_thread);
             break; // We have nothing to do...
+        }
 
         case State::Active:
         {
             // Stop the engine
             m_state = State::Stopping;
 
-            for (const auto& taskItem : m_tasks)
+            Q_ASSERT(m_thread);
-            {
+            m_thread->requestInterruption();
-                disconnect(taskItem.second.taskWatcher, &QFutureWatcher<PDFPrecompiledPage>::finished, this, &PDFAsynchronousPageCompiler::onPageCompiled);
+            m_waitCondition.wakeAll();
-                taskItem.second.taskWatcher->waitForFinished();
+            m_thread->wait();
-            }
+            delete m_thread;
+            m_thread = nullptr;
+
+            // It is safe to do not use mutex, because
+            // we have ended the work thread.
             m_tasks.clear();
 
             if (clearCache)
@@ -114,24 +211,14 @@ const PDFPrecompiledPage* PDFAsynchronousPageCompiler::getCompiledPage(PDFIntege
     }
 
     const PDFPrecompiledPage* page = m_cache.object(pageIndex);
-    if (!page && compile && !m_tasks.count(pageIndex))
+    if (!page && compile)
     {
-        // Compile the page
+        QMutexLocker locker(&m_mutex);
-        auto compilePage = [this, pageIndex]() -> PDFPrecompiledPage
+        if (!m_tasks.count(pageIndex))
         {
-            PDFPrecompiledPage compiledPage;
+            m_tasks.insert(std::make_pair(pageIndex, CompileTask(pageIndex)));
-            PDFCMSPointer cms = m_proxy->getCMSManager()->getCurrentCMS();
+            m_waitCondition.wakeOne();
-            PDFRenderer renderer(m_proxy->getDocument(), m_proxy->getFontCache(), cms.data(), m_proxy->getOptionalContentActivity(), m_proxy->getFeatures(), m_proxy->getMeshQualitySettings());
+        }
-            renderer.compile(&compiledPage, pageIndex);
-            return compiledPage;
-        };
-
-        m_proxy->getFontCache()->setCacheShrinkEnabled(this, false);
-        CompileTask& task = m_tasks[pageIndex];
-        task.taskFuture = QtConcurrent::run(compilePage);
-        task.taskWatcher = new QFutureWatcher<PDFPrecompiledPage>(this);
-        connect(task.taskWatcher, &QFutureWatcher<PDFPrecompiledPage>::finished, this, &PDFAsynchronousPageCompiler::onPageCompiled);
-        task.taskWatcher->setFuture(task.taskFuture);
     }
 
     return page;
@@ -140,43 +227,49 @@ const PDFPrecompiledPage* PDFAsynchronousPageCompiler::getCompiledPage(PDFIntege
 void PDFAsynchronousPageCompiler::onPageCompiled()
 {
     std::vector<PDFInteger> compiledPages;
+    std::map<PDFInteger, PDFRenderError> errors;
 
-    // Search all tasks for finished tasks
-    for (auto it = m_tasks.begin(); it != m_tasks.end();)
     {
-        CompileTask& task = it->second;
+        QMutexLocker locker(&m_mutex);
-        if (task.taskWatcher->isFinished())
-        {
-            if (m_state == State::Active)
-            {
-                // If we are in active state, try to store precompiled page
-                PDFPrecompiledPage* page = new PDFPrecompiledPage(task.taskWatcher->result());
-                qint64 memoryConsumptionEstimate = page->getMemoryConsumptionEstimate();
-                if (m_cache.insert(it->first, page, memoryConsumptionEstimate))
-                {
-                    compiledPages.push_back(it->first);
-                }
-                else
-                {
-                    // We can't insert page to the cache, because cache size is too small. We will
-                    // emit error string to inform the user, that cache is too small.
-                    QString message = PDFTranslationContext::tr("Precompiled page size is too high (%1 kB). Cache size is %2 kB. Increase the cache size!").arg(memoryConsumptionEstimate / 1024).arg(m_cache.maxCost() / 1024);
-                    emit renderingError(it->first, { PDFRenderError(RenderErrorType::Error, message) });
-                }
-            }
 
-            task.taskWatcher->deleteLater();
+        // Search all tasks for finished tasks
-            it = m_tasks.erase(it);
+        for (auto it = m_tasks.begin(); it != m_tasks.end();)
-        }
-        else
         {
-            // Just increment the counter
+            CompileTask& task = it->second;
-            ++it;
+            if (task.finished)
+            {
+                if (m_state == State::Active)
+                {
+                    // If we are in active state, try to store precompiled page
+                    PDFPrecompiledPage* page = new PDFPrecompiledPage(std::move(task.precompiledPage));
+                    qint64 memoryConsumptionEstimate = page->getMemoryConsumptionEstimate();
+                    if (m_cache.insert(it->first, page, memoryConsumptionEstimate))
+                    {
+                        compiledPages.push_back(it->first);
+                    }
+                    else
+                    {
+                        // We can't insert page to the cache, because cache size is too small. We will
+                        // emit error string to inform the user, that cache is too small.
+                        QString message = PDFTranslationContext::tr("Precompiled page size is too high (%1 kB). Cache size is %2 kB. Increase the cache size!").arg(memoryConsumptionEstimate / 1024).arg(m_cache.maxCost() / 1024);
+                        errors[it->first] = PDFRenderError(RenderErrorType::Error, message);
+                    }
+                }
+
+                it = m_tasks.erase(it);
+            }
+            else
+            {
+                // Just increment the counter
+                ++it;
+            }
         }
     }
 
-    // We allow font cache shrinking, when we aren't doing something in parallel.
+    for (const auto& error : errors)
-    m_proxy->getFontCache()->setCacheShrinkEnabled(this, m_tasks.empty());
+    {
+        emit renderingError(error.first, { error.second });
+    }
 
     if (!compiledPages.empty())
     {

Pdf4QtLib/sources/pdfcompiler.h

@@ -25,10 +25,31 @@
 #include <QCache>
 #include <QFuture>
 #include <QFutureWatcher>
+#include <QWaitCondition>
 
 namespace pdf
 {
 class PDFDrawWidgetProxy;
+class PDFAsynchronousPageCompiler;
+
+class PDFAsynchronousPageCompilerWorkerThread : public QThread
+{
+    Q_OBJECT
+
+public:
+    explicit PDFAsynchronousPageCompilerWorkerThread(PDFAsynchronousPageCompiler* parent);
+
+signals:
+    void pageCompiled();
+
+protected:
+    virtual void run() override;
+
+private:
+    PDFAsynchronousPageCompiler* m_compiler;
+    QMutex* m_mutex;
+    QWaitCondition* m_waitCondition;
+};
 
 /// Asynchronous page compiler compiles pages asynchronously, and stores them in the
 /// cache. Cache size can be set. This object is designed to cooperate with
@@ -42,6 +63,7 @@ private:
 
 public:
     explicit PDFAsynchronousPageCompiler(PDFDrawWidgetProxy* proxy);
+    virtual ~PDFAsynchronousPageCompiler();
 
     /// Starts the engine. Call this function only if the engine
     /// is stopped.
@@ -70,6 +92,12 @@ public:
         Stopping
     };
 
+    /// Returns current state of compiler
+    State getState() const { return m_state; }
+
+    /// Return proxy
+    PDFDrawWidgetProxy* getProxy() const { return m_proxy; }
+
     /// Tries to retrieve precompiled page from the cache. If page is not found,
     /// then nullptr is returned (no exception is thrown). If \p compile is set to true,
     /// and page is not found, and compiler is active, then new asynchronous compile
@@ -79,21 +107,34 @@ public:
     const PDFPrecompiledPage* getCompiledPage(PDFInteger pageIndex, bool compile);
 
 signals:
-    void pageImageChanged(bool all, const std::vector<PDFInteger>& pages);
+    void pageImageChanged(bool all, const std::vector<pdf::PDFInteger>& pages);
-    void renderingError(PDFInteger pageIndex, const QList<PDFRenderError>& errors);
+    void renderingError(pdf::PDFInteger pageIndex, const QList<pdf::PDFRenderError>& errors);
 
 private:
+    friend class PDFAsynchronousPageCompilerWorkerThread;
+
     void onPageCompiled();
 
     struct CompileTask
     {
-        QFuture<PDFPrecompiledPage> taskFuture;
+        CompileTask() = default;
-        QFutureWatcher<PDFPrecompiledPage>* taskWatcher = nullptr;
+        CompileTask(PDFInteger pageIndex) : pageIndex(pageIndex) { }
+
+        PDFInteger pageIndex = 0;
+        bool finished = false;
+        PDFPrecompiledPage precompiledPage;
     };
 
-    PDFDrawWidgetProxy* m_proxy;
     State m_state = State::Inactive;
+    QMutex m_mutex;
+    QWaitCondition m_waitCondition;
+    PDFAsynchronousPageCompilerWorkerThread* m_thread = nullptr;
+
+    PDFDrawWidgetProxy* m_proxy;
     QCache<PDFInteger, PDFPrecompiledPage> m_cache;
+
+    /// This task is protected by mutex. Every access to this
+    /// variable must be done with locked mutex.
     std::map<PDFInteger, CompileTask> m_tasks;
 };
 

Pdf4QtLib/sources/pdfxfaengine.cpp

@@ -12567,10 +12567,13 @@ void PDFXFAEngineImpl::drawUiCheckButton(const xfa::XFA_checkButton* checkButton
     painter->setBrush(Qt::NoBrush);
 
     bool showBorder = true;
-    if (const xfa::XFA_border* border = checkButton->getBorder())
+    if (checkButton)
     {
-        const xfa::XFA_BaseNode::PRESENCE presence = border->getPresence();
+        if (const xfa::XFA_border* border = checkButton->getBorder())
-        showBorder = presence == xfa::XFA_BaseNode::PRESENCE::Visible;
+        {
+            const xfa::XFA_BaseNode::PRESENCE presence = border->getPresence();
+            showBorder = presence == xfa::XFA_BaseNode::PRESENCE::Visible;
+        }
     }
 
     if (showBorder)