Skip to content

Conversation

@rayguo17
Copy link
Contributor

@rayguo17 rayguo17 commented Jan 5, 2026

As described in #2715, This PR implements loop_count function for AnimationDecoder Trait, currently there are three type (Gif, Webp and Apng) implementing AnimationDecoder Trait, so implementation on these specific format decoders are added.

rayguo17 and others added 2 commits January 5, 2026 08:40
Signed-off-by: rayguo17 <tin.tun.aung1@huawei.com>
Signed-off-by: rayguo17 <rayguo17@gmail.com>
Copy link
Member

@197g 197g left a comment

Choose a reason for hiding this comment

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

Seems agreeable. (fyi, I'd rather we merge this PR to introduce the type and then I adjust #2672 where the AnimationDecoder trait was removed. It fits well with the is_animated field that got added there.)

Signed-off-by: rayguo17 <rayguo17@gmail.com>
@rayguo17 rayguo17 requested a review from 197g January 7, 2026 02:22
@rayguo17
Copy link
Contributor Author

rayguo17 commented Jan 7, 2026

(fyi, I'd rather we merge this PR to introduce the type and then I adjust #2672 where the AnimationDecoder trait was removed. It fits well with the is_animated field that got added there.)

Do you mind explain a bit on the change for AnimationDecoder in #2672 ? I look through the code, and seems to me the PR only change the implementation of the iterator, and still implementing AnimationDecoder.

@197g
Copy link
Member

197g commented Jan 7, 2026

The PR replaces what AnimationDecoder can do with simply calling read_image itself multiple times. That rids us of the awkward dance where you can't easily optionally have an AnimationDecoder by moving that information into decoder or file attributes. (Deleting the actual trait would be the last commit on the PR or after it, once we're clear that all functionality replacement is good to go).

@197g 197g merged commit 256dc9d into image-rs:main Jan 7, 2026
32 checks passed
@197g 197g mentioned this pull request Jan 7, 2026
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants