Fix PngMetadata

src/ImageSharp/Formats/Png/PngEncoderCore.cs

@@ -1484,41 +1484,19 @@ internal sealed class PngEncoderCore : IImageEncoderInternals, IDisposable
 
         // Use options, then check metadata, if nothing set there then we suggest
         // a sensible default based upon the pixel format.
-        PngColorType? colorType = encoder.ColorType ?? pngMetadata.ColorType;
+        PngColorType color = encoder.ColorType ?? pngMetadata.ColorType;
-        byte? bits = (byte?)(encoder.BitDepth ?? pngMetadata.BitDepth);
+        byte bits = (byte)(encoder.BitDepth ?? pngMetadata.BitDepth);
 
-        if (colorType is null || bits is null)
+        // Ensure the bit depth and color type are a supported combination.
-        {
-            PixelTypeInfo info = TPixel.GetPixelTypeInfo();
-            PixelComponentInfo? componentInfo = info.ComponentInfo;
-
-            colorType ??= SuggestColorType<TPixel>(in info);
-
-            if (bits is null)
-            {
-                // TODO: Update once we stop abusing PixelTypeInfo in decoders.
-                if (componentInfo.HasValue)
-                {
-                    PixelComponentInfo c = componentInfo.Value;
-                    bits = (byte)SuggestBitDepth<TPixel>(in c);
-                }
-                else
-                {
-                    bits = (byte)PngBitDepth.Bit8;
-                }
-            }
-        }
-
-        // Ensure bit depth and color type are a supported combination.
         // Bit8 is the only bit depth supported by all color types.
-        byte[] validBitDepths = PngConstants.ColorTypes[colorType.Value];
+        byte[] validBitDepths = PngConstants.ColorTypes[color];
         if (Array.IndexOf(validBitDepths, bits) == -1)
         {
             bits = (byte)PngBitDepth.Bit8;
         }
 
-        this.colorType = colorType.Value;
+        this.colorType = color;
-        this.bitDepth = bits.Value;
+        this.bitDepth = bits;
 
         if (!encoder.FilterMethod.HasValue)
         {
@@ -1529,7 +1507,7 @@ internal sealed class PngEncoderCore : IImageEncoderInternals, IDisposable
         use16Bit = bits == (byte)PngBitDepth.Bit16;
         bytesPerPixel = CalculateBytesPerPixel(this.colorType, use16Bit);
 
-        this.interlaceMode = (encoder.InterlaceMethod ?? pngMetadata.InterlaceMethod)!.Value;
+        this.interlaceMode = encoder.InterlaceMethod ?? pngMetadata.InterlaceMethod;
         this.chunkFilter = encoder.SkipMetadata ? PngChunkFilter.ExcludeAll : encoder.ChunkFilter ?? PngChunkFilter.None;
     }
 
@@ -1669,47 +1647,6 @@ internal sealed class PngEncoderCore : IImageEncoderInternals, IDisposable
             _ => use16Bit ? 8 : 4,
         };
 
-    /// <summary>
-    /// Returns a suggested <see cref="PngColorType"/> for the given <typeparamref name="TPixel"/>
-    /// </summary>
-    /// <param name="info">The pixel type info.</param>
-    /// <typeparam name="TPixel">The type of pixel format.</typeparam>
-    private static PngColorType SuggestColorType<TPixel>(in PixelTypeInfo info)
-        where TPixel : unmanaged, IPixel<TPixel>
-    {
-        if (info.AlphaRepresentation == PixelAlphaRepresentation.None)
-        {
-            return info.ColorType switch
-            {
-                PixelColorType.Luminance => PngColorType.Grayscale,
-                _ => PngColorType.Rgb,
-            };
-        }
-
-        return info.ColorType switch
-        {
-            PixelColorType.Luminance | PixelColorType.Alpha or PixelColorType.Alpha => PngColorType.GrayscaleWithAlpha,
-            _ => PngColorType.RgbWithAlpha,
-        };
-    }
-
-    /// <summary>
-    /// Returns a suggested <see cref="PngBitDepth"/> for the given <typeparamref name="TPixel"/>
-    /// </summary>
-    /// <param name="info">The pixel type info.</param>
-    /// <typeparam name="TPixel">The type of pixel format.</typeparam>
-    private static PngBitDepth SuggestBitDepth<TPixel>(in PixelComponentInfo info)
-        where TPixel : unmanaged, IPixel<TPixel>
-    {
-        int bits = info.GetMaximumComponentPrecision();
-        if (bits > (int)PixelComponentBitDepth.Bit8)
-        {
-            return PngBitDepth.Bit16;
-        }
-
-        return PngBitDepth.Bit8;
-    }
-
     private unsafe struct ScratchBuffer
     {
         private const int Size = 26;

src/ImageSharp/Formats/Png/PngMetadata.cs

@@ -47,17 +47,17 @@ public class PngMetadata : IFormatMetadata<PngMetadata>
     /// Gets or sets the number of bits per sample or per palette index (not per pixel).
     /// Not all values are allowed for all <see cref="ColorType"/> values.
     /// </summary>
-    public PngBitDepth? BitDepth { get; set; }
+    public PngBitDepth BitDepth { get; set; } = PngBitDepth.Bit8;
 
     /// <summary>
     /// Gets or sets the color type.
     /// </summary>
-    public PngColorType? ColorType { get; set; }
+    public PngColorType ColorType { get; set; } = PngColorType.RgbWithAlpha;
 
     /// <summary>
     /// Gets or sets a value indicating whether this instance should write an Adam7 interlaced image.
     /// </summary>
-    public PngInterlaceMode? InterlaceMethod { get; set; } = PngInterlaceMode.None;
+    public PngInterlaceMode InterlaceMethod { get; set; } = PngInterlaceMode.None;
 
     /// <summary>
     /// Gets or sets the gamma value for the image.
@@ -100,21 +100,23 @@ public class PngMetadata : IFormatMetadata<PngMetadata>
             for (int i = 0; i < colorTable.Length; i++)
             {
                 ref Color c = ref colorTable[i];
-                if (c == metadata.BackgroundColor)
+                if (c != metadata.BackgroundColor)
                 {
-                    // Png treats background as fully empty
+                    continue;
-                    c = Color.Transparent;
-                    break;
                 }
+
+                // Png treats background as fully empty
+                c = Color.Transparent;
+                break;
             }
         }
 
         return new()
         {
-            ColorType = colorTable != null ? PngColorType.Palette : null,
+            ColorType = colorTable != null ? PngColorType.Palette : PngColorType.RgbWithAlpha,
             BitDepth = colorTable != null
                         ? (PngBitDepth)Numerics.Clamp(ColorNumerics.GetBitsNeededForColorDepth(colorTable.Length), 1, 8)
-                        : null,
+                        : PngBitDepth.Bit8,
             ColorTable = colorTable,
             RepeatCount = metadata.RepeatCount,
         };
@@ -131,12 +133,14 @@ public class PngMetadata : IFormatMetadata<PngMetadata>
             for (int i = 0; i < colorTable.Length; i++)
             {
                 ref Color c = ref colorTable[i];
-                if (c == metadata.BackgroundColor)
+                if (c != metadata.BackgroundColor)
                 {
-                    // Png treats background as fully empty
+                    continue;
-                    c = Color.Transparent;
-                    break;
                 }
+
+                // Png treats background as fully empty
+                c = Color.Transparent;
+                break;
             }
         }
 
@@ -240,6 +244,7 @@ public class PngMetadata : IFormatMetadata<PngMetadata>
                 info = PixelComponentInfo.Create(3, bpp, 16, 16, 16);
                 break;
 
+            case PngColorType.RgbWithAlpha:
             default:
 
                 alpha = PixelAlphaRepresentation.Unassociated;

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

@@ -254,49 +254,6 @@ public partial class PngEncoderTests
         }
     }
 
-    [Theory]
-    [WithBlankImages(1, 1, PixelTypes.A8, PngColorType.GrayscaleWithAlpha, PngBitDepth.Bit8)]
-    [WithBlankImages(1, 1, PixelTypes.Argb32, PngColorType.RgbWithAlpha, PngBitDepth.Bit8)]
-    [WithBlankImages(1, 1, PixelTypes.Bgr565, PngColorType.Rgb, PngBitDepth.Bit8)]
-    [WithBlankImages(1, 1, PixelTypes.Bgra4444, PngColorType.RgbWithAlpha, PngBitDepth.Bit8)]
-    [WithBlankImages(1, 1, PixelTypes.Byte4, PngColorType.RgbWithAlpha, PngBitDepth.Bit8)]
-    [WithBlankImages(1, 1, PixelTypes.HalfSingle, PngColorType.Rgb, PngBitDepth.Bit16)]
-    [WithBlankImages(1, 1, PixelTypes.HalfVector2, PngColorType.Rgb, PngBitDepth.Bit16)]
-    [WithBlankImages(1, 1, PixelTypes.HalfVector4, PngColorType.RgbWithAlpha, PngBitDepth.Bit16)]
-    [WithBlankImages(1, 1, PixelTypes.NormalizedByte2, PngColorType.Rgb, PngBitDepth.Bit8)]
-    [WithBlankImages(1, 1, PixelTypes.NormalizedByte4, PngColorType.RgbWithAlpha, PngBitDepth.Bit8)]
-    [WithBlankImages(1, 1, PixelTypes.NormalizedShort4, PngColorType.RgbWithAlpha, PngBitDepth.Bit16)]
-    [WithBlankImages(1, 1, PixelTypes.Rg32, PngColorType.Rgb, PngBitDepth.Bit16)]
-    [WithBlankImages(1, 1, PixelTypes.Rgba1010102, PngColorType.RgbWithAlpha, PngBitDepth.Bit16)]
-    [WithBlankImages(1, 1, PixelTypes.Rgba32, PngColorType.RgbWithAlpha, PngBitDepth.Bit8)]
-    [WithBlankImages(1, 1, PixelTypes.RgbaVector, PngColorType.RgbWithAlpha, PngBitDepth.Bit16)]
-    [WithBlankImages(1, 1, PixelTypes.Short2, PngColorType.Rgb, PngBitDepth.Bit16)]
-    [WithBlankImages(1, 1, PixelTypes.Short4, PngColorType.RgbWithAlpha, PngBitDepth.Bit16)]
-    [WithBlankImages(1, 1, PixelTypes.Rgb24, PngColorType.Rgb, PngBitDepth.Bit8)]
-    [WithBlankImages(1, 1, PixelTypes.Bgr24, PngColorType.Rgb, PngBitDepth.Bit8)]
-    [WithBlankImages(1, 1, PixelTypes.Bgra32, PngColorType.RgbWithAlpha, PngBitDepth.Bit8)]
-    [WithBlankImages(1, 1, PixelTypes.Rgb48, PngColorType.Rgb, PngBitDepth.Bit16)]
-    [WithBlankImages(1, 1, PixelTypes.Rgba64, PngColorType.RgbWithAlpha, PngBitDepth.Bit16)]
-    [WithBlankImages(1, 1, PixelTypes.Bgra5551, PngColorType.RgbWithAlpha, PngBitDepth.Bit8)]
-    [WithBlankImages(1, 1, PixelTypes.L8, PngColorType.Grayscale, PngBitDepth.Bit8)]
-    [WithBlankImages(1, 1, PixelTypes.L16, PngColorType.Grayscale, PngBitDepth.Bit16)]
-    [WithBlankImages(1, 1, PixelTypes.La16, PngColorType.GrayscaleWithAlpha, PngBitDepth.Bit8)]
-    [WithBlankImages(1, 1, PixelTypes.La32, PngColorType.GrayscaleWithAlpha, PngBitDepth.Bit16)]
-    public void InfersColorTypeAndBitDepth<TPixel>(TestImageProvider<TPixel> provider, PngColorType pngColorType, PngBitDepth pngBitDepth)
-        where TPixel : unmanaged, IPixel<TPixel>
-    {
-        using Stream stream = new MemoryStream();
-        PngEncoder.Encode(provider.GetImage(), stream);
-
-        stream.Seek(0, SeekOrigin.Begin);
-
-        using Image image = PngDecoder.Instance.Decode(DecoderOptions.Default, stream);
-
-        PngMetadata metadata = image.Metadata.GetPngMetadata();
-        Assert.Equal(pngColorType, metadata.ColorType);
-        Assert.Equal(pngBitDepth, metadata.BitDepth);
-    }
-
     [Theory]
     [WithFile(TestImages.Png.Palette8Bpp, nameof(PaletteLargeOnly), PixelTypes.Rgba32)]
     public void PaletteColorType_WuQuantizer<TPixel>(TestImageProvider<TPixel> provider, int paletteSize)

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

@@ -17,17 +17,13 @@ public class PngSmokeTests
     public void GeneralTest<TPixel>(TestImageProvider<TPixel> provider)
         where TPixel : unmanaged, IPixel<TPixel>
     {
-        // does saving a file then reopening mean both files are identical???
         using Image<TPixel> image = provider.GetImage();
-        using var ms = new MemoryStream();
+        using MemoryStream ms = new();
 
-        // image.Save(provider.Utility.GetTestOutputFileName("bmp"));
         image.Save(ms, new PngEncoder());
         ms.Position = 0;
         using Image<Rgba32> img2 = PngDecoder.Instance.Decode<Rgba32>(DecoderOptions.Default, ms);
-        ImageComparer.Tolerant().VerifySimilarity(image, img2);
+        ImageComparer.Exact.VerifySimilarity(image, img2);
-
-        // img2.Save(provider.Utility.GetTestOutputFileName("bmp", "_loaded"), new BmpEncoder());
     }
 
     [Theory]
@@ -37,12 +33,10 @@ public class PngSmokeTests
     {
         // does saving a file then reopening mean both files are identical???
         using Image<TPixel> image = provider.GetImage();
-        using var ms = new MemoryStream();
+        using MemoryStream ms = new();
 
-        // image.Save(provider.Utility.GetTestOutputFileName("png"));
         image.Mutate(x => x.Resize(100, 100));
 
-        // image.Save(provider.Utility.GetTestOutputFileName("png", "resize"));
         image.Save(ms, new PngEncoder());
         ms.Position = 0;
         using Image<Rgba32> img2 = PngDecoder.Instance.Decode<Rgba32>(DecoderOptions.Default, ms);