diff --git a/AsyncDisplayKit/ASCollectionView.mm b/AsyncDisplayKit/ASCollectionView.mm index 9e0dac3ca3..a4d02aca3a 100644 --- a/AsyncDisplayKit/ASCollectionView.mm +++ b/AsyncDisplayKit/ASCollectionView.mm @@ -61,14 +61,12 @@ static BOOL _isInterceptedSelector(SEL sel) */ @interface _ASCollectionViewProxy : NSProxy - (instancetype)initWithTarget:(id)target interceptor:(ASCollectionView *)interceptor; -@property (nonatomic, weak) id target; @end @implementation _ASCollectionViewProxy { id __weak _target; ASCollectionView * __weak _interceptor; } -@synthesize target = _target; - (instancetype)initWithTarget:(id)target interceptor:(ASCollectionView *)interceptor { @@ -204,14 +202,6 @@ static BOOL _isInterceptedSelector(SEL sel) // and should not trigger a relayout. _ignoreMaxSizeChange = CGSizeEqualToSize(_maxSizeForNodesConstrainedSize, CGSizeZero); - // Set up the delegate / dataSource proxy now, so we recieve key method calls from UITableView even if - // our owner never sets up asyncDelegate (technically the dataSource is required) - _proxyDelegate = [[_ASCollectionViewProxy alloc] initWithTarget:[NSNull null] interceptor:self]; - super.delegate = (id)_proxyDelegate; - - _proxyDataSource = [[_ASCollectionViewProxy alloc] initWithTarget:[NSNull null] interceptor:self]; - super.dataSource = (id)_proxyDataSource; - self.backgroundColor = [UIColor whiteColor]; [self registerClass:[UICollectionViewCell class] forCellWithReuseIdentifier:@"_ASCollectionViewCell"]; @@ -262,21 +252,22 @@ static BOOL _isInterceptedSelector(SEL sel) // Note: It's common to check if the value hasn't changed and short-circuit but we aren't doing that here to handle // the (common) case of nilling the asyncDataSource in the ViewController's dealloc. In this case our _asyncDataSource // will return as nil (ARC magic) even though the _proxyDataSource still exists. It's really important to nil out - // _proxyDataSource.target in this case because calls to _ASCollectionViewProxy will start failing and cause crashes. + // super.dataSource in this case because calls to _ASTableViewProxy will start failing and cause crashes. if (asyncDataSource == nil) { - _proxyDataSource.target = nil; + super.dataSource = nil; _asyncDataSource = nil; + _proxyDataSource = nil; _asyncDataSourceImplementsConstrainedSizeForNode = NO; } else { - _proxyDataSource.target = asyncDataSource; _asyncDataSource = asyncDataSource; - _asyncDataSourceImplementsConstrainedSizeForNode = ([_asyncDataSource respondsToSelector:@selector(collectionView:constrainedSizeForNodeAtIndexPath:)] ? 1 : 0); - // TODO: Support supplementary views with ASCollectionView. if ([_asyncDataSource respondsToSelector:@selector(collectionView:viewForSupplementaryElementOfKind:atIndexPath:)]) { ASDisplayNodeAssert(NO, @"ASCollectionView is planned to support supplementary views by September 2015. You can work around this issue by using standard items."); } + _proxyDataSource = [[_ASCollectionViewProxy alloc] initWithTarget:_asyncDataSource interceptor:self]; + super.dataSource = (id)_proxyDataSource; + _asyncDataSourceImplementsConstrainedSizeForNode = ([_asyncDataSource respondsToSelector:@selector(collectionView:constrainedSizeForNodeAtIndexPath:)] ? 1 : 0); } } @@ -285,17 +276,19 @@ static BOOL _isInterceptedSelector(SEL sel) // Note: It's common to check if the value hasn't changed and short-circuit but we aren't doing that here to handle // the (common) case of nilling the asyncDelegate in the ViewController's dealloc. In this case our _asyncDelegate // will return as nil (ARC magic) even though the _proxyDelegate still exists. It's really important to nil out - // _proxyDelegate.target in this case because calls to _ASCollectionViewProxy will start failing and cause crashes. + // super.delegate in this case because calls to _ASTableViewProxy will start failing and cause crashes. if (asyncDelegate == nil) { // order is important here, the delegate must be callable while nilling super.delegate to avoid random crashes // in UIScrollViewAccessibility. - _proxyDelegate.target = nil; + super.delegate = nil; _asyncDelegate = nil; + _proxyDelegate = nil; _asyncDelegateImplementsInsetSection = NO; } else { - _proxyDelegate.target = asyncDelegate; _asyncDelegate = asyncDelegate; + _proxyDelegate = [[_ASCollectionViewProxy alloc] initWithTarget:_asyncDelegate interceptor:self]; + super.delegate = (id)_proxyDelegate; _asyncDelegateImplementsInsetSection = ([_asyncDelegate respondsToSelector:@selector(collectionView:layout:insetForSectionAtIndex:)] ? 1 : 0); } } diff --git a/AsyncDisplayKit/ASDisplayNode.mm b/AsyncDisplayKit/ASDisplayNode.mm index 6201fb75ef..c5beb9347b 100644 --- a/AsyncDisplayKit/ASDisplayNode.mm +++ b/AsyncDisplayKit/ASDisplayNode.mm @@ -744,6 +744,29 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) } } +- (void)__setSafeFrame:(CGRect)rect +{ + ASDisplayNodeAssertThreadAffinity(self); + ASDN::MutexLocker l(_propertyLock); + + BOOL useLayer = (_layer && ASDisplayNodeThreadIsMain()); + + CGPoint origin = (useLayer ? _layer.bounds.origin : self.bounds.origin); + CGPoint anchorPoint = (useLayer ? _layer.anchorPoint : self.anchorPoint); + + CGRect bounds = (CGRect){ origin, rect.size }; + CGPoint position = CGPointMake(rect.origin.x + rect.size.width * anchorPoint.x, + rect.origin.y + rect.size.height * anchorPoint.y); + + if (useLayer) { + _layer.bounds = bounds; + _layer.position = position; + } else { + self.bounds = bounds; + self.position = position; + } +} + // These private methods ensure that subclasses are not required to call super in order for _renderingSubnodes to be properly managed. - (void)__layout @@ -1691,10 +1714,10 @@ void recursivelyEnsureDisplayForLayer(CALayer *layer) // Assume that _layout was flattened and is 1-level deep. for (ASLayout *subnodeLayout in _layout.sublayouts) { ASDisplayNodeAssert([_subnodes containsObject:subnodeLayout.layoutableObject], @"Cached sublayouts must only contain subnodes' layout."); - ((ASDisplayNode *)subnodeLayout.layoutableObject).frame = CGRectMake(subnodeLayout.position.x, - subnodeLayout.position.y, - subnodeLayout.size.width, - subnodeLayout.size.height); + [((ASDisplayNode *)subnodeLayout.layoutableObject) __setSafeFrame:CGRectMake(subnodeLayout.position.x, + subnodeLayout.position.y, + subnodeLayout.size.width, + subnodeLayout.size.height)]; } } diff --git a/AsyncDisplayKit/ASMultiplexImageNode.h b/AsyncDisplayKit/ASMultiplexImageNode.h index 1956760899..302aa20ef5 100644 --- a/AsyncDisplayKit/ASMultiplexImageNode.h +++ b/AsyncDisplayKit/ASMultiplexImageNode.h @@ -212,6 +212,18 @@ didFinishDownloadingImageWithIdentifier:(id)imageIdentifier */ - (NSURL *)multiplexImageNode:(ASMultiplexImageNode *)imageNode URLForImageIdentifier:(id)imageIdentifier; +/** + * @abstract A PHAsset for the specific asset local identifier + * @param imageNode The sender. + * @param assetLocalIdentifier The local identifier for a PHAsset that this image node is loading. + * + * @discussion This optional method can improve image performance if your data source already has the PHAsset available. + * If this method is not implemented, or returns nil, the image node will request the asset from the Photos framework. + * @note This method may be called from any thread. + * @return A PHAsset corresponding to `assetLocalIdentifier`, or nil if none is available. + */ +- (PHAsset *)multiplexImageNode:(ASMultiplexImageNode *)imageNode assetForLocalIdentifier:(NSString *)assetLocalIdentifier; + @end #pragma mark - diff --git a/AsyncDisplayKit/ASMultiplexImageNode.mm b/AsyncDisplayKit/ASMultiplexImageNode.mm index 3d26fc0610..648c9bb7a0 100644 --- a/AsyncDisplayKit/ASMultiplexImageNode.mm +++ b/AsyncDisplayKit/ASMultiplexImageNode.mm @@ -57,6 +57,7 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent struct { unsigned int image:1; unsigned int URL:1; + unsigned int asset:1; } _dataSourceFlags; // Image flags. @@ -66,7 +67,8 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent id _loadedImageIdentifier; id _loadingImageIdentifier; id _displayedImageIdentifier; - + __weak NSOperation *_phImageRequestOperation; + // Networking. id _downloadIdentifier; } @@ -168,12 +170,19 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent return [self initWithCache:nil downloader:nil]; // satisfy compiler } +- (void)dealloc +{ + [_phImageRequestOperation cancel]; +} + #pragma mark - ASDisplayNode Overrides - (void)clearContents { [super clearContents]; // This actually clears the contents, so we need to do this first for our displayedImageIdentifier to be meaningful. [self _setDisplayedImageIdentifier:nil withImage:nil]; + [_phImageRequestOperation cancel]; + if (_downloadIdentifier) { [_downloader cancelImageDownloadForIdentifier:_downloadIdentifier]; _downloadIdentifier = nil; @@ -185,6 +194,14 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent [super clearFetchedData]; if ([self _shouldClearFetchedImageData]) { + + [_phImageRequestOperation cancel]; + + if (_downloadIdentifier) { + [_downloader cancelImageDownloadForIdentifier:_downloadIdentifier]; + _downloadIdentifier = nil; + } + // setting this to nil makes the node fetch images the next time its display starts _loadedImageIdentifier = nil; self.image = nil; @@ -250,6 +267,7 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent _dataSource = dataSource; _dataSourceFlags.image = [_dataSource respondsToSelector:@selector(multiplexImageNode:imageForImageIdentifier:)]; _dataSourceFlags.URL = [_dataSource respondsToSelector:@selector(multiplexImageNode:URLForImageIdentifier:)]; + _dataSourceFlags.asset = [_dataSource respondsToSelector:@selector(multiplexImageNode:assetForLocalIdentifier:)]; } #pragma mark - @@ -518,17 +536,52 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent ASDisplayNodeAssertNotNil(request, @"request is required"); ASDisplayNodeAssertNotNil(completionBlock, @"completionBlock is required"); - // This is sometimes called on main but there's no reason to stay there - dispatch_async(dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0), ^{ - // Get the PHAsset itself. - PHFetchResult *assetFetchResult = [PHAsset fetchAssetsWithLocalIdentifiers:@[request.assetIdentifier] options:nil]; - if ([assetFetchResult count] == 0) { + /* + * Locking rationale: + * As of iOS 9, Photos.framework will eventually deadlock if you hit it with concurrent fetch requests. rdar://22984886 + * Concurrent image requests are OK, but metadata requests aren't, so we limit ourselves to one at a time. + */ + static NSLock *phRequestLock; + static NSOperationQueue *phImageRequestQueue; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + phRequestLock = [NSLock new]; + phImageRequestQueue = [NSOperationQueue new]; + phImageRequestQueue.maxConcurrentOperationCount = 10; + phImageRequestQueue.name = @"org.AsyncDisplayKit.MultiplexImageNode.phImageRequestQueue"; + }); + + // Each ASMultiplexImageNode can have max 1 inflight Photos image request operation + [_phImageRequestOperation cancel]; + + __weak __typeof(self) weakSelf = self; + NSOperation *newImageRequestOp = [NSBlockOperation blockOperationWithBlock:^{ + __strong __typeof(weakSelf) strongSelf = weakSelf; + if (strongSelf == nil) { return; } + + PHAsset *imageAsset = nil; + + // Try to get the asset immediately from the data source. + if (_dataSourceFlags.asset) { + imageAsset = [strongSelf.dataSource multiplexImageNode:strongSelf assetForLocalIdentifier:request.assetIdentifier]; + } + + // Fall back to locking and getting the PHAsset. + if (imageAsset == nil) { + [phRequestLock lock]; + // -[PHFetchResult dealloc] plays a role in the deadlock mentioned above, so we make sure the PHFetchResult is deallocated inside the critical section + @autoreleasepool { + imageAsset = [PHAsset fetchAssetsWithLocalIdentifiers:@[request.assetIdentifier] options:nil].firstObject; + } + [phRequestLock unlock]; + } + + if (imageAsset == nil) { // Error. completionBlock(nil, nil); return; } - PHAsset *imageAsset = [assetFetchResult firstObject]; PHImageRequestOptions *options = [request.options copy]; // We don't support opportunistic delivery – one request, one image. @@ -542,7 +595,7 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent options.synchronous = YES; } - PHImageManager *imageManager = self.imageManager ?: PHImageManager.defaultManager; + PHImageManager *imageManager = strongSelf.imageManager ?: PHImageManager.defaultManager; [imageManager requestImageForAsset:imageAsset targetSize:request.targetSize contentMode:request.contentMode options:options resultHandler:^(UIImage *image, NSDictionary *info) { if (NSThread.isMainThread) { dispatch_async(dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0), ^{ @@ -552,7 +605,9 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent completionBlock(image, info[PHImageErrorKey]); } }]; - }); + }]; + _phImageRequestOperation = newImageRequestOp; + [phImageRequestQueue addOperation:newImageRequestOp]; } - (void)_fetchImageWithIdentifierFromCache:(id)imageIdentifier URL:(NSURL *)imageURL completion:(void (^)(UIImage *image))completionBlock diff --git a/AsyncDisplayKit/ASTableView.mm b/AsyncDisplayKit/ASTableView.mm index d917b230b2..c93075fa74 100644 --- a/AsyncDisplayKit/ASTableView.mm +++ b/AsyncDisplayKit/ASTableView.mm @@ -59,14 +59,12 @@ static BOOL _isInterceptedSelector(SEL sel) */ @interface _ASTableViewProxy : NSProxy - (instancetype)initWithTarget:(id)target interceptor:(ASTableView *)interceptor; -@property (nonatomic, weak) id target; @end @implementation _ASTableViewProxy { id __weak _target; ASTableView * __weak _interceptor; } -@synthesize target = _target; - (instancetype)initWithTarget:(id)target interceptor:(ASTableView *)interceptor { @@ -220,14 +218,6 @@ void ASPerformBlockWithoutAnimation(BOOL withoutAnimation, void (^block)()) { // If the initial size is 0, expect a size change very soon which is part of the initial configuration // and should not trigger a relayout. _ignoreMaxWidthChange = (_maxWidthForNodesConstrainedSize == 0); - - // Set up the delegate / dataSource proxy now, so we recieve key method calls from UITableView even if - // our owner never sets up asyncDelegate (technically the dataSource is required) - _proxyDelegate = [[_ASTableViewProxy alloc] initWithTarget:[NSNull null] interceptor:self]; - super.delegate = (id)_proxyDelegate; - - _proxyDataSource = [[_ASTableViewProxy alloc] initWithTarget:[NSNull null] interceptor:self]; - super.dataSource = (id)_proxyDataSource; } - (instancetype)initWithFrame:(CGRect)frame style:(UITableViewStyle)style @@ -287,14 +277,16 @@ void ASPerformBlockWithoutAnimation(BOOL withoutAnimation, void (^block)()) { // Note: It's common to check if the value hasn't changed and short-circuit but we aren't doing that here to handle // the (common) case of nilling the asyncDataSource in the ViewController's dealloc. In this case our _asyncDataSource // will return as nil (ARC magic) even though the _proxyDataSource still exists. It's really important to nil out - // _proxyDataSource.target in this case because calls to _ASTableViewProxy will start failing and cause crashes. + // super.dataSource in this case because calls to _ASTableViewProxy will start failing and cause crashes. if (asyncDataSource == nil) { - _proxyDataSource.target = nil; + super.dataSource = nil; _asyncDataSource = nil; + _proxyDataSource = nil; } else { - _proxyDataSource.target = asyncDataSource; _asyncDataSource = asyncDataSource; + _proxyDataSource = [[_ASTableViewProxy alloc] initWithTarget:_asyncDataSource interceptor:self]; + super.dataSource = (id)_proxyDataSource; } } @@ -303,16 +295,18 @@ void ASPerformBlockWithoutAnimation(BOOL withoutAnimation, void (^block)()) { // Note: It's common to check if the value hasn't changed and short-circuit but we aren't doing that here to handle // the (common) case of nilling the asyncDelegate in the ViewController's dealloc. In this case our _asyncDelegate // will return as nil (ARC magic) even though the _proxyDelegate still exists. It's really important to nil out - // _proxyDelegate.target in this case because calls to _ASTableViewProxy will start failing and cause crashes. + // super.delegate in this case because calls to _ASTableViewProxy will start failing and cause crashes. if (asyncDelegate == nil) { // order is important here, the delegate must be callable while nilling super.delegate to avoid random crashes // in UIScrollViewAccessibility. - _proxyDelegate.target = nil; + super.delegate = nil; _asyncDelegate = nil; + _proxyDelegate = nil; } else { - _proxyDelegate.target = asyncDelegate; _asyncDelegate = asyncDelegate; + _proxyDelegate = [[_ASTableViewProxy alloc] initWithTarget:asyncDelegate interceptor:self]; + super.delegate = (id)_proxyDelegate; } } diff --git a/AsyncDisplayKit/Details/ASDataController.mm b/AsyncDisplayKit/Details/ASDataController.mm index 7b07ff76ef..6a5b361ff0 100644 --- a/AsyncDisplayKit/Details/ASDataController.mm +++ b/AsyncDisplayKit/Details/ASDataController.mm @@ -96,6 +96,25 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; #pragma mark - Cell Layout +/* + * FIXME: Shouldn't this method, as well as `_layoutNodes:atIndexPaths:withAnimationOptions:` use the word "measure" instead? + * + * Once nodes have loaded their views, we can't layout in the background so this is a chance + * to do so immediately on the main thread. + */ +- (void)_layoutNodesWithMainThreadAffinity:(NSArray *)nodes atIndexPaths:(NSArray *)indexPaths { + NSAssert(NSThread.isMainThread, @"Main thread layout must be on the main thread."); + + [indexPaths enumerateObjectsUsingBlock:^(NSIndexPath *indexPath, NSUInteger idx, __unused BOOL * stop) { + ASCellNode *node = nodes[idx]; + if (node.isNodeLoaded) { + ASSizeRange constrainedSize = [_dataSource dataController:self constrainedSizeForNodeAtIndexPath:indexPath]; + [node measureWithSizeRange:constrainedSize]; + node.frame = CGRectMake(0.0f, 0.0f, node.calculatedSize.width, node.calculatedSize.height); + } + }]; +} + - (void)_layoutNodes:(NSArray *)nodes atIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions { ASDisplayNodeAssert([NSOperationQueue currentQueue] == _editingTransactionQueue, @"Cell node layout must be initiated from edit transaction queue"); @@ -110,15 +129,22 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; NSInteger batchCount = MIN(kASDataControllerSizingCountPerProcessor, indexPaths.count - j); for (NSUInteger k = j; k < j + batchCount; k++) { - nodeBoundSizes[k] = [_dataSource dataController:self constrainedSizeForNodeAtIndexPath:indexPaths[k]]; + ASCellNode *node = nodes[k]; + if (!node.isNodeLoaded) { + nodeBoundSizes[k] = [_dataSource dataController:self constrainedSizeForNodeAtIndexPath:indexPaths[k]]; + } } dispatch_group_async(layoutGroup, dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ for (NSUInteger k = j; k < j + batchCount; k++) { ASCellNode *node = nodes[k]; - ASSizeRange constrainedSize = nodeBoundSizes[k]; - [node measureWithSizeRange:constrainedSize]; - node.frame = CGRectMake(0, 0, node.calculatedSize.width, node.calculatedSize.height); + // Only measure nodes whose views aren't loaded, since we're in the background. + // We should already have measured loaded nodes before we left the main thread, using _layoutNodesWithMainThreadAffinity: + if (!node.isNodeLoaded) { + ASSizeRange constrainedSize = nodeBoundSizes[k]; + [node measureWithSizeRange:constrainedSize]; + node.frame = CGRectMake(0.0f, 0.0f, node.calculatedSize.width, node.calculatedSize.height); + } } }); } @@ -245,6 +271,9 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; NSMutableArray *updatedIndexPaths = [NSMutableArray array]; [self _populateFromEntireDataSourceWithMutableNodes:updatedNodes mutableIndexPaths:updatedIndexPaths]; + // Measure nodes whose views are loaded before we leave the main thread + [self _layoutNodesWithMainThreadAffinity:updatedNodes atIndexPaths:updatedIndexPaths]; + [_editingTransactionQueue addOperationWithBlock:^{ LOG(@"Edit Transaction - reloadData"); @@ -399,6 +428,9 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; NSMutableArray *updatedIndexPaths = [NSMutableArray array]; [self _populateFromDataSourceWithSectionIndexSet:indexSet mutableNodes:updatedNodes mutableIndexPaths:updatedIndexPaths]; + // Measure nodes whose views are loaded before we leave the main thread + [self _layoutNodesWithMainThreadAffinity:updatedNodes atIndexPaths:updatedIndexPaths]; + [_editingTransactionQueue addOperationWithBlock:^{ LOG(@"Edit Transaction - insertSections: %@", indexSet); NSMutableArray *sectionArray = [NSMutableArray arrayWithCapacity:indexSet.count]; @@ -448,6 +480,9 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; // For example, if an initial -reloadData call is quickly followed by -reloadSections, sizing the initial set may not be done // at this time. Thus _editingNodes could be empty and crash in ASIndexPathsForMultidimensional[...] + // Measure nodes whose views are loaded before we leave the main thread + [self _layoutNodesWithMainThreadAffinity:updatedNodes atIndexPaths:updatedIndexPaths]; + [_editingTransactionQueue addOperationWithBlock:^{ NSArray *indexPaths = ASIndexPathsForMultidimensionalArrayAtIndexSet(_editingNodes, sections); @@ -482,9 +517,9 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; // update the section of indexpaths NSIndexPath *sectionIndexPath = [[NSIndexPath alloc] initWithIndex:newSection]; NSMutableArray *updatedIndexPaths = [[NSMutableArray alloc] initWithCapacity:indexPaths.count]; - [indexPaths enumerateObjectsUsingBlock:^(NSIndexPath *indexPath, NSUInteger idx, BOOL *stop) { + for (NSIndexPath *indexPath in indexPaths) { [updatedIndexPaths addObject:[sectionIndexPath indexPathByAddingIndex:[indexPath indexAtPosition:indexPath.length - 1]]]; - }]; + } // Don't re-calculate size for moving [self _insertNodes:nodes atIndexPaths:updatedIndexPaths withAnimationOptions:animationOptions]; @@ -510,6 +545,9 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; [nodes addObject:[_dataSource dataController:self nodeAtIndexPath:sortedIndexPaths[i]]]; } + // Measure nodes whose views are loaded before we leave the main thread + [self _layoutNodesWithMainThreadAffinity:nodes atIndexPaths:indexPaths]; + [_editingTransactionQueue addOperationWithBlock:^{ LOG(@"Edit Transaction - insertRows: %@", indexPaths); [self _batchLayoutNodes:nodes atIndexPaths:indexPaths withAnimationOptions:animationOptions]; @@ -527,6 +565,7 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; [_editingTransactionQueue waitUntilAllOperationsAreFinished]; // sort indexPath in order to avoid messing up the index when deleting + // FIXME: Shouldn't deletes be sorted in descending order? NSArray *sortedIndexPaths = [indexPaths sortedArrayUsingSelector:@selector(compare:)]; [_editingTransactionQueue addOperationWithBlock:^{ @@ -547,11 +586,18 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; // Reloading requires re-fetching the data. Load it on the current calling thread, locking the data source. [self accessDataSourceWithBlock:^{ NSMutableArray *nodes = [[NSMutableArray alloc] initWithCapacity:indexPaths.count]; - [indexPaths sortedArrayUsingSelector:@selector(compare:)]; - [indexPaths enumerateObjectsUsingBlock:^(NSIndexPath *indexPath, NSUInteger idx, BOOL *stop) { - [nodes addObject:[_dataSource dataController:self nodeAtIndexPath:indexPath]]; - }]; + // FIXME: This doesn't currently do anything + // FIXME: Shouldn't deletes be sorted in descending order? + [indexPaths sortedArrayUsingSelector:@selector(compare:)]; + + for (NSIndexPath *indexPath in indexPaths) { + [nodes addObject:[_dataSource dataController:self nodeAtIndexPath:indexPath]]; + } + + // Measure nodes whose views are loaded before we leave the main thread + [self _layoutNodesWithMainThreadAffinity:nodes atIndexPaths:indexPaths]; + [_editingTransactionQueue addOperationWithBlock:^{ LOG(@"Edit Transaction - reloadRows: %@", indexPaths); [self _deleteNodesAtIndexPaths:indexPaths withAnimationOptions:animationOptions]; diff --git a/AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm b/AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm index 977c98ec40..4909abdd46 100644 --- a/AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm +++ b/AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm @@ -143,22 +143,7 @@ ASDisplayNodeAssert(CATransform3DIsIdentity(self.transform), @"-[ASDisplayNode setFrame:] - self.transform must be identity in order to set the frame property. (From Apple's UIView documentation: If the transform property is not the identity transform, the value of this property is undefined and therefore should be ignored.)"); #endif - BOOL useLayer = (_layer && ASDisplayNodeThreadIsMain()); - - CGPoint origin = (useLayer ? _layer.bounds.origin : self.bounds.origin); - CGPoint anchorPoint = (useLayer ? _layer.anchorPoint : self.anchorPoint); - - CGRect bounds = (CGRect){ origin, rect.size }; - CGPoint position = CGPointMake(rect.origin.x + rect.size.width * anchorPoint.x, - rect.origin.y + rect.size.height * anchorPoint.y); - - if (useLayer) { - _layer.bounds = bounds; - _layer.position = position; - } else { - self.bounds = bounds; - self.position = position; - } + [self __setSafeFrame:rect]; } - (void)setNeedsDisplay diff --git a/AsyncDisplayKit/Private/ASDisplayNodeInternal.h b/AsyncDisplayKit/Private/ASDisplayNodeInternal.h index 8283438272..c2daf155be 100644 --- a/AsyncDisplayKit/Private/ASDisplayNodeInternal.h +++ b/AsyncDisplayKit/Private/ASDisplayNodeInternal.h @@ -126,6 +126,11 @@ typedef NS_OPTIONS(NSUInteger, ASDisplayNodeMethodOverrides) { - (ASLayout *)__measureWithSizeRange:(ASSizeRange)constrainedSize; - (void)__setNeedsLayout; +/** + * Sets a new frame to this node by changing its bounds and position. This method can be safely called even if the transform property + * contains a non-identity transform, because bounds and position can be changed in such case. + */ +- (void)__setSafeFrame:(CGRect)rect; - (void)__layout; - (void)__setSupernode:(ASDisplayNode *)supernode;