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.

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

Add SDImageCoderWebImageContext coder option, which allow custom coder plugin, to receive the context option from top-level API #2405

Merged
merged 1 commit into from Jul 31, 2018

Conversation

@dreampiggy
Copy link
Contributor

dreampiggy commented Jul 26, 2018

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: ...

Pull Request Description

Reason

This is a solution for #2404 . SDWebImageContext contains all the extra information through view category, cache, manager API, but it's not accessable from the custom coder plugin.

It's not surprise because in my mind, a decoder, should only consider about the image & image data, or anything related to the decoding option (Like animation or not, scale factor).

But however, if some custom coder plugin user, who want do some business logic related process, during image decoding. They have no choice to know the context. So I guess, maybe we can open this possibility.

Related PR

Actually, this feature request, is from SDWebImageFLPlugin's PR, which will create a SDWebImageFLCoder, to directlly produce a FLAnimatedImage, to end up that previous hack code that do image decoding during View setImage process.

The image decoding process, should always happend in image coder, but not hack into the setImageBlock. And that PR provide a better solution. And since all the decoding method is called from global queue, this can keep better performance.

…r plugin, to receive the context option from top-level API (Cache/Downloader/View Category)
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jul 26, 2018

Codecov Report

Merging #2405 into 5.x will increase coverage by 0.1%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff            @@
##              5.x    #2405     +/-   ##
=========================================
+ Coverage   68.72%   68.82%   +0.1%     
=========================================
  Files          47       47             
  Lines        6634     6649     +15     
=========================================
+ Hits         4559     4576     +17     
+ Misses       2075     2073      -2
Flag Coverage Δ
#iOS 69.94% <66.66%> (+0.1%) ⬆️
#macOS 68.67% <66.66%> (+0.06%) ⬆️
Impacted Files Coverage Δ
SDWebImage/SDImageCacheDefine.m 62.16% <50%> (-3.47%) ⬇️
SDWebImage/SDImageLoader.m 86% <75%> (-1.78%) ⬇️
SDWebImage/SDWebImageDownloader.m 80.82% <0%> (-0.35%) ⬇️
SDWebImage/SDWebImageDownloaderOperation.m 87.62% <0%> (+3.09%) ⬆️

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 919751f...ef31239. Read the comment docs.

@dreampiggy dreampiggy merged commit aff324b into SDWebImage:5.x Jul 31, 2018
2 of 3 checks passed
2 of 3 checks passed
codecov/patch 66.66% of diff hit (target 68.72%)
Details
codecov/project 68.82% (+0.1%) compared to 919751f
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@bpoplauschi bpoplauschi deleted the dreampiggy:feature_pass_context_to_coder branch Jul 31, 2018
@bpoplauschi bpoplauschi mentioned this pull request Sep 7, 2018
52 of 52 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.