Merge pull request #107 from dcaliste/toc

[office] Delegate table of content build to a dedicated thread.

pdf/pdfrenderthread.cpp

@@ -120,11 +120,11 @@ class Thread : public QThread
 
     void run() {
         QThread::exec();
-        // Delete pending search that may use document.
+        // Delete pending search and toc that may use document.
         delete searchThread;
+        delete tocModel;
         autoSaveTo();
         delete document;
-        delete tocModel;
         deleteLater();
     }
 
@@ -276,6 +276,8 @@ int PDFRenderThread::pageCount() const
 QObject* PDFRenderThread::tocModel() const
 {
     QMutexLocker(&d->thread->mutex);
+    if (d->document && !d->document->isLocked() && !d->tocModel)
+        d->tocModel = new PDFTocModel(d->document);
     return d->tocModel;
 }
 
@@ -474,8 +476,6 @@ void PDFRenderThreadQueue::processPendingJob()
 
             if (!d->document || (!d->document->isLocked() && d->document->numPages() == 0)) {
                 d->loadFailure = true;
-            } else if (!d->document->isLocked()) {
-                d->tocModel = new PDFTocModel(d->document);
             }
 
             job->deleteLater();

pdf/pdftocmodel.cpp

@@ -20,6 +20,7 @@
 
 #include <poppler-qt5.h>
 #include <QDebug>
+#include <QThread>
 
 struct PDFTocEntry
 {
@@ -32,15 +33,29 @@ struct PDFTocEntry
     int pageNumber;
 };
 
-class PDFTocModel::Private
+class TocThread: public QThread
 {
+    Q_OBJECT
 public:
-    Private()
-    {}
-    ~Private() { qDeleteAll(entries); }
+    TocThread(Poppler::Document *document, QObject *parent = 0)
+        : QThread(parent), m_document(document)
+    {
+    }
+    ~TocThread()
+    {
+        wait();
+        qDeleteAll(entries);
+    }
 
     QList<PDFTocEntry*> entries;
-    Poppler::Document *document;
+
+    void run()
+    {
+        QDomDocument *toc = m_document->toc();
+        addSynopsisChildren(toc, 0);
+        delete toc;
+        emit tocAvailable();
+    }
 
     void addSynopsisChildren(QDomNode *parent, int level)
     {
@@ -60,7 +75,7 @@ class PDFTocModel::Private
             // Not doing this for now, but leave it in here as a note to self
             // if (!e.attribute("ExternalFileName").isNull()) item.setAttribute("ExternalFileName", e.attribute("ExternalFileName"));
             if (!e.attribute("DestinationName").isNull()) {
-                Poppler::LinkDestination *dest = document->linkDestination(e.attribute("DestinationName"));
+                Poppler::LinkDestination *dest = m_document->linkDestination(e.attribute("DestinationName"));
                 if (dest) {
                     tocEntry->pageNumber = dest->pageNumber();
                     delete dest;
@@ -87,16 +102,32 @@ class PDFTocModel::Private
         }
     }
 
+signals:
+    void tocAvailable();
+
+private:
+    Poppler::Document *m_document;
+};
+
+class PDFTocModel::Private
+{
+public:
+    Private(Poppler::Document *doc)
+        : document(doc)
+        , tocReady(false)
+        , tocThread(nullptr)
+    {}
+    ~Private() { delete tocThread; }
+
+    Poppler::Document *document;
+    bool tocReady;
+    TocThread *tocThread;
 };
 
 PDFTocModel::PDFTocModel(Poppler::Document *document, QObject *parent)
     : QAbstractListModel(parent)
-    , d(new Private)
+    , d(new Private(document))
 {
-    d->document = document;
-    QDomDocument *toc = document->toc();
-    d->addSynopsisChildren(toc, 0);
-    delete toc;
 }
 
 PDFTocModel::~PDFTocModel()
@@ -116,10 +147,10 @@ QHash<int, QByteArray> PDFTocModel::roleNames() const
 QVariant PDFTocModel::data(const QModelIndex &index, int role) const
 {
     QVariant result;
-    if (index.isValid()) {
+    if (index.isValid() && d->tocReady) {
         int row = index.row();
-        if (row > -1 && row < d->entries.count()) {
-            const PDFTocEntry *entry = d->entries.at(row);
+        if (row > -1 && row < d->tocThread->entries.count()) {
+            const PDFTocEntry *entry = d->tocThread->entries.at(row);
             switch(role)
             {
             case Title:
@@ -142,12 +173,40 @@ QVariant PDFTocModel::data(const QModelIndex &index, int role) const
 
 int PDFTocModel::rowCount(const QModelIndex &parent) const
 {
-    if (parent.isValid())
+    if (parent.isValid() || !d->tocReady)
         return 0;
-    return d->entries.count();
+    return d->tocThread->entries.count();
 }
 
 int PDFTocModel::count() const
 {
-    return d->entries.count();
+    return (d->tocReady) ? d->tocThread->entries.count() : 0;
+}
+
+bool PDFTocModel::ready() const
+{
+    return d->tocReady;
 }
+
+void PDFTocModel::requestToc()
+{
+    if (d->tocThread)
+        return;
+
+    d->tocThread = new TocThread(d->document);
+    d->tocThread->start();
+    connect(d->tocThread, &TocThread::tocAvailable,
+            this, &PDFTocModel::onTocAvailable);
+}
+
+void PDFTocModel::onTocAvailable()
+{
+    d->tocReady = true;
+    emit readyChanged();
+
+    beginInsertRows(QModelIndex(), 0, d->tocThread->entries.count() - 1);
+    endInsertRows();
+    emit countChanged();
+}
+
+#include "pdftocmodel.moc"

pdf/pdftocmodel.h

@@ -27,6 +27,7 @@ class PDFTocModel : public QAbstractListModel
 {
     Q_OBJECT
     Q_PROPERTY(int count READ count NOTIFY countChanged)
+    Q_PROPERTY(bool ready READ ready NOTIFY readyChanged)
 
 public:
     enum PDFTocModelRoles {
@@ -42,9 +43,16 @@ class PDFTocModel : public QAbstractListModel
     virtual QHash<int, QByteArray> roleNames() const;
 
     int count() const;
+    bool ready() const;
+
+    Q_INVOKABLE void requestToc();
 
 Q_SIGNALS:
     void countChanged();
+    void readyChanged();
+
+private Q_SLOTS:
+    void onTocAvailable();
 
 private:
     class Private;

plugin/PDFDocumentToCPage.qml

@@ -29,6 +29,8 @@ Page {
 
     allowedOrientations: Orientation.All
 
+    onTocModelChanged: tocModel.requestToc()
+
     SilicaListView {
         id: tocListView
 
@@ -42,14 +44,18 @@ Page {
 
         ViewPlaceholder {
             id: placeholder
+            enabled: tocListView.model
+                     && tocListView.model.ready
+                     && tocListView.model.count == 0
             //% "Document has no table of content"
             text: qsTrId("sailfish-office-me-no-toc")
         }
-        // The enabled attribute of the placeholder is not set by binding since
-        // the model object comes from a different thread and QML cannot listen
-        // on signals from a different thread. Thus, the attribute is set by
-        // reading the count value instead of a binding.
-        onModelChanged: placeholder.enabled = !model || (model.count == 0)
+        BusyIndicator {
+            anchors.centerIn: parent
+            size: BusyIndicatorSize.Large
+            z: 1
+            running: !tocListView.model || !tocListView.model.ready
+        }
 
         delegate: BackgroundItem {
             id: bg
@@ -71,7 +77,7 @@ Page {
                 id: pageNumberLabel
                 anchors {
                     right: parent.right
-                    rightMargin: Theme.paddingLarge
+                    rightMargin: Theme.horizontalPageMargin
                     verticalCenter: parent.verticalCenter
                 }
                 text: (model.pageNumber === undefined) ? "" : model.pageNumber