Prevent divide-by-zero in ImageAnimator for images with 0 delay between frames

[jeffhandley]

Fixes #55972

Per the GIF spec, a frame delay can have a value of 0. That was historically used for 2 purposes:

1. Allow the animation to progress as fast as the renderer chose to animate
2. Utilize the GIF spec feature that has animation wait for user input (mouse click, key press)

While the latter of those features is archaic, we still need to handle 0-delay frames. We assign them our animation delay speed to be "1 tick". It's possible the frame could get skipped if a tick takes longer than the tick time, but this gives a uniform speed for GIFs where all frame delays are 0.

To prevent the divide-by-zero:

1. If a frame delay is <= 0, apply the tick speed
2. Ensure the total animation time increases with each frame, guarding against overflows
3. Update the ShouldAnimate property to check that the total animation time is > 0

[ghost]

Tagging subscribers to this area: @safern, @tarekgh
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Fixes #55972

Per the GIF spec, a frame delay can have a value of 0. That was historically used for 2 purposes:

1. Allow the animation to progress as fast as the renderer chose to animate
2. Utilize the GIF spec feature that has animation wait for user input (mouse click, key press)

While the latter of those features is archaic, we still need to handle 0-delay frames. We assign them our animation delay speed to be "1 tick". It's possible the frame could get skipped if a tick takes longer than the tick time, but this gives a uniform speed for GIFs where all frame delays are 0.

To prevent the divide-by-zero:

1. If a frame delay is <= 0, apply the tick speed
2. Ensure the total animation time increases with each frame, guarding against overflows
3. Update the ShouldAnimate property to check that the total animation time is > 0

Author:	jeffhandley
Assignees:	-
Labels:	area-System.Drawing
Milestone:	-

[jeffhandley]

This changes the behavior at line 160, preventing animation from occurring if the total animation time is <= 0.

[danmoseley]

this is for cases where delay is negative? does that ever happen/is it legal or are you just being careful?

[danmoseley]

if just being careful, could you instead simply force delay to be no less than zero? then I think you wouldn't need _totalAnimationTime?

[jeffhandley]

I'm being careful mostly. Here's how it comes together though.

1. If the delay is <= 0, then apply the default animation delay
2. If the accumulated last end time overflows, then collapse all of the remaining frames into that instant (essentially skipping any frames beyond the overflow of animation time)
3. Ensure that TotalAnimationTime is a non-zero value if any frames are to be animated
4. Eliminate the array access from TotalAnimationTime, replacing that with _totalAnimationTime

[AraHaan]

Speaking of animated images, I wonder when System.Drawing.Common's ImageAnimator can pick up png images with multiple frames as animated?

I guess I could try to see if I can propose an api to help provide support for animated png's if possible on the .NET side of things in .NET 7 if that is required then somehow get the image animator to actually check for it all.

And yes this is a known issue with people trying to manually hack in their own "Image animator" for their picture boxes as well.

[danmoseley]

Unrelated failures.

[safern]

LGTM, thanks for fixing so quickly!