From 29ae5f8409932728c4d5bff1369488e8b564cb17 Mon Sep 17 00:00:00 2001 From: Erik White <26148654+Erik-White@users.noreply.github.com> Date: Wed, 27 May 2026 10:28:30 +0200 Subject: [PATCH 01/11] Guard CgBI bit depth and color type --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 16 ++++++++++++++ .../Formats/Png/PngDecoderTests.cs | 22 +++++++++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 4 ++++ .../Input/Png/cgbi/colors-cgbi-bitdepth16.png | 3 +++ .../Input/Png/cgbi/colors-cgbi-palette.png | 3 +++ 5 files changed, 48 insertions(+) create mode 100644 tests/Images/Input/Png/cgbi/colors-cgbi-bitdepth16.png create mode 100644 tests/Images/Input/Png/cgbi/colors-cgbi-palette.png diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 84245254a2..0d93429bb8 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -1431,6 +1431,22 @@ private void ReadHeaderChunk(PngMetadata pngMetadata, ReadOnlySpan data) this.pngColorType = this.header.ColorType; this.Dimensions = new Size(this.header.Width, this.header.Height); + + // Apple's pngcrush emits the CgBI chunk before IHDR, so the header + // compatibility check is deferred until both chunks have been seen. + if (this.isCgbi) + { + ThrowIfInvalidCgbiContent(this.header); + } + } + + private static void ThrowIfInvalidCgbiContent(in PngHeader header) + { + if (header.BitDepth != 8 || (header.ColorType is not PngColorType.Rgb and not PngColorType.RgbWithAlpha)) + { + PngThrowHelper.ThrowInvalidImageContentException( + $"CgBI is only supported for 8-bit truecolor images. Was bit depth '{header.BitDepth}', color type '{header.ColorType}'."); + } } /// diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 2e452b896d..2fbbe695e9 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -767,6 +767,28 @@ public void Identify_AppleCgBI(string imagePath, int expectedWidth, int expected Assert.Equal(expectedHeight, imageInfo.Height); } + [Theory] + [InlineData(TestImages.Png.Cgbi.BitDepth16)] + [InlineData(TestImages.Png.Cgbi.Palette)] + public void Identify_CgBI_IncompatibleHeader_ThrowsInvalidImageContentException(string imagePath) + { + TestFile testFile = TestFile.Create(imagePath); + using MemoryStream stream = new(testFile.Bytes, false); + InvalidImageContentException ex = Assert.Throws(() => Image.Identify(stream)); + Assert.Contains("CgBI is only supported for 8-bit truecolor images", ex.Message); + } + + [Theory] + [WithFile(TestImages.Png.Cgbi.BitDepth16, PixelTypes.Rgba32)] + [WithFile(TestImages.Png.Cgbi.Palette, PixelTypes.Rgba32)] + public void Decode_CgBI_IncompatibleHeader_ThrowsInvalidImageContentException(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + InvalidImageContentException ex = Assert.Throws( + () => { using Image image = provider.GetImage(PngDecoder.Instance); }); + Assert.Contains("CgBI is only supported for 8-bit truecolor images", ex.Message); + } + [Theory] [WithFile(TestImages.Png.Splash, PixelTypes.Rgba32)] [WithFile(TestImages.Png.Bike, PixelTypes.Rgba32)] diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 1b6ae56850..7b43ab262c 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -189,6 +189,10 @@ public static class Cgbi // Issue 410: https://github.com/SixLabors/ImageSharp/issues/410 public const string Issue410 = "Png/issues/Issue_410.png"; + + // Synthetic fixtures derived from colors.png to exercise CgBI validation. + public const string BitDepth16 = "Png/cgbi/colors-cgbi-bitdepth16.png"; + public const string Palette = "Png/cgbi/colors-cgbi-palette.png"; } public static class Bad diff --git a/tests/Images/Input/Png/cgbi/colors-cgbi-bitdepth16.png b/tests/Images/Input/Png/cgbi/colors-cgbi-bitdepth16.png new file mode 100644 index 0000000000..18cfa9246d --- /dev/null +++ b/tests/Images/Input/Png/cgbi/colors-cgbi-bitdepth16.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:59610bc03f6ca867e5f71c574b3a0d1942c9e3a230c8a32bf3007cb82f286866 +size 12853 diff --git a/tests/Images/Input/Png/cgbi/colors-cgbi-palette.png b/tests/Images/Input/Png/cgbi/colors-cgbi-palette.png new file mode 100644 index 0000000000..f6406559b1 --- /dev/null +++ b/tests/Images/Input/Png/cgbi/colors-cgbi-palette.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:a3a2f20c69ae423523a8f41887e3f37257a338f2220c2ea44d35c87daf8c3aa3 +size 12853 From fdc15d283ec21dac7df993d4795389a4bab74255 Mon Sep 17 00:00:00 2001 From: Erik White <26148654+Erik-White@users.noreply.github.com> Date: Wed, 27 May 2026 10:58:59 +0200 Subject: [PATCH 02/11] Separate Zlib header validation fom stream reading --- .../Compression/Zlib/ChunkedReadStream.cs | 125 ++++++++++ .../Compression/Zlib/ZlibInflateStream.cs | 219 ++---------------- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 26 ++- 3 files changed, 167 insertions(+), 203 deletions(-) create mode 100644 src/ImageSharp/Compression/Zlib/ChunkedReadStream.cs diff --git a/src/ImageSharp/Compression/Zlib/ChunkedReadStream.cs b/src/ImageSharp/Compression/Zlib/ChunkedReadStream.cs new file mode 100644 index 0000000000..8c5189d4a9 --- /dev/null +++ b/src/ImageSharp/Compression/Zlib/ChunkedReadStream.cs @@ -0,0 +1,125 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using SixLabors.ImageSharp.IO; + +namespace SixLabors.ImageSharp.Compression.Zlib; + +/// +/// A read-only stream over a sequence of length-delimited segments. Bytes are +/// pulled from the inner stream up to the current segment's remaining length; +/// when the segment is exhausted the supplied delegate is invoked to advance +/// to the next segment and return its length. The inner stream is not owned +/// and is not disposed. +/// +internal sealed class ChunkedReadStream : Stream +{ + private static readonly Func GetDataNoOp = () => 0; + + private readonly BufferedReadStream innerStream; + private readonly Func getData; + private int currentDataRemaining; + + public ChunkedReadStream(BufferedReadStream innerStream) + : this(innerStream, GetDataNoOp) + { + } + + public ChunkedReadStream(BufferedReadStream innerStream, Func getData) + { + this.innerStream = innerStream; + this.getData = getData; + } + + /// + public override bool CanRead => this.innerStream.CanRead; + + /// + public override bool CanSeek => false; + + /// + public override bool CanWrite => throw new NotSupportedException(); + + /// + public override long Length => throw new NotSupportedException(); + + /// + public override long Position { get => throw new NotSupportedException(); set => throw new NotSupportedException(); } + + /// + /// Sets the number of bytes available to read from the current segment. + /// Must be called before reading each segment. + /// + public void SetCurrentSegmentLength(int bytes) => this.currentDataRemaining = bytes; + + /// + public override void Flush() => throw new NotSupportedException(); + + /// + public override int ReadByte() + { + this.currentDataRemaining--; + return this.innerStream.ReadByte(); + } + + /// + public override int Read(byte[] buffer, int offset, int count) + { + if (this.currentDataRemaining is 0) + { + // Current segment is exhausted; ask the caller for the next one. + this.currentDataRemaining = this.getData(); + + if (this.currentDataRemaining is 0) + { + return 0; + } + } + + int bytesToRead = Math.Min(count, this.currentDataRemaining); + this.currentDataRemaining -= bytesToRead; + int totalBytesRead = this.innerStream.Read(buffer, offset, bytesToRead); + long innerStreamLength = this.innerStream.Length; + + // Keep reading data until we've reached the end of the stream or filled the buffer. + int bytesRead = 0; + offset += totalBytesRead; + while (this.currentDataRemaining is 0 && totalBytesRead < count) + { + this.currentDataRemaining = this.getData(); + + if (this.currentDataRemaining is 0) + { + return totalBytesRead; + } + + offset += bytesRead; + + if (offset >= innerStreamLength || offset >= count) + { + return totalBytesRead; + } + + bytesToRead = Math.Min(count - totalBytesRead, this.currentDataRemaining); + this.currentDataRemaining -= bytesToRead; + bytesRead = this.innerStream.Read(buffer, offset, bytesToRead); + if (bytesRead == 0) + { + return totalBytesRead; + } + + totalBytesRead += bytesRead; + } + + return totalBytesRead; + } + + /// + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + + /// + public override void SetLength(long value) => throw new NotSupportedException(); + + /// + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); +} diff --git a/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs b/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs index 513171b179..11f34dac8a 100644 --- a/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs +++ b/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs @@ -8,9 +8,11 @@ namespace SixLabors.ImageSharp.Compression.Zlib; /// -/// Provides methods and properties for deframing streams from PNGs. +/// Reads chunked input, parses the zlib CMF/FLG header, and exposes a +/// over the remaining DEFLATE payload. The +/// Adler-32 trailer is not validated. /// -internal sealed class ZlibInflateStream : Stream +internal sealed class ZlibInflateStream : IDisposable { /// /// Used to read the Adler-32 and Crc-32 checksums. @@ -19,94 +21,13 @@ internal sealed class ZlibInflateStream : Stream /// private static readonly byte[] ChecksumBuffer = new byte[4]; - /// - /// A default delegate to get more data from the inner stream. - /// - private static readonly Func GetDataNoOp = () => 0; - - /// - /// The inner raw memory stream. - /// - private readonly BufferedReadStream innerStream; - - /// - /// A value indicating whether this instance of the given entity has been disposed. - /// - /// if this instance has been disposed; otherwise, . - /// - /// If the entity is disposed, it must not be disposed a second - /// time. The isDisposed field is set the first time the entity - /// is disposed. If the isDisposed field is true, then the Dispose() - /// method will not dispose again. This help not to prolong the entity's - /// life in the Garbage Collector. - /// - private bool isDisposed; - - /// - /// The current data remaining to be read. - /// - private int currentDataRemaining; - - /// - /// Delegate to get more data once we've exhausted the current data remaining. - /// - private readonly Func getData; - - /// - /// When true, the inflated payload is treated as a raw DEFLATE stream with no zlib - /// CMF/FLG header (and no Adler-32 trailer). This is required to decode IDATs in - /// Apple's proprietary CgBI PNG variant. - /// - private readonly bool noHeader; + private readonly ChunkedReadStream segmentStream; - /// - /// Initializes a new instance of the class. - /// - /// The inner raw stream. public ZlibInflateStream(BufferedReadStream innerStream) - : this(innerStream, GetDataNoOp, noHeader: false) - { - } + => this.segmentStream = new ChunkedReadStream(innerStream); - /// - /// Initializes a new instance of the class. - /// - /// The inner raw stream. - /// A delegate to get more data from the inner stream. public ZlibInflateStream(BufferedReadStream innerStream, Func getData) - : this(innerStream, getData, noHeader: false) - { - } - - /// - /// Initializes a new instance of the class. - /// - /// The inner raw stream. - /// A delegate to get more data from the inner stream. - /// - /// When , the payload is treated as raw DEFLATE with no zlib header. - /// - public ZlibInflateStream(BufferedReadStream innerStream, Func getData, bool noHeader) - { - this.innerStream = innerStream; - this.getData = getData; - this.noHeader = noHeader; - } - - /// - public override bool CanRead => this.innerStream.CanRead; - - /// - public override bool CanSeek => false; - - /// - public override bool CanWrite => throw new NotSupportedException(); - - /// - public override long Length => throw new NotSupportedException(); - - /// - public override long Position { get => throw new NotSupportedException(); set => throw new NotSupportedException(); } + => this.segmentStream = new ChunkedReadStream(innerStream, getData); /// /// Gets the compressed stream over the deframed inner stream. @@ -114,15 +35,16 @@ public ZlibInflateStream(BufferedReadStream innerStream, Func getData, bool public DeflateStream? CompressedStream { get; private set; } /// - /// Adds new bytes from a frame found in the original stream. + /// Sets the length of the next segment of compressed input and, on first + /// call, parses the zlib header. /// - /// The current remaining data according to the chunk length. - /// Whether the chunk to be inflated is a critical chunk. + /// The remaining data length for the current segment. + /// Whether to throw on a malformed zlib header. /// The . [MemberNotNullWhen(true, nameof(CompressedStream))] public bool AllocateNewBytes(int bytes, bool isCriticalChunk) { - this.currentDataRemaining = bytes; + this.segmentStream.SetCurrentSegmentLength(bytes); if (this.CompressedStream is null) { return this.InitializeInflateStream(isCriticalChunk); @@ -131,114 +53,15 @@ public bool AllocateNewBytes(int bytes, bool isCriticalChunk) return true; } - /// - public override void Flush() => throw new NotSupportedException(); - - /// - public override int ReadByte() + public void Dispose() { - this.currentDataRemaining--; - return this.innerStream.ReadByte(); - } - - /// - public override int Read(byte[] buffer, int offset, int count) - { - if (this.currentDataRemaining is 0) - { - // Last buffer was read in its entirety, let's make sure we don't actually have more in additional IDAT chunks. - this.currentDataRemaining = this.getData(); - - if (this.currentDataRemaining is 0) - { - return 0; - } - } - - int bytesToRead = Math.Min(count, this.currentDataRemaining); - this.currentDataRemaining -= bytesToRead; - int totalBytesRead = this.innerStream.Read(buffer, offset, bytesToRead); - long innerStreamLength = this.innerStream.Length; - - // Keep reading data until we've reached the end of the stream or filled the buffer. - int bytesRead = 0; - offset += totalBytesRead; - while (this.currentDataRemaining is 0 && totalBytesRead < count) - { - this.currentDataRemaining = this.getData(); - - if (this.currentDataRemaining is 0) - { - return totalBytesRead; - } - - offset += bytesRead; - - if (offset >= innerStreamLength || offset >= count) - { - return totalBytesRead; - } - - bytesToRead = Math.Min(count - totalBytesRead, this.currentDataRemaining); - this.currentDataRemaining -= bytesToRead; - bytesRead = this.innerStream.Read(buffer, offset, bytesToRead); - if (bytesRead == 0) - { - return totalBytesRead; - } - - totalBytesRead += bytesRead; - } - - return totalBytesRead; - } - - /// - public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); - - /// - public override void SetLength(long value) => throw new NotSupportedException(); - - /// - public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); - - /// - protected override void Dispose(bool disposing) - { - if (this.isDisposed) - { - return; - } - - if (disposing) - { - // Dispose managed resources. - if (this.CompressedStream != null) - { - this.CompressedStream.Dispose(); - this.CompressedStream = null; - } - } - - base.Dispose(disposing); - - // Call the appropriate methods to clean up - // unmanaged resources here. - // Note disposing is done. - this.isDisposed = true; + this.CompressedStream?.Dispose(); + this.segmentStream?.Dispose(); } [MemberNotNullWhen(true, nameof(CompressedStream))] private bool InitializeInflateStream(bool isCriticalChunk) { - // Apple CgBI IDATs omit the zlib CMF/FLG header and the Adler-32 trailer, - // wrapping a raw DEFLATE payload directly. Skip the header parsing in that mode. - if (this.noHeader) - { - this.CompressedStream = new DeflateStream(this, CompressionMode.Decompress, true); - return true; - } - // Read the zlib header : http://tools.ietf.org/html/rfc1950 // CMF(Compression Method and flags) // This byte is divided into a 4 - bit compression method and a @@ -250,9 +73,8 @@ private bool InitializeInflateStream(bool isCriticalChunk) // +---+---+ // |CMF|FLG| // +---+---+ - int cmf = this.innerStream.ReadByte(); - int flag = this.innerStream.ReadByte(); - this.currentDataRemaining -= 2; + int cmf = this.segmentStream.ReadByte(); + int flag = this.segmentStream.ReadByte(); if (cmf == -1 || flag == -1) { return false; @@ -290,16 +112,13 @@ private bool InitializeInflateStream(bool isCriticalChunk) { // We don't need this for inflate so simply skip by the next four bytes. // https://tools.ietf.org/html/rfc1950#page-6 - if (this.innerStream.Read(ChecksumBuffer, 0, 4) != 4) + if (this.segmentStream.Read(ChecksumBuffer, 0, 4) != 4) { return false; } - - this.currentDataRemaining -= 4; } - // Initialize the deflate BufferedReadStream. - this.CompressedStream = new DeflateStream(this, CompressionMode.Decompress, true); + this.CompressedStream = new DeflateStream(this.segmentStream, CompressionMode.Decompress, leaveOpen: true); return true; } diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 0d93429bb8..49b6fabc60 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -767,7 +767,7 @@ private int CalculateScanlineLength(int width) /// The length of the chunk that containing the compressed scanline data. /// The pixel data. /// The png metadata - /// A delegate to get more data from the inner stream for . + /// A delegate to get more data from the inner stream when chunk boundaries are crossed. /// The frame control /// The cancellation token. private void ReadScanlines( @@ -779,14 +779,34 @@ private void ReadScanlines( CancellationToken cancellationToken) where TPixel : unmanaged, IPixel { - using ZlibInflateStream inflateStream = new(this.currentStream, getData, noHeader: this.isCgbi); + // CgBI IDATs wrap a raw DEFLATE payload directly (no zlib CMF/FLG header + // and no Adler-32 trailer); skip the zlib header parser entirely. + if (this.isCgbi) + { + using ChunkedReadStream segmentStream = new(this.currentStream, getData); + segmentStream.SetCurrentSegmentLength(chunkLength); + using DeflateStream cgbiDataStream = new(segmentStream, CompressionMode.Decompress, leaveOpen: true); + this.DecodeFromDeflate(cgbiDataStream, image, pngMetadata, frameControl, cancellationToken); + return; + } + + using ZlibInflateStream inflateStream = new(this.currentStream, getData); if (!inflateStream.AllocateNewBytes(chunkLength, !this.hasImageData)) { return; } - DeflateStream dataStream = inflateStream.CompressedStream!; + this.DecodeFromDeflate(inflateStream.CompressedStream!, image, pngMetadata, frameControl, cancellationToken); + } + private void DecodeFromDeflate( + DeflateStream dataStream, + ImageFrame image, + PngMetadata pngMetadata, + in FrameControl frameControl, + CancellationToken cancellationToken) + where TPixel : unmanaged, IPixel + { if (this.header.InterlaceMethod is PngInterlaceMode.Adam7) { this.DecodeInterlacedPixelData(frameControl, dataStream, image, pngMetadata, cancellationToken); From a68921cac36dc6f5590ce2921fb0e27b6e033954 Mon Sep 17 00:00:00 2001 From: Erik White <26148654+Erik-White@users.noreply.github.com> Date: Wed, 27 May 2026 12:50:51 +0200 Subject: [PATCH 03/11] Break out cgbi helper methods --- .../Formats/Png/PngCgbiProcessor.cs | 325 ++++++++++++++++++ src/ImageSharp/Formats/Png/PngDecoderCore.cs | 305 +--------------- .../Formats/Png/PngCgbiProcessorTests.cs | 174 ++++++++++ 3 files changed, 501 insertions(+), 303 deletions(-) create mode 100644 src/ImageSharp/Formats/Png/PngCgbiProcessor.cs create mode 100644 tests/ImageSharp.Tests/Formats/Png/PngCgbiProcessorTests.cs diff --git a/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs b/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs new file mode 100644 index 0000000000..6478acced4 --- /dev/null +++ b/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs @@ -0,0 +1,325 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using System.Runtime.Intrinsics; +using System.Runtime.Intrinsics.X86; +using SixLabors.ImageSharp.Common.Helpers; +using SixLabors.ImageSharp.PixelFormats; +using static SixLabors.ImageSharp.SimdUtils; + +namespace SixLabors.ImageSharp.Formats.Png; + +/// +/// Reverses the pixel mangling applied by Apple's CgBI PNG variant. CgBI files +/// (emitted by pngcrush -iphone) swap channel order from RGB(A) to BGR(A) +/// and premultiply RGB samples by alpha. This converts a defiltered scanline back +/// to standard PNG semantics in place so the existing scanline processors can +/// consume it unchanged. CgBI is only emitted for 8-bit truecolor (with or +/// without alpha); other color types are left alone. +/// +/// +/// See https://theapplewiki.com/wiki/PNG_CgBI_Format +/// +internal static class PngCgbiProcessor +{ + // Per-pixel byte indices that swap CgBI's BGRA layout to Rgba32's RGBA. + // MMShuffle3012 expands to [2, 1, 0, 3] per 4-byte pixel; the same 64-byte + // sequence seeds all three shuffle masks (V128/V256 ctors take the leading + // 16/32 bytes). + private static readonly Vector128 BgraToRgbaShuffle128 = Vector128.Create(BuildShuffleBytes()); + + private static readonly Vector256 BgraToRgbaShuffle256 = Vector256.Create(BuildShuffleBytes()); + + private static readonly Vector512 BgraToRgbaShuffle512 = Vector512.Create(BuildShuffleBytes()); + + /// + /// Applies the inverse of Apple's CgBI pixel mangling to a defiltered scanline in place. + /// + /// The configuration used by the Rgb24 R/B swap. + /// The defiltered pixel bytes (without the leading filter byte). + /// The PNG color type from IHDR. + public static void ApplyTransform(Configuration configuration, Span scanline, PngColorType colorType) + { + if (colorType == PngColorType.RgbWithAlpha) + { + Span pixels = MemoryMarshal.Cast(scanline); + int i = 0; + + // Avx512BW is required for the per-byte vpshufb. Without it, ShuffleNative + // falls back to a software cross-lane shuffle that is slower than the V256 + // path, so skip V512 entirely on Avx512F-only hosts. + if (Vector512.IsHardwareAccelerated && Avx512BW.IsSupported && pixels.Length >= 16) + { + i = ApplyTransformVector512(scanline, pixels.Length); + } + + if (Vector256.IsHardwareAccelerated && Avx2.IsSupported && (pixels.Length - i) >= 8) + { + i = ApplyTransformVector256(scanline, i, pixels.Length); + } + + if (Vector128.IsHardwareAccelerated && (pixels.Length - i) >= 4) + { + i = ApplyTransformVector128(scanline, i, pixels.Length); + } + + for (; i < pixels.Length; i++) + { + ref Rgba32 pixel = ref pixels[i]; + pixel = new Rgba32(pixel.B, pixel.G, pixel.R, pixel.A); + UndoPremultiplicationScalar(ref pixel); + } + } + else if (colorType == PngColorType.Rgb) + { + // No alpha channel, so just swap R and B using built in SIMD-optimized pixel operations. + Span target = MemoryMarshal.Cast(scanline); + PixelOperations.Instance.FromBgr24Bytes(configuration, scanline, target, target.Length); + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void UndoPremultiplicationScalar(ref Rgba32 pixel) + { + byte a = pixel.A; + if (a is 0 or byte.MaxValue) + { + return; + } + + // Reverse: c' = c * a / 255 => c = round(c' * 255 / a) + int half = a >> 1; + byte r = (byte)Math.Min(byte.MaxValue, ((pixel.R * byte.MaxValue) + half) / a); + byte g = (byte)Math.Min(byte.MaxValue, ((pixel.G * byte.MaxValue) + half) / a); + byte b = (byte)Math.Min(byte.MaxValue, ((pixel.B * byte.MaxValue) + half) / a); + pixel = new Rgba32(r, g, b, a); + } + + internal static int ApplyTransformVector512(Span scanline, int pixelCount) + { + ref byte scanlineRef = ref MemoryMarshal.GetReference(scanline); + int i = 0; + + // The mask only swaps bytes inside each 4-byte pixel, so it is correct + // for the per-lane Avx512BW.Shuffle that ShuffleNative selects here. + Vector512 shuffleMask = BgraToRgbaShuffle512; + + Vector512 zero = Vector512.Zero; + Vector512 one = Vector512.One; + Vector512 byteMask = Vector512.Create(0xFF); + Vector512 opaque = Vector512.Create(0xFF); + Vector512 byteMax = Vector512.Create((int)byte.MaxValue); + + for (; i <= pixelCount - 16; i += 16) + { + ref byte blockRef = ref Unsafe.Add(ref scanlineRef, i * Unsafe.SizeOf()); + Vector512 bgra = Unsafe.ReadUnaligned>(ref blockRef); + Vector512 rgba = Vector512_.ShuffleNative(bgra, shuffleMask); + Vector512 packed = rgba.AsInt32(); + Vector512 alpha = Vector512.ShiftRightLogical(packed, 24); + + // Fully transparent and fully opaque pixels are identity cases for + // unpremultiplication. Masking them keeps the scalar behavior and lets + // safeAlpha avoid dividing by zero for alpha == 0. + Vector512 partialMask = ~(Vector512.Equals(alpha, zero) | Vector512.Equals(alpha, opaque)); + + Vector512 r = packed & byteMask; + Vector512 g = Vector512.ShiftRightLogical(packed, 8) & byteMask; + Vector512 b = Vector512.ShiftRightLogical(packed, 16) & byteMask; + + Vector512 safeAlpha = Vector512.ConditionalSelect(partialMask, alpha, one); + Vector512 halfAlpha = Vector512.ShiftRightLogical(safeAlpha, 1); + Vector512 safeAlphaF = Vector512.ConvertToSingle(safeAlpha); + + // The scalar path computes ((c * 255) + (a >> 1)) / a with integer + // division. Floor the positive quotient before converting so SIMD does + // not use the default round-to-nearest conversion and drift by one. + Vector512 unpremultipliedR = Vector512.Min( + byteMax, + Vector512.ConvertToInt32(Vector512.Floor(Vector512.ConvertToSingle((r * byteMax) + halfAlpha) / safeAlphaF))); + + Vector512 unpremultipliedG = Vector512.Min( + byteMax, + Vector512.ConvertToInt32(Vector512.Floor(Vector512.ConvertToSingle((g * byteMax) + halfAlpha) / safeAlphaF))); + + Vector512 unpremultipliedB = Vector512.Min( + byteMax, + Vector512.ConvertToInt32(Vector512.Floor(Vector512.ConvertToSingle((b * byteMax) + halfAlpha) / safeAlphaF))); + + // ConditionalSelect applies the expensive unpremultiply only to pixels + // where alpha is between 1 and 254; alpha 0 and 255 lanes keep the + // shuffled channel values exactly as the scalar path does. + Vector512 finalR = Vector512.ConditionalSelect(partialMask, unpremultipliedR, r); + Vector512 finalG = Vector512.ConditionalSelect(partialMask, unpremultipliedG, g); + Vector512 finalB = Vector512.ConditionalSelect(partialMask, unpremultipliedB, b); + + // Rgba32 is laid out as little-endian 0xAABBGGRR in an int lane, so + // shifting the unpacked channels back to byte offsets 0, 1, 2, and 3 + // recreates the in-memory RGBA bytes for the unaligned store. + Vector512 result = + finalR | + Vector512.ShiftLeft(finalG, 8) | + Vector512.ShiftLeft(finalB, 16) | + Vector512.ShiftLeft(alpha, 24); + + Unsafe.WriteUnaligned(ref blockRef, result.AsByte()); + } + + return i; + } + + internal static int ApplyTransformVector256(Span scanline, int startPixel, int pixelCount) + { + ref byte scanlineRef = ref MemoryMarshal.GetReference(scanline); + int i = startPixel; + + // vpshufb is 128-bit lane-local and uses only the low 4 bits of each + // index, so the same per-pixel [2,1,0,3] pattern in both lanes keeps + // every byte inside its own lane. + Vector256 shuffleMask = BgraToRgbaShuffle256; + + Vector256 zero = Vector256.Zero; + Vector256 one = Vector256.One; + Vector256 byteMask = Vector256.Create(0xFF); + Vector256 opaque = Vector256.Create(0xFF); + Vector256 byteMax = Vector256.Create((int)byte.MaxValue); + + for (; i <= pixelCount - 8; i += 8) + { + ref byte blockRef = ref Unsafe.Add(ref scanlineRef, i * Unsafe.SizeOf()); + Vector256 bgra = Unsafe.ReadUnaligned>(ref blockRef); + Vector256 rgba = Vector256_.ShufflePerLane(bgra, shuffleMask); + Vector256 packed = rgba.AsInt32(); + Vector256 alpha = Vector256.ShiftRightLogical(packed, 24); + + // Fully transparent and fully opaque pixels are identity cases for + // unpremultiplication. Masking them keeps the scalar behavior and lets + // safeAlpha avoid dividing by zero for alpha == 0. + Vector256 partialMask = ~(Vector256.Equals(alpha, zero) | Vector256.Equals(alpha, opaque)); + + Vector256 r = packed & byteMask; + Vector256 g = Vector256.ShiftRightLogical(packed, 8) & byteMask; + Vector256 b = Vector256.ShiftRightLogical(packed, 16) & byteMask; + + Vector256 safeAlpha = Vector256.ConditionalSelect(partialMask, alpha, one); + Vector256 halfAlpha = Vector256.ShiftRightLogical(safeAlpha, 1); + Vector256 safeAlphaF = Vector256.ConvertToSingle(safeAlpha); + + // The scalar path computes ((c * 255) + (a >> 1)) / a with integer + // division. Floor the positive quotient before converting so SIMD does + // not use the default round-to-nearest conversion and drift by one. + Vector256 unpremultipliedR = Vector256.Min( + byteMax, + Vector256.ConvertToInt32(Vector256.Floor(Vector256.ConvertToSingle((r * byteMax) + halfAlpha) / safeAlphaF))); + + Vector256 unpremultipliedG = Vector256.Min( + byteMax, + Vector256.ConvertToInt32(Vector256.Floor(Vector256.ConvertToSingle((g * byteMax) + halfAlpha) / safeAlphaF))); + + Vector256 unpremultipliedB = Vector256.Min( + byteMax, + Vector256.ConvertToInt32(Vector256.Floor(Vector256.ConvertToSingle((b * byteMax) + halfAlpha) / safeAlphaF))); + + // ConditionalSelect applies the expensive unpremultiply only to pixels + // where alpha is between 1 and 254; alpha 0 and 255 lanes keep the + // shuffled channel values exactly as the scalar path does. + Vector256 finalR = Vector256.ConditionalSelect(partialMask, unpremultipliedR, r); + Vector256 finalG = Vector256.ConditionalSelect(partialMask, unpremultipliedG, g); + Vector256 finalB = Vector256.ConditionalSelect(partialMask, unpremultipliedB, b); + + // Rgba32 is laid out as little-endian 0xAABBGGRR in an int lane, so + // shifting the unpacked channels back to byte offsets 0, 1, 2, and 3 + // recreates the in-memory RGBA bytes for the unaligned store. + Vector256 result = + finalR | + Vector256.ShiftLeft(finalG, 8) | + Vector256.ShiftLeft(finalB, 16) | + Vector256.ShiftLeft(alpha, 24); + + Unsafe.WriteUnaligned(ref blockRef, result.AsByte()); + } + + return i; + } + + internal static int ApplyTransformVector128(Span scanline, int startPixel, int pixelCount) + { + ref byte scanlineRef = ref MemoryMarshal.GetReference(scanline); + int i = startPixel; + + Vector128 shuffleMask = BgraToRgbaShuffle128; + + Vector128 zero = Vector128.Zero; + Vector128 one = Vector128.One; + Vector128 byteMask = Vector128.Create(0xFF); + Vector128 opaque = Vector128.Create(0xFF); + Vector128 byteMax = Vector128.Create((int)byte.MaxValue); + + for (; i <= pixelCount - 4; i += 4) + { + ref byte blockRef = ref Unsafe.Add(ref scanlineRef, i * Unsafe.SizeOf()); + Vector128 bgra = Unsafe.ReadUnaligned>(ref blockRef); + Vector128 rgba = Vector128_.ShuffleNative(bgra, shuffleMask); + Vector128 packed = rgba.AsInt32(); + Vector128 alpha = Vector128.ShiftRightLogical(packed, 24); + + // Fully transparent and fully opaque pixels are identity cases for + // unpremultiplication. Masking them keeps the scalar behavior and lets + // safeAlpha avoid dividing by zero for alpha == 0. + Vector128 partialMask = ~(Vector128.Equals(alpha, zero) | Vector128.Equals(alpha, opaque)); + + Vector128 r = packed & byteMask; + Vector128 g = Vector128.ShiftRightLogical(packed, 8) & byteMask; + Vector128 b = Vector128.ShiftRightLogical(packed, 16) & byteMask; + + Vector128 safeAlpha = Vector128.ConditionalSelect(partialMask, alpha, one); + Vector128 halfAlpha = Vector128.ShiftRightLogical(safeAlpha, 1); + Vector128 safeAlphaF = Vector128.ConvertToSingle(safeAlpha); + + // The scalar path computes ((c * 255) + (a >> 1)) / a with integer + // division. Floor the positive quotient before converting so SIMD does + // not use the default round-to-nearest conversion and drift by one. + Vector128 unpremultipliedR = Vector128.Min( + byteMax, + Vector128.ConvertToInt32(Vector128.Floor(Vector128.ConvertToSingle((r * byteMax) + halfAlpha) / safeAlphaF))); + + Vector128 unpremultipliedG = Vector128.Min( + byteMax, + Vector128.ConvertToInt32(Vector128.Floor(Vector128.ConvertToSingle((g * byteMax) + halfAlpha) / safeAlphaF))); + + Vector128 unpremultipliedB = Vector128.Min( + byteMax, + Vector128.ConvertToInt32(Vector128.Floor(Vector128.ConvertToSingle((b * byteMax) + halfAlpha) / safeAlphaF))); + + // ConditionalSelect applies the expensive unpremultiply only to pixels + // where alpha is between 1 and 254; alpha 0 and 255 lanes keep the + // shuffled channel values exactly as the scalar path does. + Vector128 finalR = Vector128.ConditionalSelect(partialMask, unpremultipliedR, r); + Vector128 finalG = Vector128.ConditionalSelect(partialMask, unpremultipliedG, g); + Vector128 finalB = Vector128.ConditionalSelect(partialMask, unpremultipliedB, b); + + // Rgba32 is laid out as little-endian 0xAABBGGRR in an int lane, so + // shifting the unpacked channels back to byte offsets 0, 1, 2, and 3 + // recreates the in-memory RGBA bytes for the unaligned store. + Vector128 result = + finalR | + Vector128.ShiftLeft(finalG, 8) | + Vector128.ShiftLeft(finalB, 16) | + Vector128.ShiftLeft(alpha, 24); + + Unsafe.WriteUnaligned(ref blockRef, result.AsByte()); + } + + return i; + } + + private static byte[] BuildShuffleBytes() + { + byte[] bytes = new byte[64]; + Span span = bytes; + Shuffle.MMShuffleSpan(ref span, Shuffle.MMShuffle3012); + return bytes; + } +} diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 49b6fabc60..f3e2bbdbe0 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -9,8 +9,6 @@ using System.IO.Hashing; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; -using System.Runtime.Intrinsics; -using System.Runtime.Intrinsics.X86; using System.Text; using SixLabors.ImageSharp.Common.Helpers; using SixLabors.ImageSharp.Compression.Zlib; @@ -922,7 +920,7 @@ private void DecodePixelDataCore( if (this.isCgbi) { - this.ApplyCgbiTransform(scanSpan[1..], this.pngColorType); + PngCgbiProcessor.ApplyTransform(this.configuration, scanSpan[1..], this.pngColorType); } this.ProcessDefilteredScanline(frameControl, currentRow, scanSpan, imageFrame, pngMetadata, blendRowBuffer); @@ -1057,7 +1055,7 @@ private void DecodeInterlacedPixelDataCore( if (this.isCgbi) { - this.ApplyCgbiTransform(scanSpan[1..], this.pngColorType); + PngCgbiProcessor.ApplyTransform(this.configuration, scanSpan[1..], this.pngColorType); } Span rowSpan = imageBuffer.DangerousGetRowSpan(currentRow); @@ -2529,303 +2527,4 @@ private static bool IsXmpTextData(ReadOnlySpan keywordBytes) private void SwapScanlineBuffers() => (this.scanline, this.previousScanline) = (this.previousScanline, this.scanline); - - /// - /// Applies the inverse of Apple's CgBI pixel mangling to a defiltered scanline. - /// CgBI PNGs are emitted by pngcrush -iphone with channel order swapped - /// from RGB(A) to BGR(A) and RGB samples premultiplied by alpha. This converts - /// the bytes back to standard PNG semantics in place so the existing scanline - /// processors can consume them unchanged. CgBI is only emitted for 8-bit - /// truecolor (with or without alpha); other color types are left alone. - /// - /// - /// See https://theapplewiki.com/wiki/PNG_CgBI_Format - /// - /// The defiltered pixel bytes (without the leading filter byte). - /// The PNG color type from IHDR. - private void ApplyCgbiTransform(Span scanline, PngColorType colorType) - { - if (colorType == PngColorType.RgbWithAlpha) - { - Span pixels = MemoryMarshal.Cast(scanline); - int i = 0; - - if (Vector512.IsHardwareAccelerated && pixels.Length >= 16) - { - i = ApplyCgbiTransformVector512(scanline, pixels.Length); - } - - if (Vector256.IsHardwareAccelerated && Avx2.IsSupported && (pixels.Length - i) >= 8) - { - i = ApplyCgbiTransformVector256(scanline, i, pixels.Length); - } - - if (Vector128.IsHardwareAccelerated && (pixels.Length - i) >= 4) - { - i = ApplyCgbiTransformVector128(scanline, i, pixels.Length); - } - - for (; i < pixels.Length; i++) - { - ref Rgba32 pixel = ref pixels[i]; - pixel = new Rgba32(pixel.B, pixel.G, pixel.R, pixel.A); - UndoCgbiPremultiplicationScalar(ref pixel); - } - } - else if (colorType == PngColorType.Rgb) - { - // No alpha channel, so just swap R and B using built in SIMD-optimized pixel operations. - Span target = MemoryMarshal.Cast(scanline); - PixelOperations.Instance.FromBgr24Bytes(this.configuration, scanline, target, target.Length); - } - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static void UndoCgbiPremultiplicationScalar(ref Rgba32 pixel) - { - byte a = pixel.A; - if (a is 0 or byte.MaxValue) - { - return; - } - - // Reverse: c' = c * a / 255 => c = round(c' * 255 / a) - int half = a >> 1; - byte r = (byte)Math.Min(byte.MaxValue, ((pixel.R * byte.MaxValue) + half) / a); - byte g = (byte)Math.Min(byte.MaxValue, ((pixel.G * byte.MaxValue) + half) / a); - byte b = (byte)Math.Min(byte.MaxValue, ((pixel.B * byte.MaxValue) + half) / a); - pixel = new Rgba32(r, g, b, a); - } - - private static int ApplyCgbiTransformVector512(Span scanline, int pixelCount) - { - ref byte scanlineRef = ref MemoryMarshal.GetReference(scanline); - int i = 0; - - Span temp = stackalloc byte[Vector512.Count]; - SimdUtils.Shuffle.MMShuffleSpan(ref temp, SimdUtils.Shuffle.MMShuffle3012); - - // MMShuffle3012 expands to [2, 1, 0, 3] for each 4-byte pixel, converting - // CgBI's BGRA byte order to Rgba32's RGBA layout while keeping alpha in place. - // The generated mask only swaps bytes inside each pixel, so it remains - // correct for the optimized 512-bit byte shuffle helper. - Vector512 shuffleMask = Unsafe.As>(ref MemoryMarshal.GetReference(temp)); - - Vector512 zero = Vector512.Zero; - Vector512 one = Vector512.One; - Vector512 byteMask = Vector512.Create(0xFF); - Vector512 opaque = Vector512.Create(0xFF); - Vector512 byteMax = Vector512.Create((int)byte.MaxValue); - - for (; i <= pixelCount - 16; i += 16) - { - ref byte blockRef = ref Unsafe.Add(ref scanlineRef, i * Unsafe.SizeOf()); - Vector512 bgra = Unsafe.ReadUnaligned>(ref blockRef); - Vector512 rgba = Vector512_.ShuffleNative(bgra, shuffleMask); - Vector512 packed = rgba.AsInt32(); - Vector512 alpha = Vector512.ShiftRightLogical(packed, 24); - - // Fully transparent and fully opaque pixels are identity cases for - // unpremultiplication. Masking them keeps the scalar behavior and lets - // safeAlpha avoid dividing by zero for alpha == 0. - Vector512 partialMask = ~(Vector512.Equals(alpha, zero) | Vector512.Equals(alpha, opaque)); - - Vector512 r = packed & byteMask; - Vector512 g = Vector512.ShiftRightLogical(packed, 8) & byteMask; - Vector512 b = Vector512.ShiftRightLogical(packed, 16) & byteMask; - - Vector512 safeAlpha = Vector512.ConditionalSelect(partialMask, alpha, one); - Vector512 halfAlpha = Vector512.ShiftRightLogical(safeAlpha, 1); - Vector512 safeAlphaF = Vector512.ConvertToSingle(safeAlpha); - - // The scalar path computes ((c * 255) + (a >> 1)) / a with integer - // division. Floor the positive quotient before converting so SIMD does - // not use the default round-to-nearest conversion and drift by one. - Vector512 unpremultipliedR = Vector512.Min( - byteMax, - Vector512.ConvertToInt32(Vector512.Floor(Vector512.ConvertToSingle((r * byteMax) + halfAlpha) / safeAlphaF))); - - Vector512 unpremultipliedG = Vector512.Min( - byteMax, - Vector512.ConvertToInt32(Vector512.Floor(Vector512.ConvertToSingle((g * byteMax) + halfAlpha) / safeAlphaF))); - - Vector512 unpremultipliedB = Vector512.Min( - byteMax, - Vector512.ConvertToInt32(Vector512.Floor(Vector512.ConvertToSingle((b * byteMax) + halfAlpha) / safeAlphaF))); - - // ConditionalSelect applies the expensive unpremultiply only to pixels - // where alpha is between 1 and 254; alpha 0 and 255 lanes keep the - // shuffled channel values exactly as the scalar path does. - Vector512 finalR = Vector512.ConditionalSelect(partialMask, unpremultipliedR, r); - Vector512 finalG = Vector512.ConditionalSelect(partialMask, unpremultipliedG, g); - Vector512 finalB = Vector512.ConditionalSelect(partialMask, unpremultipliedB, b); - - // Rgba32 is laid out as little-endian 0xAABBGGRR in an int lane, so - // shifting the unpacked channels back to byte offsets 0, 1, 2, and 3 - // recreates the in-memory RGBA bytes for the unaligned store. - Vector512 result = - finalR | - Vector512.ShiftLeft(finalG, 8) | - Vector512.ShiftLeft(finalB, 16) | - Vector512.ShiftLeft(alpha, 24); - - Unsafe.WriteUnaligned(ref blockRef, result.AsByte()); - } - - return i; - } - - private static int ApplyCgbiTransformVector256(Span scanline, int startPixel, int pixelCount) - { - ref byte scanlineRef = ref MemoryMarshal.GetReference(scanline); - int i = startPixel; - - Span temp = stackalloc byte[Vector512.Count]; - SimdUtils.Shuffle.MMShuffleSpan(ref temp, SimdUtils.Shuffle.MMShuffle3012); - - // MMShuffle3012 expands to [2, 1, 0, 3] for each 4-byte pixel, converting - // CgBI's BGRA byte order to Rgba32's RGBA layout while keeping alpha in place. - // Avx2.Shuffle is 128-bit lane-local, and the generated mask repeats inside - // each lane, so no byte ever needs to cross the lane boundary. - Vector256 shuffleMask = Unsafe.As>(ref MemoryMarshal.GetReference(temp)); - - Vector256 zero = Vector256.Zero; - Vector256 one = Vector256.One; - Vector256 byteMask = Vector256.Create(0xFF); - Vector256 opaque = Vector256.Create(0xFF); - Vector256 byteMax = Vector256.Create((int)byte.MaxValue); - - for (; i <= pixelCount - 8; i += 8) - { - ref byte blockRef = ref Unsafe.Add(ref scanlineRef, i * Unsafe.SizeOf()); - Vector256 bgra = Unsafe.ReadUnaligned>(ref blockRef); - Vector256 rgba = Vector256_.ShufflePerLane(bgra, shuffleMask); - Vector256 packed = rgba.AsInt32(); - Vector256 alpha = Vector256.ShiftRightLogical(packed, 24); - - // Fully transparent and fully opaque pixels are identity cases for - // unpremultiplication. Masking them keeps the scalar behavior and lets - // safeAlpha avoid dividing by zero for alpha == 0. - Vector256 partialMask = ~(Vector256.Equals(alpha, zero) | Vector256.Equals(alpha, opaque)); - - Vector256 r = packed & byteMask; - Vector256 g = Vector256.ShiftRightLogical(packed, 8) & byteMask; - Vector256 b = Vector256.ShiftRightLogical(packed, 16) & byteMask; - - Vector256 safeAlpha = Vector256.ConditionalSelect(partialMask, alpha, one); - Vector256 halfAlpha = Vector256.ShiftRightLogical(safeAlpha, 1); - Vector256 safeAlphaF = Vector256.ConvertToSingle(safeAlpha); - - // The scalar path computes ((c * 255) + (a >> 1)) / a with integer - // division. Floor the positive quotient before converting so SIMD does - // not use the default round-to-nearest conversion and drift by one. - Vector256 unpremultipliedR = Vector256.Min( - byteMax, - Vector256.ConvertToInt32(Vector256.Floor(Vector256.ConvertToSingle((r * byteMax) + halfAlpha) / safeAlphaF))); - - Vector256 unpremultipliedG = Vector256.Min( - byteMax, - Vector256.ConvertToInt32(Vector256.Floor(Vector256.ConvertToSingle((g * byteMax) + halfAlpha) / safeAlphaF))); - - Vector256 unpremultipliedB = Vector256.Min( - byteMax, - Vector256.ConvertToInt32(Vector256.Floor(Vector256.ConvertToSingle((b * byteMax) + halfAlpha) / safeAlphaF))); - - // ConditionalSelect applies the expensive unpremultiply only to pixels - // where alpha is between 1 and 254; alpha 0 and 255 lanes keep the - // shuffled channel values exactly as the scalar path does. - Vector256 finalR = Vector256.ConditionalSelect(partialMask, unpremultipliedR, r); - Vector256 finalG = Vector256.ConditionalSelect(partialMask, unpremultipliedG, g); - Vector256 finalB = Vector256.ConditionalSelect(partialMask, unpremultipliedB, b); - - // Rgba32 is laid out as little-endian 0xAABBGGRR in an int lane, so - // shifting the unpacked channels back to byte offsets 0, 1, 2, and 3 - // recreates the in-memory RGBA bytes for the unaligned store. - Vector256 result = - finalR | - Vector256.ShiftLeft(finalG, 8) | - Vector256.ShiftLeft(finalB, 16) | - Vector256.ShiftLeft(alpha, 24); - - Unsafe.WriteUnaligned(ref blockRef, result.AsByte()); - } - - return i; - } - - private static int ApplyCgbiTransformVector128(Span scanline, int startPixel, int pixelCount) - { - ref byte scanlineRef = ref MemoryMarshal.GetReference(scanline); - int i = startPixel; - - Span temp = stackalloc byte[Vector512.Count]; - SimdUtils.Shuffle.MMShuffleSpan(ref temp, SimdUtils.Shuffle.MMShuffle3012); - - // MMShuffle3012 expands to [2, 1, 0, 3] for each 4-byte pixel, converting - // CgBI's BGRA byte order to Rgba32's RGBA layout while keeping alpha in place. - Vector128 shuffleMask = Unsafe.As>(ref MemoryMarshal.GetReference(temp)); - - Vector128 zero = Vector128.Zero; - Vector128 one = Vector128.One; - Vector128 byteMask = Vector128.Create(0xFF); - Vector128 opaque = Vector128.Create(0xFF); - Vector128 byteMax = Vector128.Create((int)byte.MaxValue); - - for (; i <= pixelCount - 4; i += 4) - { - ref byte blockRef = ref Unsafe.Add(ref scanlineRef, i * Unsafe.SizeOf()); - Vector128 bgra = Unsafe.ReadUnaligned>(ref blockRef); - Vector128 rgba = Vector128_.ShuffleNative(bgra, shuffleMask); - Vector128 packed = rgba.AsInt32(); - Vector128 alpha = Vector128.ShiftRightLogical(packed, 24); - - // Fully transparent and fully opaque pixels are identity cases for - // unpremultiplication. Masking them keeps the scalar behavior and lets - // safeAlpha avoid dividing by zero for alpha == 0. - Vector128 partialMask = ~(Vector128.Equals(alpha, zero) | Vector128.Equals(alpha, opaque)); - - Vector128 r = packed & byteMask; - Vector128 g = Vector128.ShiftRightLogical(packed, 8) & byteMask; - Vector128 b = Vector128.ShiftRightLogical(packed, 16) & byteMask; - - Vector128 safeAlpha = Vector128.ConditionalSelect(partialMask, alpha, one); - Vector128 halfAlpha = Vector128.ShiftRightLogical(safeAlpha, 1); - Vector128 safeAlphaF = Vector128.ConvertToSingle(safeAlpha); - - // The scalar path computes ((c * 255) + (a >> 1)) / a with integer - // division. Floor the positive quotient before converting so SIMD does - // not use the default round-to-nearest conversion and drift by one. - Vector128 unpremultipliedR = Vector128.Min( - byteMax, - Vector128.ConvertToInt32(Vector128.Floor(Vector128.ConvertToSingle((r * byteMax) + halfAlpha) / safeAlphaF))); - - Vector128 unpremultipliedG = Vector128.Min( - byteMax, - Vector128.ConvertToInt32(Vector128.Floor(Vector128.ConvertToSingle((g * byteMax) + halfAlpha) / safeAlphaF))); - - Vector128 unpremultipliedB = Vector128.Min( - byteMax, - Vector128.ConvertToInt32(Vector128.Floor(Vector128.ConvertToSingle((b * byteMax) + halfAlpha) / safeAlphaF))); - - // ConditionalSelect applies the expensive unpremultiply only to pixels - // where alpha is between 1 and 254; alpha 0 and 255 lanes keep the - // shuffled channel values exactly as the scalar path does. - Vector128 finalR = Vector128.ConditionalSelect(partialMask, unpremultipliedR, r); - Vector128 finalG = Vector128.ConditionalSelect(partialMask, unpremultipliedG, g); - Vector128 finalB = Vector128.ConditionalSelect(partialMask, unpremultipliedB, b); - - // Rgba32 is laid out as little-endian 0xAABBGGRR in an int lane, so - // shifting the unpacked channels back to byte offsets 0, 1, 2, and 3 - // recreates the in-memory RGBA bytes for the unaligned store. - Vector128 result = - finalR | - Vector128.ShiftLeft(finalG, 8) | - Vector128.ShiftLeft(finalB, 16) | - Vector128.ShiftLeft(alpha, 24); - - Unsafe.WriteUnaligned(ref blockRef, result.AsByte()); - } - - return i; - } } diff --git a/tests/ImageSharp.Tests/Formats/Png/PngCgbiProcessorTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngCgbiProcessorTests.cs new file mode 100644 index 0000000000..426afb6d42 --- /dev/null +++ b/tests/ImageSharp.Tests/Formats/Png/PngCgbiProcessorTests.cs @@ -0,0 +1,174 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using System.Runtime.InteropServices; +using SixLabors.ImageSharp.Formats.Png; +using SixLabors.ImageSharp.PixelFormats; + +namespace SixLabors.ImageSharp.Tests.Formats.Png; + +[Trait("Format", "Png")] +public class PngCgbiProcessorTests +{ + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(3)] + [InlineData(4)] + [InlineData(7)] + [InlineData(8)] + [InlineData(15)] + [InlineData(16)] + [InlineData(17)] + [InlineData(31)] + [InlineData(32)] + [InlineData(33)] + [InlineData(64)] + public void ApplyTransform_RgbWithAlpha_MatchesScalar(int pixelCount) + { + // Drives the full V512/V256/V128/scalar dispatch, so it covers each + // path that is hardware-accelerated on the host plus the scalar tail. + byte[] input = CreateBgraScanline(pixelCount); + byte[] processorOutput = (byte[])input.Clone(); + byte[] scalarOutput = (byte[])input.Clone(); + + PngCgbiProcessor.ApplyTransform(Configuration.Default, processorOutput, PngColorType.RgbWithAlpha); + ApplyCgbiTransformScalarReference(scalarOutput); + + Assert.Equal(scalarOutput, processorOutput); + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(3)] + [InlineData(4)] + [InlineData(7)] + [InlineData(8)] + [InlineData(15)] + [InlineData(16)] + [InlineData(17)] + [InlineData(31)] + [InlineData(32)] + [InlineData(33)] + [InlineData(64)] + public void ApplyTransformVector512_MatchesScalar(int pixelCount) => + // Vector512 uses Vector512_.ShuffleNative which falls back to the software + // Vector512.Shuffle when Avx512BW is unavailable, so the body runs regardless + // of whether Vector512 is hardware-accelerated on the host. + AssertVectorMatchesScalar( + pixelCount, + scanline => PngCgbiProcessor.ApplyTransformVector512(scanline, scanline.Length / 4), + blockSize: 16); + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(3)] + [InlineData(4)] + [InlineData(7)] + [InlineData(8)] + [InlineData(15)] + [InlineData(16)] + [InlineData(17)] + [InlineData(31)] + [InlineData(32)] + [InlineData(64)] + public void ApplyTransformVector256_MatchesScalar(int pixelCount) => AssertVectorMatchesScalar( + pixelCount, + scanline => PngCgbiProcessor.ApplyTransformVector256(scanline, 0, scanline.Length / 4), + blockSize: 8); + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(3)] + [InlineData(4)] + [InlineData(7)] + [InlineData(8)] + [InlineData(15)] + [InlineData(16)] + [InlineData(64)] + public void ApplyTransformVector128_MatchesScalar(int pixelCount) => AssertVectorMatchesScalar( + pixelCount, + scanline => PngCgbiProcessor.ApplyTransformVector128(scanline, 0, scanline.Length / 4), + blockSize: 4); + + private static void AssertVectorMatchesScalar(int pixelCount, Func applyVector, int blockSize) + { + byte[] input = CreateBgraScanline(pixelCount); + byte[] vectorOutput = (byte[])input.Clone(); + byte[] scalarOutput = (byte[])input.Clone(); + + int processed = applyVector(vectorOutput); + + int expectedProcessed = (pixelCount / blockSize) * blockSize; + Assert.Equal(expectedProcessed, processed); + + // The vector path is responsible for whole blocks only; remaining pixels are + // handled by the scalar tail in ApplyTransform. Run the scalar reference + // over every pixel and compare the prefix the vector path actually wrote. + ApplyCgbiTransformScalarReference(scalarOutput); + + Span vectorProcessed = vectorOutput.AsSpan(0, processed * 4); + Span scalarProcessed = scalarOutput.AsSpan(0, processed * 4); + Assert.True(vectorProcessed.SequenceEqual(scalarProcessed), $"Mismatch at pixelCount={pixelCount}"); + + // Pixels past the vector's processed prefix must be untouched. + Span vectorTail = vectorOutput.AsSpan(processed * 4); + Span inputTail = input.AsSpan(processed * 4); + Assert.True(vectorTail.SequenceEqual(inputTail)); + } + + private static byte[] CreateBgraScanline(int pixelCount) + { + // Deterministic mix of edge cases (a=0, a=255, partial alpha) and varied channels. + byte[] bytes = new byte[pixelCount * 4]; + for (int p = 0; p < pixelCount; p++) + { + byte a = (p % 7) switch + { + 0 => byte.MinValue, + 1 => byte.MaxValue, + _ => (byte)((((p * 37) + 23) & 0xFF) | 1) // never zero + }; + + // CgBI premultiplied BGRA: c' = c * a / 255 + byte r = (byte)((p * 13) & 0xFF); + byte g = (byte)((p * 29) & 0xFF); + byte b = (byte)((p * 53) & 0xFF); + r = (byte)((r * a) / byte.MaxValue); + g = (byte)((g * a) / byte.MaxValue); + b = (byte)((b * a) / byte.MaxValue); + + bytes[(p * 4) + 0] = b; + bytes[(p * 4) + 1] = g; + bytes[(p * 4) + 2] = r; + bytes[(p * 4) + 3] = a; + } + + return bytes; + } + + private static void ApplyCgbiTransformScalarReference(Span scanline) + { + Span pixels = MemoryMarshal.Cast(scanline); + for (int i = 0; i < pixels.Length; i++) + { + ref Rgba32 pixel = ref pixels[i]; + pixel = new Rgba32(pixel.B, pixel.G, pixel.R, pixel.A); + + byte a = pixel.A; + if (a is 0 or byte.MaxValue) + { + continue; + } + + int half = a >> 1; + byte r = (byte)Math.Min(byte.MaxValue, ((pixel.R * byte.MaxValue) + half) / a); + byte g = (byte)Math.Min(byte.MaxValue, ((pixel.G * byte.MaxValue) + half) / a); + byte b = (byte)Math.Min(byte.MaxValue, ((pixel.B * byte.MaxValue) + half) / a); + pixel = new Rgba32(r, g, b, a); + } + } +} From 9dfc0ca044e96b1109d506cd82911f548a5e33af Mon Sep 17 00:00:00 2001 From: Erik White <26148654+Erik-White@users.noreply.github.com> Date: Wed, 27 May 2026 14:45:32 +0200 Subject: [PATCH 04/11] Remove unnecessary instruction set check --- src/ImageSharp/Formats/Png/PngCgbiProcessor.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs b/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs index 6478acced4..73af0253eb 100644 --- a/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs +++ b/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs @@ -47,10 +47,7 @@ public static void ApplyTransform(Configuration configuration, Span scanli Span pixels = MemoryMarshal.Cast(scanline); int i = 0; - // Avx512BW is required for the per-byte vpshufb. Without it, ShuffleNative - // falls back to a software cross-lane shuffle that is slower than the V256 - // path, so skip V512 entirely on Avx512F-only hosts. - if (Vector512.IsHardwareAccelerated && Avx512BW.IsSupported && pixels.Length >= 16) + if (Vector512.IsHardwareAccelerated && pixels.Length >= 16) { i = ApplyTransformVector512(scanline, pixels.Length); } @@ -102,8 +99,9 @@ internal static int ApplyTransformVector512(Span scanline, int pixelCount) ref byte scanlineRef = ref MemoryMarshal.GetReference(scanline); int i = 0; - // The mask only swaps bytes inside each 4-byte pixel, so it is correct - // for the per-lane Avx512BW.Shuffle that ShuffleNative selects here. + // Indices stay within their own 4-byte pixel, so the per-pixel pattern + // is also valid under the per-128-bit-lane vpshufb that ShuffleNative + // selects on AVX-512BW hosts. Vector512 shuffleMask = BgraToRgbaShuffle512; Vector512 zero = Vector512.Zero; From e6f09d66167acae42f180d566e21acac99fe0ffc Mon Sep 17 00:00:00 2001 From: Erik White <26148654+Erik-White@users.noreply.github.com> Date: Wed, 27 May 2026 15:00:07 +0200 Subject: [PATCH 05/11] Use constant or static values where possible --- .../Formats/Png/PngCgbiProcessor.cs | 45 +++++++++---------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs b/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs index 73af0253eb..749e4a6823 100644 --- a/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs +++ b/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs @@ -47,17 +47,17 @@ public static void ApplyTransform(Configuration configuration, Span scanli Span pixels = MemoryMarshal.Cast(scanline); int i = 0; - if (Vector512.IsHardwareAccelerated && pixels.Length >= 16) + if (Vector512.IsHardwareAccelerated && pixels.Length >= Vector512.Count) { i = ApplyTransformVector512(scanline, pixels.Length); } - if (Vector256.IsHardwareAccelerated && Avx2.IsSupported && (pixels.Length - i) >= 8) + if (Vector256.IsHardwareAccelerated && Avx2.IsSupported && (pixels.Length - i) >= Vector256.Count) { i = ApplyTransformVector256(scanline, i, pixels.Length); } - if (Vector128.IsHardwareAccelerated && (pixels.Length - i) >= 4) + if (Vector128.IsHardwareAccelerated && (pixels.Length - i) >= Vector128.Count) { i = ApplyTransformVector128(scanline, i, pixels.Length); } @@ -106,11 +106,9 @@ internal static int ApplyTransformVector512(Span scanline, int pixelCount) Vector512 zero = Vector512.Zero; Vector512 one = Vector512.One; - Vector512 byteMask = Vector512.Create(0xFF); - Vector512 opaque = Vector512.Create(0xFF); Vector512 byteMax = Vector512.Create((int)byte.MaxValue); - for (; i <= pixelCount - 16; i += 16) + for (; i <= pixelCount - Vector512.Count; i += Vector512.Count) { ref byte blockRef = ref Unsafe.Add(ref scanlineRef, i * Unsafe.SizeOf()); Vector512 bgra = Unsafe.ReadUnaligned>(ref blockRef); @@ -121,11 +119,11 @@ internal static int ApplyTransformVector512(Span scanline, int pixelCount) // Fully transparent and fully opaque pixels are identity cases for // unpremultiplication. Masking them keeps the scalar behavior and lets // safeAlpha avoid dividing by zero for alpha == 0. - Vector512 partialMask = ~(Vector512.Equals(alpha, zero) | Vector512.Equals(alpha, opaque)); + Vector512 partialMask = ~(Vector512.Equals(alpha, zero) | Vector512.Equals(alpha, byteMax)); - Vector512 r = packed & byteMask; - Vector512 g = Vector512.ShiftRightLogical(packed, 8) & byteMask; - Vector512 b = Vector512.ShiftRightLogical(packed, 16) & byteMask; + Vector512 r = packed & byteMax; + Vector512 g = Vector512.ShiftRightLogical(packed, 8) & byteMax; + Vector512 b = Vector512.ShiftRightLogical(packed, 16) & byteMax; Vector512 safeAlpha = Vector512.ConditionalSelect(partialMask, alpha, one); Vector512 halfAlpha = Vector512.ShiftRightLogical(safeAlpha, 1); @@ -180,11 +178,9 @@ internal static int ApplyTransformVector256(Span scanline, int startPixel, Vector256 zero = Vector256.Zero; Vector256 one = Vector256.One; - Vector256 byteMask = Vector256.Create(0xFF); - Vector256 opaque = Vector256.Create(0xFF); Vector256 byteMax = Vector256.Create((int)byte.MaxValue); - for (; i <= pixelCount - 8; i += 8) + for (; i <= pixelCount - Vector256.Count; i += Vector256.Count) { ref byte blockRef = ref Unsafe.Add(ref scanlineRef, i * Unsafe.SizeOf()); Vector256 bgra = Unsafe.ReadUnaligned>(ref blockRef); @@ -195,11 +191,11 @@ internal static int ApplyTransformVector256(Span scanline, int startPixel, // Fully transparent and fully opaque pixels are identity cases for // unpremultiplication. Masking them keeps the scalar behavior and lets // safeAlpha avoid dividing by zero for alpha == 0. - Vector256 partialMask = ~(Vector256.Equals(alpha, zero) | Vector256.Equals(alpha, opaque)); + Vector256 partialMask = ~(Vector256.Equals(alpha, zero) | Vector256.Equals(alpha, byteMax)); - Vector256 r = packed & byteMask; - Vector256 g = Vector256.ShiftRightLogical(packed, 8) & byteMask; - Vector256 b = Vector256.ShiftRightLogical(packed, 16) & byteMask; + Vector256 r = packed & byteMax; + Vector256 g = Vector256.ShiftRightLogical(packed, 8) & byteMax; + Vector256 b = Vector256.ShiftRightLogical(packed, 16) & byteMax; Vector256 safeAlpha = Vector256.ConditionalSelect(partialMask, alpha, one); Vector256 halfAlpha = Vector256.ShiftRightLogical(safeAlpha, 1); @@ -251,11 +247,9 @@ internal static int ApplyTransformVector128(Span scanline, int startPixel, Vector128 zero = Vector128.Zero; Vector128 one = Vector128.One; - Vector128 byteMask = Vector128.Create(0xFF); - Vector128 opaque = Vector128.Create(0xFF); Vector128 byteMax = Vector128.Create((int)byte.MaxValue); - for (; i <= pixelCount - 4; i += 4) + for (; i <= pixelCount - Vector128.Count; i += Vector128.Count) { ref byte blockRef = ref Unsafe.Add(ref scanlineRef, i * Unsafe.SizeOf()); Vector128 bgra = Unsafe.ReadUnaligned>(ref blockRef); @@ -266,11 +260,11 @@ internal static int ApplyTransformVector128(Span scanline, int startPixel, // Fully transparent and fully opaque pixels are identity cases for // unpremultiplication. Masking them keeps the scalar behavior and lets // safeAlpha avoid dividing by zero for alpha == 0. - Vector128 partialMask = ~(Vector128.Equals(alpha, zero) | Vector128.Equals(alpha, opaque)); + Vector128 partialMask = ~(Vector128.Equals(alpha, zero) | Vector128.Equals(alpha, byteMax)); - Vector128 r = packed & byteMask; - Vector128 g = Vector128.ShiftRightLogical(packed, 8) & byteMask; - Vector128 b = Vector128.ShiftRightLogical(packed, 16) & byteMask; + Vector128 r = packed & byteMax; + Vector128 g = Vector128.ShiftRightLogical(packed, 8) & byteMax; + Vector128 b = Vector128.ShiftRightLogical(packed, 16) & byteMax; Vector128 safeAlpha = Vector128.ConditionalSelect(partialMask, alpha, one); Vector128 halfAlpha = Vector128.ShiftRightLogical(safeAlpha, 1); @@ -315,9 +309,10 @@ internal static int ApplyTransformVector128(Span scanline, int startPixel, private static byte[] BuildShuffleBytes() { - byte[] bytes = new byte[64]; + byte[] bytes = new byte[Vector512.Count]; Span span = bytes; Shuffle.MMShuffleSpan(ref span, Shuffle.MMShuffle3012); + return bytes; } } From 49dc1951db9c6a8f14cae6985005e37588a16726 Mon Sep 17 00:00:00 2001 From: Erik White <26148654+Erik-White@users.noreply.github.com> Date: Wed, 27 May 2026 16:16:59 +0200 Subject: [PATCH 06/11] Fix read stream byte read semantics --- .../Compression/Zlib/ChunkedReadStream.cs | 60 +++++++++---------- 1 file changed, 27 insertions(+), 33 deletions(-) diff --git a/src/ImageSharp/Compression/Zlib/ChunkedReadStream.cs b/src/ImageSharp/Compression/Zlib/ChunkedReadStream.cs index 8c5189d4a9..b697327fff 100644 --- a/src/ImageSharp/Compression/Zlib/ChunkedReadStream.cs +++ b/src/ImageSharp/Compression/Zlib/ChunkedReadStream.cs @@ -57,57 +57,51 @@ public ChunkedReadStream(BufferedReadStream innerStream, Func getData) /// public override int ReadByte() - { - this.currentDataRemaining--; - return this.innerStream.ReadByte(); - } - - /// - public override int Read(byte[] buffer, int offset, int count) { if (this.currentDataRemaining is 0) { - // Current segment is exhausted; ask the caller for the next one. this.currentDataRemaining = this.getData(); - if (this.currentDataRemaining is 0) { - return 0; + return -1; } } - int bytesToRead = Math.Min(count, this.currentDataRemaining); - this.currentDataRemaining -= bytesToRead; - int totalBytesRead = this.innerStream.Read(buffer, offset, bytesToRead); - long innerStreamLength = this.innerStream.Length; - - // Keep reading data until we've reached the end of the stream or filled the buffer. - int bytesRead = 0; - offset += totalBytesRead; - while (this.currentDataRemaining is 0 && totalBytesRead < count) + int value = this.innerStream.ReadByte(); + if (value is not -1) { - this.currentDataRemaining = this.getData(); - - if (this.currentDataRemaining is 0) - { - return totalBytesRead; - } + this.currentDataRemaining--; + } - offset += bytesRead; + return value; + } - if (offset >= innerStreamLength || offset >= count) + /// + public override int Read(byte[] buffer, int offset, int count) + { + // Decrement currentDataRemaining only by bytes actually returned by + // innerStream.Read; a short read otherwise underflows the segment + // counter and triggers getData() before the segment is truly drained. + int totalBytesRead = 0; + while (totalBytesRead < count) + { + if (this.currentDataRemaining is 0) { - return totalBytesRead; + this.currentDataRemaining = this.getData(); + if (this.currentDataRemaining is 0) + { + break; + } } - bytesToRead = Math.Min(count - totalBytesRead, this.currentDataRemaining); - this.currentDataRemaining -= bytesToRead; - bytesRead = this.innerStream.Read(buffer, offset, bytesToRead); - if (bytesRead == 0) + int bytesToRead = Math.Min(count - totalBytesRead, this.currentDataRemaining); + int bytesRead = this.innerStream.Read(buffer, offset + totalBytesRead, bytesToRead); + if (bytesRead is 0) { - return totalBytesRead; + break; } + this.currentDataRemaining -= bytesRead; totalBytesRead += bytesRead; } From 3b37d5552d7ec554b7fbf76aec9f5fa0cac9e98e Mon Sep 17 00:00:00 2001 From: Erik White <26148654+Erik-White@users.noreply.github.com> Date: Wed, 27 May 2026 16:41:04 +0200 Subject: [PATCH 07/11] Remove redundant floor. Float to int conversion will already truncate toward zero --- .../Formats/Png/PngCgbiProcessor.cs | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs b/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs index 749e4a6823..9243896de7 100644 --- a/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs +++ b/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs @@ -129,20 +129,20 @@ internal static int ApplyTransformVector512(Span scanline, int pixelCount) Vector512 halfAlpha = Vector512.ShiftRightLogical(safeAlpha, 1); Vector512 safeAlphaF = Vector512.ConvertToSingle(safeAlpha); - // The scalar path computes ((c * 255) + (a >> 1)) / a with integer - // division. Floor the positive quotient before converting so SIMD does - // not use the default round-to-nearest conversion and drift by one. + // ConvertToInt32 truncates toward zero (cvttps2dq / fcvtzs); since + // every quotient here is non-negative, that matches the scalar + // ((c * 255) + (a >> 1)) / a integer-division floor. Vector512 unpremultipliedR = Vector512.Min( byteMax, - Vector512.ConvertToInt32(Vector512.Floor(Vector512.ConvertToSingle((r * byteMax) + halfAlpha) / safeAlphaF))); + Vector512.ConvertToInt32(Vector512.ConvertToSingle((r * byteMax) + halfAlpha) / safeAlphaF)); Vector512 unpremultipliedG = Vector512.Min( byteMax, - Vector512.ConvertToInt32(Vector512.Floor(Vector512.ConvertToSingle((g * byteMax) + halfAlpha) / safeAlphaF))); + Vector512.ConvertToInt32(Vector512.ConvertToSingle((g * byteMax) + halfAlpha) / safeAlphaF)); Vector512 unpremultipliedB = Vector512.Min( byteMax, - Vector512.ConvertToInt32(Vector512.Floor(Vector512.ConvertToSingle((b * byteMax) + halfAlpha) / safeAlphaF))); + Vector512.ConvertToInt32(Vector512.ConvertToSingle((b * byteMax) + halfAlpha) / safeAlphaF)); // ConditionalSelect applies the expensive unpremultiply only to pixels // where alpha is between 1 and 254; alpha 0 and 255 lanes keep the @@ -201,20 +201,20 @@ internal static int ApplyTransformVector256(Span scanline, int startPixel, Vector256 halfAlpha = Vector256.ShiftRightLogical(safeAlpha, 1); Vector256 safeAlphaF = Vector256.ConvertToSingle(safeAlpha); - // The scalar path computes ((c * 255) + (a >> 1)) / a with integer - // division. Floor the positive quotient before converting so SIMD does - // not use the default round-to-nearest conversion and drift by one. + // ConvertToInt32 truncates toward zero (cvttps2dq / fcvtzs); since + // every quotient here is non-negative, that matches the scalar + // ((c * 255) + (a >> 1)) / a integer-division floor. Vector256 unpremultipliedR = Vector256.Min( byteMax, - Vector256.ConvertToInt32(Vector256.Floor(Vector256.ConvertToSingle((r * byteMax) + halfAlpha) / safeAlphaF))); + Vector256.ConvertToInt32(Vector256.ConvertToSingle((r * byteMax) + halfAlpha) / safeAlphaF)); Vector256 unpremultipliedG = Vector256.Min( byteMax, - Vector256.ConvertToInt32(Vector256.Floor(Vector256.ConvertToSingle((g * byteMax) + halfAlpha) / safeAlphaF))); + Vector256.ConvertToInt32(Vector256.ConvertToSingle((g * byteMax) + halfAlpha) / safeAlphaF)); Vector256 unpremultipliedB = Vector256.Min( byteMax, - Vector256.ConvertToInt32(Vector256.Floor(Vector256.ConvertToSingle((b * byteMax) + halfAlpha) / safeAlphaF))); + Vector256.ConvertToInt32(Vector256.ConvertToSingle((b * byteMax) + halfAlpha) / safeAlphaF)); // ConditionalSelect applies the expensive unpremultiply only to pixels // where alpha is between 1 and 254; alpha 0 and 255 lanes keep the @@ -270,20 +270,20 @@ internal static int ApplyTransformVector128(Span scanline, int startPixel, Vector128 halfAlpha = Vector128.ShiftRightLogical(safeAlpha, 1); Vector128 safeAlphaF = Vector128.ConvertToSingle(safeAlpha); - // The scalar path computes ((c * 255) + (a >> 1)) / a with integer - // division. Floor the positive quotient before converting so SIMD does - // not use the default round-to-nearest conversion and drift by one. + // ConvertToInt32 truncates toward zero (cvttps2dq / fcvtzs); since + // every quotient here is non-negative, that matches the scalar + // ((c * 255) + (a >> 1)) / a integer-division floor. Vector128 unpremultipliedR = Vector128.Min( byteMax, - Vector128.ConvertToInt32(Vector128.Floor(Vector128.ConvertToSingle((r * byteMax) + halfAlpha) / safeAlphaF))); + Vector128.ConvertToInt32(Vector128.ConvertToSingle((r * byteMax) + halfAlpha) / safeAlphaF)); Vector128 unpremultipliedG = Vector128.Min( byteMax, - Vector128.ConvertToInt32(Vector128.Floor(Vector128.ConvertToSingle((g * byteMax) + halfAlpha) / safeAlphaF))); + Vector128.ConvertToInt32(Vector128.ConvertToSingle((g * byteMax) + halfAlpha) / safeAlphaF)); Vector128 unpremultipliedB = Vector128.Min( byteMax, - Vector128.ConvertToInt32(Vector128.Floor(Vector128.ConvertToSingle((b * byteMax) + halfAlpha) / safeAlphaF))); + Vector128.ConvertToInt32(Vector128.ConvertToSingle((b * byteMax) + halfAlpha) / safeAlphaF)); // ConditionalSelect applies the expensive unpremultiply only to pixels // where alpha is between 1 and 254; alpha 0 and 255 lanes keep the From 47678c748df268ceebd07fae8364af046047ec69 Mon Sep 17 00:00:00 2001 From: Erik White <26148654+Erik-White@users.noreply.github.com> Date: Wed, 27 May 2026 16:47:43 +0200 Subject: [PATCH 08/11] Use single buffer for shuffle bytes --- src/ImageSharp/Formats/Png/PngCgbiProcessor.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs b/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs index 9243896de7..e4847ad67f 100644 --- a/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs +++ b/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs @@ -26,13 +26,14 @@ internal static class PngCgbiProcessor { // Per-pixel byte indices that swap CgBI's BGRA layout to Rgba32's RGBA. // MMShuffle3012 expands to [2, 1, 0, 3] per 4-byte pixel; the same 64-byte - // sequence seeds all three shuffle masks (V128/V256 ctors take the leading - // 16/32 bytes). - private static readonly Vector128 BgraToRgbaShuffle128 = Vector128.Create(BuildShuffleBytes()); + // sequence seeds all three shuffle masks (Vector128/256 take a leading slice). + private static readonly byte[] BgraToRgbaShuffleBytes = BuildShuffleBytes(); - private static readonly Vector256 BgraToRgbaShuffle256 = Vector256.Create(BuildShuffleBytes()); + private static readonly Vector128 BgraToRgbaShuffle128 = Vector128.Create(new ReadOnlySpan(BgraToRgbaShuffleBytes, 0, Vector128.Count)); - private static readonly Vector512 BgraToRgbaShuffle512 = Vector512.Create(BuildShuffleBytes()); + private static readonly Vector256 BgraToRgbaShuffle256 = Vector256.Create(new ReadOnlySpan(BgraToRgbaShuffleBytes, 0, Vector256.Count)); + + private static readonly Vector512 BgraToRgbaShuffle512 = Vector512.Create(BgraToRgbaShuffleBytes); /// /// Applies the inverse of Apple's CgBI pixel mangling to a defiltered scanline in place. From fb207199f5b3db589f8a2013caeb72f238e6c8e7 Mon Sep 17 00:00:00 2001 From: Erik White <26148654+Erik-White@users.noreply.github.com> Date: Thu, 11 Jun 2026 16:38:29 +0200 Subject: [PATCH 09/11] Clean up/clarify test helper method --- .../Formats/Png/PngCgbiProcessorTests.cs | 38 +++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Png/PngCgbiProcessorTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngCgbiProcessorTests.cs index 426afb6d42..cd03c5e488 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngCgbiProcessorTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngCgbiProcessorTests.cs @@ -120,34 +120,42 @@ private static void AssertVectorMatchesScalar(int pixelCount, Func Assert.True(vectorTail.SequenceEqual(inputTail)); } + /// + /// Builds synthetic input for tests. Produces inputs that exercise all three alpha cases. + /// + /// Channel values laid out as a defiltered CgBI scanline in premultiplied BGRA order private static byte[] CreateBgraScanline(int pixelCount) { - // Deterministic mix of edge cases (a=0, a=255, partial alpha) and varied channels. + // The distinct strides keep the channels from being equal to each other or constant across pixels + // So a bug that e.g. swaps two channels or reuses one channel's value doesn't accidentally pass + const int alphaCaseCount = 7; + const int redStride = 13; + const int greenStride = 29; + const int blueStride = 53; + byte[] bytes = new byte[pixelCount * 4]; for (int p = 0; p < pixelCount; p++) { - byte a = (p % 7) switch + // Cycling alpha through [0..255], and an odd partial value every pixel rotation + // ensures all three branches get covered within any scanline of 3 or more pixels + byte a = (p % alphaCaseCount) switch { 0 => byte.MinValue, 1 => byte.MaxValue, - _ => (byte)((((p * 37) + 23) & 0xFF) | 1) // never zero + _ => (byte)(((p * 37) + 23) | 1) // Produce a spread of alpha values and make sure to never get 0 }; - // CgBI premultiplied BGRA: c' = c * a / 255 - byte r = (byte)((p * 13) & 0xFF); - byte g = (byte)((p * 29) & 0xFF); - byte b = (byte)((p * 53) & 0xFF); - r = (byte)((r * a) / byte.MaxValue); - g = (byte)((g * a) / byte.MaxValue); - b = (byte)((b * a) / byte.MaxValue); - - bytes[(p * 4) + 0] = b; - bytes[(p * 4) + 1] = g; - bytes[(p * 4) + 2] = r; - bytes[(p * 4) + 3] = a; + int offset = p * 4; + bytes[offset + 0] = Premultiply((byte)(p * blueStride), a); + bytes[offset + 1] = Premultiply((byte)(p * greenStride), a); + bytes[offset + 2] = Premultiply((byte)(p * redStride), a); + bytes[offset + 3] = a; } return bytes; + + // CgBI stores channels premultiplied by alpha + static byte Premultiply(byte channel, byte alpha) => (byte)(channel * alpha / byte.MaxValue); } private static void ApplyCgbiTransformScalarReference(Span scanline) From 5472d6918a94771bdc03cfbcfd4579938ba5c46b Mon Sep 17 00:00:00 2001 From: Erik White <26148654+Erik-White@users.noreply.github.com> Date: Fri, 12 Jun 2026 10:56:08 +0200 Subject: [PATCH 10/11] Use FeatureTestRunner for all instruction sets --- .../Formats/Png/PngCgbiProcessor.cs | 6 +- .../Formats/Png/PngCgbiProcessorTests.cs | 93 ++----------------- 2 files changed, 11 insertions(+), 88 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs b/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs index e4847ad67f..3addbb3c05 100644 --- a/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs +++ b/src/ImageSharp/Formats/Png/PngCgbiProcessor.cs @@ -95,7 +95,7 @@ private static void UndoPremultiplicationScalar(ref Rgba32 pixel) pixel = new Rgba32(r, g, b, a); } - internal static int ApplyTransformVector512(Span scanline, int pixelCount) + private static int ApplyTransformVector512(Span scanline, int pixelCount) { ref byte scanlineRef = ref MemoryMarshal.GetReference(scanline); int i = 0; @@ -167,7 +167,7 @@ internal static int ApplyTransformVector512(Span scanline, int pixelCount) return i; } - internal static int ApplyTransformVector256(Span scanline, int startPixel, int pixelCount) + private static int ApplyTransformVector256(Span scanline, int startPixel, int pixelCount) { ref byte scanlineRef = ref MemoryMarshal.GetReference(scanline); int i = startPixel; @@ -239,7 +239,7 @@ internal static int ApplyTransformVector256(Span scanline, int startPixel, return i; } - internal static int ApplyTransformVector128(Span scanline, int startPixel, int pixelCount) + private static int ApplyTransformVector128(Span scanline, int startPixel, int pixelCount) { ref byte scanlineRef = ref MemoryMarshal.GetReference(scanline); int i = startPixel; diff --git a/tests/ImageSharp.Tests/Formats/Png/PngCgbiProcessorTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngCgbiProcessorTests.cs index cd03c5e488..863c7fc4fc 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngCgbiProcessorTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngCgbiProcessorTests.cs @@ -4,6 +4,7 @@ using System.Runtime.InteropServices; using SixLabors.ImageSharp.Formats.Png; using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Tests.TestUtilities; namespace SixLabors.ImageSharp.Tests.Formats.Png; @@ -24,10 +25,14 @@ public class PngCgbiProcessorTests [InlineData(32)] [InlineData(33)] [InlineData(64)] - public void ApplyTransform_RgbWithAlpha_MatchesScalar(int pixelCount) + public void ApplyTransform_RgbWithAlpha_MatchesScalar(int pixelCount) => + FeatureTestRunner.RunWithHwIntrinsicsFeature( + serialized => AssertApplyTransformMatchesScalar(FeatureTestRunner.Deserialize(serialized)), + pixelCount, + HwIntrinsics.AllowAll | HwIntrinsics.DisableAVX512F | HwIntrinsics.DisableAVX | HwIntrinsics.DisableHWIntrinsic); + + private static void AssertApplyTransformMatchesScalar(int pixelCount) { - // Drives the full V512/V256/V128/scalar dispatch, so it covers each - // path that is hardware-accelerated on the host plus the scalar tail. byte[] input = CreateBgraScanline(pixelCount); byte[] processorOutput = (byte[])input.Clone(); byte[] scalarOutput = (byte[])input.Clone(); @@ -38,88 +43,6 @@ public void ApplyTransform_RgbWithAlpha_MatchesScalar(int pixelCount) Assert.Equal(scalarOutput, processorOutput); } - [Theory] - [InlineData(0)] - [InlineData(1)] - [InlineData(3)] - [InlineData(4)] - [InlineData(7)] - [InlineData(8)] - [InlineData(15)] - [InlineData(16)] - [InlineData(17)] - [InlineData(31)] - [InlineData(32)] - [InlineData(33)] - [InlineData(64)] - public void ApplyTransformVector512_MatchesScalar(int pixelCount) => - // Vector512 uses Vector512_.ShuffleNative which falls back to the software - // Vector512.Shuffle when Avx512BW is unavailable, so the body runs regardless - // of whether Vector512 is hardware-accelerated on the host. - AssertVectorMatchesScalar( - pixelCount, - scanline => PngCgbiProcessor.ApplyTransformVector512(scanline, scanline.Length / 4), - blockSize: 16); - - [Theory] - [InlineData(0)] - [InlineData(1)] - [InlineData(3)] - [InlineData(4)] - [InlineData(7)] - [InlineData(8)] - [InlineData(15)] - [InlineData(16)] - [InlineData(17)] - [InlineData(31)] - [InlineData(32)] - [InlineData(64)] - public void ApplyTransformVector256_MatchesScalar(int pixelCount) => AssertVectorMatchesScalar( - pixelCount, - scanline => PngCgbiProcessor.ApplyTransformVector256(scanline, 0, scanline.Length / 4), - blockSize: 8); - - [Theory] - [InlineData(0)] - [InlineData(1)] - [InlineData(3)] - [InlineData(4)] - [InlineData(7)] - [InlineData(8)] - [InlineData(15)] - [InlineData(16)] - [InlineData(64)] - public void ApplyTransformVector128_MatchesScalar(int pixelCount) => AssertVectorMatchesScalar( - pixelCount, - scanline => PngCgbiProcessor.ApplyTransformVector128(scanline, 0, scanline.Length / 4), - blockSize: 4); - - private static void AssertVectorMatchesScalar(int pixelCount, Func applyVector, int blockSize) - { - byte[] input = CreateBgraScanline(pixelCount); - byte[] vectorOutput = (byte[])input.Clone(); - byte[] scalarOutput = (byte[])input.Clone(); - - int processed = applyVector(vectorOutput); - - int expectedProcessed = (pixelCount / blockSize) * blockSize; - Assert.Equal(expectedProcessed, processed); - - // The vector path is responsible for whole blocks only; remaining pixels are - // handled by the scalar tail in ApplyTransform. Run the scalar reference - // over every pixel and compare the prefix the vector path actually wrote. - ApplyCgbiTransformScalarReference(scalarOutput); - - Span vectorProcessed = vectorOutput.AsSpan(0, processed * 4); - Span scalarProcessed = scalarOutput.AsSpan(0, processed * 4); - Assert.True(vectorProcessed.SequenceEqual(scalarProcessed), $"Mismatch at pixelCount={pixelCount}"); - - // Pixels past the vector's processed prefix must be untouched. - Span vectorTail = vectorOutput.AsSpan(processed * 4); - Span inputTail = input.AsSpan(processed * 4); - Assert.True(vectorTail.SequenceEqual(inputTail)); - } - /// /// Builds synthetic input for tests. Produces inputs that exercise all three alpha cases. /// From dfdc0734e34899695c170db89ebc2d522dac455c Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 23 Jul 2026 19:10:07 +1000 Subject: [PATCH 11/11] Validate CgBI chunk order in PNG decoder Handle CgBI chunks consistently when they appear after IHDR by routing both identify/decode paths through a new `ReadCgbiChunk()` method that immediately validates already-read headers. This prevents invalid CgBI images from bypassing compatibility checks when chunk order is unusual. Added PNG tests that reorder the first two chunks to confirm incompatible CgBI headers now throw in both identify and decode flows. Also renames `ZlibInflateStream` to `ZlibInflateReader` and updates EXR/PNG/TIFF call sites for clearer intent. --- ...bInflateStream.cs => ZlibInflateReader.cs} | 6 +- .../Exr/Compression/ExrBaseDecompressor.cs | 2 +- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 23 ++++++-- .../Decompressors/DeflateTiffCompression.cs | 2 +- .../Formats/Png/PngDecoderTests.cs | 55 +++++++++++++++++++ 5 files changed, 79 insertions(+), 9 deletions(-) rename src/ImageSharp/Compression/Zlib/{ZlibInflateStream.cs => ZlibInflateReader.cs} (95%) diff --git a/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs b/src/ImageSharp/Compression/Zlib/ZlibInflateReader.cs similarity index 95% rename from src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs rename to src/ImageSharp/Compression/Zlib/ZlibInflateReader.cs index 11f34dac8a..de69717da4 100644 --- a/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs +++ b/src/ImageSharp/Compression/Zlib/ZlibInflateReader.cs @@ -12,7 +12,7 @@ namespace SixLabors.ImageSharp.Compression.Zlib; /// over the remaining DEFLATE payload. The /// Adler-32 trailer is not validated. /// -internal sealed class ZlibInflateStream : IDisposable +internal sealed class ZlibInflateReader : IDisposable { /// /// Used to read the Adler-32 and Crc-32 checksums. @@ -23,10 +23,10 @@ internal sealed class ZlibInflateStream : IDisposable private readonly ChunkedReadStream segmentStream; - public ZlibInflateStream(BufferedReadStream innerStream) + public ZlibInflateReader(BufferedReadStream innerStream) => this.segmentStream = new ChunkedReadStream(innerStream); - public ZlibInflateStream(BufferedReadStream innerStream, Func getData) + public ZlibInflateReader(BufferedReadStream innerStream, Func getData) => this.segmentStream = new ChunkedReadStream(innerStream, getData); /// diff --git a/src/ImageSharp/Formats/Exr/Compression/ExrBaseDecompressor.cs b/src/ImageSharp/Formats/Exr/Compression/ExrBaseDecompressor.cs index 1b2d95ba53..f598955f43 100644 --- a/src/ImageSharp/Formats/Exr/Compression/ExrBaseDecompressor.cs +++ b/src/ImageSharp/Formats/Exr/Compression/ExrBaseDecompressor.cs @@ -45,7 +45,7 @@ protected ExrBaseDecompressor(MemoryAllocator allocator, uint bytesPerBlock, uin protected static int UndoZipCompression(BufferedReadStream stream, uint compressedBytes, Span uncompressed, uint uncompressedBytes) { long pos = stream.Position; - using ZlibInflateStream inflateStream = new( + using ZlibInflateReader inflateStream = new( stream, () => { diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index f3e2bbdbe0..28f9a989b9 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -321,7 +321,7 @@ protected override Image Decode(BufferedReadStream stream, Cance case PngChunkType.End: goto EOF; case PngChunkType.ProprietaryApple: - this.isCgbi = true; + this.ReadCgbiChunk(); break; } } @@ -525,7 +525,7 @@ protected override ImageInfo Identify(BufferedReadStream stream, CancellationTok goto EOF; case PngChunkType.ProprietaryApple: - this.isCgbi = true; + this.ReadCgbiChunk(); break; default: @@ -788,7 +788,7 @@ private void ReadScanlines( return; } - using ZlibInflateStream inflateStream = new(this.currentStream, getData); + using ZlibInflateReader inflateStream = new(this.currentStream, getData); if (!inflateStream.AllocateNewBytes(chunkLength, !this.hasImageData)) { return; @@ -1458,6 +1458,21 @@ private void ReadHeaderChunk(PngMetadata pngMetadata, ReadOnlySpan data) } } + /// + /// Marks the image as CgBI and validates a header that has already been read. + /// + private void ReadCgbiChunk() + { + this.isCgbi = true; + + // Although Apple's pngcrush normally writes CgBI before IHDR, accepting the + // reverse order must apply the same compatibility validation. + if (!Equals(this.header, default(PngHeader))) + { + ThrowIfInvalidCgbiContent(this.header); + } + } + private static void ThrowIfInvalidCgbiContent(in PngHeader header) { if (header.BitDepth != 8 || (header.ColorType is not PngColorType.Rgb and not PngColorType.RgbWithAlpha)) @@ -1952,7 +1967,7 @@ private unsafe bool TryDecompressZlibData(ReadOnlySpan compressedData, int using MemoryStream memoryStreamOutput = new(compressedData.Length); using UnmanagedMemoryStream memoryStreamInput = new(compressedDataBase, compressedData.Length); using BufferedReadStream bufferedStream = new(this.configuration, memoryStreamInput); - using ZlibInflateStream inflateStream = new(bufferedStream); + using ZlibInflateReader inflateStream = new(bufferedStream); Span destUncompressedData = destBuffer.GetSpan(); if (!inflateStream.AllocateNewBytes(compressedData.Length, false)) diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/DeflateTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/DeflateTiffCompression.cs index 4e176f28d7..d3b65c537c 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/DeflateTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/DeflateTiffCompression.cs @@ -54,7 +54,7 @@ public DeflateTiffCompression(MemoryAllocator memoryAllocator, int width, int bi protected override void Decompress(BufferedReadStream stream, int byteCount, int stripHeight, Span buffer, CancellationToken cancellationToken) { long pos = stream.Position; - using (ZlibInflateStream deframeStream = new( + using (ZlibInflateReader deframeStream = new( stream, () => { diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 2fbbe695e9..88ed512d12 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.Buffers.Binary; using System.Runtime.Intrinsics.X86; using Microsoft.DotNet.RemoteExecutor; using SixLabors.ImageSharp.Formats; @@ -789,6 +790,60 @@ public void Decode_CgBI_IncompatibleHeader_ThrowsInvalidImageContentException(() => Image.Identify(stream)); + Assert.Contains("CgBI is only supported for 8-bit truecolor images", ex.Message); + } + + [Theory] + [InlineData(TestImages.Png.Cgbi.BitDepth16)] + [InlineData(TestImages.Png.Cgbi.Palette)] + public void Decode_CgBI_AfterHeader_IncompatibleHeader_ThrowsInvalidImageContentException(string imagePath) + { + TestFile testFile = TestFile.Create(imagePath); + byte[] reordered = MoveFirstPngChunkAfterSecond(testFile.Bytes); + using MemoryStream stream = new(reordered, false); + + InvalidImageContentException ex = Assert.Throws( + () => { using Image image = PngDecoder.Instance.Decode(DecoderOptions.Default, stream); }); + Assert.Contains("CgBI is only supported for 8-bit truecolor images", ex.Message); + } + + /// + /// Moves the first PNG chunk after the second while preserving each chunk's data and CRC. + /// + /// A PNG whose first two chunks should be exchanged. + /// A copy of the PNG with its first two chunks exchanged. + private static byte[] MoveFirstPngChunkAfterSecond(byte[] source) + { + const int signatureLength = 8; + const int chunkOverheadLength = 12; + + int firstChunkLength = BinaryPrimitives.ReadInt32BigEndian(source.AsSpan(signatureLength, 4)) + chunkOverheadLength; + int secondChunkOffset = signatureLength + firstChunkLength; + int secondChunkLength = BinaryPrimitives.ReadInt32BigEndian(source.AsSpan(secondChunkOffset, 4)) + chunkOverheadLength; + byte[] reordered = new byte[source.Length]; + + source.AsSpan(0, signatureLength).CopyTo(reordered); + source.AsSpan(secondChunkOffset, secondChunkLength).CopyTo(reordered.AsSpan(signatureLength)); + source.AsSpan(signatureLength, firstChunkLength).CopyTo(reordered.AsSpan(signatureLength + secondChunkLength)); + + // Chunk CRCs cover only each chunk's type and data, so moving complete chunks + // leaves both checksums valid and isolates ordering as the tested behavior. + source.AsSpan(secondChunkOffset + secondChunkLength) + .CopyTo(reordered.AsSpan(signatureLength + secondChunkLength + firstChunkLength)); + + return reordered; + } + [Theory] [WithFile(TestImages.Png.Splash, PixelTypes.Rgba32)] [WithFile(TestImages.Png.Bike, PixelTypes.Rgba32)]