Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revert Adds support for specifying a quality indexed array of URLs #699

Merged
merged 2 commits into from
Dec 5, 2017
Merged

Revert Adds support for specifying a quality indexed array of URLs #699

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e069bcc
Revert "Adds support for specifying a quality indexed array of URLs (…
garrettmoon Dec 5, 2017
5db884e
Add CHANGELOG entry
garrettmoon Dec 5, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions Source/ASNetworkImageNode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,16 @@ NS_ASSUME_NONNULL_BEGIN
@property (nullable, nonatomic, strong, readwrite) NSURL *URL;

/**
* An array of URLs of increasing cost to download.
*
* @discussion By setting an array of URLs, the image property of this node will be managed internally. This means previously
* directly set images to the image property will be cleared out and replaced by the placeholder (<defaultImage>) image
* while loading and the final image after the new image data was downloaded and processed.
*/
@property (nullable, nonatomic, strong, readwrite) NSArray <NSURL *> *URLs;
* An array of URLs of increasing cost to download.
*
* @discussion By setting an array of URLs, the image property of this node will be managed internally. This means previously
* directly set images to the image property will be cleared out and replaced by the placeholder (<defaultImage>) image
* while loading and the final image after the new image data was downloaded and processed.
*
* @deprecated This API has been removed for now due to the increased complexity to the class that it brought.
* Please use .URL instead.
*/
@property (nullable, nonatomic, strong, readwrite) NSArray <NSURL *> *URLs ASDISPLAYNODE_DEPRECATED_MSG("Please use URL instead.");

/**
* Download and display a new image.
Expand Down
172 changes: 66 additions & 106 deletions Source/ASNetworkImageNode.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ @interface ASNetworkImageNode ()
// Only access any of these with __instanceLock__.
__weak id<ASNetworkImageNodeDelegate> _delegate;

NSArray *_URLs;
NSURL *_URL;
UIImage *_defaultImage;

NSUUID *_cacheUUID;
Expand Down Expand Up @@ -67,15 +67,13 @@ @interface ASNetworkImageNode ()
unsigned int downloaderImplementsSetPriority:1;
unsigned int downloaderImplementsAnimatedImage:1;
unsigned int downloaderImplementsCancelWithResume:1;
unsigned int downloaderImplementsDownloadURLs:1;
} _downloaderFlags;

// Immutable and set on init only. We don't need to lock in this case.
__weak id<ASImageCacheProtocol> _cache;
struct {
unsigned int cacheSupportsClearing:1;
unsigned int cacheSupportsSynchronousFetch:1;
unsigned int cacheSupportsCachedURLs:1;
} _cacheFlags;
}

Expand All @@ -97,11 +95,9 @@ - (instancetype)initWithCache:(id<ASImageCacheProtocol>)cache downloader:(id<ASI
_downloaderFlags.downloaderImplementsSetPriority = [downloader respondsToSelector:@selector(setPriority:withDownloadIdentifier:)];
_downloaderFlags.downloaderImplementsAnimatedImage = [downloader respondsToSelector:@selector(animatedImageWithData:)];
_downloaderFlags.downloaderImplementsCancelWithResume = [downloader respondsToSelector:@selector(cancelImageDownloadWithResumePossibilityForIdentifier:)];
_downloaderFlags.downloaderImplementsDownloadURLs = [downloader respondsToSelector:@selector(downloadImageWithURLs:callbackQueue:downloadProgress:completion:)];

_cacheFlags.cacheSupportsClearing = [cache respondsToSelector:@selector(clearFetchedImageFromCacheWithURL:)];
_cacheFlags.cacheSupportsSynchronousFetch = [cache respondsToSelector:@selector(synchronouslyFetchedCachedImageWithURL:)];
_cacheFlags.cacheSupportsCachedURLs = [cache respondsToSelector:@selector(cachedImageWithURLs:callbackQueue:completion:)];

_shouldCacheImage = YES;
_shouldRenderProgressImages = YES;
Expand Down Expand Up @@ -139,8 +135,8 @@ - (void)_locked_setImage:(UIImage *)image
BOOL shouldCancelAndClear = imageWasSetExternally && (imageWasSetExternally != _imageWasSetExternally);
_imageWasSetExternally = imageWasSetExternally;
if (shouldCancelAndClear) {
ASDisplayNodeAssert(_URLs == nil || _URLs.count == 0, @"Directly setting an image on an ASNetworkImageNode causes it to behave like an ASImageNode instead of an ASNetworkImageNode. If this is what you want, set the URL to nil first.");
_URLs = nil;
ASDisplayNodeAssertNil(_URL, @"Directly setting an image on an ASNetworkImageNode causes it to behave like an ASImageNode instead of an ASNetworkImageNode. If this is what you want, set the URL to nil first.");
_URL = nil;
[self _locked_cancelDownloadAndClearImageWithResumePossibility:NO];
}

Expand All @@ -159,40 +155,29 @@ - (void)_locked__setImage:(UIImage *)image
[super _locked_setImage:image];
}

- (void)setURL:(NSURL *)URL
{
if (URL) {
[self setURLs:@[URL]];
} else {
[self setURLs:nil];
}
}

- (void)setURL:(NSURL *)URL resetToDefault:(BOOL)reset
// Deprecated
- (void)setURLs:(NSArray<NSURL *> *)URLs
{
if (URL) {
[self setURLs:@[URL] resetToDefault:reset];
} else {
[self setURLs:nil resetToDefault:reset];
}
[self setURL:[URLs firstObject]];
}

- (NSURL *)URL
// Deprecated
- (NSArray<NSURL *> *)URLs
{
return [self.URLs lastObject];
return @[self.URL];
}

- (void)setURLs:(NSArray <NSURL *> *)URLs
- (void)setURL:(NSURL *)URL
{
[self setURLs:URLs resetToDefault:YES];
[self setURL:URL resetToDefault:YES];
}

- (void)setURLs:(NSArray <NSURL *> *)URLs resetToDefault:(BOOL)reset
- (void)setURL:(NSURL *)URL resetToDefault:(BOOL)reset
{
{
ASDN::MutexLocker l(__instanceLock__);

if (ASObjectIsEqual(URLs, _URLs)) {
if (ASObjectIsEqual(URL, _URL)) {
return;
}

Expand All @@ -201,25 +186,25 @@ - (void)setURLs:(NSArray <NSURL *> *)URLs resetToDefault:(BOOL)reset
_imageWasSetExternally = NO;

[self _locked_cancelImageDownloadWithResumePossibility:NO];

_imageLoaded = NO;

_URLs = URLs;
_URL = URL;

BOOL hasURL = (_URLs.count == 0);
BOOL hasURL = (_URL == nil);
if (reset || hasURL) {
[self _locked_setCurrentImageQuality:(hasURL ? 0.0 : 1.0)];
[self _locked__setImage:_defaultImage];
}
}

[self setNeedsPreload];
}

- (NSArray <NSURL *>*)URLs
- (NSURL *)URL
{
ASDN::MutexLocker l(__instanceLock__);
return _URLs;
return _URL;
}

- (void)setDefaultImage:(UIImage *)defaultImage
Expand All @@ -238,7 +223,7 @@ - (void)_locked_setDefaultImage:(UIImage *)defaultImage
_defaultImage = defaultImage;

if (!_imageLoaded) {
[self _locked_setCurrentImageQuality:((_URLs.count == 0) ? 0.0 : 1.0)];
[self _locked_setCurrentImageQuality:((_URL == nil) ? 0.0 : 1.0)];
[self _locked__setImage:defaultImage];

}
Expand Down Expand Up @@ -337,7 +322,7 @@ - (BOOL)shouldRenderProgressImages
- (BOOL)placeholderShouldPersist
{
ASDN::MutexLocker l(__instanceLock__);
return (self.image == nil && self.animatedImage == nil && _URLs.count != 0);
return (self.image == nil && self.animatedImage == nil && _URL != nil);
}

/* displayWillStartAsynchronously: in ASMultiplexImageNode has a very similar implementation. Changes here are likely necessary
Expand All @@ -349,25 +334,22 @@ - (void)displayWillStartAsynchronously:(BOOL)asynchronously
if (asynchronously == NO && _cacheFlags.cacheSupportsSynchronousFetch) {
ASDN::MutexLocker l(__instanceLock__);

if (_imageLoaded == NO && _URLs.count > 0 && _downloadIdentifier == nil) {
for (NSURL *url in [_URLs reverseObjectEnumerator]) {
UIImage *result = [[_cache synchronouslyFetchedCachedImageWithURL:url] asdk_image];
if (result) {
[self _locked_setCurrentImageQuality:1.0];
[self _locked__setImage:result];
_imageLoaded = YES;

// Call out to the delegate.
if (_delegateFlags.delegateDidLoadImageWithInfo) {
ASDN::MutexUnlocker l(__instanceLock__);
ASNetworkImageNodeDidLoadInfo info = {};
info.imageSource = ASNetworkImageSourceSynchronousCache;
[_delegate imageNode:self didLoadImage:result info:info];
} else if (_delegateFlags.delegateDidLoadImage) {
ASDN::MutexUnlocker l(__instanceLock__);
[_delegate imageNode:self didLoadImage:result];
}
break;
if (_imageLoaded == NO && _URL && _downloadIdentifier == nil) {
UIImage *result = [[_cache synchronouslyFetchedCachedImageWithURL:_URL] asdk_image];
if (result) {
[self _locked_setCurrentImageQuality:1.0];
[self _locked__setImage:result];
_imageLoaded = YES;

// Call out to the delegate.
if (_delegateFlags.delegateDidLoadImageWithInfo) {
ASDN::MutexUnlocker l(__instanceLock__);
ASNetworkImageNodeDidLoadInfo info = {};
info.imageSource = ASNetworkImageSourceSynchronousCache;
[_delegate imageNode:self didLoadImage:result info:info];
} else if (_delegateFlags.delegateDidLoadImage) {
ASDN::MutexUnlocker l(__instanceLock__);
[_delegate imageNode:self didLoadImage:result];
}
}
}
Expand Down Expand Up @@ -538,11 +520,9 @@ - (void)_locked_cancelDownloadAndClearImageWithResumePossibility:(BOOL)storeResu
_imageLoaded = NO;

if (_cacheFlags.cacheSupportsClearing) {
if (_URLs.count != 0) {
as_log_verbose(ASImageLoadingLog(), "Clearing cached image for %@ url: %@", self, _URLs);
for (NSURL *url in _URLs) {
[_cache clearFetchedImageFromCacheWithURL:url];
}
if (_URL != nil) {
as_log_verbose(ASImageLoadingLog(), "Clearing cached image for %@ url: %@", self, _URL);
[_cache clearFetchedImageFromCacheWithURL:_URL];
}
}
}
Expand Down Expand Up @@ -576,7 +556,7 @@ - (void)_locked_cancelImageDownloadWithResumePossibility:(BOOL)storeResume
- (void)_downloadImageWithCompletion:(void (^)(id <ASImageContainerProtocol> imageContainer, NSError*, id downloadIdentifier))finished
{
ASPerformBlockOnBackgroundThread(^{
NSArray <NSURL *> *urls;
NSURL *url;
id downloadIdentifier;
BOOL cancelAndReattempt = NO;

Expand All @@ -585,34 +565,23 @@ - (void)_downloadImageWithCompletion:(void (^)(id <ASImageContainerProtocol> ima
// it and try again.
{
ASDN::MutexLocker l(__instanceLock__);
urls = _URLs;
url = _URL;
}

if (_downloaderFlags.downloaderImplementsDownloadURLs) {
downloadIdentifier = [_downloader downloadImageWithURLs:urls
callbackQueue:dispatch_get_main_queue()
downloadProgress:NULL
completion:^(id <ASImageContainerProtocol> _Nullable imageContainer, NSError * _Nullable error, id _Nullable downloadIdentifier) {
if (finished != NULL) {
finished(imageContainer, error, downloadIdentifier);
}
}];
} else {
downloadIdentifier = [_downloader downloadImageWithURL:[urls lastObject]
callbackQueue:dispatch_get_main_queue()
downloadProgress:NULL
completion:^(id <ASImageContainerProtocol> _Nullable imageContainer, NSError * _Nullable error, id _Nullable downloadIdentifier) {
if (finished != NULL) {
finished(imageContainer, error, downloadIdentifier);
}
}];
}


downloadIdentifier = [_downloader downloadImageWithURL:url
callbackQueue:dispatch_get_main_queue()
downloadProgress:NULL
completion:^(id <ASImageContainerProtocol> _Nullable imageContainer, NSError * _Nullable error, id _Nullable downloadIdentifier) {
if (finished != NULL) {
finished(imageContainer, error, downloadIdentifier);
}
}];
as_log_verbose(ASImageLoadingLog(), "Downloading image for %@ url: %@", self, url);

{
ASDN::MutexLocker l(__instanceLock__);
if (ASObjectIsEqual(_URLs, urls)) {
if (ASObjectIsEqual(_URL, url)) {
// The download we kicked off is correct, no need to do any more work.
_downloadIdentifier = downloadIdentifier;
} else {
Expand Down Expand Up @@ -641,36 +610,34 @@ - (void)_lazilyLoadImageIfNecessary
__weak id<ASNetworkImageNodeDelegate> delegate = _delegate;
BOOL delegateDidStartFetchingData = _delegateFlags.delegateDidStartFetchingData;
BOOL isImageLoaded = _imageLoaded;
NSArray <NSURL *>*URLs = _URLs;
NSURL *URL = _URL;
id currentDownloadIdentifier = _downloadIdentifier;
__instanceLock__.unlock();

if (!isImageLoaded && URLs.count > 0 && currentDownloadIdentifier == nil) {
if (!isImageLoaded && URL != nil && currentDownloadIdentifier == nil) {
if (delegateDidStartFetchingData) {
[delegate imageNodeDidStartFetchingData:self];
}

// We only support file URLs if there is one URL currently
if (URLs.count == 1 && [URLs lastObject].isFileURL) {
if (URL.isFileURL) {
dispatch_async(dispatch_get_main_queue(), ^{
ASDN::MutexLocker l(__instanceLock__);

// Bail out if not the same URL anymore
if (!ASObjectIsEqual(URLs, _URLs)) {
if (!ASObjectIsEqual(URL, _URL)) {
return;
}

NSURL *URL = [URLs lastObject];
if (_shouldCacheImage) {
[self _locked__setImage:[UIImage imageNamed:URL.path.lastPathComponent]];
[self _locked__setImage:[UIImage imageNamed:_URL.path.lastPathComponent]];
} else {
// First try to load the path directly, for efficiency assuming a developer who
// doesn't want caching is trying to be as minimal as possible.
UIImage *nonAnimatedImage = [UIImage imageWithContentsOfFile:URL.path];
UIImage *nonAnimatedImage = [UIImage imageWithContentsOfFile:_URL.path];
if (nonAnimatedImage == nil) {
// If we couldn't find it, execute an -imageNamed:-like search so we can find resources even if the
// extension is not provided in the path. This allows the same path to work regardless of shouldCacheImage.
NSString *filename = [[NSBundle mainBundle] pathForResource:URL.path.lastPathComponent ofType:nil];
NSString *filename = [[NSBundle mainBundle] pathForResource:_URL.path.lastPathComponent ofType:nil];
if (filename != nil) {
nonAnimatedImage = [UIImage imageWithContentsOfFile:filename];
}
Expand All @@ -679,7 +646,7 @@ - (void)_lazilyLoadImageIfNecessary
// If the file may be an animated gif and then created an animated image.
id<ASAnimatedImageProtocol> animatedImage = nil;
if (_downloaderFlags.downloaderImplementsAnimatedImage) {
NSData *data = [NSData dataWithContentsOfURL:URL];
NSData *data = [NSData dataWithContentsOfURL:_URL];
if (data != nil) {
animatedImage = [_downloader animatedImageWithData:data];

Expand Down Expand Up @@ -719,7 +686,7 @@ - (void)_lazilyLoadImageIfNecessary
return;
}

as_log_verbose(ASImageLoadingLog(), "Downloaded image for %@ img: %@ urls: %@", self, [imageContainer asdk_image], URLs);
as_log_verbose(ASImageLoadingLog(), "Downloaded image for %@ img: %@ url: %@", self, [imageContainer asdk_image], URL);

// Grab the lock for the rest of the block
ASDN::MutexLocker l(strongSelf->__instanceLock__);
Expand Down Expand Up @@ -784,7 +751,7 @@ - (void)_lazilyLoadImageIfNecessary
_cacheUUID = cacheUUID;
__instanceLock__.unlock();

as_log_verbose(ASImageLoadingLog(), "Decaching image for %@ urls: %@", self, URLs);
as_log_verbose(ASImageLoadingLog(), "Decaching image for %@ url: %@", self, URL);

ASImageCacherCompletion completion = ^(id <ASImageContainerProtocol> imageContainer) {
// If the cache UUID changed, that means this request was cancelled.
Expand All @@ -801,20 +768,13 @@ - (void)_lazilyLoadImageIfNecessary
finished(imageContainer, error, downloadIdentifier, ASNetworkImageSourceDownload);
}];
} else {
as_log_verbose(ASImageLoadingLog(), "Decached image for %@ img: %@ urls: %@", self, [imageContainer asdk_image], URLs);
as_log_verbose(ASImageLoadingLog(), "Decached image for %@ img: %@ url: %@", self, [imageContainer asdk_image], URL);
finished(imageContainer, nil, nil, ASNetworkImageSourceAsynchronousCache);
}
};

if (_cacheFlags.cacheSupportsCachedURLs) {
[_cache cachedImageWithURLs:URLs
callbackQueue:dispatch_get_main_queue()
completion:completion];
} else {
[_cache cachedImageWithURL:[URLs lastObject]
callbackQueue:dispatch_get_main_queue()
completion:completion];
}
[_cache cachedImageWithURL:URL
callbackQueue:dispatch_get_main_queue()
completion:completion];
} else {
[self _downloadImageWithCompletion:^(id<ASImageContainerProtocol> imageContainer, NSError *error, id downloadIdentifier) {
finished(imageContainer, error, downloadIdentifier, ASNetworkImageSourceDownload);
Expand Down
Loading