Optimised jumping between messages to avoid excessive loading

pull/946/head
Morgan Pretty 6 months ago
parent 001936e1b6
commit cac29a573a

@ -1077,7 +1077,11 @@ extension ConversationVC:
return
}
self.scrollToInteractionIfNeeded(with: interactionInfo, focusBehaviour: .highlight)
self.scrollToInteractionIfNeeded(
with: interactionInfo,
focusBehaviour: .highlight,
originalIndexPath: self.tableView.indexPath(for: cell)
)
// If the message contains both links and a LinkPreview, and the user tapped on
// the LinkPreview; OR the message only contained a LinkPreview, then open the link

@ -658,7 +658,8 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
Storage.shared.removeObserver(self?.viewModel.pagedDataObserver)
// Swap the observing to the updated thread
self?.viewModel.swapToThread(updatedThreadId: unblindedId)
let newestVisibleMessageId: Int64? = self?.fullyVisibleCellViewModels()?.last?.id
self?.viewModel.swapToThread(updatedThreadId: unblindedId, focussedMessageId: newestVisibleMessageId)
// Start observing changes again
Storage.shared.addObserver(self?.viewModel.pagedDataObserver)
@ -961,14 +962,14 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
struct ItemChangeInfo {
let isInsertAtTop: Bool
let firstIndexIsVisible: Bool
let visibleIndexPath: IndexPath
let oldVisibleIndexPath: IndexPath
let visibleIndexPath: IndexPath?
let oldVisibleIndexPath: IndexPath?
init(
isInsertAtTop: Bool = false,
firstIndexIsVisible: Bool = false,
visibleIndexPath: IndexPath = IndexPath(row: 0, section: 0),
oldVisibleIndexPath: IndexPath = IndexPath(row: 0, section: 0)
visibleIndexPath: IndexPath? = nil,
oldVisibleIndexPath: IndexPath? = nil
) {
self.isInsertAtTop = isInsertAtTop
self.firstIndexIsVisible = firstIndexIsVisible
@ -982,22 +983,39 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
let wasLoadingMore: Bool = self.isLoadingMore
let wasOffsetCloseToBottom: Bool = self.isCloseToBottom
let numItemsInUpdatedData: [Int] = updatedData.map { $0.elements.count }
let didSwapAllContent: Bool = (updatedData
.first(where: { $0.model == .messages })?
.elements
.contains(where: {
$0.id == self.viewModel.interactionData
.first(where: { $0.model == .messages })?
let didSwapAllContent: Bool = {
// The dynamic headers use negative id values so by using `compactMap` and returning
// null in those cases allows us to exclude them without another iteration via `filter`
let currentIds: Set<Int64> = (self.viewModel.interactionData
.first { $0.model == .messages }?
.elements
.compactMap { $0.id > 0 ? $0.id : nil }
.asSet())
.defaulting(to: [])
let updatedIds: Set<Int64> = (updatedData
.first { $0.model == .messages }?
.elements
.first?
.id
}))
.defaulting(to: false)
let itemChangeInfo: ItemChangeInfo? = {
.compactMap { $0.id > 0 ? $0.id : nil }
.asSet())
.defaulting(to: [])
return updatedIds.isDisjoint(with: currentIds)
}()
let itemChangeInfo: ItemChangeInfo = {
guard
isInsert,
let oldSectionIndex: Int = self.viewModel.interactionData.firstIndex(where: { $0.model == .messages }),
let newSectionIndex: Int = updatedData.firstIndex(where: { $0.model == .messages }),
let firstVisibleIndexPath: IndexPath = self.tableView.indexPathsForVisibleRows?
.filter({
$0.section == oldSectionIndex &&
self.viewModel.interactionData[$0.section].elements[$0.row].cellType != .dateHeader
})
.sorted()
.first
else { return ItemChangeInfo() }
guard
let newFirstItemIndex: Int = updatedData[newSectionIndex].elements
.firstIndex(where: { item -> Bool in
// Since the first item is probably a `DateHeaderCell` (which would likely
@ -1011,20 +1029,28 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
item.id == messages[safe: 1]?.id
)
}),
let firstVisibleIndexPath: IndexPath = self.tableView.indexPathsForVisibleRows?
.filter({
$0.section == oldSectionIndex &&
self.viewModel.interactionData[$0.section].elements[$0.row].cellType != .dateHeader
})
.sorted()
.first,
let newVisibleIndex: Int = updatedData[newSectionIndex].elements
.firstIndex(where: { item in
item.id == self.viewModel.interactionData[oldSectionIndex]
.elements[firstVisibleIndexPath.row]
.id
})
else { return nil }
else {
let oldTimestamps: [Int64] = self.viewModel.interactionData[oldSectionIndex]
.elements
.filter { $0.cellType != .dateHeader }
.map { $0.timestampMs }
let newTimestamps: [Int64] = updatedData[newSectionIndex]
.elements
.filter { $0.cellType != .dateHeader }
.map { $0.timestampMs }
return ItemChangeInfo(
isInsertAtTop: ((newTimestamps.max() ?? Int64.max) < (oldTimestamps.min() ?? Int64.min)),
firstIndexIsVisible: (firstVisibleIndexPath.row == 0),
oldVisibleIndexPath: firstVisibleIndexPath
)
}
return ItemChangeInfo(
isInsertAtTop: (
@ -1039,25 +1065,25 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
)
}()
guard !isInsert || itemChangeInfo?.isInsertAtTop == true else {
guard !isInsert || (!didSwapAllContent && itemChangeInfo.isInsertAtTop) else {
self.viewModel.updateInteractionData(updatedData)
self.tableView.reloadData()
// Animate to the target interaction (or the bottom) after a slightly delay to prevent buggy
// animation conflicts
// If we had a focusedInteractionInfo then scroll to it (and hide the search
// result bar loading indicator)
if let focusedInteractionInfo: Interaction.TimestampInfo = self.focusedInteractionInfo {
// If we had a focusedInteractionInfo then scroll to it (and hide the search
// result bar loading indicator)
let delay: DispatchTime = (didSwapAllContent ?
.now() :
(.now() + .milliseconds(100))
)
DispatchQueue.main.asyncAfter(deadline: delay) { [weak self] in
self.tableView.afterNextLayoutSubviews(when: { _, _, _ in true }, then: { [weak self] in
self?.searchController.resultsBar.stopLoading()
self?.scrollToInteractionIfNeeded(
with: focusedInteractionInfo,
focusBehaviour: (self?.focusBehaviour ?? .none),
contentSwapLocation: {
switch (didSwapAllContent, itemChangeInfo.isInsertAtTop) {
case (true, true): return .earlier
case (true, false): return .later
default: return .none
}
}(),
isAnimated: true
)
@ -1066,7 +1092,7 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
self?.isLoadingMore = false
self?.autoLoadNextPageIfNeeded()
}
}
})
}
else if wasOffsetCloseToBottom && !wasLoadingMore && numItemsInserted < 5 {
/// Scroll to the bottom if an interaction was just inserted and we either just sent a message or are close enough to the
@ -1096,8 +1122,8 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
///
/// Unfortunately the UITableView also does some weird things when updating (where it won't have updated it's internal data until
/// after it performs the next layout); the below code checks a condition on layout and if it passes it calls a closure
if let itemChangeInfo: ItemChangeInfo = itemChangeInfo, itemChangeInfo.isInsertAtTop {
let oldCellRect: CGRect = self.tableView.rectForRow(at: itemChangeInfo.oldVisibleIndexPath)
if itemChangeInfo.isInsertAtTop, let visibleIndexPath: IndexPath = itemChangeInfo.visibleIndexPath, let oldVisibleIndexPath: IndexPath = itemChangeInfo.oldVisibleIndexPath {
let oldCellRect: CGRect = self.tableView.rectForRow(at: oldVisibleIndexPath)
let oldCellTopOffset: CGFloat = (self.tableView.frame.minY - self.tableView.convert(oldCellRect, to: self.tableView.superview).minY)
// The the user triggered the 'scrollToTop' animation (by tapping in the nav bar) then we
@ -1119,11 +1145,7 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
// a large number of cells when getting search results which are very far away
// only to instantly start scrolling making the calculation redundant)
UIView.performWithoutAnimation {
self?.tableView.scrollToRow(
at: itemChangeInfo.visibleIndexPath,
at: .top,
animated: false
)
self?.tableView.scrollToRow(at: visibleIndexPath, at: .top, animated: false)
self?.tableView.contentOffset.y += oldCellTopOffset
}
@ -1179,7 +1201,7 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
deleteRowsAnimation: .fade,
insertRowsAnimation: .none,
reloadRowsAnimation: .none,
interrupt: { itemChangeInfo?.isInsertAtTop == true || $0.changeCount > ConversationViewModel.pageSize }
interrupt: { itemChangeInfo.isInsertAtTop || $0.changeCount > ConversationViewModel.pageSize }
) { [weak self] updatedData in
self?.viewModel.updateInteractionData(updatedData)
}
@ -1618,7 +1640,6 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
self.scrollToInteractionIfNeeded(
with: lastInteractionInfo,
position: .bottom,
isJumpingToLastInteraction: true,
isAnimated: true
)
return
@ -1827,7 +1848,8 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
with interactionInfo: Interaction.TimestampInfo,
focusBehaviour: ConversationViewModel.FocusBehaviour = .none,
position: UITableView.ScrollPosition = .middle,
isJumpingToLastInteraction: Bool = false,
contentSwapLocation: ConversationViewModel.ContentSwapLocation = .none,
originalIndexPath: IndexPath? = nil,
isAnimated: Bool = true
) {
// Store the info incase we need to load more data (call will be re-triggered)
@ -1850,18 +1872,10 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
self.searchController.resultsBar.startLoading()
DispatchQueue.global(qos: .userInitiated).async { [weak self] in
if isJumpingToLastInteraction {
self?.viewModel.pagedDataObserver?.load(.jumpTo(
id: interactionInfo.id,
paddingForInclusive: 5
))
}
else {
self?.viewModel.pagedDataObserver?.load(.untilInclusive(
id: interactionInfo.id,
padding: 5
))
}
self?.viewModel.pagedDataObserver?.load(.jumpTo(
id: interactionInfo.id,
paddingForInclusive: 5
))
}
return
}
@ -1938,12 +1952,41 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers
return
}
// As an optimisation if the target cell is too far away we just reload the entire table instead of loading
// all intermediate messages, as a result the scroll animation can be buggy (as the contentOffset could
// actually end up on the wrong side of the destination before the scroll animation starts)
//
// To get around this we immediately jump to a position 10 cells above/below the destination and then scroll
// which appears as though the screen has properly scrolled between the messages
switch contentSwapLocation {
case .none:
if let originalIndexPath: IndexPath = originalIndexPath {
// Since we use `estimatedRowHeight` instead of an explicit height there is an annoying issue
// where the cells won't have their heights calculated correctly so jumping between cells can
// result in a scroll animation going the wrong direction - by jumping to the destination and
// back to the current cell all of the relevant cells will have their frames calculated correctly
// and the animation will look correct
self.tableView.scrollToRow(at: targetIndexPath, at: targetPosition, animated: false)
self.tableView.scrollToRow(at: originalIndexPath, at: targetPosition, animated: false)
}
case .earlier:
let targetRow: Int = min(targetIndexPath.row + 10, self.viewModel.interactionData[messageSectionIndex].elements.count - 1)
self.tableView.contentOffset = CGPoint(x: 0, y: self.tableView.rectForRow(at: IndexPath(row: targetRow, section: targetIndexPath.section)).midY)
case .later:
let targetRow: Int = min(targetIndexPath.row - 10, 0)
self.tableView.contentOffset = CGPoint(x: 0, y: self.tableView.rectForRow(at: IndexPath(row: targetRow, section: targetIndexPath.section)).midY)
}
self.tableView.scrollToRow(at: targetIndexPath, at: targetPosition, animated: true)
}
func fullyVisibleCellViewModels() -> [MessageViewModel]? {
// We remove the 'Values.mediumSpacing' as that is the distance the table content appears above the input view
let tableVisualTop: CGFloat = tableView.frame.minY//(tableView.frame.minY - (tableView.contentInset.bottom - Values.mediumSpacing))
let tableVisualTop: CGFloat = tableView.frame.minY
let tableVisualBottom: CGFloat = (tableView.frame.maxY - (tableView.contentInset.bottom - Values.mediumSpacing))
guard

@ -17,6 +17,14 @@ public class ConversationViewModel: OWSAudioPlayerDelegate {
case highlight
}
// MARK: - ContentSwapLocation
public enum ContentSwapLocation {
case none
case earlier
case later
}
// MARK: - Action
public enum Action {
@ -172,12 +180,10 @@ public class ConversationViewModel: OWSAudioPlayerDelegate {
DispatchQueue.global(qos: .userInitiated).async { [weak self] in
// If we don't have a `initialFocusedInfo` then default to `.pageBefore` (it'll query
// from a `0` offset)
guard let initialFocusedInfo: Interaction.TimestampInfo = (focusedInteractionInfo ?? initialData?.initialUnreadInteractionInfo) else {
self?.pagedDataObserver?.load(.pageBefore)
return
switch (focusedInteractionInfo ?? initialData?.initialUnreadInteractionInfo) {
case .some(let info): self?.pagedDataObserver?.load(.initialPageAround(id: info.id))
case .none: self?.pagedDataObserver?.load(.pageBefore)
}
self?.pagedDataObserver?.load(.initialPageAround(id: initialFocusedInfo.id))
}
}
@ -785,14 +791,7 @@ public class ConversationViewModel: OWSAudioPlayerDelegate {
markAsReadTrigger.send((target, timestampMs))
}
public func swapToThread(updatedThreadId: String) {
let oldestMessageId: Int64? = self.interactionData
.filter { $0.model == .messages }
.first?
.elements
.first?
.id
public func swapToThread(updatedThreadId: String, focussedMessageId: Int64?) {
self.threadId = updatedThreadId
self.observableThreadData = self.setupObservableThreadData(for: updatedThreadId)
self.pagedDataObserver = self.setupPagedObserver(
@ -804,8 +803,8 @@ public class ConversationViewModel: OWSAudioPlayerDelegate {
// Try load everything up to the initial visible message, fallback to just the initial page of messages
// if we don't have one
switch oldestMessageId {
case .some(let id): self.pagedDataObserver?.load(.untilInclusive(id: id, padding: 0))
switch focussedMessageId {
case .some(let id): self.pagedDataObserver?.load(.initialPageAround(id: id))
case .none: self.pagedDataObserver?.load(.pageBefore)
}
}

@ -633,11 +633,13 @@ public class PagedDatabaseObserver<ObservedTable, T>: TransactionObserver where
)
else { return (nil, nil) }
// If the target id is within a single page of the current cached data
// then trigger the `untilInclusive` behaviour instead
// If the targetIndex is over a page before the current content or more than a page
// after the current content then we want to reload the entire content (to avoid
// loading an excessive amount of data), otherwise we should load all messages between
// the current content and the targetIndex (plus padding)
guard
abs(targetIndex - cacheCurrentEndIndex) > currentPageInfo.pageSize ||
abs(targetIndex - currentPageInfo.pageOffset) > currentPageInfo.pageSize
(targetIndex < (currentPageInfo.pageOffset - currentPageInfo.pageSize)) ||
(targetIndex > (cacheCurrentEndIndex + currentPageInfo.pageSize))
else {
let callback: () -> () = {
self?.load(.untilInclusive(id: targetId, padding: paddingForInclusive))
@ -650,14 +652,7 @@ public class PagedDatabaseObserver<ObservedTable, T>: TransactionObserver where
let callback: () -> () = {
self?.dataCache.mutate { $0 = DataCache() }
self?.associatedRecords.forEach { $0.clearCache(db) }
self?.pageInfo.mutate {
$0 = PagedData.PageInfo(
pageSize: currentPageInfo.pageSize,
pageOffset: 0,
currentCount: 0,
totalCount: 0
)
}
self?.pageInfo.mutate { $0 = PagedData.PageInfo(pageSize: currentPageInfo.pageSize) }
self?.load(.initialPageAround(id: targetId))
}
@ -908,9 +903,15 @@ public enum PagedData {
case initialPageAround(id: SQLExpression)
case pageBefore
case pageAfter
case untilInclusive(id: SQLExpression, padding: Int)
case jumpTo(id: SQLExpression, paddingForInclusive: Int)
case reloadCurrent
/// This will be used when `jumpTo` is called and the `id` is within a single `pageSize` of the currently
/// cached data (plus the padding amount)
///
/// **Note:** If the id is already within the cache then this will do nothing (even if
/// the padding would mean more data should be loaded)
case untilInclusive(id: SQLExpression, padding: Int)
}
public enum Target<ID: SQLExpressible> {
@ -925,13 +926,6 @@ public enum PagedData {
/// This will attempt to load a page of data after the last item in the cache
case pageAfter
/// This will attempt to load all data between what is currently in the cache until the
/// specified id (plus the padding amount)
///
/// **Note:** If the id is already within the cache then this will do nothing (even if
/// the padding would mean more data should be loaded)
case untilInclusive(id: ID, padding: Int)
/// This will jump to the specified id, loading a page around it and clearing out any
/// data that was previously cached
///
@ -944,8 +938,6 @@ public enum PagedData {
case .initialPageAround(let id): return .initialPageAround(id: id.sqlExpression)
case .pageBefore: return .pageBefore
case .pageAfter: return .pageAfter
case .untilInclusive(let id, let padding):
return .untilInclusive(id: id.sqlExpression, padding: padding)
case .jumpTo(let id, let paddingForInclusive):
return .jumpTo(id: id.sqlExpression, paddingForInclusive: paddingForInclusive)

Loading…
Cancel
Save