From 7afe95f14d7d048a73baa12b3bd5f6a9bcea2ccb Mon Sep 17 00:00:00 2001
From: Shawn Rutledge <shawn.rutledge@qt.io>
Date: Wed, 5 Feb 2020 10:56:48 +0100
Subject: [PATCH] PDF multipage view: track specific link and navigation
 destinations

Unfortunately it's getting harder to do things declaratively, because we
have to avoid circular bindings, and because of needing to use imperative
APIs.  The current-page spinbox provides onValueModified() to detect when
the user modifies it, distinct from the simple fact that the value changed.
We shouldn't make bindings to set ListView.currentIndex anyway, because
that results in slow animation (and loading pages in all delegates along
the way) rather than quick jumping to the correct page.  Instead we need
to use ListView.positionViewAtIndex(), another imperative API, to get
quick jumps without having to calculate and set contentY in some other way.

Now we move toward the NavigationStack providing storage for the current
destination at all times.  Changes there will trigger programmatically
moving the ListView.

When the user scrolls manually, that generates a "destination" in the
navigation stack, such that the back button can jump back to the
previous location, and then the forward button can return to the
destination where manual scrolling ended up.

Fixes: QTBUG-77510
Change-Id: I47544210d2e0f9aa790f3d2594839678374e463d
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
---
 examples/pdf/multipage/viewer.qml          |  4 +-
 src/pdf/quick/qml/PdfMultiPageView.qml     | 50 ++++++++++++++++------
 src/pdf/quick/qquickpdfnavigationstack.cpp |  1 +
 3 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/examples/pdf/multipage/viewer.qml b/examples/pdf/multipage/viewer.qml
index 77c06f80f..bbc28cd8d 100644
--- a/examples/pdf/multipage/viewer.qml
+++ b/examples/pdf/multipage/viewer.qml
@@ -139,7 +139,7 @@ ApplicationWindow {
                 from: 1
                 to: document.pageCount
                 editable: true
-                onValueChanged: view.currentPage = value - 1
+                onValueModified: view.goToPage(value - 1)
                 Shortcut {
                     sequence: StandardKey.MoveToPreviousPage
                     onActivated: currentPageSB.value--
@@ -256,7 +256,7 @@ ApplicationWindow {
         anchors.fill: parent
         document: root.document
         searchString: searchField.text
-        onCurrentPageReallyChanged: currentPageSB.value = page + 1
+        onCurrentPageChanged: currentPageSB.value = view.currentPage + 1
     }
 
     footer: ToolBar {
diff --git a/src/pdf/quick/qml/PdfMultiPageView.qml b/src/pdf/quick/qml/PdfMultiPageView.qml
index 095081c5d..acef9fbea 100644
--- a/src/pdf/quick/qml/PdfMultiPageView.qml
+++ b/src/pdf/quick/qml/PdfMultiPageView.qml
@@ -62,16 +62,19 @@ Item {
     property real pageRotation: 0
     property string searchString
     property string selectedText
-    property alias currentPage: listView.currentIndex
+    property alias currentPage: navigationStack.currentPage
     function copySelectionToClipboard() {
         if (listView.currentItem !== null)
             listView.currentItem.selection.copyToClipboard()
     }
     property alias backEnabled: navigationStack.backAvailable
     property alias forwardEnabled: navigationStack.forwardAvailable
-    function back() { navigationStack.back() }
-    function forward() { navigationStack.forward() }
-    signal currentPageReallyChanged(page: int)
+    function back() {
+        navigationStack.back()
+    }
+    function forward() {
+        navigationStack.forward()
+    }
 
     function resetScale() {
         root.renderScale = 1
@@ -99,6 +102,16 @@ Item {
         }
     }
 
+    function goToPage(page) {
+        goToLocation(page, Qt.point(0, 0), 0)
+    }
+
+    function goToLocation(page, location, zoom) {
+        if (zoom > 0)
+            root.renderScale = zoom
+        navigationStack.push(page, location, zoom)
+    }
+
     id: root
     ListView {
         id: listView
@@ -109,11 +122,7 @@ Item {
         highlightMoveVelocity: 2000 // TODO increase velocity when setting currentIndex somehow, too
         property real rotationModulus: Math.abs(root.pageRotation % 180)
         property bool rot90: rotationModulus > 45 && rotationModulus < 135
-        property size firstPagePointSize: document.pagePointSize(0)
-        onCurrentIndexChanged: {
-            navigationStack.push(currentIndex, Qt.point(0, 0), root.renderScale)
-            root.currentPageReallyChanged(currentIndex)
-        }
+        property size firstPagePointSize: document === undefined ? Qt.size(0, 0) : document.pagePointSize(0)
         delegate: Rectangle {
             id: paper
             implicitWidth: image.width
@@ -233,7 +242,7 @@ Item {
                         cursorShape: Qt.PointingHandCursor
                         onClicked: {
                             if (page >= 0)
-                                listView.currentIndex = page
+                                root.goToLocation(page, location, zoom)
                             else
                                 Qt.openUrlExternally(url)
                         }
@@ -241,11 +250,28 @@ Item {
                 }
             }
         }
-        ScrollBar.vertical: ScrollBar { }
+        ScrollBar.vertical: ScrollBar {
+            property bool moved: false
+            onPositionChanged: moved = true
+            onActiveChanged: {
+                var currentPage = listView.indexAt(0, listView.contentY)
+                var currentItem = listView.itemAtIndex(currentPage)
+                var currentLocation = Qt.point(0, listView.contentY - currentItem.y)
+                if (active) {
+                    moved = false
+                    navigationStack.push(currentPage, currentLocation, root.renderScale);
+                } else if (moved) {
+                    navigationStack.update(currentPage, currentLocation, root.renderScale);
+                }
+            }
+        }
     }
     PdfNavigationStack {
         id: navigationStack
         onJumped: listView.currentIndex = page
-        onCurrentPageChanged: root.currentPageReallyChanged(navigationStack.currentPage)
+        onCurrentPageChanged: listView.positionViewAtIndex(currentPage, ListView.Beginning)
+        onCurrentLocationChanged: listView.contentY += currentLocation.y // currentPageChanged() MUST occur first!
+        onCurrentZoomChanged: root.renderScale = currentZoom
+        // TODO deal with horizontal location (need another Flickable probably)
     }
 }
diff --git a/src/pdf/quick/qquickpdfnavigationstack.cpp b/src/pdf/quick/qquickpdfnavigationstack.cpp
index 57acdc4bc..51f65f032 100644
--- a/src/pdf/quick/qquickpdfnavigationstack.cpp
+++ b/src/pdf/quick/qquickpdfnavigationstack.cpp
@@ -56,6 +56,7 @@ Q_LOGGING_CATEGORY(qLcNav, "qt.pdf.navigationstack")
 QQuickPdfNavigationStack::QQuickPdfNavigationStack(QObject *parent)
     : QObject(parent)
 {
+    push(0, QPointF(), 1);
 }
 
 /*!
-- 
GitLab