Skip to content

Commit

Permalink
Simplify ScrollFinisher
Browse files Browse the repository at this point in the history 
It turned out that the previous way to calculate the target position of
the viewport for ListView.Contain case was half-broken anyway, which
becomes rather apparent now that the work on navigating to replied-to
events is ongoing. On the other hand, it looks like Qt has improved its
positioning algorithm even further so even if one simply calls
chatView.positionViewAtIndex() the positioning is already perfect or
close to perfect most of the time. So the logic is now simplified; newly
instantiated components don't have to ask scrollFinisher if they've been
called out, the finisher simply tries to call positionViewAtIndex()
several times at small intervals.
  • Loading branch information
@KitsuneRal
KitsuneRal committed Dec 29, 2024
1 parent f160358 commit f60546e
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 94 deletions.
144 changes: 51 additions & 93 deletions client/qml/ScrollFinisher.qml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,10 @@ import QtQuick 2.15

Timer {
property int targetIndex: -1
property var targetPos
property int positionMode: ListView.End
property int round: 1

readonly property var lc: root.lc
readonly property real contentX: parent.contentX
readonly property real contentY: parent.contentY
readonly property real height: parent.height

/** @brief Scroll to the position making sure the timeline is actually at it
*
Expand All @@ -21,59 +17,28 @@ Timer {
* This function is the entry point to get the whole component do its job.
*/
function scrollViewTo(newTargetIndex, newPositionMode) {
console.log(lc, "Jumping to item", newTargetIndex)
parent.animateNextScroll = true
parent.positionViewAtIndex(newTargetIndex, newPositionMode)
targetIndex = newTargetIndex
positionMode = newPositionMode

if (targetPos) {
// Target coordinate already known, do a nice animated scroll to it
console.log(lc, "Scrolling to item", newTargetIndex)
parent.animateNextScroll = true
adjustPosition()
} else {
// The target item is not loaded yet; use ListView's imprecise
// positioning and fixup upon arrival
console.log(lc, "Jumping to item", newTargetIndex)
parent.positionViewAtIndex(newTargetIndex, newPositionMode)
round = 1
start()
}
}

function bindScrollTarget(item) {
targetPos = Qt.binding(() => { return item.y })
console.log(lc, "Scroll target bound, current pos:", targetPos)
}

/** @brief Bind the target position to the scroll target's ordinate
*
* This should be called from each TimelineItem component so that
* the finisher could detect when the timeline item with `targetIndex` is
* created, and try to scroll to its `y` position. Because the item might
* not be perfectly positioned at the moment of creation, it may take
* an additional iteration or two before the finisher sets the timeline
* on the right position.
*/
function maybeBindScrollTarget(delegate) {
if (targetIndex === delegate.index)
bindScrollTarget(delegate)
round = 1
start()
}

function logFixup(nameForLog, topContentY, bottomContentY) {
var topShownIndex = parent.indexAt(contentX, topContentY)
var bottomShownIndex = parent.indexAt(contentX, bottomContentY)
if (bottomShownIndex !== -1 && targetIndex <= topShownIndex
&& targetIndex >= bottomShownIndex)
return true // The item is within the expected range

console.log(lc, "Fixing up item", targetIndex, "to be", nameForLog,
"- fixup round #" + scrollFinisher.round,
"(" + topShownIndex + "-" + bottomShownIndex,
"range is shown now)")
const contentX = parent.contentX
const topShownIndex = parent.indexAt(contentX, topContentY)
const bottomShownIndex = parent.indexAt(contentX, bottomContentY - 1)
if (bottomShownIndex !== -1
&& (targetIndex > topShownIndex || targetIndex < bottomShownIndex))
console.log(lc, "Fixing up item", targetIndex, "to be", nameForLog,
"- fixup round #" + scrollFinisher.round,
"(" + topShownIndex + "-" + bottomShownIndex,
"range is shown now)")
}

/** @return true if no further action is needed;
* false if the finisher has to be restarted.
*/
/** @return true if no further action is needed; false if the finisher has to be restarted. */
function adjustPosition() {
if (targetIndex === 0) {
if (parent.bottommostVisibleIndex === 0)
Expand All @@ -84,60 +49,53 @@ Timer {
console.warn(lc, "Fixing up the viewport to be at sync edge")
parent.positionViewAtBeginning()
} else {
// The viewport is divided into thirds; ListView.End should
// place targetIndex at the top third, Center corresponds
// to the middle third; Beginning is not used for now.
var nameForLog, topContentY, bottomContentY
switch (positionMode) {
case ListView.Contain:
logFixup("fully visible", contentY, contentY + height)
if (targetPos)
targetPos = Math.max(targetPos,
Math.min(contentY, targetPos + height))
break
case ListView.Center:
logFixup("in the centre", contentY + height / 3,
contentY + 2 * height / 3)
if (targetPos)
targetPos -= height / 2
break
case ListView.End:
logFixup("at the top", contentY, contentY + height / 3)
break
default:
console.warn(lc, "fixupPosition: Unsupported positioning mode:",
positionMode)
return true // Refuse to do anything with it
}

// If the target item moved away too far and got destroyed,
// repeat positioning; otherwise, position the canvas exactly
// where it should be
if (targetPos) {
// NB: this.contentY is a readonly alias
parent.contentY = targetPos
targetPos = undefined
return true
const height = parent.height
const contentY = parent.contentY
const viewport1stThird = contentY + height / 3
const item = parent.itemAtIndex(targetIndex)
if (item) {
// The viewport is divided into thirds; ListView.End should
// place targetIndex at the top third, Center corresponds
// to the middle third; Beginning is not used for now.
switch (positionMode) {
case ListView.Contain:
if (item.y >= contentY || item.y + item.height < contentY + height)
return true // Positioning successful
logFixup("fully visible", contentY, contentY + height)
break
case ListView.Center:
const viewport2ndThird = contentY + 2 * height / 3
const itemMidPoint = item.y + item.height / 2
if (itemMidPoint >= viewport1stThird && itemMidPoint < viewport2ndThird)
return true
logFixup("in the centre", viewport1stThird, viewport2ndThird)
break
case ListView.End:
if (item.y >= contentY && item.y < viewport1stThird)
return true
logFixup("at the top", contentY, viewport1stThird)
break
default:
console.warn(lc, "fixupPosition: Unsupported positioning mode:", positionMode)
return true // Refuse to do anything with it
}
}
// If the target item moved away too far and got destroyed, repeat positioning
parent.animateNextScroll = true
parent.positionViewAtIndex(targetIndex, positionMode)
return false
}
return false
}

interval: 120 // small enough to avoid visual stutter
onTriggered: {
if (parent.count === 0 || !targetPos)
if (parent.count === 0)
return

if (adjustPosition() || ++round > 3) { // Give up after 3 rounds
targetPos = undefined
if (adjustPosition() || ++round > 3 /* Give up after 3 rounds */) {
targetIndex = -1
parent.saveViewport(true)
} else // Positioning is still in flux, might need another round
start()
}
onTargetIndexChanged: {
var item = parent.itemAtIndex(targetIndex)
if (item)
bindScrollTarget(item)
}
}
1 change: 0 additions & 1 deletion client/qml/TimelineItem.qml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ Item {
if (bottomEdgeShown)
bottomEdgeShownChanged()
readMarkerHereChanged()
scrollFinisher.maybeBindScrollTarget(this)
}

property bool showingDetails
Expand Down

0 comments on commit f60546e

Please sign in to comment.