Only set GIF canvas background when required. - #2982
Conversation
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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.
| return ClampIndex(match); | |
| // If no match was found, return byte.MaxValue to indicate "no valid background index". | |
| return match == -1 ? byte.MaxValue : ClampIndex(match); |
There was a problem hiding this comment.
0 is the default in GIF
Prerequisites
Description
Fix #2980