Skip to content

Commit 8546c74

Browse files
JimBobSquarePantsJimBobSquarePants
authored
Merge pull request #2710 from SpaceCheetah/FixPNG
Fix animated png handling (issue #2708)
2 parents 85e3be1 + 94d7f3c commit 8546c74

18 files changed

Lines changed: 157 additions & 32 deletions

File tree

‎src/ImageSharp/Formats/Png/PngDecoderCore.cs‎

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -234,8 +234,7 @@ public Image<TPixel> Decode<TPixel>(BufferedReadStream stream, CancellationToken
234234
PngThrowHelper.ThrowMissingFrameControl();
235235
}
236236

237-
previousFrameControl ??= new((uint)this.header.Width, (uint)this.header.Height);
238-
this.InitializeFrame(previousFrameControl.Value, currentFrameControl.Value, image, previousFrame, out currentFrame);
237+
this.InitializeFrame(previousFrameControl, currentFrameControl.Value, image, previousFrame, out currentFrame);
239238

240239
this.currentStream.Position += 4;
241240
this.ReadScanlines(
@@ -246,11 +245,16 @@ public Image<TPixel> Decode<TPixel>(BufferedReadStream stream, CancellationToken
246245
currentFrameControl.Value,
247246
cancellationToken);
248247

249-
previousFrame = currentFrame;
250-
previousFrameControl = currentFrameControl;
248+
// if current frame dispose is restore to previous, then from future frame's perspective, it never happened
249+
if (currentFrameControl.Value.DisposeOperation != PngDisposalMethod.RestoreToPrevious)
250+
{
251+
previousFrame = currentFrame;
252+
previousFrameControl = currentFrameControl;
253+
}
254+
251255
break;
252256
case PngChunkType.Data:
253-
257+
pngMetadata.AnimateRootFrame = currentFrameControl != null;
254258
currentFrameControl ??= new((uint)this.header.Width, (uint)this.header.Height);
255259
if (image is null)
256260
{
@@ -267,9 +271,12 @@ public Image<TPixel> Decode<TPixel>(BufferedReadStream stream, CancellationToken
267271
this.ReadNextDataChunk,
268272
currentFrameControl.Value,
269273
cancellationToken);
274+
if (pngMetadata.AnimateRootFrame)
275+
{
276+
previousFrame = currentFrame;
277+
previousFrameControl = currentFrameControl;
278+
}
270279

271-
previousFrame = currentFrame;
272-
previousFrameControl = currentFrameControl;
273280
break;
274281
case PngChunkType.Palette:
275282
this.palette = chunk.Data.GetSpan().ToArray();
@@ -638,7 +645,7 @@ private void InitializeImage<TPixel>(ImageMetadata metadata, FrameControl frameC
638645
/// <param name="previousFrame">The previous frame.</param>
639646
/// <param name="frame">The created frame</param>
640647
private void InitializeFrame<TPixel>(
641-
FrameControl previousFrameControl,
648+
FrameControl? previousFrameControl,
642649
FrameControl currentFrameControl,
643650
Image<TPixel> image,
644651
ImageFrame<TPixel>? previousFrame,
@@ -651,12 +658,16 @@ private void InitializeFrame<TPixel>(
651658
frame = image.Frames.AddFrame(previousFrame ?? image.Frames.RootFrame);
652659

653660
// If the first `fcTL` chunk uses a `dispose_op` of APNG_DISPOSE_OP_PREVIOUS it should be treated as APNG_DISPOSE_OP_BACKGROUND.
654-
if (previousFrameControl.DisposeOperation == PngDisposalMethod.RestoreToBackground
655-
|| (previousFrame is null && previousFrameControl.DisposeOperation == PngDisposalMethod.RestoreToPrevious))
661+
// So, if restoring to before first frame, clear entire area. Same if first frame (previousFrameControl null).
662+
if (previousFrameControl == null || (previousFrame is null && previousFrameControl.Value.DisposeOperation == PngDisposalMethod.RestoreToPrevious))
663+
{
664+
Buffer2DRegion<TPixel> pixelRegion = frame.PixelBuffer.GetRegion();
665+
pixelRegion.Clear();
666+
}
667+
else if (previousFrameControl.Value.DisposeOperation == PngDisposalMethod.RestoreToBackground)
656668
{
657-
Rectangle restoreArea = previousFrameControl.Bounds;
658-
Rectangle interest = Rectangle.Intersect(frame.Bounds(), restoreArea);
659-
Buffer2DRegion<TPixel> pixelRegion = frame.PixelBuffer.GetRegion(interest);
669+
Rectangle restoreArea = previousFrameControl.Value.Bounds;
670+
Buffer2DRegion<TPixel> pixelRegion = frame.PixelBuffer.GetRegion(restoreArea);
660671
pixelRegion.Clear();
661672
}
662673

‎src/ImageSharp/Formats/Png/PngEncoderCore.cs‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
167167

168168
ImageFrame<TPixel>? clonedFrame = null;
169169
ImageFrame<TPixel> currentFrame = image.Frames.RootFrame;
170+
int currentFrameIndex = 0;
170171

171172
bool clearTransparency = this.encoder.TransparentColorMode is PngTransparentColorMode.Clear;
172173
if (clearTransparency)
@@ -195,29 +196,50 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
195196

196197
if (image.Frames.Count > 1)
197198
{
198-
this.WriteAnimationControlChunk(stream, (uint)image.Frames.Count, pngMetadata.RepeatCount);
199+
this.WriteAnimationControlChunk(stream, (uint)(image.Frames.Count - (pngMetadata.AnimateRootFrame ? 0 : 1)), pngMetadata.RepeatCount);
200+
}
201+
202+
// If the first frame isn't animated, write it as usual and skip it when writing animated frames
203+
if (!pngMetadata.AnimateRootFrame || image.Frames.Count == 1)
204+
{
205+
FrameControl frameControl = new((uint)this.width, (uint)this.height);
206+
this.WriteDataChunks(frameControl, currentFrame.PixelBuffer.GetRegion(), quantized, stream, false);
207+
currentFrameIndex++;
208+
}
199209

200-
// Write the first frame.
210+
if (image.Frames.Count > 1)
211+
{
212+
// Write the first animated frame.
213+
currentFrame = image.Frames[currentFrameIndex];
201214
PngFrameMetadata frameMetadata = GetPngFrameMetadata(currentFrame);
202215
PngDisposalMethod previousDisposal = frameMetadata.DisposalMethod;
203216
FrameControl frameControl = this.WriteFrameControlChunk(stream, frameMetadata, currentFrame.Bounds(), 0);
204-
this.WriteDataChunks(frameControl, currentFrame.PixelBuffer.GetRegion(), quantized, stream, false);
217+
uint sequenceNumber = 1;
218+
if (pngMetadata.AnimateRootFrame)
219+
{
220+
this.WriteDataChunks(frameControl, currentFrame.PixelBuffer.GetRegion(), quantized, stream, false);
221+
}
222+
else
223+
{
224+
sequenceNumber += this.WriteDataChunks(frameControl, currentFrame.PixelBuffer.GetRegion(), quantized, stream, true);
225+
}
226+
227+
currentFrameIndex++;
205228

206229
// Capture the global palette for reuse on subsequent frames.
207230
ReadOnlyMemory<TPixel>? previousPalette = quantized?.Palette.ToArray();
208231

209232
// Write following frames.
210-
uint increment = 0;
211233
ImageFrame<TPixel> previousFrame = image.Frames.RootFrame;
212234

213235
// This frame is reused to store de-duplicated pixel buffers.
214236
using ImageFrame<TPixel> encodingFrame = new(image.Configuration, previousFrame.Size());
215237

216-
for (int i = 1; i < image.Frames.Count; i++)
238+
for (; currentFrameIndex < image.Frames.Count; currentFrameIndex++)
217239
{
218240
ImageFrame<TPixel>? prev = previousDisposal == PngDisposalMethod.RestoreToBackground ? null : previousFrame;
219-
currentFrame = image.Frames[i];
220-
ImageFrame<TPixel>? nextFrame = i < image.Frames.Count - 1 ? image.Frames[i + 1] : null;
241+
currentFrame = image.Frames[currentFrameIndex];
242+
ImageFrame<TPixel>? nextFrame = currentFrameIndex < image.Frames.Count - 1 ? image.Frames[currentFrameIndex + 1] : null;
221243

222244
frameMetadata = GetPngFrameMetadata(currentFrame);
223245
bool blend = frameMetadata.BlendMethod == PngBlendMethod.Over;
@@ -238,22 +260,17 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
238260
}
239261

240262
// Each frame control sequence number must be incremented by the number of frame data chunks that follow.
241-
frameControl = this.WriteFrameControlChunk(stream, frameMetadata, bounds, (uint)i + increment);
263+
frameControl = this.WriteFrameControlChunk(stream, frameMetadata, bounds, sequenceNumber);
242264

243265
// Dispose of previous quantized frame and reassign.
244266
quantized?.Dispose();
245267
quantized = this.CreateQuantizedImageAndUpdateBitDepth(pngMetadata, encodingFrame, bounds, previousPalette);
246-
increment += this.WriteDataChunks(frameControl, encodingFrame.PixelBuffer.GetRegion(bounds), quantized, stream, true);
268+
sequenceNumber += this.WriteDataChunks(frameControl, encodingFrame.PixelBuffer.GetRegion(bounds), quantized, stream, true) + 1;
247269

248270
previousFrame = currentFrame;
249271
previousDisposal = frameMetadata.DisposalMethod;
250272
}
251273
}
252-
else
253-
{
254-
FrameControl frameControl = new((uint)this.width, (uint)this.height);
255-
this.WriteDataChunks(frameControl, currentFrame.PixelBuffer.GetRegion(), quantized, stream, false);
256-
}
257274

258275
this.WriteEndChunk(stream);
259276

‎src/ImageSharp/Formats/Png/PngMetadata.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ private PngMetadata(PngMetadata other)
2929
this.InterlaceMethod = other.InterlaceMethod;
3030
this.TransparentColor = other.TransparentColor;
3131
this.RepeatCount = other.RepeatCount;
32+
this.AnimateRootFrame = other.AnimateRootFrame;
3233

3334
if (other.ColorTable?.Length > 0)
3435
{
@@ -83,6 +84,11 @@ private PngMetadata(PngMetadata other)
8384
/// </summary>
8485
public uint RepeatCount { get; set; } = 1;
8586

87+
/// <summary>
88+
/// Gets or sets a value indicating whether the root frame is shown as part of the animated sequence
89+
/// </summary>
90+
public bool AnimateRootFrame { get; set; } = true;
91+
8692
/// <inheritdoc/>
8793
public IDeepCloneable DeepClone() => new PngMetadata(this);
8894

‎src/ImageSharp/Formats/Png/PngScanlineProcessor.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,9 @@ public static void ProcessInterlacedPaletteScanline<TPixel>(
180180
ref byte scanlineSpanRef = ref MemoryMarshal.GetReference(scanlineSpan);
181181
ref TPixel rowSpanRef = ref MemoryMarshal.GetReference(rowSpan);
182182
ref Color paletteBase = ref MemoryMarshal.GetReference(palette.Value.Span);
183+
uint offset = pixelOffset + frameControl.XOffset;
183184

184-
for (nuint x = pixelOffset, o = 0; x < frameControl.XMax; x += increment, o++)
185+
for (nuint x = offset, o = 0; x < frameControl.XMax; x += increment, o++)
185186
{
186187
uint index = Unsafe.Add(ref scanlineSpanRef, o);
187188
Unsafe.Add(ref rowSpanRef, x) = TPixel.FromRgba32(Unsafe.Add(ref paletteBase, index).ToPixel<Rgba32>());

‎tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,9 @@ public partial class PngDecoderTests
8787
TestImages.Png.DisposeBackgroundRegion,
8888
TestImages.Png.DisposePreviousFirst,
8989
TestImages.Png.DisposeBackgroundBeforeRegion,
90-
TestImages.Png.BlendOverMultiple
90+
TestImages.Png.BlendOverMultiple,
91+
TestImages.Png.FrameOffset,
92+
TestImages.Png.DefaultNotAnimated
9193
};
9294

9395
[Theory]

‎tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System.Buffers.Binary;
55
using SixLabors.ImageSharp.Formats.Png;
6+
using SixLabors.ImageSharp.Formats.Png.Chunks;
67
using SixLabors.ImageSharp.PixelFormats;
78

89
// ReSharper disable InconsistentNaming
@@ -59,6 +60,38 @@ public void EndChunk_IsLast()
5960
}
6061
}
6162

63+
[Theory]
64+
[WithFile(TestImages.Png.DefaultNotAnimated, PixelTypes.Rgba32)]
65+
[WithFile(TestImages.Png.APng, PixelTypes.Rgba32)]
66+
public void AcTL_CorrectlyWritten<TPixel>(TestImageProvider<TPixel> provider)
67+
where TPixel : unmanaged, IPixel<TPixel>
68+
{
69+
using Image<TPixel> image = provider.GetImage(PngDecoder.Instance);
70+
PngMetadata metadata = image.Metadata.GetPngMetadata();
71+
int correctFrameCount = image.Frames.Count - (metadata.AnimateRootFrame ? 0 : 1);
72+
using MemoryStream memStream = new();
73+
image.Save(memStream, PngEncoder);
74+
memStream.Position = 0;
75+
Span<byte> bytesSpan = memStream.ToArray().AsSpan(8); // Skip header.
76+
bool foundAcTl = false;
77+
while (bytesSpan.Length > 0 && !foundAcTl)
78+
{
79+
int length = BinaryPrimitives.ReadInt32BigEndian(bytesSpan[..4]);
80+
PngChunkType type = (PngChunkType)BinaryPrimitives.ReadInt32BigEndian(bytesSpan.Slice(4, 4));
81+
if (type == PngChunkType.AnimationControl)
82+
{
83+
AnimationControl control = AnimationControl.Parse(bytesSpan[8..]);
84+
foundAcTl = true;
85+
Assert.True(control.NumberFrames == correctFrameCount);
86+
Assert.True(control.NumberPlays == metadata.RepeatCount);
87+
}
88+
89+
bytesSpan = bytesSpan[(4 + 4 + length + 4)..];
90+
}
91+
92+
Assert.True(foundAcTl);
93+
}
94+
6295
[Theory]
6396
[InlineData(PngChunkType.Gamma)]
6497
[InlineData(PngChunkType.Chroma)]

‎tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,8 @@ public void Encode_WithPngTransparentColorBehaviorClear_Works(PngColorType color
448448

449449
[Theory]
450450
[WithFile(TestImages.Png.APng, PixelTypes.Rgba32)]
451+
[WithFile(TestImages.Png.DefaultNotAnimated, PixelTypes.Rgba32)]
452+
[WithFile(TestImages.Png.FrameOffset, PixelTypes.Rgba32)]
451453
public void Encode_APng<TPixel>(TestImageProvider<TPixel> provider)
452454
where TPixel : unmanaged, IPixel<TPixel>
453455
{
@@ -459,15 +461,17 @@ public void Encode_APng<TPixel>(TestImageProvider<TPixel> provider)
459461
image.DebugSave(provider: provider, encoder: PngEncoder, null, false);
460462

461463
using Image<Rgba32> output = Image.Load<Rgba32>(memStream);
462-
ImageComparer.Exact.VerifySimilarity(output, image);
463464

464-
Assert.Equal(5, image.Frames.Count);
465+
// some loss from original, due to compositing
466+
ImageComparer.TolerantPercentage(0.01f).VerifySimilarity(output, image);
467+
465468
Assert.Equal(image.Frames.Count, output.Frames.Count);
466469

467470
PngMetadata originalMetadata = image.Metadata.GetPngMetadata();
468471
PngMetadata outputMetadata = output.Metadata.GetPngMetadata();
469472

470473
Assert.Equal(originalMetadata.RepeatCount, outputMetadata.RepeatCount);
474+
Assert.Equal(originalMetadata.AnimateRootFrame, outputMetadata.AnimateRootFrame);
471475

472476
for (int i = 0; i < image.Frames.Count; i++)
473477
{

‎tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ public void CloneIsDeep()
3232
InterlaceMethod = PngInterlaceMode.Adam7,
3333
Gamma = 2,
3434
TextData = new List<PngTextData> { new PngTextData("name", "value", "foo", "bar") },
35-
RepeatCount = 123
35+
RepeatCount = 123,
36+
AnimateRootFrame = false
3637
};
3738

3839
PngMetadata clone = (PngMetadata)meta.DeepClone();
@@ -44,6 +45,7 @@ public void CloneIsDeep()
4445
Assert.False(meta.TextData.Equals(clone.TextData));
4546
Assert.True(meta.TextData.SequenceEqual(clone.TextData));
4647
Assert.True(meta.RepeatCount == clone.RepeatCount);
48+
Assert.True(meta.AnimateRootFrame == clone.AnimateRootFrame);
4749

4850
clone.BitDepth = PngBitDepth.Bit2;
4951
clone.ColorType = PngColorType.Palette;
@@ -144,6 +146,26 @@ public void Decode_ReadsExifData<TPixel>(TestImageProvider<TPixel> provider)
144146
VerifyExifDataIsPresent(exif);
145147
}
146148

149+
[Theory]
150+
[WithFile(TestImages.Png.DefaultNotAnimated, PixelTypes.Rgba32)]
151+
public void Decode_IdentifiesDefaultFrameNotAnimated<TPixel>(TestImageProvider<TPixel> provider)
152+
where TPixel : unmanaged, IPixel<TPixel>
153+
{
154+
using Image<TPixel> image = provider.GetImage(PngDecoder.Instance);
155+
PngMetadata meta = image.Metadata.GetFormatMetadata(PngFormat.Instance);
156+
Assert.False(meta.AnimateRootFrame);
157+
}
158+
159+
[Theory]
160+
[WithFile(TestImages.Png.APng, PixelTypes.Rgba32)]
161+
public void Decode_IdentifiesDefaultFrameAnimated<TPixel>(TestImageProvider<TPixel> provider)
162+
where TPixel : unmanaged, IPixel<TPixel>
163+
{
164+
using Image<TPixel> image = provider.GetImage(PngDecoder.Instance);
165+
PngMetadata meta = image.Metadata.GetFormatMetadata(PngFormat.Instance);
166+
Assert.True(meta.AnimateRootFrame);
167+
}
168+
147169
[Theory]
148170
[WithFile(TestImages.Png.PngWithMetadata, PixelTypes.Rgba32)]
149171
public void Decode_IgnoresExifData_WhenIgnoreMetadataIsTrue<TPixel>(TestImageProvider<TPixel> provider)

‎tests/ImageSharp.Tests/TestImages.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ public static class Png
7373
public const string DisposeBackgroundRegion = "Png/animated/15-dispose-background-region.png";
7474
public const string DisposePreviousFirst = "Png/animated/12-dispose-prev-first.png";
7575
public const string BlendOverMultiple = "Png/animated/21-blend-over-multiple.png";
76+
public const string FrameOffset = "Png/animated/frame-offset.png";
77+
public const string DefaultNotAnimated = "Png/animated/default-not-animated.png";
7678
public const string Issue2666 = "Png/issues/Issue_2666.png";
7779

7880
// Filtered test images from http://www.schaik.com/pngsuite/pngsuite_fil_png.html

‎tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_default-not-animated.png/00.png‎

Lines changed: 3 additions & 0 deletions
Loading

0 commit comments

Comments
 (0)