From 51cd84eb2f2354544eacdc8a2db3748daebe2fa2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 18 Dec 2023 09:05:53 +0000 Subject: [PATCH 01/37] Bump actions/upload-artifact from 3 to 4 Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 3 to 4. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](https://github.com/actions/upload-artifact/compare/v3...v4) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/build-and-test.yml | 2 +- .github/workflows/code-coverage.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 75bcb8a25..51765be6e 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -132,7 +132,7 @@ jobs: XUNIT_PATH: .\tests\ImageSharp.Tests # Required for xunit - name: Export Failed Output - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 if: failure() with: name: actual_output_${{ runner.os }}_${{ matrix.options.framework }}${{ matrix.options.runtime }}.zip diff --git a/.github/workflows/code-coverage.yml b/.github/workflows/code-coverage.yml index 62b6477ee..53eef4504 100644 --- a/.github/workflows/code-coverage.yml +++ b/.github/workflows/code-coverage.yml @@ -74,7 +74,7 @@ jobs: XUNIT_PATH: .\tests\ImageSharp.Tests # Required for xunit - name: Export Failed Output - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 if: failure() with: name: actual_output_${{ runner.os }}_${{ matrix.options.framework }}${{ matrix.options.runtime }}.zip From 6570fc6c7446d594e20bc8c85e1404d48fc59862 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 1 Jan 2024 13:22:33 +1000 Subject: [PATCH 02/37] Update WebpFrameMetadata.cs --- src/ImageSharp/Formats/Webp/WebpFrameMetadata.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Webp/WebpFrameMetadata.cs b/src/ImageSharp/Formats/Webp/WebpFrameMetadata.cs index 422ad6bc7..cd1b5d590 100644 --- a/src/ImageSharp/Formats/Webp/WebpFrameMetadata.cs +++ b/src/ImageSharp/Formats/Webp/WebpFrameMetadata.cs @@ -48,7 +48,7 @@ public class WebpFrameMetadata : IDeepCloneable internal static WebpFrameMetadata FromAnimatedMetadata(AnimatedImageFrameMetadata metadata) => new() { - FrameDelay = (uint)metadata.Duration.Milliseconds, + FrameDelay = (uint)metadata.Duration.TotalMilliseconds, BlendMethod = metadata.BlendMode == FrameBlendMode.Source ? WebpBlendMethod.Source : WebpBlendMethod.Over, DisposalMethod = metadata.DisposalMode == FrameDisposalMode.RestoreToBackground ? WebpDisposalMethod.RestoreToBackground : WebpDisposalMethod.DoNotDispose }; From bb9ed6533370ed83ee7adc7d507d5b1f3929fa89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Mutnia=C5=84ski?= Date: Sun, 7 Jan 2024 15:49:34 +0100 Subject: [PATCH 03/37] add jpeg com marker support --- ImageSharp.sln | 1 + .../Formats/Jpeg/JpegDecoderCore.cs | 20 ++++++++- .../Formats/Jpeg/JpegEncoderCore.cs | 45 +++++++++++++++++++ src/ImageSharp/Formats/Jpeg/JpegMetadata.cs | 7 +++ .../Formats/Jpeg/MetadataExtensions.cs | 21 +++++++++ .../Formats/Jpg/JpegDecoderTests.cs | 15 +++++++ .../Formats/Jpg/JpegEncoderTests.Metadata.cs | 44 ++++++++++++++++++ .../Formats/Jpg/JpegMetadataTests.cs | 24 +++++++++- tests/ImageSharp.Tests/TestImages.cs | 1 + .../Input/Jpg/issues/issue-2067-comment.jpg | 3 ++ 10 files changed, 179 insertions(+), 2 deletions(-) create mode 100644 tests/Images/Input/Jpg/issues/issue-2067-comment.jpg diff --git a/ImageSharp.sln b/ImageSharp.sln index 2967acb8f..82eeefcde 100644 --- a/ImageSharp.sln +++ b/ImageSharp.sln @@ -237,6 +237,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "issues", "issues", "{5C9B68 tests\Images\Input\Jpg\issues\issue750-exif-tranform.jpg = tests\Images\Input\Jpg\issues\issue750-exif-tranform.jpg tests\Images\Input\Jpg\issues\Issue845-Incorrect-Quality99.jpg = tests\Images\Input\Jpg\issues\Issue845-Incorrect-Quality99.jpg tests\Images\Input\Jpg\issues\issue855-incorrect-colorspace.jpg = tests\Images\Input\Jpg\issues\issue855-incorrect-colorspace.jpg + tests\Images\Input\Jpg\issues\issue-2067-comment.jpg = tests\Images\Input\Jpg\issues\issue-2067-comment.jpg EndProjectSection EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "fuzz", "fuzz", "{516A3532-6AC2-417B-AD79-9BD5D0D378A0}" diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index ccace190f..e11923ac8 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -6,6 +6,7 @@ using System.Buffers; using System.Buffers.Binary; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; +using System.Text; using SixLabors.ImageSharp.Common.Helpers; using SixLabors.ImageSharp.Formats.Jpeg.Components; using SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder; @@ -481,7 +482,7 @@ internal sealed class JpegDecoderCore : IRawJpegData, IImageDecoderInternals case JpegConstants.Markers.APP15: case JpegConstants.Markers.COM: - stream.Skip(markerContentByteSize); + this.ProcessComMarker(stream, markerContentByteSize); break; case JpegConstants.Markers.DAC: @@ -515,6 +516,23 @@ internal sealed class JpegDecoderCore : IRawJpegData, IImageDecoderInternals this.scanDecoder = null; } + /// + /// Assigns COM marker bytes to comment property + /// + /// The input stream. + /// The remaining bytes in the segment block. + private void ProcessComMarker(BufferedReadStream stream, int markerContentByteSize) + { + Span temp = stackalloc byte[markerContentByteSize]; + char[] chars = new char[markerContentByteSize]; + JpegMetadata metadata = this.Metadata.GetFormatMetadata(JpegFormat.Instance); + + stream.Read(temp); + Encoding.ASCII.GetChars(temp, chars); + + metadata.Comments.Add(chars); + } + /// /// Returns encoded colorspace based on the adobe APP14 marker. /// diff --git a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs index 7fc2a1f45..cc6042b6e 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs @@ -3,6 +3,7 @@ #nullable disable using System.Buffers.Binary; +using System.Text; using SixLabors.ImageSharp.Common.Helpers; using SixLabors.ImageSharp.Formats.Jpeg.Components; using SixLabors.ImageSharp.Formats.Jpeg.Components.Encoder; @@ -89,6 +90,9 @@ internal sealed unsafe partial class JpegEncoderCore : IImageEncoderInternals // Write Exif, XMP, ICC and IPTC profiles this.WriteProfiles(metadata, buffer); + // Write comments + this.WriteComment(jpegMetadata); + // Write the image dimensions. this.WriteStartOfFrame(image.Width, image.Height, frameConfig, buffer); @@ -167,6 +171,47 @@ internal sealed unsafe partial class JpegEncoderCore : IImageEncoderInternals this.outputStream.Write(buffer, 0, 18); } + /// + /// Writes comment + /// + /// The image metadata. + private void WriteComment(JpegMetadata metadata) + { + if (metadata.Comments is { Count: 0 }) + { + return; + } + + // Length (comment strings lengths) + (comments markers with payload sizes) + int commentsBytes = metadata.Comments.Sum(x => x.Length) + (metadata.Comments.Count * 4); + int commentStart = 0; + Span commentBuffer = stackalloc byte[commentsBytes]; + + foreach (Memory comment in metadata.Comments) + { + int totalComLength = comment.Length + 4; + + Span commentData = commentBuffer.Slice(commentStart, totalComLength); + Span markers = commentData.Slice(0, 2); + Span payloadSize = commentData.Slice(2, 2); + Span payload = commentData.Slice(4, comment.Length); + + // Beginning of comment ff fe + markers[0] = JpegConstants.Markers.XFF; + markers[1] = JpegConstants.Markers.COM; + + // Write payload size + BinaryPrimitives.WriteInt16BigEndian(payloadSize, (short)(commentData.Length - 2)); + + Encoding.ASCII.GetBytes(comment.Span, payload); + + // Indicate begin of next comment in buffer + commentStart += totalComLength; + } + + this.outputStream.Write(commentBuffer, 0, commentBuffer.Length); + } + /// /// Writes the Define Huffman Table marker and tables. /// diff --git a/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs b/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs index 59fc2f9cb..61fe3b214 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs @@ -15,6 +15,7 @@ public class JpegMetadata : IDeepCloneable /// public JpegMetadata() { + this.Comments = new List>(); } /// @@ -25,6 +26,7 @@ public class JpegMetadata : IDeepCloneable { this.ColorType = other.ColorType; + this.Comments = other.Comments; this.LuminanceQuality = other.LuminanceQuality; this.ChrominanceQuality = other.ChrominanceQuality; } @@ -101,6 +103,11 @@ public class JpegMetadata : IDeepCloneable /// public bool? Progressive { get; internal set; } + /// + /// Gets the comments. + /// + public ICollection>? Comments { get; } + /// public IDeepCloneable DeepClone() => new JpegMetadata(this); } diff --git a/src/ImageSharp/Formats/Jpeg/MetadataExtensions.cs b/src/ImageSharp/Formats/Jpeg/MetadataExtensions.cs index 753dfdb60..53efc7d0c 100644 --- a/src/ImageSharp/Formats/Jpeg/MetadataExtensions.cs +++ b/src/ImageSharp/Formats/Jpeg/MetadataExtensions.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.Text; using SixLabors.ImageSharp.Formats.Jpeg; using SixLabors.ImageSharp.Metadata; @@ -17,4 +18,24 @@ public static partial class MetadataExtensions /// The metadata this method extends. /// The . public static JpegMetadata GetJpegMetadata(this ImageMetadata metadata) => metadata.GetFormatMetadata(JpegFormat.Instance); + + /// + /// Saves the comment into + /// + /// The metadata this method extends. + /// The comment string. + public static void SaveComment(this JpegMetadata metadata, string comment) + { + ASCIIEncoding encoding = new(); + + byte[] bytes = encoding.GetBytes(comment); + metadata.Comments?.Add(encoding.GetChars(bytes)); + } + + /// + /// Gets the comments from + /// + /// The metadata this method extends. + /// The IEnumerable string of comments. + public static IEnumerable? GetComments(this JpegMetadata metadata) => metadata.Comments?.Select(x => x.ToString()); } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index c8d93f6e9..b219e715f 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -364,4 +364,19 @@ public partial class JpegDecoderTests image.DebugSave(provider); image.CompareToOriginal(provider); } + + [Theory] + [WithFile(TestImages.Jpeg.Issues.Issue2067_CommentMarker, PixelTypes.Rgba32)] + public void JpegDecoder_DecodeMetadataComment(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + string expectedComment = "TEST COMMENT"; + using Image image = provider.GetImage(JpegDecoder.Instance); + JpegMetadata metadata = image.Metadata.GetJpegMetadata(); + + Assert.Equal(1, metadata.Comments?.Count); + Assert.Equal(expectedComment, metadata.GetComments()?.FirstOrDefault()); + image.DebugSave(provider); + image.CompareToOriginal(provider); + } } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs index 2b721b9b5..50f47a134 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs @@ -154,6 +154,50 @@ public partial class JpegEncoderTests } } + [Theory] + [WithFile(TestImages.Jpeg.Issues.Issue2067_CommentMarker, PixelTypes.Rgba32)] + public void Encode_PreservesComments(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + // arrange + using var input = provider.GetImage(JpegDecoder.Instance); + using var memStream = new MemoryStream(); + + // act + input.Save(memStream, JpegEncoder); + + // assert + memStream.Position = 0; + using var output = Image.Load(memStream); + JpegMetadata actual = output.Metadata.GetJpegMetadata(); + Assert.NotEmpty(actual.Comments); + Assert.Equal(1, actual.Comments.Count); + Assert.Equal("TEST COMMENT", actual.Comments.ElementAt(0).ToString()); + } + + [Fact] + public void Encode_SavesMultipleComments() + { + // arrange + using var input = new Image(1, 1); + JpegMetadata meta = input.Metadata.GetJpegMetadata(); + using var memStream = new MemoryStream(); + + // act + meta.SaveComment("First comment"); + meta.SaveComment("Second Comment"); + input.Save(memStream, JpegEncoder); + + // assert + memStream.Position = 0; + using var output = Image.Load(memStream); + JpegMetadata actual = output.Metadata.GetJpegMetadata(); + Assert.NotEmpty(actual.Comments); + Assert.Equal(2, actual.Comments.Count); + Assert.Equal(meta.Comments.ElementAt(0).ToString(), actual.Comments.ElementAt(0).ToString()); + Assert.Equal(meta.Comments.ElementAt(1).ToString(), actual.Comments.ElementAt(1).ToString()); + } + [Theory] [WithFile(TestImages.Jpeg.Baseline.Floorplan, PixelTypes.Rgb24, JpegEncodingColor.Luminance)] [WithFile(TestImages.Jpeg.Baseline.Jpeg444, PixelTypes.Rgb24, JpegEncodingColor.YCbCrRatio444)] diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs index 05f22667d..901bb4619 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.Collections.ObjectModel; using SixLabors.ImageSharp.Formats.Jpeg; namespace SixLabors.ImageSharp.Tests.Formats.Jpg; @@ -55,6 +56,27 @@ public class JpegMetadataTests var meta = new JpegMetadata { LuminanceQuality = qualityLuma, ChrominanceQuality = qualityChroma }; - Assert.Equal(meta.Quality, qualityLuma); + Assert.Equal(meta.Quality, qualityLuma); + } + + [Fact] + public void Comment_EmptyComment() + { + var meta = new JpegMetadata(); + + Assert.True(Array.Empty>().SequenceEqual(meta.Comments)); + } + + [Fact] + public void Comment_OnlyComment() + { + string comment = "test comment"; + var expectedCollection = new Collection> { new(comment.ToCharArray()) }; + + var meta = new JpegMetadata(); + meta.Comments?.Add(comment.ToCharArray()); + + Assert.Equal(1, meta.Comments?.Count); + Assert.True(expectedCollection.FirstOrDefault().ToString() == meta.Comments?.FirstOrDefault().ToString()); } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 8aa95d349..0dab4ff87 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -309,6 +309,7 @@ public static class TestImages public const string Issue2564 = "Jpg/issues/issue-2564.jpg"; public const string HangBadScan = "Jpg/issues/Hang_C438A851.jpg"; public const string Issue2517 = "Jpg/issues/issue2517-bad-d7.jpg"; + public const string Issue2067_CommentMarker = "Jpg/issues/issue-2067-comment.jpg"; public static class Fuzz { diff --git a/tests/Images/Input/Jpg/issues/issue-2067-comment.jpg b/tests/Images/Input/Jpg/issues/issue-2067-comment.jpg new file mode 100644 index 000000000..18dc6f2e3 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/issue-2067-comment.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d87b5429adeffcfac535aa8af2ec9801bf6c965a2e6751cfec4f8534195ba8f4 +size 21082 From b3a8452edc95ed4603b13fc78dab90394b5a2891 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Mutnia=C5=84ski?= Date: Sun, 7 Jan 2024 18:53:03 +0100 Subject: [PATCH 04/37] Rename SaveComment to SetComment, add index, add clearcomments --- .../Formats/Jpeg/MetadataExtensions.cs | 23 +++++++++++++++---- .../Formats/Jpg/JpegDecoderTests.Metadata.cs | 15 ++++++++++++ .../Formats/Jpg/JpegDecoderTests.cs | 15 ------------ .../Formats/Jpg/JpegEncoderTests.Metadata.cs | 10 ++++---- 4 files changed, 38 insertions(+), 25 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/MetadataExtensions.cs b/src/ImageSharp/Formats/Jpeg/MetadataExtensions.cs index 53efc7d0c..0c66fcbdd 100644 --- a/src/ImageSharp/Formats/Jpeg/MetadataExtensions.cs +++ b/src/ImageSharp/Formats/Jpeg/MetadataExtensions.cs @@ -20,22 +20,35 @@ public static partial class MetadataExtensions public static JpegMetadata GetJpegMetadata(this ImageMetadata metadata) => metadata.GetFormatMetadata(JpegFormat.Instance); /// - /// Saves the comment into + /// Sets the comment in /// /// The metadata this method extends. + /// The index of comment to be inserted to. /// The comment string. - public static void SaveComment(this JpegMetadata metadata, string comment) + public static void SetComment(this JpegMetadata metadata, int index, string comment) { - ASCIIEncoding encoding = new(); + if (metadata.Comments == null) + { + return; + } + ASCIIEncoding encoding = new(); byte[] bytes = encoding.GetBytes(comment); - metadata.Comments?.Add(encoding.GetChars(bytes)); + List>? comments = metadata.Comments as List>; + comments?.Insert(index, encoding.GetChars(bytes)); } /// /// Gets the comments from /// /// The metadata this method extends. + /// The index of comment. /// The IEnumerable string of comments. - public static IEnumerable? GetComments(this JpegMetadata metadata) => metadata.Comments?.Select(x => x.ToString()); + public static string? GetComment(this JpegMetadata metadata, int index) => metadata.Comments?.ElementAtOrDefault(index).ToString(); + + /// + /// Clears comments + /// + /// The . + public static void ClearComments(this JpegMetadata metadata) => metadata.Comments?.Clear(); } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs index 1c203e734..fb37a956d 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs @@ -425,6 +425,21 @@ public partial class JpegDecoderTests VerifyEncodedStrings(exif); } + [Theory] + [WithFile(TestImages.Jpeg.Issues.Issue2067_CommentMarker, PixelTypes.Rgba32)] + public void JpegDecoder_DecodeMetadataComment(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + string expectedComment = "TEST COMMENT"; + using Image image = provider.GetImage(JpegDecoder.Instance); + JpegMetadata metadata = image.Metadata.GetJpegMetadata(); + + Assert.Equal(1, metadata.Comments?.Count); + Assert.Equal(expectedComment, metadata.GetComment(0)); + image.DebugSave(provider); + image.CompareToOriginal(provider); + } + private static void VerifyEncodedStrings(ExifProfile exif) { Assert.NotNull(exif); diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index b219e715f..c8d93f6e9 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -364,19 +364,4 @@ public partial class JpegDecoderTests image.DebugSave(provider); image.CompareToOriginal(provider); } - - [Theory] - [WithFile(TestImages.Jpeg.Issues.Issue2067_CommentMarker, PixelTypes.Rgba32)] - public void JpegDecoder_DecodeMetadataComment(TestImageProvider provider) - where TPixel : unmanaged, IPixel - { - string expectedComment = "TEST COMMENT"; - using Image image = provider.GetImage(JpegDecoder.Instance); - JpegMetadata metadata = image.Metadata.GetJpegMetadata(); - - Assert.Equal(1, metadata.Comments?.Count); - Assert.Equal(expectedComment, metadata.GetComments()?.FirstOrDefault()); - image.DebugSave(provider); - image.CompareToOriginal(provider); - } } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs index 50f47a134..fa54859a3 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs @@ -184,8 +184,8 @@ public partial class JpegEncoderTests using var memStream = new MemoryStream(); // act - meta.SaveComment("First comment"); - meta.SaveComment("Second Comment"); + meta.SetComment(0, "First comment"); + meta.SetComment(1, "Second Comment"); input.Save(memStream, JpegEncoder); // assert @@ -193,9 +193,9 @@ public partial class JpegEncoderTests using var output = Image.Load(memStream); JpegMetadata actual = output.Metadata.GetJpegMetadata(); Assert.NotEmpty(actual.Comments); - Assert.Equal(2, actual.Comments.Count); - Assert.Equal(meta.Comments.ElementAt(0).ToString(), actual.Comments.ElementAt(0).ToString()); - Assert.Equal(meta.Comments.ElementAt(1).ToString(), actual.Comments.ElementAt(1).ToString()); + Assert.Equal(2, actual.Comments?.Count); + Assert.Equal(meta.Comments?.ElementAt(0).ToString(), actual.Comments?.ElementAt(0).ToString()); + Assert.Equal(meta.Comments?.ElementAt(1).ToString(), actual.Comments?.ElementAt(1).ToString()); } [Theory] From d6165909fd7be5e45b2f176373dd9694e1ab93e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Mutnia=C5=84ski?= Date: Sun, 7 Jan 2024 20:13:19 +0100 Subject: [PATCH 05/37] tab --- tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs index 901bb4619..8b991228b 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs @@ -56,7 +56,7 @@ public class JpegMetadataTests var meta = new JpegMetadata { LuminanceQuality = qualityLuma, ChrominanceQuality = qualityChroma }; - Assert.Equal(meta.Quality, qualityLuma); + Assert.Equal(meta.Quality, qualityLuma); } [Fact] From 6580494b5aff13480f81ed623b14b5c3c1eeba62 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 22 Jan 2024 09:43:09 +0000 Subject: [PATCH 06/37] Bump actions/cache from 3 to 4 Bumps [actions/cache](https://github.com/actions/cache) from 3 to 4. - [Release notes](https://github.com/actions/cache/releases) - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md) - [Commits](https://github.com/actions/cache/compare/v3...v4) --- updated-dependencies: - dependency-name: actions/cache dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/build-and-test.yml | 6 +++--- .github/workflows/code-coverage.yml | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 75bcb8a25..57488a1d0 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -67,7 +67,7 @@ jobs: run: git lfs ls-files -l | cut -d' ' -f1 | sort > .lfs-assets-id - name: Git Setup LFS Cache - uses: actions/cache@v3 + uses: actions/cache@v4 id: lfs-cache with: path: .git/lfs @@ -80,7 +80,7 @@ jobs: uses: NuGet/setup-nuget@v1 - name: NuGet Setup Cache - uses: actions/cache@v3 + uses: actions/cache@v4 id: nuget-cache with: path: ~/.nuget @@ -162,7 +162,7 @@ jobs: uses: NuGet/setup-nuget@v1 - name: NuGet Setup Cache - uses: actions/cache@v3 + uses: actions/cache@v4 id: nuget-cache with: path: ~/.nuget diff --git a/.github/workflows/code-coverage.yml b/.github/workflows/code-coverage.yml index 62b6477ee..f9d2da0c8 100644 --- a/.github/workflows/code-coverage.yml +++ b/.github/workflows/code-coverage.yml @@ -34,7 +34,7 @@ jobs: run: git lfs ls-files -l | cut -d' ' -f1 | sort > .lfs-assets-id - name: Git Setup LFS Cache - uses: actions/cache@v3 + uses: actions/cache@v4 id: lfs-cache with: path: .git/lfs @@ -47,7 +47,7 @@ jobs: uses: NuGet/setup-nuget@v1 - name: NuGet Setup Cache - uses: actions/cache@v3 + uses: actions/cache@v4 id: nuget-cache with: path: ~/.nuget From 51272021290112a51812c53eefb9d0d4124b13ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Mutnia=C5=84ski?= Date: Tue, 23 Jan 2024 17:00:34 +0100 Subject: [PATCH 07/37] pr comment fix --- src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 2 +- src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs | 2 +- .../Formats/Jpg/JpegEncoderTests.Metadata.cs | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index e11923ac8..d6b40fa7f 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -523,7 +523,7 @@ internal sealed class JpegDecoderCore : IRawJpegData, IImageDecoderInternals /// The remaining bytes in the segment block. private void ProcessComMarker(BufferedReadStream stream, int markerContentByteSize) { - Span temp = stackalloc byte[markerContentByteSize]; + Span temp = new byte[markerContentByteSize]; char[] chars = new char[markerContentByteSize]; JpegMetadata metadata = this.Metadata.GetFormatMetadata(JpegFormat.Instance); diff --git a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs index cc6042b6e..1b2b4cbb1 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs @@ -185,7 +185,7 @@ internal sealed unsafe partial class JpegEncoderCore : IImageEncoderInternals // Length (comment strings lengths) + (comments markers with payload sizes) int commentsBytes = metadata.Comments.Sum(x => x.Length) + (metadata.Comments.Count * 4); int commentStart = 0; - Span commentBuffer = stackalloc byte[commentsBytes]; + Span commentBuffer = new byte[commentsBytes]; foreach (Memory comment in metadata.Comments) { diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs index fa54859a3..bd68eaf20 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs @@ -137,7 +137,7 @@ public partial class JpegEncoderTests [MemberData(nameof(QualityFiles))] public void Encode_PreservesQuality(string imagePath, int quality) { - var testFile = TestFile.Create(imagePath); + TestFile testFile = TestFile.Create(imagePath); using (Image input = testFile.CreateRgba32Image()) { using (var memStream = new MemoryStream()) @@ -160,7 +160,7 @@ public partial class JpegEncoderTests where TPixel : unmanaged, IPixel { // arrange - using var input = provider.GetImage(JpegDecoder.Instance); + using Image input = provider.GetImage(JpegDecoder.Instance); using var memStream = new MemoryStream(); // act @@ -168,7 +168,7 @@ public partial class JpegEncoderTests // assert memStream.Position = 0; - using var output = Image.Load(memStream); + using Image output = Image.Load(memStream); JpegMetadata actual = output.Metadata.GetJpegMetadata(); Assert.NotEmpty(actual.Comments); Assert.Equal(1, actual.Comments.Count); @@ -190,7 +190,7 @@ public partial class JpegEncoderTests // assert memStream.Position = 0; - using var output = Image.Load(memStream); + using Image output = Image.Load(memStream); JpegMetadata actual = output.Metadata.GetJpegMetadata(); Assert.NotEmpty(actual.Comments); Assert.Equal(2, actual.Comments?.Count); From 9260be9d2996c614d7c992540ebef6ab4d2ca7b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Mutnia=C5=84ski?= Date: Thu, 25 Jan 2024 19:19:41 +0100 Subject: [PATCH 08/37] Add com character limit, comment as IList, remove unnecessary extension methods --- .../Formats/Jpeg/JpegEncoderCore.cs | 43 +++++++++++-------- src/ImageSharp/Formats/Jpeg/JpegMetadata.cs | 2 +- .../Formats/Jpeg/MetadataExtensions.cs | 33 -------------- .../Formats/Jpg/JpegDecoderTests.Metadata.cs | 4 +- .../Formats/Jpg/JpegEncoderTests.Metadata.cs | 12 +++--- 5 files changed, 34 insertions(+), 60 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs index 1b2b4cbb1..4dc920207 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs @@ -177,39 +177,46 @@ internal sealed unsafe partial class JpegEncoderCore : IImageEncoderInternals /// The image metadata. private void WriteComment(JpegMetadata metadata) { - if (metadata.Comments is { Count: 0 }) + int maxCommentLength = 65533; + + if (metadata.Comments.Count == 0) { return; } - // Length (comment strings lengths) + (comments markers with payload sizes) - int commentsBytes = metadata.Comments.Sum(x => x.Length) + (metadata.Comments.Count * 4); - int commentStart = 0; - Span commentBuffer = new byte[commentsBytes]; - - foreach (Memory comment in metadata.Comments) + for (int i = 0; i < metadata.Comments.Count; i++) { - int totalComLength = comment.Length + 4; + Memory chars = metadata.Comments[i]; + + if (chars.Length > maxCommentLength) + { + Memory splitComment = chars.Slice(maxCommentLength, chars.Length - maxCommentLength); + metadata.Comments.Insert(i + 1, splitComment); - Span commentData = commentBuffer.Slice(commentStart, totalComLength); - Span markers = commentData.Slice(0, 2); - Span payloadSize = commentData.Slice(2, 2); - Span payload = commentData.Slice(4, comment.Length); + // We don't want to keep the extra bytes + chars = chars.Slice(0, maxCommentLength); + } + + int commentLength = chars.Length + 4; + + Span comment = new byte[commentLength]; + Span markers = comment.Slice(0, 2); + Span payloadSize = comment.Slice(2, 2); + Span payload = comment.Slice(4, chars.Length); // Beginning of comment ff fe markers[0] = JpegConstants.Markers.XFF; markers[1] = JpegConstants.Markers.COM; // Write payload size - BinaryPrimitives.WriteInt16BigEndian(payloadSize, (short)(commentData.Length - 2)); + int comWithoutMarker = commentLength - 2; + payloadSize[0] = (byte)((comWithoutMarker >> 8) & 0xFF); + payloadSize[1] = (byte)(comWithoutMarker & 0xFF); - Encoding.ASCII.GetBytes(comment.Span, payload); + Encoding.ASCII.GetBytes(chars.Span, payload); - // Indicate begin of next comment in buffer - commentStart += totalComLength; + this.outputStream.Write(comment, 0, comment.Length); } - - this.outputStream.Write(commentBuffer, 0, commentBuffer.Length); } /// diff --git a/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs b/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs index 61fe3b214..bf758dfd0 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs @@ -106,7 +106,7 @@ public class JpegMetadata : IDeepCloneable /// /// Gets the comments. /// - public ICollection>? Comments { get; } + public IList> Comments { get; } /// public IDeepCloneable DeepClone() => new JpegMetadata(this); diff --git a/src/ImageSharp/Formats/Jpeg/MetadataExtensions.cs b/src/ImageSharp/Formats/Jpeg/MetadataExtensions.cs index 0c66fcbdd..7330e74b7 100644 --- a/src/ImageSharp/Formats/Jpeg/MetadataExtensions.cs +++ b/src/ImageSharp/Formats/Jpeg/MetadataExtensions.cs @@ -18,37 +18,4 @@ public static partial class MetadataExtensions /// The metadata this method extends. /// The . public static JpegMetadata GetJpegMetadata(this ImageMetadata metadata) => metadata.GetFormatMetadata(JpegFormat.Instance); - - /// - /// Sets the comment in - /// - /// The metadata this method extends. - /// The index of comment to be inserted to. - /// The comment string. - public static void SetComment(this JpegMetadata metadata, int index, string comment) - { - if (metadata.Comments == null) - { - return; - } - - ASCIIEncoding encoding = new(); - byte[] bytes = encoding.GetBytes(comment); - List>? comments = metadata.Comments as List>; - comments?.Insert(index, encoding.GetChars(bytes)); - } - - /// - /// Gets the comments from - /// - /// The metadata this method extends. - /// The index of comment. - /// The IEnumerable string of comments. - public static string? GetComment(this JpegMetadata metadata, int index) => metadata.Comments?.ElementAtOrDefault(index).ToString(); - - /// - /// Clears comments - /// - /// The . - public static void ClearComments(this JpegMetadata metadata) => metadata.Comments?.Clear(); } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs index fb37a956d..369e71abf 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs @@ -434,8 +434,8 @@ public partial class JpegDecoderTests using Image image = provider.GetImage(JpegDecoder.Instance); JpegMetadata metadata = image.Metadata.GetJpegMetadata(); - Assert.Equal(1, metadata.Comments?.Count); - Assert.Equal(expectedComment, metadata.GetComment(0)); + Assert.Equal(1, metadata.Comments.Count); + Assert.Equal(expectedComment.ToCharArray(), metadata.Comments.ElementAtOrDefault(0)); image.DebugSave(provider); image.CompareToOriginal(provider); } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs index bd68eaf20..8cc64acea 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs @@ -172,7 +172,7 @@ public partial class JpegEncoderTests JpegMetadata actual = output.Metadata.GetJpegMetadata(); Assert.NotEmpty(actual.Comments); Assert.Equal(1, actual.Comments.Count); - Assert.Equal("TEST COMMENT", actual.Comments.ElementAt(0).ToString()); + Assert.Equal("TEST COMMENT", actual.Comments.ElementAtOrDefault(0).ToString()); } [Fact] @@ -184,8 +184,8 @@ public partial class JpegEncoderTests using var memStream = new MemoryStream(); // act - meta.SetComment(0, "First comment"); - meta.SetComment(1, "Second Comment"); + meta.Comments.Add("First comment".ToCharArray()); + meta.Comments.Add("Second Comment".ToCharArray()); input.Save(memStream, JpegEncoder); // assert @@ -193,9 +193,9 @@ public partial class JpegEncoderTests using Image output = Image.Load(memStream); JpegMetadata actual = output.Metadata.GetJpegMetadata(); Assert.NotEmpty(actual.Comments); - Assert.Equal(2, actual.Comments?.Count); - Assert.Equal(meta.Comments?.ElementAt(0).ToString(), actual.Comments?.ElementAt(0).ToString()); - Assert.Equal(meta.Comments?.ElementAt(1).ToString(), actual.Comments?.ElementAt(1).ToString()); + Assert.Equal(2, actual.Comments.Count); + Assert.Equal(meta.Comments.ElementAtOrDefault(0).ToString(), actual.Comments.ElementAtOrDefault(0).ToString()); + Assert.Equal(meta.Comments.ElementAtOrDefault(1).ToString(), actual.Comments.ElementAtOrDefault(1).ToString()); } [Theory] From 980347e96fae9642cbd89bccb5a732d61865558c Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 2 Feb 2024 15:51:22 +1000 Subject: [PATCH 09/37] ENhance NormalizedFloatToByteSaturate --- src/ImageSharp/Common/Helpers/Numerics.cs | 20 +++ .../Helpers/SimdUtils.ExtendedIntrinsics.cs | 4 +- .../SimdUtils.FallbackIntrinsics128.cs | 4 +- .../Common/Helpers/SimdUtils.HwIntrinsics.cs | 150 +++++++++++------- src/ImageSharp/Common/Helpers/SimdUtils.cs | 49 +++--- .../Common/Helpers/Vector128Utilities.cs | 87 +++++++++- .../Common/Helpers/Vector256Utilities.cs | 39 ++++- .../Common/Helpers/Vector512Utilities.cs | 37 ++++- .../PixelOperations/Rgba32.PixelOperations.cs | 16 +- .../ImageSharp.Benchmarks/Bulk/FromVector4.cs | 92 ++++++----- .../Bulk/FromVector4_Rgb24.cs | 66 +++----- .../ImageSharp.Tests/Common/SimdUtilsTests.cs | 5 +- .../TestUtilities/BasicSerializer.cs | 22 +-- .../FeatureTesting/FeatureTestRunner.cs | 81 ++++++---- 14 files changed, 443 insertions(+), 229 deletions(-) diff --git a/src/ImageSharp/Common/Helpers/Numerics.cs b/src/ImageSharp/Common/Helpers/Numerics.cs index ca28a7aab..5f85734e8 100644 --- a/src/ImageSharp/Common/Helpers/Numerics.cs +++ b/src/ImageSharp/Common/Helpers/Numerics.cs @@ -1010,6 +1010,26 @@ internal static class Numerics where TVector : struct => (uint)span.Length / (uint)Vector256.Count; + /// + /// Gets the count of vectors that safely fit into the given span. + /// + /// The type of the vector. + /// The given span. + /// Count of vectors that safely fit into the span. + public static nuint Vector512Count(this Span span) + where TVector : struct + => (uint)span.Length / (uint)Vector512.Count; + + /// + /// Gets the count of vectors that safely fit into the given span. + /// + /// The type of the vector. + /// The given span. + /// Count of vectors that safely fit into the span. + public static nuint Vector512Count(this ReadOnlySpan span) + where TVector : struct + => (uint)span.Length / (uint)Vector512.Count; + /// /// Gets the count of vectors that safely fit into the given span. /// diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.ExtendedIntrinsics.cs b/src/ImageSharp/Common/Helpers/SimdUtils.ExtendedIntrinsics.cs index ac122fc7d..7d07dcaae 100644 --- a/src/ImageSharp/Common/Helpers/SimdUtils.ExtendedIntrinsics.cs +++ b/src/ImageSharp/Common/Helpers/SimdUtils.ExtendedIntrinsics.cs @@ -95,7 +95,7 @@ internal static partial class SimdUtils /// internal static void ByteToNormalizedFloat(ReadOnlySpan source, Span dest) { - VerifySpanInput(source, dest, Vector.Count); + DebugVerifySpanInput(source, dest, Vector.Count); nuint n = dest.VectorCount(); @@ -130,7 +130,7 @@ internal static partial class SimdUtils ReadOnlySpan source, Span dest) { - VerifySpanInput(source, dest, Vector.Count); + DebugVerifySpanInput(source, dest, Vector.Count); nuint n = dest.VectorCount(); diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.FallbackIntrinsics128.cs b/src/ImageSharp/Common/Helpers/SimdUtils.FallbackIntrinsics128.cs index a551cebd0..90b313fb9 100644 --- a/src/ImageSharp/Common/Helpers/SimdUtils.FallbackIntrinsics128.cs +++ b/src/ImageSharp/Common/Helpers/SimdUtils.FallbackIntrinsics128.cs @@ -69,7 +69,7 @@ internal static partial class SimdUtils [MethodImpl(InliningOptions.ColdPath)] internal static void ByteToNormalizedFloat(ReadOnlySpan source, Span dest) { - VerifySpanInput(source, dest, 4); + DebugVerifySpanInput(source, dest, 4); uint count = (uint)dest.Length / 4; if (count == 0) @@ -103,7 +103,7 @@ internal static partial class SimdUtils ReadOnlySpan source, Span dest) { - VerifySpanInput(source, dest, 4); + DebugVerifySpanInput(source, dest, 4); uint count = (uint)source.Length / 4; if (count == 0) diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs b/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs index f27852a82..feb55ebe5 100644 --- a/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs +++ b/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs @@ -17,8 +17,13 @@ internal static partial class SimdUtils { public static class HwIntrinsics { +#pragma warning disable SA1117 // Parameters should be on same line or separate lines +#pragma warning disable SA1137 // Elements should have the same indentation [MethodImpl(MethodImplOptions.AggressiveInlining)] // too much IL for JIT to inline, so give a hint - public static Vector256 PermuteMaskDeinterleave8x32() => Vector256.Create(0, 0, 0, 0, 4, 0, 0, 0, 1, 0, 0, 0, 5, 0, 0, 0, 2, 0, 0, 0, 6, 0, 0, 0, 3, 0, 0, 0, 7, 0, 0, 0).AsInt32(); + public static Vector256 PermuteMaskDeinterleave8x32() => Vector256.Create(0, 4, 1, 5, 2, 6, 3, 7); + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static Vector512 PermuteMaskDeinterleave16x32() => Vector512.Create(0, 4, 8, 12, 1, 5, 9, 13, 2, 6, 10, 14, 3, 7, 11, 15); [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Vector256 PermuteMaskEvenOdd8x32() => Vector256.Create(0, 0, 0, 0, 2, 0, 0, 0, 4, 0, 0, 0, 6, 0, 0, 0, 1, 0, 0, 0, 3, 0, 0, 0, 5, 0, 0, 0, 7, 0, 0, 0).AsUInt32(); @@ -38,17 +43,18 @@ internal static partial class SimdUtils [MethodImpl(MethodImplOptions.AggressiveInlining)] private static Vector128 ShuffleMaskSlice4Nx16() => Vector128.Create(0, 1, 2, 4, 5, 6, 8, 9, 10, 12, 13, 14, 0x80, 0x80, 0x80, 0x80); -#pragma warning disable SA1003, SA1116, SA1117 // Parameters should be on same line or separate lines [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static Vector256 ShuffleMaskShiftAlpha() => Vector256.Create((byte) - 0, 1, 2, 4, 5, 6, 8, 9, 10, 12, 13, 14, 3, 7, 11, 15, - 0, 1, 2, 4, 5, 6, 8, 9, 10, 12, 13, 14, 3, 7, 11, 15); + private static Vector256 ShuffleMaskShiftAlpha() => Vector256.Create( + (byte)0, 1, 2, 4, 5, 6, 8, 9, 10, 12, 13, 14, 3, 7, 11, 15, + 0, 1, 2, 4, 5, 6, 8, 9, 10, 12, 13, 14, 3, 7, 11, 15); [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static Vector256 PermuteMaskShiftAlpha8x32() => Vector256.Create( - 0, 0, 0, 0, 1, 0, 0, 0, 2, 0, 0, 0, 4, 0, 0, 0, - 5, 0, 0, 0, 6, 0, 0, 0, 3, 0, 0, 0, 7, 0, 0, 0).AsUInt32(); -#pragma warning restore SA1003, SA1116, SA1117 // Parameters should be on same line or separate lines + public static Vector256 PermuteMaskShiftAlpha8x32() + => Vector256.Create( + 0, 0, 0, 0, 1, 0, 0, 0, 2, 0, 0, 0, 4, 0, 0, 0, + 5, 0, 0, 0, 6, 0, 0, 0, 3, 0, 0, 0, 7, 0, 0, 0).AsUInt32(); +#pragma warning restore SA1137 // Elements should have the same indentation +#pragma warning restore SA1117 // Parameters should be on same line or separate lines /// /// Shuffle single-precision (32-bit) floating-point elements in @@ -795,7 +801,7 @@ internal static partial class SimdUtils { if (Avx2.IsSupported) { - VerifySpanInput(source, dest, Vector256.Count); + DebugVerifySpanInput(source, dest, Vector256.Count); nuint n = dest.Vector256Count(); @@ -828,7 +834,7 @@ internal static partial class SimdUtils else { // Sse - VerifySpanInput(source, dest, Vector128.Count); + DebugVerifySpanInput(source, dest, Vector128.Count); nuint n = dest.Vector128Count(); @@ -881,17 +887,24 @@ internal static partial class SimdUtils /// /// as many elements as possible, slicing them down (keeping the remainder). /// + /// The source buffer. + /// The destination buffer. [MethodImpl(InliningOptions.ShortMethod)] internal static void NormalizedFloatToByteSaturateReduce( ref ReadOnlySpan source, - ref Span dest) + ref Span destination) { - DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same length!"); + DebugGuard.IsTrue(source.Length == destination.Length, nameof(source), "Input spans must be of same length!"); - if (Avx2.IsSupported || Sse2.IsSupported) + if (Avx512BW.IsSupported || Avx2.IsSupported || Sse2.IsSupported || AdvSimd.IsSupported) { int remainder; - if (Avx2.IsSupported) + + if (Avx512BW.IsSupported) + { + remainder = Numerics.ModuloP2(source.Length, Vector512.Count); + } + else if (Avx2.IsSupported) { remainder = Numerics.ModuloP2(source.Length, Vector256.Count); } @@ -906,10 +919,10 @@ internal static partial class SimdUtils { NormalizedFloatToByteSaturate( source[..adjustedCount], - dest[..adjustedCount]); + destination[..adjustedCount]); source = source[adjustedCount..]; - dest = dest[adjustedCount..]; + destination = destination[adjustedCount..]; } } } @@ -917,25 +930,59 @@ internal static partial class SimdUtils /// /// Implementation of , which is faster on new .NET runtime. /// + /// The source buffer. + /// The destination buffer. /// /// Implementation is based on MagicScaler code: /// https://github.com/saucecontrol/PhotoSauce/blob/b5811908041200488aa18fdfd17df5fc457415dc/src/MagicScaler/Magic/Processors/ConvertersFloat.cs#L541-L622 /// internal static void NormalizedFloatToByteSaturate( ReadOnlySpan source, - Span dest) + Span destination) { - if (Avx2.IsSupported) + if (Avx512BW.IsSupported) { - VerifySpanInput(source, dest, Vector256.Count); + DebugVerifySpanInput(source, destination, Vector512.Count); + + nuint n = destination.Vector512Count(); - nuint n = dest.Vector256Count(); + ref Vector512 sourceBase = ref Unsafe.As>(ref MemoryMarshal.GetReference(source)); + ref Vector512 destinationBase = ref Unsafe.As>(ref MemoryMarshal.GetReference(destination)); - ref Vector256 sourceBase = - ref Unsafe.As>(ref MemoryMarshal.GetReference(source)); + Vector512 scale = Vector512.Create((float)byte.MaxValue); + Vector512 mask = PermuteMaskDeinterleave16x32(); - ref Vector256 destBase = - ref Unsafe.As>(ref MemoryMarshal.GetReference(dest)); + for (nuint i = 0; i < n; i++) + { + ref Vector512 s = ref Unsafe.Add(ref sourceBase, i * 4); + + Vector512 f0 = scale * s; + Vector512 f1 = scale * Unsafe.Add(ref s, 1); + Vector512 f2 = scale * Unsafe.Add(ref s, 2); + Vector512 f3 = scale * Unsafe.Add(ref s, 3); + + Vector512 w0 = Vector512Utilities.ConvertToInt32RoundToEven(f0); + Vector512 w1 = Vector512Utilities.ConvertToInt32RoundToEven(f1); + Vector512 w2 = Vector512Utilities.ConvertToInt32RoundToEven(f2); + Vector512 w3 = Vector512Utilities.ConvertToInt32RoundToEven(f3); + + Vector512 u0 = Avx512BW.PackSignedSaturate(w0, w1); + Vector512 u1 = Avx512BW.PackSignedSaturate(w2, w3); + Vector512 b = Avx512BW.PackUnsignedSaturate(u0, u1); + b = Avx512F.PermuteVar16x32(b.AsInt32(), mask).AsByte(); + + Unsafe.Add(ref destinationBase, i) = b; + } + } + else + if (Avx2.IsSupported) + { + DebugVerifySpanInput(source, destination, Vector256.Count); + + nuint n = destination.Vector256Count(); + + ref Vector256 sourceBase = ref Unsafe.As>(ref MemoryMarshal.GetReference(source)); + ref Vector256 destinationBase = ref Unsafe.As>(ref MemoryMarshal.GetReference(destination)); Vector256 scale = Vector256.Create((float)byte.MaxValue); Vector256 mask = PermuteMaskDeinterleave8x32(); @@ -944,36 +991,33 @@ internal static partial class SimdUtils { ref Vector256 s = ref Unsafe.Add(ref sourceBase, i * 4); - Vector256 f0 = Avx.Multiply(scale, s); - Vector256 f1 = Avx.Multiply(scale, Unsafe.Add(ref s, 1)); - Vector256 f2 = Avx.Multiply(scale, Unsafe.Add(ref s, 2)); - Vector256 f3 = Avx.Multiply(scale, Unsafe.Add(ref s, 3)); + Vector256 f0 = scale * s; + Vector256 f1 = scale * Unsafe.Add(ref s, 1); + Vector256 f2 = scale * Unsafe.Add(ref s, 2); + Vector256 f3 = scale * Unsafe.Add(ref s, 3); - Vector256 w0 = Avx.ConvertToVector256Int32(f0); - Vector256 w1 = Avx.ConvertToVector256Int32(f1); - Vector256 w2 = Avx.ConvertToVector256Int32(f2); - Vector256 w3 = Avx.ConvertToVector256Int32(f3); + Vector256 w0 = Vector256Utilities.ConvertToInt32RoundToEven(f0); + Vector256 w1 = Vector256Utilities.ConvertToInt32RoundToEven(f1); + Vector256 w2 = Vector256Utilities.ConvertToInt32RoundToEven(f2); + Vector256 w3 = Vector256Utilities.ConvertToInt32RoundToEven(f3); Vector256 u0 = Avx2.PackSignedSaturate(w0, w1); Vector256 u1 = Avx2.PackSignedSaturate(w2, w3); Vector256 b = Avx2.PackUnsignedSaturate(u0, u1); b = Avx2.PermuteVar8x32(b.AsInt32(), mask).AsByte(); - Unsafe.Add(ref destBase, i) = b; + Unsafe.Add(ref destinationBase, i) = b; } } else { - // Sse - VerifySpanInput(source, dest, Vector128.Count); - - nuint n = dest.Vector128Count(); + // Sse, AdvSimd + DebugVerifySpanInput(source, destination, Vector128.Count); - ref Vector128 sourceBase = - ref Unsafe.As>(ref MemoryMarshal.GetReference(source)); + nuint n = destination.Vector128Count(); - ref Vector128 destBase = - ref Unsafe.As>(ref MemoryMarshal.GetReference(dest)); + ref Vector128 sourceBase = ref Unsafe.As>(ref MemoryMarshal.GetReference(source)); + ref Vector128 destinationBase = ref Unsafe.As>(ref MemoryMarshal.GetReference(destination)); Vector128 scale = Vector128.Create((float)byte.MaxValue); @@ -981,20 +1025,20 @@ internal static partial class SimdUtils { ref Vector128 s = ref Unsafe.Add(ref sourceBase, i * 4); - Vector128 f0 = Sse.Multiply(scale, s); - Vector128 f1 = Sse.Multiply(scale, Unsafe.Add(ref s, 1)); - Vector128 f2 = Sse.Multiply(scale, Unsafe.Add(ref s, 2)); - Vector128 f3 = Sse.Multiply(scale, Unsafe.Add(ref s, 3)); + Vector128 f0 = scale * s; + Vector128 f1 = scale * Unsafe.Add(ref s, 1); + Vector128 f2 = scale * Unsafe.Add(ref s, 2); + Vector128 f3 = scale * Unsafe.Add(ref s, 3); - Vector128 w0 = Sse2.ConvertToVector128Int32(f0); - Vector128 w1 = Sse2.ConvertToVector128Int32(f1); - Vector128 w2 = Sse2.ConvertToVector128Int32(f2); - Vector128 w3 = Sse2.ConvertToVector128Int32(f3); + Vector128 w0 = Vector128Utilities.ConvertToInt32RoundToEven(f0); + Vector128 w1 = Vector128Utilities.ConvertToInt32RoundToEven(f1); + Vector128 w2 = Vector128Utilities.ConvertToInt32RoundToEven(f2); + Vector128 w3 = Vector128Utilities.ConvertToInt32RoundToEven(f3); - Vector128 u0 = Sse2.PackSignedSaturate(w0, w1); - Vector128 u1 = Sse2.PackSignedSaturate(w2, w3); + Vector128 u0 = Vector128Utilities.PackSignedSaturate(w0, w1); + Vector128 u1 = Vector128Utilities.PackSignedSaturate(w2, w3); - Unsafe.Add(ref destBase, i) = Sse2.PackUnsignedSaturate(u0, u1); + Unsafe.Add(ref destinationBase, i) = Vector128Utilities.PackUnsignedSaturate(u0, u1); } } } diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.cs b/src/ImageSharp/Common/Helpers/SimdUtils.cs index 497e3cc6a..002c1f8da 100644 --- a/src/ImageSharp/Common/Helpers/SimdUtils.cs +++ b/src/ImageSharp/Common/Helpers/SimdUtils.cs @@ -94,35 +94,31 @@ internal static partial class SimdUtils } /// - /// Convert all values normalized into [0..1] from 'source' into 'dest' buffer of . + /// Convert all values normalized into [0..1] from 'source' into 'destination' buffer of . /// The values are scaled up into [0-255] and rounded, overflows are clamped. - /// should be the of the same size as , + /// should be the of the same size as , /// but there are no restrictions on the span's length. /// /// The source span of floats - /// The destination span of bytes + /// The destination span of bytes [MethodImpl(InliningOptions.ShortMethod)] - internal static void NormalizedFloatToByteSaturate(ReadOnlySpan source, Span dest) + internal static void NormalizedFloatToByteSaturate(ReadOnlySpan source, Span destination) { - DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same length!"); - - HwIntrinsics.NormalizedFloatToByteSaturateReduce(ref source, ref dest); - - // Also deals with the remainder from previous conversions: - FallbackIntrinsics128.NormalizedFloatToByteSaturateReduce(ref source, ref dest); + DebugGuard.IsTrue(source.Length == destination.Length, nameof(source), "Input spans must be of same length!"); + HwIntrinsics.NormalizedFloatToByteSaturateReduce(ref source, ref destination); // Deal with the remainder: if (source.Length > 0) { - ConvertNormalizedFloatToByteRemainder(source, dest); + ConvertNormalizedFloatToByteRemainder(source, destination); } } [MethodImpl(InliningOptions.ColdPath)] - private static void ConvertByteToNormalizedFloatRemainder(ReadOnlySpan source, Span dest) + private static void ConvertByteToNormalizedFloatRemainder(ReadOnlySpan source, Span destination) { ref byte sBase = ref MemoryMarshal.GetReference(source); - ref float dBase = ref MemoryMarshal.GetReference(dest); + ref float dBase = ref MemoryMarshal.GetReference(destination); // There are at most 3 elements at this point, having a for loop is overkill. // Let's minimize the no. of instructions! @@ -140,23 +136,14 @@ internal static partial class SimdUtils } } - [MethodImpl(InliningOptions.ColdPath)] - private static void ConvertNormalizedFloatToByteRemainder(ReadOnlySpan source, Span dest) + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ConvertNormalizedFloatToByteRemainder(ReadOnlySpan source, Span destination) { ref float sBase = ref MemoryMarshal.GetReference(source); - ref byte dBase = ref MemoryMarshal.GetReference(dest); - - switch (source.Length) + ref byte dBase = ref MemoryMarshal.GetReference(destination); + for (int i = 0; i < source.Length; i++) { - case 3: - Unsafe.Add(ref dBase, 2) = ConvertToByte(Unsafe.Add(ref sBase, 2)); - goto case 2; - case 2: - Unsafe.Add(ref dBase, 1) = ConvertToByte(Unsafe.Add(ref sBase, 1)); - goto case 1; - case 1: - dBase = ConvertToByte(sBase); - break; + Unsafe.Add(ref dBase, i) = ConvertToByte(Unsafe.Add(ref sBase, i)); } } @@ -173,7 +160,7 @@ internal static partial class SimdUtils } [Conditional("DEBUG")] - private static void VerifySpanInput(ReadOnlySpan source, Span dest, int shouldBeDivisibleBy) + private static void DebugVerifySpanInput(ReadOnlySpan source, Span dest, int shouldBeDivisibleBy) { DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same length!"); DebugGuard.IsTrue( @@ -183,11 +170,11 @@ internal static partial class SimdUtils } [Conditional("DEBUG")] - private static void VerifySpanInput(ReadOnlySpan source, Span dest, int shouldBeDivisibleBy) + private static void DebugVerifySpanInput(ReadOnlySpan source, Span destination, int shouldBeDivisibleBy) { - DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same length!"); + DebugGuard.IsTrue(source.Length == destination.Length, nameof(source), "Input spans must be of same length!"); DebugGuard.IsTrue( - Numerics.ModuloP2(dest.Length, shouldBeDivisibleBy) == 0, + Numerics.ModuloP2(destination.Length, shouldBeDivisibleBy) == 0, nameof(source), $"length should be divisible by {shouldBeDivisibleBy}!"); } diff --git a/src/ImageSharp/Common/Helpers/Vector128Utilities.cs b/src/ImageSharp/Common/Helpers/Vector128Utilities.cs index 981f9a47f..a07fa8ca6 100644 --- a/src/ImageSharp/Common/Helpers/Vector128Utilities.cs +++ b/src/ImageSharp/Common/Helpers/Vector128Utilities.cs @@ -26,7 +26,7 @@ internal static class Vector128Utilities public static bool SupportsShuffleFloat { [MethodImpl(MethodImplOptions.AggressiveInlining)] - get => Sse.IsSupported; + get => Sse.IsSupported || AdvSimd.IsSupported; } /// @@ -62,6 +62,7 @@ internal static class Vector128Utilities /// The input vector from which values are selected. /// The shuffle control byte. /// The . + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Vector128 Shuffle(Vector128 vector, [ConstantExpected] byte control) { if (Sse.IsSupported) @@ -69,6 +70,17 @@ internal static class Vector128Utilities return Sse.Shuffle(vector, vector, control); } + if (AdvSimd.IsSupported) + { +#pragma warning disable CA1857 // A constant is expected for the parameter + Vector128 result = Vector128.Create(AdvSimd.Extract(vector, (byte)(control & 0x3))); + result = AdvSimd.Insert(result, 1, AdvSimd.Extract(vector, (byte)((control >> 2) & 0x3))); + result = AdvSimd.Insert(result, 2, AdvSimd.Extract(vector, (byte)((control >> 4) & 0x3))); + result = AdvSimd.Insert(result, 3, AdvSimd.Extract(vector, (byte)((control >> 6) & 0x3))); +#pragma warning restore CA1857 // A constant is expected for the parameter + return result; + } + ThrowUnreachableException(); return default; } @@ -84,6 +96,7 @@ internal static class Vector128Utilities /// /// A new vector containing the values from selected by the given . /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Vector128 Shuffle(Vector128 vector, Vector128 indices) { if (Ssse3.IsSupported) @@ -155,6 +168,7 @@ internal static class Vector128Utilities /// The right hand source vector. /// An 8-bit mask used for the operation. /// The . + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Vector128 AlignRight(Vector128 left, Vector128 right, [ConstantExpected(Max = (byte)15)] byte mask) { if (Ssse3.IsSupported) @@ -171,6 +185,77 @@ internal static class Vector128Utilities return default; } + /// + /// Performs a conversion from a 128-bit vector of 4 single-precision floating-point values to a 128-bit vector of 4 signed 32-bit integer values. + /// Rounding is equivalent to . + /// + /// The value to convert. + /// The . + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static Vector128 ConvertToInt32RoundToEven(Vector128 vector) + { + if (Sse2.IsSupported) + { + return Sse2.ConvertToVector128Int32(vector); + } + + if (AdvSimd.IsSupported) + { + return AdvSimd.ConvertToInt32RoundToEven(vector); + } + + Vector128 sign = vector & Vector128.Create(-0.0f); + Vector128 val_2p23_f32 = sign | Vector128.Create(8388608.0f); + + val_2p23_f32 = (vector + val_2p23_f32) - val_2p23_f32; + return Vector128.ConvertToInt32(val_2p23_f32 | sign); + } + + /// + /// Packs signed 16-bit integers to unsigned 8-bit integers and saturates. + /// + /// The left hand source vector. + /// The right hand source vector. + /// The . + public static Vector128 PackUnsignedSaturate(Vector128 left, Vector128 right) + { + if (Sse2.IsSupported) + { + return Sse2.PackUnsignedSaturate(left, right); + } + + if (AdvSimd.IsSupported) + { + return AdvSimd.ExtractNarrowingSaturateUnsignedUpper(AdvSimd.ExtractNarrowingSaturateUnsignedLower(left), right); + } + + ThrowUnreachableException(); + return default; + } + + /// + /// Packs signed 32-bit integers to signed 16-bit integers and saturates. + /// + /// The left hand source vector. + /// The right hand source vector. + /// The . + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static Vector128 PackSignedSaturate(Vector128 left, Vector128 right) + { + if (Sse2.IsSupported) + { + return Sse2.PackSignedSaturate(left, right); + } + + if (AdvSimd.IsSupported) + { + return AdvSimd.ExtractNarrowingSaturateUpper(AdvSimd.ExtractNarrowingSaturateLower(left), right); + } + + ThrowUnreachableException(); + return default; + } + [DoesNotReturn] private static void ThrowUnreachableException() => throw new UnreachableException(); } diff --git a/src/ImageSharp/Common/Helpers/Vector256Utilities.cs b/src/ImageSharp/Common/Helpers/Vector256Utilities.cs index 14fa24b31..6e8c0d1de 100644 --- a/src/ImageSharp/Common/Helpers/Vector256Utilities.cs +++ b/src/ImageSharp/Common/Helpers/Vector256Utilities.cs @@ -25,7 +25,7 @@ internal static class Vector256Utilities public static bool SupportsShuffleFloat { [MethodImpl(MethodImplOptions.AggressiveInlining)] - get => Avx.IsSupported; + get => Avx.IsSupported || Sse.IsSupported; } /// @@ -43,6 +43,7 @@ internal static class Vector256Utilities /// The input vector from which values are selected. /// The shuffle control byte. /// The . + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Vector256 Shuffle(Vector256 vector, [ConstantExpected] byte control) { if (Avx.IsSupported) @@ -50,6 +51,13 @@ internal static class Vector256Utilities return Avx.Shuffle(vector, vector, control); } + if (Sse.IsSupported) + { + Vector128 lower = vector.GetLower(); + Vector128 upper = vector.GetUpper(); + return Vector256.Create(Sse.Shuffle(lower, lower, control), Sse.Shuffle(upper, upper, control)); + } + ThrowUnreachableException(); return default; } @@ -62,6 +70,7 @@ internal static class Vector256Utilities /// The per-element indices used to select a value from . /// /// The . + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Vector256 Shuffle(Vector256 vector, Vector256 indices) { if (Avx2.IsSupported) @@ -73,6 +82,34 @@ internal static class Vector256Utilities return default; } + /// + /// Performs a conversion from a 256-bit vector of 8 single-precision floating-point values to a 256-bit vector of 8 signed 32-bit integer values. + /// Rounding is equivalent to . + /// + /// The value to convert. + /// The . + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static Vector256 ConvertToInt32RoundToEven(Vector256 vector) + { + if (Avx.IsSupported) + { + return Avx.ConvertToVector256Int32(vector); + } + + if (Sse2.IsSupported) + { + Vector128 lower = Sse2.ConvertToVector128Int32(vector.GetLower()); + Vector128 upper = Sse2.ConvertToVector128Int32(vector.GetUpper()); + return Vector256.Create(lower, upper); + } + + Vector256 sign = vector & Vector256.Create(-0.0f); + Vector256 val_2p23_f32 = sign | Vector256.Create(8388608.0f); + + val_2p23_f32 = (vector + val_2p23_f32) - val_2p23_f32; + return Vector256.ConvertToInt32(val_2p23_f32 | sign); + } + [DoesNotReturn] private static void ThrowUnreachableException() => throw new UnreachableException(); } diff --git a/src/ImageSharp/Common/Helpers/Vector512Utilities.cs b/src/ImageSharp/Common/Helpers/Vector512Utilities.cs index 5488b4064..0165af90e 100644 --- a/src/ImageSharp/Common/Helpers/Vector512Utilities.cs +++ b/src/ImageSharp/Common/Helpers/Vector512Utilities.cs @@ -25,7 +25,7 @@ internal static class Vector512Utilities public static bool SupportsShuffleFloat { [MethodImpl(MethodImplOptions.AggressiveInlining)] - get => Avx512F.IsSupported; + get => Avx512F.IsSupported || Avx.IsSupported; } /// @@ -51,6 +51,13 @@ internal static class Vector512Utilities return Avx512F.Shuffle(vector, vector, control); } + if (Avx.IsSupported) + { + Vector256 lower = vector.GetLower(); + Vector256 upper = vector.GetUpper(); + return Vector512.Create(Avx.Shuffle(lower, lower, control), Avx.Shuffle(upper, upper, control)); + } + ThrowUnreachableException(); return default; } @@ -75,6 +82,34 @@ internal static class Vector512Utilities return default; } + /// + /// Performs a conversion from a 512-bit vector of 16 single-precision floating-point values to a 512-bit vector of 16 signed 32-bit integer values. + /// Rounding is equivalent to . + /// + /// The value to convert. + /// The . + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static Vector512 ConvertToInt32RoundToEven(Vector512 vector) + { + if (Avx512F.IsSupported) + { + return Avx512F.ConvertToVector512Int32(vector); + } + + if (Avx.IsSupported) + { + Vector256 lower = Avx.ConvertToVector256Int32(vector.GetLower()); + Vector256 upper = Avx.ConvertToVector256Int32(vector.GetUpper()); + return Vector512.Create(lower, upper); + } + + Vector512 sign = vector & Vector512.Create(-0.0f); + Vector512 val_2p23_f32 = sign | Vector512.Create(8388608.0f); + + val_2p23_f32 = (vector + val_2p23_f32) - val_2p23_f32; + return Vector512.ConvertToInt32(val_2p23_f32 | sign); + } + [DoesNotReturn] private static void ThrowUnreachableException() => throw new UnreachableException(); } diff --git a/src/ImageSharp/PixelFormats/PixelImplementations/PixelOperations/Rgba32.PixelOperations.cs b/src/ImageSharp/PixelFormats/PixelImplementations/PixelOperations/Rgba32.PixelOperations.cs index ed89585dc..065e34c33 100644 --- a/src/ImageSharp/PixelFormats/PixelImplementations/PixelOperations/Rgba32.PixelOperations.cs +++ b/src/ImageSharp/PixelFormats/PixelImplementations/PixelOperations/Rgba32.PixelOperations.cs @@ -20,15 +20,15 @@ public partial struct Rgba32 /// public override void ToVector4( Configuration configuration, - ReadOnlySpan sourcePixels, + ReadOnlySpan source, Span destinationVectors, PixelConversionModifiers modifiers) { - Guard.DestinationShouldNotBeTooShort(sourcePixels, destinationVectors, nameof(destinationVectors)); + Guard.DestinationShouldNotBeTooShort(source, destinationVectors, nameof(destinationVectors)); - destinationVectors = destinationVectors[..sourcePixels.Length]; + destinationVectors = destinationVectors[..source.Length]; SimdUtils.ByteToNormalizedFloat( - MemoryMarshal.Cast(sourcePixels), + MemoryMarshal.Cast(source), MemoryMarshal.Cast(destinationVectors)); Vector4Converters.ApplyForwardConversionModifiers(destinationVectors, modifiers); } @@ -37,16 +37,16 @@ public partial struct Rgba32 public override void FromVector4Destructive( Configuration configuration, Span sourceVectors, - Span destinationPixels, + Span destination, PixelConversionModifiers modifiers) { - Guard.DestinationShouldNotBeTooShort(sourceVectors, destinationPixels, nameof(destinationPixels)); + Guard.DestinationShouldNotBeTooShort(sourceVectors, destination, nameof(destination)); - destinationPixels = destinationPixels[..sourceVectors.Length]; + destination = destination[..sourceVectors.Length]; Vector4Converters.ApplyBackwardConversionModifiers(sourceVectors, modifiers); SimdUtils.NormalizedFloatToByteSaturate( MemoryMarshal.Cast(sourceVectors), - MemoryMarshal.Cast(destinationPixels)); + MemoryMarshal.Cast(destination)); } /// diff --git a/tests/ImageSharp.Benchmarks/Bulk/FromVector4.cs b/tests/ImageSharp.Benchmarks/Bulk/FromVector4.cs index ecd16b957..dff687fa1 100644 --- a/tests/ImageSharp.Benchmarks/Bulk/FromVector4.cs +++ b/tests/ImageSharp.Benchmarks/Bulk/FromVector4.cs @@ -18,9 +18,9 @@ namespace SixLabors.ImageSharp.Benchmarks.Bulk; public abstract class FromVector4 where TPixel : unmanaged, IPixel { - protected IMemoryOwner source; + protected IMemoryOwner Source { get; set; } - protected IMemoryOwner destination; + protected IMemoryOwner Destination { get; set; } protected Configuration Configuration => Configuration.Default; @@ -31,22 +31,22 @@ public abstract class FromVector4 [GlobalSetup] public void Setup() { - this.destination = this.Configuration.MemoryAllocator.Allocate(this.Count); - this.source = this.Configuration.MemoryAllocator.Allocate(this.Count); + this.Destination = this.Configuration.MemoryAllocator.Allocate(this.Count); + this.Source = this.Configuration.MemoryAllocator.Allocate(this.Count); } [GlobalCleanup] public void Cleanup() { - this.destination.Dispose(); - this.source.Dispose(); + this.Destination.Dispose(); + this.Source.Dispose(); } // [Benchmark] public void PerElement() { - ref Vector4 s = ref MemoryMarshal.GetReference(this.source.GetSpan()); - ref TPixel d = ref MemoryMarshal.GetReference(this.destination.GetSpan()); + ref Vector4 s = ref MemoryMarshal.GetReference(this.Source.GetSpan()); + ref TPixel d = ref MemoryMarshal.GetReference(this.Destination.GetSpan()); for (nuint i = 0; i < (uint)this.Count; i++) { Unsafe.Add(ref d, i) = TPixel.FromVector4(Unsafe.Add(ref s, i)); @@ -55,11 +55,11 @@ public abstract class FromVector4 [Benchmark(Baseline = true)] public void PixelOperations_Base() - => new PixelOperations().FromVector4Destructive(this.Configuration, this.source.GetSpan(), this.destination.GetSpan()); + => new PixelOperations().FromVector4Destructive(this.Configuration, this.Source.GetSpan(), this.Destination.GetSpan()); [Benchmark] public void PixelOperations_Specialized() - => PixelOperations.Instance.FromVector4Destructive(this.Configuration, this.source.GetSpan(), this.destination.GetSpan()); + => PixelOperations.Instance.FromVector4Destructive(this.Configuration, this.Source.GetSpan(), this.Destination.GetSpan()); } public class FromVector4Rgba32 : FromVector4 @@ -67,8 +67,8 @@ public class FromVector4Rgba32 : FromVector4 [Benchmark] public void FallbackIntrinsics128() { - Span sBytes = MemoryMarshal.Cast(this.source.GetSpan()); - Span dFloats = MemoryMarshal.Cast(this.destination.GetSpan()); + Span sBytes = MemoryMarshal.Cast(this.Source.GetSpan()); + Span dFloats = MemoryMarshal.Cast(this.Destination.GetSpan()); SimdUtils.FallbackIntrinsics128.NormalizedFloatToByteSaturate(sBytes, dFloats); } @@ -76,8 +76,8 @@ public class FromVector4Rgba32 : FromVector4 [Benchmark] public void ExtendedIntrinsic() { - Span sBytes = MemoryMarshal.Cast(this.source.GetSpan()); - Span dFloats = MemoryMarshal.Cast(this.destination.GetSpan()); + Span sBytes = MemoryMarshal.Cast(this.Source.GetSpan()); + Span dFloats = MemoryMarshal.Cast(this.Destination.GetSpan()); SimdUtils.ExtendedIntrinsics.NormalizedFloatToByteSaturate(sBytes, dFloats); } @@ -85,8 +85,8 @@ public class FromVector4Rgba32 : FromVector4 [Benchmark] public void UseHwIntrinsics() { - Span sBytes = MemoryMarshal.Cast(this.source.GetSpan()); - Span dFloats = MemoryMarshal.Cast(this.destination.GetSpan()); + Span sBytes = MemoryMarshal.Cast(this.Source.GetSpan()); + Span dFloats = MemoryMarshal.Cast(this.Destination.GetSpan()); SimdUtils.HwIntrinsics.NormalizedFloatToByteSaturate(sBytes, dFloats); } @@ -96,8 +96,8 @@ public class FromVector4Rgba32 : FromVector4 [Benchmark] public void UseAvx2_Grouped() { - Span src = MemoryMarshal.Cast(this.source.GetSpan()); - Span dest = MemoryMarshal.Cast(this.destination.GetSpan()); + Span src = MemoryMarshal.Cast(this.Source.GetSpan()); + Span dest = MemoryMarshal.Cast(this.Destination.GetSpan()); nuint n = (uint)dest.Length / (uint)Vector.Count; @@ -107,7 +107,7 @@ public class FromVector4Rgba32 : FromVector4 ref byte maskBase = ref MemoryMarshal.GetReference(PermuteMaskDeinterleave8x32); Vector256 mask = Unsafe.As>(ref maskBase); - var maxBytes = Vector256.Create(255f); + Vector256 maxBytes = Vector256.Create(255f); for (nuint i = 0; i < n; i++) { @@ -137,25 +137,37 @@ public class FromVector4Rgba32 : FromVector4 } } - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static Vector256 ConvertToInt32(Vector256 vf, Vector256 scale) - { - vf = Avx.Multiply(scale, vf); - return Avx.ConvertToVector256Int32(vf); - } - - // *** RESULTS 2020 March: *** - // Intel Core i7-8650U CPU 1.90GHz (Kaby Lake R), 1 CPU, 8 logical and 4 physical cores - // .NET Core SDK=3.1.200-preview-014971 - // Job-IUZXZT : .NET Core 3.1.2 (CoreCLR 4.700.20.6602, CoreFX 4.700.20.6702), X64 RyuJIT - // - // | Method | Count | Mean | Error | StdDev | Ratio | RatioSD | Gen 0 | Gen 1 | Gen 2 | Allocated | - // |---------------------------- |------ |-----------:|------------:|----------:|------:|--------:|------:|------:|------:|----------:| - // | FallbackIntrinsics128 | 1024 | 2,952.6 ns | 1,680.77 ns | 92.13 ns | 3.32 | 0.16 | - | - | - | - | - // | BasicIntrinsics256 | 1024 | 1,664.5 ns | 928.11 ns | 50.87 ns | 1.87 | 0.09 | - | - | - | - | - // | ExtendedIntrinsic | 1024 | 890.6 ns | 375.48 ns | 20.58 ns | 1.00 | 0.00 | - | - | - | - | - // | UseAvx2 | 1024 | 299.0 ns | 30.47 ns | 1.67 ns | 0.34 | 0.01 | - | - | - | - | - // | UseAvx2_Grouped | 1024 | 318.1 ns | 48.19 ns | 2.64 ns | 0.36 | 0.01 | - | - | - | - | - // | PixelOperations_Base | 1024 | 8,136.9 ns | 1,834.82 ns | 100.57 ns | 9.14 | 0.26 | - | - | - | 24 B | - // | PixelOperations_Specialized | 1024 | 951.1 ns | 123.93 ns | 6.79 ns | 1.07 | 0.03 | - | - | - | - | + /* + BenchmarkDotNet v0.13.10, Windows 11 (10.0.22631.3085/23H2/2023Update/SunValley3) + 11th Gen Intel Core i7-11370H 3.30GHz, 1 CPU, 8 logical and 4 physical cores + .NET SDK 8.0.200-preview.23624.5 + [Host] : .NET 8.0.1 (8.0.123.58001), X64 RyuJIT AVX2 + Job-YJYLLR : .NET 8.0.1 (8.0.123.58001), X64 RyuJIT AVX2 + + Runtime=.NET 8.0 Arguments=/p:DebugType=portable IterationCount=3 + LaunchCount=1 WarmupCount=3 + + | Method | Count | Mean | Error | StdDev | Ratio | RatioSD | Allocated | Alloc Ratio | + |---------------------------- |------ |------------:|-------------:|-----------:|------:|--------:|----------:|------------:| + | PixelOperations_Base | 64 | 114.80 ns | 16.459 ns | 0.902 ns | 1.00 | 0.00 | - | NA | + | PixelOperations_Specialized | 64 | 28.91 ns | 80.482 ns | 4.411 ns | 0.25 | 0.04 | - | NA | + | FallbackIntrinsics128 | 64 | 133.60 ns | 23.750 ns | 1.302 ns | 1.16 | 0.02 | - | NA | + | ExtendedIntrinsic | 64 | 40.11 ns | 10.183 ns | 0.558 ns | 0.35 | 0.01 | - | NA | + | UseHwIntrinsics | 64 | 14.71 ns | 4.860 ns | 0.266 ns | 0.13 | 0.00 | - | NA | + | UseAvx2_Grouped | 64 | 20.23 ns | 11.619 ns | 0.637 ns | 0.18 | 0.00 | - | NA | + | | | | | | | | | | + | PixelOperations_Base | 256 | 387.94 ns | 31.591 ns | 1.732 ns | 1.00 | 0.00 | - | NA | + | PixelOperations_Specialized | 256 | 50.93 ns | 22.388 ns | 1.227 ns | 0.13 | 0.00 | - | NA | + | FallbackIntrinsics128 | 256 | 509.72 ns | 249.926 ns | 13.699 ns | 1.31 | 0.04 | - | NA | + | ExtendedIntrinsic | 256 | 140.32 ns | 9.353 ns | 0.513 ns | 0.36 | 0.00 | - | NA | + | UseHwIntrinsics | 256 | 41.99 ns | 16.000 ns | 0.877 ns | 0.11 | 0.00 | - | NA | + | UseAvx2_Grouped | 256 | 63.81 ns | 2.360 ns | 0.129 ns | 0.16 | 0.00 | - | NA | + | | | | | | | | | | + | PixelOperations_Base | 2048 | 2,979.49 ns | 2,023.706 ns | 110.926 ns | 1.00 | 0.00 | - | NA | + | PixelOperations_Specialized | 2048 | 326.19 ns | 19.077 ns | 1.046 ns | 0.11 | 0.00 | - | NA | + | FallbackIntrinsics128 | 2048 | 3,885.95 ns | 411.078 ns | 22.533 ns | 1.31 | 0.05 | - | NA | + | ExtendedIntrinsic | 2048 | 1,078.58 ns | 136.960 ns | 7.507 ns | 0.36 | 0.01 | - | NA | + | UseHwIntrinsics | 2048 | 312.07 ns | 68.662 ns | 3.764 ns | 0.10 | 0.00 | - | NA | + | UseAvx2_Grouped | 2048 | 451.83 ns | 41.742 ns | 2.288 ns | 0.15 | 0.01 | - | NA | + */ } diff --git a/tests/ImageSharp.Benchmarks/Bulk/FromVector4_Rgb24.cs b/tests/ImageSharp.Benchmarks/Bulk/FromVector4_Rgb24.cs index 27fab64dd..c6125ef8f 100644 --- a/tests/ImageSharp.Benchmarks/Bulk/FromVector4_Rgb24.cs +++ b/tests/ImageSharp.Benchmarks/Bulk/FromVector4_Rgb24.cs @@ -7,48 +7,26 @@ using SixLabors.ImageSharp.PixelFormats; namespace SixLabors.ImageSharp.Benchmarks.Bulk; [Config(typeof(Config.Short))] -public class FromVector4_Rgb24 : FromVector4 -{ -} +public class FromVector4_Rgb24 : FromVector4; -// 2020-11-02 -// ########## -// -// BenchmarkDotNet = v0.12.1, OS = Windows 10.0.19041.572(2004 /?/ 20H1) -// Intel Core i7-8650U CPU 1.90GHz (Kaby Lake R), 1 CPU, 8 logical and 4 physical cores -// .NET Core SDK=3.1.403 -// [Host] : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT -// Job-XYEQXL : .NET Framework 4.8 (4.8.4250.0), X64 RyuJIT -// Job-HSXNJV : .NET Core 2.1.23 (CoreCLR 4.6.29321.03, CoreFX 4.6.29321.01), X64 RyuJIT -// Job-YUREJO : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT -// -// IterationCount=3 LaunchCount=1 WarmupCount=3 -// -// | Method | Job | Runtime | Count | Mean | Error | StdDev | Ratio | RatioSD | Gen 0 | Gen 1 | Gen 2 | Allocated | -// |---------------------------- |----------- |-------------- |------ |-----------:|------------:|----------:|------:|--------:|-------:|------:|------:|----------:| -// | PixelOperations_Base | Job-XYEQXL | .NET 4.7.2 | 64 | 343.2 ns | 305.91 ns | 16.77 ns | 1.00 | 0.00 | 0.0057 | - | - | 24 B | -// | PixelOperations_Specialized | Job-XYEQXL | .NET 4.7.2 | 64 | 320.8 ns | 19.93 ns | 1.09 ns | 0.94 | 0.05 | - | - | - | - | -// | | | | | | | | | | | | | | -// | PixelOperations_Base | Job-HSXNJV | .NET Core 2.1 | 64 | 234.3 ns | 17.98 ns | 0.99 ns | 1.00 | 0.00 | 0.0052 | - | - | 24 B | -// | PixelOperations_Specialized | Job-HSXNJV | .NET Core 2.1 | 64 | 246.0 ns | 82.34 ns | 4.51 ns | 1.05 | 0.02 | - | - | - | - | -// | | | | | | | | | | | | | | -// | PixelOperations_Base | Job-YUREJO | .NET Core 3.1 | 64 | 222.3 ns | 39.46 ns | 2.16 ns | 1.00 | 0.00 | 0.0057 | - | - | 24 B | -// | PixelOperations_Specialized | Job-YUREJO | .NET Core 3.1 | 64 | 243.4 ns | 33.58 ns | 1.84 ns | 1.09 | 0.01 | - | - | - | - | -// | | | | | | | | | | | | | | -// | PixelOperations_Base | Job-XYEQXL | .NET 4.7.2 | 256 | 824.9 ns | 32.77 ns | 1.80 ns | 1.00 | 0.00 | 0.0057 | - | - | 24 B | -// | PixelOperations_Specialized | Job-XYEQXL | .NET 4.7.2 | 256 | 967.0 ns | 39.09 ns | 2.14 ns | 1.17 | 0.01 | 0.0172 | - | - | 72 B | -// | | | | | | | | | | | | | | -// | PixelOperations_Base | Job-HSXNJV | .NET Core 2.1 | 256 | 756.9 ns | 94.43 ns | 5.18 ns | 1.00 | 0.00 | 0.0048 | - | - | 24 B | -// | PixelOperations_Specialized | Job-HSXNJV | .NET Core 2.1 | 256 | 1,003.3 ns | 3,192.09 ns | 174.97 ns | 1.32 | 0.22 | 0.0172 | - | - | 72 B | -// | | | | | | | | | | | | | | -// | PixelOperations_Base | Job-YUREJO | .NET Core 3.1 | 256 | 748.6 ns | 248.03 ns | 13.60 ns | 1.00 | 0.00 | 0.0057 | - | - | 24 B | -// | PixelOperations_Specialized | Job-YUREJO | .NET Core 3.1 | 256 | 437.0 ns | 36.48 ns | 2.00 ns | 0.58 | 0.01 | 0.0172 | - | - | 72 B | -// | | | | | | | | | | | | | | -// | PixelOperations_Base | Job-XYEQXL | .NET 4.7.2 | 2048 | 5,751.6 ns | 704.24 ns | 38.60 ns | 1.00 | 0.00 | - | - | - | 24 B | -// | PixelOperations_Specialized | Job-XYEQXL | .NET 4.7.2 | 2048 | 4,391.6 ns | 718.17 ns | 39.37 ns | 0.76 | 0.00 | 0.0153 | - | - | 72 B | -// | | | | | | | | | | | | | | -// | PixelOperations_Base | Job-HSXNJV | .NET Core 2.1 | 2048 | 6,202.0 ns | 1,815.18 ns | 99.50 ns | 1.00 | 0.00 | - | - | - | 24 B | -// | PixelOperations_Specialized | Job-HSXNJV | .NET Core 2.1 | 2048 | 4,225.6 ns | 1,004.03 ns | 55.03 ns | 0.68 | 0.01 | 0.0153 | - | - | 72 B | -// | | | | | | | | | | | | | | -// | PixelOperations_Base | Job-YUREJO | .NET Core 3.1 | 2048 | 6,157.1 ns | 2,516.98 ns | 137.96 ns | 1.00 | 0.00 | - | - | - | 24 B | -// | PixelOperations_Specialized | Job-YUREJO | .NET Core 3.1 | 2048 | 1,822.7 ns | 1,764.43 ns | 96.71 ns | 0.30 | 0.02 | 0.0172 | - | - | 72 B | +/* + BenchmarkDotNet v0.13.10, Windows 11 (10.0.22631.3085/23H2/2023Update/SunValley3) +11th Gen Intel Core i7-11370H 3.30GHz, 1 CPU, 8 logical and 4 physical cores +.NET SDK 8.0.200-preview.23624.5 + [Host] : .NET 8.0.1 (8.0.123.58001), X64 RyuJIT AVX2 + Job-NEHCEM : .NET 8.0.1 (8.0.123.58001), X64 RyuJIT AVX2 + +Runtime=.NET 8.0 Arguments=/p:DebugType=portable IterationCount=3 +LaunchCount=1 WarmupCount=3 + +| Method | Count | Mean | Error | StdDev | Ratio | Gen0 | Allocated | Alloc Ratio | +|---------------------------- |------ |------------:|----------:|---------:|------:|-------:|----------:|------------:| +| PixelOperations_Base | 64 | 95.87 ns | 13.60 ns | 0.745 ns | 1.00 | - | - | NA | +| PixelOperations_Specialized | 64 | 97.34 ns | 30.34 ns | 1.663 ns | 1.02 | - | - | NA | +| | | | | | | | | | +| PixelOperations_Base | 256 | 337.80 ns | 88.10 ns | 4.829 ns | 1.00 | - | - | NA | +| PixelOperations_Specialized | 256 | 195.07 ns | 30.54 ns | 1.674 ns | 0.58 | 0.0153 | 96 B | NA | +| | | | | | | | | | +| PixelOperations_Base | 2048 | 2,561.79 ns | 162.45 ns | 8.905 ns | 1.00 | - | - | NA | +| PixelOperations_Specialized | 2048 | 741.85 ns | 18.05 ns | 0.989 ns | 0.29 | 0.0153 | 96 B | NA | + */ diff --git a/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs b/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs index c81eaea63..200371679 100644 --- a/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs +++ b/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs @@ -112,6 +112,7 @@ public partial class SimdUtilsTests public static readonly TheoryData ArraySizesDivisibleBy4 = new() { 0, 4, 8, 28, 1020 }; public static readonly TheoryData ArraySizesDivisibleBy3 = new() { 0, 3, 9, 36, 957 }; public static readonly TheoryData ArraySizesDivisibleBy32 = new() { 0, 32, 512 }; + public static readonly TheoryData ArraySizesDivisibleBy64 = new() { 0, 64, 512 }; public static readonly TheoryData ArbitraryArraySizes = new() { 0, 1, 2, 3, 4, 7, 8, 9, 15, 16, 17, 63, 64, 255, 511, 512, 513, 514, 515, 516, 517, 518, 519, 520 }; @@ -199,7 +200,7 @@ public partial class SimdUtilsTests } [Theory] - [MemberData(nameof(ArraySizesDivisibleBy32))] + [MemberData(nameof(ArraySizesDivisibleBy64))] public void HwIntrinsics_BulkConvertNormalizedFloatToByteClampOverflows(int count) { if (!Sse2.IsSupported) @@ -214,7 +215,7 @@ public partial class SimdUtilsTests FeatureTestRunner.RunWithHwIntrinsicsFeature( RunTest, count, - HwIntrinsics.AllowAll | HwIntrinsics.DisableAVX2); + HwIntrinsics.AllowAll | HwIntrinsics.DisableAVX512BW | HwIntrinsics.DisableAVX2); } [Theory] diff --git a/tests/ImageSharp.Tests/TestUtilities/BasicSerializer.cs b/tests/ImageSharp.Tests/TestUtilities/BasicSerializer.cs index d161b8019..216ed95b8 100644 --- a/tests/ImageSharp.Tests/TestUtilities/BasicSerializer.cs +++ b/tests/ImageSharp.Tests/TestUtilities/BasicSerializer.cs @@ -13,14 +13,14 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities; /// internal class BasicSerializer : IXunitSerializationInfo { - private readonly Dictionary map = new Dictionary(); + private readonly Dictionary map = []; public const char Separator = ':'; private string DumpToString(Type type) { - using var ms = new MemoryStream(); - using var writer = new StreamWriter(ms); + using MemoryStream ms = new(); + using StreamWriter writer = new(ms); writer.WriteLine(type.FullName); foreach (KeyValuePair kv in this.map) { @@ -29,16 +29,16 @@ internal class BasicSerializer : IXunitSerializationInfo writer.Flush(); byte[] data = ms.ToArray(); - return System.Convert.ToBase64String(data); + return Convert.ToBase64String(data); } private Type LoadDump(string dump) { - byte[] data = System.Convert.FromBase64String(dump); + byte[] data = Convert.FromBase64String(dump); - using var ms = new MemoryStream(data); - using var reader = new StreamReader(ms); - var type = Type.GetType(reader.ReadLine()); + using MemoryStream ms = new(data); + using StreamReader reader = new(ms); + Type type = Type.GetType(reader.ReadLine()); for (string s = reader.ReadLine(); s != null; s = reader.ReadLine()) { string[] kv = s.Split(Separator); @@ -50,7 +50,7 @@ internal class BasicSerializer : IXunitSerializationInfo public static string Serialize(IXunitSerializable serializable) { - var serializer = new BasicSerializer(); + BasicSerializer serializer = new(); serializable.Serialize(serializer); return serializer.DumpToString(serializable.GetType()); } @@ -58,10 +58,10 @@ internal class BasicSerializer : IXunitSerializationInfo public static T Deserialize(string dump) where T : IXunitSerializable { - var serializer = new BasicSerializer(); + BasicSerializer serializer = new(); Type type = serializer.LoadDump(dump); - var result = (T)Activator.CreateInstance(type); + T result = (T)Activator.CreateInstance(type); result.Deserialize(serializer); return result; } diff --git a/tests/ImageSharp.Tests/TestUtilities/FeatureTesting/FeatureTestRunner.cs b/tests/ImageSharp.Tests/TestUtilities/FeatureTesting/FeatureTestRunner.cs index 5a9a72f96..07ad5e8f0 100644 --- a/tests/ImageSharp.Tests/TestUtilities/FeatureTesting/FeatureTestRunner.cs +++ b/tests/ImageSharp.Tests/TestUtilities/FeatureTesting/FeatureTestRunner.cs @@ -2,6 +2,7 @@ // Licensed under the Six Labors Split License. using System.Diagnostics; +using System.Globalization; using Microsoft.DotNet.RemoteExecutor; using Xunit.Abstractions; @@ -12,7 +13,7 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities; /// public static class FeatureTestRunner { - private static readonly char[] SplitChars = { ',', ' ' }; + private static readonly char[] SplitChars = [',', ' ']; /// /// Allows the deserialization of parameters passed to the feature test. @@ -40,7 +41,7 @@ public static class FeatureTestRunner /// The value. public static T Deserialize(string value) where T : IConvertible - => (T)Convert.ChangeType(value, typeof(T)); + => (T)Convert.ChangeType(value, typeof(T), CultureInfo.InvariantCulture); /// /// Runs the given test within an environment @@ -127,6 +128,7 @@ public static class FeatureTestRunner /// Runs the given test within an environment /// where the given features. /// + /// The type of argument. /// The test action to run. /// The intrinsics features. /// The value to pass as a parameter to the test action. @@ -170,6 +172,7 @@ public static class FeatureTestRunner /// Runs the given test within an environment /// where the given features. /// + /// The type of argument. /// The test action to run. /// The intrinsics features. /// The value to pass as a parameter to the test action. @@ -214,6 +217,8 @@ public static class FeatureTestRunner /// Runs the given test within an environment /// where the given features. /// + /// The type of argument. + /// The addition type of argument. /// The test action to run. /// The intrinsics features. /// The value to pass as a parameter to the test action. @@ -261,6 +266,7 @@ public static class FeatureTestRunner /// Runs the given test within an environment /// where the given features. /// + /// The type of argument. /// The test action to run. /// The intrinsics features. /// The value to pass as a parameter to the test action. @@ -307,6 +313,7 @@ public static class FeatureTestRunner /// Runs the given test within an environment /// where the given features. /// + /// The type of argument. /// The test action to run. /// The value to pass as a parameter to the test action. /// The intrinsics features. @@ -350,6 +357,7 @@ public static class FeatureTestRunner /// Runs the given test within an environment /// where the given features. /// + /// The type of argument. /// The test action to run. /// The value to pass as a parameter #0 to the test action. /// The value to pass as a parameter #1 to the test action. @@ -395,10 +403,10 @@ public static class FeatureTestRunner internal static Dictionary ToFeatureKeyValueCollection(this HwIntrinsics intrinsics) { // Loop through and translate the given values into COMPlus equivalents - Dictionary features = new(); + Dictionary features = []; foreach (string intrinsic in intrinsics.ToString("G").Split(SplitChars, StringSplitOptions.RemoveEmptyEntries)) { - HwIntrinsics key = (HwIntrinsics)Enum.Parse(typeof(HwIntrinsics), intrinsic); + HwIntrinsics key = Enum.Parse(intrinsic); switch (intrinsic) { case nameof(HwIntrinsics.AllowAll): @@ -418,40 +426,47 @@ public static class FeatureTestRunner } /// -/// See -/// -/// ends up impacting all SIMD support(including System.Numerics) -/// but not things like , , and . -/// +/// See /// [Flags] #pragma warning disable RCS1135 // Declare enum member with zero value (when enum has FlagsAttribute). -public enum HwIntrinsics +public enum HwIntrinsics : long #pragma warning restore RCS1135 // Declare enum member with zero value (when enum has FlagsAttribute). { // Use flags so we can pass multiple values without using params. // Don't base on 0 or use inverse for All as that doesn't translate to string values. - DisableHWIntrinsic = 1 << 0, - DisableSSE = 1 << 1, - DisableSSE2 = 1 << 2, - DisableAES = 1 << 3, - DisablePCLMULQDQ = 1 << 4, - DisableSSE3 = 1 << 5, - DisableSSSE3 = 1 << 6, - DisableSSE41 = 1 << 7, - DisableSSE42 = 1 << 8, - DisablePOPCNT = 1 << 9, - DisableAVX = 1 << 10, - DisableFMA = 1 << 11, - DisableAVX2 = 1 << 12, - DisableBMI1 = 1 << 13, - DisableBMI2 = 1 << 14, - DisableLZCNT = 1 << 15, - DisableArm64AdvSimd = 1 << 16, - DisableArm64Crc32 = 1 << 17, - DisableArm64Dp = 1 << 18, - DisableArm64Aes = 1 << 19, - DisableArm64Sha1 = 1 << 20, - DisableArm64Sha256 = 1 << 21, - AllowAll = 1 << 22 + DisableHWIntrinsic = 1L << 0, + DisableSSE = 1L << 1, + DisableSSE2 = 1L << 2, + DisableAES = 1L << 3, + DisablePCLMULQDQ = 1L << 4, + DisableSSE3 = 1L << 5, + DisableSSSE3 = 1L << 6, + DisableSSE41 = 1L << 7, + DisableSSE42 = 1L << 8, + DisablePOPCNT = 1L << 9, + DisableAVX = 1L << 10, + DisableFMA = 1L << 11, + DisableAVX2 = 1L << 12, + DisableAVXVNNI = 1L << 13, + DisableAVX512BW = 1L << 14, + DisableAVX512BW_VL = 1L << 15, + DisableAVX512CD = 1L << 16, + DisableAVX512CD_VL = 1L << 17, + DisableAVX512DQ = 1L << 18, + DisableAVX512DQ_VL = 1L << 19, + DisableAVX512F = 1L << 20, + DisableAVX512F_VL = 1L << 21, + DisableAVX512VBMI = 1L << 22, + DisableAVX512VBMI_VL = 1L << 23, + DisableBMI1 = 1L << 24, + DisableBMI2 = 1L << 25, + DisableLZCNT = 1L << 26, + DisableArm64AdvSimd = 1L << 27, + DisableArm64Crc32 = 1L << 28, + DisableArm64Dp = 1L << 29, + DisableArm64Aes = 1L << 30, + DisableArm64Sha1 = 1L << 31, + DisableArm64Sha256 = 1L << 32, + AllowAll = 1L << 33 } From 89cd8492f1aef9b233f3b6b203efa4083cc04215 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 2 Feb 2024 18:59:27 +1000 Subject: [PATCH 10/37] Remove unused methods --- .../Helpers/SimdUtils.ExtendedIntrinsics.cs | 144 ------------------ .../SimdUtils.FallbackIntrinsics128.cs | 61 -------- .../ImageSharp.Benchmarks/Bulk/FromVector4.cs | 18 --- .../Bulk/ToVector4_Rgba32.cs | 9 -- .../ImageSharp.Tests/Common/SimdUtilsTests.cs | 18 --- 5 files changed, 250 deletions(-) diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.ExtendedIntrinsics.cs b/src/ImageSharp/Common/Helpers/SimdUtils.ExtendedIntrinsics.cs index 7d07dcaae..3c2f189cf 100644 --- a/src/ImageSharp/Common/Helpers/SimdUtils.ExtendedIntrinsics.cs +++ b/src/ImageSharp/Common/Helpers/SimdUtils.ExtendedIntrinsics.cs @@ -3,7 +3,6 @@ using System.Numerics; using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; // ReSharper disable MemberHidesStaticFromOuterClass namespace SixLabors.ImageSharp; @@ -35,148 +34,5 @@ internal static partial class SimdUtils dest1 = Vector.ConvertToSingle(i1); dest2 = Vector.ConvertToSingle(i2); } - - /// - /// as many elements as possible, slicing them down (keeping the remainder). - /// - [MethodImpl(InliningOptions.ShortMethod)] - internal static void ByteToNormalizedFloatReduce( - ref ReadOnlySpan source, - ref Span dest) - { - DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same length!"); - - if (!IsAvailable) - { - return; - } - - int remainder = Numerics.ModuloP2(source.Length, Vector.Count); - int adjustedCount = source.Length - remainder; - - if (adjustedCount > 0) - { - ByteToNormalizedFloat(source[..adjustedCount], dest[..adjustedCount]); - - source = source[adjustedCount..]; - dest = dest[adjustedCount..]; - } - } - - /// - /// as many elements as possible, slicing them down (keeping the remainder). - /// - [MethodImpl(InliningOptions.ShortMethod)] - internal static void NormalizedFloatToByteSaturateReduce( - ref ReadOnlySpan source, - ref Span dest) - { - DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same length!"); - - if (!IsAvailable) - { - return; - } - - int remainder = Numerics.ModuloP2(source.Length, Vector.Count); - int adjustedCount = source.Length - remainder; - - if (adjustedCount > 0) - { - NormalizedFloatToByteSaturate(source[..adjustedCount], dest[..adjustedCount]); - - source = source[adjustedCount..]; - dest = dest[adjustedCount..]; - } - } - - /// - /// Implementation , which is faster on new RyuJIT runtime. - /// - internal static void ByteToNormalizedFloat(ReadOnlySpan source, Span dest) - { - DebugVerifySpanInput(source, dest, Vector.Count); - - nuint n = dest.VectorCount(); - - ref Vector sourceBase = ref Unsafe.As>(ref MemoryMarshal.GetReference(source)); - ref Vector destBase = ref Unsafe.As>(ref MemoryMarshal.GetReference(dest)); - - for (nuint i = 0; i < n; i++) - { - Vector b = Unsafe.Add(ref sourceBase, i); - - Vector.Widen(b, out Vector s0, out Vector s1); - Vector.Widen(s0, out Vector w0, out Vector w1); - Vector.Widen(s1, out Vector w2, out Vector w3); - - Vector f0 = ConvertToSingle(w0); - Vector f1 = ConvertToSingle(w1); - Vector f2 = ConvertToSingle(w2); - Vector f3 = ConvertToSingle(w3); - - ref Vector d = ref Unsafe.Add(ref destBase, i * 4); - d = f0; - Unsafe.Add(ref d, 1) = f1; - Unsafe.Add(ref d, 2) = f2; - Unsafe.Add(ref d, 3) = f3; - } - } - - /// - /// Implementation of , which is faster on new .NET runtime. - /// - internal static void NormalizedFloatToByteSaturate( - ReadOnlySpan source, - Span dest) - { - DebugVerifySpanInput(source, dest, Vector.Count); - - nuint n = dest.VectorCount(); - - ref Vector sourceBase = - ref Unsafe.As>(ref MemoryMarshal.GetReference(source)); - ref Vector destBase = ref Unsafe.As>(ref MemoryMarshal.GetReference(dest)); - - for (nuint i = 0; i < n; i++) - { - ref Vector s = ref Unsafe.Add(ref sourceBase, i * 4); - - Vector f0 = s; - Vector f1 = Unsafe.Add(ref s, 1); - Vector f2 = Unsafe.Add(ref s, 2); - Vector f3 = Unsafe.Add(ref s, 3); - - Vector w0 = ConvertToUInt32(f0); - Vector w1 = ConvertToUInt32(f1); - Vector w2 = ConvertToUInt32(f2); - Vector w3 = ConvertToUInt32(f3); - - var u0 = Vector.Narrow(w0, w1); - var u1 = Vector.Narrow(w2, w3); - - Unsafe.Add(ref destBase, i) = Vector.Narrow(u0, u1); - } - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static Vector ConvertToUInt32(Vector vf) - { - var maxBytes = new Vector(255f); - vf *= maxBytes; - vf += new Vector(0.5f); - vf = Vector.Min(Vector.Max(vf, Vector.Zero), maxBytes); - var vi = Vector.ConvertToInt32(vf); - return Vector.AsVectorUInt32(vi); - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static Vector ConvertToSingle(Vector u) - { - var vi = Vector.AsVectorInt32(u); - var v = Vector.ConvertToSingle(vi); - v *= new Vector(1f / 255f); - return v; - } } } diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.FallbackIntrinsics128.cs b/src/ImageSharp/Common/Helpers/SimdUtils.FallbackIntrinsics128.cs index 90b313fb9..fcf441c47 100644 --- a/src/ImageSharp/Common/Helpers/SimdUtils.FallbackIntrinsics128.cs +++ b/src/ImageSharp/Common/Helpers/SimdUtils.FallbackIntrinsics128.cs @@ -39,30 +39,6 @@ internal static partial class SimdUtils } } - /// - /// as many elements as possible, slicing them down (keeping the remainder). - /// - [MethodImpl(InliningOptions.ShortMethod)] - internal static void NormalizedFloatToByteSaturateReduce( - ref ReadOnlySpan source, - ref Span dest) - { - DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same length!"); - - int remainder = Numerics.Modulo4(source.Length); - int adjustedCount = source.Length - remainder; - - if (adjustedCount > 0) - { - NormalizedFloatToByteSaturate( - source[..adjustedCount], - dest[..adjustedCount]); - - source = source[adjustedCount..]; - dest = dest[adjustedCount..]; - } - } - /// /// Implementation of using . /// @@ -95,43 +71,6 @@ internal static partial class SimdUtils } } - /// - /// Implementation of using . - /// - [MethodImpl(InliningOptions.ColdPath)] - internal static void NormalizedFloatToByteSaturate( - ReadOnlySpan source, - Span dest) - { - DebugVerifySpanInput(source, dest, 4); - - uint count = (uint)source.Length / 4; - if (count == 0) - { - return; - } - - ref Vector4 sBase = ref Unsafe.As(ref MemoryMarshal.GetReference(source)); - ref ByteVector4 dBase = ref Unsafe.As(ref MemoryMarshal.GetReference(dest)); - - var half = new Vector4(0.5f); - var maxBytes = new Vector4(255f); - - for (nuint i = 0; i < count; i++) - { - Vector4 s = Unsafe.Add(ref sBase, i); - s *= maxBytes; - s += half; - s = Numerics.Clamp(s, Vector4.Zero, maxBytes); - - ref ByteVector4 d = ref Unsafe.Add(ref dBase, i); - d.X = (byte)s.X; - d.Y = (byte)s.Y; - d.Z = (byte)s.Z; - d.W = (byte)s.W; - } - } - [StructLayout(LayoutKind.Sequential)] private struct ByteVector4 { diff --git a/tests/ImageSharp.Benchmarks/Bulk/FromVector4.cs b/tests/ImageSharp.Benchmarks/Bulk/FromVector4.cs index dff687fa1..53c26a57e 100644 --- a/tests/ImageSharp.Benchmarks/Bulk/FromVector4.cs +++ b/tests/ImageSharp.Benchmarks/Bulk/FromVector4.cs @@ -64,24 +64,6 @@ public abstract class FromVector4 public class FromVector4Rgba32 : FromVector4 { - [Benchmark] - public void FallbackIntrinsics128() - { - Span sBytes = MemoryMarshal.Cast(this.Source.GetSpan()); - Span dFloats = MemoryMarshal.Cast(this.Destination.GetSpan()); - - SimdUtils.FallbackIntrinsics128.NormalizedFloatToByteSaturate(sBytes, dFloats); - } - - [Benchmark] - public void ExtendedIntrinsic() - { - Span sBytes = MemoryMarshal.Cast(this.Source.GetSpan()); - Span dFloats = MemoryMarshal.Cast(this.Destination.GetSpan()); - - SimdUtils.ExtendedIntrinsics.NormalizedFloatToByteSaturate(sBytes, dFloats); - } - [Benchmark] public void UseHwIntrinsics() { diff --git a/tests/ImageSharp.Benchmarks/Bulk/ToVector4_Rgba32.cs b/tests/ImageSharp.Benchmarks/Bulk/ToVector4_Rgba32.cs index 1ceae8414..9c7ecbc49 100644 --- a/tests/ImageSharp.Benchmarks/Bulk/ToVector4_Rgba32.cs +++ b/tests/ImageSharp.Benchmarks/Bulk/ToVector4_Rgba32.cs @@ -30,15 +30,6 @@ public class ToVector4_Rgba32 : ToVector4 this.source.GetSpan(), this.destination.GetSpan()); - [Benchmark] - public void ExtendedIntrinsics() - { - Span sBytes = MemoryMarshal.Cast(this.source.GetSpan()); - Span dFloats = MemoryMarshal.Cast(this.destination.GetSpan()); - - SimdUtils.ExtendedIntrinsics.ByteToNormalizedFloat(sBytes, dFloats); - } - [Benchmark] public void HwIntrinsics() { diff --git a/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs b/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs index 200371679..e9e4550b0 100644 --- a/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs +++ b/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs @@ -122,12 +122,6 @@ public partial class SimdUtilsTests count, (s, d) => SimdUtils.FallbackIntrinsics128.ByteToNormalizedFloat(s.Span, d.Span)); - [Theory] - [MemberData(nameof(ArraySizesDivisibleBy32))] - public void ExtendedIntrinsics_BulkConvertByteToNormalizedFloat(int count) => TestImpl_BulkConvertByteToNormalizedFloat( - count, - (s, d) => SimdUtils.ExtendedIntrinsics.ByteToNormalizedFloat(s.Span, d.Span)); - [Theory] [MemberData(nameof(ArraySizesDivisibleBy32))] public void HwIntrinsics_BulkConvertByteToNormalizedFloat(int count) @@ -166,18 +160,6 @@ public partial class SimdUtilsTests Assert.Equal(expected, result, new ApproximateFloatComparer(1e-5f)); } - [Theory] - [MemberData(nameof(ArraySizesDivisibleBy4))] - public void FallbackIntrinsics128_BulkConvertNormalizedFloatToByteClampOverflows(int count) => TestImpl_BulkConvertNormalizedFloatToByteClampOverflows( - count, - (s, d) => SimdUtils.FallbackIntrinsics128.NormalizedFloatToByteSaturate(s.Span, d.Span)); - - [Theory] - [MemberData(nameof(ArraySizesDivisibleBy32))] - public void ExtendedIntrinsics_BulkConvertNormalizedFloatToByteClampOverflows(int count) => TestImpl_BulkConvertNormalizedFloatToByteClampOverflows( - count, - (s, d) => SimdUtils.ExtendedIntrinsics.NormalizedFloatToByteSaturate(s.Span, d.Span)); - [Theory] [InlineData(1234)] public void ExtendedIntrinsics_ConvertToSingle(short scale) From 68b7dbcf58d7a944197a78bd40236c9d4057840b Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 2 Feb 2024 19:48:20 +1000 Subject: [PATCH 11/37] Update src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs Co-authored-by: Clinton Ingram --- src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs b/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs index feb55ebe5..df844582a 100644 --- a/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs +++ b/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs @@ -49,10 +49,7 @@ internal static partial class SimdUtils 0, 1, 2, 4, 5, 6, 8, 9, 10, 12, 13, 14, 3, 7, 11, 15); [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static Vector256 PermuteMaskShiftAlpha8x32() - => Vector256.Create( - 0, 0, 0, 0, 1, 0, 0, 0, 2, 0, 0, 0, 4, 0, 0, 0, - 5, 0, 0, 0, 6, 0, 0, 0, 3, 0, 0, 0, 7, 0, 0, 0).AsUInt32(); + public static Vector256 PermuteMaskShiftAlpha8x32() => Vector256.Create(0u, 1, 2, 4, 5, 6, 3, 7); #pragma warning restore SA1137 // Elements should have the same indentation #pragma warning restore SA1117 // Parameters should be on same line or separate lines From 9d737b71f758bd764e68c562db9e3a0d72221b21 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 2 Feb 2024 19:51:01 +1000 Subject: [PATCH 12/37] Avoid downclocking --- src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs b/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs index feb55ebe5..f5fa73cf0 100644 --- a/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs +++ b/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs @@ -896,7 +896,9 @@ internal static partial class SimdUtils { DebugGuard.IsTrue(source.Length == destination.Length, nameof(source), "Input spans must be of same length!"); - if (Avx512BW.IsSupported || Avx2.IsSupported || Sse2.IsSupported || AdvSimd.IsSupported) + if ((Vector512.IsHardwareAccelerated && Avx512BW.IsSupported) || + (Vector256.IsHardwareAccelerated && Avx2.IsSupported) || + (Vector128.IsHardwareAccelerated && (Sse2.IsSupported || AdvSimd.IsSupported))) { int remainder; From ecdd12e7b58ae6ef01947b849f844ca7e5b0b254 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 5 Feb 2024 09:10:56 +0000 Subject: [PATCH 13/37] Bump codecov/codecov-action from 3 to 4 Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 3 to 4. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/codecov/codecov-action/compare/v3...v4) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/code-coverage.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/code-coverage.yml b/.github/workflows/code-coverage.yml index 62b6477ee..19a9689ce 100644 --- a/.github/workflows/code-coverage.yml +++ b/.github/workflows/code-coverage.yml @@ -81,7 +81,7 @@ jobs: path: tests/Images/ActualOutput/ - name: Codecov Update - uses: codecov/codecov-action@v3 + uses: codecov/codecov-action@v4 if: matrix.options.codecov == true && startsWith(github.repository, 'SixLabors') with: flags: unittests From c6758df08b193cca5243ec030db5305c0846c3dd Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 6 Feb 2024 16:22:50 +1000 Subject: [PATCH 14/37] Optimize and cleanup ByteToNormalizedFloatReduce --- src/ImageSharp/Common/Helpers/Numerics.cs | 20 ++ .../Common/Helpers/SimdUtils.Convert.cs | 77 +++++++ .../Helpers/SimdUtils.ExtendedIntrinsics.cs | 38 ---- .../SimdUtils.FallbackIntrinsics128.cs | 83 -------- .../Common/Helpers/SimdUtils.HwIntrinsics.cs | 190 +++++++++++------- src/ImageSharp/Common/Helpers/SimdUtils.cs | 97 --------- .../Common/Helpers/Vector128Utilities.cs | 13 +- .../Formats/Jpeg/Components/Block8x8F.cs | 42 ++-- .../Utils/Vector4Converters.RgbaCompatible.cs | 75 ++++--- tests/ImageSharp.Benchmarks/Bulk/ToVector4.cs | 25 +-- .../Bulk/ToVector4_Bgra32.cs | 4 +- .../Bulk/ToVector4_Rgb24.cs | 4 +- .../Bulk/ToVector4_Rgba32.cs | 63 +++--- .../LoadResizeSave/README.md | 2 +- .../ImageSharp.Tests/Common/SimdUtilsTests.cs | 36 +--- 15 files changed, 344 insertions(+), 425 deletions(-) create mode 100644 src/ImageSharp/Common/Helpers/SimdUtils.Convert.cs delete mode 100644 src/ImageSharp/Common/Helpers/SimdUtils.ExtendedIntrinsics.cs delete mode 100644 src/ImageSharp/Common/Helpers/SimdUtils.FallbackIntrinsics128.cs diff --git a/src/ImageSharp/Common/Helpers/Numerics.cs b/src/ImageSharp/Common/Helpers/Numerics.cs index 5f85734e8..777de2dc9 100644 --- a/src/ImageSharp/Common/Helpers/Numerics.cs +++ b/src/ImageSharp/Common/Helpers/Numerics.cs @@ -1069,4 +1069,24 @@ internal static class Numerics public static nuint Vector256Count(int length) where TVector : struct => (uint)length / (uint)Vector256.Count; + + /// + /// Gets the count of vectors that safely fit into the given span. + /// + /// The type of the vector. + /// The given span. + /// Count of vectors that safely fit into the span. + public static nuint Vector512Count(this Span span) + where TVector : struct + => (uint)span.Length / (uint)Vector512.Count; + + /// + /// Gets the count of vectors that safely fit into length. + /// + /// The type of the vector. + /// The given length. + /// Count of vectors that safely fit into the length. + public static nuint Vector512Count(int length) + where TVector : struct + => (uint)length / (uint)Vector512.Count; } diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.Convert.cs b/src/ImageSharp/Common/Helpers/SimdUtils.Convert.cs new file mode 100644 index 000000000..1b5a418de --- /dev/null +++ b/src/ImageSharp/Common/Helpers/SimdUtils.Convert.cs @@ -0,0 +1,77 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; + +namespace SixLabors.ImageSharp; + +internal static partial class SimdUtils +{ + /// + /// Converts all input -s to -s normalized into [0..1]. + /// should be the of the same size as , + /// but there are no restrictions on the span's length. + /// + /// The source span of bytes + /// The destination span of floats + [MethodImpl(InliningOptions.ShortMethod)] + internal static void ByteToNormalizedFloat(ReadOnlySpan source, Span destination) + { + DebugGuard.IsTrue(source.Length == destination.Length, nameof(source), "Input spans must be of same length!"); + + HwIntrinsics.ByteToNormalizedFloatReduce(ref source, ref destination); + + if (source.Length > 0) + { + ConvertByteToNormalizedFloatRemainder(source, destination); + } + } + + /// + /// Convert all values normalized into [0..1] from 'source' into 'destination' buffer of . + /// The values are scaled up into [0-255] and rounded, overflows are clamped. + /// should be the of the same size as , + /// but there are no restrictions on the span's length. + /// + /// The source span of floats + /// The destination span of bytes + [MethodImpl(InliningOptions.ShortMethod)] + internal static void NormalizedFloatToByteSaturate(ReadOnlySpan source, Span destination) + { + DebugGuard.IsTrue(source.Length == destination.Length, nameof(source), "Input spans must be of same length!"); + + HwIntrinsics.NormalizedFloatToByteSaturateReduce(ref source, ref destination); + + if (source.Length > 0) + { + ConvertNormalizedFloatToByteRemainder(source, destination); + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ConvertByteToNormalizedFloatRemainder(ReadOnlySpan source, Span destination) + { + ref byte sBase = ref MemoryMarshal.GetReference(source); + ref float dBase = ref MemoryMarshal.GetReference(destination); + + for (int i = 0; i < source.Length; i++) + { + Unsafe.Add(ref dBase, i) = Unsafe.Add(ref sBase, i) / 255f; + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ConvertNormalizedFloatToByteRemainder(ReadOnlySpan source, Span destination) + { + ref float sBase = ref MemoryMarshal.GetReference(source); + ref byte dBase = ref MemoryMarshal.GetReference(destination); + for (int i = 0; i < source.Length; i++) + { + Unsafe.Add(ref dBase, i) = ConvertToByte(Unsafe.Add(ref sBase, i)); + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static byte ConvertToByte(float f) => (byte)Numerics.Clamp((f * 255F) + 0.5F, 0, 255F); +} diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.ExtendedIntrinsics.cs b/src/ImageSharp/Common/Helpers/SimdUtils.ExtendedIntrinsics.cs deleted file mode 100644 index 3c2f189cf..000000000 --- a/src/ImageSharp/Common/Helpers/SimdUtils.ExtendedIntrinsics.cs +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright (c) Six Labors. -// Licensed under the Six Labors Split License. - -using System.Numerics; -using System.Runtime.CompilerServices; - -// ReSharper disable MemberHidesStaticFromOuterClass -namespace SixLabors.ImageSharp; - -internal static partial class SimdUtils -{ - /// - /// Implementation methods based on newer API-s (Vector.Widen, Vector.Narrow, Vector.ConvertTo*). - /// Only accelerated only on RyuJIT having dotnet/coreclr#10662 merged (.NET Core 2.1+ .NET 4.7.2+) - /// See: - /// https://github.com/dotnet/coreclr/pull/10662 - /// API Proposal: - /// https://github.com/dotnet/corefx/issues/15957 - /// - public static class ExtendedIntrinsics - { - public static bool IsAvailable { get; } = Vector.IsHardwareAccelerated; - - /// - /// Widen and convert a vector of values into 2 vectors of -s. - /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static void ConvertToSingle( - Vector source, - out Vector dest1, - out Vector dest2) - { - Vector.Widen(source, out Vector i1, out Vector i2); - dest1 = Vector.ConvertToSingle(i1); - dest2 = Vector.ConvertToSingle(i2); - } - } -} diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.FallbackIntrinsics128.cs b/src/ImageSharp/Common/Helpers/SimdUtils.FallbackIntrinsics128.cs deleted file mode 100644 index fcf441c47..000000000 --- a/src/ImageSharp/Common/Helpers/SimdUtils.FallbackIntrinsics128.cs +++ /dev/null @@ -1,83 +0,0 @@ -// Copyright (c) Six Labors. -// Licensed under the Six Labors Split License. - -using System.Numerics; -using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; - -// ReSharper disable MemberHidesStaticFromOuterClass -namespace SixLabors.ImageSharp; - -internal static partial class SimdUtils -{ - /// - /// Fallback implementation based on (128bit). - /// For , efficient software fallback implementations are present, - /// and we hope that even mono's JIT is able to emit SIMD instructions for that type :P - /// - public static class FallbackIntrinsics128 - { - /// - /// as many elements as possible, slicing them down (keeping the remainder). - /// - [MethodImpl(InliningOptions.ShortMethod)] - internal static void ByteToNormalizedFloatReduce( - ref ReadOnlySpan source, - ref Span dest) - { - DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same length!"); - - int remainder = Numerics.Modulo4(source.Length); - int adjustedCount = source.Length - remainder; - - if (adjustedCount > 0) - { - ByteToNormalizedFloat(source[..adjustedCount], dest[..adjustedCount]); - - source = source[adjustedCount..]; - dest = dest[adjustedCount..]; - } - } - - /// - /// Implementation of using . - /// - [MethodImpl(InliningOptions.ColdPath)] - internal static void ByteToNormalizedFloat(ReadOnlySpan source, Span dest) - { - DebugVerifySpanInput(source, dest, 4); - - uint count = (uint)dest.Length / 4; - if (count == 0) - { - return; - } - - ref ByteVector4 sBase = ref Unsafe.As(ref MemoryMarshal.GetReference(source)); - ref Vector4 dBase = ref Unsafe.As(ref MemoryMarshal.GetReference(dest)); - - const float scale = 1f / 255f; - Vector4 d = default; - - for (nuint i = 0; i < count; i++) - { - ref ByteVector4 s = ref Unsafe.Add(ref sBase, i); - d.X = s.X; - d.Y = s.Y; - d.Z = s.Z; - d.W = s.W; - d *= scale; - Unsafe.Add(ref dBase, i) = d; - } - } - - [StructLayout(LayoutKind.Sequential)] - private struct ByteVector4 - { - public byte X; - public byte Y; - public byte Z; - public byte W; - } - } -} diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs b/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs index 487331360..6f0b4b4e3 100644 --- a/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs +++ b/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs @@ -752,17 +752,23 @@ internal static partial class SimdUtils /// /// as many elements as possible, slicing them down (keeping the remainder). /// + /// The source buffer. + /// The destination buffer. [MethodImpl(InliningOptions.ShortMethod)] internal static void ByteToNormalizedFloatReduce( ref ReadOnlySpan source, - ref Span dest) + ref Span destination) { - DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same length!"); + DebugGuard.IsTrue(source.Length == destination.Length, nameof(source), "Input spans must be of same length!"); if (Avx2.IsSupported || Sse2.IsSupported) { int remainder; - if (Avx2.IsSupported) + if (Vector512.IsHardwareAccelerated && Avx512F.IsSupported) + { + remainder = Numerics.ModuloP2(source.Length, Vector512.Count); + } + else if (Avx2.IsSupported) { remainder = Numerics.ModuloP2(source.Length, Vector256.Count); } @@ -775,10 +781,10 @@ internal static partial class SimdUtils if (adjustedCount > 0) { - ByteToNormalizedFloat(source[..adjustedCount], dest[..adjustedCount]); + ByteToNormalizedFloat(source[..adjustedCount], destination[..adjustedCount]); source = source[adjustedCount..]; - dest = dest[adjustedCount..]; + destination = destination[adjustedCount..]; } } } @@ -786,97 +792,127 @@ internal static partial class SimdUtils /// /// Implementation , which is faster on new RyuJIT runtime. /// + /// The source buffer. + /// The destination buffer. /// /// Implementation is based on MagicScaler code: /// https://github.com/saucecontrol/PhotoSauce/blob/b5811908041200488aa18fdfd17df5fc457415dc/src/MagicScaler/Magic/Processors/ConvertersFloat.cs#L80-L182 /// internal static unsafe void ByteToNormalizedFloat( ReadOnlySpan source, - Span dest) + Span destination) { - fixed (byte* sourceBase = source) + if (Avx512F.IsSupported) { - if (Avx2.IsSupported) - { - DebugVerifySpanInput(source, dest, Vector256.Count); - - nuint n = dest.Vector256Count(); + DebugVerifySpanInput(source, destination, Vector512.Count); - ref Vector256 destBase = - ref Unsafe.As>(ref MemoryMarshal.GetReference(dest)); + nuint n = destination.Vector512Count(); - Vector256 scale = Vector256.Create(1 / (float)byte.MaxValue); + ref byte sourceBase = ref MemoryMarshal.GetReference(source); + ref Vector512 destinationBase = ref Unsafe.As>(ref MemoryMarshal.GetReference(destination)); - for (nuint i = 0; i < n; i++) - { - nuint si = (uint)Vector256.Count * i; - Vector256 i0 = Avx2.ConvertToVector256Int32(sourceBase + si); - Vector256 i1 = Avx2.ConvertToVector256Int32(sourceBase + si + Vector256.Count); - Vector256 i2 = Avx2.ConvertToVector256Int32(sourceBase + si + (Vector256.Count * 2)); - Vector256 i3 = Avx2.ConvertToVector256Int32(sourceBase + si + (Vector256.Count * 3)); - - Vector256 f0 = Avx.Multiply(scale, Avx.ConvertToVector256Single(i0)); - Vector256 f1 = Avx.Multiply(scale, Avx.ConvertToVector256Single(i1)); - Vector256 f2 = Avx.Multiply(scale, Avx.ConvertToVector256Single(i2)); - Vector256 f3 = Avx.Multiply(scale, Avx.ConvertToVector256Single(i3)); - - ref Vector256 d = ref Unsafe.Add(ref destBase, i * 4); - - d = f0; - Unsafe.Add(ref d, 1) = f1; - Unsafe.Add(ref d, 2) = f2; - Unsafe.Add(ref d, 3) = f3; - } + for (nuint i = 0; i < n; i++) + { + nuint si = (uint)Vector512.Count * i; + Vector512 i0 = Avx512F.ConvertToVector512Int32(Vector128.LoadUnsafe(ref sourceBase, si)); + Vector512 i1 = Avx512F.ConvertToVector512Int32(Vector128.LoadUnsafe(ref sourceBase, si + (nuint)Vector512.Count)); + Vector512 i2 = Avx512F.ConvertToVector512Int32(Vector128.LoadUnsafe(ref sourceBase, si + (nuint)(Vector512.Count * 2))); + Vector512 i3 = Avx512F.ConvertToVector512Int32(Vector128.LoadUnsafe(ref sourceBase, si + (nuint)(Vector512.Count * 3))); + + // Declare multiplier on each line. Codegen is better. + Vector512 f0 = Vector512.Create(1 / (float)byte.MaxValue) * Avx512F.ConvertToVector512Single(i0); + Vector512 f1 = Vector512.Create(1 / (float)byte.MaxValue) * Avx512F.ConvertToVector512Single(i1); + Vector512 f2 = Vector512.Create(1 / (float)byte.MaxValue) * Avx512F.ConvertToVector512Single(i2); + Vector512 f3 = Vector512.Create(1 / (float)byte.MaxValue) * Avx512F.ConvertToVector512Single(i3); + + ref Vector512 d = ref Unsafe.Add(ref destinationBase, i * 4); + + d = f0; + Unsafe.Add(ref d, 1) = f1; + Unsafe.Add(ref d, 2) = f2; + Unsafe.Add(ref d, 3) = f3; } - else + } + else if (Avx2.IsSupported) + { + DebugVerifySpanInput(source, destination, Vector256.Count); + + nuint n = destination.Vector256Count(); + + ref byte sourceBase = ref MemoryMarshal.GetReference(source); + ref Vector256 destinationBase = ref Unsafe.As>(ref MemoryMarshal.GetReference(destination)); + + for (nuint i = 0; i < n; i++) { - // Sse - DebugVerifySpanInput(source, dest, Vector128.Count); + nuint si = (uint)Vector256.Count * i; + Vector256 i0 = Avx2.ConvertToVector256Int32(Vector128.LoadUnsafe(ref sourceBase, si)); + Vector256 i1 = Avx2.ConvertToVector256Int32(Vector128.LoadUnsafe(ref sourceBase, si + (nuint)Vector256.Count)); + Vector256 i2 = Avx2.ConvertToVector256Int32(Vector128.LoadUnsafe(ref sourceBase, si + (nuint)(Vector256.Count * 2))); + + // Ensure overreads past 16 byte boundary do not happen in debug due to lack of containment. + ref ulong refULong = ref Unsafe.As(ref Unsafe.Add(ref sourceBase, si)); + Vector256 i3 = Avx2.ConvertToVector256Int32(Vector128.CreateScalarUnsafe(Unsafe.Add(ref refULong, 3)).AsByte()); + + // Declare multiplier on each line. Codegen is better. + Vector256 f0 = Vector256.Create(1 / (float)byte.MaxValue) * Avx.ConvertToVector256Single(i0); + Vector256 f1 = Vector256.Create(1 / (float)byte.MaxValue) * Avx.ConvertToVector256Single(i1); + Vector256 f2 = Vector256.Create(1 / (float)byte.MaxValue) * Avx.ConvertToVector256Single(i2); + Vector256 f3 = Vector256.Create(1 / (float)byte.MaxValue) * Avx.ConvertToVector256Single(i3); + + ref Vector256 d = ref Unsafe.Add(ref destinationBase, i * 4); + + d = f0; + Unsafe.Add(ref d, 1) = f1; + Unsafe.Add(ref d, 2) = f2; + Unsafe.Add(ref d, 3) = f3; + } + } + else if (Sse2.IsSupported || AdvSimd.IsSupported) + { + DebugVerifySpanInput(source, destination, Vector128.Count); - nuint n = dest.Vector128Count(); + nuint n = destination.Vector128Count(); + + ref byte sourceBase = ref MemoryMarshal.GetReference(source); + ref Vector128 destinationBase = ref Unsafe.As>(ref MemoryMarshal.GetReference(destination)); - ref Vector128 destBase = - ref Unsafe.As>(ref MemoryMarshal.GetReference(dest)); + Vector128 scale = Vector128.Create(1 / (float)byte.MaxValue); + Vector128 zero = Vector128.Zero; - Vector128 scale = Vector128.Create(1 / (float)byte.MaxValue); - Vector128 zero = Vector128.Zero; + for (nuint i = 0; i < n; i++) + { + nuint si = (uint)Vector128.Count * i; - for (nuint i = 0; i < n; i++) + Vector128 i0, i1, i2, i3; + if (Sse41.IsSupported) { - nuint si = (uint)Vector128.Count * i; - - Vector128 i0, i1, i2, i3; - if (Sse41.IsSupported) - { - i0 = Sse41.ConvertToVector128Int32(sourceBase + si); - i1 = Sse41.ConvertToVector128Int32(sourceBase + si + Vector128.Count); - i2 = Sse41.ConvertToVector128Int32(sourceBase + si + (Vector128.Count * 2)); - i3 = Sse41.ConvertToVector128Int32(sourceBase + si + (Vector128.Count * 3)); - } - else - { - Vector128 b = Sse2.LoadVector128(sourceBase + si); - Vector128 s0 = Sse2.UnpackLow(b, zero).AsInt16(); - Vector128 s1 = Sse2.UnpackHigh(b, zero).AsInt16(); - - i0 = Sse2.UnpackLow(s0, zero.AsInt16()).AsInt32(); - i1 = Sse2.UnpackHigh(s0, zero.AsInt16()).AsInt32(); - i2 = Sse2.UnpackLow(s1, zero.AsInt16()).AsInt32(); - i3 = Sse2.UnpackHigh(s1, zero.AsInt16()).AsInt32(); - } - - Vector128 f0 = Sse.Multiply(scale, Sse2.ConvertToVector128Single(i0)); - Vector128 f1 = Sse.Multiply(scale, Sse2.ConvertToVector128Single(i1)); - Vector128 f2 = Sse.Multiply(scale, Sse2.ConvertToVector128Single(i2)); - Vector128 f3 = Sse.Multiply(scale, Sse2.ConvertToVector128Single(i3)); - - ref Vector128 d = ref Unsafe.Add(ref destBase, i * 4); - - d = f0; - Unsafe.Add(ref d, 1) = f1; - Unsafe.Add(ref d, 2) = f2; - Unsafe.Add(ref d, 3) = f3; + ref int refInt = ref Unsafe.As(ref Unsafe.Add(ref sourceBase, si)); + + i0 = Sse41.ConvertToVector128Int32(Vector128.CreateScalarUnsafe(refInt).AsByte()); + i1 = Sse41.ConvertToVector128Int32(Vector128.CreateScalarUnsafe(Unsafe.Add(ref refInt, 1)).AsByte()); + i2 = Sse41.ConvertToVector128Int32(Vector128.CreateScalarUnsafe(Unsafe.Add(ref refInt, 2)).AsByte()); + i3 = Sse41.ConvertToVector128Int32(Vector128.CreateScalarUnsafe(Unsafe.Add(ref refInt, 3)).AsByte()); } + else + { + // Sse2, AdvSimd + Vector128 b = Vector128.LoadUnsafe(ref sourceBase, si); + (Vector128 s0, Vector128 s1) = Vector128.Widen(b); + (i0, i1) = Vector128.Widen(s0.AsInt16()); + (i2, i3) = Vector128.Widen(s1.AsInt16()); + } + + Vector128 f0 = scale * Vector128.ConvertToSingle(i0); + Vector128 f1 = scale * Vector128.ConvertToSingle(i1); + Vector128 f2 = scale * Vector128.ConvertToSingle(i2); + Vector128 f3 = scale * Vector128.ConvertToSingle(i3); + + ref Vector128 d = ref Unsafe.Add(ref destinationBase, i * 4); + + d = f0; + Unsafe.Add(ref d, 1) = f1; + Unsafe.Add(ref d, 2) = f2; + Unsafe.Add(ref d, 3) = f3; } } } diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.cs b/src/ImageSharp/Common/Helpers/SimdUtils.cs index 002c1f8da..0279e57cc 100644 --- a/src/ImageSharp/Common/Helpers/SimdUtils.cs +++ b/src/ImageSharp/Common/Helpers/SimdUtils.cs @@ -22,13 +22,6 @@ internal static partial class SimdUtils public static bool HasVector8 { get; } = Vector.IsHardwareAccelerated && Vector.Count == 8 && Vector.Count == 8; - /// - /// Gets a value indicating whether code is being JIT-ed to SSE instructions - /// where float and integer registers are of size 128 byte. - /// - public static bool HasVector4 { get; } = - Vector.IsHardwareAccelerated && Vector.Count == 4; - /// /// Transform all scalars in 'v' in a way that converting them to would have rounding semantics. /// @@ -69,96 +62,6 @@ internal static partial class SimdUtils } } - /// - /// Converts all input -s to -s normalized into [0..1]. - /// should be the of the same size as , - /// but there are no restrictions on the span's length. - /// - /// The source span of bytes - /// The destination span of floats - [MethodImpl(InliningOptions.ShortMethod)] - internal static void ByteToNormalizedFloat(ReadOnlySpan source, Span dest) - { - DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same length!"); - - HwIntrinsics.ByteToNormalizedFloatReduce(ref source, ref dest); - - // Also deals with the remainder from previous conversions: - FallbackIntrinsics128.ByteToNormalizedFloatReduce(ref source, ref dest); - - // Deal with the remainder: - if (source.Length > 0) - { - ConvertByteToNormalizedFloatRemainder(source, dest); - } - } - - /// - /// Convert all values normalized into [0..1] from 'source' into 'destination' buffer of . - /// The values are scaled up into [0-255] and rounded, overflows are clamped. - /// should be the of the same size as , - /// but there are no restrictions on the span's length. - /// - /// The source span of floats - /// The destination span of bytes - [MethodImpl(InliningOptions.ShortMethod)] - internal static void NormalizedFloatToByteSaturate(ReadOnlySpan source, Span destination) - { - DebugGuard.IsTrue(source.Length == destination.Length, nameof(source), "Input spans must be of same length!"); - HwIntrinsics.NormalizedFloatToByteSaturateReduce(ref source, ref destination); - - // Deal with the remainder: - if (source.Length > 0) - { - ConvertNormalizedFloatToByteRemainder(source, destination); - } - } - - [MethodImpl(InliningOptions.ColdPath)] - private static void ConvertByteToNormalizedFloatRemainder(ReadOnlySpan source, Span destination) - { - ref byte sBase = ref MemoryMarshal.GetReference(source); - ref float dBase = ref MemoryMarshal.GetReference(destination); - - // There are at most 3 elements at this point, having a for loop is overkill. - // Let's minimize the no. of instructions! - switch (source.Length) - { - case 3: - Unsafe.Add(ref dBase, 2) = Unsafe.Add(ref sBase, 2) / 255f; - goto case 2; - case 2: - Unsafe.Add(ref dBase, 1) = Unsafe.Add(ref sBase, 1) / 255f; - goto case 1; - case 1: - dBase = sBase / 255f; - break; - } - } - - [MethodImpl(MethodImplOptions.NoInlining)] - private static void ConvertNormalizedFloatToByteRemainder(ReadOnlySpan source, Span destination) - { - ref float sBase = ref MemoryMarshal.GetReference(source); - ref byte dBase = ref MemoryMarshal.GetReference(destination); - for (int i = 0; i < source.Length; i++) - { - Unsafe.Add(ref dBase, i) = ConvertToByte(Unsafe.Add(ref sBase, i)); - } - } - - [MethodImpl(InliningOptions.ShortMethod)] - private static byte ConvertToByte(float f) => (byte)Numerics.Clamp((f * 255F) + 0.5F, 0, 255F); - - [Conditional("DEBUG")] - private static void VerifyHasVector8(string operation) - { - if (!HasVector8) - { - throw new NotSupportedException($"{operation} is supported only on AVX2 CPU!"); - } - } - [Conditional("DEBUG")] private static void DebugVerifySpanInput(ReadOnlySpan source, Span dest, int shouldBeDivisibleBy) { diff --git a/src/ImageSharp/Common/Helpers/Vector128Utilities.cs b/src/ImageSharp/Common/Helpers/Vector128Utilities.cs index a07fa8ca6..b6dd319f0 100644 --- a/src/ImageSharp/Common/Helpers/Vector128Utilities.cs +++ b/src/ImageSharp/Common/Helpers/Vector128Utilities.cs @@ -26,7 +26,7 @@ internal static class Vector128Utilities public static bool SupportsShuffleFloat { [MethodImpl(MethodImplOptions.AggressiveInlining)] - get => Sse.IsSupported || AdvSimd.IsSupported; + get => Sse.IsSupported; } /// @@ -70,17 +70,6 @@ internal static class Vector128Utilities return Sse.Shuffle(vector, vector, control); } - if (AdvSimd.IsSupported) - { -#pragma warning disable CA1857 // A constant is expected for the parameter - Vector128 result = Vector128.Create(AdvSimd.Extract(vector, (byte)(control & 0x3))); - result = AdvSimd.Insert(result, 1, AdvSimd.Extract(vector, (byte)((control >> 2) & 0x3))); - result = AdvSimd.Insert(result, 2, AdvSimd.Extract(vector, (byte)((control >> 4) & 0x3))); - result = AdvSimd.Insert(result, 3, AdvSimd.Extract(vector, (byte)((control >> 6) & 0x3))); -#pragma warning restore CA1857 // A constant is expected for the parameter - return result; - } - ThrowUnreachableException(); return default; } diff --git a/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.cs b/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.cs index d432e82d2..018df5f9f 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.cs @@ -386,29 +386,33 @@ internal partial struct Block8x8F : IEquatable public void LoadFromInt16ExtendedAvx2(ref Block8x8 source) { DebugGuard.IsTrue( - SimdUtils.HasVector8, + Avx2.IsSupported, "LoadFromUInt16ExtendedAvx2 only works on AVX2 compatible architecture!"); - ref Vector sRef = ref Unsafe.As>(ref source); - ref Vector dRef = ref Unsafe.As>(ref this); + ref short sRef = ref Unsafe.As(ref source); + ref Vector256 dRef = ref Unsafe.As>(ref this); - // Vector.Count == 16 on AVX2 + // Vector256.Count == 16 on AVX2 // We can process 2 block rows in a single step - SimdUtils.ExtendedIntrinsics.ConvertToSingle(sRef, out Vector top, out Vector bottom); - dRef = top; - Unsafe.Add(ref dRef, 1) = bottom; - - SimdUtils.ExtendedIntrinsics.ConvertToSingle(Unsafe.Add(ref sRef, 1), out top, out bottom); - Unsafe.Add(ref dRef, 2) = top; - Unsafe.Add(ref dRef, 3) = bottom; - - SimdUtils.ExtendedIntrinsics.ConvertToSingle(Unsafe.Add(ref sRef, 2), out top, out bottom); - Unsafe.Add(ref dRef, 4) = top; - Unsafe.Add(ref dRef, 5) = bottom; - - SimdUtils.ExtendedIntrinsics.ConvertToSingle(Unsafe.Add(ref sRef, 3), out top, out bottom); - Unsafe.Add(ref dRef, 6) = top; - Unsafe.Add(ref dRef, 7) = bottom; + Vector256 top = Avx2.ConvertToVector256Int32(Vector128.LoadUnsafe(ref sRef)); + Vector256 bottom = Avx2.ConvertToVector256Int32(Vector128.LoadUnsafe(ref sRef, (nuint)Vector256.Count)); + dRef = Avx.ConvertToVector256Single(top); + Unsafe.Add(ref dRef, 1) = Avx.ConvertToVector256Single(bottom); + + top = Avx2.ConvertToVector256Int32(Vector128.LoadUnsafe(ref sRef, (nuint)(Vector256.Count * 2))); + bottom = Avx2.ConvertToVector256Int32(Vector128.LoadUnsafe(ref sRef, (nuint)(Vector256.Count * 3))); + Unsafe.Add(ref dRef, 2) = Avx.ConvertToVector256Single(top); + Unsafe.Add(ref dRef, 3) = Avx.ConvertToVector256Single(bottom); + + top = Avx2.ConvertToVector256Int32(Vector128.LoadUnsafe(ref sRef, (nuint)(Vector256.Count * 4))); + bottom = Avx2.ConvertToVector256Int32(Vector128.LoadUnsafe(ref sRef, (nuint)(Vector256.Count * 5))); + Unsafe.Add(ref dRef, 4) = Avx.ConvertToVector256Single(top); + Unsafe.Add(ref dRef, 5) = Avx.ConvertToVector256Single(bottom); + + top = Avx2.ConvertToVector256Int32(Vector128.LoadUnsafe(ref sRef, (nuint)(Vector256.Count * 6))); + bottom = Avx2.ConvertToVector256Int32(Vector128.LoadUnsafe(ref sRef, (nuint)(Vector256.Count * 7))); + Unsafe.Add(ref dRef, 6) = Avx.ConvertToVector256Single(top); + Unsafe.Add(ref dRef, 7) = Avx.ConvertToVector256Single(bottom); } /// diff --git a/src/ImageSharp/PixelFormats/Utils/Vector4Converters.RgbaCompatible.cs b/src/ImageSharp/PixelFormats/Utils/Vector4Converters.RgbaCompatible.cs index 8616ecb3b..9e649f3c0 100644 --- a/src/ImageSharp/PixelFormats/Utils/Vector4Converters.RgbaCompatible.cs +++ b/src/ImageSharp/PixelFormats/Utils/Vector4Converters.RgbaCompatible.cs @@ -5,6 +5,7 @@ using System.Buffers; using System.Numerics; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; +using System.Runtime.Intrinsics; namespace SixLabors.ImageSharp.PixelFormats.Utils; @@ -31,74 +32,86 @@ internal static partial class Vector4Converters /// Provides an efficient default implementation for /// The method works by internally converting to a therefore it's not applicable for that type! /// - [MethodImpl(InliningOptions.ShortMethod)] + /// The type of pixel format. + /// The configuration. + /// The pixel operations instance. + /// The source buffer. + /// The destination buffer. + /// The conversion modifier flags. + [MethodImpl(MethodImplOptions.AggressiveInlining)] internal static void ToVector4( Configuration configuration, PixelOperations pixelOperations, - ReadOnlySpan sourcePixels, - Span destVectors, + ReadOnlySpan source, + Span destination, PixelConversionModifiers modifiers) where TPixel : unmanaged, IPixel { Guard.NotNull(configuration, nameof(configuration)); - Guard.DestinationShouldNotBeTooShort(sourcePixels, destVectors, nameof(destVectors)); + Guard.DestinationShouldNotBeTooShort(source, destination, nameof(destination)); - int count = sourcePixels.Length; + int count = source.Length; // Not worth for small buffers: if (count < Vector4ConversionThreshold) { - Default.UnsafeToVector4(sourcePixels, destVectors, modifiers); + Default.UnsafeToVector4(source, destination, modifiers); return; } - // Using the last quarter of 'destVectors' as a temporary buffer to avoid allocation: + // Using the last quarter of 'destination' as a temporary buffer to avoid allocation: int countWithoutLastItem = count - 1; - ReadOnlySpan reducedSource = sourcePixels[..countWithoutLastItem]; - Span lastQuarterOfDestBuffer = MemoryMarshal.Cast(destVectors).Slice((3 * count) + 1, countWithoutLastItem); - pixelOperations.ToRgba32(configuration, reducedSource, lastQuarterOfDestBuffer); + ReadOnlySpan reducedSource = source[..countWithoutLastItem]; + Span lastQuarterOfDestination = MemoryMarshal.Cast(destination).Slice((3 * count) + 1, countWithoutLastItem); + pixelOperations.ToRgba32(configuration, reducedSource, lastQuarterOfDestination); - // 'destVectors' and 'lastQuarterOfDestBuffer' are overlapping buffers, + // 'destination' and 'lastQuarterOfDestination' are overlapping buffers, // but we are always reading/writing at different positions: SimdUtils.ByteToNormalizedFloat( - MemoryMarshal.Cast(lastQuarterOfDestBuffer), - MemoryMarshal.Cast(destVectors[..countWithoutLastItem])); + MemoryMarshal.Cast(lastQuarterOfDestination), + MemoryMarshal.Cast(destination[..countWithoutLastItem])); - destVectors[countWithoutLastItem] = sourcePixels[countWithoutLastItem].ToVector4(); + destination[countWithoutLastItem] = source[countWithoutLastItem].ToVector4(); // TODO: Investigate optimized 1-pass approach! - ApplyForwardConversionModifiers(destVectors, modifiers); + ApplyForwardConversionModifiers(destination, modifiers); } /// /// Provides an efficient default implementation for /// The method is works by internally converting to a therefore it's not applicable for that type! /// - [MethodImpl(InliningOptions.ShortMethod)] + /// The type of pixel format. + /// The configuration. + /// The pixel operations instance. + /// The source buffer. + /// The destination buffer. + /// The conversion modifier flags. + [MethodImpl(MethodImplOptions.AggressiveInlining)] internal static void FromVector4( Configuration configuration, PixelOperations pixelOperations, - Span sourceVectors, - Span destPixels, + Span source, + Span destination, PixelConversionModifiers modifiers) where TPixel : unmanaged, IPixel { Guard.NotNull(configuration, nameof(configuration)); - Guard.DestinationShouldNotBeTooShort(sourceVectors, destPixels, nameof(destPixels)); + Guard.DestinationShouldNotBeTooShort(source, destination, nameof(destination)); - int count = sourceVectors.Length; + int count = source.Length; // Not worth for small buffers: if (count < Vector4ConversionThreshold) { - Default.UnsafeFromVector4(sourceVectors, destPixels, modifiers); + Default.UnsafeFromVector4(source, destination, modifiers); return; } // TODO: Investigate optimized 1-pass approach! - ApplyBackwardConversionModifiers(sourceVectors, modifiers); + ApplyBackwardConversionModifiers(source, modifiers); // For the opposite direction it's not easy to implement the trick used in RunRgba32CompatibleToVector4Conversion, // so let's allocate a temporary buffer as usually: @@ -106,20 +119,30 @@ internal static partial class Vector4Converters Span tempSpan = tempBuffer.Memory.Span; SimdUtils.NormalizedFloatToByteSaturate( - MemoryMarshal.Cast(sourceVectors), + MemoryMarshal.Cast(source), MemoryMarshal.Cast(tempSpan)); - pixelOperations.FromRgba32(configuration, tempSpan, destPixels); + pixelOperations.FromRgba32(configuration, tempSpan, destination); } private static int CalculateVector4ConversionThreshold() { - if (!Vector.IsHardwareAccelerated) + if (!Vector128.IsHardwareAccelerated) { return int.MaxValue; } - return SimdUtils.ExtendedIntrinsics.IsAvailable && SimdUtils.HasVector8 ? 256 : 128; + if (Vector512.IsHardwareAccelerated) + { + return 512; + } + + if (Vector256.IsHardwareAccelerated) + { + return 256; + } + + return 128; } } } diff --git a/tests/ImageSharp.Benchmarks/Bulk/ToVector4.cs b/tests/ImageSharp.Benchmarks/Bulk/ToVector4.cs index 3b4360b16..0df8d9818 100644 --- a/tests/ImageSharp.Benchmarks/Bulk/ToVector4.cs +++ b/tests/ImageSharp.Benchmarks/Bulk/ToVector4.cs @@ -14,9 +14,9 @@ namespace SixLabors.ImageSharp.Benchmarks.Bulk; public abstract class ToVector4 where TPixel : unmanaged, IPixel { - protected IMemoryOwner source; + protected IMemoryOwner Source { get; set; } - protected IMemoryOwner destination; + protected IMemoryOwner Destination { get; set; } protected Configuration Configuration => Configuration.Default; @@ -26,22 +26,22 @@ public abstract class ToVector4 [GlobalSetup] public void Setup() { - this.source = this.Configuration.MemoryAllocator.Allocate(this.Count); - this.destination = this.Configuration.MemoryAllocator.Allocate(this.Count); + this.Source = this.Configuration.MemoryAllocator.Allocate(this.Count); + this.Destination = this.Configuration.MemoryAllocator.Allocate(this.Count); } [GlobalCleanup] public void Cleanup() { - this.source.Dispose(); - this.destination.Dispose(); + this.Source.Dispose(); + this.Destination.Dispose(); } // [Benchmark] public void Naive() { - Span s = this.source.GetSpan(); - Span d = this.destination.GetSpan(); + Span s = this.Source.GetSpan(); + Span d = this.Destination.GetSpan(); for (int i = 0; i < this.Count; i++) { @@ -50,11 +50,8 @@ public abstract class ToVector4 } [Benchmark] - public void PixelOperations_Specialized() - { - PixelOperations.Instance.ToVector4( + public void PixelOperations_Specialized() => PixelOperations.Instance.ToVector4( this.Configuration, - this.source.GetSpan(), - this.destination.GetSpan()); - } + this.Source.GetSpan(), + this.Destination.GetSpan()); } diff --git a/tests/ImageSharp.Benchmarks/Bulk/ToVector4_Bgra32.cs b/tests/ImageSharp.Benchmarks/Bulk/ToVector4_Bgra32.cs index 934a17dc9..6499632b6 100644 --- a/tests/ImageSharp.Benchmarks/Bulk/ToVector4_Bgra32.cs +++ b/tests/ImageSharp.Benchmarks/Bulk/ToVector4_Bgra32.cs @@ -16,8 +16,8 @@ public class ToVector4_Bgra32 : ToVector4 { new PixelOperations().ToVector4( this.Configuration, - this.source.GetSpan(), - this.destination.GetSpan()); + this.Source.GetSpan(), + this.Destination.GetSpan()); } // RESULTS: diff --git a/tests/ImageSharp.Benchmarks/Bulk/ToVector4_Rgb24.cs b/tests/ImageSharp.Benchmarks/Bulk/ToVector4_Rgb24.cs index d5d6e31b5..adedabf8f 100644 --- a/tests/ImageSharp.Benchmarks/Bulk/ToVector4_Rgb24.cs +++ b/tests/ImageSharp.Benchmarks/Bulk/ToVector4_Rgb24.cs @@ -16,8 +16,8 @@ public class ToVector4_Rgb24 : ToVector4 { new PixelOperations().ToVector4( this.Configuration, - this.source.GetSpan(), - this.destination.GetSpan()); + this.Source.GetSpan(), + this.Destination.GetSpan()); } } diff --git a/tests/ImageSharp.Benchmarks/Bulk/ToVector4_Rgba32.cs b/tests/ImageSharp.Benchmarks/Bulk/ToVector4_Rgba32.cs index 9c7ecbc49..113793a03 100644 --- a/tests/ImageSharp.Benchmarks/Bulk/ToVector4_Rgba32.cs +++ b/tests/ImageSharp.Benchmarks/Bulk/ToVector4_Rgba32.cs @@ -14,27 +14,18 @@ namespace SixLabors.ImageSharp.Benchmarks.Bulk; [Config(typeof(Config.Short))] public class ToVector4_Rgba32 : ToVector4 { - [Benchmark] - public void FallbackIntrinsics128() - { - Span sBytes = MemoryMarshal.Cast(this.source.GetSpan()); - Span dFloats = MemoryMarshal.Cast(this.destination.GetSpan()); - - SimdUtils.FallbackIntrinsics128.ByteToNormalizedFloat(sBytes, dFloats); - } - [Benchmark] public void PixelOperations_Base() => new PixelOperations().ToVector4( this.Configuration, - this.source.GetSpan(), - this.destination.GetSpan()); + this.Source.GetSpan(), + this.Destination.GetSpan()); [Benchmark] public void HwIntrinsics() { - Span sBytes = MemoryMarshal.Cast(this.source.GetSpan()); - Span dFloats = MemoryMarshal.Cast(this.destination.GetSpan()); + Span sBytes = MemoryMarshal.Cast(this.Source.GetSpan()); + Span dFloats = MemoryMarshal.Cast(this.Destination.GetSpan()); SimdUtils.HwIntrinsics.ByteToNormalizedFloat(sBytes, dFloats); } @@ -42,8 +33,8 @@ public class ToVector4_Rgba32 : ToVector4 // [Benchmark] public void ExtendedIntrinsics_BulkConvertByteToNormalizedFloat_2Loops() { - Span sBytes = MemoryMarshal.Cast(this.source.GetSpan()); - Span dFloats = MemoryMarshal.Cast(this.destination.GetSpan()); + Span sBytes = MemoryMarshal.Cast(this.Source.GetSpan()); + Span dFloats = MemoryMarshal.Cast(this.Destination.GetSpan()); nuint n = (uint)dFloats.Length / (uint)Vector.Count; @@ -67,14 +58,14 @@ public class ToVector4_Rgba32 : ToVector4 } n = (uint)(dFloats.Length / Vector.Count); - var scale = new Vector(1f / 255f); + Vector scale = new(1f / 255f); for (nuint i = 0; i < n; i++) { ref Vector dRef = ref Unsafe.Add(ref destBase, i); - var du = Vector.AsVectorInt32(dRef); - var v = Vector.ConvertToSingle(du); + Vector du = Vector.AsVectorInt32(dRef); + Vector v = Vector.ConvertToSingle(du); v *= scale; dRef = v; @@ -84,14 +75,14 @@ public class ToVector4_Rgba32 : ToVector4 // [Benchmark] public void ExtendedIntrinsics_BulkConvertByteToNormalizedFloat_ConvertInSameLoop() { - Span sBytes = MemoryMarshal.Cast(this.source.GetSpan()); - Span dFloats = MemoryMarshal.Cast(this.destination.GetSpan()); + Span sBytes = MemoryMarshal.Cast(this.Source.GetSpan()); + Span dFloats = MemoryMarshal.Cast(this.Destination.GetSpan()); nuint n = (uint)dFloats.Length / (uint)Vector.Count; ref Vector sourceBase = ref Unsafe.As>(ref MemoryMarshal.GetReference((ReadOnlySpan)sBytes)); ref Vector destBase = ref Unsafe.As>(ref MemoryMarshal.GetReference(dFloats)); - var scale = new Vector(1f / 255f); + Vector scale = new(1f / 255f); for (nuint i = 0; i < n; i++) { @@ -117,8 +108,8 @@ public class ToVector4_Rgba32 : ToVector4 [MethodImpl(MethodImplOptions.AggressiveInlining)] private static Vector ConvertToNormalizedSingle(Vector u, Vector scale) { - var vi = Vector.AsVectorInt32(u); - var v = Vector.ConvertToSingle(vi); + Vector vi = Vector.AsVectorInt32(u); + Vector v = Vector.ConvertToSingle(vi); v *= scale; return v; } @@ -151,4 +142,30 @@ public class ToVector4_Rgba32 : ToVector4 PixelOperations_Base | Core | 2048 | 6,752.68 ns | 272.820 ns | 15.4148 ns | 1.67 | 0.02 | - | 24 B | PixelOperations_Specialized | Core | 2048 | 1,126.13 ns | 79.192 ns | 4.4745 ns |!! 0.28 | 0.00 | - | 0 B | <--- ExtendedIntrinsics rock! */ + + /* + BenchmarkDotNet v0.13.10, Windows 11 (10.0.22631.3085/23H2/2023Update/SunValley3) + 11th Gen Intel Core i7-11370H 3.30GHz, 1 CPU, 8 logical and 4 physical cores + .NET SDK 8.0.200-preview.23624.5 + [Host] : .NET 8.0.1 (8.0.123.58001), X64 RyuJIT AVX2 + Job-DFEQJT : .NET 8.0.1 (8.0.123.58001), X64 RyuJIT AVX2 + + Runtime=.NET 8.0 Arguments=/p:DebugType=portable IterationCount=3 + LaunchCount=1 WarmupCount=3 + + | Method | Count | Mean | Error | StdDev | Allocated | + |---------------------------- |------ |------------:|-----------:|----------:|----------:| + | FallbackIntrinsics128 | 64 | 139.66 ns | 27.429 ns | 1.503 ns | - | + | PixelOperations_Base | 64 | 124.65 ns | 29.653 ns | 1.625 ns | - | + | HwIntrinsics | 64 | 18.16 ns | 4.731 ns | 0.259 ns | - | + | PixelOperations_Specialized | 64 | 27.94 ns | 15.220 ns | 0.834 ns | - | + | FallbackIntrinsics128 | 256 | 525.07 ns | 34.397 ns | 1.885 ns | - | + | PixelOperations_Base | 256 | 464.17 ns | 46.897 ns | 2.571 ns | - | + | HwIntrinsics | 256 | 43.88 ns | 4.525 ns | 0.248 ns | - | + | PixelOperations_Specialized | 256 | 55.57 ns | 14.587 ns | 0.800 ns | - | + | FallbackIntrinsics128 | 2048 | 4,148.44 ns | 476.583 ns | 26.123 ns | - | + | PixelOperations_Base | 2048 | 3,608.42 ns | 66.293 ns | 3.634 ns | - | + | HwIntrinsics | 2048 | 361.42 ns | 35.576 ns | 1.950 ns | - | + | PixelOperations_Specialized | 2048 | 374.82 ns | 33.371 ns | 1.829 ns | - | + */ } diff --git a/tests/ImageSharp.Benchmarks/LoadResizeSave/README.md b/tests/ImageSharp.Benchmarks/LoadResizeSave/README.md index 6cb48eb48..98f472241 100644 --- a/tests/ImageSharp.Benchmarks/LoadResizeSave/README.md +++ b/tests/ImageSharp.Benchmarks/LoadResizeSave/README.md @@ -1,4 +1,4 @@ -The benchmarks have been adapted from the +The benchmarks have been adapted from the [PhotoSauce's MemoryStress project](https://github.com/saucecontrol/core-imaging-playground/tree/beeees/MemoryStress). ### Setup diff --git a/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs b/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs index e9e4550b0..36b301264 100644 --- a/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs +++ b/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs @@ -3,6 +3,7 @@ using System.Numerics; using System.Runtime.CompilerServices; +using System.Runtime.Intrinsics.Arm; using System.Runtime.Intrinsics.X86; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Tests.TestUtilities; @@ -117,16 +118,10 @@ public partial class SimdUtilsTests public static readonly TheoryData ArbitraryArraySizes = new() { 0, 1, 2, 3, 4, 7, 8, 9, 15, 16, 17, 63, 64, 255, 511, 512, 513, 514, 515, 516, 517, 518, 519, 520 }; [Theory] - [MemberData(nameof(ArraySizesDivisibleBy4))] - public void FallbackIntrinsics128_BulkConvertByteToNormalizedFloat(int count) => TestImpl_BulkConvertByteToNormalizedFloat( - count, - (s, d) => SimdUtils.FallbackIntrinsics128.ByteToNormalizedFloat(s.Span, d.Span)); - - [Theory] - [MemberData(nameof(ArraySizesDivisibleBy32))] + [MemberData(nameof(ArraySizesDivisibleBy64))] public void HwIntrinsics_BulkConvertByteToNormalizedFloat(int count) { - if (!Sse2.IsSupported) + if (!Sse2.IsSupported && !AdvSimd.IsSupported) { return; } @@ -138,7 +133,7 @@ public partial class SimdUtilsTests FeatureTestRunner.RunWithHwIntrinsicsFeature( RunTest, count, - HwIntrinsics.AllowAll | HwIntrinsics.DisableAVX2 | HwIntrinsics.DisableSSE41); + HwIntrinsics.AllowAll | HwIntrinsics.DisableAVX512F | HwIntrinsics.DisableAVX2 | HwIntrinsics.DisableSSE41); } [Theory] @@ -160,32 +155,11 @@ public partial class SimdUtilsTests Assert.Equal(expected, result, new ApproximateFloatComparer(1e-5f)); } - [Theory] - [InlineData(1234)] - public void ExtendedIntrinsics_ConvertToSingle(short scale) - { - int n = Vector.Count; - short[] sData = new Random(scale).GenerateRandomInt16Array(2 * n, (short)-scale, scale); - float[] fData = sData.Select(u => (float)u).ToArray(); - - Vector source = new(sData); - - Vector expected1 = new(fData, 0); - Vector expected2 = new(fData, n); - - // Act: - SimdUtils.ExtendedIntrinsics.ConvertToSingle(source, out Vector actual1, out Vector actual2); - - // Assert: - Assert.Equal(expected1, actual1); - Assert.Equal(expected2, actual2); - } - [Theory] [MemberData(nameof(ArraySizesDivisibleBy64))] public void HwIntrinsics_BulkConvertNormalizedFloatToByteClampOverflows(int count) { - if (!Sse2.IsSupported) + if (!Sse2.IsSupported && !AdvSimd.IsSupported) { return; } From 0ec839fb931f3d1096c91c397857c31bf888b354 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 6 Feb 2024 17:34:13 +1000 Subject: [PATCH 15/37] Update SimdUtils.HwIntrinsics.cs --- src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs b/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs index 6f0b4b4e3..6e45f1619 100644 --- a/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs +++ b/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs @@ -761,10 +761,13 @@ internal static partial class SimdUtils { DebugGuard.IsTrue(source.Length == destination.Length, nameof(source), "Input spans must be of same length!"); - if (Avx2.IsSupported || Sse2.IsSupported) + if ((Vector512.IsHardwareAccelerated && Avx512F.IsSupported) || + Avx2.IsSupported || + Sse2.IsSupported || + AdvSimd.IsSupported) { int remainder; - if (Vector512.IsHardwareAccelerated && Avx512F.IsSupported) + if (Avx512F.IsSupported) { remainder = Numerics.ModuloP2(source.Length, Vector512.Count); } @@ -877,7 +880,6 @@ internal static partial class SimdUtils ref Vector128 destinationBase = ref Unsafe.As>(ref MemoryMarshal.GetReference(destination)); Vector128 scale = Vector128.Create(1 / (float)byte.MaxValue); - Vector128 zero = Vector128.Zero; for (nuint i = 0; i < n; i++) { From c10863fee66d11cd4d9d0b030c4f7235dc3c1b1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Mutnia=C5=84ski?= Date: Tue, 6 Feb 2024 15:33:11 +0100 Subject: [PATCH 16/37] Replace Memory with string --- .../Formats/Jpeg/JpegDecoderCore.cs | 5 ++--- .../Formats/Jpeg/JpegEncoderCore.cs | 22 +++++++++---------- src/ImageSharp/Formats/Jpeg/JpegMetadata.cs | 4 ++-- .../Formats/Jpg/JpegDecoderTests.Metadata.cs | 2 +- .../Formats/Jpg/JpegEncoderTests.Metadata.cs | 10 ++++----- .../Formats/Jpg/JpegMetadataTests.cs | 10 ++++----- 6 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index d6b40fa7f..5f3fa33fa 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -524,13 +524,12 @@ internal sealed class JpegDecoderCore : IRawJpegData, IImageDecoderInternals private void ProcessComMarker(BufferedReadStream stream, int markerContentByteSize) { Span temp = new byte[markerContentByteSize]; - char[] chars = new char[markerContentByteSize]; JpegMetadata metadata = this.Metadata.GetFormatMetadata(JpegFormat.Instance); stream.Read(temp); - Encoding.ASCII.GetChars(temp, chars); + string comment = Encoding.ASCII.GetString(temp); - metadata.Comments.Add(chars); + metadata.Comments.Add(comment); } /// diff --git a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs index 4dc920207..8658f6b25 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs @@ -186,23 +186,23 @@ internal sealed unsafe partial class JpegEncoderCore : IImageEncoderInternals for (int i = 0; i < metadata.Comments.Count; i++) { - Memory chars = metadata.Comments[i]; + string comment = metadata.Comments[i]; - if (chars.Length > maxCommentLength) + if (comment.Length > maxCommentLength) { - Memory splitComment = chars.Slice(maxCommentLength, chars.Length - maxCommentLength); + string splitComment = comment.Substring(maxCommentLength, comment.Length - maxCommentLength); metadata.Comments.Insert(i + 1, splitComment); // We don't want to keep the extra bytes - chars = chars.Slice(0, maxCommentLength); + comment = comment.Substring(0, maxCommentLength); } - int commentLength = chars.Length + 4; + int commentLength = comment.Length + 4; - Span comment = new byte[commentLength]; - Span markers = comment.Slice(0, 2); - Span payloadSize = comment.Slice(2, 2); - Span payload = comment.Slice(4, chars.Length); + Span commentSpan = new byte[commentLength]; + Span markers = commentSpan.Slice(0, 2); + Span payloadSize = commentSpan.Slice(2, 2); + Span payload = commentSpan.Slice(4, comment.Length); // Beginning of comment ff fe markers[0] = JpegConstants.Markers.XFF; @@ -213,9 +213,9 @@ internal sealed unsafe partial class JpegEncoderCore : IImageEncoderInternals payloadSize[0] = (byte)((comWithoutMarker >> 8) & 0xFF); payloadSize[1] = (byte)(comWithoutMarker & 0xFF); - Encoding.ASCII.GetBytes(chars.Span, payload); + Encoding.ASCII.GetBytes(comment, payload); - this.outputStream.Write(comment, 0, comment.Length); + this.outputStream.Write(commentSpan, 0, commentSpan.Length); } } diff --git a/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs b/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs index bf758dfd0..5b96fdf96 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs @@ -15,7 +15,7 @@ public class JpegMetadata : IDeepCloneable /// public JpegMetadata() { - this.Comments = new List>(); + this.Comments = new List(); } /// @@ -106,7 +106,7 @@ public class JpegMetadata : IDeepCloneable /// /// Gets the comments. /// - public IList> Comments { get; } + public IList Comments { get; } /// public IDeepCloneable DeepClone() => new JpegMetadata(this); diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs index 369e71abf..222d1fb8c 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs @@ -435,7 +435,7 @@ public partial class JpegDecoderTests JpegMetadata metadata = image.Metadata.GetJpegMetadata(); Assert.Equal(1, metadata.Comments.Count); - Assert.Equal(expectedComment.ToCharArray(), metadata.Comments.ElementAtOrDefault(0)); + Assert.Equal(expectedComment, metadata.Comments.ElementAtOrDefault(0)); image.DebugSave(provider); image.CompareToOriginal(provider); } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs index 8cc64acea..f33234e32 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs @@ -172,7 +172,7 @@ public partial class JpegEncoderTests JpegMetadata actual = output.Metadata.GetJpegMetadata(); Assert.NotEmpty(actual.Comments); Assert.Equal(1, actual.Comments.Count); - Assert.Equal("TEST COMMENT", actual.Comments.ElementAtOrDefault(0).ToString()); + Assert.Equal("TEST COMMENT", actual.Comments.ElementAtOrDefault(0)); } [Fact] @@ -184,8 +184,8 @@ public partial class JpegEncoderTests using var memStream = new MemoryStream(); // act - meta.Comments.Add("First comment".ToCharArray()); - meta.Comments.Add("Second Comment".ToCharArray()); + meta.Comments.Add("First comment"); + meta.Comments.Add("Second Comment"); input.Save(memStream, JpegEncoder); // assert @@ -194,8 +194,8 @@ public partial class JpegEncoderTests JpegMetadata actual = output.Metadata.GetJpegMetadata(); Assert.NotEmpty(actual.Comments); Assert.Equal(2, actual.Comments.Count); - Assert.Equal(meta.Comments.ElementAtOrDefault(0).ToString(), actual.Comments.ElementAtOrDefault(0).ToString()); - Assert.Equal(meta.Comments.ElementAtOrDefault(1).ToString(), actual.Comments.ElementAtOrDefault(1).ToString()); + Assert.Equal(meta.Comments.ElementAtOrDefault(0), actual.Comments.ElementAtOrDefault(0)); + Assert.Equal(meta.Comments.ElementAtOrDefault(1), actual.Comments.ElementAtOrDefault(1)); } [Theory] diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs index 8b991228b..64d7edd75 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs @@ -64,19 +64,19 @@ public class JpegMetadataTests { var meta = new JpegMetadata(); - Assert.True(Array.Empty>().SequenceEqual(meta.Comments)); + Assert.True(Array.Empty().SequenceEqual(meta.Comments)); } [Fact] public void Comment_OnlyComment() { string comment = "test comment"; - var expectedCollection = new Collection> { new(comment.ToCharArray()) }; + var expectedCollection = new Collection { comment }; var meta = new JpegMetadata(); - meta.Comments?.Add(comment.ToCharArray()); + meta.Comments.Add(comment); - Assert.Equal(1, meta.Comments?.Count); - Assert.True(expectedCollection.FirstOrDefault().ToString() == meta.Comments?.FirstOrDefault().ToString()); + Assert.Equal(1, meta.Comments.Count); + Assert.True(expectedCollection.FirstOrDefault() == meta.Comments.FirstOrDefault()); } } From d2251287ceaeb0c827d62b29a651775fd2daaa83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Mutnia=C5=84ski?= Date: Tue, 6 Feb 2024 16:16:35 +0100 Subject: [PATCH 17/37] Introduce JpegComData.cs --- src/ImageSharp/Formats/Jpeg/JpegComData.cs | 32 +++++++++++++++++++ .../Formats/Jpeg/JpegDecoderCore.cs | 2 +- .../Formats/Jpeg/JpegEncoderCore.cs | 4 +-- src/ImageSharp/Formats/Jpeg/JpegMetadata.cs | 4 +-- .../Formats/Jpg/JpegDecoderTests.Metadata.cs | 2 +- .../Formats/Jpg/JpegEncoderTests.Metadata.cs | 10 +++--- .../Formats/Jpg/JpegMetadataTests.cs | 6 ++-- 7 files changed, 46 insertions(+), 14 deletions(-) create mode 100644 src/ImageSharp/Formats/Jpeg/JpegComData.cs diff --git a/src/ImageSharp/Formats/Jpeg/JpegComData.cs b/src/ImageSharp/Formats/Jpeg/JpegComData.cs new file mode 100644 index 000000000..26ade0217 --- /dev/null +++ b/src/ImageSharp/Formats/Jpeg/JpegComData.cs @@ -0,0 +1,32 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +namespace SixLabors.ImageSharp.Formats.Jpeg; + +/// +/// Contains JPEG comment +/// +public readonly struct JpegComData +{ + /// + /// Converts string to + /// + /// The comment string. + /// The + public static JpegComData FromString(string value) => new(value.AsMemory()); + + /// + /// Initializes a new instance of the struct. + /// + /// The comment ReadOnlyMemory of chars. + public JpegComData(ReadOnlyMemory value) + => this.Value = value; + + public ReadOnlyMemory Value { get; } + + /// + /// Converts Value to string + /// + /// The comment string. + public override string ToString() => this.Value.ToString(); +} diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 5f3fa33fa..e43c20a7d 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -529,7 +529,7 @@ internal sealed class JpegDecoderCore : IRawJpegData, IImageDecoderInternals stream.Read(temp); string comment = Encoding.ASCII.GetString(temp); - metadata.Comments.Add(comment); + metadata.Comments.Add(JpegComData.FromString(comment)); } /// diff --git a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs index 8658f6b25..a55435c53 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs @@ -186,12 +186,12 @@ internal sealed unsafe partial class JpegEncoderCore : IImageEncoderInternals for (int i = 0; i < metadata.Comments.Count; i++) { - string comment = metadata.Comments[i]; + string comment = metadata.Comments[i].ToString(); if (comment.Length > maxCommentLength) { string splitComment = comment.Substring(maxCommentLength, comment.Length - maxCommentLength); - metadata.Comments.Insert(i + 1, splitComment); + metadata.Comments.Insert(i + 1, JpegComData.FromString(splitComment)); // We don't want to keep the extra bytes comment = comment.Substring(0, maxCommentLength); diff --git a/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs b/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs index 5b96fdf96..fe1324a86 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs @@ -15,7 +15,7 @@ public class JpegMetadata : IDeepCloneable /// public JpegMetadata() { - this.Comments = new List(); + this.Comments = new List(); } /// @@ -106,7 +106,7 @@ public class JpegMetadata : IDeepCloneable /// /// Gets the comments. /// - public IList Comments { get; } + public IList Comments { get; } /// public IDeepCloneable DeepClone() => new JpegMetadata(this); diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs index 222d1fb8c..cbb2befcd 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs @@ -435,7 +435,7 @@ public partial class JpegDecoderTests JpegMetadata metadata = image.Metadata.GetJpegMetadata(); Assert.Equal(1, metadata.Comments.Count); - Assert.Equal(expectedComment, metadata.Comments.ElementAtOrDefault(0)); + Assert.Equal(expectedComment, metadata.Comments.ElementAtOrDefault(0).ToString()); image.DebugSave(provider); image.CompareToOriginal(provider); } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs index f33234e32..e49afedbb 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs @@ -172,7 +172,7 @@ public partial class JpegEncoderTests JpegMetadata actual = output.Metadata.GetJpegMetadata(); Assert.NotEmpty(actual.Comments); Assert.Equal(1, actual.Comments.Count); - Assert.Equal("TEST COMMENT", actual.Comments.ElementAtOrDefault(0)); + Assert.Equal("TEST COMMENT", actual.Comments.ElementAtOrDefault(0).ToString()); } [Fact] @@ -184,8 +184,8 @@ public partial class JpegEncoderTests using var memStream = new MemoryStream(); // act - meta.Comments.Add("First comment"); - meta.Comments.Add("Second Comment"); + meta.Comments.Add(JpegComData.FromString("First comment")); + meta.Comments.Add(JpegComData.FromString("Second Comment")); input.Save(memStream, JpegEncoder); // assert @@ -194,8 +194,8 @@ public partial class JpegEncoderTests JpegMetadata actual = output.Metadata.GetJpegMetadata(); Assert.NotEmpty(actual.Comments); Assert.Equal(2, actual.Comments.Count); - Assert.Equal(meta.Comments.ElementAtOrDefault(0), actual.Comments.ElementAtOrDefault(0)); - Assert.Equal(meta.Comments.ElementAtOrDefault(1), actual.Comments.ElementAtOrDefault(1)); + Assert.Equal(meta.Comments.ElementAtOrDefault(0).ToString(), actual.Comments.ElementAtOrDefault(0).ToString()); + Assert.Equal(meta.Comments.ElementAtOrDefault(1).ToString(), actual.Comments.ElementAtOrDefault(1).ToString()); } [Theory] diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs index 64d7edd75..e07c42f89 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs @@ -64,7 +64,7 @@ public class JpegMetadataTests { var meta = new JpegMetadata(); - Assert.True(Array.Empty().SequenceEqual(meta.Comments)); + Assert.True(Array.Empty().SequenceEqual(meta.Comments)); } [Fact] @@ -74,9 +74,9 @@ public class JpegMetadataTests var expectedCollection = new Collection { comment }; var meta = new JpegMetadata(); - meta.Comments.Add(comment); + meta.Comments.Add(JpegComData.FromString(comment)); Assert.Equal(1, meta.Comments.Count); - Assert.True(expectedCollection.FirstOrDefault() == meta.Comments.FirstOrDefault()); + Assert.True(expectedCollection.FirstOrDefault() == meta.Comments.FirstOrDefault().ToString()); } } From 4e494d427f667c94fb59bcad972ac2f45410e826 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 14 Feb 2024 11:55:07 +1000 Subject: [PATCH 18/37] Simplify checks --- .../Common/Helpers/SimdUtils.Convert.cs | 7 ++--- .../Common/Helpers/SimdUtils.HwIntrinsics.cs | 26 +++++++------------ 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.Convert.cs b/src/ImageSharp/Common/Helpers/SimdUtils.Convert.cs index 1b5a418de..5318ad049 100644 --- a/src/ImageSharp/Common/Helpers/SimdUtils.Convert.cs +++ b/src/ImageSharp/Common/Helpers/SimdUtils.Convert.cs @@ -57,7 +57,7 @@ internal static partial class SimdUtils for (int i = 0; i < source.Length; i++) { - Unsafe.Add(ref dBase, i) = Unsafe.Add(ref sBase, i) / 255f; + Unsafe.Add(ref dBase, (uint)i) = Unsafe.Add(ref sBase, (uint)i) / 255f; } } @@ -66,12 +66,13 @@ internal static partial class SimdUtils { ref float sBase = ref MemoryMarshal.GetReference(source); ref byte dBase = ref MemoryMarshal.GetReference(destination); + for (int i = 0; i < source.Length; i++) { - Unsafe.Add(ref dBase, i) = ConvertToByte(Unsafe.Add(ref sBase, i)); + Unsafe.Add(ref dBase, (uint)i) = ConvertToByte(Unsafe.Add(ref sBase, (uint)i)); } } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static byte ConvertToByte(float f) => (byte)Numerics.Clamp((f * 255F) + 0.5F, 0, 255F); + private static byte ConvertToByte(float f) => (byte)Numerics.Clamp((f * 255f) + 0.5f, 0, 255f); } diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs b/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs index 6e45f1619..17ccb396d 100644 --- a/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs +++ b/src/ImageSharp/Common/Helpers/SimdUtils.HwIntrinsics.cs @@ -761,13 +761,10 @@ internal static partial class SimdUtils { DebugGuard.IsTrue(source.Length == destination.Length, nameof(source), "Input spans must be of same length!"); - if ((Vector512.IsHardwareAccelerated && Avx512F.IsSupported) || - Avx2.IsSupported || - Sse2.IsSupported || - AdvSimd.IsSupported) + if (Vector128.IsHardwareAccelerated) { int remainder; - if (Avx512F.IsSupported) + if (Vector512.IsHardwareAccelerated && Avx512F.IsSupported) { remainder = Numerics.ModuloP2(source.Length, Vector512.Count); } @@ -805,7 +802,7 @@ internal static partial class SimdUtils ReadOnlySpan source, Span destination) { - if (Avx512F.IsSupported) + if (Vector512.IsHardwareAccelerated && Avx512F.IsSupported) { DebugVerifySpanInput(source, destination, Vector512.Count); @@ -870,7 +867,7 @@ internal static partial class SimdUtils Unsafe.Add(ref d, 3) = f3; } } - else if (Sse2.IsSupported || AdvSimd.IsSupported) + else if (Vector128.IsHardwareAccelerated) { DebugVerifySpanInput(source, destination, Vector128.Count); @@ -897,7 +894,7 @@ internal static partial class SimdUtils } else { - // Sse2, AdvSimd + // Sse2, AdvSimd, etc Vector128 b = Vector128.LoadUnsafe(ref sourceBase, si); (Vector128 s0, Vector128 s1) = Vector128.Widen(b); (i0, i1) = Vector128.Widen(s0.AsInt16()); @@ -931,13 +928,11 @@ internal static partial class SimdUtils { DebugGuard.IsTrue(source.Length == destination.Length, nameof(source), "Input spans must be of same length!"); - if ((Vector512.IsHardwareAccelerated && Avx512BW.IsSupported) || - (Vector256.IsHardwareAccelerated && Avx2.IsSupported) || - (Vector128.IsHardwareAccelerated && (Sse2.IsSupported || AdvSimd.IsSupported))) + if (Sse2.IsSupported || AdvSimd.IsSupported) { int remainder; - if (Avx512BW.IsSupported) + if (Vector512.IsHardwareAccelerated && Avx512BW.IsSupported) { remainder = Numerics.ModuloP2(source.Length, Vector512.Count); } @@ -977,7 +972,7 @@ internal static partial class SimdUtils ReadOnlySpan source, Span destination) { - if (Avx512BW.IsSupported) + if (Vector512.IsHardwareAccelerated && Avx512BW.IsSupported) { DebugVerifySpanInput(source, destination, Vector512.Count); @@ -1011,8 +1006,7 @@ internal static partial class SimdUtils Unsafe.Add(ref destinationBase, i) = b; } } - else - if (Avx2.IsSupported) + else if (Avx2.IsSupported) { DebugVerifySpanInput(source, destination, Vector256.Count); @@ -1046,7 +1040,7 @@ internal static partial class SimdUtils Unsafe.Add(ref destinationBase, i) = b; } } - else + else if (Sse2.IsSupported || AdvSimd.IsSupported) { // Sse, AdvSimd DebugVerifySpanInput(source, destination, Vector128.Count); From d865ab96f27d07435355d858fa67fb6fbad028ae Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 23 Feb 2024 13:00:47 +0000 Subject: [PATCH 19/37] Bump NuGet/setup-nuget from 1 to 2 Bumps [NuGet/setup-nuget](https://github.com/nuget/setup-nuget) from 1 to 2. - [Release notes](https://github.com/nuget/setup-nuget/releases) - [Commits](https://github.com/nuget/setup-nuget/compare/v1...v2) --- updated-dependencies: - dependency-name: NuGet/setup-nuget dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/build-and-test.yml | 4 ++-- .github/workflows/code-coverage.yml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 57488a1d0..52a52c472 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -77,7 +77,7 @@ jobs: run: git lfs pull - name: NuGet Install - uses: NuGet/setup-nuget@v1 + uses: NuGet/setup-nuget@v2 - name: NuGet Setup Cache uses: actions/cache@v4 @@ -159,7 +159,7 @@ jobs: submodules: recursive - name: NuGet Install - uses: NuGet/setup-nuget@v1 + uses: NuGet/setup-nuget@v2 - name: NuGet Setup Cache uses: actions/cache@v4 diff --git a/.github/workflows/code-coverage.yml b/.github/workflows/code-coverage.yml index 58e19631f..d531e6860 100644 --- a/.github/workflows/code-coverage.yml +++ b/.github/workflows/code-coverage.yml @@ -44,7 +44,7 @@ jobs: run: git lfs pull - name: NuGet Install - uses: NuGet/setup-nuget@v1 + uses: NuGet/setup-nuget@v2 - name: NuGet Setup Cache uses: actions/cache@v4 From a940a55f5374e0baacb4da23dc85e7dd91807109 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 23 Feb 2024 23:07:34 +1000 Subject: [PATCH 20/37] Make DrawImage processor more robust to bad input. --- .../Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs b/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs index 0d81270b1..d5499d586 100644 --- a/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs +++ b/src/ImageSharp/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs @@ -98,9 +98,10 @@ internal class DrawImageProcessor : ImageProcessor top = 0; } - // clamp the height/width to the availible space left to prevent overflowing + // Clamp the height/width to the available space left to prevent overflowing foregroundRectangle.Width = Math.Min(source.Width - left, foregroundRectangle.Width); foregroundRectangle.Height = Math.Min(source.Height - top, foregroundRectangle.Height); + foregroundRectangle = Rectangle.Intersect(foregroundRectangle, this.ForegroundImage.Bounds); int width = foregroundRectangle.Width; int height = foregroundRectangle.Height; @@ -111,7 +112,6 @@ internal class DrawImageProcessor : ImageProcessor } // Sanitize the dimensions so that we don't try and sample outside the image. - foregroundRectangle = Rectangle.Intersect(foregroundRectangle, this.ForegroundImage.Bounds); Rectangle backgroundRectangle = Rectangle.Intersect(new(left, top, width, height), this.SourceRectangle); Configuration configuration = this.Configuration; From b4049ce517ddd94fdbac398633f895957defe157 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 24 Feb 2024 00:12:15 +1000 Subject: [PATCH 21/37] Don't skip data read on animation and frame control chunks --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 9 +++++++-- tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs | 8 ++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Png/issues/Issue_2666.png | 3 +++ 4 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 tests/Images/Input/Png/issues/Issue_2666.png diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 95154d68d..179bf38ba 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -1871,8 +1871,13 @@ internal sealed class PngDecoderCore : IImageDecoderInternals PngChunkType type = this.ReadChunkType(buffer); // If we're reading color metadata only we're only interested in the IHDR and tRNS chunks. - // We can skip all other chunk data in the stream for better performance. - if (this.colorMetadataOnly && type != PngChunkType.Header && type != PngChunkType.Transparency && type != PngChunkType.Palette) + // We can skip most other chunk data in the stream for better performance. + if (this.colorMetadataOnly && + type != PngChunkType.Header && + type != PngChunkType.Transparency && + type != PngChunkType.Palette && + type != PngChunkType.AnimationControl && + type != PngChunkType.FrameControl) { chunk = new PngChunk(length, type); return true; diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index bc277bf48..743d1f797 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.IO; using System.Runtime.Intrinsics.X86; using Microsoft.DotNet.RemoteExecutor; using SixLabors.ImageSharp.Formats; @@ -665,4 +666,11 @@ public partial class PngDecoderTests Assert.True(eofHitCounter.EofHitCount <= 3); Assert.Equal(new Size(200, 120), eofHitCounter.Image.Size); } + + [Fact] + public void Decode_Issue2666() + { + string path = Path.GetFullPath(Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, TestImages.Png.Issue2666)); + using Image image = Image.Load(path); + } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 8aa95d349..82db31358 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -73,6 +73,7 @@ public static class TestImages public const string DisposeBackgroundRegion = "Png/animated/15-dispose-background-region.png"; public const string DisposePreviousFirst = "Png/animated/12-dispose-prev-first.png"; public const string BlendOverMultiple = "Png/animated/21-blend-over-multiple.png"; + public const string Issue2666 = "Png/Issues/Issue_2666.png"; // Filtered test images from http://www.schaik.com/pngsuite/pngsuite_fil_png.html public const string Filter0 = "Png/filter0.png"; diff --git a/tests/Images/Input/Png/issues/Issue_2666.png b/tests/Images/Input/Png/issues/Issue_2666.png new file mode 100644 index 000000000..b918fd474 --- /dev/null +++ b/tests/Images/Input/Png/issues/Issue_2666.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:ed7665cdfd5fad00c5995040350a254b96af6c0c95ab13975f2291e9d3fce0f3 +size 8244837 From 83ecbdb19fb180266a154896ccb9949f3bdd8c5d Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 26 Feb 2024 10:27:03 +1000 Subject: [PATCH 22/37] Fix path --- tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs | 1 - tests/ImageSharp.Tests/TestImages.cs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 743d1f797..b9f6075fb 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -1,7 +1,6 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -using System.IO; using System.Runtime.Intrinsics.X86; using Microsoft.DotNet.RemoteExecutor; using SixLabors.ImageSharp.Formats; diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 82db31358..4e33bbd8a 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -73,7 +73,7 @@ public static class TestImages public const string DisposeBackgroundRegion = "Png/animated/15-dispose-background-region.png"; public const string DisposePreviousFirst = "Png/animated/12-dispose-prev-first.png"; public const string BlendOverMultiple = "Png/animated/21-blend-over-multiple.png"; - public const string Issue2666 = "Png/Issues/Issue_2666.png"; + public const string Issue2666 = "Png/issues/Issue_2666.png"; // Filtered test images from http://www.schaik.com/pngsuite/pngsuite_fil_png.html public const string Filter0 = "Png/filter0.png"; From 3c65383dec1c0291824596a485c6840b8bf38e1a Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 26 Feb 2024 11:56:46 +1000 Subject: [PATCH 23/37] Prevent underflow --- src/ImageSharp/Formats/Webp/AlphaDecoder.cs | 10 ++++------ .../ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs | 11 +++++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Webp/issues/Issue2670.webp | 3 +++ 4 files changed, 19 insertions(+), 6 deletions(-) create mode 100644 tests/Images/Input/Webp/issues/Issue2670.webp diff --git a/src/ImageSharp/Formats/Webp/AlphaDecoder.cs b/src/ImageSharp/Formats/Webp/AlphaDecoder.cs index 63e654135..125ed0acd 100644 --- a/src/ImageSharp/Formats/Webp/AlphaDecoder.cs +++ b/src/ImageSharp/Formats/Webp/AlphaDecoder.cs @@ -6,6 +6,7 @@ using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.Intrinsics; +using System.Runtime.Intrinsics.Arm; using System.Runtime.Intrinsics.X86; using SixLabors.ImageSharp.Formats.Webp.BitReader; using SixLabors.ImageSharp.Formats.Webp.Lossless; @@ -311,18 +312,15 @@ internal class AlphaDecoder : IDisposable private static void HorizontalUnfilter(Span prev, Span input, Span dst, int width) { - if (Sse2.IsSupported) + // TODO: Investigate AdvSim support for this method. + if (Sse2.IsSupported && width >= 9) { dst[0] = (byte)(input[0] + (prev.IsEmpty ? 0 : prev[0])); - if (width <= 1) - { - return; - } - nuint i; Vector128 last = Vector128.Zero.WithElement(0, dst[0]); ref byte srcRef = ref MemoryMarshal.GetReference(input); ref byte dstRef = ref MemoryMarshal.GetReference(dst); + for (i = 1; i <= (uint)width - 8; i += 8) { Vector128 a0 = Vector128.Create(Unsafe.As(ref Unsafe.Add(ref srcRef, i)), 0); diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs index c38b13075..0dda304b6 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs @@ -439,6 +439,17 @@ public class WebpDecoderTests image.CompareToOriginal(provider, ReferenceDecoder); } + // https://github.com/SixLabors/ImageSharp/issues/2670 + [Theory] + [WithFile(Lossy.Issue2670, PixelTypes.Rgba32)] + public void WebpDecoder_CanDecode_Issue2670(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(WebpDecoder.Instance); + image.DebugSave(provider); + image.CompareToOriginal(provider, ReferenceDecoder); + } + [Theory] [WithFile(Lossless.LossLessCorruptImage3, PixelTypes.Rgba32)] public void WebpDecoder_ThrowImageFormatException_OnInvalidImages(TestImageProvider provider) diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 4e33bbd8a..b2085c683 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -804,6 +804,7 @@ public static class TestImages public const string Issue1594 = "Webp/issues/Issue1594.webp"; public const string Issue2243 = "Webp/issues/Issue2243.webp"; public const string Issue2257 = "Webp/issues/Issue2257.webp"; + public const string Issue2670 = "Webp/issues/Issue2670.webp"; } } diff --git a/tests/Images/Input/Webp/issues/Issue2670.webp b/tests/Images/Input/Webp/issues/Issue2670.webp new file mode 100644 index 000000000..4dd124898 --- /dev/null +++ b/tests/Images/Input/Webp/issues/Issue2670.webp @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:23ad5eb449f693af68e51dd108a6b9847a8eb48b82ca5b848395a54c2e0be08f +size 152 From 15619ecd88bc67b6bf4a002094ba241836f8f10a Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 26 Feb 2024 12:01:11 +1000 Subject: [PATCH 24/37] Fix file name --- tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs | 2 +- tests/ImageSharp.Tests/TestImages.cs | 2 +- .../Images/Input/Webp/issues/{Issue2670.webp => Issue2676.webp} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename tests/Images/Input/Webp/issues/{Issue2670.webp => Issue2676.webp} (100%) diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs index 0dda304b6..b0b7c294a 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs @@ -441,7 +441,7 @@ public class WebpDecoderTests // https://github.com/SixLabors/ImageSharp/issues/2670 [Theory] - [WithFile(Lossy.Issue2670, PixelTypes.Rgba32)] + [WithFile(Lossy.Issue2676, PixelTypes.Rgba32)] public void WebpDecoder_CanDecode_Issue2670(TestImageProvider provider) where TPixel : unmanaged, IPixel { diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index b2085c683..1eba38e4a 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -804,7 +804,7 @@ public static class TestImages public const string Issue1594 = "Webp/issues/Issue1594.webp"; public const string Issue2243 = "Webp/issues/Issue2243.webp"; public const string Issue2257 = "Webp/issues/Issue2257.webp"; - public const string Issue2670 = "Webp/issues/Issue2670.webp"; + public const string Issue2676 = "Webp/issues/Issue2676.webp"; } } diff --git a/tests/Images/Input/Webp/issues/Issue2670.webp b/tests/Images/Input/Webp/issues/Issue2676.webp similarity index 100% rename from tests/Images/Input/Webp/issues/Issue2670.webp rename to tests/Images/Input/Webp/issues/Issue2676.webp From 12d05b77cd3cbf1ceab13f8c60c62f8600060fbc Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 26 Feb 2024 12:04:03 +1000 Subject: [PATCH 25/37] Revert "Fix file name" This reverts commit 15619ecd88bc67b6bf4a002094ba241836f8f10a. --- tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs | 2 +- tests/ImageSharp.Tests/TestImages.cs | 2 +- .../Images/Input/Webp/issues/{Issue2676.webp => Issue2670.webp} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename tests/Images/Input/Webp/issues/{Issue2676.webp => Issue2670.webp} (100%) diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs index b0b7c294a..0dda304b6 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs @@ -441,7 +441,7 @@ public class WebpDecoderTests // https://github.com/SixLabors/ImageSharp/issues/2670 [Theory] - [WithFile(Lossy.Issue2676, PixelTypes.Rgba32)] + [WithFile(Lossy.Issue2670, PixelTypes.Rgba32)] public void WebpDecoder_CanDecode_Issue2670(TestImageProvider provider) where TPixel : unmanaged, IPixel { diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 1eba38e4a..b2085c683 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -804,7 +804,7 @@ public static class TestImages public const string Issue1594 = "Webp/issues/Issue1594.webp"; public const string Issue2243 = "Webp/issues/Issue2243.webp"; public const string Issue2257 = "Webp/issues/Issue2257.webp"; - public const string Issue2676 = "Webp/issues/Issue2676.webp"; + public const string Issue2670 = "Webp/issues/Issue2670.webp"; } } diff --git a/tests/Images/Input/Webp/issues/Issue2676.webp b/tests/Images/Input/Webp/issues/Issue2670.webp similarity index 100% rename from tests/Images/Input/Webp/issues/Issue2676.webp rename to tests/Images/Input/Webp/issues/Issue2670.webp From 223f6474c2035171de7fa4e43a8cd088f172e4a1 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 26 Feb 2024 12:04:37 +1000 Subject: [PATCH 26/37] Update AlphaDecoder.cs --- src/ImageSharp/Formats/Webp/AlphaDecoder.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ImageSharp/Formats/Webp/AlphaDecoder.cs b/src/ImageSharp/Formats/Webp/AlphaDecoder.cs index 125ed0acd..7033bf167 100644 --- a/src/ImageSharp/Formats/Webp/AlphaDecoder.cs +++ b/src/ImageSharp/Formats/Webp/AlphaDecoder.cs @@ -6,7 +6,6 @@ using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.Intrinsics; -using System.Runtime.Intrinsics.Arm; using System.Runtime.Intrinsics.X86; using SixLabors.ImageSharp.Formats.Webp.BitReader; using SixLabors.ImageSharp.Formats.Webp.Lossless; From 2a548818ac3c7a5e328c72757f5464007579d851 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 26 Feb 2024 16:35:28 +1000 Subject: [PATCH 27/37] Use a smarter approach to determine the transparent index --- src/ImageSharp/Formats/Png/PngEncoderCore.cs | 20 ++++++++++++++++++- .../Formats/Png/PngEncoderTests.cs | 17 ++++++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 3 +++ ...antized_Encode_Alpha_Rgba32_issue_2668.png | 3 +++ tests/Images/Input/Png/issues/Issue_2668.png | 3 +++ 5 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 tests/Images/External/ReferenceOutput/PngEncoderTests/Issue2668_Quantized_Encode_Alpha_Rgba32_issue_2668.png create mode 100644 tests/Images/Input/Png/issues/Issue_2668.png diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs index ddef1c9cd..ea94270f2 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs @@ -3,6 +3,7 @@ using System.Buffers; using System.Buffers.Binary; +using System.Numerics; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using SixLabors.ImageSharp.Common.Helpers; @@ -1527,7 +1528,24 @@ internal sealed class PngEncoderCore : IImageEncoderInternals, IDisposable { // We can use the color data from the decoded metadata here. // We avoid dithering by default to preserve the original colors. - this.derivedTransparencyIndex = metadata.ColorTable.Value.Span.IndexOf(Color.Transparent); + ReadOnlySpan palette = metadata.ColorTable.Value.Span; + + // Certain operations perform alpha premultiplication, which can cause the color to change so we + // must search for the transparency index in the palette. + // Transparent pixels are much more likely to be found at the end of a palette. + int index = -1; + for (int i = palette.Length - 1; i >= 0; i--) + { + Vector4 instance = palette[i].ToScaledVector4(); + if (instance.W == 0f) + { + index = i; + break; + } + } + + this.derivedTransparencyIndex = index; + this.quantizer = new PaletteQuantizer(metadata.ColorTable.Value, new() { Dither = null }, this.derivedTransparencyIndex); } else diff --git a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs index e70854b08..950f1d2e3 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs @@ -8,6 +8,7 @@ using SixLabors.ImageSharp.Formats.Png; using SixLabors.ImageSharp.Formats.Webp; using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Processing.Processors.Quantization; using SixLabors.ImageSharp.Tests.TestUtilities; using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; @@ -678,6 +679,22 @@ public partial class PngEncoderTests encoded.CompareToReferenceOutput(ImageComparer.Exact, provider); } + // https://github.com/SixLabors/ImageSharp/issues/2469 + [Theory] + [WithFile(TestImages.Png.Issue2668, PixelTypes.Rgba32)] + public void Issue2668_Quantized_Encode_Alpha(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(PngDecoder.Instance); + image.Mutate(x => x.Resize(100, 100)); + + PngEncoder encoder = new() { BitDepth = PngBitDepth.Bit8, ColorType = PngColorType.Palette }; + + string actualOutputFile = provider.Utility.SaveTestOutputFile(image, "png", encoder); + using Image encoded = Image.Load(actualOutputFile); + encoded.CompareToReferenceOutput(ImageComparer.Exact, provider); + } + private static void TestPngEncoderCore( TestImageProvider provider, PngColorType pngColorType, diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 4e33bbd8a..a6532a090 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -151,6 +151,9 @@ public static class TestImages // Issue 2447: https://github.com/SixLabors/ImageSharp/issues/2447 public const string Issue2447 = "Png/issues/issue_2447.png"; + // Issue 2668: https://github.com/SixLabors/ImageSharp/issues/2668 + public const string Issue2668 = "Png/issues/issue_2668.png"; + public static class Bad { public const string MissingDataChunk = "Png/xdtn0g01.png"; diff --git a/tests/Images/External/ReferenceOutput/PngEncoderTests/Issue2668_Quantized_Encode_Alpha_Rgba32_issue_2668.png b/tests/Images/External/ReferenceOutput/PngEncoderTests/Issue2668_Quantized_Encode_Alpha_Rgba32_issue_2668.png new file mode 100644 index 000000000..7af5391f7 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/PngEncoderTests/Issue2668_Quantized_Encode_Alpha_Rgba32_issue_2668.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:f934af128b85b9e8f557d71ac8b1f1473a0922d0754fc0c4ece0d0e3d8d94c39 +size 7702 diff --git a/tests/Images/Input/Png/issues/Issue_2668.png b/tests/Images/Input/Png/issues/Issue_2668.png new file mode 100644 index 000000000..2ca8c4617 --- /dev/null +++ b/tests/Images/Input/Png/issues/Issue_2668.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:e8e5b2b933fd8fefd161f1d22970cb60247fd2d93b6c07b8b9ee1fdbc2241a3c +size 390225 From ed2cd3184ff9478f265fbb4ec40482d44e6d971d Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 26 Feb 2024 20:38:29 +1000 Subject: [PATCH 28/37] Fix casing --- tests/ImageSharp.Tests/TestImages.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index a6532a090..3d9da1072 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -152,7 +152,7 @@ public static class TestImages public const string Issue2447 = "Png/issues/issue_2447.png"; // Issue 2668: https://github.com/SixLabors/ImageSharp/issues/2668 - public const string Issue2668 = "Png/issues/issue_2668.png"; + public const string Issue2668 = "Png/issues/Issue_2668.png"; public static class Bad { From f6d24ed15c94a5a7ca20bdcbb3b4a52c15564544 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 26 Feb 2024 20:49:13 +1000 Subject: [PATCH 29/37] Update src/ImageSharp/Formats/Webp/AlphaDecoder.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Günther Foidl --- src/ImageSharp/Formats/Webp/AlphaDecoder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Webp/AlphaDecoder.cs b/src/ImageSharp/Formats/Webp/AlphaDecoder.cs index 7033bf167..63571617f 100644 --- a/src/ImageSharp/Formats/Webp/AlphaDecoder.cs +++ b/src/ImageSharp/Formats/Webp/AlphaDecoder.cs @@ -311,7 +311,7 @@ internal class AlphaDecoder : IDisposable private static void HorizontalUnfilter(Span prev, Span input, Span dst, int width) { - // TODO: Investigate AdvSim support for this method. + // TODO: Investigate AdvSimd support for this method. if (Sse2.IsSupported && width >= 9) { dst[0] = (byte)(input[0] + (prev.IsEmpty ? 0 : prev[0])); From 624277ba8803fc8b8062139c44cefe88118473a5 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 26 Feb 2024 21:01:40 +1000 Subject: [PATCH 30/37] Update Issue2668_Quantized_Encode_Alpha_Rgba32_Issue_2668.png --- ...png => Issue2668_Quantized_Encode_Alpha_Rgba32_Issue_2668.png} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/Images/External/ReferenceOutput/PngEncoderTests/{Issue2668_Quantized_Encode_Alpha_Rgba32_issue_2668.png => Issue2668_Quantized_Encode_Alpha_Rgba32_Issue_2668.png} (100%) diff --git a/tests/Images/External/ReferenceOutput/PngEncoderTests/Issue2668_Quantized_Encode_Alpha_Rgba32_issue_2668.png b/tests/Images/External/ReferenceOutput/PngEncoderTests/Issue2668_Quantized_Encode_Alpha_Rgba32_Issue_2668.png similarity index 100% rename from tests/Images/External/ReferenceOutput/PngEncoderTests/Issue2668_Quantized_Encode_Alpha_Rgba32_issue_2668.png rename to tests/Images/External/ReferenceOutput/PngEncoderTests/Issue2668_Quantized_Encode_Alpha_Rgba32_Issue_2668.png From f44b761f3b6731e2c20b56f10515962178ed958c Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 27 Feb 2024 11:09:36 +1000 Subject: [PATCH 31/37] Add fixes 2668, 2676, and 2677 to main (#2678) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Prevent underflow * Fix file name * Revert "Fix file name" This reverts commit 15619ecd88bc67b6bf4a002094ba241836f8f10a. * Update AlphaDecoder.cs * Use a smarter approach to determine the transparent index * Fix casing * Update src/ImageSharp/Formats/Webp/AlphaDecoder.cs Co-authored-by: Günther Foidl * Update Issue2668_Quantized_Encode_Alpha_Rgba32_Issue_2668.png * Normalize Color API * Aggressive inlining --------- Co-authored-by: Günther Foidl --- src/ImageSharp/Color/Color.cs | 93 +++++++++---------- .../Formats/Gif/GifFrameMetadata.cs | 4 +- src/ImageSharp/Formats/Png/PngEncoderCore.cs | 20 +++- src/ImageSharp/Formats/Webp/AlphaDecoder.cs | 9 +- .../Color/ColorTests.CastTo.cs | 2 +- .../Formats/Png/PngEncoderTests.cs | 17 ++++ .../Formats/WebP/WebpDecoderTests.cs | 11 +++ tests/ImageSharp.Tests/TestImages.cs | 4 + ...antized_Encode_Alpha_Rgba32_Issue_2668.png | 3 + tests/Images/Input/Png/issues/Issue_2668.png | 3 + tests/Images/Input/Webp/issues/Issue2670.webp | 3 + 11 files changed, 112 insertions(+), 57 deletions(-) create mode 100644 tests/Images/External/ReferenceOutput/PngEncoderTests/Issue2668_Quantized_Encode_Alpha_Rgba32_Issue_2668.png create mode 100644 tests/Images/Input/Png/issues/Issue_2668.png create mode 100644 tests/Images/Input/Webp/issues/Issue2670.webp diff --git a/src/ImageSharp/Color/Color.cs b/src/ImageSharp/Color/Color.cs index 82ecab390..8f54680ec 100644 --- a/src/ImageSharp/Color/Color.cs +++ b/src/ImageSharp/Color/Color.cs @@ -25,7 +25,7 @@ public readonly partial struct Color : IEquatable /// Initializes a new instance of the struct. /// /// The containing the color information. - [MethodImpl(InliningOptions.ShortMethod)] + [MethodImpl(MethodImplOptions.AggressiveInlining)] private Color(Vector4 vector) { this.data = Numerics.Clamp(vector, Vector4.Zero, Vector4.One); @@ -36,28 +36,13 @@ public readonly partial struct Color : IEquatable /// Initializes a new instance of the struct. /// /// The pixel containing color information. - [MethodImpl(InliningOptions.ShortMethod)] + [MethodImpl(MethodImplOptions.AggressiveInlining)] private Color(IPixel pixel) { this.boxedHighPrecisionPixel = pixel; this.data = default; } - /// - /// Converts a to . - /// - /// The . - /// The . - public static explicit operator Vector4(Color color) => color.ToScaledVector4(); - - /// - /// Converts an to . - /// - /// The . - /// The . - [MethodImpl(InliningOptions.ShortMethod)] - public static explicit operator Color(Vector4 source) => new(source); - /// /// Checks whether two structures are equal. /// @@ -67,7 +52,7 @@ public readonly partial struct Color : IEquatable /// True if the parameter is equal to the parameter; /// otherwise, false. /// - [MethodImpl(InliningOptions.ShortMethod)] + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static bool operator ==(Color left, Color right) => left.Equals(right); /// @@ -79,36 +64,44 @@ public readonly partial struct Color : IEquatable /// True if the parameter is not equal to the parameter; /// otherwise, false. /// - [MethodImpl(InliningOptions.ShortMethod)] + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static bool operator !=(Color left, Color right) => !left.Equals(right); /// /// Creates a from the given . /// - /// The pixel to convert from. + /// The pixel to convert from. /// The pixel format. /// The . - [MethodImpl(InliningOptions.ShortMethod)] - public static Color FromPixel(TPixel pixel) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static Color FromPixel(TPixel source) where TPixel : unmanaged, IPixel { // Avoid boxing in case we can convert to Vector4 safely and efficiently PixelTypeInfo info = TPixel.GetPixelTypeInfo(); if (info.ComponentInfo.HasValue && info.ComponentInfo.Value.GetMaximumComponentPrecision() <= (int)PixelComponentBitDepth.Bit32) { - return new(pixel.ToScaledVector4()); + return new(source.ToScaledVector4()); } - return new(pixel); + return new(source); } + /// + /// Creates a from a generic scaled . + /// + /// The vector to load the pixel from. + /// The . + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static Color FromScaledVector(Vector4 source) => new(source); + /// /// Bulk converts a span of a specified type to a span of . /// /// The pixel type to convert to. /// The source pixel span. /// The destination color span. - [MethodImpl(InliningOptions.ShortMethod)] + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static void FromPixel(ReadOnlySpan source, Span destination) where TPixel : unmanaged, IPixel { @@ -120,7 +113,7 @@ public readonly partial struct Color : IEquatable { for (int i = 0; i < destination.Length; i++) { - destination[i] = new(source[i].ToScaledVector4()); + destination[i] = FromScaledVector(source[i].ToScaledVector4()); } } else @@ -143,7 +136,7 @@ public readonly partial struct Color : IEquatable /// /// The . /// - [MethodImpl(InliningOptions.ShortMethod)] + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Color ParseHex(string hex) { Rgba32 rgba = Rgba32.ParseHex(hex); @@ -162,7 +155,7 @@ public readonly partial struct Color : IEquatable /// /// The . /// - [MethodImpl(InliningOptions.ShortMethod)] + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static bool TryParseHex(string hex, out Color result) { result = default; @@ -236,16 +229,16 @@ public readonly partial struct Color : IEquatable /// The color having it's alpha channel altered. public Color WithAlpha(float alpha) { - Vector4 v = (Vector4)this; + Vector4 v = this.ToScaledVector4(); v.W = alpha; - return new Color(v); + return FromScaledVector(v); } /// /// Gets the hexadecimal representation of the color instance in rrggbbaa form. /// /// A hexadecimal string representation of the value. - [MethodImpl(InliningOptions.ShortMethod)] + [MethodImpl(MethodImplOptions.AggressiveInlining)] public string ToHex() { if (this.boxedHighPrecisionPixel is not null) @@ -263,8 +256,8 @@ public readonly partial struct Color : IEquatable /// Converts the color instance to a specified type. /// /// The pixel type to convert to. - /// The pixel value. - [MethodImpl(InliningOptions.ShortMethod)] + /// The . + [MethodImpl(MethodImplOptions.AggressiveInlining)] public TPixel ToPixel() where TPixel : unmanaged, IPixel { @@ -281,13 +274,30 @@ public readonly partial struct Color : IEquatable return TPixel.FromScaledVector4(this.boxedHighPrecisionPixel.ToScaledVector4()); } + /// + /// Expands the color into a generic ("scaled") representation + /// with values scaled and clamped between 0 and 1. + /// The vector components are typically expanded in least to greatest significance order. + /// + /// The . + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public Vector4 ToScaledVector4() + { + if (this.boxedHighPrecisionPixel is null) + { + return this.data; + } + + return this.boxedHighPrecisionPixel.ToScaledVector4(); + } + /// /// Bulk converts a span of to a span of a specified type. /// /// The pixel type to convert to. /// The source color span. /// The destination pixel span. - [MethodImpl(InliningOptions.ShortMethod)] + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static void ToPixel(ReadOnlySpan source, Span destination) where TPixel : unmanaged, IPixel { @@ -301,7 +311,7 @@ public readonly partial struct Color : IEquatable } /// - [MethodImpl(InliningOptions.ShortMethod)] + [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool Equals(Color other) { if (this.boxedHighPrecisionPixel is null && other.boxedHighPrecisionPixel is null) @@ -316,7 +326,7 @@ public readonly partial struct Color : IEquatable public override bool Equals(object? obj) => obj is Color other && this.Equals(other); /// - [MethodImpl(InliningOptions.ShortMethod)] + [MethodImpl(MethodImplOptions.AggressiveInlining)] public override int GetHashCode() { if (this.boxedHighPrecisionPixel is null) @@ -326,15 +336,4 @@ public readonly partial struct Color : IEquatable return this.boxedHighPrecisionPixel.GetHashCode(); } - - [MethodImpl(InliningOptions.ShortMethod)] - private Vector4 ToScaledVector4() - { - if (this.boxedHighPrecisionPixel is null) - { - return this.data; - } - - return this.boxedHighPrecisionPixel.ToScaledVector4(); - } } diff --git a/src/ImageSharp/Formats/Gif/GifFrameMetadata.cs b/src/ImageSharp/Formats/Gif/GifFrameMetadata.cs index f8734bb5a..6598def2a 100644 --- a/src/ImageSharp/Formats/Gif/GifFrameMetadata.cs +++ b/src/ImageSharp/Formats/Gif/GifFrameMetadata.cs @@ -82,13 +82,13 @@ public class GifFrameMetadata : IDeepCloneable { // TODO: v4 How do I link the parent metadata to the frame metadata to get the global color table? int index = -1; - float background = 1f; + const float background = 1f; if (metadata.ColorTable.HasValue) { ReadOnlySpan colorTable = metadata.ColorTable.Value.Span; for (int i = 0; i < colorTable.Length; i++) { - Vector4 vector = (Vector4)colorTable[i]; + Vector4 vector = colorTable[i].ToScaledVector4(); if (vector.W < background) { index = i; diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs index 993f53269..113fef595 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs @@ -4,6 +4,7 @@ using System.Buffers; using System.Buffers.Binary; using System.IO.Hashing; +using System.Numerics; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using SixLabors.ImageSharp.Common.Helpers; @@ -1559,7 +1560,24 @@ internal sealed class PngEncoderCore : IImageEncoderInternals, IDisposable { // We can use the color data from the decoded metadata here. // We avoid dithering by default to preserve the original colors. - this.derivedTransparencyIndex = metadata.ColorTable.Value.Span.IndexOf(Color.Transparent); + ReadOnlySpan palette = metadata.ColorTable.Value.Span; + + // Certain operations perform alpha premultiplication, which can cause the color to change so we + // must search for the transparency index in the palette. + // Transparent pixels are much more likely to be found at the end of a palette. + int index = -1; + for (int i = palette.Length - 1; i >= 0; i--) + { + Vector4 instance = palette[i].ToScaledVector4(); + if (instance.W == 0f) + { + index = i; + break; + } + } + + this.derivedTransparencyIndex = index; + this.quantizer = new PaletteQuantizer(metadata.ColorTable.Value, new() { Dither = null }, this.derivedTransparencyIndex); } else diff --git a/src/ImageSharp/Formats/Webp/AlphaDecoder.cs b/src/ImageSharp/Formats/Webp/AlphaDecoder.cs index 63e654135..63571617f 100644 --- a/src/ImageSharp/Formats/Webp/AlphaDecoder.cs +++ b/src/ImageSharp/Formats/Webp/AlphaDecoder.cs @@ -311,18 +311,15 @@ internal class AlphaDecoder : IDisposable private static void HorizontalUnfilter(Span prev, Span input, Span dst, int width) { - if (Sse2.IsSupported) + // TODO: Investigate AdvSimd support for this method. + if (Sse2.IsSupported && width >= 9) { dst[0] = (byte)(input[0] + (prev.IsEmpty ? 0 : prev[0])); - if (width <= 1) - { - return; - } - nuint i; Vector128 last = Vector128.Zero.WithElement(0, dst[0]); ref byte srcRef = ref MemoryMarshal.GetReference(input); ref byte dstRef = ref MemoryMarshal.GetReference(dst); + for (i = 1; i <= (uint)width - 8; i += 8) { Vector128 a0 = Vector128.Create(Unsafe.As(ref Unsafe.Add(ref srcRef, i)), 0); diff --git a/tests/ImageSharp.Tests/Color/ColorTests.CastTo.cs b/tests/ImageSharp.Tests/Color/ColorTests.CastTo.cs index 14a5a44f1..4247345c7 100644 --- a/tests/ImageSharp.Tests/Color/ColorTests.CastTo.cs +++ b/tests/ImageSharp.Tests/Color/ColorTests.CastTo.cs @@ -105,7 +105,7 @@ public partial class ColorTests public void Vector4Constructor() { // Act: - Color color = (Color)Vector4.One; + Color color = Color.FromScaledVector(Vector4.One); // Assert: Assert.Equal(new RgbaVector(1, 1, 1, 1), color.ToPixel()); diff --git a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs index 0fdd49630..a70fb86df 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs @@ -8,6 +8,7 @@ using SixLabors.ImageSharp.Formats.Png; using SixLabors.ImageSharp.Formats.Webp; using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Processing.Processors.Quantization; using SixLabors.ImageSharp.Tests.TestUtilities; using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; @@ -679,6 +680,22 @@ public partial class PngEncoderTests encoded.CompareToReferenceOutput(ImageComparer.Exact, provider); } + // https://github.com/SixLabors/ImageSharp/issues/2469 + [Theory] + [WithFile(TestImages.Png.Issue2668, PixelTypes.Rgba32)] + public void Issue2668_Quantized_Encode_Alpha(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(PngDecoder.Instance); + image.Mutate(x => x.Resize(100, 100)); + + PngEncoder encoder = new() { BitDepth = PngBitDepth.Bit8, ColorType = PngColorType.Palette }; + + string actualOutputFile = provider.Utility.SaveTestOutputFile(image, "png", encoder); + using Image encoded = Image.Load(actualOutputFile); + encoded.CompareToReferenceOutput(ImageComparer.Exact, provider); + } + private static void TestPngEncoderCore( TestImageProvider provider, PngColorType pngColorType, diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs index c38b13075..0dda304b6 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs @@ -439,6 +439,17 @@ public class WebpDecoderTests image.CompareToOriginal(provider, ReferenceDecoder); } + // https://github.com/SixLabors/ImageSharp/issues/2670 + [Theory] + [WithFile(Lossy.Issue2670, PixelTypes.Rgba32)] + public void WebpDecoder_CanDecode_Issue2670(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(WebpDecoder.Instance); + image.DebugSave(provider); + image.CompareToOriginal(provider, ReferenceDecoder); + } + [Theory] [WithFile(Lossless.LossLessCorruptImage3, PixelTypes.Rgba32)] public void WebpDecoder_ThrowImageFormatException_OnInvalidImages(TestImageProvider provider) diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 933fcc4fa..022c3654a 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -151,6 +151,9 @@ public static class TestImages // Issue 2447: https://github.com/SixLabors/ImageSharp/issues/2447 public const string Issue2447 = "Png/issues/issue_2447.png"; + // Issue 2668: https://github.com/SixLabors/ImageSharp/issues/2668 + public const string Issue2668 = "Png/issues/Issue_2668.png"; + public static class Bad { public const string MissingDataChunk = "Png/xdtn0g01.png"; @@ -806,6 +809,7 @@ public static class TestImages public const string Issue1594 = "Webp/issues/Issue1594.webp"; public const string Issue2243 = "Webp/issues/Issue2243.webp"; public const string Issue2257 = "Webp/issues/Issue2257.webp"; + public const string Issue2670 = "Webp/issues/Issue2670.webp"; } } diff --git a/tests/Images/External/ReferenceOutput/PngEncoderTests/Issue2668_Quantized_Encode_Alpha_Rgba32_Issue_2668.png b/tests/Images/External/ReferenceOutput/PngEncoderTests/Issue2668_Quantized_Encode_Alpha_Rgba32_Issue_2668.png new file mode 100644 index 000000000..7af5391f7 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/PngEncoderTests/Issue2668_Quantized_Encode_Alpha_Rgba32_Issue_2668.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:f934af128b85b9e8f557d71ac8b1f1473a0922d0754fc0c4ece0d0e3d8d94c39 +size 7702 diff --git a/tests/Images/Input/Png/issues/Issue_2668.png b/tests/Images/Input/Png/issues/Issue_2668.png new file mode 100644 index 000000000..2ca8c4617 --- /dev/null +++ b/tests/Images/Input/Png/issues/Issue_2668.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:e8e5b2b933fd8fefd161f1d22970cb60247fd2d93b6c07b8b9ee1fdbc2241a3c +size 390225 diff --git a/tests/Images/Input/Webp/issues/Issue2670.webp b/tests/Images/Input/Webp/issues/Issue2670.webp new file mode 100644 index 000000000..4dd124898 --- /dev/null +++ b/tests/Images/Input/Webp/issues/Issue2670.webp @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:23ad5eb449f693af68e51dd108a6b9847a8eb48b82ca5b848395a54c2e0be08f +size 152 From 8af9a8068e4b4f54a0a33438fc9937609f28ed79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Mutnia=C5=84ski?= Date: Wed, 28 Feb 2024 11:01:01 +0100 Subject: [PATCH 32/37] PR comments changes --- .../Formats/Jpeg/JpegDecoderCore.cs | 11 ++-- .../Formats/Jpeg/JpegEncoderCore.cs | 53 ++++++++++++------- .../Formats/Jpg/JpegEncoderTests.Metadata.cs | 23 ++++++++ 3 files changed, 65 insertions(+), 22 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index e43c20a7d..cf5e449e7 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -523,13 +523,16 @@ internal sealed class JpegDecoderCore : IRawJpegData, IImageDecoderInternals /// The remaining bytes in the segment block. private void ProcessComMarker(BufferedReadStream stream, int markerContentByteSize) { - Span temp = new byte[markerContentByteSize]; + char[] temp = new char[markerContentByteSize]; JpegMetadata metadata = this.Metadata.GetFormatMetadata(JpegFormat.Instance); - stream.Read(temp); - string comment = Encoding.ASCII.GetString(temp); + for (int i = 0; i < markerContentByteSize; i++) + { + int read = stream.ReadByte(); + temp[i] = (char)read; + } - metadata.Comments.Add(JpegComData.FromString(comment)); + metadata.Comments.Add(new JpegComData(temp)); } /// diff --git a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs index a55435c53..4ef4cea2d 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs @@ -3,6 +3,7 @@ #nullable disable using System.Buffers.Binary; +using System.Collections; using System.Text; using SixLabors.ImageSharp.Common.Helpers; using SixLabors.ImageSharp.Formats.Jpeg.Components; @@ -184,39 +185,55 @@ internal sealed unsafe partial class JpegEncoderCore : IImageEncoderInternals return; } - for (int i = 0; i < metadata.Comments.Count; i++) + // We don't want to modify original metadata + List comments = new(metadata.Comments); + + int totalPayloadLength = 0; + for (int i = 0; i < comments.Count; i++) { - string comment = metadata.Comments[i].ToString(); + JpegComData comment = comments[i]; + ReadOnlyMemory currentComment = comment.Value; - if (comment.Length > maxCommentLength) + if (comment.Value.Length > maxCommentLength) { - string splitComment = comment.Substring(maxCommentLength, comment.Length - maxCommentLength); - metadata.Comments.Insert(i + 1, JpegComData.FromString(splitComment)); + ReadOnlyMemory splitComment = + currentComment.Slice(maxCommentLength, currentComment.Length - maxCommentLength); + comments.Insert(i + 1, new JpegComData(splitComment)); // We don't want to keep the extra bytes - comment = comment.Substring(0, maxCommentLength); + comments[i] = new JpegComData(currentComment.Slice(0, maxCommentLength)); } - int commentLength = comment.Length + 4; + totalPayloadLength += comment.Value.Length + 4; + } + + Span payload = new byte[totalPayloadLength]; + int currentCommentStartingIndex = 0; - Span commentSpan = new byte[commentLength]; - Span markers = commentSpan.Slice(0, 2); - Span payloadSize = commentSpan.Slice(2, 2); - Span payload = commentSpan.Slice(4, comment.Length); + for (int i = 0; i < comments.Count; i++) + { + ReadOnlyMemory comment = comments[i].Value; // Beginning of comment ff fe - markers[0] = JpegConstants.Markers.XFF; - markers[1] = JpegConstants.Markers.COM; + payload[currentCommentStartingIndex] = JpegConstants.Markers.XFF; + payload[currentCommentStartingIndex + 1] = JpegConstants.Markers.COM; // Write payload size - int comWithoutMarker = commentLength - 2; - payloadSize[0] = (byte)((comWithoutMarker >> 8) & 0xFF); - payloadSize[1] = (byte)(comWithoutMarker & 0xFF); + int comWithoutMarker = comment.Length + 2; + payload[currentCommentStartingIndex + 2] = (byte)((comWithoutMarker >> 8) & 0xFF); + payload[currentCommentStartingIndex + 3] = (byte)(comWithoutMarker & 0xFF); - Encoding.ASCII.GetBytes(comment, payload); + char[] commentChars = comment.ToArray(); + for (int j = 0; j < commentChars.Length; j++) + { + // Initial 4 bytes are always reserved + payload[4 + currentCommentStartingIndex + j] = (byte)commentChars[j]; + } - this.outputStream.Write(commentSpan, 0, commentSpan.Length); + currentCommentStartingIndex += comment.Length + 4; } + + this.outputStream.Write(payload, 0, payload.Length); } /// diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs index e49afedbb..f0593b462 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs @@ -198,6 +198,29 @@ public partial class JpegEncoderTests Assert.Equal(meta.Comments.ElementAtOrDefault(1).ToString(), actual.Comments.ElementAtOrDefault(1).ToString()); } + [Fact] + public void Encode_SaveTooLongComment() + { + // arrange + string longString = new('c', 65534); + using var input = new Image(1, 1); + JpegMetadata meta = input.Metadata.GetJpegMetadata(); + using var memStream = new MemoryStream(); + + // act + meta.Comments.Add(JpegComData.FromString(longString)); + input.Save(memStream, JpegEncoder); + + // assert + memStream.Position = 0; + using Image output = Image.Load(memStream); + JpegMetadata actual = output.Metadata.GetJpegMetadata(); + Assert.NotEmpty(actual.Comments); + Assert.Equal(2, actual.Comments.Count); + Assert.Equal(longString[..65533], actual.Comments.ElementAtOrDefault(0).ToString()); + Assert.Equal("c", actual.Comments.ElementAtOrDefault(1).ToString()); + } + [Theory] [WithFile(TestImages.Jpeg.Baseline.Floorplan, PixelTypes.Rgb24, JpegEncodingColor.Luminance)] [WithFile(TestImages.Jpeg.Baseline.Jpeg444, PixelTypes.Rgb24, JpegEncodingColor.YCbCrRatio444)] From edc87ad90d4fcb0783c0c38be01a91a0dd26cd28 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 1 Mar 2024 20:18:35 +1000 Subject: [PATCH 33/37] Limit ancillary chunk size. --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 38 +++++++++---------- .../Formats/Png/PngDecoderOptions.cs | 6 +++ .../Formats/Png/PngDecoderTests.cs | 19 ++++++++++ tests/ImageSharp.Tests/TestImages.cs | 4 +- tests/Images/Input/Png/issues/bad-ztxt.png | 3 ++ tests/Images/Input/Png/issues/bad-ztxt2.png | 3 ++ 6 files changed, 52 insertions(+), 21 deletions(-) create mode 100644 tests/Images/Input/Png/issues/bad-ztxt.png create mode 100644 tests/Images/Input/Png/issues/bad-ztxt2.png diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 179bf38ba..a96c53c10 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -120,6 +120,11 @@ internal sealed class PngDecoderCore : IImageDecoderInternals /// private readonly PngCrcChunkHandling pngCrcChunkHandling; + /// + /// The maximum memory in bytes that a zTXt, sPLT, iTXt, iCCP, or unknown chunk can occupy when decompressed. + /// + private readonly int maxUncompressedLength; + /// /// Initializes a new instance of the class. /// @@ -132,6 +137,7 @@ internal sealed class PngDecoderCore : IImageDecoderInternals this.skipMetadata = options.GeneralOptions.SkipMetadata; this.memoryAllocator = this.configuration.MemoryAllocator; this.pngCrcChunkHandling = options.PngCrcChunkHandling; + this.maxUncompressedLength = options.MaxUncompressedAncillaryChunkSizeBytes; } internal PngDecoderCore(PngDecoderOptions options, bool colorMetadataOnly) @@ -143,6 +149,7 @@ internal sealed class PngDecoderCore : IImageDecoderInternals this.configuration = options.GeneralOptions.Configuration; this.memoryAllocator = this.configuration.MemoryAllocator; this.pngCrcChunkHandling = options.PngCrcChunkHandling; + this.maxUncompressedLength = options.MaxUncompressedAncillaryChunkSizeBytes; } /// @@ -596,23 +603,7 @@ internal sealed class PngDecoderCore : IImageDecoderInternals private void InitializeImage(ImageMetadata metadata, FrameControl frameControl, out Image image) where TPixel : unmanaged, IPixel { - // When ignoring data CRCs, we can't use the image constructor that leaves the buffer uncleared. - if (this.pngCrcChunkHandling is PngCrcChunkHandling.IgnoreData or PngCrcChunkHandling.IgnoreAll) - { - image = new Image( - this.configuration, - this.header.Width, - this.header.Height, - metadata); - } - else - { - image = Image.CreateUninitialized( - this.configuration, - this.header.Width, - this.header.Height, - metadata); - } + image = new Image(this.configuration, this.header.Width, this.header.Height, metadata); PngFrameMetadata frameMetadata = image.Frames.RootFrame.Metadata.GetPngMetadata(); frameMetadata.FromChunk(in frameControl); @@ -1572,7 +1563,7 @@ internal sealed class PngDecoderCore : IImageDecoderInternals ReadOnlySpan compressedData = data[(zeroIndex + 2)..]; - if (this.TryDecompressZlibData(compressedData, out byte[] iccpProfileBytes)) + if (this.TryDecompressZlibData(compressedData, this.maxUncompressedLength, out byte[] iccpProfileBytes)) { metadata.IccProfile = new IccProfile(iccpProfileBytes); } @@ -1582,9 +1573,10 @@ internal sealed class PngDecoderCore : IImageDecoderInternals /// Tries to decompress zlib compressed data. /// /// The compressed data. + /// The maximum uncompressed length. /// The uncompressed bytes array. /// True, if de-compressing was successful. - private unsafe bool TryDecompressZlibData(ReadOnlySpan compressedData, out byte[] uncompressedBytesArray) + private unsafe bool TryDecompressZlibData(ReadOnlySpan compressedData, int maxLength, out byte[] uncompressedBytesArray) { fixed (byte* compressedDataBase = compressedData) { @@ -1604,6 +1596,12 @@ internal sealed class PngDecoderCore : IImageDecoderInternals int bytesRead = inflateStream.CompressedStream.Read(destUncompressedData, 0, destUncompressedData.Length); while (bytesRead != 0) { + if (memoryStreamOutput.Length > maxLength) + { + uncompressedBytesArray = Array.Empty(); + return false; + } + memoryStreamOutput.Write(destUncompressedData[..bytesRead]); bytesRead = inflateStream.CompressedStream.Read(destUncompressedData, 0, destUncompressedData.Length); } @@ -1746,7 +1744,7 @@ internal sealed class PngDecoderCore : IImageDecoderInternals /// The . private bool TryDecompressTextData(ReadOnlySpan compressedData, Encoding encoding, [NotNullWhen(true)] out string? value) { - if (this.TryDecompressZlibData(compressedData, out byte[] uncompressedData)) + if (this.TryDecompressZlibData(compressedData, this.maxUncompressedLength, out byte[] uncompressedData)) { value = encoding.GetString(uncompressedData); return true; diff --git a/src/ImageSharp/Formats/Png/PngDecoderOptions.cs b/src/ImageSharp/Formats/Png/PngDecoderOptions.cs index ab6ba4770..abfa4b1da 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderOptions.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderOptions.cs @@ -15,4 +15,10 @@ public sealed class PngDecoderOptions : ISpecializedDecoderOptions /// Gets a value indicating how to handle validation of any CRC (Cyclic Redundancy Check) data within the encoded PNG. /// public PngCrcChunkHandling PngCrcChunkHandling { get; init; } = PngCrcChunkHandling.IgnoreNonCritical; + + /// + /// Gets the maximum memory in bytes that a zTXt, sPLT, iTXt, iCCP, or unknown chunk can occupy when decompressed. + /// Defaults to 8MB + /// + public int MaxUncompressedAncillaryChunkSizeBytes { get; init; } = 8 * 1024 * 1024; // 8MB } diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index b9f6075fb..9b165526e 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -672,4 +672,23 @@ public partial class PngDecoderTests string path = Path.GetFullPath(Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, TestImages.Png.Issue2666)); using Image image = Image.Load(path); } + + [Theory] + + [InlineData(TestImages.Png.Bad.BadZTXT)] + [InlineData(TestImages.Png.Bad.BadZTXT2)] + public void Decode_BadZTXT(string file) + { + string path = Path.GetFullPath(Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, file)); + using Image image = Image.Load(path); + } + + [Theory] + [InlineData(TestImages.Png.Bad.BadZTXT)] + [InlineData(TestImages.Png.Bad.BadZTXT2)] + public void Info_BadZTXT(string file) + { + string path = Path.GetFullPath(Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, file)); + _ = Image.Identify(path); + } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index e72f6fad8..00f7370f3 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -186,8 +186,10 @@ public static class TestImages // Invalid color type. public const string ColorTypeOne = "Png/xc1n0g08.png"; public const string ColorTypeNine = "Png/xc9n2c08.png"; - public const string FlagOfGermany0000016446 = "Png/issues/flag_of_germany-0000016446.png"; + + public const string BadZTXT = "Png/issues/bad-ztxt.png"; + public const string BadZTXT2 = "Png/issues/bad-ztxt2.png"; } } diff --git a/tests/Images/Input/Png/issues/bad-ztxt.png b/tests/Images/Input/Png/issues/bad-ztxt.png new file mode 100644 index 000000000..710f888d0 --- /dev/null +++ b/tests/Images/Input/Png/issues/bad-ztxt.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:132a70cf0ac458a55cf4a44f4c6c025587491d304595835959955de6682fa472 +size 3913750 diff --git a/tests/Images/Input/Png/issues/bad-ztxt2.png b/tests/Images/Input/Png/issues/bad-ztxt2.png new file mode 100644 index 000000000..958c00e3f --- /dev/null +++ b/tests/Images/Input/Png/issues/bad-ztxt2.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:778a5fc8e915d79e9f55e58c6e4f646ae55dd7e866e65960754cb67a2b445987 +size 93 From 3f22857a80c854791837333e3e5caca38d2d1363 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 1 Mar 2024 23:49:55 +1000 Subject: [PATCH 34/37] Allow nightlies from previous releases --- .github/workflows/build-and-test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index b37557401..5a2e11266 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -4,6 +4,7 @@ on: push: branches: - main + - release/* tags: - "v*" pull_request: From 0ac9f3db4daccc1b768d8f6b795a0f4da2f780f5 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 7 Mar 2024 17:35:04 +1000 Subject: [PATCH 35/37] Update AlphaDecoder.cs --- src/ImageSharp/Formats/Webp/AlphaDecoder.cs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/ImageSharp/Formats/Webp/AlphaDecoder.cs b/src/ImageSharp/Formats/Webp/AlphaDecoder.cs index 63571617f..eccd9ede8 100644 --- a/src/ImageSharp/Formats/Webp/AlphaDecoder.cs +++ b/src/ImageSharp/Formats/Webp/AlphaDecoder.cs @@ -6,7 +6,9 @@ using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.Intrinsics; +using System.Runtime.Intrinsics.Arm; using System.Runtime.Intrinsics.X86; +using SixLabors.ImageSharp.Common.Helpers; using SixLabors.ImageSharp.Formats.Webp.BitReader; using SixLabors.ImageSharp.Formats.Webp.Lossless; using SixLabors.ImageSharp.Memory; @@ -311,8 +313,7 @@ internal class AlphaDecoder : IDisposable private static void HorizontalUnfilter(Span prev, Span input, Span dst, int width) { - // TODO: Investigate AdvSimd support for this method. - if (Sse2.IsSupported && width >= 9) + if ((Sse2.IsSupported || AdvSimd.IsSupported) && width >= 9) { dst[0] = (byte)(input[0] + (prev.IsEmpty ? 0 : prev[0])); nuint i; @@ -323,17 +324,17 @@ internal class AlphaDecoder : IDisposable for (i = 1; i <= (uint)width - 8; i += 8) { Vector128 a0 = Vector128.Create(Unsafe.As(ref Unsafe.Add(ref srcRef, i)), 0); - Vector128 a1 = Sse2.Add(a0.AsByte(), last.AsByte()); - Vector128 a2 = Sse2.ShiftLeftLogical128BitLane(a1, 1); - Vector128 a3 = Sse2.Add(a1, a2); - Vector128 a4 = Sse2.ShiftLeftLogical128BitLane(a3, 2); - Vector128 a5 = Sse2.Add(a3, a4); - Vector128 a6 = Sse2.ShiftLeftLogical128BitLane(a5, 4); - Vector128 a7 = Sse2.Add(a5, a6); + Vector128 a1 = a0.AsByte() + last.AsByte(); + Vector128 a2 = Vector128Utilities.ShiftLeftBytesInVector(a1, 1); + Vector128 a3 = a1 + a2; + Vector128 a4 = Vector128Utilities.ShiftLeftBytesInVector(a3, 2); + Vector128 a5 = a3 + a4; + Vector128 a6 = Vector128Utilities.ShiftLeftBytesInVector(a5, 4); + Vector128 a7 = a5 + a6; ref byte outputRef = ref Unsafe.Add(ref dstRef, i); Unsafe.As>(ref outputRef) = a7.GetLower(); - last = Sse2.ShiftRightLogical(a7.AsInt64(), 56).AsInt32(); + last = Vector128.ShiftRightLogical(a7.AsInt64(), 56).AsInt32(); } for (; i < (uint)width; ++i) From 8bf86986a45d2e36fbd74a68bbd7dea65195ad90 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 7 Mar 2024 19:41:24 +1000 Subject: [PATCH 36/37] Update PngDecoderCore.cs --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 194ee1661..6a321a3ba 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -125,7 +125,8 @@ internal sealed class PngDecoderCore : IImageDecoderInternals /// A reusable Crc32 hashing instance. /// private readonly Crc32 crc32 = new(); - + + /// /// The maximum memory in bytes that a zTXt, sPLT, iTXt, iCCP, or unknown chunk can occupy when decompressed. /// private readonly int maxUncompressedLength; From d8484da7399659a78c044545589a4c5ec67bbda0 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 13 Mar 2024 15:10:20 +1000 Subject: [PATCH 37/37] Remove allocations and cleanup. --- src/ImageSharp/Formats/Jpeg/JpegComData.cs | 24 ++-- .../Formats/Jpeg/JpegDecoderCore.cs | 9 +- .../Formats/Jpeg/JpegEncoderCore.cs | 77 +++++-------- .../Formats/Jpg/JpegEncoderTests.Metadata.cs | 104 ++++++++---------- 4 files changed, 92 insertions(+), 122 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegComData.cs b/src/ImageSharp/Formats/Jpeg/JpegComData.cs index 26ade0217..4e832d903 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegComData.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegComData.cs @@ -1,32 +1,32 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Six Labors Split License. namespace SixLabors.ImageSharp.Formats.Jpeg; /// -/// Contains JPEG comment +/// Represents a JPEG comment /// public readonly struct JpegComData { - /// - /// Converts string to - /// - /// The comment string. - /// The - public static JpegComData FromString(string value) => new(value.AsMemory()); - /// /// Initializes a new instance of the struct. /// - /// The comment ReadOnlyMemory of chars. + /// The comment buffer. public JpegComData(ReadOnlyMemory value) => this.Value = value; + /// + /// Gets the value. + /// public ReadOnlyMemory Value { get; } /// - /// Converts Value to string + /// Converts string to /// - /// The comment string. + /// The comment string. + /// The + public static JpegComData FromString(string value) => new(value.AsMemory()); + + /// public override string ToString() => this.Value.ToString(); } diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index cf5e449e7..906505b76 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -6,7 +6,6 @@ using System.Buffers; using System.Buffers.Binary; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; -using System.Text; using SixLabors.ImageSharp.Common.Helpers; using SixLabors.ImageSharp.Formats.Jpeg.Components; using SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder; @@ -481,6 +480,8 @@ internal sealed class JpegDecoderCore : IRawJpegData, IImageDecoderInternals break; case JpegConstants.Markers.APP15: + stream.Skip(markerContentByteSize); + break; case JpegConstants.Markers.COM: this.ProcessComMarker(stream, markerContentByteSize); break; @@ -523,16 +524,16 @@ internal sealed class JpegDecoderCore : IRawJpegData, IImageDecoderInternals /// The remaining bytes in the segment block. private void ProcessComMarker(BufferedReadStream stream, int markerContentByteSize) { - char[] temp = new char[markerContentByteSize]; + char[] chars = new char[markerContentByteSize]; JpegMetadata metadata = this.Metadata.GetFormatMetadata(JpegFormat.Instance); for (int i = 0; i < markerContentByteSize; i++) { int read = stream.ReadByte(); - temp[i] = (char)read; + chars[i] = (char)read; } - metadata.Comments.Add(new JpegComData(temp)); + metadata.Comments.Add(new JpegComData(chars)); } /// diff --git a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs index 4ef4cea2d..243bbe051 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs @@ -2,9 +2,8 @@ // Licensed under the Six Labors Split License. #nullable disable +using System.Buffers; using System.Buffers.Binary; -using System.Collections; -using System.Text; using SixLabors.ImageSharp.Common.Helpers; using SixLabors.ImageSharp.Formats.Jpeg.Components; using SixLabors.ImageSharp.Formats.Jpeg.Components.Encoder; @@ -27,6 +26,9 @@ internal sealed unsafe partial class JpegEncoderCore : IImageEncoderInternals /// private static readonly JpegFrameConfig[] FrameConfigs = CreateFrameConfigs(); + /// + /// The current calling encoder. + /// private readonly JpegEncoder encoder; /// @@ -92,7 +94,7 @@ internal sealed unsafe partial class JpegEncoderCore : IImageEncoderInternals this.WriteProfiles(metadata, buffer); // Write comments - this.WriteComment(jpegMetadata); + this.WriteComments(image.Configuration, jpegMetadata); // Write the image dimensions. this.WriteStartOfFrame(image.Width, image.Height, frameConfig, buffer); @@ -173,67 +175,48 @@ internal sealed unsafe partial class JpegEncoderCore : IImageEncoderInternals } /// - /// Writes comment + /// Writes the COM tags. /// + /// The configuration. /// The image metadata. - private void WriteComment(JpegMetadata metadata) + private void WriteComments(Configuration configuration, JpegMetadata metadata) { - int maxCommentLength = 65533; - if (metadata.Comments.Count == 0) { return; } - // We don't want to modify original metadata - List comments = new(metadata.Comments); - - int totalPayloadLength = 0; - for (int i = 0; i < comments.Count; i++) + const int maxCommentLength = 65533; + using IMemoryOwner bufferOwner = configuration.MemoryAllocator.Allocate(maxCommentLength); + Span buffer = bufferOwner.Memory.Span; + foreach (JpegComData comment in metadata.Comments) { - JpegComData comment = comments[i]; - ReadOnlyMemory currentComment = comment.Value; - - if (comment.Value.Length > maxCommentLength) + int totalLength = comment.Value.Length; + if (totalLength == 0) { - ReadOnlyMemory splitComment = - currentComment.Slice(maxCommentLength, currentComment.Length - maxCommentLength); - comments.Insert(i + 1, new JpegComData(splitComment)); - - // We don't want to keep the extra bytes - comments[i] = new JpegComData(currentComment.Slice(0, maxCommentLength)); + continue; } - totalPayloadLength += comment.Value.Length + 4; - } - - Span payload = new byte[totalPayloadLength]; - int currentCommentStartingIndex = 0; - - for (int i = 0; i < comments.Count; i++) - { - ReadOnlyMemory comment = comments[i].Value; + // Loop through and split the comment into multiple comments if the comment length + // is greater than the maximum allowed length. + while (totalLength > 0) + { + int currentLength = Math.Min(totalLength, maxCommentLength); - // Beginning of comment ff fe - payload[currentCommentStartingIndex] = JpegConstants.Markers.XFF; - payload[currentCommentStartingIndex + 1] = JpegConstants.Markers.COM; + // Write the marker header. + this.WriteMarkerHeader(JpegConstants.Markers.COM, currentLength + 2, buffer); - // Write payload size - int comWithoutMarker = comment.Length + 2; - payload[currentCommentStartingIndex + 2] = (byte)((comWithoutMarker >> 8) & 0xFF); - payload[currentCommentStartingIndex + 3] = (byte)(comWithoutMarker & 0xFF); + ReadOnlySpan commentValue = comment.Value.Span.Slice(comment.Value.Length - totalLength, currentLength); + for (int i = 0; i < commentValue.Length; i++) + { + buffer[i] = (byte)commentValue[i]; + } - char[] commentChars = comment.ToArray(); - for (int j = 0; j < commentChars.Length; j++) - { - // Initial 4 bytes are always reserved - payload[4 + currentCommentStartingIndex + j] = (byte)commentChars[j]; + // Write the comment. + this.outputStream.Write(buffer, 0, currentLength); + totalLength -= currentLength; } - - currentCommentStartingIndex += comment.Length + 4; } - - this.outputStream.Write(payload, 0, payload.Length); } /// diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs index f0593b462..f06fbe963 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs @@ -32,19 +32,19 @@ public partial class JpegEncoderTests public void Encode_PreservesIptcProfile() { // arrange - using var input = new Image(1, 1); - var expectedProfile = new IptcProfile(); + using Image input = new(1, 1); + IptcProfile expectedProfile = new(); expectedProfile.SetValue(IptcTag.Country, "ESPAÑA"); expectedProfile.SetValue(IptcTag.City, "unit-test-city"); input.Metadata.IptcProfile = expectedProfile; // act - using var memStream = new MemoryStream(); + using MemoryStream memStream = new(); input.Save(memStream, JpegEncoder); // assert memStream.Position = 0; - using var output = Image.Load(memStream); + using Image output = Image.Load(memStream); IptcProfile actual = output.Metadata.IptcProfile; Assert.NotNull(actual); IEnumerable values = expectedProfile.Values; @@ -55,17 +55,17 @@ public partial class JpegEncoderTests public void Encode_PreservesExifProfile() { // arrange - using var input = new Image(1, 1); + using Image input = new(1, 1); input.Metadata.ExifProfile = new ExifProfile(); input.Metadata.ExifProfile.SetValue(ExifTag.Software, "unit_test"); // act - using var memStream = new MemoryStream(); + using MemoryStream memStream = new(); input.Save(memStream, JpegEncoder); // assert memStream.Position = 0; - using var output = Image.Load(memStream); + using Image output = Image.Load(memStream); ExifProfile actual = output.Metadata.ExifProfile; Assert.NotNull(actual); IReadOnlyList values = input.Metadata.ExifProfile.Values; @@ -76,16 +76,16 @@ public partial class JpegEncoderTests public void Encode_PreservesIccProfile() { // arrange - using var input = new Image(1, 1); + using Image input = new(1, 1); input.Metadata.IccProfile = new IccProfile(IccTestDataProfiles.Profile_Random_Array); // act - using var memStream = new MemoryStream(); + using MemoryStream memStream = new(); input.Save(memStream, JpegEncoder); // assert memStream.Position = 0; - using var output = Image.Load(memStream); + using Image output = Image.Load(memStream); IccProfile actual = output.Metadata.IccProfile; Assert.NotNull(actual); IccProfile values = input.Metadata.IccProfile; @@ -99,12 +99,10 @@ public partial class JpegEncoderTests { Exception ex = Record.Exception(() => { - var encoder = new JpegEncoder(); - using (var stream = new MemoryStream()) - { - using Image image = provider.GetImage(JpegDecoder.Instance); - image.Save(stream, encoder); - } + JpegEncoder encoder = new(); + using MemoryStream stream = new(); + using Image image = provider.GetImage(JpegDecoder.Instance); + image.Save(stream, encoder); }); Assert.Null(ex); @@ -114,23 +112,17 @@ public partial class JpegEncoderTests [MemberData(nameof(RatioFiles))] public void Encode_PreserveRatio(string imagePath, int xResolution, int yResolution, PixelResolutionUnit resolutionUnit) { - var testFile = TestFile.Create(imagePath); - using (Image input = testFile.CreateRgba32Image()) - { - using (var memStream = new MemoryStream()) - { - input.Save(memStream, JpegEncoder); - - memStream.Position = 0; - using (var output = Image.Load(memStream)) - { - ImageMetadata meta = output.Metadata; - Assert.Equal(xResolution, meta.HorizontalResolution); - Assert.Equal(yResolution, meta.VerticalResolution); - Assert.Equal(resolutionUnit, meta.ResolutionUnits); - } - } - } + TestFile testFile = TestFile.Create(imagePath); + using Image input = testFile.CreateRgba32Image(); + using MemoryStream memStream = new(); + input.Save(memStream, JpegEncoder); + + memStream.Position = 0; + using Image output = Image.Load(memStream); + ImageMetadata meta = output.Metadata; + Assert.Equal(xResolution, meta.HorizontalResolution); + Assert.Equal(yResolution, meta.VerticalResolution); + Assert.Equal(resolutionUnit, meta.ResolutionUnits); } [Theory] @@ -138,20 +130,14 @@ public partial class JpegEncoderTests public void Encode_PreservesQuality(string imagePath, int quality) { TestFile testFile = TestFile.Create(imagePath); - using (Image input = testFile.CreateRgba32Image()) - { - using (var memStream = new MemoryStream()) - { - input.Save(memStream, JpegEncoder); - - memStream.Position = 0; - using (var output = Image.Load(memStream)) - { - JpegMetadata meta = output.Metadata.GetJpegMetadata(); - Assert.Equal(quality, meta.Quality); - } - } - } + using Image input = testFile.CreateRgba32Image(); + using MemoryStream memStream = new(); + input.Save(memStream, JpegEncoder); + + memStream.Position = 0; + using Image output = Image.Load(memStream); + JpegMetadata meta = output.Metadata.GetJpegMetadata(); + Assert.Equal(quality, meta.Quality); } [Theory] @@ -161,7 +147,7 @@ public partial class JpegEncoderTests { // arrange using Image input = provider.GetImage(JpegDecoder.Instance); - using var memStream = new MemoryStream(); + using MemoryStream memStream = new(); // act input.Save(memStream, JpegEncoder); @@ -172,16 +158,16 @@ public partial class JpegEncoderTests JpegMetadata actual = output.Metadata.GetJpegMetadata(); Assert.NotEmpty(actual.Comments); Assert.Equal(1, actual.Comments.Count); - Assert.Equal("TEST COMMENT", actual.Comments.ElementAtOrDefault(0).ToString()); + Assert.Equal("TEST COMMENT", actual.Comments[0].ToString()); } [Fact] public void Encode_SavesMultipleComments() { // arrange - using var input = new Image(1, 1); + using Image input = new(1, 1); JpegMetadata meta = input.Metadata.GetJpegMetadata(); - using var memStream = new MemoryStream(); + using MemoryStream memStream = new(); // act meta.Comments.Add(JpegComData.FromString("First comment")); @@ -194,8 +180,8 @@ public partial class JpegEncoderTests JpegMetadata actual = output.Metadata.GetJpegMetadata(); Assert.NotEmpty(actual.Comments); Assert.Equal(2, actual.Comments.Count); - Assert.Equal(meta.Comments.ElementAtOrDefault(0).ToString(), actual.Comments.ElementAtOrDefault(0).ToString()); - Assert.Equal(meta.Comments.ElementAtOrDefault(1).ToString(), actual.Comments.ElementAtOrDefault(1).ToString()); + Assert.Equal(meta.Comments[0].ToString(), actual.Comments[0].ToString()); + Assert.Equal(meta.Comments[1].ToString(), actual.Comments[1].ToString()); } [Fact] @@ -203,9 +189,9 @@ public partial class JpegEncoderTests { // arrange string longString = new('c', 65534); - using var input = new Image(1, 1); + using Image input = new(1, 1); JpegMetadata meta = input.Metadata.GetJpegMetadata(); - using var memStream = new MemoryStream(); + using MemoryStream memStream = new(); // act meta.Comments.Add(JpegComData.FromString(longString)); @@ -217,8 +203,8 @@ public partial class JpegEncoderTests JpegMetadata actual = output.Metadata.GetJpegMetadata(); Assert.NotEmpty(actual.Comments); Assert.Equal(2, actual.Comments.Count); - Assert.Equal(longString[..65533], actual.Comments.ElementAtOrDefault(0).ToString()); - Assert.Equal("c", actual.Comments.ElementAtOrDefault(1).ToString()); + Assert.Equal(longString[..65533], actual.Comments[0].ToString()); + Assert.Equal("c", actual.Comments[1].ToString()); } [Theory] @@ -231,14 +217,14 @@ public partial class JpegEncoderTests { // arrange using Image input = provider.GetImage(JpegDecoder.Instance); - using var memoryStream = new MemoryStream(); + using MemoryStream memoryStream = new(); // act input.Save(memoryStream, JpegEncoder); // assert memoryStream.Position = 0; - using var output = Image.Load(memoryStream); + using Image output = Image.Load(memoryStream); JpegMetadata meta = output.Metadata.GetJpegMetadata(); Assert.Equal(expectedColorType, meta.ColorType); }