Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve Gif color palettes and deduplicate frame pixels. #2455

Merged
merged 33 commits into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
12da625
Preserve color palettes and deduplicate frame pixels.
JimBobSquarePants May 15, 2023
73067b9
Fix NRE
JimBobSquarePants May 15, 2023
43aaad1
Make color tables mutable
JimBobSquarePants May 17, 2023
d196b22
Merge branch 'main' into js/gif-fixes
JimBobSquarePants May 18, 2023
fc7219d
Add quantizer property tests
JimBobSquarePants May 19, 2023
b126b77
Use background index as fallback during dedup.
JimBobSquarePants May 19, 2023
14f9127
Merge branch 'main' into js/gif-fixes
JimBobSquarePants May 22, 2023
c8fe7f2
Merge branch 'main' into js/gif-fixes
JimBobSquarePants Jun 8, 2023
53510f0
Add explicit tests for #2450
JimBobSquarePants Jun 10, 2023
18167b2
Merge branch 'main' into js/gif-fixes
JimBobSquarePants Jun 10, 2023
2c7123c
Merge branch 'main' into js/gif-fixes
JimBobSquarePants Jun 12, 2023
0d4a23a
Merge branch 'main' into js/gif-fixes
JimBobSquarePants Jun 12, 2023
eaf23dd
Merge branch 'main' into js/gif-fixes
JimBobSquarePants Jun 13, 2023
925bff0
Merge branch 'main' into js/gif-fixes
JimBobSquarePants Jun 25, 2023
e307da5
Update SimdUtils.HwIntrinsics.cs
JimBobSquarePants Jun 29, 2023
4f6a53c
Compare to zero
JimBobSquarePants Jun 29, 2023
a29b5fc
Trim buffer to minimum region.
JimBobSquarePants Jul 5, 2023
f33f67d
Merge branch 'main' into js/gif-fixes
JimBobSquarePants Jul 5, 2023
98ed0f1
Refactor and fix gif encoder
JimBobSquarePants Jul 9, 2023
c3fc062
Update src/ImageSharp/Formats/Gif/GifEncoderCore.cs
JimBobSquarePants Jul 9, 2023
ef08c82
Update src/ImageSharp/Formats/Gif/GifEncoderCore.cs
JimBobSquarePants Jul 9, 2023
4b15595
Merge branch 'main' into js/gif-fixes
JimBobSquarePants Jul 9, 2023
5416edb
Vectorize TrimTransparentPixels in GifEncoderCore
gfoidl Jul 27, 2023
c8f1f2c
Simplified check if there are any non-equal bytes
gfoidl Jul 27, 2023
949e6ad
Merge pull request #2500 from gfoidl/git-transparency-simd
JimBobSquarePants Aug 9, 2023
4ef363d
Only compare a subset of frames.
JimBobSquarePants Aug 10, 2023
f9e10ae
Merge branch 'main' into js/gif-fixes
JimBobSquarePants Aug 10, 2023
c0ca0cc
Merge branch 'main' into js/gif-fixes
JimBobSquarePants Aug 17, 2023
1a6b465
Cleanup based on feedback
JimBobSquarePants Aug 21, 2023
b48dbc5
if transparencyIndex is outside the palette, ignore it
antonfirsov Sep 1, 2023
c17eacf
Merge branch 'main' into js/gif-fixes
JimBobSquarePants Sep 2, 2023
b56633e
Review fixes
JimBobSquarePants Sep 11, 2023
9ddebdd
Merge branch 'main' into js/gif-fixes
JimBobSquarePants Sep 15, 2023
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
4 changes: 2 additions & 2 deletions src/ImageSharp/Color/Color.Conversions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public Color(Vector4 vector)
/// </summary>
/// <param name="color">The <see cref="Color"/>.</param>
/// <returns>The <see cref="Vector4"/>.</returns>
public static explicit operator Vector4(Color color) => color.ToVector4();
public static explicit operator Vector4(Color color) => color.ToScaledVector4();

/// <summary>
/// Converts an <see cref="Vector4"/> to <see cref="Color"/>.
Expand Down Expand Up @@ -228,7 +228,7 @@ internal Bgr24 ToBgr24()
}

[MethodImpl(InliningOptions.ShortMethod)]
internal Vector4 ToVector4()
internal Vector4 ToScaledVector4()
{
if (this.boxedHighPrecisionPixel is null)
{
Expand Down
27 changes: 27 additions & 0 deletions src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,33 @@ public static Vector256<float> MultiplyAddNegated(
return Avx.Subtract(c, Avx.Multiply(a, b));
}

/// <summary>
/// Blend packed 8-bit integers from <paramref name="left"/> and <paramref name="right"/> using <paramref name="mask"/>.
/// The high bit of each corresponding <paramref name="mask"/> byte determines the selection.
/// If the high bit is set the element of <paramref name="left"/> is selected.
/// The element of <paramref name="right"/> is selected otherwise.
/// </summary>
/// <param name="left">The left vector.</param>
/// <param name="right">The right vector.</param>
/// <param name="mask">The mask vector.</param>
/// <returns>The <see cref="Vector256{T}"/>.</returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static Vector128<byte> BlendVariable(Vector128<byte> left, Vector128<byte> right, Vector128<byte> mask)
{
if (Sse41.IsSupported)
{
return Sse41.BlendVariable(left, right, mask);
}
else if (Sse2.IsSupported)
{
return Sse2.Or(Sse2.And(right, mask), Sse2.AndNot(mask, left));
}

// Use a signed shift right to create a mask with the sign bit.
Vector128<short> signedMask = AdvSimd.ShiftRightArithmetic(mask.AsInt16(), 7);
return AdvSimd.BitwiseSelect(signedMask, right.AsInt16(), left.AsInt16()).AsByte();
Comment on lines +655 to +656
Copy link
Member

Choose a reason for hiding this comment

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

Again, nothing to do with the PR but it's a bit messy now that it's unclear for a caller which instruction set is supported in various methods on SimdUtils and Numerics. Originally public methods of SimdUtils were supposed to work regardless of the instructions and SimdUtils.HwIntrinsics used to contain private implementation details for the System.Runtime.Intrinsics branch. Would be nice to figure out new general rules later.

Copy link
Member Author

Choose a reason for hiding this comment

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

This has become a bit of a dumping ground over the years. My plan for v4 is a mass simplification of intrinsics utilizing things like Vector128<T> to allow us to implement better cross chipset approaches. We can do a tidy up at the same time.

}

/// <summary>
/// <see cref="ByteToNormalizedFloat"/> as many elements as possible, slicing them down (keeping the remainder).
/// </summary>
Expand Down
6 changes: 6 additions & 0 deletions src/ImageSharp/Formats/Bmp/BmpEncoder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Six Labors Split License.

using SixLabors.ImageSharp.Advanced;
using SixLabors.ImageSharp.Processing;

namespace SixLabors.ImageSharp.Formats.Bmp;

Expand All @@ -10,6 +11,11 @@ namespace SixLabors.ImageSharp.Formats.Bmp;
/// </summary>
public sealed class BmpEncoder : QuantizingImageEncoder
{
/// <summary>
/// Initializes a new instance of the <see cref="BmpEncoder"/> class.
/// </summary>
public BmpEncoder() => this.Quantizer = KnownQuantizers.Octree;

/// <summary>
/// Gets the number of bits per pixel.
/// </summary>
Expand Down
3 changes: 2 additions & 1 deletion src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using SixLabors.ImageSharp.Memory;
using SixLabors.ImageSharp.Metadata;
using SixLabors.ImageSharp.PixelFormats;
using SixLabors.ImageSharp.Processing;
using SixLabors.ImageSharp.Processing.Processors.Quantization;

namespace SixLabors.ImageSharp.Formats.Bmp;
Expand Down Expand Up @@ -100,7 +101,7 @@ public BmpEncoderCore(BmpEncoder encoder, MemoryAllocator memoryAllocator)
{
this.memoryAllocator = memoryAllocator;
this.bitsPerPixel = encoder.BitsPerPixel;
this.quantizer = encoder.Quantizer;
this.quantizer = encoder.Quantizer ?? KnownQuantizers.Octree;
this.pixelSamplingStrategy = encoder.PixelSamplingStrategy;
this.infoHeaderType = encoder.SupportTransparency ? BmpInfoHeaderType.WinVersion4 : BmpInfoHeaderType.WinVersion3;
}
Expand Down
70 changes: 53 additions & 17 deletions src/ImageSharp/Formats/Gif/GifDecoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,16 @@ internal sealed class GifDecoderCore : IImageDecoderInternals
/// </summary>
private IMemoryOwner<byte>? globalColorTable;

/// <summary>
/// The current local color table.
/// </summary>
private IMemoryOwner<byte>? currentLocalColorTable;

/// <summary>
/// Gets the size in bytes of the current local color table.
/// </summary>
private int currentLocalColorTableSize;

/// <summary>
/// The area to restore.
/// </summary>
Expand Down Expand Up @@ -159,6 +169,7 @@ public Image<TPixel> Decode<TPixel>(BufferedReadStream stream, CancellationToken
finally
{
this.globalColorTable?.Dispose();
this.currentLocalColorTable?.Dispose();
}

if (image is null)
Expand Down Expand Up @@ -229,6 +240,7 @@ public ImageInfo Identify(BufferedReadStream stream, CancellationToken cancellat
finally
{
this.globalColorTable?.Dispose();
this.currentLocalColorTable?.Dispose();
}

if (this.logicalScreenDescriptor.Width == 0 && this.logicalScreenDescriptor.Height == 0)
Expand Down Expand Up @@ -332,7 +344,7 @@ private void ReadApplicationExtension(BufferedReadStream stream)
if (subBlockSize == GifConstants.NetscapeLoopingSubBlockSize)
{
stream.Read(this.buffer.Span, 0, GifConstants.NetscapeLoopingSubBlockSize);
this.gifMetadata!.RepeatCount = GifNetscapeLoopingApplicationExtension.Parse(this.buffer.Span.Slice(1)).RepeatCount;
this.gifMetadata!.RepeatCount = GifNetscapeLoopingApplicationExtension.Parse(this.buffer.Span[1..]).RepeatCount;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
this.gifMetadata!.RepeatCount = GifNetscapeLoopingApplicationExtension.Parse(this.buffer.Span[1..]).RepeatCount;
this.gifMetadata!.RepeatCount = GifNetscapeLoopingApplicationExtension.Parse(this.buffer.Span.Slice(1)).RepeatCount;

Perf-wise Slice is better than the range-operator. Unfortunately Roslyn compiles "bad" IL here, which the JIT doesn't optimize away.

So either

  • keep Slice
  • wait for Roslyn to be fixed (which takes some years already for that issue)

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm just taking the hit here as we have elsewhere. It's annoying but worth the readability improvements.

Copy link
Member

@antonfirsov antonfirsov Aug 19, 2023

Choose a reason for hiding this comment

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

I don't think this is a critical path to care about if Slice is faster. Simpler looking code is better.

stream.Skip(1); // Skip the terminator.
return;
}
Expand Down Expand Up @@ -415,25 +427,27 @@ private void ReadFrame<TPixel>(BufferedReadStream stream, ref Image<TPixel>? ima
{
this.ReadImageDescriptor(stream);

IMemoryOwner<byte>? localColorTable = null;
Buffer2D<byte>? indices = null;
try
{
// Determine the color table for this frame. If there is a local one, use it otherwise use the global color table.
if (this.imageDescriptor.LocalColorTableFlag)
bool hasLocalColorTable = this.imageDescriptor.LocalColorTableFlag;

if (hasLocalColorTable)
{
int length = this.imageDescriptor.LocalColorTableSize * 3;
localColorTable = this.configuration.MemoryAllocator.Allocate<byte>(length, AllocationOptions.Clean);
stream.Read(localColorTable.GetSpan());
// Read and store the local color table. We allocate the maximum possible size and slice to match.
int length = this.currentLocalColorTableSize = this.imageDescriptor.LocalColorTableSize * 3;
this.currentLocalColorTable ??= this.configuration.MemoryAllocator.Allocate<byte>(768, AllocationOptions.Clean);
stream.Read(this.currentLocalColorTable.GetSpan()[..length]);
}

indices = this.configuration.MemoryAllocator.Allocate2D<byte>(this.imageDescriptor.Width, this.imageDescriptor.Height, AllocationOptions.Clean);
this.ReadFrameIndices(stream, indices);

Span<byte> rawColorTable = default;
if (localColorTable != null)
if (hasLocalColorTable)
{
rawColorTable = localColorTable.GetSpan();
rawColorTable = this.currentLocalColorTable!.GetSpan()[..this.currentLocalColorTableSize];
}
else if (this.globalColorTable != null)
{
Expand All @@ -448,7 +462,6 @@ private void ReadFrame<TPixel>(BufferedReadStream stream, ref Image<TPixel>? ima
}
finally
{
localColorTable?.Dispose();
indices?.Dispose();
}
}
Expand Down Expand Up @@ -509,7 +522,10 @@ private void ReadFrameColors<TPixel>(ref Image<TPixel>? image, ref ImageFrame<TP
prevFrame = previousFrame;
}

currentFrame = image!.Frames.CreateFrame();
// We create a clone of the frame and add it.
// We will overpaint the difference of pixels on the current frame to create a complete image.
// This ensures that we have enough pixel data to process without distortion. #2450
currentFrame = image!.Frames.AddFrame(previousFrame);

this.SetFrameMetadata(currentFrame.Metadata);

Expand Down Expand Up @@ -631,7 +647,10 @@ private void ReadFrameMetadata(BufferedReadStream stream, List<ImageFrameMetadat
// Skip the color table for this frame if local.
if (this.imageDescriptor.LocalColorTableFlag)
{
stream.Skip(this.imageDescriptor.LocalColorTableSize * 3);
// Read and store the local color table. We allocate the maximum possible size and slice to match.
int length = this.currentLocalColorTableSize = this.imageDescriptor.LocalColorTableSize * 3;
this.currentLocalColorTable ??= this.configuration.MemoryAllocator.Allocate<byte>(768, AllocationOptions.Clean);
stream.Read(this.currentLocalColorTable.GetSpan()[..length]);
}

// Skip the frame indices. Pixels length + mincode size.
Expand Down Expand Up @@ -682,21 +701,30 @@ private void SetFrameMetadata(ImageFrameMetadata metadata)
{
GifFrameMetadata gifMeta = metadata.GetGifMetadata();
gifMeta.ColorTableMode = GifColorTableMode.Global;
gifMeta.ColorTableLength = this.logicalScreenDescriptor.GlobalColorTableSize;
}

if (this.imageDescriptor.LocalColorTableFlag
&& this.imageDescriptor.LocalColorTableSize > 0)
{
GifFrameMetadata gifMeta = metadata.GetGifMetadata();
gifMeta.ColorTableMode = GifColorTableMode.Local;
gifMeta.ColorTableLength = this.imageDescriptor.LocalColorTableSize;

Color[] colorTable = new Color[this.imageDescriptor.LocalColorTableSize];
ReadOnlySpan<Rgb24> rgbTable = MemoryMarshal.Cast<byte, Rgb24>(this.currentLocalColorTable!.GetSpan()[..this.currentLocalColorTableSize]);
for (int i = 0; i < colorTable.Length; i++)
{
colorTable[i] = new Color(rgbTable[i]);
}

gifMeta.LocalColorTable = colorTable;
}

// Graphics control extensions is optional.
if (this.graphicsControlExtension != default)
{
GifFrameMetadata gifMeta = metadata.GetGifMetadata();
gifMeta.HasTransparency = this.graphicsControlExtension.TransparencyFlag;
gifMeta.TransparencyIndex = this.graphicsControlExtension.TransparencyIndex;
gifMeta.FrameDelay = this.graphicsControlExtension.DelayTime;
gifMeta.DisposalMethod = this.graphicsControlExtension.DisposalMethod;
}
Expand Down Expand Up @@ -751,14 +779,22 @@ private void ReadLogicalScreenDescriptorAndGlobalColorTable(BufferedReadStream s
if (this.logicalScreenDescriptor.GlobalColorTableFlag)
{
int globalColorTableLength = this.logicalScreenDescriptor.GlobalColorTableSize * 3;
this.gifMetadata.GlobalColorTableLength = globalColorTableLength;

if (globalColorTableLength > 0)
{
this.globalColorTable = this.memoryAllocator.Allocate<byte>(globalColorTableLength, AllocationOptions.Clean);

// Read the global color table data from the stream
stream.Read(this.globalColorTable.GetSpan());
// Read the global color table data from the stream and preserve it in the gif metadata
Span<byte> globalColorTableSpan = this.globalColorTable.GetSpan();
stream.Read(globalColorTableSpan);

Color[] colorTable = new Color[this.logicalScreenDescriptor.GlobalColorTableSize];
ReadOnlySpan<Rgb24> rgbTable = MemoryMarshal.Cast<byte, Rgb24>(globalColorTableSpan);
for (int i = 0; i < colorTable.Length; i++)
{
colorTable[i] = new Color(rgbTable[i]);
}

this.gifMetadata.GlobalColorTable = colorTable;
}
}
}
Expand Down
Loading
Loading