Skip to content

Commit

Permalink
Prevent divide-by-zero in ImageAnimator for images with 0 delay betwe…
Browse files Browse the repository at this point in the history
…en frames (#56223)
  • Loading branch information
jeffhandley committed Jul 24, 2021
1 parent 4bf1750 commit 7d30da6
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ namespace System.Drawing
/// </summary>
public sealed partial class ImageAnimator
{
// We use a timer to apply an animation tick speeds of something a bit shorter than 50ms
// such that if the requested frame rate is about 20 frames per second, we will rarely skip
// a frame entirely. Sometimes we'll show a few more frames if available, but we will never
// show more than 25 frames a second and that's OK.
internal const int AnimationDelayMS = 40;

/// <summary>
/// A list of images to be animated.
/// </summary>
Expand Down Expand Up @@ -387,7 +393,7 @@ private static void AnimateImages()

while (true)
{
Thread.Sleep(40);
Thread.Sleep(AnimationDelayMS);

// Because Thread.Sleep is not accurate, capture how much time has actually elapsed during the animation
long timeElapsed = stopwatch.ElapsedMilliseconds;
Expand Down
47 changes: 22 additions & 25 deletions src/libraries/System.Drawing.Common/src/System/Drawing/ImageInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ private sealed class ImageInfo
private readonly bool _animated;
private EventHandler? _onFrameChangedHandler;
private readonly long[]? _frameEndTimes;
private long _totalAnimationTime;
private long _frameTimer;

public ImageInfo(Image image)
Expand Down Expand Up @@ -66,7 +67,21 @@ public ImageInfo(Image image)
}

// Frame delays are stored in 1/100ths of a second; convert to milliseconds while accumulating
_frameEndTimes[f] = (lastEndTime += (BitConverter.ToInt32(values, i) * 10));
// Per spec, a frame delay can be 0 which is treated as a single animation tick
int delay = BitConverter.ToInt32(values, i) * 10;
lastEndTime += delay > 0 ? delay : ImageAnimator.AnimationDelayMS;

// Guard against overflows
if (lastEndTime < _totalAnimationTime)
{
lastEndTime = _totalAnimationTime;
}
else
{
_totalAnimationTime = lastEndTime;
}

_frameEndTimes[f] = lastEndTime;
}
}

Expand Down Expand Up @@ -95,24 +110,12 @@ public ImageInfo(Image image)
/// <summary>
/// Whether the image supports animation.
/// </summary>
public bool Animated
{
get
{
return _animated;
}
}
public bool Animated => _animated;

/// <summary>
/// The current frame has changed but the image has not yet been updated.
/// </summary>
public bool FrameDirty
{
get
{
return _frameDirty;
}
}
public bool FrameDirty => _frameDirty;

public EventHandler? FrameChangedHandler
{
Expand All @@ -127,15 +130,15 @@ public EventHandler? FrameChangedHandler
}

/// <summary>
/// The total animation time of the image, in milliseconds.
/// The total animation time of the image in milliseconds, or <value>0</value> if not animated.
/// </summary>
private long TotalAnimationTime => Animated ? _frameEndTimes![_frameCount - 1] : 0;
private long TotalAnimationTime => Animated ? _totalAnimationTime : 0;

/// <summary>
/// Whether animation should progress, respecting the image's animation support
/// and if there are animation frames or loops remaining.
/// </summary>
private bool ShouldAnimate => Animated ? (_loopCount == 0 || _loop <= _loopCount) : false;
private bool ShouldAnimate => TotalAnimationTime > 0 ? (_loopCount == 0 || _loop <= _loopCount) : false;

/// <summary>
/// Advance the animation by the specified number of milliseconds. If the advancement
Expand Down Expand Up @@ -195,13 +198,7 @@ public void AdvanceAnimationBy(long milliseconds)
/// <summary>
/// The image this object wraps.
/// </summary>
internal Image Image
{
get
{
return _image;
}
}
internal Image Image => _image;

/// <summary>
/// Selects the current frame as the active frame in the image.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public void AnimateAndCaptureFrames()
"animated-timer-10fps-repeat-infinite.gif",
"animated-timer-100fps-repeat-2.gif",
"animated-timer-100fps-repeat-infinite.gif",
"animated-timer-0-delay-all-frames.gif",
};

Dictionary<string, EventHandler> handlers = new();
Expand Down

0 comments on commit 7d30da6

Please sign in to comment.