Added animationTransformer on SDAnimatedImageView, allows the animated image to apply post-transform
#3761
Conversation
This allows the animated image to apply post-transform Currently we don't change the old `SDWebImageTransformAnimatedImage` behavior. This feature is opt-in
|
Demo video: Demo.mov |
| if (self.tintApplied) { | ||
| self.imageView.animationTransformer = nil; | ||
| } else { | ||
| self.imageView.animationTransformer = [SDImageTintTransformer transformerWithColor:UIColor.blackColor]; |
There was a problem hiding this comment.
@Kudo Usage code on SDAnimatedImageView with custom post-transform
Transformer can be created with 2 thing.
- The block to handle
(UIImage) -> (UIImage), your main logic. - The String transformer key match your logic, the key is used to cache whether we can share the frame cache or create a new one
frame pool is an advanced thing which want to solve the issue, when user display 10 SDAnimatedImageView at the same time. In old SD 5.x version, we will create 10 different players and 10 threads do the same decoding thing, which is waste of CPU.
With frame pool, the sameSDAnimatedImageinstance with the sametransformer keywill share the same frame cache.
I remember expo comment this issue 1 year ago: expo/expo#25920 (comment)
There was a problem hiding this comment.
sorry was busy on releasing new stuff from the past week and late for the response.
i tried to apply this pr's patch locally and the new animationTransformer works well. i like this api and super excited to use it in formal.
just one more question: the animationTransformer only works for animated images now. could we have similar post-transform for static images?
There was a problem hiding this comment.
could we have similar post-transform for static images?
Static image itself supports transformer. You can pass context options SDWebImageContextImageTransformer to use that. It's not per-imageView level control, it's per-image level control, and it also effect the cache (by defaults the transformed image stored to different cache key)
There was a problem hiding this comment.
There was a problem hiding this comment.
cool thanks for sharing the info. it's just a sense of inconsistent usage between static image and animated image. but no problem we can manage the difference internally. will try the new release in the next expo sdk. thanks for having all the great help.
There was a problem hiding this comment.
inconsistent
The actual reason why this API looks inconsistent is because, we don't Cache the animated image frame into global SDImageCache at all. Which will result OOM, and really a bad design. The same as why I advise developers to avoid using_UIAnimatedImage (image object returned by UIImage animatedImageWithFrames:), because it's cached and always eat RAM in global cache.
Developer should realize the different on performance about static/animated for complicated use case and optimize for this. (like transformer, since you can process image to return any new image, which may be time-consuming or RAM consuming)
So, the animationTransformer is applied to image view level, all the cache or status only effect one image view. In current design, mostly the different image view can not share the frame cache. (only when the image view use the same SDAnimatedImage object and use the same animationTransformer.transformerKey can share cache)
Normal transformer is applied to image object level, by default only the UIImage (not _UIAnimatedImage or SDAnimatedImage)
|
|
||
| - (UIImage *)animatedImageFrameAtIndex:(NSUInteger)index { | ||
| UIImage *frame = [self.provider animatedImageFrameAtIndex:index]; | ||
| return [self.transformer transformedImageWithImage:frame forKey:@""]; |
There was a problem hiding this comment.
Actually...The transformer API's nullability is wrong. The input and output should both be nonnull...But history is marked as nullable (Why ?)
This will change in 6.0 as well.
|
I'll add test and merge later... Seems no huge feedback changes from expo (The actual feature request cames from) |
|
This Released in v5.20.0 @Kudo |
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 / refers to the following issues: ...
Pull Request Description
Note: Currently we don't change the old
SDWebImageTransformAnimatedImagebehavior. This feature is opt-inWhy use transformer as naming
Because finally I think this naming matches the exists
Image Transformerconcept of static image, and can re-use the code/user who already write their transformer.And, the most important thing is that
SDAnimatedImageViewfrom 5.10 has aframe pooldesign, which allowsmultiple image view who render the same animated image, share the frame bufferI want to re-use this frame buffer pool design, but now we have 2 thing, only
transformer + animated image contentequal means we can share the frame buffer.The exists transformer API has a
transformerKeydesign, which solve the cache issue, so I think it's useful and I just pick it.