Skip to content

Fix 2087 round gif#2140

Closed
hetovar wants to merge 7 commits into
facebook:masterfrom
hetovar:fix-2087-RoundGif
Closed

Fix 2087 round gif#2140
hetovar wants to merge 7 commits into
facebook:masterfrom
hetovar:fix-2087-RoundGif

Conversation

@hetovar

@hetovar hetovar commented Jun 21, 2018

Copy link
Copy Markdown

Motivation

Making rounding GIF would make the application more appealing to users as well as adding some more functionality.

Test Plan

Added some test in order to show that the drawable is able to be drawn with either the property or without it.

@oprisnik oprisnik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this. Same comments as for #2145

I think this should be approached by adding postprocessor support for animation frames and utilizing our existing (and way more performant) rounding postprocessor: https://github.com/facebook/fresco/blob/master/imagepipeline/src/main/java/com/facebook/imagepipeline/postprocessors/RoundAsCirclePostprocessor.java

Basically, you would set the rounding postprocessor for the animated image, which would then be applied in the backend.

You also need to account for this when the frame is cached since the frame cache can be shared across multiple GIFs, so the cache key must include the postprocessor cache key if there is one or otherwise we should only cache the original frame (or cache nothing).

@woyunowuyuda and @hetovar you basically worked on the same thing here, (GIF and animated WebP use the same infra), so you should coordinate.

@erikandre

Copy link
Copy Markdown
Contributor

Closing this as we are not able to accept the contribution as-is and no updates has been made since July.

@erikandre erikandre closed this Sep 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

5 participants