From 2e7d526a6987ac5e94f35161c73e613ae7d4df50 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Thu, 4 Mar 2021 18:42:11 +0800 Subject: [PATCH 1/3] Performance enhancement: Allows to use progressive decoding image to callback the completion when finished. Which can reduce the decoding pressure and animation loading delay --- SDWebImage/Core/SDImageLoader.h | 13 +++++++++++++ SDWebImage/Core/SDImageLoader.m | 18 ++++++++++++++---- .../Core/SDWebImageDownloaderOperation.m | 11 ++++++++++- 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/SDWebImage/Core/SDImageLoader.h b/SDWebImage/Core/SDImageLoader.h index 25f1af40..addad836 100644 --- a/SDWebImage/Core/SDImageLoader.h +++ b/SDWebImage/Core/SDImageLoader.h @@ -9,6 +9,7 @@ #import "SDWebImageCompat.h" #import "SDWebImageDefine.h" #import "SDWebImageOperation.h" +#import "SDImageCoder.h" typedef void(^SDImageLoaderProgressBlock)(NSInteger receivedSize, NSInteger expectedSize, NSURL * _Nullable targetURL); typedef void(^SDImageLoaderCompletedBlock)(UIImage * _Nullable image, NSData * _Nullable data, NSError * _Nullable error, BOOL finished); @@ -50,6 +51,18 @@ FOUNDATION_EXPORT UIImage * _Nullable SDImageLoaderDecodeImageData(NSData * _Non */ FOUNDATION_EXPORT UIImage * _Nullable SDImageLoaderDecodeProgressiveImageData(NSData * _Nonnull imageData, NSURL * _Nonnull imageURL, BOOL finished, id _Nonnull operation, SDWebImageOptions options, SDWebImageContext * _Nullable context); +/** + This function get the progressive decoder for current loading operation. If no progressive decoding is happended or decoder is not able to construct, return nil. + @return The progressive decoder associated with the loading operation. + */ +FOUNDATION_EXPORT id _Nullable SDImageLoaderGetProgressiveCoder(id _Nonnull operation); + +/** + This function set the progressive decoder for current loading operation. If no progressive decoding is happended, pass nil. + @param operation The loading operation to associate the progerssive decoder. + */ +FOUNDATION_EXPORT void SDImageLoaderSetProgressiveCoder(id _Nonnull operation, id _Nullable progressiveCoder); + #pragma mark - SDImageLoader /** diff --git a/SDWebImage/Core/SDImageLoader.m b/SDWebImage/Core/SDImageLoader.m index c529954e..7b565bff 100644 --- a/SDWebImage/Core/SDImageLoader.m +++ b/SDWebImage/Core/SDImageLoader.m @@ -15,8 +15,20 @@ #import "SDInternalMacros.h" #import "objc/runtime.h" +SDWebImageContextOption const SDWebImageContextLoaderCachedImage = @"loaderCachedImage"; + static void * SDImageLoaderProgressiveCoderKey = &SDImageLoaderProgressiveCoderKey; +id SDImageLoaderGetProgressiveCoder(id operation) { + NSCParameterAssert(operation); + return objc_getAssociatedObject(operation, SDImageLoaderProgressiveCoderKey); +} + +void SDImageLoaderSetProgressiveCoder(id operation, id progressiveCoder) { + NSCParameterAssert(operation); + objc_setAssociatedObject(operation, SDImageLoaderProgressiveCoderKey, progressiveCoder, OBJC_ASSOCIATION_RETAIN_NONATOMIC); +} + UIImage * _Nullable SDImageLoaderDecodeImageData(NSData * _Nonnull imageData, NSURL * _Nonnull imageURL, SDWebImageOptions options, SDWebImageContext * _Nullable context) { NSCParameterAssert(imageData); NSCParameterAssert(imageURL); @@ -136,7 +148,7 @@ UIImage * _Nullable SDImageLoaderDecodeProgressiveImageData(NSData * _Nonnull im SDImageCoderOptions *coderOptions = [mutableCoderOptions copy]; // Grab the progressive image coder - id progressiveCoder = objc_getAssociatedObject(operation, SDImageLoaderProgressiveCoderKey); + id progressiveCoder = SDImageLoaderGetProgressiveCoder(operation); if (!progressiveCoder) { id imageCoder = context[SDWebImageContextImageCoder]; // Check the progressive coder if provided @@ -152,7 +164,7 @@ UIImage * _Nullable SDImageLoaderDecodeProgressiveImageData(NSData * _Nonnull im } } } - objc_setAssociatedObject(operation, SDImageLoaderProgressiveCoderKey, progressiveCoder, OBJC_ASSOCIATION_RETAIN_NONATOMIC); + SDImageLoaderSetProgressiveCoder(operation, progressiveCoder); } // If we can't find any progressive coder, disable progressive download if (!progressiveCoder) { @@ -196,5 +208,3 @@ UIImage * _Nullable SDImageLoaderDecodeProgressiveImageData(NSData * _Nonnull im return image; } - -SDWebImageContextOption const SDWebImageContextLoaderCachedImage = @"loaderCachedImage"; diff --git a/SDWebImage/Core/SDWebImageDownloaderOperation.m b/SDWebImage/Core/SDWebImageDownloaderOperation.m index 0c21a530..92d7ad9b 100644 --- a/SDWebImage/Core/SDWebImageDownloaderOperation.m +++ b/SDWebImage/Core/SDWebImageDownloaderOperation.m @@ -11,6 +11,7 @@ #import "SDInternalMacros.h" #import "SDWebImageDownloaderResponseModifier.h" #import "SDWebImageDownloaderDecryptor.h" +#import "SDAnimatedImage.h" static NSString *const kProgressCallbackKey = @"progress"; static NSString *const kCompletedCallbackKey = @"completed"; @@ -262,6 +263,7 @@ typedef NSMutableDictionary SDCallbacksDictionary; - (void)done { self.finished = YES; self.executing = NO; + SDImageLoaderSetProgressiveCoder(self, nil); [self reset]; } @@ -471,7 +473,14 @@ didReceiveResponse:(NSURLResponse *)response // decode the image in coder queue, cancel all previous decoding process [self.coderQueue cancelAllOperations]; [self.coderQueue addOperationWithBlock:^{ - UIImage *image = SDImageLoaderDecodeImageData(imageData, self.request.URL, [[self class] imageOptionsFromDownloaderOptions:self.options], self.context); + // check if we already use progressive decoding, use that to produce faster decoding + id progressiveCoder = SDImageLoaderGetProgressiveCoder(self); + UIImage *image; + if (progressiveCoder) { + image = SDImageLoaderDecodeProgressiveImageData(imageData, self.request.URL, YES, self, [[self class] imageOptionsFromDownloaderOptions:self.options], self.context); + } else { + image = SDImageLoaderDecodeImageData(imageData, self.request.URL, [[self class] imageOptionsFromDownloaderOptions:self.options], self.context); + } CGSize imageSize = image.size; if (imageSize.width == 0 || imageSize.height == 0) { NSString *description = image == nil ? @"Downloaded image decode failed" : @"Downloaded image has 0 pixels"; From 319a308995d2b79ecec111f235df0f945e2bf54f Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Thu, 4 Mar 2021 18:54:55 +0800 Subject: [PATCH 2/3] Change to not mark the `is_incremental` when the progressive decoding `finished` is YES. This can avoid one extra decoding during progressive loading --- SDWebImage/Core/SDImageLoader.m | 4 ++-- SDWebImage/Core/SDWebImageDownloaderOperation.m | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/SDWebImage/Core/SDImageLoader.m b/SDWebImage/Core/SDImageLoader.m index 7b565bff..d6b95948 100644 --- a/SDWebImage/Core/SDImageLoader.m +++ b/SDWebImage/Core/SDImageLoader.m @@ -202,8 +202,8 @@ UIImage * _Nullable SDImageLoaderDecodeProgressiveImageData(NSData * _Nonnull im if (shouldDecode) { image = [SDImageCoderHelper decodedImageWithImage:image]; } - // mark the image as progressive (completionBlock one are not mark as progressive) - image.sd_isIncremental = YES; + // mark the image as progressive (completed one are not mark as progressive) + image.sd_isIncremental = !finished; } return image; diff --git a/SDWebImage/Core/SDWebImageDownloaderOperation.m b/SDWebImage/Core/SDWebImageDownloaderOperation.m index 92d7ad9b..c0f269e0 100644 --- a/SDWebImage/Core/SDWebImageDownloaderOperation.m +++ b/SDWebImage/Core/SDWebImageDownloaderOperation.m @@ -263,7 +263,6 @@ typedef NSMutableDictionary SDCallbacksDictionary; - (void)done { self.finished = YES; self.executing = NO; - SDImageLoaderSetProgressiveCoder(self, nil); [self reset]; } @@ -388,7 +387,8 @@ didReceiveResponse:(NSURLResponse *)response // Using data decryptor will disable the progressive decoding, since there are no support for progressive decrypt BOOL supportProgressive = (self.options & SDWebImageDownloaderProgressiveLoad) && !self.decryptor; - if (supportProgressive) { + // Progressive decoding Only decode partial image, full image in `URLSession:task:didCompleteWithError:` + if (supportProgressive && !finished) { // Get the image data NSData *imageData = [self.imageData copy]; @@ -396,7 +396,7 @@ didReceiveResponse:(NSURLResponse *)response if (self.coderQueue.operationCount == 0) { // NSOperation have autoreleasepool, don't need to create extra one [self.coderQueue addOperationWithBlock:^{ - UIImage *image = SDImageLoaderDecodeProgressiveImageData(imageData, self.request.URL, finished, self, [[self class] imageOptionsFromDownloaderOptions:self.options], self.context); + UIImage *image = SDImageLoaderDecodeProgressiveImageData(imageData, self.request.URL, NO, self, [[self class] imageOptionsFromDownloaderOptions:self.options], self.context); if (image) { // We do not keep the progressive decoding image even when `finished`=YES. Because they are for view rendering but not take full function from downloader options. And some coders implementation may not keep consistent between progressive decoding and normal decoding. From d277514cbe09a43a540baa0fda39d52a27a95fd5 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Thu, 4 Mar 2021 21:09:22 +0800 Subject: [PATCH 3/3] Added `test28ProgressiveDownloadShouldUseSameCoder` --- Tests/Tests/SDWebImageDownloaderTests.m | 26 +++++++++++++++++++++++++ Tests/Tests/SDWebImageManagerTests.m | 2 ++ 2 files changed, 28 insertions(+) diff --git a/Tests/Tests/SDWebImageDownloaderTests.m b/Tests/Tests/SDWebImageDownloaderTests.m index 808fbe30..ba4ec5b2 100644 --- a/Tests/Tests/SDWebImageDownloaderTests.m +++ b/Tests/Tests/SDWebImageDownloaderTests.m @@ -707,6 +707,32 @@ [self waitForExpectationsWithCommonTimeout]; } +- (void)test28ProgressiveDownloadShouldUseSameCoder { + XCTestExpectation *expectation = [self expectationWithDescription:@"Progressive download should use the same coder for each animated image"]; + SDWebImageDownloader *downloader = [[SDWebImageDownloader alloc] init]; + + __block SDWebImageDownloadToken *token; + __block id progressiveCoder; + token = [downloader downloadImageWithURL:[NSURL URLWithString:kTestGIFURL] options:SDWebImageDownloaderProgressiveLoad context:@{SDWebImageContextAnimatedImageClass : SDAnimatedImage.class} progress:nil completed:^(UIImage * _Nullable image, NSData * _Nullable data, NSError * _Nullable error, BOOL finished) { + expect(error).beNil(); + expect([image isKindOfClass:SDAnimatedImage.class]).beTruthy(); + id coder = ((SDAnimatedImage *)image).animatedCoder; + if (!progressiveCoder) { + progressiveCoder = coder; + } + expect(progressiveCoder).equal(coder); + if (!finished) { + progressiveCoder = coder; + } else { + [expectation fulfill]; + } + }]; + + [self waitForExpectationsWithCommonTimeoutUsingHandler:^(NSError * _Nullable error) { + [downloader invalidateSessionAndCancel:YES]; + }]; +} + #pragma mark - SDWebImageLoader - (void)test30CustomImageLoaderWorks { XCTestExpectation *expectation = [self expectationWithDescription:@"Custom image not works"]; diff --git a/Tests/Tests/SDWebImageManagerTests.m b/Tests/Tests/SDWebImageManagerTests.m index 30d9e6b5..7355f500 100644 --- a/Tests/Tests/SDWebImageManagerTests.m +++ b/Tests/Tests/SDWebImageManagerTests.m @@ -269,6 +269,7 @@ expect(image).notTo.beNil(); expect(image.size).equal(CGSizeMake(1000, 1000)); if (finished) { + expect(image.sd_isIncremental).beFalsy(); [expectation fulfill]; } else { expect(image.sd_isIncremental).beTruthy(); @@ -290,6 +291,7 @@ expect(image.imageOrientation).equal(orientation); #endif if (finished) { + expect(image.sd_isIncremental).beFalsy(); [expectation fulfill]; } else { expect(image.sd_isIncremental).beTruthy();