Skip to content

Commit

Permalink
[ASRangeController] Fix stability of "minimum" rangeMode if the app h…
Browse files Browse the repository at this point in the history
…as more than one layout before scrolling. (#790)

This should result in memory savings in many apps, since errant relayouts are pretty common.
  • Loading branch information
appleguy committed Feb 9, 2018
1 parent b4a269a commit 31227da
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 16 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.
- **Important** ASDisplayNode's cornerRadius is a new thread-safe bridged property that should be preferred over CALayer's. Use the latter at your own risk! [Huy Nguyen](https://github.com/nguyenhuy) [#749](https://github.com/TextureGroup/Texture/pull/749).
- [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)
Expand Down
7 changes: 4 additions & 3 deletions Source/ASCollectionView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1549,13 +1549,10 @@ - (void)collectionView:(UICollectionView *)collectionView moveItemAtIndexPath:(N

- (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 updateCurrentRangeWithMode:ASLayoutRangeModeFull];
[self _checkForBatchFetching];
}

for (_ASCollectionViewCell *cell in _cellsForVisibilityUpdates) {
// _cellsForVisibilityUpdates only includes cells for ASCellNode subclasses with overrides of the visibility method.
[cell cellNodeVisibilityEvent:ASCellNodeVisibilityEventVisibleRectChanged inScrollView:scrollView];
Expand Down Expand Up @@ -1594,6 +1591,10 @@ - (void)scrollViewDidEndDecelerating:(UIScrollView *)scrollView

- (void)scrollViewWillBeginDragging:(UIScrollView *)scrollView
{
// If a scroll happens the current range mode needs to go to full
_rangeController.contentHasBeenScrolled = YES;
[_rangeController updateCurrentRangeWithMode:ASLayoutRangeModeFull];

for (_ASCollectionViewCell *cell in _cellsForVisibilityUpdates) {
[cell cellNodeVisibilityEvent:ASCellNodeVisibilityEventWillBeginDragging inScrollView:scrollView];
}
Expand Down
9 changes: 5 additions & 4 deletions Source/ASTableView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1214,13 +1214,10 @@ - (void)scrollViewDidScroll:(UIScrollView *)scrollView
[super scrollViewDidScroll:scrollView];
return;
}
// If a scroll happenes the current range mode needs to go to full
ASInterfaceState interfaceState = [self interfaceStateForRangeController:_rangeController];
if (ASInterfaceStateIncludesVisible(interfaceState)) {
[_rangeController updateCurrentRangeWithMode:ASLayoutRangeModeFull];
[self _checkForBatchFetching];
}

}
for (_ASTableViewCell *tableCell in _cellsForVisibilityUpdates) {
[[tableCell node] cellNodeVisibilityEvent:ASCellNodeVisibilityEventVisibleRectChanged
inScrollView:scrollView
Expand Down Expand Up @@ -1272,6 +1269,10 @@ - (void)scrollViewWillBeginDragging:(UIScrollView *)scrollView
[super scrollViewWillBeginDragging:scrollView];
return;
}
// If a scroll happens the current range mode needs to go to full
_rangeController.contentHasBeenScrolled = YES;
[_rangeController updateCurrentRangeWithMode:ASLayoutRangeModeFull];

for (_ASTableViewCell *tableViewCell in _cellsForVisibilityUpdates) {
[[tableViewCell node] cellNodeVisibilityEvent:ASCellNodeVisibilityEventWillBeginDragging
inScrollView:scrollView
Expand Down
5 changes: 5 additions & 0 deletions Source/Details/ASRangeController.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ AS_SUBCLASSING_RESTRICTED
*/
@property (nonatomic, weak) id<ASRangeControllerDelegate> delegate;

/**
* Property that indicates whether the scroll view for this range controller has ever changed its contentOffset.
*/
@property (nonatomic, assign) BOOL contentHasBeenScrolled;

@end


Expand Down
32 changes: 23 additions & 9 deletions Source/Details/ASRangeController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ @interface ASRangeController ()
NSSet<NSIndexPath *> *_allPreviousIndexPaths;
NSHashTable<ASCellNode *> *_visibleNodes;
ASLayoutRangeMode _currentRangeMode;
BOOL _contentHasBeenScrolled;
BOOL _preserveCurrentRangeMode;
BOOL _didRegisterForNodeDisplayNotifications;
CFTimeInterval _pendingDisplayNodesTimestamp;
Expand Down Expand Up @@ -77,6 +78,7 @@ - (instancetype)init

_rangeIsValid = YES;
_currentRangeMode = ASLayoutRangeModeUnspecified;
_contentHasBeenScrolled = NO;
_preserveCurrentRangeMode = NO;
_previousScrollDirection = ASScrollDirectionDown | ASScrollDirectionRight;

Expand Down Expand Up @@ -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.
Expand All @@ -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 && _contentHasBeenScrolled);

// 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];
}

Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 31227da

Please sign in to comment.