From 2a480d24dcde57efe0ae9564838bb6e56a9ee8eb Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 16 Dec 2020 10:12:07 -0300 Subject: [PATCH] Modify layout to handle performBatchUpdates(). --- .../CV/CVLoadCoordinator.swift | 45 +++----- .../ConversationView/CVViewState.swift | 12 ++- .../ConversationCollectionView.h | 3 +- .../ConversationCollectionView.m | 8 ++ .../ConversationViewController.m | 100 +++++++++++++----- .../ConversationViewLayout.swift | 25 +++-- 6 files changed, 125 insertions(+), 68 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/CV/CVLoadCoordinator.swift b/Signal/src/ViewControllers/ConversationView/CV/CVLoadCoordinator.swift index 4b9590d66e..00b248a800 100644 --- a/Signal/src/ViewControllers/ConversationView/CV/CVLoadCoordinator.swift +++ b/Signal/src/ViewControllers/ConversationView/CV/CVLoadCoordinator.swift @@ -528,17 +528,25 @@ public class CVLoadCoordinator: NSObject { let (loadPromise, loadResolver) = Promise.pending() + var attemptCount = 0 + let viewState = self.viewState - func canLoad() -> Bool { - !viewState.isScrollingToTop && !viewState.isUserScrolling + func canLandLoad() -> Bool { + let canLandLoad = !viewState.hasScrollingAnimation && !viewState.isUserScrolling + attemptCount += 1 + Logger.verbose("attemptCount: \(attemptCount), canLandLoad: \(canLandLoad), hasScrollingAnimation: \(viewState.hasScrollingAnimation), isUserScrolling: \(viewState.isUserScrolling), ") + return canLandLoad +// !viewState.hasScrollingAnimation && !viewState.isUserScrolling } func tryToResolve() { - guard canLoad() else { + guard canLandLoad() else { // TODO: async() or asyncAfter()? Logger.verbose("Waiting to land load.") + // DispatchQueue.asyncAfter() will take longer to perform + // its block than DispatchQueue.async() if the CPU is under + // heavy load. That's desirable in this case. DispatchQueue.main.asyncAfter(deadline: .now() + 0.001) { -// DispatchQueue.main.async { tryToResolve() } return @@ -573,31 +581,6 @@ public class CVLoadCoordinator: NSObject { return loadPromise } - // public func waitUntilCanLandLoad() -> Promise { - // AssertIsOnMainThread() - // - // guard let canLandLoadPromise = canLandLoadPromise else { - // return Promise.value(()) - // } - // return canLandLoadPromise.promise - // } - // private func tryToFireCanLandLoad() { - // AssertIsOnMainThread() - // - // guard let canLandLoadPromise = canLandLoadPromise else { - // return - // } - // guard !isScrollingToTop else { - // return - // } - // canLandLoadPromise.resolver.fulfill(()) - // self.canLandLoadPromise = nil - // } - // private struct PromiseAndResolver { - // let promise: Promise - // let resolver: Resolver - // } - // private var canLandLoadPromise: PromiseAndResolver? // - @@ -872,6 +855,10 @@ extension CVLoadCoordinator: UIScrollViewDelegate { public func scrollViewDidScrollToTop(_ scrollView: UIScrollView) { delegate?.scrollViewDidScrollToTop?(scrollView) } + + public func scrollViewDidEndScrollingAnimation(_ scrollView: UIScrollView) { + delegate?.scrollViewDidEndScrollingAnimation?(scrollView) + } } // MARK: - diff --git a/Signal/src/ViewControllers/ConversationView/CVViewState.swift b/Signal/src/ViewControllers/ConversationView/CVViewState.swift index 730c81bb42..1072e2339d 100644 --- a/Signal/src/ViewControllers/ConversationView/CVViewState.swift +++ b/Signal/src/ViewControllers/ConversationView/CVViewState.swift @@ -64,7 +64,9 @@ public class CVViewState: NSObject { @objc public var isUserScrolling = false @objc - public var isScrollingToTop = false + public var scrollingAnimationStartDate: Date? + @objc + public var hasScrollingAnimation: Bool { scrollingAnimationStartDate != nil } @objc public var scrollContinuity: ScrollContinuity = .bottom public var scrollContinuityMap: CVScrollContinuityMap? @@ -202,11 +204,13 @@ public extension ConversationViewController { set { viewState.isUserScrolling = newValue } } - var isScrollingToTop: Bool { - get { viewState.isScrollingToTop } - set { viewState.isScrollingToTop = newValue } + var scrollingAnimationStartDate: Date? { + get { viewState.scrollingAnimationStartDate } + set { viewState.scrollingAnimationStartDate = newValue } } + var hasScrollingAnimation: Bool { viewState.hasScrollingAnimation } + var scrollContinuity: ScrollContinuity { get { viewState.scrollContinuity } set { viewState.scrollContinuity = newValue } diff --git a/Signal/src/ViewControllers/ConversationView/ConversationCollectionView.h b/Signal/src/ViewControllers/ConversationView/ConversationCollectionView.h index 419c4574eb..3f39ea32f7 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationCollectionView.h +++ b/Signal/src/ViewControllers/ConversationView/ConversationCollectionView.h @@ -1,5 +1,5 @@ // -// Copyright (c) 2019 Open Whisper Systems. All rights reserved. +// Copyright (c) 2020 Open Whisper Systems. All rights reserved. // NS_ASSUME_NONNULL_BEGIN @@ -8,6 +8,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)collectionViewWillChangeSizeFrom:(CGSize)oldSize to:(CGSize)newSize; - (void)collectionViewDidChangeSizeFrom:(CGSize)oldSize to:(CGSize)newSize; +- (void)collectionViewWillAnimate; @end diff --git a/Signal/src/ViewControllers/ConversationView/ConversationCollectionView.m b/Signal/src/ViewControllers/ConversationView/ConversationCollectionView.m index 24fbf8cec0..315fee3d62 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationCollectionView.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationCollectionView.m @@ -64,6 +64,10 @@ NS_ASSUME_NONNULL_BEGIN - (void)setContentOffset:(CGPoint)contentOffset animated:(BOOL)animated { + if (animated) { + [self.layoutDelegate collectionViewWillAnimate]; + } + [super setContentOffset:contentOffset animated:animated]; } @@ -97,6 +101,10 @@ NS_ASSUME_NONNULL_BEGIN - (void)scrollRectToVisible:(CGRect)rect animated:(BOOL)animated { + if (animated) { + [self.layoutDelegate collectionViewWillAnimate]; + } + [super scrollRectToVisible:rect animated:animated]; } diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 28ad13d9b4..112004464f 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -936,6 +936,9 @@ typedef enum : NSUInteger { - (void)readTimerDidFire { + if (self.layout.isPerformingBatchUpdates || self.hasScrollingAnimation) { + return; + } [self markVisibleMessagesAsRead]; } @@ -3173,10 +3176,15 @@ typedef enum : NSUInteger { { self.userHasScrolled = YES; self.isUserScrolling = YES; + [self scrollingAnimationDidStart]; } - (void)scrollViewDidEndDragging:(UIScrollView *)scrollView willDecelerate:(BOOL)willDecelerate { + if (!willDecelerate) { + [self scrollingAnimationDidComplete]; + } + if (!self.isUserScrolling) { return; } @@ -3192,6 +3200,8 @@ typedef enum : NSUInteger { - (void)scrollViewDidEndDecelerating:(UIScrollView *)scrollView { + [self scrollingAnimationDidComplete]; + if (!self.isWaitingForDeceleration) { return; } @@ -3203,24 +3213,74 @@ typedef enum : NSUInteger { - (BOOL)scrollViewShouldScrollToTop:(UIScrollView *)scrollView { - self.isScrollingToTop = YES; - // isScrollingToTop blocks landing of loads, so we must ensure - // that it is always cleared in a timely way, even if the animation - // is cancelled. Wait no more than 2 seconds. - __weak ConversationViewController *weakSelf = self; - dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)2.f * NSEC_PER_SEC), dispatch_get_main_queue(), ^{ - // isScrollingToTop should already have been cleared... - OWSAssertDebug(!weakSelf.isScrollingToTop); - // ...but we want to make sure. - weakSelf.isScrollingToTop = NO; - }); + [self scrollingAnimationDidStart]; + return YES; } - (void)scrollViewDidScrollToTop:(UIScrollView *)scrollView { - self.isScrollingToTop = NO; + [self scrollingAnimationDidComplete]; +} + +- (void)scrollViewDidEndScrollingAnimation:(UIScrollView *)scrollView +{ + [self scrollingAnimationDidComplete]; +} + +#pragma mark - ConversationCollectionViewDelegate + +- (void)collectionViewWillChangeSizeFrom:(CGSize)oldSize to:(CGSize)newSize +{ + OWSAssertIsOnMainThread(); +} + +- (void)collectionViewDidChangeSizeFrom:(CGSize)oldSize to:(CGSize)newSize +{ + OWSAssertIsOnMainThread(); + + if (oldSize.width != newSize.width) { + [self resetForSizeOrOrientationChange]; + } +} + +- (void)collectionViewWillAnimate +{ + [self scrollingAnimationDidStart]; +} + +- (void)scrollingAnimationDidStart +{ + OWSAssertIsOnMainThread(); + + NSDate *startDate = [NSDate new]; + self.scrollingAnimationStartDate = startDate; + + // scrollingAnimationStartDate blocks landing of loads, so we must ensure + // that it is always cleared in a timely way, even if the animation + // is cancelled. Wait no more than N seconds. + __weak ConversationViewController *weakSelf = self; + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)5.f * NSEC_PER_SEC), dispatch_get_main_queue(), ^{ + ConversationViewController *_Nullable strongSelf = weakSelf; + if (!strongSelf) { + return; + } + + // scrollingAnimationStartDate should already have been cleared, + // but we need to ensure that it is cleared in a timely way. + if ([NSObject isNullableObject:strongSelf.scrollingAnimationStartDate equalTo:startDate]) { + OWSFailDebug(@"Scrolling animation did not complete in a timely way."); + [strongSelf scrollingAnimationDidComplete]; + } + }); +} + +- (void)scrollingAnimationDidComplete +{ + OWSAssertIsOnMainThread(); + + self.scrollingAnimationStartDate = nil; [self autoLoadMoreIfNecessary]; } @@ -3549,22 +3609,6 @@ typedef enum : NSUInteger { } } -#pragma mark - ConversationCollectionViewDelegate - -- (void)collectionViewWillChangeSizeFrom:(CGSize)oldSize to:(CGSize)newSize -{ - OWSAssertIsOnMainThread(); -} - -- (void)collectionViewDidChangeSizeFrom:(CGSize)oldSize to:(CGSize)newSize -{ - OWSAssertIsOnMainThread(); - - if (oldSize.width != newSize.width) { - [self resetForSizeOrOrientationChange]; - } -} - #pragma mark - ContactsPickerDelegate - (void)contactsPickerDidCancel:(ContactsPicker *)contactsPicker diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.swift b/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.swift index fc543013ec..999a67dbe0 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.swift +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.swift @@ -380,7 +380,8 @@ public class ConversationViewLayout: UICollectionViewLayout { // the initial (last) layout state for items. private var lastLayoutInfo: LayoutInfo? - private var isPerformingBatchUpdates = false + @objc + public var isPerformingBatchUpdates = false private var hasInvalidatedDataSourceCounts = false @objc @@ -447,20 +448,32 @@ public class ConversationViewLayout: UICollectionViewLayout { super.finalizeLayoutTransition() } + // MARK: - + // A layout can return the content offset to be applied during transition or update animations. public override func targetContentOffset(forProposedContentOffset proposedContentOffset: CGPoint, withScrollingVelocity velocity: CGPoint) -> CGPoint { - return proposedContentOffset + guard isPerformingBatchUpdates else { + return proposedContentOffset + } + guard let delegate = delegate else { + return super.targetContentOffset(forProposedContentOffset: proposedContentOffset, + withScrollingVelocity: velocity) + } + return delegate.targetContentOffset(forProposedContentOffset: proposedContentOffset) } // A layout can return the content offset to be applied during transition or update animations. public override func targetContentOffset(forProposedContentOffset proposedContentOffset: CGPoint) -> CGPoint { - return proposedContentOffset + guard isPerformingBatchUpdates else { + return proposedContentOffset + } + guard let delegate = delegate else { + return super.targetContentOffset(forProposedContentOffset: proposedContentOffset) + } + return delegate.targetContentOffset(forProposedContentOffset: proposedContentOffset) } - private var initialLayoutInfo: LayoutInfo? { - lastLayoutInfo - } private var finalLayoutInfo: LayoutInfo { ensureCurrentLayoutInfo()