Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
73 changes: 48 additions & 25 deletions src/ImageSharp/Formats/Gif/GifDecoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,12 @@ private bool ReadFrame<TPixel>(
backgroundColor = Color.Transparent;
}

// We zero the alpha only when this frame declares transparency so that
// frames with a transparent index coalesce over a transparent canvas rather than
// baking the LSD background as a matte. When the flag is not set, this frame will
// write an opaque color for every addressed pixel; keeping the LSD background
// opaque here allows ReadFrameColors to show that background in uncovered areas
// for non-transparent GIFs that rely on it. We still do not prefill the canvas here.
if (this.graphicsControlExtension.TransparencyFlag)
{
backgroundColor = backgroundColor.WithAlpha(0);
Expand All @@ -498,24 +504,18 @@ private bool ReadFrame<TPixel>(
this.ReadFrameColors(stream, ref image, ref previousFrame, ref previousDisposalMode, colorTable, backgroundColor.ToPixel<TPixel>());

// Update from newly decoded frame.
if (this.graphicsControlExtension.DisposalMethod != FrameDisposalMode.RestoreToPrevious)
FrameDisposalMode disposalMethod = this.graphicsControlExtension.DisposalMethod;
if (disposalMethod != FrameDisposalMode.RestoreToPrevious)
{
if (this.backgroundColorIndex < colorTable.Length)
{
backgroundColor = Color.FromPixel(colorTable[this.backgroundColorIndex]);
}
else
{
backgroundColor = Color.Transparent;
}

// TODO: I don't understand why this is always set to alpha of zero.
// This should be dependent on the transparency flag of the graphics
// control extension. ImageMagick does the same.
// if (this.graphicsControlExtension.TransparencyFlag)
{
backgroundColor = backgroundColor.WithAlpha(0);
}
// Do not key this on the transparency flag. Disposal handling is determined by
// the previous frame's disposal, not by whether the current frame declares a transparent
// index. For editing we carry a transparent background so that RestoreToBackground clears
// remove pixels to transparent rather than painting an opaque matte. The LSD background
// color is display advice and should be used only when explicitly flattening or when
// rendering with an option to honor it.
backgroundColor = (this.backgroundColorIndex < colorTable.Length)
? Color.FromPixel(colorTable[this.backgroundColorIndex]).WithAlpha(0)
: Color.Transparent;
}

// Skip any remaining blocks
Expand Down Expand Up @@ -546,30 +546,45 @@ private void ReadFrameColors<TPixel>(
GifImageDescriptor descriptor = this.imageDescriptor;
int imageWidth = this.logicalScreenDescriptor.Width;
int imageHeight = this.logicalScreenDescriptor.Height;
bool transFlag = this.graphicsControlExtension.TransparencyFlag;
bool useTransparency = this.graphicsControlExtension.TransparencyFlag;
bool useBackground;
FrameDisposalMode disposalMethod = this.graphicsControlExtension.DisposalMethod;
ImageFrame<TPixel> currentFrame;
ImageFrame<TPixel>? restoreFrame = null;

if (previousFrame is null && previousDisposalMode is null)
{
image = transFlag
? new Image<TPixel>(this.configuration, imageWidth, imageHeight, this.metadata)
: new Image<TPixel>(this.configuration, imageWidth, imageHeight, backgroundPixel, this.metadata);
// First frame: prefill with LSD background iff a GCT exists (policy: HonorBackgroundColor).
useBackground =
this.logicalScreenDescriptor.GlobalColorTableFlag
&& disposalMethod == FrameDisposalMode.RestoreToBackground;

image = useBackground
? new Image<TPixel>(this.configuration, imageWidth, imageHeight, backgroundPixel, this.metadata)
: new Image<TPixel>(this.configuration, imageWidth, imageHeight, this.metadata);

this.SetFrameMetadata(image.Frames.RootFrame.Metadata);
currentFrame = image.Frames.RootFrame;
}
else
{
// Subsequent frames: use LSD background iff previous disposal was RestoreToBackground and a GCT exists.
useBackground =
this.logicalScreenDescriptor.GlobalColorTableFlag
&& previousDisposalMode == FrameDisposalMode.RestoreToBackground;

if (previousFrame != null)
{
currentFrame = image!.Frames.AddFrame(previousFrame);
}
else
else if (useBackground)
{
currentFrame = image!.Frames.CreateFrame(backgroundPixel);
}
else
{
currentFrame = image!.Frames.CreateFrame();
}

this.SetFrameMetadata(currentFrame.Metadata);

Expand All @@ -580,7 +595,7 @@ private void ReadFrameColors<TPixel>(

if (previousDisposalMode == FrameDisposalMode.RestoreToBackground)
{
this.RestoreToBackground(currentFrame, backgroundPixel, transFlag);
this.RestoreToBackground(currentFrame, backgroundPixel, !useBackground);
}
}

Expand Down Expand Up @@ -670,12 +685,18 @@ private void ReadFrameColors<TPixel>(
// Take the descriptorLeft..maxX slice of the row, so the loop can be simplified.
row = row[descriptorLeft..maxX];

if (!transFlag)
if (!useTransparency)
{
for (int x = 0; x < row.Length; x++)
{
int index = indicesRow[x];
index = Numerics.Clamp(index, 0, colorTableMaxIdx);

// Treat any out of bounds values as background.
if (index > colorTableMaxIdx)
{
index = Numerics.Clamp(index, 0, colorTableMaxIdx);
}

row[x] = TPixel.FromRgb24(colorTable[index]);
}
}
Expand All @@ -686,6 +707,8 @@ private void ReadFrameColors<TPixel>(
int index = indicesRow[x];

// Treat any out of bounds values as transparent.
// We explicitly set the pixel to transparent rather than alter the inbound
// color palette.
if (index > colorTableMaxIdx || index == transIndex)
{
continue;
Expand Down
86 changes: 45 additions & 41 deletions src/ImageSharp/Formats/Gif/GifEncoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,15 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
// Is this a gif with color information. If so use that, otherwise use octree.
if (gifMetadata.ColorTableMode == FrameColorTableMode.Global && gifMetadata.GlobalColorTable?.Length > 0)
{
int transparencyIndex = GetTransparentIndex(quantized, frameMetadata);
if (transparencyIndex >= 0 || gifMetadata.GlobalColorTable.Value.Length < 256)
int ti = GetTransparentIndex(quantized, frameMetadata);
if (ti >= 0 || gifMetadata.GlobalColorTable.Value.Length < 256)
{
// We avoid dithering by default to preserve the original colors.
globalQuantizer = new PaletteQuantizer(gifMetadata.GlobalColorTable.Value, options.DeepClone(o => o.Dither = null));
globalQuantizer = new PaletteQuantizer(
gifMetadata.GlobalColorTable.Value,
options.DeepClone(o => o.Dither = null),
ti,
Color.Transparent);
}
else
{
Expand Down Expand Up @@ -173,20 +177,14 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
WriteHeader(stream);

// Write the LSD.
int derivedTransparencyIndex = GetTransparentIndex(quantized, null);
if (derivedTransparencyIndex >= 0)
int transparencyIndex = GetTransparentIndex(quantized, null);
if (transparencyIndex >= 0)
{
frameMetadata.HasTransparency = true;
frameMetadata.TransparencyIndex = ClampIndex(derivedTransparencyIndex);
frameMetadata.TransparencyIndex = ClampIndex(transparencyIndex);
}

// TODO: We should be checking the metadata here also I think?
if (!TryGetBackgroundIndex(quantized, this.backgroundColor, out byte backgroundIndex))
{
backgroundIndex = derivedTransparencyIndex >= 0
? frameMetadata.TransparencyIndex
: gifMetadata.BackgroundColorIndex;
}
byte backgroundIndex = GetBackgroundIndex(quantized, gifMetadata, this.backgroundColor);

// Get the number of bits.
int bitDepth = ColorNumerics.GetBitsNeededForColorDepth(quantized.Palette.Length);
Expand Down Expand Up @@ -224,7 +222,7 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
image,
globalQuantizer,
globalFrameQuantizer,
derivedTransparencyIndex,
transparencyIndex,
frameMetadata.DisposalMode,
cancellationToken);
}
Expand Down Expand Up @@ -334,15 +332,23 @@ private void EncodeAdditionalFrame<TPixel>(
{
// Capture any explicit transparency index from the metadata.
// We use it to determine the value to use to replace duplicate pixels.
int transparencyIndex = metadata.HasTransparency ? metadata.TransparencyIndex : -1;
bool useTransparency = metadata.HasTransparency;
int transparencyIndex = useTransparency ? metadata.TransparencyIndex : -1;

ImageFrame<TPixel>? previous = previousDisposalMode == FrameDisposalMode.RestoreToBackground
? null :
previousFrame;

Color background = metadata.DisposalMode == FrameDisposalMode.RestoreToBackground
? this.backgroundColor ?? Color.Transparent
: Color.Transparent;
// If the previous frame has a value we need to check the disposal mode of that frame
// to determine if we should use the background color to fill the encoding frame
// when de-duplicating.
FrameDisposalMode disposalMode = previous is null ?
metadata.DisposalMode :
previous.Metadata.GetGifMetadata().DisposalMode;

Color background = !useTransparency && disposalMode == FrameDisposalMode.RestoreToBackground
? this.backgroundColor ?? Color.Transparent
: Color.Transparent;

// Deduplicate and quantize the frame capturing only required parts.
(bool difference, Rectangle bounds) =
Expand Down Expand Up @@ -491,6 +497,7 @@ private IndexedImageFrame<TPixel> QuantizeFrameAndUpdateMetadata<TPixel>(
return quantized;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static byte ClampIndex(int value) => (byte)Numerics.Clamp(value, byte.MinValue, byte.MaxValue);

/// <summary>
Expand Down Expand Up @@ -531,41 +538,38 @@ private static int GetTransparentIndex<TPixel>(IndexedImageFrame<TPixel>? quanti
/// Returns the index of the background color in the palette.
/// </summary>
/// <param name="quantized">The current quantized frame.</param>
/// <param name="metadata">The gif metadata</param>
/// <param name="background">The background color to match.</param>
/// <param name="index">The index in the palette of the background color.</param>
/// <typeparam name="TPixel">The pixel format.</typeparam>
/// <returns>The <see cref="bool"/>.</returns>
private static bool TryGetBackgroundIndex<TPixel>(
IndexedImageFrame<TPixel>? quantized,
Color? background,
out byte index)
/// <returns>The <see cref="byte"/> index of the background color.</returns>
private static byte GetBackgroundIndex<TPixel>(IndexedImageFrame<TPixel>? quantized, GifMetadata metadata, Color? background)
where TPixel : unmanaged, IPixel<TPixel>
{
int match = -1;
if (quantized != null && background.HasValue)
if (quantized != null)
{
TPixel backgroundPixel = background.Value.ToPixel<TPixel>();
ReadOnlySpan<TPixel> palette = quantized.Palette.Span;
for (int i = 0; i < palette.Length; i++)
if (background.HasValue)
{
if (!backgroundPixel.Equals(palette[i]))
TPixel backgroundPixel = background.Value.ToPixel<TPixel>();
ReadOnlySpan<TPixel> palette = quantized.Palette.Span;
for (int i = 0; i < palette.Length; i++)
{
continue;
}
if (!backgroundPixel.Equals(palette[i]))
{
continue;
}

match = i;
break;
match = i;
break;
}
}
else if (metadata.BackgroundColorIndex < quantized.Palette.Length)
{
match = metadata.BackgroundColorIndex;
}
}

if (match >= 0)
{
index = (byte)Numerics.Clamp(match, 0, 255);
return true;
}

index = 0;
return false;
return ClampIndex(match);
Copy link
Preview

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

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

The method returns ClampIndex(match) but match could be -1, which would be clamped to 0. This could incorrectly return index 0 as the background when no match is found and no valid background index exists from metadata.

Suggested change
return ClampIndex(match);
// If no match was found, return byte.MaxValue to indicate "no valid background index".
return match == -1 ? byte.MaxValue : ClampIndex(match);

Copilot uses AI. Check for mistakes.

Copy link
Member Author

@JimBobSquarePants JimBobSquarePants Sep 10, 2025

Choose a reason for hiding this comment

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

0 is the default in GIF

}

/// <summary>
Expand Down
10 changes: 10 additions & 0 deletions tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -398,4 +398,14 @@ public void Issue2953<TPixel>(TestImageProvider<TPixel> provider)
Assert.Throws<InvalidImageContentException>(() => Image.Identify(options, testFile.FullPath));
Assert.Throws<InvalidImageContentException>(() => Image.Load(options, testFile.FullPath));
}

[Theory]
[WithFile(TestImages.Gif.Issues.Issue2980, PixelTypes.Rgba32)]
public void Issue2980<TPixel>(TestImageProvider<TPixel> provider)
where TPixel : unmanaged, IPixel<TPixel>
{
using Image<TPixel> image = provider.GetImage();
image.DebugSaveMultiFrame(provider);
image.CompareToReferenceOutputMultiFrame(provider, ImageComparer.Exact);
}
}
2 changes: 0 additions & 2 deletions tests/ImageSharp.Tests/Quantization/QuantizedImageTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ public void OctreeQuantizerYieldsCorrectTransparentPixel<TPixel>(
where TPixel : unmanaged, IPixel<TPixel>
{
using Image<TPixel> image = provider.GetImage();
Assert.True(image[0, 0].Equals(default));

QuantizerOptions options = new();
if (!dither)
Expand All @@ -79,7 +78,6 @@ public void WuQuantizerYieldsCorrectTransparentPixel<TPixel>(TestImageProvider<T
where TPixel : unmanaged, IPixel<TPixel>
{
using Image<TPixel> image = provider.GetImage();
Assert.True(image[0, 0].Equals(default));

QuantizerOptions options = new();
if (!dither)
Expand Down
1 change: 1 addition & 0 deletions tests/ImageSharp.Tests/TestImages.cs
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,7 @@ public static class Issues
public const string Issue2859_A = "Gif/issues/issue_2859_A.gif";
public const string Issue2859_B = "Gif/issues/issue_2859_B.gif";
public const string Issue2953 = "Gif/issues/issue_2953.gif";
public const string Issue2980 = "Gif/issues/issue_2980.gif";
}

public static readonly string[] Animated =
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Loading