From f3ff11ea6b9f36c1cbdf9965c0366ef97a120906 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 7 Dec 2023 10:52:56 +1000 Subject: [PATCH 1/7] Handle dedup of local palette of 256 length --- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 40 +++++++++++++++---- .../Formats/Gif/MetadataExtensions.cs | 10 ++++- .../Formats/Webp/BitReader/Vp8LBitReader.cs | 4 +- tests/ImageSharp.Tests/TestImages.cs | 4 +- tests/Images/Input/Gif/18-bit_RGB_Cube.gif | 3 ++ 5 files changed, 48 insertions(+), 13 deletions(-) create mode 100644 tests/Images/Input/Gif/18-bit_RGB_Cube.gif diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index f0e1aafd7d..9988848d11 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -380,18 +380,42 @@ private IndexedImageFrame QuantizeAdditionalFrameAndUpdateMetadata palette = metadata.LocalColorTable.Value; - if (hasDuplicates && !metadata.HasTransparency) { - // A difference was captured but the metadata does not have transparency. + // Duplicates were captured but the metadata does not have transparency. metadata.HasTransparency = true; - transparencyIndex = palette.Length; - metadata.TransparencyIndex = ClampIndex(transparencyIndex); - } - PaletteQuantizer quantizer = new(palette, new() { Dither = null }, transparencyIndex); - using IQuantizer frameQuantizer = quantizer.CreatePixelSpecificQuantizer(this.configuration, quantizer.Options); - quantized = frameQuantizer.BuildPaletteAndQuantizeFrame(encodingFrame, bounds); + if (palette.Length < 256) + { + // We can use the existing palette and set the transparent index as the length. + // decoders will ignore this value. + transparencyIndex = palette.Length; + metadata.TransparencyIndex = ClampIndex(transparencyIndex); + + PaletteQuantizer quantizer = new(palette, new() { Dither = null }, transparencyIndex); + using IQuantizer frameQuantizer = quantizer.CreatePixelSpecificQuantizer(this.configuration, quantizer.Options); + quantized = frameQuantizer.BuildPaletteAndQuantizeFrame(encodingFrame, bounds); + } + else + { + // We must quantize the frame to generate a local color table. + IQuantizer quantizer = this.hasQuantizer ? this.quantizer! : KnownQuantizers.Octree; + using IQuantizer frameQuantizer = quantizer.CreatePixelSpecificQuantizer(this.configuration, quantizer.Options); + quantized = frameQuantizer.BuildPaletteAndQuantizeFrame(encodingFrame, bounds); + + // The transparency index derived by the quantizer will differ from the index + // within the metadata. We need to update the metadata to reflect this. + int derivedTransparencyIndex = GetTransparentIndex(quantized, null); + metadata.TransparencyIndex = ClampIndex(derivedTransparencyIndex); + } + } + else + { + // Just use the local palette. + PaletteQuantizer quantizer = new(palette, new() { Dither = null }, transparencyIndex); + using IQuantizer frameQuantizer = quantizer.CreatePixelSpecificQuantizer(this.configuration, quantizer.Options); + quantized = frameQuantizer.BuildPaletteAndQuantizeFrame(encodingFrame, bounds); + } } else { diff --git a/src/ImageSharp/Formats/Gif/MetadataExtensions.cs b/src/ImageSharp/Formats/Gif/MetadataExtensions.cs index 16f788e3db..42602f2c78 100644 --- a/src/ImageSharp/Formats/Gif/MetadataExtensions.cs +++ b/src/ImageSharp/Formats/Gif/MetadataExtensions.cs @@ -77,14 +77,20 @@ internal static AnimatedImageMetadata ToAnimatedImageMetadata(this GifMetadata s } internal static AnimatedImageFrameMetadata ToAnimatedImageFrameMetadata(this GifFrameMetadata source) - => new() + { + // For most scenarios we would consider the blend method to be 'Over' however if a frame has a disposal method of 'RestoreToBackground' or + // has a local palette with 256 colors and is not transparent we should use 'Source'. + bool blendSource = source.DisposalMethod == GifDisposalMethod.RestoreToBackground || (source.LocalColorTable?.Length == 256 && !source.HasTransparency); + + return new() { ColorTable = source.LocalColorTable, ColorTableMode = source.ColorTableMode == GifColorTableMode.Global ? FrameColorTableMode.Global : FrameColorTableMode.Local, Duration = TimeSpan.FromMilliseconds(source.FrameDelay * 10), DisposalMode = GetMode(source.DisposalMethod), - BlendMode = source.DisposalMethod == GifDisposalMethod.RestoreToBackground ? FrameBlendMode.Source : FrameBlendMode.Over, + BlendMode = blendSource ? FrameBlendMode.Source : FrameBlendMode.Over, }; + } private static FrameDisposalMode GetMode(GifDisposalMethod method) => method switch { diff --git a/src/ImageSharp/Formats/Webp/BitReader/Vp8LBitReader.cs b/src/ImageSharp/Formats/Webp/BitReader/Vp8LBitReader.cs index 659576cf11..6d3cab1514 100644 --- a/src/ImageSharp/Formats/Webp/BitReader/Vp8LBitReader.cs +++ b/src/ImageSharp/Formats/Webp/BitReader/Vp8LBitReader.cs @@ -71,7 +71,7 @@ public Vp8LBitReader(IMemoryOwner data) this.Eos = false; ulong currentValue = 0; - System.Span dataSpan = this.Data.Memory.Span; + Span dataSpan = this.Data.Memory.Span; for (int i = 0; i < 8; i++) { currentValue |= (ulong)dataSpan[i] << (8 * i); @@ -103,7 +103,7 @@ public Vp8LBitReader(Stream inputStream, uint imageDataSize, MemoryAllocator mem } ulong currentValue = 0; - System.Span dataSpan = this.Data.Memory.Span; + Span dataSpan = this.Data.Memory.Span; for (int i = 0; i < length; i++) { currentValue |= (ulong)dataSpan[i] << (8 * i); diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 7e862f7d4f..b628529028 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -487,6 +487,7 @@ public static class Gif public const string GlobalQuantizationTest = "Gif/GlobalQuantizationTest.gif"; public const string MixedDisposal = "Gif/mixed-disposal.gif"; public const string M4nb = "Gif/m4nb.gif"; + public const string Bit18RGBCube = "Gif/18-bit_RGB_Cube.gif"; // Test images from https://github.com/robert-ancell/pygif/tree/master/test-suite public const string ZeroSize = "Gif/image-zero-size.gif"; @@ -533,7 +534,8 @@ public static class Issues Issues.Issue2450_A, Issues.Issue2450_B, Issues.BadDescriptorWidth, - Issues.Issue1530 + Issues.Issue1530, + Bit18RGBCube }; } diff --git a/tests/Images/Input/Gif/18-bit_RGB_Cube.gif b/tests/Images/Input/Gif/18-bit_RGB_Cube.gif new file mode 100644 index 0000000000..5446661b41 --- /dev/null +++ b/tests/Images/Input/Gif/18-bit_RGB_Cube.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:5148c8c192385966ec6ad5b3d35195a878355d71146a958e5ef02b7642a4e883 +size 4311986 From 7ab7b226d93b3e06cd34e096f82101e8f91a15ad Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Fri, 8 Dec 2023 14:06:31 +0000 Subject: [PATCH 2/7] handle when foreground overhangs bottom of background --- .../DrawImageProcessor{TPixelBg,TPixelFg}.cs | 14 +++++++++++++ .../Drawing/DrawImageTests.cs | 21 +++++++++++++++++++ .../Drawing/DrawImageTests/Issue2603.png | 3 +++ 3 files changed, 38 insertions(+) create mode 100644 tests/Images/External/ReferenceOutput/Drawing/DrawImageTests/Issue2603.png diff --git a/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs b/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs index 378ea20fa8..c51df8af80 100644 --- a/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs +++ b/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs @@ -96,6 +96,20 @@ protected override void OnFrameApply(ImageFrame source) top = 0; } + if (left + foregroundRectangle.Width > source.Width) + { + // will overhange, lets trim it down + int diff = (left + foregroundRectangle.Width) - source.Width; + foregroundRectangle.Width -= diff; + } + + if (top + foregroundRectangle.Height > source.Height) + { + // will overhange, lets trim it down + int diff = (top + foregroundRectangle.Height) - source.Height; + foregroundRectangle.Height -= diff; + } + int width = foregroundRectangle.Width; int height = foregroundRectangle.Height; if (width <= 0 || height <= 0) diff --git a/tests/ImageSharp.Tests/Drawing/DrawImageTests.cs b/tests/ImageSharp.Tests/Drawing/DrawImageTests.cs index 8b0db773ab..6cd6e15e25 100644 --- a/tests/ImageSharp.Tests/Drawing/DrawImageTests.cs +++ b/tests/ImageSharp.Tests/Drawing/DrawImageTests.cs @@ -251,4 +251,25 @@ public void Issue2447_C(TestImageProvider provider) appendPixelTypeToFileName: false, appendSourceFileOrDescription: false); } + + [Theory] + [WithFile(TestImages.Png.Issue2447, PixelTypes.Rgba32)] + public void Issue2603(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image foreground = provider.GetImage(); + using Image background = new(100, 100, new Rgba32(0, 255, 255)); + + background.Mutate(c => c.DrawImage(foreground, new Point(80, 80), new Rectangle(32, 32, 32, 32), 1F)); + + background.DebugSave( + provider, + appendPixelTypeToFileName: false, + appendSourceFileOrDescription: false); + + background.CompareToReferenceOutput( + provider, + appendPixelTypeToFileName: false, + appendSourceFileOrDescription: false); + } } diff --git a/tests/Images/External/ReferenceOutput/Drawing/DrawImageTests/Issue2603.png b/tests/Images/External/ReferenceOutput/Drawing/DrawImageTests/Issue2603.png new file mode 100644 index 0000000000..1940dbba04 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/Drawing/DrawImageTests/Issue2603.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:307ef8cea7999e615420740bb0a403a64b24f1fff5356c2ac45c1952f84c5e4c +size 508 From dd705eb0587f147f748aa0dec5032c2738f23327 Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Sat, 9 Dec 2023 10:32:00 +0000 Subject: [PATCH 3/7] prevent potential overflow --- .../Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs b/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs index c51df8af80..e2aca1d90a 100644 --- a/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs +++ b/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs @@ -96,14 +96,14 @@ protected override void OnFrameApply(ImageFrame source) top = 0; } - if (left + foregroundRectangle.Width > source.Width) + if (left > source.Width - foregroundRectangle.Width) { // will overhange, lets trim it down int diff = (left + foregroundRectangle.Width) - source.Width; foregroundRectangle.Width -= diff; } - if (top + foregroundRectangle.Height > source.Height) + if (top > source.Height - foregroundRectangle.Height) { // will overhange, lets trim it down int diff = (top + foregroundRectangle.Height) - source.Height; From 6c7b59fc06cf8da9af070cad8aebae059cb68e47 Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Sat, 9 Dec 2023 10:43:21 +0000 Subject: [PATCH 4/7] reduce to a single par of clamping operations --- .../DrawImageProcessor{TPixelBg,TPixelFg}.cs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs b/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs index e2aca1d90a..031bcbdf78 100644 --- a/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs +++ b/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs @@ -96,19 +96,9 @@ protected override void OnFrameApply(ImageFrame source) top = 0; } - if (left > source.Width - foregroundRectangle.Width) - { - // will overhange, lets trim it down - int diff = (left + foregroundRectangle.Width) - source.Width; - foregroundRectangle.Width -= diff; - } - - if (top > source.Height - foregroundRectangle.Height) - { - // will overhange, lets trim it down - int diff = (top + foregroundRectangle.Height) - source.Height; - foregroundRectangle.Height -= diff; - } + // clamp the height/width to the availible space left to prevent overflowing + foregroundRectangle.Width = Math.Min(source.Width - left, foregroundRectangle.Width); + foregroundRectangle.Height = Math.Min(source.Height - top, foregroundRectangle.Height); int width = foregroundRectangle.Width; int height = foregroundRectangle.Height; From 9906ee5ad7944946065004ae4d25b7baf8414ff1 Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Fri, 8 Dec 2023 13:40:30 +0000 Subject: [PATCH 5/7] correctly calculate Rect when negative target set --- .../DrawImageProcessor{TPixelBg,TPixelFg}.cs | 2 ++ .../Drawing/DrawImageTests.cs | 21 +++++++++++++++++++ .../DrawImageTests/Issue2608_NegOffset.png | 3 +++ 3 files changed, 26 insertions(+) create mode 100644 tests/Images/External/ReferenceOutput/Drawing/DrawImageTests/Issue2608_NegOffset.png diff --git a/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs b/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs index 378ea20fa8..2ac8365042 100644 --- a/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs +++ b/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs @@ -87,12 +87,14 @@ protected override void OnFrameApply(ImageFrame source) if (this.BackgroundLocation.X < 0) { foregroundRectangle.Width += this.BackgroundLocation.X; + foregroundRectangle.X -= this.BackgroundLocation.X; left = 0; } if (this.BackgroundLocation.Y < 0) { foregroundRectangle.Height += this.BackgroundLocation.Y; + foregroundRectangle.Y -= this.BackgroundLocation.Y; top = 0; } diff --git a/tests/ImageSharp.Tests/Drawing/DrawImageTests.cs b/tests/ImageSharp.Tests/Drawing/DrawImageTests.cs index 8b0db773ab..4d1c592ef2 100644 --- a/tests/ImageSharp.Tests/Drawing/DrawImageTests.cs +++ b/tests/ImageSharp.Tests/Drawing/DrawImageTests.cs @@ -251,4 +251,25 @@ public void Issue2447_C(TestImageProvider provider) appendPixelTypeToFileName: false, appendSourceFileOrDescription: false); } + + [Theory] + [WithFile(TestImages.Png.Issue2447, PixelTypes.Rgba32)] + public void Issue2608_NegOffset(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image foreground = provider.GetImage(); + using Image background = new(100, 100, new Rgba32(0, 255, 255)); + + background.Mutate(c => c.DrawImage(foreground, new Point(-10, -10), new Rectangle(32, 32, 32, 32), 1F)); + + background.DebugSave( + provider, + appendPixelTypeToFileName: false, + appendSourceFileOrDescription: false); + + background.CompareToReferenceOutput( + provider, + appendPixelTypeToFileName: false, + appendSourceFileOrDescription: false); + } } diff --git a/tests/Images/External/ReferenceOutput/Drawing/DrawImageTests/Issue2608_NegOffset.png b/tests/Images/External/ReferenceOutput/Drawing/DrawImageTests/Issue2608_NegOffset.png new file mode 100644 index 0000000000..747bbca38a --- /dev/null +++ b/tests/Images/External/ReferenceOutput/Drawing/DrawImageTests/Issue2608_NegOffset.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:edfd0e17aa304f5b16ad42a761c044c3d617aaee9b9e1e74674567bbcd74d532 +size 590 From 436f74d23cb257d1983cf0d55842013be192e83f Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 11 Dec 2023 20:35:25 +1000 Subject: [PATCH 6/7] Handle global ani with 256 palette and no trans --- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 19 ++++++++++++------- .../Formats/Gif/MetadataExtensions.cs | 3 +++ tests/ImageSharp.Tests/TestImages.cs | 4 +++- .../Images/Input/Gif/global-256-no-trans.gif | 3 +++ 4 files changed, 21 insertions(+), 8 deletions(-) create mode 100644 tests/Images/Input/Gif/global-256-no-trans.gif diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index 9988848d11..1215768e4b 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -103,7 +103,14 @@ public void Encode(Image image, Stream stream, CancellationToken { // We avoid dithering by default to preserve the original colors. int transparencyIndex = GetTransparentIndex(quantized, frameMetadata); - this.quantizer = new PaletteQuantizer(gifMetadata.GlobalColorTable.Value, new() { Dither = null }, transparencyIndex); + if (transparencyIndex >= 0 || gifMetadata.GlobalColorTable.Value.Length < 256) + { + this.quantizer = new PaletteQuantizer(gifMetadata.GlobalColorTable.Value, new() { Dither = null }, transparencyIndex); + } + else + { + this.quantizer = KnownQuantizers.Octree; + } } else { @@ -198,19 +205,17 @@ private static GifMetadata GetGifMetadata(Image image) private static GifFrameMetadata GetGifFrameMetadata(ImageFrame frame, int transparencyIndex) where TPixel : unmanaged, IPixel { + GifFrameMetadata? metadata = null; if (frame.Metadata.TryGetGifMetadata(out GifFrameMetadata? gif)) { - return (GifFrameMetadata)gif.DeepClone(); + metadata = (GifFrameMetadata)gif.DeepClone(); } - - GifFrameMetadata? metadata = null; - if (frame.Metadata.TryGetPngMetadata(out PngFrameMetadata? png)) + else if (frame.Metadata.TryGetPngMetadata(out PngFrameMetadata? png)) { AnimatedImageFrameMetadata ani = png.ToAnimatedImageFrameMetadata(); metadata = GifFrameMetadata.FromAnimatedMetadata(ani); } - - if (frame.Metadata.TryGetWebpFrameMetadata(out WebpFrameMetadata? webp)) + else if (frame.Metadata.TryGetWebpFrameMetadata(out WebpFrameMetadata? webp)) { AnimatedImageFrameMetadata ani = webp.ToAnimatedImageFrameMetadata(); metadata = GifFrameMetadata.FromAnimatedMetadata(ani); diff --git a/src/ImageSharp/Formats/Gif/MetadataExtensions.cs b/src/ImageSharp/Formats/Gif/MetadataExtensions.cs index 42602f2c78..ad06462e77 100644 --- a/src/ImageSharp/Formats/Gif/MetadataExtensions.cs +++ b/src/ImageSharp/Formats/Gif/MetadataExtensions.cs @@ -82,6 +82,9 @@ internal static AnimatedImageFrameMetadata ToAnimatedImageFrameMetadata(this Gif // has a local palette with 256 colors and is not transparent we should use 'Source'. bool blendSource = source.DisposalMethod == GifDisposalMethod.RestoreToBackground || (source.LocalColorTable?.Length == 256 && !source.HasTransparency); + // If the color table is global and frame has no transparency. Consider it 'Source' also. + blendSource |= source.ColorTableMode == GifColorTableMode.Global && !source.HasTransparency; + return new() { ColorTable = source.LocalColorTable, diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index b628529028..8aa95d3496 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -488,6 +488,7 @@ public static class Gif public const string MixedDisposal = "Gif/mixed-disposal.gif"; public const string M4nb = "Gif/m4nb.gif"; public const string Bit18RGBCube = "Gif/18-bit_RGB_Cube.gif"; + public const string Global256NoTrans = "Gif/global-256-no-trans.gif"; // Test images from https://github.com/robert-ancell/pygif/tree/master/test-suite public const string ZeroSize = "Gif/image-zero-size.gif"; @@ -535,7 +536,8 @@ public static class Issues Issues.Issue2450_B, Issues.BadDescriptorWidth, Issues.Issue1530, - Bit18RGBCube + Bit18RGBCube, + Global256NoTrans }; } diff --git a/tests/Images/Input/Gif/global-256-no-trans.gif b/tests/Images/Input/Gif/global-256-no-trans.gif new file mode 100644 index 0000000000..1afa0d21d7 --- /dev/null +++ b/tests/Images/Input/Gif/global-256-no-trans.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:ce8ed23b4e21328886f5aa7579079123ff6401efdf65e162e565b056ffddab56 +size 669286 From 30dee5a64ee92fe08534e81c913089a18cacb751 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 11 Dec 2023 21:04:39 +1000 Subject: [PATCH 7/7] Bump diff for windows --- tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs index c81b7eb6cd..e70854b082 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs @@ -499,7 +499,7 @@ public void Encode_AnimatedFormatTransform_FromGif(TestImageProvider