From 31ee89c3c79df79d03a4c8e59cd7589abb6f4ea6 Mon Sep 17 00:00:00 2001 From: Avently <7953703+avently@users.noreply.github.com> Date: Thu, 16 Jan 2025 04:54:56 -0800 Subject: [PATCH] optimized item identifiers to use merged item directly --- apps/ios/Shared/Model/ChatModel.swift | 11 ++- .../Shared/Views/Chat/ChatItemsMerger.swift | 37 ++------ apps/ios/Shared/Views/Chat/ChatView.swift | 2 +- apps/ios/Shared/Views/Chat/ReverseList.swift | 86 +++++-------------- 4 files changed, 37 insertions(+), 99 deletions(-) diff --git a/apps/ios/Shared/Model/ChatModel.swift b/apps/ios/Shared/Model/ChatModel.swift index 2e76e5c946..f3fad315f1 100644 --- a/apps/ios/Shared/Model/ChatModel.swift +++ b/apps/ios/Shared/Model/ChatModel.swift @@ -952,12 +952,17 @@ final class ChatModel: ObservableObject { // returns the previous member in the same merge group and the count of members in this group func getPrevHiddenMember(_ member: GroupMember, _ range: ClosedRange) -> (GroupMember?, Int) { + let items = im.reversedChatItems var prevMember: GroupMember? = nil var memberIds: Set = [] for i in range { - if case let .groupRcv(m) = im.reversedChatItems[i].chatDir { - if prevMember == nil && m.groupMemberId != member.groupMemberId { prevMember = m } - memberIds.insert(m.groupMemberId) + if i < items.count { + if case let .groupRcv(m) = items[i].chatDir { + if prevMember == nil && m.groupMemberId != member.groupMemberId { prevMember = m } + memberIds.insert(m.groupMemberId) + } + } else { + logger.error("getPrevHiddenMember: index >= count of reversed items: \(i) vs \(items.count)") } } return (prevMember, memberIds.count) diff --git a/apps/ios/Shared/Views/Chat/ChatItemsMerger.swift b/apps/ios/Shared/Views/Chat/ChatItemsMerger.swift index 5976249b39..26451f6d83 100644 --- a/apps/ios/Shared/Views/Chat/ChatItemsMerger.swift +++ b/apps/ios/Shared/Views/Chat/ChatItemsMerger.swift @@ -114,15 +114,11 @@ struct MergedItems { } -enum MergedItem: /*Identifiable, */Hashable { -// var id: Int64 { -// get { -// switch self { -// case let .single(item, _): item.item.id -// case let .grouped(items, _, _, _, _, _, _): items.boxedValue.last!.item.id -// } -// } -// } +enum MergedItem: Hashable, Equatable { + // equatable and hashable implementations allows to NSDiffableDataSourceSnapshot to see the difference and correcrly scroll items we want. Without any of it, the scroll position will be random in ReverseList + static func == (lhs: Self, rhs: Self) -> Bool { + lhs.hashValue == rhs.hashValue + } var hashValue: Int { self.newest().item.hashValue } @@ -275,26 +271,7 @@ class ActiveChatState { } } -struct BoxedValue2: /*Identifiable, */Hashable { -// var id: Int64 { (boxedValue as! MergedItem).id } - - static func == (lhs: BoxedValue2, rhs: BoxedValue2) -> Bool { - lhs.boxedValue == rhs.boxedValue - } - - var hashValue: Int { (boxedValue as! MergedItem).newest().hashValue } - - func hash(into hasher: inout Hasher) { - hasher.combine("\((boxedValue as! MergedItem).newest())") - } - - var boxedValue : T - init(_ value: T) { - self.boxedValue = value - } -} - -class BoxedValue: Hashable { +class BoxedValue: Equatable, Hashable { static func == (lhs: BoxedValue, rhs: BoxedValue) -> Bool { lhs.boxedValue == rhs.boxedValue } @@ -313,7 +290,7 @@ extension ReverseList.Controller { @MainActor func visibleItemIndexesNonReversed(_ mergedItems: Binding) -> ClosedRange { let zero = 0 ... 0 - if itemCount == 0 { + if mergedItems.wrappedValue.items.count == 0 { return zero } let listState = getListState() ?? ListState() diff --git a/apps/ios/Shared/Views/Chat/ChatView.swift b/apps/ios/Shared/Views/Chat/ChatView.swift index 3eaa4fbb1c..5bb000879e 100644 --- a/apps/ios/Shared/Views/Chat/ChatView.swift +++ b/apps/ios/Shared/Views/Chat/ChatView.swift @@ -445,7 +445,7 @@ struct ChatView: View { return GeometryReader { g in let _ = logger.debug("LALAL RELOAD \(im.reversedChatItems.count)") // LALAL CAN I CHANGE BINDING LIKE THIS IN ignoreLoadingRequests? - ReverseList(items: im.reversedChatItems, mergedItems: $mergedItems, revealedItems: $revealedItems, unreadCount: Binding.constant(chat.chatStats.unreadCount), scrollState: $scrollModel.state, loadingMoreItems: $loadingMoreItems, allowLoadMoreItems: $allowLoadMoreItems, ignoreLoadingRequests: searchValueIsEmpty ? $ignoreLoadingRequests : Binding.constant([])) { index, mergedItem in + ReverseList(mergedItems: mergedItems, revealedItems: $revealedItems, unreadCount: Binding.constant(chat.chatStats.unreadCount), scrollState: $scrollModel.state, loadingMoreItems: $loadingMoreItems, allowLoadMoreItems: $allowLoadMoreItems, ignoreLoadingRequests: searchValueIsEmpty ? $ignoreLoadingRequests : Binding.constant([])) { index, mergedItem in let ci = switch mergedItem { case let .single(item, _): item.item case let .grouped(items, _, _, _, _, _, _): items.boxedValue.last!.item diff --git a/apps/ios/Shared/Views/Chat/ReverseList.swift b/apps/ios/Shared/Views/Chat/ReverseList.swift index 6932dce3c2..1c1c703db5 100644 --- a/apps/ios/Shared/Views/Chat/ReverseList.swift +++ b/apps/ios/Shared/Views/Chat/ReverseList.swift @@ -12,8 +12,7 @@ import SimpleXChat /// A List, which displays it's items in reverse order - from bottom to top struct ReverseList: UIViewControllerRepresentable { - let items: Array - @Binding var mergedItems: MergedItems + var mergedItems: MergedItems @Binding var revealedItems: Set @Binding var unreadCount: Int @@ -37,11 +36,11 @@ struct ReverseList: UIViewControllerRepresentable { func updateUIViewController(_ controller: Controller, context: Context) { controller.representer = self - if case let .scrollingTo(destination) = scrollState, !items.isEmpty, !controller.scrollToItemInProgress { + if case let .scrollingTo(destination) = scrollState, !mergedItems.items.isEmpty, !controller.scrollToItemInProgress { controller.view.layer.removeAllAnimations() switch destination { case let .item(id): - let row = items.firstIndex(where: { $0.id == id }) + let row = mergedItems.indexInParentItems[id] if let row { logger.debug("LALAL SCROLLING TO \(row)") controller.scroll(to: row, position: .bottom) @@ -64,11 +63,11 @@ struct ReverseList: UIViewControllerRepresentable { // so it's better to just wait until dragging ends if waitOnEndedScrolling && controller.tableView.isDragging/* && !controller.tableView.isDecelerating*/ { controller.runBlockOnEndDecelerating = { - controller.update(items: items) + controller.update() } } else { controller.runBlockOnEndDecelerating = nil - controller.update(items: items) + controller.update() } } } @@ -77,17 +76,7 @@ struct ReverseList: UIViewControllerRepresentable { public class Controller: UITableViewController { private enum Section { case main } var representer: ReverseList - // Here Int means hash of the ChatItem that is inside MergedItem.newest().item.hashValue. - // Putting MergedItem here directly prevents UITableViewDiffableDataSource to make partial updates - // which looks like UITableView scrolls to bottom on insert values to bottom instead of - // remains in the same scroll position - private var dataSource: UITableViewDiffableDataSource! - var itemCount: Int { - get { - representer.mergedItems.items.count - } - } - private var itemsInPrevSnapshot: Dictionary = [:] + private var dataSource: UITableViewDiffableDataSource! private let updateFloatingButtons = PassthroughSubject() private var bag = Set() @@ -128,17 +117,17 @@ struct ReverseList: UIViewControllerRepresentable { } // 3. Configure data source - self.dataSource = UITableViewDiffableDataSource( + self.dataSource = UITableViewDiffableDataSource( tableView: tableView ) { (tableView, indexPath, item) -> UITableViewCell? in let cell = tableView.dequeueReusableCell(withIdentifier: cellReuseId, for: indexPath) if #available(iOS 16.0, *) { - cell.contentConfiguration = UIHostingConfiguration { self.representer.content(indexPath.item, self.itemsInPrevSnapshot[item]!) } + cell.contentConfiguration = UIHostingConfiguration { self.representer.content(indexPath.item, item) } .margins(.all, 0) .minSize(height: 1) // Passing zero will result in system default of 44 points being used } else { if let cell = cell as? HostingCell { - cell.set(content: self.representer.content(indexPath.item, self.itemsInPrevSnapshot[item]!), parent: self) + cell.set(content: self.representer.content(indexPath.item, item), parent: self) } else { fatalError("Unexpected Cell Type for: \(item)") } @@ -269,46 +258,26 @@ struct ReverseList: UIViewControllerRepresentable { } } - func update(items: [ChatItem]) { - var prevSnapshot = dataSource.snapshot() + func update() { + let prevSnapshot = dataSource.snapshot() let wasCount = prevSnapshot.numberOfItems - let willBeCount = representer.mergedItems.items.count + let items = representer.mergedItems.items + let willBeCount = items.count let c = 1 let insertedSeveralNewestItems = wasCount != 0 && willBeCount - wasCount == c && prevSnapshot.itemIdentifiers.first!.hashValue == self.representer.mergedItems.items[c].hashValue logger.debug("LALAL WAS \(wasCount) will be \(self.representer.mergedItems.items.count)") - //self.representer.mergedItems = MergedItems.create(items, representer.unreadCount, representer.revealedItems, ItemsModel.shared.chatState) - let snapshot: NSDiffableDataSourceSnapshot - let itemsInCurrentSnapshot: Dictionary - if insertedSeveralNewestItems { - var new = itemsInPrevSnapshot - for i in 0 ..< c { - prevSnapshot.insertItems([representer.mergedItems.items[c - i].hashValue], beforeItem: prevSnapshot.itemIdentifiers.first!) - new[representer.mergedItems.items[c - i].hashValue] = representer.mergedItems.items[c - i] - } - itemsInCurrentSnapshot = new - snapshot = prevSnapshot - } else { - logger.debug("LALAL BEFORE snap") - var new: Dictionary = [:] - let identifiers = representer.mergedItems.items.map({ merged in - new[merged.hashValue] = merged - return merged.hashValue - }) - logger.debug("LALAL AFTER snap") - - if identifiers == prevSnapshot.itemIdentifiers { + if items == prevSnapshot.itemIdentifiers { logger.debug("LALAL SAME ITEMS, not rebuilding the tableview") // update counters because they are static, unbound to specific chat and will become outdated if a new empty chat was open after non-empty one with unread messages updateFloatingButtons.send() return } - var snap = NSDiffableDataSourceSnapshot() - snap.appendSections([.main]) - snap.appendItems(identifiers) - itemsInCurrentSnapshot = new - snapshot = snap - } + var snapshot = NSDiffableDataSourceSnapshot() + snapshot.appendSections([.main]) + snapshot.appendItems(items) + dataSource.defaultRowAnimation = .none + let wasContentHeight = tableView.contentSize.height let wasOffset = tableView.contentOffset.y let listState = getListState() @@ -317,12 +286,11 @@ struct ReverseList: UIViewControllerRepresentable { let countDiff = max(0, snapshot.numberOfItems - prevSnapshot.numberOfItems) // Sets content offset on initial load if wasCount == 0 { - itemsInPrevSnapshot = itemsInCurrentSnapshot dataSource.apply( snapshot, animatingDifferences: insertedSeveralNewestItems ) - if let firstUnreadItem = snapshot.itemIdentifiers.lastIndex(where: { hash in itemsInPrevSnapshot[hash]!.hasUnread() }) { + if let firstUnreadItem = snapshot.itemIdentifiers.lastIndex(where: { $0.hasUnread() }) { scrollToRowWhenKnowSize(firstUnreadItem) } else { tableView.setContentOffset( @@ -332,18 +300,14 @@ struct ReverseList: UIViewControllerRepresentable { } } else if wasCount != snapshot.numberOfItems { logger.debug("LALAL drag \(self.tableView.isDragging), decel \(self.tableView.isDecelerating)") - // tableView.panGestureRecognizer.isEnabled = false - // logger.debug("LALAL drag2 \(self.tableView.isDragging), decel \(self.tableView.isDecelerating)") if useSmoothScrolling && tableView.isDecelerating { - // CATransaction.begin() - itemsInPrevSnapshot = itemsInCurrentSnapshot + tableView.panGestureRecognizer.isEnabled = false tableView.beginUpdates() dataSource.apply( snapshot, animatingDifferences: false ) tableView.endUpdates() - // CATransaction.commit() tableView.panGestureRecognizer.isEnabled = true } else { // remember current translation @@ -354,7 +318,6 @@ struct ReverseList: UIViewControllerRepresentable { translationToApply = t } } - itemsInPrevSnapshot = itemsInCurrentSnapshot dataSource.apply( snapshot, animatingDifferences: false @@ -516,13 +479,6 @@ struct ReverseList: UIViewControllerRepresentable { await preloadItems(self.representer.mergedItems, self.representer.allowLoadMoreItems, state, self.representer.$ignoreLoadingRequests) { pagination in logger.debug("LALAL LOADING INSIDE") let triedToLoad = await self.representer.loadItems(false, pagination, { self.visibleItemIndexesNonReversed(Binding.constant(self.representer.mergedItems)) }) - let superview = self.tableView.superview - if triedToLoad, let superview { -// let t = self.tableView.panGestureRecognizer.translation(in: superview) -// if t.y != 0 { -// self.translationToApply = t -// } - } return triedToLoad } }