Skip to content

Conversation

@JimBobSquarePants
Copy link
Member

@JimBobSquarePants JimBobSquarePants commented Sep 10, 2025

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Fix #2980

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes issue #2980 by improving how GIF decoder and encoder handle canvas background colors, particularly ensuring that background colors are only set when required and that transparency handling is correctly applied.

  • Optimizes background color handling to only apply when disposal methods require it
  • Removes unnecessary transparency alpha manipulation during decoding
  • Adds proper background index calculation that respects metadata values

Reviewed Changes

Copilot reviewed 74 out of 74 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/Images/Input/Gif/issues/issue_2980.gif Adds test GIF file for issue reproduction
tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2980_* New reference output frames for issue 2980 test
tests/Images/External/ReferenceOutput/GifDecoderTests/* Updated reference outputs reflecting improved transparency handling
tests/ImageSharp.Tests/TestImages.cs Adds constant for new test GIF
tests/ImageSharp.Tests/Quantization/QuantizedImageTests.cs Removes transparency assertions that are no longer valid
tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs Adds test case for issue 2980
src/ImageSharp/Formats/Gif/GifEncoderCore.cs Improves background index calculation and transparency handling
src/ImageSharp/Formats/Gif/GifDecoderCore.cs Optimizes canvas background setting logic

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.


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

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

@JimBobSquarePants JimBobSquarePants merged commit 3948fa8 into main Sep 10, 2025
9 checks passed
@JimBobSquarePants JimBobSquarePants deleted the js/fix-2980 branch September 10, 2025 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants