Skip to content

Commit

Permalink
Fix re-entrance + caching when using IGListCollectionViewLayout
Browse files Browse the repository at this point in the history
Summary:
The issue I've been seeing is that list kit would not recalculate layout for when collection view bounds would change

IGListCollectionViewLayout has code to invalidate layout when collection view size chances however it would not happen without explicitly calling `….collectionViewLayout invalidateLayout];`

It appears that in the loop of calculating the layout we would accidentally call the code that would start caching existing layout data (not calculated at the point of call) and cache the value

This does not seem to happen all the time, however moving the cache purge to the end of layout calculation loop seems reasonable since at this point we have the new values that can't be known to the cache

As long as the consumer uses `IGListCollectionViewLayout` explicitly or implicitly through convenience initialize of `IGListCollectionView` this fix should work as is

Reviewed By: fabiomassimo

Differential Revision: D50197736

fbshipit-source-id: cb6d818f8be965f2e6ba494ffea083a6ab35682d
  • Loading branch information
Sash Zats authored and facebook-github-bot committed Oct 12, 2023
1 parent 3d1cad3 commit d220f8a
Showing 1 changed file with 9 additions and 5 deletions.
14 changes: 9 additions & 5 deletions Source/IGListKit/IGListCollectionViewLayout.mm
Original file line number Diff line number Diff line change
Expand Up @@ -474,11 +474,7 @@ - (NSString *)_classNameForDelegate:(id<UICollectionViewDelegateFlowLayout>)dele
- (void)_calculateLayoutIfNeeded {
if (_minimumInvalidatedSection == NSNotFound) {
return;
}

// purge attribute caches so they are rebuilt
[_attributesCache removeAllObjects];
[self _resetSupplementaryAttributesCache];
}

UICollectionView *collectionView = self.collectionView;
id<UICollectionViewDelegateFlowLayout> delegate = (id<UICollectionViewDelegateFlowLayout>)collectionView.delegate;
Expand Down Expand Up @@ -540,6 +536,8 @@ - (void)_calculateLayoutIfNeeded {

for (NSInteger item = 0; item < itemCount; item++) {
NSIndexPath *indexPath = [NSIndexPath indexPathForItem:item inSection:section];
// Following method subsequentally calls -layoutAttributesForItemAtIndexPath: and caches attributes that are not ready yet (we only calculate them at the end of this for loop)
// This results in _attributesCache[indexPath] being set to incorrect value. If we end up calling prepareLayout in response to frame change we
const CGSize size = [delegate collectionView:collectionView layout:self sizeForItemAtIndexPath:indexPath];

IGAssert(CGSizeGetLengthInDirection(size, fixedDirection) <= paddedLengthInFixedDirection
Expand Down Expand Up @@ -656,6 +654,12 @@ - (void)_calculateLayoutIfNeeded {
_sectionData[section].lastNextRowCoordInScrollDirection = nextRowCoordInScrollDirection;
}

// Reason we are purging attributes at the end is because in some circumstances calling
// -[delegate collectionView: layout: sizeForItemAtIndexPath:] results in creating the cache with incorrect values
// See the comment next to the call for more information
[_attributesCache removeAllObjects];
[self _resetSupplementaryAttributesCache];

_minimumInvalidatedSection = NSNotFound;
}

Expand Down

0 comments on commit d220f8a

Please sign in to comment.