Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix ImageAnimator to render the correct frames #52236

Merged
merged 11 commits into from
May 6, 2021
Merged

Fix ImageAnimator to render the correct frames #52236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
fc8c2be
Blend the Windows/Unix implementations of ImageAnimator to fix StopAn…
jeffhandley Apr 28, 2021
5c298b4
Temporarily add animated gifs to the test project (awaiting arcade up…
jeffhandley May 4, 2021
6a2f9fc
Rewrite ImageAnimator to render the correct frames
jeffhandley May 4, 2021
d51deef
Add ImageAnimatorManualTests
jeffhandley May 1, 2021
87ffa25
Address PR feedback, including using a StopWatch for the timing
jeffhandley May 4, 2021
10c6005
Respect animation loop counts
jeffhandley May 4, 2021
c431ca7
Limit animation to 4/100ths of a second
jeffhandley May 5, 2021
7e94a95
Apply the IsDrawingSupported condition to ImageAnimator tests
jeffhandley May 5, 2021
191d77f
Merge branch 'main' into jeffhandley/imageanimator
jeffhandley May 5, 2021
e4c1fef
Revert "Temporarily add animated gifs to the test project (awaiting a…
jeffhandley May 5, 2021
d750190
Update manual test comment
jeffhandley May 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/libraries/System.Drawing.Common/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,9 +281,6 @@
<data name="InvalidEx2BoundArgument" xml:space="preserve">
<value>Value of '{1}' is not valid for '{0}'. '{0}' should be greater than or equal to {2} and less than or equal to {3}.</value>
</data>
<data name="InvalidFrame" xml:space="preserve">
<value>Frame is not valid. Frame must be between 0 and FrameCount.</value>
</data>
<data name="InvalidGDIHandle" xml:space="preserve">
<value>Win32 handle that was passed to {0} is not valid or is the wrong type.</value>
</data>
Expand Down
5 changes: 2 additions & 3 deletions src/libraries/System.Drawing.Common/src/System.Drawing.Common.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@
<Compile Include="System\Drawing\GraphicsUnit.cs" />
<Compile Include="System\Drawing\IconConverter.cs" />
<Compile Include="System\Drawing\Image.cs" />
<Compile Include="System\Drawing\ImageAnimator.cs" />
<Compile Include="System\Drawing\ImageConverter.cs" />
<Compile Include="System\Drawing\ImageFormatConverter.cs" />
<Compile Include="System\Drawing\ImageInfo.cs" />
<Compile Include="System\Drawing\ImageType.cs" />
<Compile Include="System\Drawing\NumericsExtensions.cs" />
<Compile Include="System\Drawing\Pen.cs" />
Expand Down Expand Up @@ -194,8 +196,6 @@
<Compile Include="System\Drawing\GraphicsContext.cs" />
<Compile Include="System\Drawing\Icon.Windows.cs" />
<Compile Include="System\Drawing\Image.Windows.cs" />
<Compile Include="System\Drawing\ImageAnimator.Windows.cs" />
<Compile Include="System\Drawing\ImageInfo.cs" />
<Compile Include="System\Drawing\Imaging\BitmapData.Windows.cs" />
<Compile Include="System\Drawing\Imaging\Metafile.Windows.cs" />
<Compile Include="System\Drawing\Imaging\MetafileHeader.Windows.cs" />
Expand Down Expand Up @@ -344,7 +344,6 @@
<Compile Include="System\Drawing\GdiPlusStreamHelper.Unix.cs" />
<Compile Include="System\Drawing\LibX11Functions.cs" />
<Compile Include="System\Drawing\MarshallingHelpers.cs" />
<Compile Include="System\Drawing\ImageAnimator.Unix.cs" />
<Compile Include="System\Drawing\Image.Unix.cs" />
<Compile Include="System\Drawing\Pen.Unix.cs" />
<Compile Include="System\Drawing\Region.Unix.cs" />
Expand Down
194 changes: 0 additions & 194 deletions src/libraries/System.Drawing.Common/src/System/Drawing/ImageAnimator.Unix.cs
View file
Open in desktop

This file was deleted.

44 changes: 18 additions & 26 deletions ...c/System/Drawing/ImageAnimator.Windows.cs → ...ommon/src/System/Drawing/ImageAnimator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ private ImageAnimator()
/// </summary>
public static void UpdateFrames(Image image)
{
if (!s_anyFrameDirty || image == null || s_imageInfoList == null)
if (image == null || s_imageInfoList == null)
{
return;
}
Expand Down Expand Up @@ -130,10 +130,10 @@ public static void UpdateFrames(Image image)
imageInfo.UpdateFrame();
}
}

foundImage = true;
}

if (imageInfo.FrameDirty)
else if (imageInfo.FrameDirty)
{
foundDirty = true;
}
Expand Down Expand Up @@ -161,6 +161,7 @@ public static void UpdateFrames()
{
return;
}

if (t_threadWriterLockWaitCount > 0)
{
// Cannot acquire reader lock at this time, frames update will be missed.
Expand All @@ -179,6 +180,7 @@ public static void UpdateFrames()
imageInfo.UpdateFrame();
}
}

s_anyFrameDirty = false;
}
finally
Expand Down Expand Up @@ -257,7 +259,7 @@ public static void Animate(Image image, EventHandler onFrameChangedHandler)
//
if (s_animationThread == null)
{
s_animationThread = new Thread(new ThreadStart(AnimateImages50ms));
s_animationThread = new Thread(new ThreadStart(AnimateImages));
s_animationThread.Name = nameof(ImageAnimator);
s_animationThread.IsBackground = true;
s_animationThread.Start();
Expand Down Expand Up @@ -370,21 +372,28 @@ public static void StopAnimate(Image image, EventHandler onFrameChangedHandler)
}
}


/// <summary>
/// Worker thread procedure which implements the main animation loop.
/// NOTE: This is the ONLY code the worker thread executes, keeping it in one method helps better understand
/// any synchronization issues.
/// WARNING: Also, this is the only place where ImageInfo objects (not the contained image object) are modified,
/// so no access synchronization is required to modify them.
/// </summary>
private static void AnimateImages50ms()
private static void AnimateImages()
{
Debug.Assert(s_imageInfoList != null, "Null images list");
Stopwatch stopwatch = new Stopwatch();
stopwatch.Start();
Comment on lines +385 to +386
Copy link
Member

Choose a reason for hiding this comment

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

Nit:

Stopwatch stopwatch = Stopwatch.StartNew();


while (true)
{
// Acquire reader-lock to access imageInfoList, elemens in the list can be modified w/o needing a writer-lock.
Thread.Sleep(40);

// Because Thread.Sleep is not accurate, capture how much time has actually elapsed during the animation
long timeElapsed = stopwatch.ElapsedMilliseconds;
stopwatch.Restart();

// Acquire reader-lock to access imageInfoList, elements in the list can be modified w/o needing a writer-lock.
// Observe that we don't need to check if the thread is waiting or a writer lock here since the thread this
// method runs in never acquires a writer lock.
s_rwImgListLock.AcquireReaderLock(Timeout.Infinite);
Expand All @@ -394,24 +403,9 @@ private static void AnimateImages50ms()
{
ImageInfo imageInfo = s_imageInfoList[i];

// Frame delay is measured in 1/100ths of a second. This thread
// sleeps for 50 ms = 5/100ths of a second between frame updates,
// so we increase the frame delay count 5/100ths of a second
// at a time.
//
imageInfo.FrameTimer += 5;
if (imageInfo.FrameTimer >= imageInfo.FrameDelay(imageInfo.Frame))
if (imageInfo.Animated)
{
imageInfo.FrameTimer = 0;

if (imageInfo.Frame + 1 < imageInfo.FrameCount)
{
imageInfo.Frame++;
}
else
{
imageInfo.Frame = 0;
}
imageInfo.AdvanceAnimationBy(timeElapsed);

if (imageInfo.FrameDirty)
{
Expand All @@ -424,8 +418,6 @@ private static void AnimateImages50ms()
{
s_rwImgListLock.ReleaseReaderLock();
}

Thread.Sleep(50);
}
}
}
Expand Down
Loading