Skip to content

Commit a888544

Browse files
JimBobSquarePantsJimBobSquarePants
committed
Fix GIF handling of unused global tables.
1 parent 6f38753 commit a888544

File tree

6 files changed

+54
-24
lines changed

6 files changed

+54
-24
lines changed

‎src/ImageSharp/Formats/Gif/GifEncoderCore.cs

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -88,18 +88,24 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
8888
GifMetadata gifMetadata = GetGifMetadata(image);
8989
this.colorTableMode ??= gifMetadata.ColorTableMode;
9090
bool useGlobalTable = this.colorTableMode == GifColorTableMode.Global;
91-
92-
// Quantize the first image frame returning a palette.
93-
IndexedImageFrame<TPixel>? quantized = null;
91+
bool useGlobalTableForFirstFrame = useGlobalTable;
9492

9593
// Work out if there is an explicit transparent index set for the frame. We use that to ensure the
9694
// correct value is set for the background index when quantizing.
9795
GifFrameMetadata frameMetadata = GetGifFrameMetadata(image.Frames.RootFrame, -1);
96+
if (frameMetadata.ColorTableMode == GifColorTableMode.Local)
97+
{
98+
useGlobalTableForFirstFrame = false;
99+
}
100+
101+
// TODO: The first frame should not need to be quantized using the global table if it has a local color table.
98102

103+
// Quantize the first image frame returning a palette.
104+
IndexedImageFrame<TPixel>? quantized = null;
99105
if (this.quantizer is null)
100106
{
101107
// Is this a gif with color information. If so use that, otherwise use octree.
102-
if (gifMetadata.ColorTableMode == GifColorTableMode.Global && gifMetadata.GlobalColorTable?.Length > 0)
108+
if (useGlobalTable && gifMetadata.GlobalColorTable?.Length > 0)
103109
{
104110
// We avoid dithering by default to preserve the original colors.
105111
int transparencyIndex = GetTransparentIndex(quantized, frameMetadata);
@@ -118,8 +124,9 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
118124
}
119125
}
120126

121-
using (IQuantizer<TPixel> frameQuantizer = this.quantizer.CreatePixelSpecificQuantizer<TPixel>(this.configuration))
127+
if (useGlobalTableForFirstFrame)
122128
{
129+
using IQuantizer<TPixel> frameQuantizer = this.quantizer.CreatePixelSpecificQuantizer<TPixel>(this.configuration);
123130
if (useGlobalTable)
124131
{
125132
frameQuantizer.BuildPalette(this.pixelSamplingStrategy, image);
@@ -131,6 +138,17 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
131138
quantized = frameQuantizer.QuantizeFrame(image.Frames.RootFrame, image.Bounds);
132139
}
133140
}
141+
else
142+
{
143+
quantized = this.QuantizeAdditionalFrameAndUpdateMetadata(
144+
image.Frames.RootFrame,
145+
image.Frames.RootFrame.Bounds(),
146+
frameMetadata,
147+
true,
148+
default,
149+
false,
150+
frameMetadata.HasTransparency ? frameMetadata.TransparencyIndex : -1);
151+
}
134152

135153
// Write the header.
136154
WriteHeader(stream);
@@ -243,8 +261,8 @@ private void EncodeAdditionalFrames<TPixel>(
243261
return;
244262
}
245263

246-
PaletteQuantizer<TPixel> paletteQuantizer = default;
247-
bool hasPaletteQuantizer = false;
264+
PaletteQuantizer<TPixel> globalPaletteQuantizer = default;
265+
bool hasGlobalPaletteQuantizer = false;
248266

249267
// Store the first frame as a reference for de-duplication comparison.
250268
ImageFrame<TPixel> previousFrame = image.Frames.RootFrame;
@@ -260,14 +278,14 @@ private void EncodeAdditionalFrames<TPixel>(
260278
GifFrameMetadata gifMetadata = GetGifFrameMetadata(currentFrame, globalTransparencyIndex);
261279
bool useLocal = this.colorTableMode == GifColorTableMode.Local || (gifMetadata.ColorTableMode == GifColorTableMode.Local);
262280

263-
if (!useLocal && !hasPaletteQuantizer && i > 0)
281+
if (!useLocal && !hasGlobalPaletteQuantizer && i > 0)
264282
{
265283
// The palette quantizer can reuse the same global pixel map across multiple frames since the palette is unchanging.
266284
// This allows a reduction of memory usage across multi-frame gifs using a global palette
267285
// and also allows use to reuse the cache from previous runs.
268286
int transparencyIndex = gifMetadata.HasTransparency ? gifMetadata.TransparencyIndex : -1;
269-
paletteQuantizer = new(this.configuration, this.quantizer!.Options, globalPalette, transparencyIndex);
270-
hasPaletteQuantizer = true;
287+
globalPaletteQuantizer = new(this.configuration, this.quantizer!.Options, globalPalette, transparencyIndex);
288+
hasGlobalPaletteQuantizer = true;
271289
}
272290

273291
this.EncodeAdditionalFrame(
@@ -278,16 +296,16 @@ private void EncodeAdditionalFrames<TPixel>(
278296
encodingFrame,
279297
useLocal,
280298
gifMetadata,
281-
paletteQuantizer,
299+
globalPaletteQuantizer,
282300
previousDisposalMethod);
283301

284302
previousFrame = currentFrame;
285303
previousDisposalMethod = gifMetadata.DisposalMethod;
286304
}
287305

288-
if (hasPaletteQuantizer)
306+
if (hasGlobalPaletteQuantizer)
289307
{
290-
paletteQuantizer.Dispose();
308+
globalPaletteQuantizer.Dispose();
291309
}
292310
}
293311

‎src/ImageSharp/Formats/Gif/MetadataExtensions.cs

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -60,21 +60,15 @@ public static bool TryGetGifMetadata(this ImageFrameMetadata source, [NotNullWhe
6060
=> source.TryGetFormatMetadata(GifFormat.Instance, out metadata);
6161

6262
internal static AnimatedImageMetadata ToAnimatedImageMetadata(this GifMetadata source)
63-
{
64-
Color background = Color.Transparent;
65-
if (source.GlobalColorTable != null)
66-
{
67-
background = source.GlobalColorTable.Value.Span[source.BackgroundColorIndex];
68-
}
6963

70-
return new()
64+
// We cannot trust the global GIF palette so don't use it.
65+
// https://github.com/SixLabors/ImageSharp/issues/2866
66+
=> new()
7167
{
72-
ColorTable = source.GlobalColorTable,
7368
ColorTableMode = source.ColorTableMode == GifColorTableMode.Global ? FrameColorTableMode.Global : FrameColorTableMode.Local,
7469
RepeatCount = source.RepeatCount,
75-
BackgroundColor = background,
70+
BackgroundColor = Color.Transparent,
7671
};
77-
}
7872

7973
internal static AnimatedImageFrameMetadata ToAnimatedImageFrameMetadata(this GifFrameMetadata source)
8074
{

‎tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using SixLabors.ImageSharp.PixelFormats;
99
using SixLabors.ImageSharp.Processing.Processors.Quantization;
1010
using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison;
11+
using System.Linq;
1112

1213
// ReSharper disable InconsistentNaming
1314
namespace SixLabors.ImageSharp.Tests.Formats.Gif;
@@ -382,4 +383,17 @@ public void Encode_Animated_VisualTest<TPixel>(TestImageProvider<TPixel> provide
382383
provider.Utility.SaveTestOutputFile(image, "png", new PngEncoder(), "animated");
383384
provider.Utility.SaveTestOutputFile(image, "gif", new GifEncoder(), "animated");
384385
}
386+
387+
[Theory]
388+
[WithFile(TestImages.Gif.Issues.Issue2866, PixelTypes.Rgba32)]
389+
public void GifEncoder_CanDecode_Issue2866<TPixel>(TestImageProvider<TPixel> provider)
390+
where TPixel : unmanaged, IPixel<TPixel>
391+
{
392+
using Image<TPixel> image = provider.GetImage();
393+
394+
bool anyGlobal = ((IEnumerable<ImageFrame>)image.Frames).Any(x => x.Metadata.GetGifMetadata().ColorTableMode == GifColorTableMode.Global);
395+
396+
// image.DebugSaveMultiFrame(provider);
397+
provider.Utility.SaveTestOutputFile(image, "gif", new GifEncoder(), "animated");
398+
}
385399
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,7 @@ public void Encode_APng<TPixel>(TestImageProvider<TPixel> provider)
485485

486486
[Theory]
487487
[WithFile(TestImages.Gif.Leo, PixelTypes.Rgba32)]
488+
[WithFile(TestImages.Gif.Issues.Issue2866, PixelTypes.Rgba32)]
488489
public void Encode_AnimatedFormatTransform_FromGif<TPixel>(TestImageProvider<TPixel> provider)
489490
where TPixel : unmanaged, IPixel<TPixel>
490491
{
@@ -494,7 +495,6 @@ public void Encode_AnimatedFormatTransform_FromGif<TPixel>(TestImageProvider<TPi
494495
}
495496

496497
using Image<TPixel> image = provider.GetImage(GifDecoder.Instance);
497-
498498
using MemoryStream memStream = new();
499499
image.Save(memStream, PngEncoder);
500500
memStream.Position = 0;

‎tests/ImageSharp.Tests/TestImages.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,7 @@ public static class Issues
535535
public const string Issue2450_B = "Gif/issues/issue_2450_2.gif";
536536
public const string Issue2198 = "Gif/issues/issue_2198.gif";
537537
public const string Issue2758 = "Gif/issues/issue_2758.gif";
538+
public const string Issue2866 = "Gif/issues/issue_2866.gif";
538539
}
539540

540541
public static readonly string[] Animated =

‎tests/Images/Input/Gif/issues/issue_2866.gif

Lines changed: 3 additions & 0 deletions
Loading

0 commit comments

Comments
 (0)