Skip to content

Commit

Permalink
Revert Adds support for specifying a quality indexed array of URLs (T…
Browse files Browse the repository at this point in the history 
…extureGroup#699)

* Revert "Adds support for specifying a quality indexed array of URLs (TextureGroup#557)"

This reverts commit 3c77d4a.

* Add CHANGELOG entry
  • Loading branch information
@garrettmoon @bernieperez
garrettmoon authored and bernieperez committed Apr 25, 2018
1 parent 250f7ab commit 0d817d1
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 176 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## master
* Add your own contributions to the next release on the line below this with your name.
- [ASNetworkImageNode] Deprecates .URLs in favor of .URL [Garrett Moon](https://github.com/garrettmoon) [#699](https://github.com/TextureGroup/Texture/pull/699)
- [iOS11] Update project settings and fix errors [Eke](https://github.com/Eke) [#676](https://github.com/TextureGroup/Texture/pull/676)
- [ASCollectionView] Improve performance and behavior of rotation / bounds changes. [Scott Goodson](https://github.com/appleguy) [#431](https://github.com/TextureGroup/Texture/pull/431)
- [ASCollectionView] Improve index space translation of Flow Layout Delegate methods. [Scott Goodson](https://github.com/appleguy)
Expand Down
17 changes: 10 additions & 7 deletions Source/ASNetworkImageNode.h
View file
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
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

0 comments on commit 0d817d1

Please sign in to comment.