Add fixes 2668, 2676, and 2677 to main (#2678)

* Prevent underflow

* Fix file name

* Revert "Fix file name"

This reverts commit 15619ec.

* 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 <[email protected]>

* Update Issue2668_Quantized_Encode_Alpha_Rgba32_Issue_2668.png

* Normalize Color API

* Aggressive inlining

---------

Co-authored-by: Günther Foidl <[email protected]>

Author: JimBobSquarePants

src/ImageSharp/Color/Color.cs

@@ -25,7 +25,7 @@ namespace SixLabors.ImageSharp;
     /// Initializes a new instance of the <see cref="Color"/> struct.
     /// </summary>
     /// <param name="vector">The <see cref="Vector4"/> containing the color information.</param>
-    [MethodImpl(InliningOptions.ShortMethod)]
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
     private Color(Vector4 vector)
     {
         this.data = Numerics.Clamp(vector, Vector4.Zero, Vector4.One);
@@ -36,28 +36,13 @@ private Color(Vector4 vector)
     /// Initializes a new instance of the <see cref="Color"/> struct.
     /// </summary>
     /// <param name="pixel">The pixel containing color information.</param>
-    [MethodImpl(InliningOptions.ShortMethod)]
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
     private Color(IPixel pixel)
     {
         this.boxedHighPrecisionPixel = pixel;
         this.data = default;
     }
 
-    /// <summary>
-    /// Converts a <see cref="Color"/> to <see cref="Vector4"/>.
-    /// </summary>
-    /// <param name="color">The <see cref="Color"/>.</param>
-    /// <returns>The <see cref="Vector4"/>.</returns>
-    public static explicit operator Vector4(Color color) => color.ToScaledVector4();
-
-    /// <summary>
-    /// Converts an <see cref="Vector4"/> to <see cref="Color"/>.
-    /// </summary>
-    /// <param name="source">The <see cref="Vector4"/>.</param>
-    /// <returns>The <see cref="Color"/>.</returns>
-    [MethodImpl(InliningOptions.ShortMethod)]
-    public static explicit operator Color(Vector4 source) => new(source);
-
     /// <summary>
     /// Checks whether two <see cref="Color"/> structures are equal.
     /// </summary>
@@ -67,7 +52,7 @@ private Color(IPixel pixel)
     /// True if the <paramref name="left"/> parameter is equal to the <paramref name="right"/> parameter;
     /// otherwise, false.
     /// </returns>
-    [MethodImpl(InliningOptions.ShortMethod)]
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public static bool operator ==(Color left, Color right) => left.Equals(right);
 
     /// <summary>
@@ -79,36 +64,44 @@ private Color(IPixel pixel)
     /// True if the <paramref name="left"/> parameter is not equal to the <paramref name="right"/> parameter;
     /// otherwise, false.
     /// </returns>
-    [MethodImpl(InliningOptions.ShortMethod)]
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public static bool operator !=(Color left, Color right) => !left.Equals(right);
 
     /// <summary>
     /// Creates a <see cref="Color"/> from the given <typeparamref name="TPixel"/>.
     /// </summary>
-    /// <param name="pixel">The pixel to convert from.</param>
+    /// <param name="source">The pixel to convert from.</param>
     /// <typeparam name="TPixel">The pixel format.</typeparam>
     /// <returns>The <see cref="Color"/>.</returns>
-    [MethodImpl(InliningOptions.ShortMethod)]
-    public static Color FromPixel<TPixel>(TPixel pixel)
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public static Color FromPixel<TPixel>(TPixel source)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         // 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);
     }
 
+    /// <summary>
+    /// Creates a <see cref="Color"/> from a generic scaled <see cref="Vector4"/>.
+    /// </summary>
+    /// <param name="source">The vector to load the pixel from.</param>
+    /// <returns>The <see cref="Color"/>.</returns>
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public static Color FromScaledVector(Vector4 source) => new(source);
+
     /// <summary>
     /// Bulk converts a span of a specified <typeparamref name="TPixel"/> type to a span of <see cref="Color"/>.
     /// </summary>
     /// <typeparam name="TPixel">The pixel type to convert to.</typeparam>
     /// <param name="source">The source pixel span.</param>
     /// <param name="destination">The destination color span.</param>
-    [MethodImpl(InliningOptions.ShortMethod)]
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public static void FromPixel<TPixel>(ReadOnlySpan<TPixel> source, Span<Color> destination)
         where TPixel : unmanaged, IPixel<TPixel>
     {
@@ -120,7 +113,7 @@ public static void FromPixel<TPixel>(ReadOnlySpan<TPixel> source, Span<Color> de
         {
             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 static void FromPixel<TPixel>(ReadOnlySpan<TPixel> source, Span<Color> de
     /// <returns>
     /// The <see cref="Color"/>.
     /// </returns>
-    [MethodImpl(InliningOptions.ShortMethod)]
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public static Color ParseHex(string hex)
     {
         Rgba32 rgba = Rgba32.ParseHex(hex);
@@ -162,7 +155,7 @@ public static Color ParseHex(string hex)
     /// <returns>
     /// The <see cref="bool"/>.
     /// </returns>
-    [MethodImpl(InliningOptions.ShortMethod)]
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public static bool TryParseHex(string hex, out Color result)
     {
         result = default;
@@ -236,16 +229,16 @@ public static bool TryParse(string input, out Color result)
     /// <returns>The color having it's alpha channel altered.</returns>
     public Color WithAlpha(float alpha)
     {
-        Vector4 v = (Vector4)this;
+        Vector4 v = this.ToScaledVector4();
         v.W = alpha;
-        return new Color(v);
+        return FromScaledVector(v);
     }
 
     /// <summary>
     /// Gets the hexadecimal representation of the color instance in rrggbbaa form.
     /// </summary>
     /// <returns>A hexadecimal string representation of the value.</returns>
-    [MethodImpl(InliningOptions.ShortMethod)]
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public string ToHex()
     {
         if (this.boxedHighPrecisionPixel is not null)
@@ -263,8 +256,8 @@ public string ToHex()
     /// Converts the color instance to a specified <typeparamref name="TPixel"/> type.
     /// </summary>
     /// <typeparam name="TPixel">The pixel type to convert to.</typeparam>
-    /// <returns>The pixel value.</returns>
-    [MethodImpl(InliningOptions.ShortMethod)]
+    /// <returns>The <typeparamref name="TPixel"/>.</returns>
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public TPixel ToPixel<TPixel>()
         where TPixel : unmanaged, IPixel<TPixel>
     {
@@ -281,13 +274,30 @@ public TPixel ToPixel<TPixel>()
         return TPixel.FromScaledVector4(this.boxedHighPrecisionPixel.ToScaledVector4());
     }
 
+    /// <summary>
+    /// Expands the color into a generic ("scaled") <see cref="Vector4"/> representation
+    /// with values scaled and clamped between <value>0</value> and <value>1</value>.
+    /// The vector components are typically expanded in least to greatest significance order.
+    /// </summary>
+    /// <returns>The <see cref="Vector4"/>.</returns>
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public Vector4 ToScaledVector4()
+    {
+        if (this.boxedHighPrecisionPixel is null)
+        {
+            return this.data;
+        }
+
+        return this.boxedHighPrecisionPixel.ToScaledVector4();
+    }
+
     /// <summary>
     /// Bulk converts a span of <see cref="Color"/> to a span of a specified <typeparamref name="TPixel"/> type.
     /// </summary>
     /// <typeparam name="TPixel">The pixel type to convert to.</typeparam>
     /// <param name="source">The source color span.</param>
     /// <param name="destination">The destination pixel span.</param>
-    [MethodImpl(InliningOptions.ShortMethod)]
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public static void ToPixel<TPixel>(ReadOnlySpan<Color> source, Span<TPixel> destination)
         where TPixel : unmanaged, IPixel<TPixel>
     {
@@ -301,7 +311,7 @@ public static void ToPixel<TPixel>(ReadOnlySpan<Color> source, Span<TPixel> dest
     }
 
     /// <inheritdoc />
-    [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 bool Equals(Color other)
     public override bool Equals(object? obj) => obj is Color other && this.Equals(other);
 
     /// <inheritdoc />
-    [MethodImpl(InliningOptions.ShortMethod)]
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public override int GetHashCode()
     {
         if (this.boxedHighPrecisionPixel is null)
@@ -326,15 +336,4 @@ public override int GetHashCode()
 
         return this.boxedHighPrecisionPixel.GetHashCode();
     }
-
-    [MethodImpl(InliningOptions.ShortMethod)]
-    private Vector4 ToScaledVector4()
-    {
-        if (this.boxedHighPrecisionPixel is null)
-        {
-            return this.data;
-        }
-
-        return this.boxedHighPrecisionPixel.ToScaledVector4();
-    }
 }

src/ImageSharp/Formats/Gif/GifFrameMetadata.cs

@@ -82,13 +82,13 @@ internal static GifFrameMetadata FromAnimatedMetadata(AnimatedImageFrameMetadata
     {
         // 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<Color> 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;

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 @@ private void SanitizeAndSetEncoderOptions<TPixel>(
             {
                 // 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<Color> 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

src/ImageSharp/Formats/Webp/AlphaDecoder.cs

@@ -311,18 +311,15 @@ private static void ColorIndexInverseTransformAlpha(
 
     private static void HorizontalUnfilter(Span<byte> prev, Span<byte> input, Span<byte> 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<int> last = Vector128<int>.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<long> a0 = Vector128.Create(Unsafe.As<byte, long>(ref Unsafe.Add(ref srcRef, i)), 0);

tests/ImageSharp.Tests/Color/ColorTests.CastTo.cs

@@ -105,7 +105,7 @@ public void Bgr24()
         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<RgbaVector>());

tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs

@@ -8,6 +8,7 @@
 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 void Issue2469_Quantized_Encode_Artifacts<TPixel>(TestImageProvider<TPixe
         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<TPixel>(TestImageProvider<TPixel> provider)
+        where TPixel : unmanaged, IPixel<TPixel>
+    {
+        using Image<TPixel> 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<Rgba32> encoded = Image.Load<Rgba32>(actualOutputFile);
+        encoded.CompareToReferenceOutput(ImageComparer.Exact, provider);
+    }
+
     private static void TestPngEncoderCore<TPixel>(
         TestImageProvider<TPixel> provider,
         PngColorType pngColorType,

tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs

@@ -439,6 +439,17 @@ public void WebpDecoder_CanDecode_Issue2257<TPixel>(TestImageProvider<TPixel> pr
         image.CompareToOriginal(provider, ReferenceDecoder);
     }
 
+    // https://github.com/SixLabors/ImageSharp/issues/2670
+    [Theory]
+    [WithFile(Lossy.Issue2670, PixelTypes.Rgba32)]
+    public void WebpDecoder_CanDecode_Issue2670<TPixel>(TestImageProvider<TPixel> provider)
+        where TPixel : unmanaged, IPixel<TPixel>
+    {
+        using Image<TPixel> image = provider.GetImage(WebpDecoder.Instance);
+        image.DebugSave(provider);
+        image.CompareToOriginal(provider, ReferenceDecoder);
+    }
+
     [Theory]
     [WithFile(Lossless.LossLessCorruptImage3, PixelTypes.Rgba32)]
     public void WebpDecoder_ThrowImageFormatException_OnInvalidImages<TPixel>(TestImageProvider<TPixel> provider)

tests/ImageSharp.Tests/TestImages.cs

@@ -151,6 +151,9 @@ public static class Png
         // 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 Lossy
             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";
         }
     }