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

diff --git a/src/libraries/System.Drawing.Common/src/System/Drawing/ImageAnimator.cs b/src/libraries/System.Drawing.Common/src/System/Drawing/ImageAnimator.cs
index 49ce7c0b5985845ab918fe85063613a443bafa04..fdc6d751440a3d645b8da194772a5bff0a33b588 100644 (file)
--- a/src/libraries/System.Drawing.Common/src/System/Drawing/ImageAnimator.cs
+++ b/src/libraries/System.Drawing.Common/src/System/Drawing/ImageAnimator.cs
@@ -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>
@@ -387,7 +393,7 @@ namespace System.Drawing
 
             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;

diff --git a/src/libraries/System.Drawing.Common/src/System/Drawing/ImageInfo.cs b/src/libraries/System.Drawing.Common/src/System/Drawing/ImageInfo.cs
index 24b041395580a002514b4f15fb22116fd8dbe80b..a5eed4554097322c39688c1dbd839a259dd3c512 100644 (file)
--- a/src/libraries/System.Drawing.Common/src/System/Drawing/ImageInfo.cs
+++ b/src/libraries/System.Drawing.Common/src/System/Drawing/ImageInfo.cs
@@ -29,6 +29,7 @@ namespace System.Drawing
             private readonly bool _animated;
             private EventHandler? _onFrameChangedHandler;
             private readonly long[]? _frameEndTimes;
+            private long _totalAnimationTime;
             private long _frameTimer;
 
             public ImageInfo(Image image)
@@ -66,7 +67,21 @@ namespace System.Drawing
                             }
 
                             // 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;
                         }
                     }
 
@@ -95,24 +110,12 @@ namespace System.Drawing
             /// <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
             {
@@ -127,15 +130,15 @@ namespace System.Drawing
             }
 
             /// <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
@@ -195,13 +198,7 @@ namespace System.Drawing
             /// <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.

diff --git a/src/libraries/System.Drawing.Common/tests/System/Drawing/ImageAnimator.ManualTests.cs b/src/libraries/System.Drawing.Common/tests/System/Drawing/ImageAnimator.ManualTests.cs
index 63cb4e3f9329edb9f8af9316553be33ac4689707..84d9da6d2c1a385c8198b337fa3e69255484e597 100644 (file)
--- a/src/libraries/System.Drawing.Common/tests/System/Drawing/ImageAnimator.ManualTests.cs
+++ b/src/libraries/System.Drawing.Common/tests/System/Drawing/ImageAnimator.ManualTests.cs
@@ -43,6 +43,7 @@ namespace System.Drawing.Tests
                 "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();