QQuickTableView: update viewportMoved() to take syncView into account

Now that several table views can stay in sync through the
syncView parent-child chain, we also need to ensure that the
position of the content views stays in sync. This patch will
recursively go through all connected views when one of the
views are moved and set the same position on them all according
to the syncDirection flag.

Change-Id: I5a5b8e795426484eeab3771f6c8d4c9b7da046eb
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>

2 files changed, 122 insertions, 35 deletions

diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp
index 8c64c11bb1..da097f8888 100644
--- a/src/quick/items/qquicktableview.cpp
+++ b/src/quick/items/qquicktableview.cpp
@@ -1583,6 +1583,11 @@ void QQuickTableViewPrivate::beginRebuildTable()
     loadedTableInnerRect = QRect();
     clearEdgeSizeCache();
 
+    if (syncHorizontally)
+        setLocalViewportX(syncView->contentX());
+    if (syncVertically)
+        setLocalViewportY(syncView->contentY());
+
     if (!model) {
         qCDebug(lcTableViewDelegateLifecycle()) << "no model found, leaving table empty";
         return;
@@ -2175,6 +2180,84 @@ void QQuickTableViewPrivate::modelResetCallback()
     scheduleRebuildTable(RebuildOption::All);
 }
 
+void QQuickTableViewPrivate::scheduleRebuildIfFastFlick()
+{
+    Q_Q(QQuickTableView);
+
+    // If the viewport has moved more than one page vertically or horizontally, we switch
+    // strategy from refilling edges around the current table to instead rebuild the table
+    // from scratch inside the new viewport. This will greatly improve performance when flicking
+    // a long distance in one go, which can easily happen when dragging on scrollbars.
+
+    // Check the viewport moved more than one page vertically
+    if (!viewportRect.intersects(QRectF(viewportRect.x(), q->contentY(), 1, q->height()))) {
+        scheduledRebuildOptions |= RebuildOption::CalculateNewTopLeftRow;
+        scheduledRebuildOptions |= RebuildOption::ViewportOnly;
+    }
+
+    // Check the viewport moved more than one page horizontally
+    if (!viewportRect.intersects(QRectF(q->contentX(), viewportRect.y(), q->width(), 1))) {
+        scheduledRebuildOptions |= RebuildOption::CalculateNewTopLeftColumn;
+        scheduledRebuildOptions |= RebuildOption::ViewportOnly;
+    }
+}
+
+void QQuickTableViewPrivate::setLocalViewportX(qreal contentX)
+{
+    // Set the new viewport position if changed, but don't trigger any
+    // rebuilds or updates. We use this function internally to distinguish
+    // external flicking from internal sync-ing of the content view.
+    Q_Q(QQuickTableView);
+    QBoolBlocker blocker(inSetLocalViewportPos, true);
+
+    if (qFuzzyCompare(contentX, q->contentX()))
+        return;
+
+    q->setContentX(contentX);
+}
+
+void QQuickTableViewPrivate::setLocalViewportY(qreal contentY)
+{
+    // Set the new viewport position if changed, but don't trigger any
+    // rebuilds or updates. We use this function internally to distinguish
+    // external flicking from internal sync-ing of the content view.
+    Q_Q(QQuickTableView);
+    QBoolBlocker blocker(inSetLocalViewportPos, true);
+
+    if (qFuzzyCompare(contentY, q->contentY()))
+        return;
+
+    q->setContentY(contentY);
+}
+
+void QQuickTableViewPrivate::syncViewportPosRecursive()
+{
+    Q_Q(QQuickTableView);
+    QBoolBlocker recursionGuard(inSyncViewportPosRecursive, true);
+
+    if (syncView) {
+        auto syncView_d = syncView->d_func();
+        if (!syncView_d->inSyncViewportPosRecursive) {
+            if (syncHorizontally)
+                syncView_d->setLocalViewportX(q->contentX());
+            if (syncVertically)
+                syncView_d->setLocalViewportY(q->contentY());
+            syncView_d->syncViewportPosRecursive();
+        }
+    }
+
+    for (auto syncChild : qAsConst(syncChildren)) {
+        auto syncChild_d = syncChild->d_func();
+        if (!syncChild_d->inSyncViewportPosRecursive) {
+            if (syncChild_d->syncHorizontally)
+                syncChild_d->setLocalViewportX(q->contentX());
+            if (syncChild_d->syncVertically)
+                syncChild_d->setLocalViewportY(q->contentY());
+            syncChild_d->syncViewportPosRecursive();
+        }
+    }
+}
+
 QQuickTableView::QQuickTableView(QQuickItem *parent)
     : QQuickFlickable(*(new QQuickTableViewPrivate), parent)
 {
@@ -2400,45 +2483,42 @@ void QQuickTableView::geometryChanged(const QRectF &newGeometry, const QRectF &o
 void QQuickTableView::viewportMoved(Qt::Orientations orientation)
 {
     Q_D(QQuickTableView);
+
+    // If the new viewport position was set from the setLocalViewportXY()
+    // functions, we just update the position silently and return. Otherwise, if
+    // the viewport was flicked by the user, or some other control, we
+    // recursively sync all the views in the hierarchy to the same position.
     QQuickFlickable::viewportMoved(orientation);
+    if (d->inSetLocalViewportPos)
+        return;
 
-    QQuickTableViewPrivate::RebuildOptions options = QQuickTableViewPrivate::RebuildOption::None;
+    // Move all views in the syncView hierarchy to the same contentX/Y.
+    // We need to start from this view (and not the root syncView) to
+    // ensure that we respect all the individual syncDirection flags
+    // between the individual views in the hierarchy.
+    d->syncViewportPosRecursive();
 
-    // Check the viewport moved more than one page vertically
-    if (!d->viewportRect.intersects(QRectF(d->viewportRect.x(), contentY(), 1, height())))
-        options |= QQuickTableViewPrivate::RebuildOption::CalculateNewTopLeftRow;
-    // Check the viewport moved more than one page horizontally
-    if (!d->viewportRect.intersects(QRectF(contentX(), d->viewportRect.y(), width(), 1)))
-        options |= QQuickTableViewPrivate::RebuildOption::CalculateNewTopLeftColumn;
-
-    if (options) {
-        // When the viewport has moved more than one page vertically or horizontally, we switch
-        // strategy from refilling edges around the current table to instead rebuild the table
-        // from scratch inside the new viewport. This will greatly improve performance when flicking
-        // a long distance in one go, which can easily happen when dragging on scrollbars.
-        options |= QQuickTableViewPrivate::RebuildOption::ViewportOnly;
-        d->scheduleRebuildTable(options);
-    }
-
-    if (d->scheduledRebuildOptions) {
-        // No reason to do anything, since we're about to rebuild the whole table anyway.
-        // Besides, calling updatePolish, which will start the rebuild, can easily cause
-        // binding loops to happen since we usually end up modifying the geometry of the
-        // viewport (contentItem) as well.
-        return;
-    }
+    auto rootView = d->rootSyncView();
+    auto rootView_d = rootView->d_func();
 
-    // Calling polish() will schedule a polish event. But while the user is flicking, several
-    // mouse events will be handled before we get an updatePolish() call. And the updatePolish()
-    // call will only see the last mouse position. This results in a stuttering flick experience
-    // (especially on windows). We improve on this by calling updatePolish() directly. But this
-    // has the pitfall that we open up for recursive callbacks. E.g while inside updatePolish(), we
-    // load/unload items, and emit signals. The application can listen to those signals and set a
-    // new contentX/Y on the flickable. So we need to guard for this, to avoid unexpected behavior.
-    if (d->polishing)
-        polish();
-    else
-        d->updatePolish();
+    rootView_d->scheduleRebuildIfFastFlick();
+
+    if (!rootView_d->polishScheduled) {
+        if (rootView_d->scheduledRebuildOptions) {
+            // When we need to rebuild, collecting several viewport
+            // moves and do a single polish gives a quicker UI.
+            rootView->polish();
+        } else {
+            // Updating the table right away when flicking
+            // slowly gives a smoother experience.
+            const bool updated = rootView->d_func()->updateTableRecursive();
+            if (!updated) {
+                // One, or more, of the views are already in an
+                // update, so we need to wait a cycle.
+                rootView->polish();
+            }
+        }
+    }
 }
 
 void QQuickTableViewPrivate::_q_componentFinalized()
diff --git a/src/quick/items/qquicktableview_p_p.h b/src/quick/items/qquicktableview_p_p.h
index 13c69d0b43..0b15e63342 100644
--- a/src/quick/items/qquicktableview_p_p.h
+++ b/src/quick/items/qquicktableview_p_p.h
@@ -251,6 +251,8 @@ public:
     bool polishing = false;
     bool syncVertically = false;
     bool syncHorizontally = false;
+    bool inSetLocalViewportPos = false;
+    bool inSyncViewportPosRecursive = false;
 
     QJSValue rowHeightProvider;
     QJSValue columnWidthProvider;
@@ -394,6 +396,11 @@ public:
     void layoutChangedCallback(const QList<QPersistentModelIndex> &parents, QAbstractItemModel::LayoutChangeHint hint);
     void modelResetCallback();
 
+    void scheduleRebuildIfFastFlick();
+    void setLocalViewportX(qreal contentX);
+    void setLocalViewportY(qreal contentY);
+    void syncViewportPosRecursive();
+
     void _q_componentFinalized();
     void registerCallbackWhenBindingsAreEvaluated();