From 3ef62627e55de2802bca35e5e921d4d136b1973c Mon Sep 17 00:00:00 2001 From: Damien Caliste Date: Thu, 15 Dec 2016 21:48:10 +0100 Subject: [PATCH] [office] Delegate table of content build to a dedicated thread. --- pdf/pdfrenderthread.cpp | 8 +-- pdf/pdftocmodel.cpp | 93 ++++++++++++++++++++++++++++------- pdf/pdftocmodel.h | 8 +++ plugin/PDFDocumentToCPage.qml | 18 ++++--- 4 files changed, 100 insertions(+), 27 deletions(-) diff --git a/pdf/pdfrenderthread.cpp b/pdf/pdfrenderthread.cpp index 4abb0b62..9ed269f3 100644 --- a/pdf/pdfrenderthread.cpp +++ b/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(); diff --git a/pdf/pdftocmodel.cpp b/pdf/pdftocmodel.cpp index 13c33eed..c7649a43 100644 --- a/pdf/pdftocmodel.cpp +++ b/pdf/pdftocmodel.cpp @@ -20,6 +20,7 @@ #include #include +#include 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 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 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" diff --git a/pdf/pdftocmodel.h b/pdf/pdftocmodel.h index 78c6fb4d..7da3aae9 100644 --- a/pdf/pdftocmodel.h +++ b/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 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; diff --git a/plugin/PDFDocumentToCPage.qml b/plugin/PDFDocumentToCPage.qml index 38420cbd..a3e702b6 100644 --- a/plugin/PDFDocumentToCPage.qml +++ b/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