Use `SDWebImageAvoidDecodeImage` to allow user to control force decode feature for individual image request #2283

dreampiggy · 2018-04-15T17:39:36Z

New Pull Request Checklist

I have read and understood the CONTRIBUTING guide
I have read the Documentation
I have searched for a similar pull request in the project and found none
I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)
I have added the required tests to prove the fix/feature I am adding
I have updated the documentation (if necessary)
I have run the tests and they pass
I have run the lint and it passes (pod lib lint)

This merge request fixes / reffers to the following issues: #1927

Pull Request Description

Reason

This is about that decompressImages feature. We all know that what decompressImages will do. It actually call sd_decodedImageWithImage: in the background, to draw the image from Image/IO on the bitmap context, this can help to force decode the image. Then when we set this UIImage on the UIImageView, it does not cause any lag or blocking on the main queue to wait for Core Animation firstly decode it.

This options is good for performance, and it seems that most of application use this well. However, due to the force decode process, this will consume more memory, especially for huge images. It's a trade-off between Time & Space.

In previous SDWebImage, we have a option to enable/disable this feature. We have these:

SDImageCache: SDImageCache.config.shouldDecompressImages
SDWebImageDownloader: SDWebImageDownloader.shouldDecompressImages
SDWebImageDownloadOperation: SDWebImageDownloadOperation.shouldDecompressImages

But however, they are both central control. Which means if I disable on in shared downloader, or shared cache, this will effect all the image request for this. In most cases, we do not want this. Because if user want to set this option for individual image request, they have no way to do this, except create another SDImageCache and SDWebImageDownloader instance, which is really heavy on usage and cause many problem when syncing for shared cache/downloader.

From the real world application experience by our company(Such as Tik Tok), most images are small enough (less than 1000x1000), the force decode process can help to increase nearly 4-5 frame rates on common UITableView or UIScrollView. It's quite rare we need turn it off, unless some huge images which consume much memory.

So, we'd better move that option, into SDWebImageOptions level, to allow we control it for each image request from sd_setImageWithURL:

Design

From the description above, we should treat force decode ON as the default value. And this options can be used both in manager, cache, downloader. So we just put it in the SDWebImageOptions. Naming it SDWebImageAvoidDecodeImage to specify that you can avoid this force decode feature.

There are also the correspond options for SDImageCacheOptions and SDWebImageDownloaderOptions, keep it the same name as usual.

Implementation

/**
 * By default, we will decode the image in the background during cache query and download from the network. This can help to improve performance because when rendering image on the screen, it need to be firstly decoded. But this happen on the main queue by Core Animation.
 * However, this process may increase the memory usage as well. If you are experiencing a issue due to excessive memory consumption, This flag can prevent decode the image.
 */
SDWebImageAvoidDecodeImage = 1 << 17

…e feature for individual image request. Replace all the central control for `decompressImages`

dreampiggy · 2018-04-15T17:49:27Z

Current break change (Need change code)

SDImageCacheConfig.h

shouldDecompressImages REMOVED. Use SDImageCacheAvoidDecodeImage in cache options instead.

SDWebImageDownloader.h

shouldDecompressImages REMOVED. Use SDWebImageDownloaderAvoidDecodeImage in downloader options instead.

SDWebImageDownloadOperation.h

shouldDecompressImages REMOVED. Use SDWebImageDownloaderAvoidDecodeImage in downloader options instead.

codecov-io · 2018-04-15T17:55:10Z

Codecov Report

❗ No coverage uploaded for pull request base (5.x@1367b18). Click here to learn what that means.
The diff coverage is 25%.

@@          Coverage Diff           @@
##             5.x    #2283   +/-   ##
======================================
  Coverage       ?   42.75%           
======================================
  Files          ?       51           
  Lines          ?     6063           
  Branches       ?        0           
======================================
  Hits           ?     2592           
  Misses         ?     3471           
  Partials       ?        0

Impacted Files	Coverage Δ
SDWebImage/SDImageCacheConfig.m	`100% <ø> (ø)`
SDWebImage/SDWebImageDownloaderConfig.m	`88.88% <ø> (ø)`
SDWebImage/SDWebImageDownloader.m	`79.77% <ø> (ø)`
SDWebImage/SDImageCache.m	`53.2% <0%> (ø)`
SDWebImage/SDWebImageDownloaderOperation.m	`73.26% <50%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1367b18...781c079. Read the comment docs.

mythodeia

👍

ripperhe · 2019-05-08T08:43:21Z

I need central control. Can I set the default Option to SDWebImageAvoidDecodeImage?
Is there any code equivalent to the following two sentences?

[[SDImageCache sharedImageCache] setShouldDecompressImages:NO];
[[SDWebImageDownloader sharedDownloader] setShouldDecompressImages:NO];

ripperhe · 2019-05-08T08:53:36Z

@dreampiggy 呃呃呃，帮忙看看

dreampiggy · 2019-05-08T09:52:43Z

@ripperhe You and @welsonxue have the exact opposite request for this option. 😅

To solve this problem, there are 3 solutions, maybe...

Change your code

You can try build a wrapper category method, which always pass the default option SDWebImageAvoidDecodeImage to the original sd_setImageWithURL: API. Then change all your code to call your wrapper one instead.

@implementation UIImageView (MyWebCache)
- (void)my_setImageWithURL:(NSURL *)url options:(SDWebImageOption)options {
    SDWebImageOption newOptions = SDWebImageAvoidDecodeImage | options;
    [self sd_setImageWithURL:url options:newOptions];
}
@end

See comment here: #2679. Similar problem.

Allows global control to override

We can use a global control, which override your SDWebImageOption. However, this is a problem: How can we override ?

This enum can only represent two meaning: YES (use force decoding) or NO (do not use). It can not represent this meanning: default (use the global control instead). Consider this code:

SDWebImageManager.sharedManager.avoidDecodeImage = NO;
[imageView sd_setImageWithURL:url option:SDWebImageAvoidDecodeImage];
// What's the final result for this option ?

One way for this, it to use the 5.0 new context option, which use a 3-case enum, or can use @(YES) for YES, @(NO) for NO, and nil for default.

This may cause API breaking, in my opinion. Or we need to deprecate the old option SDWebImageAvoidDecodeImage to use new context option like SDWebImageContextForceDecodeImage instead.

Add a default options feature

We can introduce a feature called default options, which will append the default option to final SDWebImageOptions.

This can also fix other related option where user want to take both global control && per-image-request control like SDWebImageDecodeFirstFrameOnly.

But actually, this feature have one disadvantage, how to solve the problem when some image request don't want the default option, but others want ?

SDWebImageManager.sharedManager.defaultOptions = SDWebImageAvoidDecodeImage;
[imageView sd_setImageWithURL:url option:0]; // How to do if I really want this image request, to trun the force decode on?
// Maybe something like this ?
[imageView sd_setImageWithURL:url option:SDWebImageIgnoreDefaultOptions]; // Ignore default options appending, final options result to 0

Which solution to use, need some discussion. You can create a new feature request for this.

ripperhe · 2019-05-08T11:28:42Z

Thank you very much for your reply. I get a rough idea of what you mean.

I finally decided to use version 4.4.6 at the moment and update it later when default options are available.

There are too many places to use SDWebImage, and I'm not sure what risks there are.I'm looking forward to the next version.

dreampiggy · 2019-05-08T13:03:56Z

Moved the discussion and feature request to #2719

Use SDWebImageAvoidDecodeImage to allow user to control force decod…

781c079

…e feature for individual image request. Replace all the central control for `decompressImages`

dreampiggy added this to the 5.0.0 milestone Apr 15, 2018

dreampiggy added the feature label Apr 15, 2018

dreampiggy requested review from bpoplauschi, mythodeia and skyline75489 April 15, 2018 17:39

dreampiggy mentioned this pull request Apr 15, 2018

shouldDecompressImages property related problem? #1927

Closed

3 tasks

mythodeia approved these changes Apr 16, 2018

View reviewed changes

bpoplauschi approved these changes Apr 16, 2018

View reviewed changes

dreampiggy mentioned this pull request Apr 16, 2018

Feature custom image loader - Supports loader protocol #2256

Merged

8 tasks

dreampiggy changed the title ~~Use SDWebImageAvoidDecodeImage to allow user to control force decod…~~ Use SDWebImageAvoidDecodeImage to allow user to control force decode feature for individual image request Apr 16, 2018

dreampiggy merged commit c5647c8 into SDWebImage:5.x Apr 16, 2018

bpoplauschi deleted the feature_force_decode_individual_request branch April 16, 2018 09:06

dreampiggy mentioned this pull request Aug 14, 2018

App crashes when load many images from firestore - swift #2418

Closed

3 tasks

bpoplauschi mentioned this pull request Aug 14, 2018

High memory consumption in release prod #2448

Closed

3 tasks

dreampiggy mentioned this pull request May 8, 2019

Feature Request: Supports to specify the options in both per-image-request level as well as global control level #2719

Closed

3 tasks

dreampiggy mentioned this pull request May 23, 2019

Supports global control on options and context for individual image request #2736

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Use `SDWebImageAvoidDecodeImage` to allow user to control force decode feature for individual image request #2283

Use `SDWebImageAvoidDecodeImage` to allow user to control force decode feature for individual image request #2283

dreampiggy commented Apr 15, 2018 •

edited

Loading

Uh oh!

dreampiggy commented Apr 15, 2018 •

edited

Loading

Uh oh!

codecov-io commented Apr 15, 2018

Uh oh!

mythodeia left a comment

Uh oh!

ripperhe commented May 8, 2019

Uh oh!

ripperhe commented May 8, 2019

Uh oh!

dreampiggy commented May 8, 2019 •

edited

Loading

Uh oh!

ripperhe commented May 8, 2019

Uh oh!

dreampiggy commented May 8, 2019

Uh oh!

Uh oh!

Use SDWebImageAvoidDecodeImage to allow user to control force decode feature for individual image request #2283

Use SDWebImageAvoidDecodeImage to allow user to control force decode feature for individual image request #2283

Conversation

dreampiggy commented Apr 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

New Pull Request Checklist

Pull Request Description

Reason

Design

Implementation

Uh oh!

dreampiggy commented Apr 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current break change (Need change code)

SDImageCacheConfig.h

SDWebImageDownloader.h

SDWebImageDownloadOperation.h

Uh oh!

codecov-io commented Apr 15, 2018

Codecov Report

Uh oh!

mythodeia left a comment

Choose a reason for hiding this comment

Uh oh!

ripperhe commented May 8, 2019

Uh oh!

ripperhe commented May 8, 2019

Uh oh!

dreampiggy commented May 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change your code

Allows global control to override

Add a default options feature

Uh oh!

ripperhe commented May 8, 2019

Uh oh!

dreampiggy commented May 8, 2019

Uh oh!

Use `SDWebImageAvoidDecodeImage` to allow user to control force decode feature for individual image request #2283

Use `SDWebImageAvoidDecodeImage` to allow user to control force decode feature for individual image request #2283

dreampiggy commented Apr 15, 2018 •

edited

Loading

dreampiggy commented Apr 15, 2018 •

edited

Loading

dreampiggy commented May 8, 2019 •

edited

Loading