Redefine PdfSearchModel.currentResult as document-based; use PdfLink

PdfSearchModel.currentResult is now the index within the whole list
of search results, rather than starting over from 0 on each page.
This simplifies some code.

The way that PdfMultiPageView uses PdfSearchModel, the currentPage
property is read-only: it tells the page of currentResult (which we need
in the view, because each page has the ability to show a
current-search-result highlight shape, but only one page actually needs
to show it). The controls in a viewer to iterate search results (up and
down arrows in the footer) simply increment and derement currentResult;
the currentPage property gets updated to tell the view which page
currently holds currentResult; and the ListView highlight follows along
in the sidebar too, because ListView.currentIndex is now bound to
searchModel.currentResult.

But in PdfScrollablePageView, we still need to bind the currentPage
property, to get the currentPageBoundingPolygons property updated
when we switch pages. Since that viewer only sees one page at a
time, it's much more declarative to do it that way, rather than
calling the invokable boundingPolygonsOnPage(int) function.
Bindings get updated on their own; whereas in PdfMultiPageView
it's a bit inelegant that we need to call boundingPolygonsOnPage()
repeatedly, at the right times so that the highlights are never
shown in the wrong places at the wrong time. It could be avoided
if we had a separate per-page model object to filter the results
from the main PdfSearchModel; but that would add significant API
complexity, and perhaps be too confusing for anyone who tries to
re-implement a QML-based viewer component.

The current search result highlight now stays on the page where
the user left it: scrolling manually to another page will no
longer choose a current result on the new page, as it did before.
This is more consistent with typical applications.

A currentResultLink property is added, to make it easy to call
PdfPageNavigator.jump(link), thus passing along the QPdfLink.rectangles.
The same link object gets re-emitted in the PdfPageNavigator.jumped
signal to tell the view to scroll in such a way as to get those
rectangles visible in the viewport, via TableView.positionViewAtCell().

There are a couple of drive-by fixes:
QQuickPdfSearchModel::documentChanged() doesn't need to be declared,
because we are using the signal inherited from QPdfSearchModel.
And const-correctness is improved in the implementation of
boundingPolygonsOnPage().

[ChangeLog][QtPDF] PdfSearchModel.currentResult is now the result index
within the whole set of search results rather than on currentPage;
and changing currentPage no longer makes currentResult change.
In the views, this means the current highlighted search result stays
on the same page even when the user views a different page.

Change-Id: I96957f50e703f62101b3d3c708ff5f27b162cd8d
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>

5 files changed, 59 insertions, 62 deletions

diff --git a/src/pdf/qpdfsearchmodel.cpp b/src/pdf/qpdfsearchmodel.cpp
index d13ab6c75..d1fc0d190 100644
--- a/src/pdf/qpdfsearchmodel.cpp
+++ b/src/pdf/qpdfsearchmodel.cpp
@@ -360,6 +360,8 @@ bool QPdfSearchModelPrivate::doSearch(int page)
 
 QPdfSearchModelPrivate::PageAndIndex QPdfSearchModelPrivate::pageAndIndexForResult(int resultIndex)
 {
+    if (pagesSearched.isEmpty())
+        return {-1, -1};
     const int pageCount = document->pageCount();
     int totalSoFar = 0;
     int previousTotalSoFar = 0;
diff --git a/src/pdfquick/PdfMultiPageView.qml b/src/pdfquick/PdfMultiPageView.qml
index c4e5f1093..3ab0d1024 100644
--- a/src/pdfquick/PdfMultiPageView.qml
+++ b/src/pdfquick/PdfMultiPageView.qml
@@ -650,7 +650,6 @@ Item {
             jumping = false
             previousPage = current.page
         }
-        onCurrentPageChanged: searchModel.currentPage = currentPage
 
         property url documentSource: root.document.source
         onDocumentSourceChanged: {
@@ -663,8 +662,6 @@ Item {
     PdfSearchModel {
         id: searchModel
         document: root.document === undefined ? null : root.document
-        // TODO maybe avoid jumping if the result is already fully visible in the viewport
-        onCurrentResultBoundingRectChanged: root.goToLocation(currentPage,
-            Qt.point(currentResultBoundingRect.x, currentResultBoundingRect.y), 0)
+        onCurrentResultChanged: pageNavigator.jump(currentResultLink)
     }
 }
diff --git a/src/pdfquick/PdfScrollablePageView.qml b/src/pdfquick/PdfScrollablePageView.qml
index 7c467e47a..600c3ab8e 100644
--- a/src/pdfquick/PdfScrollablePageView.qml
+++ b/src/pdfquick/PdfScrollablePageView.qml
@@ -356,9 +356,7 @@ Flickable {
     PdfSearchModel {
         id: searchModel
         document: root.document === undefined ? null : root.document
-        // TODO maybe avoid jumping if the result is already fully visible in the viewport
-        onCurrentResultBoundingRectChanged: root.goToLocation(currentPage,
-            Qt.point(currentResultBoundingRect.x, currentResultBoundingRect.y), 0)
+        onCurrentResultChanged: pageNavigator.jump(currentResultLink)
     }
 
     PdfPageNavigator {
diff --git a/src/pdfquick/qquickpdfsearchmodel.cpp b/src/pdfquick/qquickpdfsearchmodel.cpp
index 18728dcd0..cffadf52a 100644
--- a/src/pdfquick/qquickpdfsearchmodel.cpp
+++ b/src/pdfquick/qquickpdfsearchmodel.cpp
@@ -110,15 +110,16 @@ void QQuickPdfSearchModel::setDocument(QQuickPdfDocument *document)
     }
     \endqml
 
+    It becomes empty whenever \c {currentPage != currentResultLink.page}.
+
     \sa PathMultiline
 */
 QList<QPolygonF> QQuickPdfSearchModel::currentResultBoundingPolygons() const
 {
     QList<QPolygonF> ret;
-    const auto &results = const_cast<QQuickPdfSearchModel *>(this)->resultsOnPage(m_currentPage);
-    if (m_currentResult < 0 || m_currentResult >= results.count())
+    const auto result = currentResultLink();
+    if (result.page() != m_currentPage)
         return ret;
-    const auto result = results[m_currentResult];
     for (auto rect : result.rectangles())
         ret << QPolygonF(rect);
     return ret;
@@ -127,23 +128,21 @@ QList<QPolygonF> QQuickPdfSearchModel::currentResultBoundingPolygons() const
 /*!
     \qmlproperty point PdfSearchModel::currentResultBoundingRect
 
-    The bounding box containing all \l currentResultBoundingPolygons.
-
-    When this property changes, a scrollable view should automatically scroll
-    itself in such a way as to ensure that this region is visible; for example,
-    it could try to position the upper-left corner near the upper-left of its
-    own viewport, subject to the constraints of the scrollable area.
+    The bounding box containing all \l currentResultBoundingPolygons,
+    if \c {currentPage == currentResultLink.page}; otherwise, an invalid rectangle.
 */
 QRectF QQuickPdfSearchModel::currentResultBoundingRect() const
 {
     QRectF ret;
-    const auto &results = const_cast<QQuickPdfSearchModel *>(this)->resultsOnPage(m_currentPage);
-    if (m_currentResult < 0 || m_currentResult >= results.count())
+    const auto result = currentResultLink();
+    if (result.page() != m_currentPage)
         return ret;
-    auto rects = results[m_currentResult].rectangles();
-    ret = rects.takeFirst();
-    for (auto rect : rects)
-        ret = ret.united(rect);
+    auto rects = result.rectangles();
+    if (!rects.isEmpty()) {
+        ret = rects.takeFirst();
+        for (auto rect : rects)
+            ret = ret.united(rect);
+    }
     return ret;
 }
 
@@ -219,9 +218,9 @@ QList<QPolygonF> QQuickPdfSearchModel::boundingPolygonsOnPage(int page)
     updatePage(page);
 
     QList<QPolygonF> ret;
-    auto m = QPdfSearchModel::resultsOnPage(page);
-    for (auto result : m) {
-        for (auto rect : result.rectangles())
+    const auto m = QPdfSearchModel::resultsOnPage(page);
+    for (const auto &result : m) {
+        for (const auto &rect : result.rectangles())
             ret << QPolygonF(rect);
     }
 
@@ -255,51 +254,50 @@ void QQuickPdfSearchModel::setCurrentPage(int currentPage)
 /*!
     \qmlproperty int PdfSearchModel::currentResult
 
-    The result index on \l currentPage for which \l currentResultBoundingPolygons
-    should provide the regions to highlight.
+    The result index within the whole set of search results, for which
+    \l currentResultBoundingPolygons should provide the regions to highlight
+    if currentPage matches \c currentResultLink.page.
 */
 void QQuickPdfSearchModel::setCurrentResult(int currentResult)
 {
     if (m_currentResult == currentResult)
         return;
 
-    int currentResultWas = currentResult;
-    int currentPageWas = m_currentPage;
-    if (currentResult < 0) {
-        setCurrentPage(m_currentPage - 1);
-        while (resultsOnPage(m_currentPage).count() == 0 && m_currentPage != currentPageWas) {
-            m_suspendSignals = true;
-            setCurrentPage(m_currentPage - 1);
-        }
-        if (m_suspendSignals) {
-            emit currentPageChanged();
-            m_suspendSignals = false;
-        }
-        const auto results = resultsOnPage(m_currentPage);
-        currentResult = results.count() - 1;
+    const int currentResultWas = m_currentResult;
+    const int currentPageWas = m_currentPage;
+    const int resultCount = rowCount({});
+
+    // wrap around at the ends
+    if (currentResult >= resultCount) {
+        currentResult = 0;
+    } else if (currentResult < 0) {
+        currentResult = resultCount - 1;
+    }
+
+    const QPdfLink link = resultAtIndex(currentResult);
+    if (link.isValid()) {
+        setCurrentPage(link.page());
+        m_currentResult = currentResult;
+        emit currentResultChanged();
+        emit currentResultLinkChanged();
+        emit currentResultBoundingPolygonsChanged();
+        emit currentResultBoundingRectChanged();
+        qCDebug(qLcSearch) << "currentResult was" << currentResultWas
+                      << "requested" << currentResult << "on page" << currentPageWas
+                      << "->" << m_currentResult << "on page" << m_currentPage;
     } else {
-        const auto results = resultsOnPage(m_currentPage);
-        if (currentResult >= results.count()) {
-            setCurrentPage(m_currentPage + 1);
-            while (resultsOnPage(m_currentPage).count() == 0 && m_currentPage != currentPageWas) {
-                m_suspendSignals = true;
-                setCurrentPage(m_currentPage + 1);
-            }
-            if (m_suspendSignals) {
-                emit currentPageChanged();
-                m_suspendSignals = false;
-            }
-            currentResult = 0;
-        }
+        qWarning() << "failed to find result" << currentResult << "in range 0 ->" << resultCount;
     }
-    qCDebug(qLcSearch) << "currentResult was" << m_currentResult
-                  << "requested" << currentResultWas << "on page" << currentPageWas
-                  << "->" << currentResult << "on page" << m_currentPage;
+}
 
-    m_currentResult = currentResult;
-    emit currentResultChanged();
-    emit currentResultBoundingPolygonsChanged();
-    emit currentResultBoundingRectChanged();
+/*!
+    \qmlproperty QPdfLink PdfSearchModel::currentResultLink
+
+    The result at index \l currentResult.
+*/
+QPdfLink QQuickPdfSearchModel::currentResultLink() const
+{
+    return resultAtIndex(m_currentResult);
 }
 
 /*!
diff --git a/src/pdfquick/qquickpdfsearchmodel_p.h b/src/pdfquick/qquickpdfsearchmodel_p.h
index 1ddd1d833..1ce981c13 100644
--- a/src/pdfquick/qquickpdfsearchmodel_p.h
+++ b/src/pdfquick/qquickpdfsearchmodel_p.h
@@ -65,6 +65,7 @@ class  Q_PDFQUICK_EXPORT QQuickPdfSearchModel : public QPdfSearchModel
     Q_PROPERTY(QQuickPdfDocument *document READ document WRITE setDocument NOTIFY documentChanged)
     Q_PROPERTY(int currentPage READ currentPage WRITE setCurrentPage NOTIFY currentPageChanged)
     Q_PROPERTY(int currentResult READ currentResult WRITE setCurrentResult NOTIFY currentResultChanged)
+    Q_PROPERTY(QPdfLink currentResultLink READ currentResultLink NOTIFY currentResultLinkChanged)
     Q_PROPERTY(QList<QPolygonF> currentPageBoundingPolygons READ currentPageBoundingPolygons NOTIFY currentPageBoundingPolygonsChanged)
     Q_PROPERTY(QList<QPolygonF> currentResultBoundingPolygons READ currentResultBoundingPolygons NOTIFY currentResultBoundingPolygonsChanged)
     Q_PROPERTY(QRectF currentResultBoundingRect READ currentResultBoundingRect NOTIFY currentResultBoundingRectChanged)
@@ -86,14 +87,15 @@ public:
     int currentResult() const { return m_currentResult; }
     void setCurrentResult(int currentResult);
 
+    QPdfLink currentResultLink() const;
     QList<QPolygonF> currentPageBoundingPolygons() const;
     QList<QPolygonF> currentResultBoundingPolygons() const;
     QRectF currentResultBoundingRect() const;
 
 signals:
-    void documentChanged();
     void currentPageChanged();
     void currentResultChanged();
+    void currentResultLinkChanged();
     void currentPageBoundingPolygonsChanged();
     void currentResultBoundingPolygonsChanged();
     void currentResultBoundingRectChanged();