From c7b68c2ddeb360c2eb9511280ba5e48c0f29f961 Mon Sep 17 00:00:00 2001 From: Scott Goodson Date: Mon, 5 Feb 2018 12:14:15 -0800 Subject: [PATCH] [ASRangeController] Fix stability of "minimum" rangeMode if the app has more than one layout before scrolling. This should result in memory savings in many apps, since errant relayouts are pretty common. --- CHANGELOG.md | 1 + Source/ASCollectionView.mm | 1 + Source/ASTableView.mm | 1 + Source/Details/ASRangeController.h | 5 +++++ Source/Details/ASRangeController.mm | 32 +++++++++++++++++++++-------- 5 files changed, 31 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dc967bc1..f62db9a4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## master * Add your own contributions to the next release on the line below this with your name. +- [ASRangeController] Fix stability of "minimum" rangeMode if the app has more than one layout before scrolling. - [ASCellNode] Adds mapping for UITableViewCell focusStyle [Alex Hill](https://github.com/alexhillc) [#727](https://github.com/TextureGroup/Texture/pull/727) - [ASNetworkImageNode] Fix capturing self in the block while loading image in ASNetworkImageNode. [Denis Mororozov](https://github.com/morozkin) [#777](https://github.com/TextureGroup/Texture/pull/777) - [ASTraitCollection] Add new properties of UITraitCollection to ASTraitCollection. [Yevgen Pogribnyi](https://github.com/ypogribnyi) diff --git a/Source/ASCollectionView.mm b/Source/ASCollectionView.mm index 1b06d6bef..99e32c26e 100644 --- a/Source/ASCollectionView.mm +++ b/Source/ASCollectionView.mm @@ -1552,6 +1552,7 @@ - (void)scrollViewDidScroll:(UIScrollView *)scrollView // If a scroll happenes the current range mode needs to go to full ASInterfaceState interfaceState = [self interfaceStateForRangeController:_rangeController]; if (ASInterfaceStateIncludesVisible(interfaceState)) { + _rangeController.contentOffsetHasChanged = YES; [_rangeController updateCurrentRangeWithMode:ASLayoutRangeModeFull]; [self _checkForBatchFetching]; } diff --git a/Source/ASTableView.mm b/Source/ASTableView.mm index acfe23906..0fea19e20 100644 --- a/Source/ASTableView.mm +++ b/Source/ASTableView.mm @@ -1217,6 +1217,7 @@ - (void)scrollViewDidScroll:(UIScrollView *)scrollView // If a scroll happenes the current range mode needs to go to full ASInterfaceState interfaceState = [self interfaceStateForRangeController:_rangeController]; if (ASInterfaceStateIncludesVisible(interfaceState)) { + _rangeController.contentOffsetHasChanged = YES; [_rangeController updateCurrentRangeWithMode:ASLayoutRangeModeFull]; [self _checkForBatchFetching]; } diff --git a/Source/Details/ASRangeController.h b/Source/Details/ASRangeController.h index 46a5cbf69..152c4e8b5 100644 --- a/Source/Details/ASRangeController.h +++ b/Source/Details/ASRangeController.h @@ -100,6 +100,11 @@ AS_SUBCLASSING_RESTRICTED */ @property (nonatomic, weak) id delegate; +/** + * Property that indicates whether the scroll view for this range controller has ever changed its contentOffset. + */ +@property (nonatomic, assign) BOOL contentOffsetHasChanged; + @end diff --git a/Source/Details/ASRangeController.mm b/Source/Details/ASRangeController.mm index 818fb3a1d..025be91fa 100644 --- a/Source/Details/ASRangeController.mm +++ b/Source/Details/ASRangeController.mm @@ -45,6 +45,7 @@ @interface ASRangeController () NSSet *_allPreviousIndexPaths; NSHashTable *_visibleNodes; ASLayoutRangeMode _currentRangeMode; + BOOL _contentOffsetHasChanged; BOOL _preserveCurrentRangeMode; BOOL _didRegisterForNodeDisplayNotifications; CFTimeInterval _pendingDisplayNodesTimestamp; @@ -77,6 +78,7 @@ - (instancetype)init _rangeIsValid = YES; _currentRangeMode = ASLayoutRangeModeUnspecified; + _contentOffsetHasChanged = NO; _preserveCurrentRangeMode = NO; _previousScrollDirection = ASScrollDirectionDown | ASScrollDirectionRight; @@ -222,10 +224,6 @@ - (void)_updateVisibleNodeIndexPaths auto visibleElements = [_dataSource visibleElementsForRangeController:self]; NSHashTable *newVisibleNodes = [NSHashTable hashTableWithOptions:NSHashTableObjectPointerPersonality]; - if (visibleElements.count == 0) { // if we don't have any visibleNodes currently (scrolled before or after content)... - [self _setVisibleNodes:newVisibleNodes]; - return; // don't do anything for this update, but leave _rangeIsValid == NO to make sure we update it later - } ASSignpostStart(ASSignpostRangeControllerUpdate); // Get the scroll direction. Default to using the previous one, if they're not scrolling. @@ -234,12 +232,28 @@ - (void)_updateVisibleNodeIndexPaths scrollDirection = _previousScrollDirection; } _previousScrollDirection = scrollDirection; - + + if (visibleElements.count == 0) { // if we don't have any visibleNodes currently (scrolled before or after content)... + // Verify the actual state by checking the layout with a "VisibleOnly" range. + // This allows us to avoid thrashing through -didExitVisibleState in the case of -reloadData, since that generates didEndDisplayingCell calls. + // Those didEndDisplayingCell calls result in items being removed from the visibleElements returned by the _dataSource, even though the layout remains correct. + visibleElements = [_layoutController elementsForScrolling:scrollDirection rangeMode:ASLayoutRangeModeVisibleOnly rangeType:ASLayoutRangeTypeDisplay map:map]; + for (ASCollectionElement *element in visibleElements) { + [newVisibleNodes addObject:element.node]; + } + [self _setVisibleNodes:newVisibleNodes]; + return; // don't do anything for this update, but leave _rangeIsValid == NO to make sure we update it later + } + ASInterfaceState selfInterfaceState = [self interfaceState]; ASLayoutRangeMode rangeMode = _currentRangeMode; - // If the range mode is explicitly set via updateCurrentRangeWithMode: it will last in that mode until the - // range controller becomes visible again or explicitly changes the range mode again - if ((!_preserveCurrentRangeMode && ASInterfaceStateIncludesVisible(selfInterfaceState)) || [[self class] isFirstRangeUpdateForRangeMode:rangeMode]) { + BOOL updateRangeMode = (!_preserveCurrentRangeMode && _contentOffsetHasChanged); + + // If we've never scrolled before, we never update the range mode, so it doesn't jump into Full too early. + // This can happen if we have multiple, noisy updates occurring from application code before the user has engaged. + // If the range mode is explicitly set via updateCurrentRangeWithMode:, we'll preserve that for at least one update cycle. + // Once the user has scrolled and the range is visible, we'll always resume managing the range mode automatically. + if ((updateRangeMode && ASInterfaceStateIncludesVisible(selfInterfaceState)) || [[self class] isFirstRangeUpdateForRangeMode:rangeMode]) { rangeMode = [ASRangeController rangeModeForInterfaceState:selfInterfaceState currentRangeMode:_currentRangeMode]; } @@ -412,7 +426,7 @@ - (void)_updateVisibleNodeIndexPaths // NSLog(@"custom: %@", visibleNodePathsSet); // } [modifiedIndexPaths sortUsingSelector:@selector(compare:)]; - NSLog(@"Range update complete; modifiedIndexPaths: %@", [self descriptionWithIndexPaths:modifiedIndexPaths]); + NSLog(@"Range update complete; modifiedIndexPaths: %@, rangeMode: %d", [self descriptionWithIndexPaths:modifiedIndexPaths], rangeMode); #endif ASSignpostEnd(ASSignpostRangeControllerUpdate);