ScrollView: Don't depend on originY for the internal scrollbar

While originY (and originX) are always zero for Flickable,
other types, like ListView, can and will set its value as
items are added or removed from the view. However, having
the internal scrollbars depend on originY may cause extra
updates and interfere with ListView transitions.

This changes just hardcodes the scrollbar's minimum value
to 0 and translates its value by originY. We also made the
code in ScrollViewHelper a bit more readable, mostly by
renaming the top-level item's id from 'wheelArea' to
'scrollHelper'.

Change-Id: Iac0b4e1dc3ca50ff3cc50fbb85ccaf1eca88edba
Task-number: QTBUG-48044
Reviewed-by: Mitch Curtis <mitch.curtis@theqtcompany.com>

2 files changed, 35 insertions, 40 deletions

diff --git a/src/controls/Private/ScrollViewHelper.qml b/src/controls/Private/ScrollViewHelper.qml
index e63d33ff..d34aa555 100644
--- a/src/controls/Private/ScrollViewHelper.qml
+++ b/src/controls/Private/ScrollViewHelper.qml
@@ -44,7 +44,7 @@ import QtQuick.Controls.Private 1.0
         \inqmlmodule QtQuick.Controls.Private
 */
 Item {
-    id: wheelarea
+    id: scrollHelper
 
     property alias horizontalScrollBar: hscrollbar
     property alias verticalScrollBar: vscrollbar
@@ -53,8 +53,6 @@ Item {
     property int availableWidth
     property int contentHeight
     property int contentWidth
-    property real originX
-    property real originY
     property bool active
     property int horizontalScrollBarPolicy: Qt.ScrollBarAsNeeded
     property int verticalScrollBarPolicy: Qt.ScrollBarAsNeeded
@@ -72,12 +70,10 @@ Item {
     function doLayout() {
         if (!recursionGuard) {
             recursionGuard = true
-            wheelarea.availableWidth = viewport.width
-            wheelarea.availableHeight = viewport.height
-            wheelarea.contentWidth = flickableItem !== null ? flickableItem.contentWidth : 0
-            wheelarea.contentHeight = flickableItem !== null ? flickableItem.contentHeight : 0
-            wheelarea.originX = flickableItem !== null ? flickableItem.originX : 0
-            wheelarea.originY = flickableItem !== null ? flickableItem.originY : 0
+            scrollHelper.availableWidth = viewport.width
+            scrollHelper.availableHeight = viewport.height
+            scrollHelper.contentWidth = flickableItem !== null ? flickableItem.contentWidth : 0
+            scrollHelper.contentHeight = flickableItem !== null ? flickableItem.contentHeight : 0
             recursionGuard = false
         }
     }
@@ -92,12 +88,6 @@ Item {
         target: flickableItem
         onContentWidthChanged: doLayout()
         onContentHeightChanged: doLayout()
-        onOriginXChanged: doLayout()
-        onOriginYChanged: doLayout()
-    }
-
-    Connections {
-        target: flickableItem
         onContentXChanged: {
             hscrollbar.flash()
             vscrollbar.flash()
@@ -123,15 +113,17 @@ Item {
 
     ScrollBar {
         id: hscrollbar
+        readonly property int scrollAmount: contentWidth - availableWidth
+        readonly property bool scrollable: scrollAmount > 0
         isTransient: !!__panel && !!__panel.isTransient
         active: !!__panel && (__panel.sunken || __panel.activeControl !== "none")
         enabled: !isTransient || __panel.visible
         orientation: Qt.Horizontal
-        visible: horizontalScrollBarPolicy ==  Qt.ScrollBarAsNeeded ? (contentWidth > availableWidth) : horizontalScrollBarPolicy == Qt.ScrollBarAlwaysOn
+        visible: horizontalScrollBarPolicy ==  Qt.ScrollBarAsNeeded ? scrollable : horizontalScrollBarPolicy == Qt.ScrollBarAlwaysOn
         height: visible ? implicitHeight : 0
         z: 1
-        maximumValue: contentWidth > availableWidth ? originX + contentWidth - availableWidth : 0
-        minimumValue: originX
+        maximumValue: scrollable ? scrollAmount : 0
+        minimumValue: 0
         anchors.bottom: parent.bottom
         anchors.left: parent.left
         anchors.right: cornerFill.left
@@ -139,20 +131,20 @@ Item {
         anchors.bottomMargin: bottomMargin
         onValueChanged: {
             if (!blockUpdates) {
-                flickableItem.contentX = value
+                flickableItem.contentX = value + flickableItem.originX
             }
         }
         Binding {
             target: hscrollbar.__panel
             property: "raised"
-            value: vscrollbar.active || wheelarea.active
+            value: vscrollbar.active || scrollHelper.active
             when: hscrollbar.isTransient
         }
         Binding {
             target: hscrollbar.__panel
             property: "visible"
             value: true
-            when: !hscrollbar.isTransient || wheelarea.active
+            when: !hscrollbar.isTransient || scrollHelper.active
         }
         function flash() {
             if (hscrollbar.isTransient) {
@@ -170,36 +162,38 @@ Item {
 
     ScrollBar {
         id: vscrollbar
+        readonly property int scrollAmount: contentHeight - availableHeight
+        readonly property bool scrollable: scrollAmount > 0
         isTransient: !!__panel && !!__panel.isTransient
         active: !!__panel && (__panel.sunken || __panel.activeControl !== "none")
         enabled: !isTransient || __panel.visible
         orientation: Qt.Vertical
-        visible: verticalScrollBarPolicy ==  Qt.ScrollBarAsNeeded ? (contentHeight > availableHeight) : verticalScrollBarPolicy == Qt.ScrollBarAlwaysOn
+        visible: verticalScrollBarPolicy === Qt.ScrollBarAsNeeded ? scrollable : verticalScrollBarPolicy === Qt.ScrollBarAlwaysOn
         width: visible ? implicitWidth : 0
         z: 1
         anchors.bottom: cornerFill.top
-        maximumValue: contentHeight > availableHeight ? originY + contentHeight - availableHeight + __viewTopMargin : 0
-        minimumValue: originY
+        maximumValue: scrollable ? scrollAmount + __viewTopMargin : 0
+        minimumValue: 0
         anchors.right: parent.right
         anchors.top: parent.top
         anchors.topMargin: __scrollBarTopMargin + topMargin
         anchors.rightMargin: rightMargin
         onValueChanged: {
             if (flickableItem && !blockUpdates && enabled) {
-                flickableItem.contentY = value
+                flickableItem.contentY = value + flickableItem.originY
             }
         }
         Binding {
             target: vscrollbar.__panel
             property: "raised"
-            value: hscrollbar.active || wheelarea.active
+            value: hscrollbar.active || scrollHelper.active
             when: vscrollbar.isTransient
         }
         Binding {
             target: vscrollbar.__panel
             property: "visible"
             value: true
-            when: !vscrollbar.isTransient || wheelarea.active
+            when: !vscrollbar.isTransient || scrollHelper.active
         }
         function flash() {
             if (vscrollbar.isTransient) {
diff --git a/src/controls/ScrollView.qml b/src/controls/ScrollView.qml
index 74d5ee7d..e6001b04 100644
--- a/src/controls/ScrollView.qml
+++ b/src/controls/ScrollView.qml
@@ -238,13 +238,13 @@ FocusScope {
 
             onContentYChanged:  {
                 scroller.blockUpdates = true
-                scroller.verticalScrollBar.value = flickableItem.contentY
+                scroller.verticalScrollBar.value = flickableItem.contentY - flickableItem.originY
                 scroller.blockUpdates = false
             }
 
             onContentXChanged:  {
                 scroller.blockUpdates = true
-                scroller.horizontalScrollBar.value = flickableItem.contentX
+                scroller.horizontalScrollBar.value = flickableItem.contentX - flickableItem.originX
                 scroller.blockUpdates = false
             }
 
@@ -274,11 +274,11 @@ FocusScope {
             property bool horizontalRecursionGuard: false
             property bool verticalRecursionGuard: false
 
-            horizontalMinimumValue: flickableItem ? flickableItem.originX : 0
-            horizontalMaximumValue: flickableItem ? flickableItem.originX + flickableItem.contentWidth - viewport.width : 0
+            horizontalMinimumValue: 0
+            horizontalMaximumValue: flickableItem ? flickableItem.contentWidth - viewport.width : 0
 
-            verticalMinimumValue: flickableItem ? flickableItem.originY : 0
-            verticalMaximumValue: flickableItem ? flickableItem.originY + flickableItem.contentHeight - viewport.height + __viewTopMargin : 0
+            verticalMinimumValue: 0
+            verticalMaximumValue: flickableItem ? flickableItem.contentHeight - viewport.height + __viewTopMargin : 0
 
             // The default scroll speed for typical angle-based mouse wheels. The value
             // comes originally from QTextEdit, which sets 20px steps by default, as well as
@@ -291,32 +291,33 @@ FocusScope {
 
                 onContentYChanged: {
                     wheelArea.verticalRecursionGuard = true
-                    wheelArea.verticalValue = flickableItem.contentY
+                    wheelArea.verticalValue = flickableItem.contentY - flickableItem.originY
                     wheelArea.verticalRecursionGuard = false
                 }
                 onContentXChanged: {
                     wheelArea.horizontalRecursionGuard = true
-                    wheelArea.horizontalValue = flickableItem.contentX
+                    wheelArea.horizontalValue = flickableItem.contentX - flickableItem.originX
                     wheelArea.horizontalRecursionGuard = false
                 }
             }
 
             onVerticalValueChanged: {
                 if (!verticalRecursionGuard) {
-                    if (flickableItem.contentY < flickThreshold && verticalDelta > speedThreshold) {
+                    var effectiveContentY = flickableItem.contentY - flickableItem.originY
+                    if (effectiveContentY < flickThreshold && verticalDelta > speedThreshold) {
                         flickableItem.flick(ignored, Math.min(maxFlick, acceleration * verticalDelta))
-                    } else if (flickableItem.contentY > flickableItem.contentHeight
-                               - flickThreshold - viewport.height && verticalDelta < -speedThreshold) {
+                    } else if (effectiveContentY > flickableItem.contentHeight - flickThreshold - viewport.height
+                               && verticalDelta < -speedThreshold) {
                         flickableItem.flick(ignored, Math.max(-maxFlick, acceleration * verticalDelta))
                     } else {
-                        flickableItem.contentY = verticalValue
+                        flickableItem.contentY = verticalValue + flickableItem.originY
                     }
                 }
             }
 
             onHorizontalValueChanged: {
                 if (!horizontalRecursionGuard)
-                    flickableItem.contentX = horizontalValue
+                    flickableItem.contentX = horizontalValue + flickableItem.originX
             }
         }