From 21cad903550101f9c8f928d4d34c7784dc47c259 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Mon, 5 Dec 2016 14:51:07 -0800 Subject: [PATCH 1/3] Ensure we always fill leading screens for batching --- AsyncDisplayKit/Private/ASBatchFetching.m | 8 ++- AsyncDisplayKitTests/ASCollectionViewTests.mm | 60 +++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/AsyncDisplayKit/Private/ASBatchFetching.m b/AsyncDisplayKit/Private/ASBatchFetching.m index 22883b9d4a..539a8a1f90 100644 --- a/AsyncDisplayKit/Private/ASBatchFetching.m +++ b/AsyncDisplayKit/Private/ASBatchFetching.m @@ -61,10 +61,14 @@ BOOL ASDisplayShouldFetchBatchForContext(ASBatchContext *context, return YES; } - BOOL isScrollingTowardEnd = (ASScrollDirectionContainsDown(scrollDirection) || ASScrollDirectionContainsRight(scrollDirection)); + // If they are scrolling toward the head of content, don't batch fetch. + BOOL isScrollingTowardHead = (ASScrollDirectionContainsUp(scrollDirection) || ASScrollDirectionContainsLeft(scrollDirection)); + if (isScrollingTowardHead) { + return NO; + } CGFloat triggerDistance = viewLength * leadingScreens; CGFloat remainingDistance = contentLength - viewLength - offset; - return isScrollingTowardEnd && remainingDistance <= triggerDistance; + return remainingDistance <= triggerDistance; } diff --git a/AsyncDisplayKitTests/ASCollectionViewTests.mm b/AsyncDisplayKitTests/ASCollectionViewTests.mm index 27ad94ec7d..646dbddd46 100644 --- a/AsyncDisplayKitTests/ASCollectionViewTests.mm +++ b/AsyncDisplayKitTests/ASCollectionViewTests.mm @@ -891,4 +891,64 @@ [self waitForExpectationsWithTimeout:3 handler:nil]; } +- (void)testThatWeBatchFetchUntilLeadingScreensRequirementIsMet_Animated +{ + [self _primitiveBatchFetchingFillTestAnimated:YES]; +} + +- (void)testThatWeBatchFetchUntilLeadingScreensRequirementIsMet_Nonanimated +{ + [self _primitiveBatchFetchingFillTestAnimated:NO]; +} + +- (void)_primitiveBatchFetchingFillTestAnimated:(BOOL)animated +{ + UIWindow *window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds]; + ASCollectionViewTestController *testController = [[ASCollectionViewTestController alloc] initWithNibName:nil bundle:nil]; + window.rootViewController = testController; + + // Start with 1 empty section + __block NSInteger itemCount = 0; + testController.asyncDelegate->_itemCounts = {itemCount}; + [window makeKeyAndVisible]; + [window layoutIfNeeded]; + + ASCollectionNode *cn = testController.collectionNode; + [cn waitUntilAllUpdatesAreCommitted]; + XCTAssertGreaterThan(cn.bounds.size.height, cn.view.contentSize.height, @"Expected initial data not to fill collection view area."); + + XCTestExpectation *expectation = [self expectationWithDescription:@"Completed all batch fetches"]; + __weak ASCollectionViewTestController *weakController = testController; + __block NSInteger batchFetchCount = 0; + testController.asyncDelegate.willBeginBatchFetch = ^(ASBatchContext *context) { + dispatch_async(dispatch_get_main_queue(), ^{ + NSInteger fetchIndex = batchFetchCount++; + + NSInteger itemCount = weakController.asyncDelegate->_itemCounts[0]; + weakController.asyncDelegate->_itemCounts[0] = (itemCount + 1); + if (animated) { + [cn insertItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:itemCount inSection:0] ]]; + } else { + [cn performBatchAnimated:NO updates:^{ + [cn insertItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:itemCount inSection:0] ]]; + } completion:nil]; + } + + [context completeBatchFetching:YES]; + + // If no more batch fetches have happened in 1 second, assume we're done. + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ + if (fetchIndex == batchFetchCount - 1) { + [expectation fulfill]; + } + }); + }); + }; + [self waitForExpectationsWithTimeout:60 handler:nil]; + CGFloat contentHeight = cn.view.contentSize.height; + CGFloat requiredContentHeight = CGRectGetMaxY(cn.bounds) + CGRectGetHeight(cn.bounds) * cn.view.leadingScreensForBatching; + XCTAssertGreaterThan(batchFetchCount, 2); + XCTAssertGreaterThanOrEqual(contentHeight, requiredContentHeight); +} + @end From 9bc58dc024e20a6d82916c6d0007e08790dca020 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Mon, 5 Dec 2016 15:00:39 -0800 Subject: [PATCH 2/3] When not visible, only batch fetch to fill visible area --- AsyncDisplayKit/ASCollectionView.mm | 6 +++++ AsyncDisplayKit/ASTableView.mm | 6 +++++ AsyncDisplayKit/Private/ASBatchFetching.h | 4 ++- AsyncDisplayKit/Private/ASBatchFetching.m | 15 ++++++++--- AsyncDisplayKitTests/ASBatchFetchingTests.m | 28 ++++++++++----------- 5 files changed, 40 insertions(+), 19 deletions(-) diff --git a/AsyncDisplayKit/ASCollectionView.mm b/AsyncDisplayKit/ASCollectionView.mm index 5d31f49a4c..642a9fb825 100644 --- a/AsyncDisplayKit/ASCollectionView.mm +++ b/AsyncDisplayKit/ASCollectionView.mm @@ -1708,6 +1708,12 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell"; [_rangeController setNeedsUpdate]; [_rangeController updateIfNeeded]; } + + // When we aren't visible, we will only fetch up to the visible area. Now that we are visible, + // we will fetch visible area + leading screens, so we need to check. + if (visible) { + [self _checkForBatchFetching]; + } } #pragma mark ASCALayerExtendedDelegate diff --git a/AsyncDisplayKit/ASTableView.mm b/AsyncDisplayKit/ASTableView.mm index 1f0ec1d5c2..44da18d057 100644 --- a/AsyncDisplayKit/ASTableView.mm +++ b/AsyncDisplayKit/ASTableView.mm @@ -1725,6 +1725,12 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell"; [_rangeController setNeedsUpdate]; [_rangeController updateIfNeeded]; } + + // When we aren't visible, we will only fetch up to the visible area. Now that we are visible, + // we will fetch visible area + leading screens, so we need to check. + if (visible) { + [self _checkForBatchFetching]; + } } @end diff --git a/AsyncDisplayKit/Private/ASBatchFetching.h b/AsyncDisplayKit/Private/ASBatchFetching.h index a09fb898d3..7effc6bcb7 100644 --- a/AsyncDisplayKit/Private/ASBatchFetching.h +++ b/AsyncDisplayKit/Private/ASBatchFetching.h @@ -45,6 +45,7 @@ BOOL ASDisplayShouldFetchBatchForScrollView(UIScrollView Date: Mon, 5 Dec 2016 15:37:49 -0800 Subject: [PATCH 3/3] Beef up the tests --- AsyncDisplayKitTests/ASCollectionViewTests.mm | 64 ++++++++++++++----- 1 file changed, 47 insertions(+), 17 deletions(-) diff --git a/AsyncDisplayKitTests/ASCollectionViewTests.mm b/AsyncDisplayKitTests/ASCollectionViewTests.mm index 646dbddd46..76b896b774 100644 --- a/AsyncDisplayKitTests/ASCollectionViewTests.mm +++ b/AsyncDisplayKitTests/ASCollectionViewTests.mm @@ -891,31 +891,49 @@ [self waitForExpectationsWithTimeout:3 handler:nil]; } -- (void)testThatWeBatchFetchUntilLeadingScreensRequirementIsMet_Animated +- (void)testThatWeBatchFetchUntilContentRequirementIsMet_Animated { - [self _primitiveBatchFetchingFillTestAnimated:YES]; + [self _primitiveBatchFetchingFillTestAnimated:YES visible:YES controller:nil]; } -- (void)testThatWeBatchFetchUntilLeadingScreensRequirementIsMet_Nonanimated +- (void)testThatWeBatchFetchUntilContentRequirementIsMet_Nonanimated { - [self _primitiveBatchFetchingFillTestAnimated:NO]; + [self _primitiveBatchFetchingFillTestAnimated:NO visible:YES controller:nil]; } -- (void)_primitiveBatchFetchingFillTestAnimated:(BOOL)animated +- (void)testThatWeBatchFetchUntilContentRequirementIsMet_Invisible { - UIWindow *window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds]; - ASCollectionViewTestController *testController = [[ASCollectionViewTestController alloc] initWithNibName:nil bundle:nil]; - window.rootViewController = testController; + [self _primitiveBatchFetchingFillTestAnimated:NO visible:NO controller:nil]; +} +- (void)testThatWhenWeBecomeVisibleWeWillFetchAdditionalContent +{ + ASCollectionViewTestController *ctrl = [[ASCollectionViewTestController alloc] initWithNibName:nil bundle:nil]; // Start with 1 empty section - __block NSInteger itemCount = 0; - testController.asyncDelegate->_itemCounts = {itemCount}; - [window makeKeyAndVisible]; - [window layoutIfNeeded]; + ctrl.asyncDelegate->_itemCounts = {0}; + [self _primitiveBatchFetchingFillTestAnimated:NO visible:NO controller:ctrl]; + XCTAssertGreaterThan([ctrl.collectionNode numberOfItemsInSection:0], 0); + [self _primitiveBatchFetchingFillTestAnimated:NO visible:YES controller:ctrl]; +} +- (void)_primitiveBatchFetchingFillTestAnimated:(BOOL)animated visible:(BOOL)visible controller:(nullable ASCollectionViewTestController *)testController +{ + if (testController == nil) { + testController = [[ASCollectionViewTestController alloc] initWithNibName:nil bundle:nil]; + // Start with 1 empty section + testController.asyncDelegate->_itemCounts = {0}; + } ASCollectionNode *cn = testController.collectionNode; - [cn waitUntilAllUpdatesAreCommitted]; - XCTAssertGreaterThan(cn.bounds.size.height, cn.view.contentSize.height, @"Expected initial data not to fill collection view area."); + + UIWindow *window = nil; + UIView *view = nil; + if (visible) { + window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds]; + view = window; + } else { + view = cn.view; + view.frame = [UIScreen mainScreen].bounds; + } XCTestExpectation *expectation = [self expectationWithDescription:@"Completed all batch fetches"]; __weak ASCollectionViewTestController *weakController = testController; @@ -923,7 +941,7 @@ testController.asyncDelegate.willBeginBatchFetch = ^(ASBatchContext *context) { dispatch_async(dispatch_get_main_queue(), ^{ NSInteger fetchIndex = batchFetchCount++; - + NSInteger itemCount = weakController.asyncDelegate->_itemCounts[0]; weakController.asyncDelegate->_itemCounts[0] = (itemCount + 1); if (animated) { @@ -944,11 +962,23 @@ }); }); }; + window.rootViewController = testController; + + [window makeKeyAndVisible]; + [view layoutIfNeeded]; + [self waitForExpectationsWithTimeout:60 handler:nil]; CGFloat contentHeight = cn.view.contentSize.height; - CGFloat requiredContentHeight = CGRectGetMaxY(cn.bounds) + CGRectGetHeight(cn.bounds) * cn.view.leadingScreensForBatching; + CGFloat requiredContentHeight; + CGFloat itemHeight = [cn.view layoutAttributesForItemAtIndexPath:[NSIndexPath indexPathForItem:0 inSection:0]].size.height; + if (visible) { + requiredContentHeight = CGRectGetMaxY(cn.bounds) + CGRectGetHeight(cn.bounds) * cn.view.leadingScreensForBatching; + } else { + requiredContentHeight = CGRectGetMaxY(cn.bounds); + } XCTAssertGreaterThan(batchFetchCount, 2); - XCTAssertGreaterThanOrEqual(contentHeight, requiredContentHeight); + XCTAssertGreaterThanOrEqual(contentHeight, requiredContentHeight, @"Loaded too little content."); + XCTAssertLessThanOrEqual(contentHeight, requiredContentHeight + 2 * itemHeight, @"Loaded too much content."); } @end