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

Remove kCGImageSourceShouldCache option when creating image source #2472

Merged
merged 1 commit into from Sep 6, 2018

Conversation

@zhongwuzw
Copy link
Member

zhongwuzw commented Sep 5, 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

kCGImageSourceShouldCache is only valid when creating an image from image source, image source would not treat this option. Only kCGImageSourceTypeIdentifierHint can pass to options when creating image source. Please see Documents

@zhongwuzw zhongwuzw requested review from bpoplauschi and dreampiggy Sep 5, 2018
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Sep 5, 2018

Codecov Report

Merging #2472 into 5.x will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##              5.x    #2472      +/-   ##
==========================================
+ Coverage   77.87%   77.89%   +0.02%     
==========================================
  Files          47       47              
  Lines        6621     6619       -2     
==========================================
  Hits         5156     5156              
+ Misses       1465     1463       -2
Flag Coverage Δ
#iOS 77.83% <100%> (+0.04%) ⬆️
#macOS 76.65% <100%> (+0.02%) ⬆️
Impacted Files Coverage Δ
SDWebImage/SDImageAPNGCoder.m 86.72% <100%> (-0.06%) ⬇️
SDWebImage/SDImageGIFCoder.m 87.74% <100%> (+0.34%) ⬆️
SDWebImage/SDAnimatedImage.m 80.4% <0%> (+0.4%) ⬆️

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 0f14258...b1b48cc. Read the comment docs.

@dreampiggy

This comment has been minimized.

Copy link
Contributor

dreampiggy commented Sep 6, 2018

Emmm...From the documents, the only public options for CGImageSource level, is kCGImageSourceTypeIdentifierHint, it's used to provide information about the image format. That kCGImageSourceShouldCache is only available in CGImageSourceCreateImageAtIndex && CGImageSourceCopyPropertiesAtIndex API. So it's OK to remove it.

I've also check that it indeed behavior. The only available options in CGImageSource level, including the follwings...

  • kCGImageSourceTypeIdentifierHint (Public)
  • kCGImageSourceShouldMemoryMap (Private)
  • kCGImageSourceRespectHEIFFileOrder (Private)
  • kCGImageSourceColorTransformAdobeRGBTosRGB (Private)
@zhongwuzw

This comment has been minimized.

Copy link
Member Author

zhongwuzw commented Sep 6, 2018

@dreampiggy 👍 Yeah, correctly as you said.

@bpoplauschi bpoplauschi merged commit f22a698 into SDWebImage:5.x Sep 6, 2018
3 checks passed
3 checks passed
codecov/patch 100% of diff hit (target 77.87%)
Details
codecov/project 77.89% (+0.02%) compared to 0f14258
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@zhongwuzw zhongwuzw deleted the zhongwuzw:remove-image-source-option branch Sep 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.