Skip to content

Commit 48fc481

Browse files
garrettmoongarrettmoon
committed
Addressing comments
1 parent 820390e commit 48fc481

6 files changed

Lines changed: 200 additions & 148 deletions

File tree

‎AsyncDisplayKit.podspec‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ Pod::Spec.new do |spec|
5151

5252
spec.subspec 'PINRemoteImage' do |pin|
5353
pin.xcconfig = { 'GCC_PREPROCESSOR_DEFINITIONS' => '$(inherited) PIN_REMOTE_IMAGE=1' }
54-
pin.dependency 'PINRemoteImage', '>= 2'
54+
pin.dependency 'PINRemoteImage/iOS', '>= 2'
5555
pin.dependency 'AsyncDisplayKit/ASDealloc2MainObject'
5656
end
5757

‎AsyncDisplayKit/ASMultiplexImageNode.mm‎

Lines changed: 66 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ @interface ASMultiplexImageNode ()
4848
{
4949
@private
5050
// Core.
51-
id<ASImageCacheProtocol> _cache;
52-
id<ASImageDownloaderProtocol> _downloader;
51+
id<ASImageCacheProtocol, ASImageCacheProtocolDeprecated> _cache;
52+
id<ASImageDownloaderProtocol, ASImageDownloaderProtocolDeprecated> _downloader;
5353

5454
__weak id<ASMultiplexImageNodeDelegate> _delegate;
5555
struct {
@@ -85,7 +85,8 @@ @interface ASMultiplexImageNode ()
8585
BOOL _downloaderSupportsNewProtocol;
8686
BOOL _downloaderImplementsSetProgress;
8787
BOOL _downloaderImplementsSetPriority;
88-
BOOL _cacherSupportsNewProtocol;
88+
BOOL _cacheSupportsNewProtocol;
89+
BOOL _cacheSupportsClearing;
8990
}
9091

9192
//! @abstract Read-write redeclaration of property declared in ASMultiplexImageNode.h.
@@ -156,13 +157,6 @@ - (void)_loadPHAssetWithRequest:(ASPhotosFrameworkImageRequest *)request identif
156157
*/
157158
- (void)_downloadImageWithIdentifier:(id)imageIdentifier URL:(NSURL *)imageURL completion:(void (^)(UIImage *image, NSError *error))completionBlock;
158159

159-
/**
160-
@abstract Returns a Boolean value indicating whether the downloaded image should be removed when clearing fetched data
161-
@discussion Downloaded image data should only be cleared out if a cache is present
162-
@return YES if an image cache is available; otherwise, NO.
163-
*/
164-
- (BOOL)_shouldClearFetchedImageData;
165-
166160
@end
167161

168162
@implementation ASMultiplexImageNode
@@ -176,16 +170,17 @@ - (instancetype)initWithCache:(id<ASImageCacheProtocol>)cache downloader:(id<ASI
176170
_cache = cache;
177171
_downloader = downloader;
178172

179-
NSAssert([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:.");
173+
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:.");
180174

181-
_downloaderSupportsNewProtocol = [downloader respondsToSelector:@selector(downloadImageWithURL:callbackQueue:downloadProgress:completion:)] ? YES : NO;
175+
_downloaderSupportsNewProtocol = [downloader respondsToSelector:@selector(downloadImageWithURL:callbackQueue:downloadProgress:completion:)];
182176

183-
NSAssert(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:");
177+
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:");
184178

185-
_downloaderImplementsSetProgress = [downloader respondsToSelector:@selector(setProgressImageBlock:callbackQueue:withDownloadIdentifier:)] ? YES : NO;
186-
_downloaderImplementsSetPriority = [downloader respondsToSelector:@selector(setPriority:withDownloadIdentifier:)] ? YES : NO;
179+
_downloaderImplementsSetProgress = [downloader respondsToSelector:@selector(setProgressImageBlock:callbackQueue:withDownloadIdentifier:)];
180+
_downloaderImplementsSetPriority = [downloader respondsToSelector:@selector(setPriority:withDownloadIdentifier:)];
187181

188-
_cacherSupportsNewProtocol = [cache respondsToSelector:@selector(cachedImageWithURL:callbackQueue:completion:)] ? YES : NO;
182+
_cacheSupportsNewProtocol = [cache respondsToSelector:@selector(cachedImageWithURL:callbackQueue:completion:)];
183+
_cacheSupportsClearing = [cache respondsToSelector:@selector(clearFetchedImageFromCacheWithURL:)];
189184

190185
self.shouldBypassEnsureDisplay = YES;
191186

@@ -220,16 +215,17 @@ - (void)clearFetchedData
220215
{
221216
[super clearFetchedData];
222217

223-
if ([self _shouldClearFetchedImageData]) {
224-
225-
[_phImageRequestOperation cancel];
226-
227-
[self _setDownloadIdentifier:nil];
218+
[_phImageRequestOperation cancel];
228219

229-
// setting this to nil makes the node fetch images the next time its display starts
230-
_loadedImageIdentifier = nil;
231-
self.image = nil;
220+
[self _setDownloadIdentifier:nil];
221+
222+
if (_cacheSupportsClearing) {
223+
[_cache clearFetchedImageFromCacheWithURL:[_dataSource multiplexImageNode:self URLForImageIdentifier:self.loadedImageIdentifier]];
232224
}
225+
226+
// setting this to nil makes the node fetch images the next time its display starts
227+
_loadedImageIdentifier = nil;
228+
self.image = nil;
233229
}
234230

235231
- (void)fetchData
@@ -266,6 +262,8 @@ - (void)displayDidFinish
266262
}
267263
}
268264

265+
/* displayWillStart in ASNetworkImageNode has a very similar implementation. Changes here are likely necessary
266+
in ASNetworkImageNode as well. */
269267
- (void)displayWillStart
270268
{
271269
[super displayWillStart];
@@ -276,36 +274,55 @@ - (void)displayWillStart
276274
{
277275
ASDN::MutexLocker l(_downloadIdentifierLock);
278276
if (_downloadIdentifier != nil) {
279-
[_downloader setPriority:ASImageDownloaderPriorityDisplay withDownloadIdentifier:_downloadIdentifier];
277+
[_downloader setPriority:ASImageDownloaderPriorityImminent withDownloadIdentifier:_downloadIdentifier];
280278
}
281279
}
282280
}
281+
}
282+
283+
/* visibilityDidChange in ASNetworkImageNode has a very similar implementation. Changes here are likely necessary
284+
in ASNetworkImageNode as well. */
285+
- (void)visibilityDidChange:(BOOL)isVisible
286+
{
287+
[super visibilityDidChange:isVisible];
283288

284-
if (self.image == nil) {
285-
if (_downloaderImplementsSetProgress) {
286-
{
287-
ASDN::MutexLocker l(_downloadIdentifierLock);
288-
289-
if (_downloadIdentifier != nil) {
290-
__weak __typeof__(self) weakSelf = self;
291-
[_downloader setProgressImageBlock:^(UIImage * _Nonnull progressImage, id _Nullable downloadIdentifier) {
292-
__typeof__(self) strongSelf = weakSelf;
293-
if (strongSelf == nil) {
294-
return;
295-
}
296-
297-
ASDN::MutexLocker l(strongSelf->_downloadIdentifierLock);
298-
//Getting a result back for a different download identifier, download must not have been successfully canceled
299-
if (![strongSelf->_downloadIdentifier isEqual:downloadIdentifier] && downloadIdentifier != nil) {
300-
return;
301-
}
302-
303-
strongSelf.image = progressImage;
304-
} callbackQueue:dispatch_get_main_queue() withDownloadIdentifier:_downloadIdentifier];
305-
}
289+
if (_downloaderImplementsSetPriority) {
290+
ASDN::MutexLocker l(_downloadIdentifierLock);
291+
if (_downloadIdentifier != nil) {
292+
if (isVisible) {
293+
[_downloader setPriority:ASImageDownloaderPriorityVisible withDownloadIdentifier:_downloadIdentifier];
294+
} else {
295+
[_downloader setPriority:ASImageDownloaderPriorityPreload withDownloadIdentifier:_downloadIdentifier];
306296
}
307297
}
308298
}
299+
300+
if (_downloaderImplementsSetProgress) {
301+
ASDN::MutexLocker l(_downloadIdentifierLock);
302+
303+
if (_downloadIdentifier != nil) {
304+
__weak __typeof__(self) weakSelf = self;
305+
ASImageDownloaderProgressImage progress = nil;
306+
if (isVisible) {
307+
progress = ^(UIImage * _Nonnull progressImage, id _Nullable downloadIdentifier) {
308+
__typeof__(self) strongSelf = weakSelf;
309+
if (strongSelf == nil) {
310+
return;
311+
}
312+
313+
ASDN::MutexLocker l(strongSelf->_downloadIdentifierLock);
314+
//Getting a result back for a different download identifier, download must not have been successfully canceled
315+
if (ASObjectIsEqual(strongSelf->_downloadIdentifier, downloadIdentifier) == NO && downloadIdentifier != nil) {
316+
return;
317+
}
318+
319+
strongSelf.image = progressImage;
320+
};
321+
}
322+
323+
[_downloader setProgressImageBlock:progress callbackQueue:dispatch_get_main_queue() withDownloadIdentifier:_downloadIdentifier];
324+
}
325+
}
309326
}
310327

311328
#pragma mark - Core
@@ -695,7 +712,7 @@ - (void)_fetchImageWithIdentifierFromCache:(id)imageIdentifier URL:(NSURL *)imag
695712
ASDisplayNodeAssertNotNil(completionBlock, @"completionBlock is required");
696713

697714
if (_cache) {
698-
if (_cacherSupportsNewProtocol) {
715+
if (_cacheSupportsNewProtocol) {
699716
[_cache cachedImageWithURL:imageURL callbackQueue:dispatch_get_main_queue() completion:^(UIImage *imageFromCache) {
700717
completionBlock(imageFromCache);
701718
}];
@@ -746,7 +763,7 @@ - (void)_downloadImageWithIdentifier:(id)imageIdentifier URL:(NSURL *)imageURL c
746763

747764
ASDN::MutexLocker l(_downloadIdentifierLock);
748765
//Getting a result back for a different download identifier, download must not have been successfully canceled
749-
if (![_downloadIdentifier isEqual:downloadIdentifier] && downloadIdentifier != nil) {
766+
if (ASObjectIsEqual(_downloadIdentifier, downloadIdentifier) == NO && downloadIdentifier != nil) {
750767
return;
751768
}
752769

@@ -811,10 +828,6 @@ - (void)_finishedLoadingImage:(UIImage *)image forIdentifier:(id)imageIdentifier
811828
[self _loadNextImage];
812829
}
813830

814-
- (BOOL)_shouldClearFetchedImageData {
815-
return _cache != nil;
816-
}
817-
818831
@end
819832
#if TARGET_OS_IOS
820833
@implementation NSURL (ASPhotosFrameworkURLs)

‎AsyncDisplayKit/ASNetworkImageNode.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,19 +45,19 @@ NS_ASSUME_NONNULL_BEGIN
4545
/**
4646
* The delegate, which must conform to the <ASNetworkImageNodeDelegate> protocol.
4747
*/
48-
@property (nonatomic, weak, readwrite) id<ASNetworkImageNodeDelegate> delegate;
48+
@property (atomic, weak, readwrite) id<ASNetworkImageNodeDelegate> delegate;
4949

5050
/**
5151
* A placeholder image to display while the URL is loading.
5252
*/
53-
@property (nullable, nonatomic, strong, readwrite) UIImage *defaultImage;
53+
@property (nullable, atomic, strong, readwrite) UIImage *defaultImage;
5454

5555
/**
5656
* The URL of a new image to download and display.
5757
*
5858
* @discussion Changing this property will reset the displayed image to a placeholder (<defaultImage>) while loading.
5959
*/
60-
@property (nullable, nonatomic, strong, readwrite) NSURL *URL;
60+
@property (nullable, atomic, strong, readwrite) NSURL *URL;
6161

6262
/**
6363
* Download and display a new image.

0 commit comments

Comments
 (0)