From 273063e0aa86fc2ff7856028abcb63e846126eaa Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 22 Nov 2017 11:57:33 -0800 Subject: [PATCH 1/2] ConversationView first load avoids redundant layout We are laying out the collection view, invalidating the layout, and then laying out the collection view again on first appearance of the conversation view. This is quite expensive - removing it shaves off about 30% of load time. // FREEBIE --- .../ConversationView/ConversationViewController.m | 9 ++++++++- .../ConversationView/ConversationViewLayout.h | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index bdbc5e3b8..5280584f5 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -4461,7 +4461,14 @@ typedef enum : NSUInteger { // Snapshot the scroll state by measuring the "distance from top of view to // bottom of content"; if the mapping's "window" size grows, it will grow // _upward_. - CGFloat viewTopToContentBottom = self.safeContentHeight - self.collectionView.contentOffset.y; + CGFloat viewTopToContentBottom = 0; + if ([self.collectionView.collectionViewLayout isKindOfClass:[ConversationViewLayout class]]) { + ConversationViewLayout *conversationViewLayout + = (ConversationViewLayout *)self.collectionView.collectionViewLayout; + if (conversationViewLayout.hasLayout) { + viewTopToContentBottom = self.safeContentHeight - self.collectionView.contentOffset.y; + } + } NSUInteger oldCellCount = [self.messageMappings numberOfItemsInGroup:self.thread.uniqueId]; diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.h b/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.h index 538192cf8..d617c7b6a 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.h +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.h @@ -37,7 +37,7 @@ typedef NS_ENUM(NSInteger, ConversationViewLayoutAlignment) { @interface ConversationViewLayout : UICollectionViewLayout @property (nonatomic, weak) id delegate; - +@property (nonatomic, readonly) BOOL hasLayout; @property (nonatomic, readonly) int contentWidth; @end From 4f520646ccb5c1fa66c9a84a4b9c7bb05132f5f1 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 30 May 2018 10:45:25 -0400 Subject: [PATCH 2/2] Avoid double layout in conversation view; but carefully. --- .../ConversationView/ConversationViewController.m | 14 ++++++++------ .../ConversationView/ConversationViewLayout.h | 3 ++- .../ConversationView/ConversationViewLayout.m | 10 ++++++++++ 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 5280584f5..6c0414ebf 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -4462,12 +4462,14 @@ typedef enum : NSUInteger { // bottom of content"; if the mapping's "window" size grows, it will grow // _upward_. CGFloat viewTopToContentBottom = 0; - if ([self.collectionView.collectionViewLayout isKindOfClass:[ConversationViewLayout class]]) { - ConversationViewLayout *conversationViewLayout - = (ConversationViewLayout *)self.collectionView.collectionViewLayout; - if (conversationViewLayout.hasLayout) { - viewTopToContentBottom = self.safeContentHeight - self.collectionView.contentOffset.y; - } + OWSAssert([self.collectionView.collectionViewLayout isKindOfClass:[ConversationViewLayout class]]); + ConversationViewLayout *conversationViewLayout + = (ConversationViewLayout *)self.collectionView.collectionViewLayout; + // To avoid laying out the collection view during initial view + // presentation, don't trigger layout here (via safeContentHeight) + // until layout has been done at least once. + if (conversationViewLayout.hasEverHadLayout) { + viewTopToContentBottom = self.safeContentHeight - self.collectionView.contentOffset.y; } NSUInteger oldCellCount = [self.messageMappings numberOfItemsInGroup:self.thread.uniqueId]; diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.h b/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.h index d617c7b6a..822c56ee4 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.h +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.h @@ -1,5 +1,5 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // NS_ASSUME_NONNULL_BEGIN @@ -38,6 +38,7 @@ typedef NS_ENUM(NSInteger, ConversationViewLayoutAlignment) { @property (nonatomic, weak) id delegate; @property (nonatomic, readonly) BOOL hasLayout; +@property (nonatomic, readonly) BOOL hasEverHadLayout; @property (nonatomic, readonly) int contentWidth; @end diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.m b/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.m index 22a8eb9db..1835f7e95 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.m @@ -19,6 +19,7 @@ NS_ASSUME_NONNULL_BEGIN // layout without incurring any of the (great) expense of performing an // unnecessary layout pass. @property (nonatomic) BOOL hasLayout; +@property (nonatomic) BOOL hasEverHadLayout; @property (nonatomic) int contentWidth; @@ -37,6 +38,15 @@ NS_ASSUME_NONNULL_BEGIN return self; } +- (void)setHasLayout:(BOOL)hasLayout +{ + _hasLayout = hasLayout; + + if (hasLayout) { + self.hasEverHadLayout = YES; + } +} + - (void)invalidateLayout { [super invalidateLayout];