[ASCollectionView] Fix index space translation of Flow Layout Delegate methods. (#467)

* [ASCollectionView] Fix index space translation of Flow Layout Delegate methods. This includes a few other cleanups, including overflow of signed integer indices. * [ASCollectionView] Improve code sharing of UIKit size method calls; ensure delegate invalidation re-fetches supplementary sizes too. * [ASCollectionView] Final method ordering and doc-comment for new _sizeForUIKitCellWithKind:atIndexPath: method.
2025-12-22 14:20:20 +00:00 · 2017-10-09 09:19:46 -07:00
parent 550da242b7
commit c6e3dd7a5d
8 changed files with 137 additions and 49 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,7 @@
 ## master

 * Add your own contributions to the next release on the line below this with your name.
+- [ASCollectionView] Improve index space translation of Flow Layout Delegate methods. [Scott Goodson](https://github.com/appleguy)
 - [ASVideoNode] Fix unreleased time observer. [Flo Vouin](https://github.com/flovouin)
 - [PINCache] Set a default .byteLimit to reduce disk usage and startup time. [#595](https://github.com/TextureGroup/Texture/pull/595) [Scott Goodson](https://github.com/appleguy)
 - [ASNetworkImageNode] Fix deadlock in GIF handling. [#582](https://github.com/TextureGroup/Texture/pull/582) [Garrett Moon](https://github.com/garrettmoon)
--- a/Source/ASCollectionNode+Beta.h
+++ b/Source/ASCollectionNode+Beta.h
@@ -67,6 +67,8 @@ NS_ASSUME_NONNULL_BEGIN

 - (void)endUpdatesAnimated:(BOOL)animated completion:(nullable void (^)(BOOL))completion ASDISPLAYNODE_DEPRECATED_MSG("Use -performBatchUpdates:completion: instead.");

+- (void)invalidateFlowLayoutDelegateMetrics;
+
@end

 NS_ASSUME_NONNULL_END
--- a/Source/ASCollectionNode.h
+++ b/Source/ASCollectionNode.h
@@ -876,6 +876,10 @@ NS_ASSUME_NONNULL_BEGIN

 - (void)collectionView:(UICollectionView *)collectionView didEndDisplayingCell:(UICollectionViewCell *)cell forItemAtIndexPath:(NSIndexPath *)indexPath;

+- (void)collectionView:(UICollectionView *)collectionView willDisplaySupplementaryView:(UICollectionReusableView *)view forElementKind:(NSString *)elementKind atIndexPath:(NSIndexPath *)indexPath;
+
+- (void)collectionView:(UICollectionView *)collectionView didEndDisplayingSupplementaryView:(UICollectionReusableView *)view forElementOfKind:(NSString *)elementKind atIndexPath:(NSIndexPath *)indexPath;
+
@end

 NS_ASSUME_NONNULL_END
--- a/Source/ASCollectionNode.mm
+++ b/Source/ASCollectionNode.mm
@@ -805,6 +805,13 @@
  }
 }

+- (void)invalidateFlowLayoutDelegateMetrics {
+  ASDisplayNodeAssertMainThread();
+  if (self.nodeLoaded) {
+    [self.view invalidateFlowLayoutDelegateMetrics];
+  }
+}
+
 - (void)insertSections:(NSIndexSet *)sections
 {
  ASDisplayNodeAssertMainThread();
--- a/Source/ASCollectionView.mm
+++ b/Source/ASCollectionView.mm
@@ -29,6 +29,7 @@
 #import <AsyncDisplayKit/ASDataController.h>
 #import <AsyncDisplayKit/ASDisplayNodeExtras.h>
 #import <AsyncDisplayKit/ASDisplayNode+FrameworkPrivate.h>
+#import <AsyncDisplayKit/ASDisplayNode+Subclasses.h>
 #import <AsyncDisplayKit/ASElementMap.h>
 #import <AsyncDisplayKit/ASInternalHelpers.h>
 #import <AsyncDisplayKit/UICollectionViewLayout+ASConvenience.h>
@@ -63,6 +64,14 @@
    return __val; \
  }

+#define ASIndexPathForSection(section) [NSIndexPath indexPathForItem:0 inSection:section]
+
+#define ASFlowLayoutDefault(layout, property, default)                                        \
+({                                                                                            \
+  UICollectionViewFlowLayout *flowLayout = ASDynamicCast(layout, UICollectionViewFlowLayout); \
+  flowLayout ? flowLayout.property : default;                                                 \
+})
+
 /// What, if any, invalidation should we perform during the next -layoutSubviews.
 typedef NS_ENUM(NSUInteger, ASCollectionViewInvalidationStyle) {
  /// Perform no invalidation.
@@ -192,6 +201,8 @@ static NSString * const kReuseIdentifier = @"_ASCollectionReuseIdentifier";
    unsigned int interop:1;
    unsigned int interopWillDisplayCell:1;
    unsigned int interopDidEndDisplayingCell:1;
+    unsigned int interopWillDisplaySupplementaryView:1;
+    unsigned int interopdidEndDisplayingSupplementaryView:1;
  } _asyncDelegateFlags;
  
  struct {
@@ -532,6 +543,8 @@ static NSString * const kReuseIdentifier = @"_ASCollectionReuseIdentifier";
      id<ASCollectionDelegateInterop> interopDelegate = (id<ASCollectionDelegateInterop>)_asyncDelegate;
      _asyncDelegateFlags.interopWillDisplayCell = [interopDelegate respondsToSelector:@selector(collectionView:willDisplayCell:forItemAtIndexPath:)];
      _asyncDelegateFlags.interopDidEndDisplayingCell = [interopDelegate respondsToSelector:@selector(collectionView:didEndDisplayingCell:forItemAtIndexPath:)];
+      _asyncDelegateFlags.interopWillDisplaySupplementaryView = [interopDelegate respondsToSelector:@selector(collectionView:willDisplaySupplementaryView:forElementKind:atIndexPath:)];
+      _asyncDelegateFlags.interopdidEndDisplayingSupplementaryView = [interopDelegate respondsToSelector:@selector(collectionView:didEndDisplayingSupplementaryView:forElementOfKind:atIndexPath:)];
    }
  }

@@ -771,6 +784,25 @@ static NSString * const kReuseIdentifier = @"_ASCollectionReuseIdentifier";
  self.dataController.usesSynchronousDataLoading = usesSynchronousDataLoading;
 }

+- (void)invalidateFlowLayoutDelegateMetrics {
+  for (ASCollectionElement *element in self.dataController.pendingMap) {
+    // This may be either a Supplementary or Item type element.
+    // For UIKit passthrough cells of either type, re-fetch their sizes from the standard UIKit delegate methods.
+    ASCellNode *node = element.node;
+    if (node.shouldUseUIKitCell) {
+      NSIndexPath *indexPath = [self indexPathForNode:node];
+      NSString *kind = [element supplementaryElementKind];
+      CGSize previousSize = node.style.preferredSize;
+      CGSize size = [self _sizeForUIKitCellWithKind:kind atIndexPath:indexPath];
+
+      if (!CGSizeEqualToSize(previousSize, size)) {
+        node.style.preferredSize = size;
+        [node invalidateCalculatedLayout];
+      }
+    }
+  }
+}
+
 #pragma mark Internal

 - (void)_configureCollectionViewLayout:(nonnull UICollectionViewLayout *)layout
@@ -781,6 +813,46 @@ static NSString * const kReuseIdentifier = @"_ASCollectionReuseIdentifier";
  }
 }

+/**
+ This method is called only for UIKit Passthrough cells - either regular Items or Supplementary elements.
+ It checks if the delegate implements the UICollectionViewFlowLayout methods that provide sizes, and if not,
+ uses the default values set on the flow layout. If a flow layout is not in use, UICollectionView Passthrough
+ cells must be sized by logic in the Layout object, and Texture does not participate in these paths.
+*/
+- (CGSize)_sizeForUIKitCellWithKind:(NSString *)kind atIndexPath:(NSIndexPath *)indexPath
+{
+  CGSize size = CGSizeZero;
+  UICollectionViewLayout *l = self.collectionViewLayout;
+
+  if (kind == nil) {
+    ASDisplayNodeAssert(_asyncDataSourceFlags.interop, @"This code should not be called except for UIKit passthrough compatibility");
+    SEL sizeForItem = @selector(collectionView:layout:sizeForItemAtIndexPath:);
+    if ([_asyncDelegate respondsToSelector:sizeForItem]) {
+      size = [(id)_asyncDelegate collectionView:self layout:l sizeForItemAtIndexPath:indexPath];
+    } else {
+      size = ASFlowLayoutDefault(l, itemSize, CGSizeZero);
+    }
+  } else if ([kind isEqualToString:UICollectionElementKindSectionHeader]) {
+    ASDisplayNodeAssert(_asyncDataSourceFlags.interopViewForSupplementaryElement, @"This code should not be called except for UIKit passthrough compatibility");
+    SEL sizeForHeader = @selector(collectionView:layout:referenceSizeForHeaderInSection:);
+    if ([_asyncDelegate respondsToSelector:sizeForHeader]) {
+      size = [(id)_asyncDelegate collectionView:self layout:l referenceSizeForHeaderInSection:indexPath.section];
+    } else {
+      size = ASFlowLayoutDefault(l, headerReferenceSize, CGSizeZero);
+    }
+  } else if ([kind isEqualToString:UICollectionElementKindSectionFooter]) {
+    ASDisplayNodeAssert(_asyncDataSourceFlags.interopViewForSupplementaryElement, @"This code should not be called except for UIKit passthrough compatibility");
+    SEL sizeForFooter = @selector(collectionView:layout:referenceSizeForFooterInSection:);
+    if ([_asyncDelegate respondsToSelector:sizeForFooter]) {
+      size = [(id)_asyncDelegate collectionView:self layout:l referenceSizeForFooterInSection:indexPath.section];
+    } else {
+      size = ASFlowLayoutDefault(l, footerReferenceSize, CGSizeZero);
+    }
+  }
+
+  return size;
+}
+
 /**
 Performing nested batch updates with super (e.g. resizing a cell node & updating collection view during same frame)
 can cause super to throw data integrity exceptions because it checks the data source counts before
@@ -961,13 +1033,6 @@ static NSString * const kReuseIdentifier = @"_ASCollectionReuseIdentifier";
  return [_dataController.visibleMap numberOfItemsInSection:section];
 }

-#define ASIndexPathForSection(section) [NSIndexPath indexPathForItem:0 inSection:section]
-#define ASFlowLayoutDefault(layout, property, default)                                        \
-({                                                                                            \
-  UICollectionViewFlowLayout *flowLayout = ASDynamicCast(layout, UICollectionViewFlowLayout); \
-  flowLayout ? flowLayout.property : default;                                                 \
-})
-
 - (CGSize)collectionView:(UICollectionView *)collectionView layout:(UICollectionViewLayout *)layout
                                            sizeForItemAtIndexPath:(NSIndexPath *)indexPath
 {
@@ -1097,7 +1162,11 @@ minimumLineSpacingForSectionAtIndex:(NSInteger)section
 - (void)collectionView:(UICollectionView *)collectionView willDisplayCell:(UICollectionViewCell *)rawCell forItemAtIndexPath:(NSIndexPath *)indexPath
 {
  if (_asyncDelegateFlags.interopWillDisplayCell) {
-    [(id <ASCollectionDelegateInterop>)_asyncDelegate collectionView:collectionView willDisplayCell:rawCell forItemAtIndexPath:indexPath];
+    ASCellNode *node = [self nodeForItemAtIndexPath:indexPath];
+    NSIndexPath *modelIndexPath = [self indexPathForNode:node];
+    if (modelIndexPath && node.shouldUseUIKitCell) {
+      [(id <ASCollectionDelegateInterop>)_asyncDelegate collectionView:collectionView willDisplayCell:rawCell forItemAtIndexPath:modelIndexPath];
+    }
  }

  _ASCollectionViewCell *cell = ASDynamicCastStrict(rawCell, _ASCollectionViewCell);
@@ -1154,7 +1223,11 @@ minimumLineSpacingForSectionAtIndex:(NSInteger)section
 - (void)collectionView:(UICollectionView *)collectionView didEndDisplayingCell:(UICollectionViewCell *)rawCell forItemAtIndexPath:(NSIndexPath *)indexPath
 {
  if (_asyncDelegateFlags.interopDidEndDisplayingCell) {
-    [(id <ASCollectionDelegateInterop>)_asyncDelegate collectionView:collectionView didEndDisplayingCell:rawCell forItemAtIndexPath:indexPath];
+    ASCellNode *node = [self nodeForItemAtIndexPath:indexPath];
+    NSIndexPath *modelIndexPath = [self indexPathForNode:node];
+    if (modelIndexPath && node.shouldUseUIKitCell) {
+      [(id <ASCollectionDelegateInterop>)_asyncDelegate collectionView:collectionView didEndDisplayingCell:rawCell forItemAtIndexPath:modelIndexPath];
+    }
  }

  _ASCollectionViewCell *cell = ASDynamicCastStrict(rawCell, _ASCollectionViewCell);
@@ -1195,6 +1268,14 @@ minimumLineSpacingForSectionAtIndex:(NSInteger)section

 - (void)collectionView:(UICollectionView *)collectionView willDisplaySupplementaryView:(UICollectionReusableView *)rawView forElementKind:(NSString *)elementKind atIndexPath:(NSIndexPath *)indexPath
 {
+  if (_asyncDelegateFlags.interopWillDisplaySupplementaryView) {
+    ASCellNode *node = [self nodeForItemAtIndexPath:indexPath];
+    NSIndexPath *modelIndexPath = [self indexPathForNode:node];
+    if (modelIndexPath && node.shouldUseUIKitCell) {
+      [(id <ASCollectionDelegateInterop>)_asyncDelegate collectionView:collectionView willDisplaySupplementaryView:rawView forElementKind:elementKind atIndexPath:modelIndexPath];
+    }
+  }
+
  _ASCollectionReusableView *view = ASDynamicCastStrict(rawView, _ASCollectionReusableView);
  if (view == nil) {
    return;
@@ -1228,6 +1309,14 @@ minimumLineSpacingForSectionAtIndex:(NSInteger)section

 - (void)collectionView:(UICollectionView *)collectionView didEndDisplayingSupplementaryView:(UICollectionReusableView *)rawView forElementOfKind:(NSString *)elementKind atIndexPath:(NSIndexPath *)indexPath
 {
+  if (_asyncDelegateFlags.interopdidEndDisplayingSupplementaryView) {
+    ASCellNode *node = [self nodeForItemAtIndexPath:indexPath];
+    NSIndexPath *modelIndexPath = [self indexPathForNode:node];
+    if (modelIndexPath && node.shouldUseUIKitCell) {
+      [(id <ASCollectionDelegateInterop>)_asyncDelegate collectionView:collectionView didEndDisplayingSupplementaryView:rawView forElementOfKind:elementKind atIndexPath:modelIndexPath];
+    }
+  }
+
  _ASCollectionReusableView *view = ASDynamicCastStrict(rawView, _ASCollectionReusableView);
  if (view == nil) {
    return;
@@ -1427,7 +1516,7 @@ minimumLineSpacingForSectionAtIndex:(NSInteger)section
  }
  
  for (_ASCollectionViewCell *cell in _cellsForVisibilityUpdates) {
-    // Only nodes that respond to the selector are added to _cellsForVisibilityUpdates
+    // _cellsForVisibilityUpdates only includes cells for ASCellNode subclasses with overrides of the visibility method.
    [cell cellNodeVisibilityEvent:ASCellNodeVisibilityEventVisibleRectChanged inScrollView:scrollView];
  }
  if (_asyncDelegateFlags.scrollViewDidScroll) {
@@ -1680,6 +1769,7 @@ minimumLineSpacingForSectionAtIndex:(NSInteger)section

 - (ASCellNodeBlock)dataController:(ASDataController *)dataController nodeBlockAtIndexPath:(NSIndexPath *)indexPath
 {
+  ASDisplayNodeAssertMainThread();
  ASCellNodeBlock block = nil;
  ASCellNode *cell = nil;

@@ -1707,14 +1797,7 @@ minimumLineSpacingForSectionAtIndex:(NSInteger)section

  if (block == nil) {
    if (_asyncDataSourceFlags.interop) {
-      UICollectionViewLayout *layout = self.collectionViewLayout;
-      CGSize preferredSize = CGSizeZero;
-      SEL sizeForItem = @selector(collectionView:layout:sizeForItemAtIndexPath:);
-      if ([_asyncDelegate respondsToSelector:sizeForItem]) {
-        preferredSize = [(id)_asyncDelegate collectionView:self layout:layout sizeForItemAtIndexPath:indexPath];
-      } else {
-        preferredSize = ASFlowLayoutDefault(layout, itemSize, CGSizeZero);
-      }
+      CGSize preferredSize = [self _sizeForUIKitCellWithKind:nil atIndexPath:indexPath];
      block = ^{
        ASCellNode *node = [[ASCellNode alloc] init];
        node.shouldUseUIKitCell = YES;
@@ -1790,6 +1873,7 @@ minimumLineSpacingForSectionAtIndex:(NSInteger)section

 - (ASCellNodeBlock)dataController:(ASDataController *)dataController supplementaryNodeBlockOfKind:(NSString *)kind atIndexPath:(NSIndexPath *)indexPath
 {
+  ASDisplayNodeAssertMainThread();
  ASCellNodeBlock nodeBlock = nil;
  ASCellNode *node = nil;
  if (_asyncDataSourceFlags.collectionNodeNodeBlockForSupplementaryElement) {
@@ -1809,27 +1893,12 @@ minimumLineSpacingForSectionAtIndex:(NSInteger)section
    if (node) {
      nodeBlock = ^{ return node; };
    } else {
-      BOOL useUIKitCell = _asyncDataSourceFlags.interop;
+      // In this case, the app code returned nil for the node and the nodeBlock.
+      // If the UIKit method is implemented, then we should use it. Otherwise the CGSizeZero default will cause UIKit to not show it.
      CGSize preferredSize = CGSizeZero;
+      BOOL useUIKitCell = _asyncDataSourceFlags.interopViewForSupplementaryElement;
      if (useUIKitCell) {
-        UICollectionViewLayout *layout = self.collectionViewLayout;
-        if ([kind isEqualToString:UICollectionElementKindSectionHeader]) {
-          SEL sizeForHeader = @selector(collectionView:layout:referenceSizeForHeaderInSection:);
-          if ([_asyncDelegate respondsToSelector:sizeForHeader]) {
-            preferredSize = [(id)_asyncDelegate collectionView:self layout:layout
-                               referenceSizeForHeaderInSection:indexPath.section];
-          } else {
-            preferredSize = ASFlowLayoutDefault(layout, headerReferenceSize, CGSizeZero);
-          }
-        } else if ([kind isEqualToString:UICollectionElementKindSectionFooter]) {
-          SEL sizeForFooter = @selector(collectionView:layout:referenceSizeForFooterInSection:);
-          if ([_asyncDelegate respondsToSelector:sizeForFooter]) {
-            preferredSize = [(id)_asyncDelegate collectionView:self layout:layout
-                               referenceSizeForFooterInSection:indexPath.section];
-          } else {
-            preferredSize = ASFlowLayoutDefault(layout, footerReferenceSize, CGSizeZero);
-          }
-        }
+        preferredSize = [self _sizeForUIKitCellWithKind:kind atIndexPath:indexPath];
      }
      nodeBlock = ^{
        ASCellNode *node = [[ASCellNode alloc] init];
--- a/Source/Base/ASAssert.h
+++ b/Source/Base/ASAssert.h
@@ -64,8 +64,8 @@
 #define ASDisplayNodeConditionalAssert(shouldTestCondition, condition, desc, ...) ASDisplayNodeAssert((!(shouldTestCondition) || (condition)), desc, ##__VA_ARGS__)
 #define ASDisplayNodeConditionalCAssert(shouldTestCondition, condition, desc, ...) ASDisplayNodeCAssert((!(shouldTestCondition) || (condition)), desc, ##__VA_ARGS__)

-#define ASDisplayNodeCAssertPositiveReal(description, num) ASDisplayNodeCAssert(num >= 0 && num <= CGFLOAT_MAX, @"%@ must be a real positive integer.", description)
-#define ASDisplayNodeCAssertInfOrPositiveReal(description, num) ASDisplayNodeCAssert(isinf(num) || (num >= 0 && num <= CGFLOAT_MAX), @"%@ must be infinite or a real positive integer.", description)
+#define ASDisplayNodeCAssertPositiveReal(description, num) ASDisplayNodeCAssert(num >= 0 && num <= CGFLOAT_MAX, @"%@ must be a real positive integer: %f.", description, (CGFloat)num)
+#define ASDisplayNodeCAssertInfOrPositiveReal(description, num) ASDisplayNodeCAssert(isinf(num) || (num >= 0 && num <= CGFLOAT_MAX), @"%@ must be infinite or a real positive integer: %f.", description, (CGFloat)num)

 #define ASDisplayNodeErrorDomain @"ASDisplayNodeErrorDomain"
 #define ASDisplayNodeNonFatalErrorCode 1
--- a/Source/Details/ASDataController.mm
+++ b/Source/Details/ASDataController.mm
@@ -791,7 +791,7 @@ typedef dispatch_block_t ASDataControllerCompletionBlock;
      element.constrainedSize = newConstrainedSize;

      // Node may not be allocated yet (e.g node virtualization or same size optimization)
-      // Call context.nodeIfAllocated here to avoid immature node allocation and layout
+      // Call context.nodeIfAllocated here to avoid premature node allocation and layout
      ASCellNode *node = element.nodeIfAllocated;
      if (node) {
        [self _layoutNode:node withConstrainedSize:newConstrainedSize];
--- a/Source/Private/ASCollectionView+Undeprecated.h
+++ b/Source/Private/ASCollectionView+Undeprecated.h
@@ -296,6 +296,11 @@ NS_ASSUME_NONNULL_BEGIN
 */
 - (nullable NSIndexPath *)indexPathForNode:(ASCellNode *)cellNode AS_WARN_UNUSED_RESULT;

+/**
+ *  Invalidates and recalculates the cached sizes stored for pass-through cells used in interop mode.
+ */
+- (void)invalidateFlowLayoutDelegateMetrics;
+
 - (void)setContentOffset:(CGPoint)contentOffset animated:(BOOL)animated;

@end