From 062bcf3631de69f596d970c21aa1ce25869d02a2 Mon Sep 17 00:00:00 2001 From: Gareth Reese Date: Tue, 21 Jun 2016 07:51:58 +0100 Subject: [PATCH 1/6] [ASVideoNode] issue #1782 Placeholder images are replaced by a blank placeholder. Now checks the .URL property of the parent class as well as the .image to ensure that new placeholders aren't generated. --- AsyncDisplayKit/ASVideoNode.mm | 4 ++-- examples/Videos/Sample/ViewController.m | 28 ++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/AsyncDisplayKit/ASVideoNode.mm b/AsyncDisplayKit/ASVideoNode.mm index 8290e9da4d..95bb35f7dd 100644 --- a/AsyncDisplayKit/ASVideoNode.mm +++ b/AsyncDisplayKit/ASVideoNode.mm @@ -151,7 +151,7 @@ static NSString * const kStatus = @"status"; self.player = [AVPlayer playerWithPlayerItem:playerItem]; } - if (self.image == nil) { + if (self.image == nil && self.URL == nil) { [self generatePlaceholderImage]; } @@ -284,7 +284,7 @@ static NSString * const kStatus = @"status"; if ([change[NSKeyValueChangeNewKey] integerValue] == AVPlayerItemStatusReadyToPlay) { self.playerState = ASVideoNodePlayerStateReadyToPlay; // If we don't yet have a placeholder image update it now that we should have data available for it - if (self.image == nil) { + if (self.image == nil && self.URL == nil) { [self generatePlaceholderImage]; } } diff --git a/examples/Videos/Sample/ViewController.m b/examples/Videos/Sample/ViewController.m index 43a0fd6755..e2a6e22b88 100644 --- a/examples/Videos/Sample/ViewController.m +++ b/examples/Videos/Sample/ViewController.m @@ -46,6 +46,9 @@ ASVideoNode *simonVideoNode = self.simonVideoNode; [_rootNode addSubnode:simonVideoNode]; + ASVideoNode *hlsVideoNode = self.hlsVideoNode; + [_rootNode addSubnode:hlsVideoNode]; + _rootNode.layoutSpecBlock = ^ASLayoutSpec *(ASDisplayNode * _Nonnull node, ASSizeRange constrainedSize) { guitarVideoNode.layoutPosition = CGPointMake(0, 0); guitarVideoNode.preferredFrameSize = CGSizeMake([UIScreen mainScreen].bounds.size.width, [UIScreen mainScreen].bounds.size.height/3); @@ -55,8 +58,13 @@ simonVideoNode.layoutPosition = CGPointMake(0, [UIScreen mainScreen].bounds.size.height - ([UIScreen mainScreen].bounds.size.height/3)); simonVideoNode.preferredFrameSize = CGSizeMake([UIScreen mainScreen].bounds.size.width/2, [UIScreen mainScreen].bounds.size.height/3); - return [ASStaticLayoutSpec staticLayoutSpecWithChildren:@[guitarVideoNode, nicCageVideoNode, simonVideoNode]]; + + hlsVideoNode.layoutPosition = CGPointMake(0, [UIScreen mainScreen].bounds.size.height/3); + hlsVideoNode.preferredFrameSize = CGSizeMake([UIScreen mainScreen].bounds.size.width/2, [UIScreen mainScreen].bounds.size.height/3); + + return [ASStaticLayoutSpec staticLayoutSpecWithChildren:@[guitarVideoNode, nicCageVideoNode, simonVideoNode, hlsVideoNode]]; }; + [self.view addSubnode:_rootNode]; } @@ -117,6 +125,24 @@ return simonVideoNode; } +- (ASVideoNode *)hlsVideoNode; +{ + ASVideoNode *hlsVideoNode = [[ASVideoNode alloc] init]; + + hlsVideoNode.delegate = self; + hlsVideoNode.asset = [AVAsset assetWithURL:[NSURL URLWithString:@"http://devimages.apple.com/iphone/samples/bipbop/gear1/prog_index.m3u8"]]; + hlsVideoNode.gravity = AVLayerVideoGravityResize; + hlsVideoNode.backgroundColor = [UIColor lightGrayColor]; + hlsVideoNode.shouldAutorepeat = YES; + hlsVideoNode.shouldAutoplay = YES; + hlsVideoNode.muted = YES; + + // Placeholder image + hlsVideoNode.URL = [NSURL URLWithString:@"https://upload.wikimedia.org/wikipedia/en/5/52/Testcard_F.jpg"]; + + return hlsVideoNode; +} + - (ASButtonNode *)playButton; { ASButtonNode *playButtonNode = [[ASButtonNode alloc] init]; From 56a534349604621383eab9b62b8d84d86bf89514 Mon Sep 17 00:00:00 2001 From: Kiel Gillard Date: Mon, 27 Jun 2016 11:29:28 +1000 Subject: [PATCH 2/6] Collect subnodes passing the test, not the node with the subnodes passing the test. --- AsyncDisplayKit/ASDisplayNodeExtras.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AsyncDisplayKit/ASDisplayNodeExtras.mm b/AsyncDisplayKit/ASDisplayNodeExtras.mm index 46eb46daaf..d9601ba9d6 100644 --- a/AsyncDisplayKit/ASDisplayNodeExtras.mm +++ b/AsyncDisplayKit/ASDisplayNodeExtras.mm @@ -144,7 +144,7 @@ static void _ASDisplayNodeFindAllSubnodes(NSMutableArray *array, ASDisplayNode * for (ASDisplayNode *subnode in node.subnodes) { if (block(subnode)) { - [array addObject:node]; + [array addObject:subnode]; } _ASDisplayNodeFindAllSubnodes(array, subnode, block); From cc74fb0cdcdc46daa98ae734d85a92c083b3cf09 Mon Sep 17 00:00:00 2001 From: Kiel Gillard Date: Mon, 27 Jun 2016 12:53:33 +1000 Subject: [PATCH 3/6] Prove the change gets the correct nodes with a pair of tests. --- AsyncDisplayKit.xcodeproj/project.pbxproj | 4 + .../ASDisplayNodeExtrasTests.m | 76 +++++++++++++++++++ 2 files changed, 80 insertions(+) create mode 100644 AsyncDisplayKitTests/ASDisplayNodeExtrasTests.m diff --git a/AsyncDisplayKit.xcodeproj/project.pbxproj b/AsyncDisplayKit.xcodeproj/project.pbxproj index 62c3baffc7..99ed51bbd8 100644 --- a/AsyncDisplayKit.xcodeproj/project.pbxproj +++ b/AsyncDisplayKit.xcodeproj/project.pbxproj @@ -596,6 +596,7 @@ E5711A2C1C840C81009619D4 /* ASIndexedNodeContext.h in Headers */ = {isa = PBXBuildFile; fileRef = E5711A2A1C840C81009619D4 /* ASIndexedNodeContext.h */; }; E5711A2E1C840C96009619D4 /* ASIndexedNodeContext.mm in Sources */ = {isa = PBXBuildFile; fileRef = E5711A2D1C840C96009619D4 /* ASIndexedNodeContext.mm */; }; E5711A301C840C96009619D4 /* ASIndexedNodeContext.mm in Sources */ = {isa = PBXBuildFile; fileRef = E5711A2D1C840C96009619D4 /* ASIndexedNodeContext.mm */; }; + F711994E1D20C21100568860 /* ASDisplayNodeExtrasTests.m in Sources */ = {isa = PBXBuildFile; fileRef = F711994D1D20C21100568860 /* ASDisplayNodeExtrasTests.m */; }; /* End PBXBuildFile section */ /* Begin PBXContainerItemProxy section */ @@ -965,6 +966,7 @@ E5711A2A1C840C81009619D4 /* ASIndexedNodeContext.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASIndexedNodeContext.h; sourceTree = ""; }; E5711A2D1C840C96009619D4 /* ASIndexedNodeContext.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ASIndexedNodeContext.mm; sourceTree = ""; }; EFA731F0396842FF8AB635EE /* libPods-AsyncDisplayKitTests.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = "libPods-AsyncDisplayKitTests.a"; sourceTree = BUILT_PRODUCTS_DIR; }; + F711994D1D20C21100568860 /* ASDisplayNodeExtrasTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASDisplayNodeExtrasTests.m; sourceTree = ""; }; FB07EABBCF28656C6297BC2D /* Pods-AsyncDisplayKitTests.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-AsyncDisplayKitTests.debug.xcconfig"; path = "Pods/Target Support Files/Pods-AsyncDisplayKitTests/Pods-AsyncDisplayKitTests.debug.xcconfig"; sourceTree = ""; }; /* End PBXFileReference section */ @@ -1210,6 +1212,7 @@ 058D0A36195D057000B7D73C /* ASTextNodeTests.m */, 058D0A37195D057000B7D73C /* ASTextNodeWordKernerTests.mm */, AEEC47E31C21D3D200EC1693 /* ASVideoNodeTests.m */, + F711994D1D20C21100568860 /* ASDisplayNodeExtrasTests.m */, 058D09C6195D04C000B7D73C /* Supporting Files */, 052EE06A1A15A0D8002C6279 /* TestResources */, 2538B6F21BC5D2A2003CA0B4 /* ASCollectionViewFlowLayoutInspectorTests.m */, @@ -2169,6 +2172,7 @@ 9F06E5CD1B4CAF4200F015D8 /* ASCollectionViewTests.m in Sources */, 2911485C1A77147A005D0878 /* ASControlNodeTests.m in Sources */, CC3B208E1C3F7D0A00798563 /* ASWeakSetTests.m in Sources */, + F711994E1D20C21100568860 /* ASDisplayNodeExtrasTests.m in Sources */, ACF6ED5D1B178DC700DA7C62 /* ASDimensionTests.mm in Sources */, 058D0A38195D057000B7D73C /* ASDisplayLayerTests.m in Sources */, 2538B6F31BC5D2A2003CA0B4 /* ASCollectionViewFlowLayoutInspectorTests.m in Sources */, diff --git a/AsyncDisplayKitTests/ASDisplayNodeExtrasTests.m b/AsyncDisplayKitTests/ASDisplayNodeExtrasTests.m new file mode 100644 index 0000000000..6f1731d211 --- /dev/null +++ b/AsyncDisplayKitTests/ASDisplayNodeExtrasTests.m @@ -0,0 +1,76 @@ +// +// ASDisplayNodeExtrasTests.m +// AsyncDisplayKit +// +// Created by Kiel Gillard on 27/06/2016. +// Copyright © 2016 Facebook. All rights reserved. +// + +#import +#import +#import + +@interface ASDisplayNodeExtrasTests : XCTestCase + +@end + +@interface TestDisplayNode : ASDisplayNode +@end + +@implementation TestDisplayNode +@end + +@implementation ASDisplayNodeExtrasTests + +- (void)testShallowFindSubnodesOfSubclass { + ASDisplayNode *supernode = [[ASDisplayNode alloc] initWithLayerBlock:^CALayer * _Nonnull{ + return [CALayer layer]; + }]; + NSUInteger count = 10; + NSMutableArray *expected = [[NSMutableArray alloc] initWithCapacity:count]; + for (NSUInteger nodeIndex = 0; nodeIndex < count; nodeIndex++) { + TestDisplayNode *node = [[TestDisplayNode alloc] initWithLayerBlock:^CALayer * _Nonnull{ + return [CALayer layer]; + }]; + [supernode addSubnode:node]; + [expected addObject:node]; + } + NSArray *found = ASDisplayNodeFindAllSubnodesOfClass(supernode, [TestDisplayNode class]); + XCTAssertEqualObjects(found, expected, @"Expecting %lu %@ nodes, found %lu", (unsigned long)count, [TestDisplayNode class], (unsigned long)found.count); +} + +- (void)testDeepFindSubnodesOfSubclass { + ASDisplayNode *supernode = [[ASDisplayNode alloc] initWithLayerBlock:^CALayer * _Nonnull{ + return [CALayer layer]; + }]; + + const NSUInteger count = 2; + const NSUInteger levels = 2; + const NSUInteger capacity = [[self class] capacityForCount:count levels:levels]; + NSMutableArray *expected = [[NSMutableArray alloc] initWithCapacity:capacity]; + + [[self class] addSubnodesToNode:supernode number:count remainingLevels:levels accumulated:expected]; + + NSArray *found = ASDisplayNodeFindAllSubnodesOfClass(supernode, [TestDisplayNode class]); + XCTAssertEqualObjects(found, expected, @"Expecting %lu %@ nodes, found %lu", (unsigned long)count, [TestDisplayNode class], (unsigned long)found.count); +} + ++ (void)addSubnodesToNode:(ASDisplayNode *)supernode number:(NSUInteger)number remainingLevels:(NSUInteger)level accumulated:(inout NSMutableArray *)expected { + if (level == 0) return; + for (NSUInteger nodeIndex = 0; nodeIndex < number; nodeIndex++) { + TestDisplayNode *node = [[TestDisplayNode alloc] initWithLayerBlock:^CALayer * _Nonnull{ + return [CALayer layer]; + }]; + [supernode addSubnode:node]; + [expected addObject:node]; + [self addSubnodesToNode:node number:number remainingLevels:(level - 1) accumulated:expected]; + } +} + +// Graph theory is failing me atm. ++ (NSUInteger)capacityForCount:(NSUInteger)count levels:(NSUInteger)level { + if (level == 0) return 0; + return pow(count, level) + [self capacityForCount:count levels:(level - 1)]; +} + +@end From 9501299eed1d450ef5f9b62db4495fc871c052a0 Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Fri, 24 Jun 2016 07:57:20 -0700 Subject: [PATCH 4/6] Use flags to cache instead of instance variables for caching respond to selector calls --- AsyncDisplayKit/ASNetworkImageNode.mm | 84 ++++++++++++++------------- 1 file changed, 44 insertions(+), 40 deletions(-) diff --git a/AsyncDisplayKit/ASNetworkImageNode.mm b/AsyncDisplayKit/ASNetworkImageNode.mm index fbfea44213..77559209c0 100755 --- a/AsyncDisplayKit/ASNetworkImageNode.mm +++ b/AsyncDisplayKit/ASNetworkImageNode.mm @@ -43,24 +43,28 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; BOOL _imageLoaded; CGFloat _currentImageQuality; CGFloat _renderedImageQuality; - - // TODO: Move this to flags - BOOL _delegateSupportsDidStartFetchingData; - BOOL _delegateSupportsDidFailWithError; - BOOL _delegateSupportsDidFinishDecoding; - BOOL _delegateSupportsDidLoadImage; - BOOL _shouldRenderProgressImages; - //set on init only - BOOL _downloaderSupportsNewProtocol; - BOOL _downloaderImplementsSetProgress; - BOOL _downloaderImplementsSetPriority; - BOOL _downloaderImplementsAnimatedImage; + struct { + unsigned int delegateDidStartFetchingData:1; + unsigned int delegateDidFailWithError:1; + unsigned int delegateDidFinishDecoding:1; + unsigned int delegateDidLoadImage:1; + } _delegateFlags; - BOOL _cacheSupportsNewProtocol; - BOOL _cacheSupportsClearing; - BOOL _cacheSupportsSynchronousFetch; + //set on init only + struct { + unsigned int downloaderSupportsNewProtocol:1; + unsigned int downloaderImplementsSetProgress:1; + unsigned int downloaderImplementsSetPriority:1; + unsigned int downloaderImplementsAnimatedImage:1; + } _downloaderFlags; + + struct { + unsigned int cacheSupportsNewProtocol:1; + unsigned int cacheSupportsClearing:1; + unsigned int cacheSupportsSynchronousFetch:1; + } _cacheFlags; } @end @@ -76,17 +80,17 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; ASDisplayNodeAssert([downloader respondsToSelector:@selector(downloadImageWithURL:callbackQueue:downloadProgress:completion:)] || [downloader respondsToSelector:@selector(downloadImageWithURL:callbackQueue:downloadProgressBlock:completion:)], @"downloader must respond to either downloadImageWithURL:callbackQueue:downloadProgress:completion: or downloadImageWithURL:callbackQueue:downloadProgressBlock:completion:."); - _downloaderSupportsNewProtocol = [downloader respondsToSelector:@selector(downloadImageWithURL:callbackQueue:downloadProgress:completion:)]; + _downloaderFlags.downloaderSupportsNewProtocol = [downloader respondsToSelector:@selector(downloadImageWithURL:callbackQueue:downloadProgress:completion:)]; ASDisplayNodeAssert(cache == nil || [cache respondsToSelector:@selector(cachedImageWithURL:callbackQueue:completion:)] || [cache respondsToSelector:@selector(fetchCachedImageWithURL:callbackQueue:completion:)], @"cacher must respond to either cachedImageWithURL:callbackQueue:completion: or fetchCachedImageWithURL:callbackQueue:completion:"); - _downloaderImplementsSetProgress = [downloader respondsToSelector:@selector(setProgressImageBlock:callbackQueue:withDownloadIdentifier:)]; - _downloaderImplementsSetPriority = [downloader respondsToSelector:@selector(setPriority:withDownloadIdentifier:)]; - _downloaderImplementsAnimatedImage = [downloader respondsToSelector:@selector(animatedImageWithData:)]; + _downloaderFlags.downloaderImplementsSetProgress = [downloader respondsToSelector:@selector(setProgressImageBlock:callbackQueue:withDownloadIdentifier:)]; + _downloaderFlags.downloaderImplementsSetPriority = [downloader respondsToSelector:@selector(setPriority:withDownloadIdentifier:)]; + _downloaderFlags.downloaderImplementsAnimatedImage = [downloader respondsToSelector:@selector(animatedImageWithData:)]; - _cacheSupportsNewProtocol = [cache respondsToSelector:@selector(cachedImageWithURL:callbackQueue:completion:)]; - _cacheSupportsClearing = [cache respondsToSelector:@selector(clearFetchedImageFromCacheWithURL:)]; - _cacheSupportsSynchronousFetch = [cache respondsToSelector:@selector(synchronouslyFetchedCachedImageWithURL:)]; + _cacheFlags.cacheSupportsNewProtocol = [cache respondsToSelector:@selector(cachedImageWithURL:callbackQueue:completion:)]; + _cacheFlags.cacheSupportsClearing = [cache respondsToSelector:@selector(clearFetchedImageFromCacheWithURL:)]; + _cacheFlags.cacheSupportsSynchronousFetch = [cache respondsToSelector:@selector(synchronouslyFetchedCachedImageWithURL:)]; _shouldCacheImage = YES; _shouldRenderProgressImages = YES; @@ -214,10 +218,10 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; ASDN::MutexLocker l(_lock); _delegate = delegate; - _delegateSupportsDidStartFetchingData = [delegate respondsToSelector:@selector(imageNodeDidStartFetchingData:)]; - _delegateSupportsDidFailWithError = [delegate respondsToSelector:@selector(imageNode:didFailWithError:)]; - _delegateSupportsDidFinishDecoding = [delegate respondsToSelector:@selector(imageNodeDidFinishDecoding:)]; - _delegateSupportsDidLoadImage = [delegate respondsToSelector:@selector(imageNode:didLoadImage:)]; + _delegateFlags.delegateDidStartFetchingData = [delegate respondsToSelector:@selector(imageNodeDidStartFetchingData:)]; + _delegateFlags.delegateDidFailWithError = [delegate respondsToSelector:@selector(imageNode:didFailWithError:)]; + _delegateFlags.delegateDidFinishDecoding = [delegate respondsToSelector:@selector(imageNodeDidFinishDecoding:)]; + _delegateFlags.delegateDidLoadImage = [delegate respondsToSelector:@selector(imageNode:didLoadImage:)]; } - (id)delegate @@ -258,7 +262,7 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; { [super displayWillStart]; - if (_cacheSupportsSynchronousFetch) { + if (_cacheFlags.cacheSupportsSynchronousFetch) { ASDN::MutexLocker l(_lock); if (_imageLoaded == NO && _URL && _downloadIdentifier == nil) { UIImage *result = [[_cache synchronouslyFetchedCachedImageWithURL:_URL] asdk_image]; @@ -275,7 +279,7 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; // TODO: Consider removing this; it predates ASInterfaceState, which now ensures that even non-range-managed nodes get a -fetchData call. [self fetchData]; - if (self.image == nil && _downloaderImplementsSetPriority) { + if (self.image == nil && _downloaderFlags.downloaderImplementsSetPriority) { ASDN::MutexLocker l(_lock); if (_downloadIdentifier != nil) { [_downloader setPriority:ASImageDownloaderPriorityImminent withDownloadIdentifier:_downloadIdentifier]; @@ -289,7 +293,7 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; { [super visibleStateDidChange:isVisible]; - if (_downloaderImplementsSetPriority) { + if (_downloaderFlags.downloaderImplementsSetPriority) { _lock.lock(); if (_downloadIdentifier != nil) { if (isVisible) { @@ -314,7 +318,7 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; [self _cancelImageDownload]; [self _clearImage]; - if (_cacheSupportsClearing) { + if (_cacheFlags.cacheSupportsClearing) { [_cache clearFetchedImageFromCacheWithURL:_URL]; } } @@ -344,7 +348,7 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; ASInterfaceState interfaceState = self.interfaceState; ASDN::MutexLocker l(_lock); - if (!_downloaderImplementsSetProgress || _downloadIdentifier == nil) { + if (!_downloaderFlags.downloaderImplementsSetProgress || _downloadIdentifier == nil) { return; } @@ -411,7 +415,7 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; { ASPerformBlockOnBackgroundThread(^{ _lock.lock(); - if (_downloaderSupportsNewProtocol) { + if (_downloaderFlags.downloaderSupportsNewProtocol) { _downloadIdentifier = [_downloader downloadImageWithURL:_URL callbackQueue:dispatch_get_main_queue() downloadProgress:NULL @@ -447,7 +451,7 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; if (!_imageLoaded && _URL != nil && _downloadIdentifier == nil) { { ASDN::MutexLocker l(_lock); - if (_delegateSupportsDidStartFetchingData) { + if (_delegateFlags.delegateDidStartFetchingData) { [_delegate imageNodeDidStartFetchingData:self]; } } @@ -474,7 +478,7 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; // If the file may be an animated gif and then created an animated image. id animatedImage = nil; - if (_downloaderImplementsAnimatedImage) { + if (_downloaderFlags.downloaderImplementsAnimatedImage) { NSData *data = [NSData dataWithContentsOfURL:_URL]; animatedImage = [_downloader animatedImageWithData:data]; @@ -497,7 +501,7 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; dispatch_async(dispatch_get_main_queue(), ^{ self.currentImageQuality = 1.0; }); - if (_delegateSupportsDidLoadImage) { + if (_delegateFlags.delegateDidLoadImage) { [_delegate imageNode:self didLoadImage:self.image]; } }); @@ -519,7 +523,7 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; if (imageContainer != nil) { strongSelf->_imageLoaded = YES; - if ([imageContainer asdk_animatedImageData] && _downloaderImplementsAnimatedImage) { + if ([imageContainer asdk_animatedImageData] && _downloaderFlags.downloaderImplementsAnimatedImage) { strongSelf.animatedImage = [_downloader animatedImageWithData:[imageContainer asdk_animatedImageData]]; } else { strongSelf.image = [imageContainer asdk_image]; @@ -534,11 +538,11 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; strongSelf->_cacheUUID = nil; if (imageContainer != nil) { - if (strongSelf->_delegateSupportsDidLoadImage) { + if (strongSelf->_delegateFlags.delegateDidLoadImage) { [strongSelf->_delegate imageNode:strongSelf didLoadImage:strongSelf.image]; } } - else if (error && strongSelf->_delegateSupportsDidFailWithError) { + else if (error && strongSelf->_delegateFlags.delegateDidFailWithError) { [strongSelf->_delegate imageNode:strongSelf didFailWithError:error]; } }; @@ -560,7 +564,7 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; } }; - if (_cacheSupportsNewProtocol) { + if (_cacheFlags.cacheSupportsNewProtocol) { [_cache cachedImageWithURL:_URL callbackQueue:dispatch_get_main_queue() completion:cacheCompletion]; @@ -588,7 +592,7 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; [super displayDidFinish]; ASDN::MutexLocker l(_lock); - if (_delegateSupportsDidFinishDecoding && self.layer.contents != nil) { + if (_delegateFlags.delegateDidFinishDecoding && self.layer.contents != nil) { /* We store the image quality in _currentImageQuality whenever _image is set. On the following displayDidFinish, we'll know that _currentImageQuality is the quality of the image that has just finished rendering. In order for this to be accurate, we need to be sure we are on main thread when we set _currentImageQuality. Otherwise, it is possible for _currentImageQuality From d2d8b0a1cb2027bdcda171117ba5d6e811d02564 Mon Sep 17 00:00:00 2001 From: Scott Goodson Date: Fri, 1 Jul 2016 20:11:23 -0700 Subject: [PATCH 5/6] [ASDisplayNode] Adjust behavior of -removeFromSupernode to ensure "root" nodes are removed from their superview/superlayer. This situation is relatively uncommon. If a user manually uses -[UIView addSubnode:], the convenience category method, and then calls -[ASDisplayNode removeFromSuperview] -- we would bypass performing the actual removal as no supernode pointer is set. After further consideration, the special handling here to support divergence between the supernode pointer and the view / layer hierarchy is not something we need to maintain going forward, and removing it makes addressing this easy. --- AsyncDisplayKit/ASDisplayNode.mm | 21 +++++++-------------- AsyncDisplayKit/Details/_ASDisplayView.mm | 5 +++-- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/AsyncDisplayKit/ASDisplayNode.mm b/AsyncDisplayKit/ASDisplayNode.mm index cf38ddd804..32b1c80b6a 100644 --- a/AsyncDisplayKit/ASDisplayNode.mm +++ b/AsyncDisplayKit/ASDisplayNode.mm @@ -1543,27 +1543,20 @@ static NSInteger incrementIfFound(NSInteger i) { __weak UIView *view = _view; __weak CALayer *layer = _layer; BOOL layerBacked = _flags.layerBacked; + BOOL isNodeLoaded = (layer != nil || view != nil); _propertyLock.unlock(); - - if (supernode == nil) { - return; - } + // Clear supernode's reference to us before removing the view from the hierarchy, as _ASDisplayView + // will trigger us to clear our _supernode pointer in willMoveToSuperview:nil. + // This may result in removing the last strong reference, triggering deallocation after this method. [supernode _removeSubnode:self]; - if (self.nodeLoaded && supernode.nodeLoaded) { - // Check to ensure that our view or layer is actually inside of our supernode; otherwise, don't remove it. - // Though _ASDisplayView decouples the supernode if it is inserted inside another view hierarchy, this is - // more difficult to guarantee with _ASDisplayLayer because CoreAnimation doesn't have a -didMoveToSuperlayer. + if (isNodeLoaded && (supernode == nil || supernode.isNodeLoaded)) { ASPerformBlockOnMainThread(^{ if (layerBacked || supernode.layerBacked) { - if (layer.superlayer == supernode.layer) { - [layer removeFromSuperlayer]; - } + [layer removeFromSuperlayer]; } else { - if (view.superview == supernode.view) { - [view removeFromSuperview]; - } + [view removeFromSuperview]; } }); } diff --git a/AsyncDisplayKit/Details/_ASDisplayView.mm b/AsyncDisplayKit/Details/_ASDisplayView.mm index ba0de48e0f..2d4deb5259 100644 --- a/AsyncDisplayKit/Details/_ASDisplayView.mm +++ b/AsyncDisplayKit/Details/_ASDisplayView.mm @@ -86,8 +86,7 @@ UIView *currentSuperview = self.superview; if (!currentSuperview && newSuperview) { self.keepalive_node = _node; - } - else if (currentSuperview && !newSuperview) { + } else if (currentSuperview && !newSuperview) { // Clearing keepalive_node may cause deallocation of the node. In this case, __exitHierarchy may not have an opportunity (e.g. _node will be cleared // by the time -didMoveToWindow occurs after this) to clear the Visible interfaceState, which we need to do before deallocation to meet an API guarantee. if (_node.inHierarchy) { @@ -95,6 +94,8 @@ } self.keepalive_node = nil; } + + ASDisplayNodeAssert(self.keepalive_node == nil || newSuperview != nil, @"Keepalive reference should not exist if there is no superview."); if (newSuperview) { ASDisplayNode *supernode = _node.supernode; From 9be2f1db4e05cfbd058ff441f8adfe4da076b133 Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Wed, 29 Jun 2016 11:20:24 -0700 Subject: [PATCH 6/6] Prevent calling endUpdatesAnimated:completion: in an unbalanced way --- .../Details/ASChangeSetDataController.m | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/AsyncDisplayKit/Details/ASChangeSetDataController.m b/AsyncDisplayKit/Details/ASChangeSetDataController.m index f6615d1bdb..3a31b8ce82 100644 --- a/AsyncDisplayKit/Details/ASChangeSetDataController.m +++ b/AsyncDisplayKit/Details/ASChangeSetDataController.m @@ -18,22 +18,19 @@ #import "ASDataController+Subclasses.h" -@interface ASChangeSetDataController () - -@property (nonatomic, assign) NSUInteger changeSetBatchUpdateCounter; -@property (nonatomic, strong) _ASHierarchyChangeSet *changeSet; - -@end - -@implementation ASChangeSetDataController +@implementation ASChangeSetDataController { + NSInteger _changeSetBatchUpdateCounter; + _ASHierarchyChangeSet *_changeSet; +} #pragma mark - Batching (External API) - (void)beginUpdates { ASDisplayNodeAssertMainThread(); - if (_changeSetBatchUpdateCounter == 0) { + if (_changeSetBatchUpdateCounter <= 0) { _changeSet = [_ASHierarchyChangeSet new]; + _changeSetBatchUpdateCounter = 0; } _changeSetBatchUpdateCounter++; } @@ -43,6 +40,9 @@ ASDisplayNodeAssertMainThread(); _changeSetBatchUpdateCounter--; + // Prevent calling endUpdatesAnimated:completion: in an unbalanced way + NSAssert(_changeSetBatchUpdateCounter >= 0, @"endUpdatesAnimated:completion: called without having a balanced beginUpdates call"); + if (_changeSetBatchUpdateCounter == 0) { [_changeSet markCompleted];