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 APNG coder support #2149

Merged
merged 3 commits into from
Mar 29, 2018
Merged

Conversation

dreampiggy
Copy link
Contributor

@dreampiggy dreampiggy commented Jan 3, 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: #1060

Pull Request Description

This PR add the build-in support for APNG image format. See here: https://en.wikipedia.org/wiki/APNG

For static PNG image, it can decode the first frame. For APNG image, it can decode the animated image.

This coder conform to SDWebImageProgressiveCoder, which means you can do incremental image loading for APNG frames.

This coder also conform to SDAnimatedImageCoder protocol and can be intergrate in SDAnimatedImageView to support big APNG rendering.

This PR is based on the animated coder refactor branch. So if that branch is merged firstly, the diff may looks more clear. Refactor branch merged

@dreampiggy dreampiggy added this to the 5.0.0 milestone Jan 3, 2018
@codecov-io
Copy link

codecov-io commented Jan 3, 2018

Codecov Report

Merging #2149 into 5.x will decrease coverage by 0.93%.
The diff coverage is 49.17%.

Impacted file tree graph

@@            Coverage Diff            @@
##              5.x   #2149      +/-   ##
=========================================
- Coverage   74.63%   73.7%   -0.94%     
=========================================
  Files          50      51       +1     
  Lines        6372    6613     +241     
  Branches      575     597      +22     
=========================================
+ Hits         4756    4874     +118     
- Misses       1558    1681     +123     
  Partials       58      58
Impacted Files Coverage Δ
SDWebImage/SDWebImageCodersManager.m 70.31% <100%> (ø) ⬆️
Tests/Tests/SDWebImageDecoderTests.m 100% <100%> (ø) ⬆️
SDWebImage/SDWebImageAPNGCoder.m 43.26% <43.26%> (ø)
SDWebImage/SDAnimatedImageRep.m 86.53% <82.14%> (-5.13%) ⬇️

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 beb958b...5da5ec9. Read the comment docs.

@dreampiggy
Copy link
Contributor Author

Merged. This is actual a separate component of image decoding and have no conflict with current codebase. I test with iOS & macOS image and it works.

@dreampiggy dreampiggy merged commit 04c31af into SDWebImage:5.x Mar 29, 2018
@bpoplauschi bpoplauschi deleted the refactor_apng_coder branch April 2, 2018 13:28
@bpoplauschi
Copy link
Member

@dreampiggy is this the same coder from https://github.com/SDWebImage/SDWebImageAPNGCoder? If so, we can do some dependency management to avoid duplicating the code

@dreampiggy
Copy link
Contributor Author

dreampiggy commented Apr 2, 2018

@bpoplauschi Yes..Nearlly the same for the UIAnimatedImage based animated code. However, current 5.x APNGCoder supports SDWebImageAnimatedCoder for advanced animated image rendering.

You're right, maybe dependency management is better than current subspec management. But however, I think why we need another cocoapods may because that feature (Like WebP coder, or MapKit) need another dependency which is no need for Core Lib.

Since APNG support is done by Apple from Image/IO framework, which is already the main dependency for SDWebImage. It contains the same dependency level as GIF coder. I think separating WebP coder and FLAnimatedImage to another cococapods is more important than APNG or GIF :)

For current that SDWebImageAPNGCoder. Maybe the best way is to close it or mark it as deprecated.

@bpoplauschi
Copy link
Member

I see. Basically on the 5.x branch is a more rich version of the APNG coder. OK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants