-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Prevent divide-by-zero in ImageAnimator for images with 0 delay between frames #56223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) | ||
|
|
@@ -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) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is for cases where delay is negative? does that ever happen/is it legal or are you just being careful?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if just being careful, could you instead simply force delay to be no less than zero? then I think you wouldn't need _totalAnimationTime?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm being careful mostly. Here's how it comes together though.
|
||
| { | ||
| lastEndTime = _totalAnimationTime; | ||
| } | ||
| else | ||
| { | ||
| _totalAnimationTime = lastEndTime; | ||
| } | ||
|
|
||
| _frameEndTimes[f] = lastEndTime; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -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 | ||
| { | ||
|
|
@@ -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; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This changes the behavior at line 160, preventing animation from occurring if the total animation time is <= 0. |
||
|
|
||
| /// <summary> | ||
| /// Advance the animation by the specified number of milliseconds. If the advancement | ||
|
|
@@ -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. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.