Skip to content
This repository was archived by the owner on Jan 17, 2019. It is now read-only.

Commit a410956

Browse files
authored
Merge pull request #89 from spotify/separate-rendering-logic
Separate UICollectionView update logic and diff only before reloads
2 parents 9b01f01 + ed81a31 commit a410956

File tree

4 files changed

+182
-60
lines changed

4 files changed

+182
-60
lines changed

HubFramework.xcodeproj/project.pbxproj

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@
114114
DDBCF36E1C68DE2C00693038 /* CoreGraphics.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = DDBCF36D1C68DE2C00693038 /* CoreGraphics.framework */; };
115115
DDBCF36F1C68DE4900693038 /* CoreGraphics.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = DDBCF36D1C68DE2C00693038 /* CoreGraphics.framework */; };
116116
DDBCF3701C68DE5000693038 /* UIKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = DDBCF36B1C68DD2300693038 /* UIKit.framework */; };
117+
F64C5C2D1DB82CA30077E619 /* HUBViewModelRenderer.m in Sources */ = {isa = PBXBuildFile; fileRef = F64C5C2C1DB82CA30077E619 /* HUBViewModelRenderer.m */; };
117118
F66658D91D9925CC0097929F /* HUBViewModelDiff.m in Sources */ = {isa = PBXBuildFile; fileRef = F66658D81D9925CC0097929F /* HUBViewModelDiff.m */; };
118119
F6665AA71D9947E00097929F /* HUBViewModelDiffTests.m in Sources */ = {isa = PBXBuildFile; fileRef = F6665AA61D9947E00097929F /* HUBViewModelDiffTests.m */; };
119120
F6AC23C21DA2863A001B1A6A /* HUBComponentWrapperTests.m in Sources */ = {isa = PBXBuildFile; fileRef = F6AC23C11DA2863A001B1A6A /* HUBComponentWrapperTests.m */; };
@@ -391,6 +392,8 @@
391392
DDA41C8E1C6CB5C00056E511 /* HUBUtilities.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = HUBUtilities.h; sourceTree = "<group>"; };
392393
DDBCF36B1C68DD2300693038 /* UIKit.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = UIKit.framework; path = System/Library/Frameworks/UIKit.framework; sourceTree = SDKROOT; };
393394
DDBCF36D1C68DE2C00693038 /* CoreGraphics.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = CoreGraphics.framework; path = System/Library/Frameworks/CoreGraphics.framework; sourceTree = SDKROOT; };
395+
F64C5C2B1DB82CA30077E619 /* HUBViewModelRenderer.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = HUBViewModelRenderer.h; sourceTree = "<group>"; };
396+
F64C5C2C1DB82CA30077E619 /* HUBViewModelRenderer.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = HUBViewModelRenderer.m; sourceTree = "<group>"; };
394397
F663FE7C1D10BECE003E19B6 /* HUBActionContext.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = HUBActionContext.h; sourceTree = "<group>"; };
395398
F66658D71D9925CC0097929F /* HUBViewModelDiff.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = HUBViewModelDiff.h; sourceTree = "<group>"; };
396399
F66658D81D9925CC0097929F /* HUBViewModelDiff.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = HUBViewModelDiff.m; sourceTree = "<group>"; };
@@ -991,6 +994,8 @@
991994
children = (
992995
8AF9FA051C5254F5003F3D6C /* HUBViewModelImplementation.h */,
993996
8AF9FA061C5254F5003F3D6C /* HUBViewModelImplementation.m */,
997+
F64C5C2B1DB82CA30077E619 /* HUBViewModelRenderer.h */,
998+
F64C5C2C1DB82CA30077E619 /* HUBViewModelRenderer.m */,
994999
8AF5B57D1C64B59E001FF228 /* HUBViewModelLoaderImplementation.h */,
9951000
8AF5B57E1C64B59E001FF228 /* HUBViewModelLoaderImplementation.m */,
9961001
8AD064721C68F0820086C081 /* HUBViewModelLoaderFactoryImplementation.h */,
@@ -1112,6 +1117,7 @@
11121117
52977ACA1DA7D0B40064629E /* HUBBlockContentOperationFactory.m in Sources */,
11131118
8A6ACAB31D7D893400102EA9 /* HUBActionContextImplementation.m in Sources */,
11141119
8A786BBE1C5A595900B2AB9E /* HUBJSONPathImplementation.m in Sources */,
1120+
F64C5C2D1DB82CA30077E619 /* HUBViewModelRenderer.m in Sources */,
11151121
8AA29C851C4FAA9200E972B7 /* HUBComponentModelImplementation.m in Sources */,
11161122
8AD14E8A1D994BB40008E182 /* HUBDefaultImageLoaderFactory.m in Sources */,
11171123
8AD064741C68F0820086C081 /* HUBViewModelLoaderFactoryImplementation.m in Sources */,

sources/HUBViewControllerImplementation.m

Lines changed: 19 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
#import "HUBActionPerformer.h"
5050
#import "HUBViewModelDiff.h"
5151
#import "HUBComponentGestureRecognizer.h"
52+
#import "HUBViewModelRenderer.h"
5253

5354
static NSTimeInterval const HUBImageDownloadTimeThreshold = 0.07;
5455

@@ -74,6 +75,7 @@ @interface HUBViewControllerImplementation () <
7475
@property (nonatomic, strong, nullable, readonly) id<HUBContentReloadPolicy> contentReloadPolicy;
7576
@property (nonatomic, strong, nullable, readonly) id<HUBImageLoader> imageLoader;
7677
@property (nonatomic, strong, nullable) UICollectionView *collectionView;
78+
@property (nonatomic, strong, nullable) HUBViewModelRenderer *viewModelRenderer;
7779
@property (nonatomic, assign) BOOL collectionViewIsScrolling;
7880
@property (nonatomic, strong, readonly) NSMutableSet<NSString *> *registeredCollectionViewCellReuseIdentifiers;
7981
@property (nonatomic, strong, readonly) NSMutableDictionary<NSURL *, NSMutableArray<HUBComponentImageLoadingContext *> *> *componentImageLoadingContexts;
@@ -87,10 +89,8 @@ @interface HUBViewControllerImplementation () <
8789
@property (nonatomic, strong, readonly) HUBComponentReusePool *childComponentReusePool;
8890
@property (nonatomic, strong, nullable) HUBComponentWrapper *highlightedComponentWrapper;
8991
@property (nonatomic, strong, nullable) id<HUBViewModel> viewModel;
90-
@property (nonatomic, strong, nullable) HUBViewModelDiff *lastViewModelDiff;
9192
@property (nonatomic, assign) BOOL viewHasAppeared;
9293
@property (nonatomic, assign) BOOL viewHasBeenLaidOut;
93-
@property (nonatomic) BOOL viewModelIsInitial;
9494
@property (nonatomic) BOOL viewModelHasChangedSinceLastLayoutUpdate;
9595
@property (nonatomic) CGFloat visibleKeyboardHeight;
9696

@@ -137,7 +137,6 @@ - (instancetype)initWithViewURI:(NSURL *)viewURI
137137
_actionHandler = actionHandler;
138138
_scrollHandler = scrollHandler;
139139
_imageLoader = imageLoader;
140-
_viewModelIsInitial = YES;
141140
_registeredCollectionViewCellReuseIdentifiers = [NSMutableSet new];
142141
_componentImageLoadingContexts = [NSMutableDictionary new];
143142
_contentOffsetObservingComponentWrappers = [NSHashTable hashTableWithOptions:NSPointerFunctionsWeakMemory];
@@ -228,8 +227,11 @@ - (void)viewDidLayoutSubviews
228227
self.viewHasBeenLaidOut = YES;
229228

230229
if (self.viewModel != nil) {
231-
id<HUBViewModel> const viewModel = self.viewModel;
232-
[self reloadCollectionViewWithViewModel:viewModel animated:NO];
230+
if (self.viewModelHasChangedSinceLastLayoutUpdate || !CGRectEqualToRect(self.collectionView.frame, self.view.bounds)) {
231+
self.collectionView.frame = self.view.bounds;
232+
id<HUBViewModel> const viewModel = self.viewModel;
233+
[self reloadCollectionViewWithViewModel:viewModel animated:NO];
234+
}
233235
}
234236
}
235237

@@ -351,16 +353,10 @@ - (void)viewModelLoader:(id<HUBViewModelLoader>)viewModelLoader didLoadViewModel
351353

352354
id<HUBViewControllerDelegate> const delegate = self.delegate;
353355
[delegate viewController:self willUpdateWithViewModel:viewModel];
354-
355-
if (self.viewModel != nil && !self.viewModelIsInitial) {
356-
id<HUBViewModel> const currentModel = self.viewModel;
357-
self.lastViewModelDiff = [HUBViewModelDiff diffFromViewModel:currentModel toViewModel:viewModel];
358-
}
359356

360357
HUBCopyNavigationItemProperties(self.navigationItem, viewModel.navigationItem);
361358

362359
self.viewModel = viewModel;
363-
self.viewModelIsInitial = NO;
364360
self.viewModelHasChangedSinceLastLayoutUpdate = YES;
365361
[self.view setNeedsLayout];
366362

@@ -761,67 +757,30 @@ - (void)createCollectionViewIfNeeded
761757

762758
- (void)reloadCollectionViewWithViewModel:(id<HUBViewModel>)viewModel animated:(BOOL)animated
763759
{
764-
if (!self.viewModelHasChangedSinceLastLayoutUpdate) {
765-
if (CGRectEqualToRect(self.collectionView.frame, self.view.bounds)) {
766-
return;
767-
}
768-
}
769-
770-
self.collectionView.frame = self.view.bounds;
771-
772-
[self saveStatesForVisibleComponents];
773-
774760
if (![self.collectionView.collectionViewLayout isKindOfClass:[HUBCollectionViewLayout class]]) {
775761
self.collectionView.collectionViewLayout = [[HUBCollectionViewLayout alloc] initWithComponentRegistry:self.componentRegistry
776762
componentLayoutManager:self.componentLayoutManager];
777763
}
778-
779-
HUBCollectionViewLayout * const layout = (HUBCollectionViewLayout *)self.collectionView.collectionViewLayout;
780-
781-
/* Performing batch updates inbetween viewDidLoad and viewDidAppear is seemingly not allowed, as it
782-
causes an assertion inside a private UICollectionView method. If no diff exists, fall back to
783-
a complete reload. */
784-
if (!self.viewHasAppeared || self.lastViewModelDiff == nil) {
785-
[self.collectionView reloadData];
786-
787-
[layout computeForCollectionViewSize:self.collectionView.frame.size viewModel:viewModel diff:self.lastViewModelDiff];
788764

789-
if (self.viewHasAppeared) {
790-
/* Forcing a re-layout as the reloadData-call doesn't trigger the numberOfItemsInSection:-calls
791-
by itself, and batch update calls don't play well without having an initial item count. */
792-
[self.collectionView setNeedsLayout];
793-
[self.collectionView layoutIfNeeded];
794-
}
795-
796-
self.lastViewModelDiff = nil;
797-
} else {
798-
void (^updateBlock)() = ^{
799-
[self.collectionView performBatchUpdates:^{
800-
HUBViewModelDiff * const lastDiff = self.lastViewModelDiff;
801-
802-
[self.collectionView insertItemsAtIndexPaths:lastDiff.insertedBodyComponentIndexPaths];
803-
[self.collectionView deleteItemsAtIndexPaths:lastDiff.deletedBodyComponentIndexPaths];
804-
[self.collectionView reloadItemsAtIndexPaths:lastDiff.reloadedBodyComponentIndexPaths];
805-
806-
[layout computeForCollectionViewSize:self.collectionView.frame.size viewModel:viewModel diff:self.lastViewModelDiff];
807-
} completion:^(BOOL finished) {
808-
self.lastViewModelDiff = nil;
809-
}];
810-
};
811-
812-
if (animated) {
813-
updateBlock();
814-
} else {
815-
[UIView performWithoutAnimation:updateBlock];
816-
}
765+
if (self.viewModelRenderer == nil) {
766+
UICollectionView * const nonnullCollectionView = self.collectionView;
767+
self.viewModelRenderer = [[HUBViewModelRenderer alloc] initWithCollectionView:nonnullCollectionView];
817768
}
769+
770+
[self saveStatesForVisibleComponents];
771+
772+
[self.viewModelRenderer renderViewModel:viewModel
773+
usingBatchUpdates:self.viewHasAppeared
774+
animated:animated
775+
completion:^{
776+
[self.delegate viewControllerDidFinishRendering:self];
777+
}];
818778

819779
[self configureHeaderComponent];
820780
[self configureOverlayComponents];
821781
[self headerAndOverlayComponentViewsWillAppear];
822782

823783
self.viewModelHasChangedSinceLastLayoutUpdate = NO;
824-
[self.delegate viewControllerDidFinishRendering:self];
825784
}
826785

827786
- (void)saveStatesForVisibleComponents

sources/HUBViewModelRenderer.h

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* Copyright (c) 2016 Spotify AB.
3+
*
4+
* Licensed to the Apache Software Foundation (ASF) under one
5+
* or more contributor license agreements. See the NOTICE file
6+
* distributed with this work for additional information
7+
* regarding copyright ownership. The ASF licenses this file
8+
* to you under the Apache License, Version 2.0 (the
9+
* "License"); you may not use this file except in compliance
10+
* with the License. You may obtain a copy of the License at
11+
*
12+
* http://www.apache.org/licenses/LICENSE-2.0
13+
*
14+
* Unless required by applicable law or agreed to in writing,
15+
* software distributed under the License is distributed on an
16+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
17+
* KIND, either express or implied. See the License for the
18+
* specific language governing permissions and limitations
19+
* under the License.
20+
*/
21+
22+
#import <UIKit/UIKit.h>
23+
#import "HUBViewModel.h"
24+
#import "HUBHeaderMacros.h"
25+
26+
NS_ASSUME_NONNULL_BEGIN
27+
28+
/**
29+
* A class used to render view models in a collection view.
30+
*/
31+
@interface HUBViewModelRenderer : NSObject
32+
33+
/**
34+
* Initializes a @c HUBViewModelRenderer with a provided collection view.
35+
*
36+
* @param collectionView The collection view to use for rendering.
37+
*/
38+
- (instancetype)initWithCollectionView:(UICollectionView *)collectionView HUB_DESIGNATED_INITIALIZER;
39+
40+
/**
41+
* Renders the provided view model in the collection view.
42+
*
43+
* @param viewModel The view model to render.
44+
* @param usingBatchUpdates Whether the renderer should render using batch updates or not.
45+
* @param animated Whether the renderer should render with animations or not.
46+
* @param completionBlock The block to be called once the rendering is completed.
47+
*/
48+
- (void)renderViewModel:(id<HUBViewModel>)viewModel
49+
usingBatchUpdates:(BOOL)usingBatchUpdates
50+
animated:(BOOL)animated
51+
completion:(void(^)())completionBlock;
52+
53+
@end
54+
55+
NS_ASSUME_NONNULL_END

sources/HUBViewModelRenderer.m

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
/*
2+
* Copyright (c) 2016 Spotify AB.
3+
*
4+
* Licensed to the Apache Software Foundation (ASF) under one
5+
* or more contributor license agreements. See the NOTICE file
6+
* distributed with this work for additional information
7+
* regarding copyright ownership. The ASF licenses this file
8+
* to you under the Apache License, Version 2.0 (the
9+
* "License"); you may not use this file except in compliance
10+
* with the License. You may obtain a copy of the License at
11+
*
12+
* http://www.apache.org/licenses/LICENSE-2.0
13+
*
14+
* Unless required by applicable law or agreed to in writing,
15+
* software distributed under the License is distributed on an
16+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
17+
* KIND, either express or implied. See the License for the
18+
* specific language governing permissions and limitations
19+
* under the License.
20+
*/
21+
22+
#import "HUBViewModelRenderer.h"
23+
#import "HUBViewModelDiff.h"
24+
#import "HUBCollectionViewLayout.h"
25+
26+
NS_ASSUME_NONNULL_BEGIN
27+
28+
@interface HUBViewModelRenderer ()
29+
30+
@property (nonatomic, strong, readonly) UICollectionView *collectionView;
31+
@property (nonatomic, strong, nullable) id<HUBViewModel> lastRenderedViewModel;
32+
33+
@end
34+
35+
@implementation HUBViewModelRenderer
36+
37+
- (instancetype)initWithCollectionView:(UICollectionView *)collectionView
38+
{
39+
self = [super init];
40+
if (self) {
41+
_collectionView = collectionView;
42+
}
43+
return self;
44+
}
45+
46+
- (void)renderViewModel:(id<HUBViewModel>)viewModel
47+
usingBatchUpdates:(BOOL)usingBatchUpdates
48+
animated:(BOOL)animated
49+
completion:(void (^)())completionBlock
50+
{
51+
HUBViewModelDiff *diff;
52+
if (self.lastRenderedViewModel != nil) {
53+
id<HUBViewModel> nonnullViewModel = self.lastRenderedViewModel;
54+
diff = [HUBViewModelDiff diffFromViewModel:nonnullViewModel toViewModel:viewModel];
55+
}
56+
57+
HUBCollectionViewLayout * const layout = (HUBCollectionViewLayout *)self.collectionView.collectionViewLayout;
58+
59+
if (!usingBatchUpdates || diff == nil) {
60+
[self.collectionView reloadData];
61+
62+
[layout computeForCollectionViewSize:self.collectionView.frame.size viewModel:viewModel diff:diff];
63+
64+
/* Below is a workaround for an issue caused by UICollectionView not asking for numberOfItemsInSection
65+
before viewDidAppear is called or instantly after a call to reloadData. If reloadData is called
66+
after viewDidAppear has been called, followed by a call to performBatchUpdates, UICollectionView will
67+
ask for the initial number of items right before the batch updates, and for the new count while inside
68+
the update block. This will often trigger an assertion if there are any insertions / deletions, as
69+
the data model has already changed before the update. Forcing a layoutSubviews however, manually
70+
triggers the numberOfItems call.
71+
*/
72+
if (usingBatchUpdates && diff == nil) {
73+
[self.collectionView setNeedsLayout];
74+
[self.collectionView layoutIfNeeded];
75+
}
76+
completionBlock();
77+
} else {
78+
void (^updateBlock)() = ^{
79+
[self.collectionView performBatchUpdates:^{
80+
[self.collectionView insertItemsAtIndexPaths:diff.insertedBodyComponentIndexPaths];
81+
[self.collectionView deleteItemsAtIndexPaths:diff.deletedBodyComponentIndexPaths];
82+
[self.collectionView reloadItemsAtIndexPaths:diff.reloadedBodyComponentIndexPaths];
83+
84+
[layout computeForCollectionViewSize:self.collectionView.frame.size viewModel:viewModel diff:diff];
85+
} completion:^(BOOL finished) {
86+
completionBlock();
87+
}];
88+
};
89+
90+
if (animated) {
91+
updateBlock();
92+
} else {
93+
[UIView performWithoutAnimation:updateBlock];
94+
}
95+
}
96+
97+
self.lastRenderedViewModel = viewModel;
98+
}
99+
100+
@end
101+
102+
NS_ASSUME_NONNULL_END

0 commit comments

Comments
 (0)