Fix 16-bit binary PBM sample byte order

src/ImageSharp/Formats/Pbm/BinaryDecoder.cs

@@ -2,6 +2,8 @@
 // Licensed under the Six Labors Split License.
 
 using System.Buffers;
+using System.Buffers.Binary;
+using System.Runtime.InteropServices;
 using SixLabors.ImageSharp.IO;
 using SixLabors.ImageSharp.Memory;
 using SixLabors.ImageSharp.PixelFormats;
@@ -13,21 +15,25 @@ namespace SixLabors.ImageSharp.Formats.Pbm;
 /// </summary>
 internal class BinaryDecoder
 {
+    /// <summary>
+    /// The luminance value written for an unset bit in the black and white format.
+    /// </summary>
     private static L8 white = new(255);
+
+    /// <summary>
+    /// The luminance value written for a set bit in the black and white format.
+    /// </summary>
     private static L8 black = new(0);
 
     /// <summary>
     /// Decode the specified pixels.
     /// </summary>
-    /// <typeparam name="TPixel">The type of pixel to encode to.</typeparam>
+    /// <typeparam name="TPixel">The type of pixel to decode to.</typeparam>
     /// <param name="configuration">The configuration.</param>
-    /// <param name="pixels">The pixel array to encode into.</param>
+    /// <param name="pixels">The pixel buffer to decode into.</param>
     /// <param name="stream">The stream to read the data from.</param>
-    /// <param name="colorType">The ColorType to decode.</param>
-    /// <param name="componentType">Data type of the pixles components.</param>
-    /// <exception cref="InvalidImageContentException">
-    /// Thrown if an invalid combination of setting is requested.
-    /// </exception>
+    /// <param name="colorType">The color type of the encoded pixels.</param>
+    /// <param name="componentType">The data type of the pixel components.</param>
     public static void Process<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream, PbmColorType colorType, PbmComponentType componentType)
         where TPixel : unmanaged, IPixel<TPixel>
     {
@@ -59,6 +65,15 @@ public static void Process<TPixel>(Configuration configuration, Buffer2D<TPixel>
         }
     }
 
+    /// <summary>
+    /// Decodes 8-bit binary grayscale (PGM) pixel data.
+    /// Each pixel is a single byte that holds its luminance value.
+    /// When the stream ends early, the rows that were not read keep their default value.
+    /// </summary>
+    /// <typeparam name="TPixel">The type of pixel to decode to.</typeparam>
+    /// <param name="configuration">The configuration.</param>
+    /// <param name="pixels">The pixel buffer to decode into.</param>
+    /// <param name="stream">The stream to read the data from.</param>
     private static void ProcessGrayscale<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream)
         where TPixel : unmanaged, IPixel<TPixel>
     {
@@ -85,6 +100,15 @@ private static void ProcessGrayscale<TPixel>(Configuration configuration, Buffer
         }
     }
 
+    /// <summary>
+    /// Decodes 16-bit binary grayscale (PGM) pixel data.
+    /// Each pixel is one 16-bit sample, stored most significant byte first.
+    /// When the stream ends early, the rows that were not read keep their default value.
+    /// </summary>
+    /// <typeparam name="TPixel">The type of pixel to decode to.</typeparam>
+    /// <param name="configuration">The configuration.</param>
+    /// <param name="pixels">The pixel buffer to decode into.</param>
+    /// <param name="stream">The stream to read the data from.</param>
     private static void ProcessWideGrayscale<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream)
         where TPixel : unmanaged, IPixel<TPixel>
     {
@@ -102,6 +126,10 @@ private static void ProcessWideGrayscale<TPixel>(Configuration configuration, Bu
                 return;
             }
 
+            // The binary format stores 16-bit samples most significant byte first,
+            // but L16 expects native (little-endian) byte order.
+            SwapSampleBytes(rowSpan);
+
             Span<TPixel> pixelSpan = pixels.DangerousGetRowSpan(y);
             PixelOperations<TPixel>.Instance.FromL16Bytes(
                 configuration,
@@ -111,6 +139,15 @@ private static void ProcessWideGrayscale<TPixel>(Configuration configuration, Bu
         }
     }
 
+    /// <summary>
+    /// Decodes 8-bit binary color (PPM) pixel data.
+    /// Each pixel is three bytes in red, green, blue order.
+    /// When the stream ends early, the rows that were not read keep their default value.
+    /// </summary>
+    /// <typeparam name="TPixel">The type of pixel to decode to.</typeparam>
+    /// <param name="configuration">The configuration.</param>
+    /// <param name="pixels">The pixel buffer to decode into.</param>
+    /// <param name="stream">The stream to read the data from.</param>
     private static void ProcessRgb<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream)
         where TPixel : unmanaged, IPixel<TPixel>
     {
@@ -137,6 +174,15 @@ private static void ProcessRgb<TPixel>(Configuration configuration, Buffer2D<TPi
         }
     }
 
+    /// <summary>
+    /// Decodes 16-bit binary color (PPM) pixel data.
+    /// Each pixel is three 16-bit samples in red, green, blue order, stored most significant byte first.
+    /// When the stream ends early, the rows that were not read keep their default value.
+    /// </summary>
+    /// <typeparam name="TPixel">The type of pixel to decode to.</typeparam>
+    /// <param name="configuration">The configuration.</param>
+    /// <param name="pixels">The pixel buffer to decode into.</param>
+    /// <param name="stream">The stream to read the data from.</param>
     private static void ProcessWideRgb<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream)
         where TPixel : unmanaged, IPixel<TPixel>
     {
@@ -154,6 +200,10 @@ private static void ProcessWideRgb<TPixel>(Configuration configuration, Buffer2D
                 return;
             }
 
+            // The binary format stores 16-bit samples most significant byte first,
+            // but Rgb48 expects native (little-endian) byte order.
+            SwapSampleBytes(rowSpan);
+
             Span<TPixel> pixelSpan = pixels.DangerousGetRowSpan(y);
             PixelOperations<TPixel>.Instance.FromRgb48Bytes(
                 configuration,
@@ -163,6 +213,30 @@ private static void ProcessWideRgb<TPixel>(Configuration configuration, Buffer2D
         }
     }
 
+    /// <summary>
+    /// Reverses the byte order of each 16-bit sample in the given row when the host is little-endian.
+    /// The binary PGM and PPM formats store multi-byte samples most significant byte first.
+    /// </summary>
+    /// <param name="rowSpan">The row of big-endian sample data to convert in place.</param>
+    private static void SwapSampleBytes(Span<byte> rowSpan)
+    {
+        if (BitConverter.IsLittleEndian)
+        {
+            Span<ushort> samples = MemoryMarshal.Cast<byte, ushort>(rowSpan);
+            BinaryPrimitives.ReverseEndianness(samples, samples);
+        }
+    }
+
+    /// <summary>
+    /// Decodes binary black and white (PBM) pixel data.
+    /// Each byte holds eight pixels, most significant bit first, and a set bit means black.
+    /// Each row starts on a byte boundary, so the last byte of a row can hold unused bits.
+    /// When the stream ends early, the pixels that were not read keep their default value.
+    /// </summary>
+    /// <typeparam name="TPixel">The type of pixel to decode to.</typeparam>
+    /// <param name="configuration">The configuration.</param>
+    /// <param name="pixels">The pixel buffer to decode into.</param>
+    /// <param name="stream">The stream to read the data from.</param>
     private static void ProcessBlackAndWhite<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream)
         where TPixel : unmanaged, IPixel<TPixel>
     {

src/ImageSharp/Formats/Pbm/BinaryEncoder.cs

@@ -2,6 +2,8 @@
 // Licensed under the Six Labors Split License.
 
 using System.Buffers;
+using System.Buffers.Binary;
+using System.Runtime.InteropServices;
 using SixLabors.ImageSharp.Memory;
 using SixLabors.ImageSharp.PixelFormats;
 
@@ -13,14 +15,14 @@ namespace SixLabors.ImageSharp.Formats.Pbm;
 internal class BinaryEncoder
 {
     /// <summary>
-    /// Decode pixels into the PBM binary encoding.
+    /// Encode pixels into the PBM binary encoding.
     /// </summary>
     /// <typeparam name="TPixel">The type of input pixel.</typeparam>
     /// <param name="configuration">The configuration.</param>
     /// <param name="stream">The byte stream to write to.</param>
     /// <param name="image">The input image.</param>
-    /// <param name="colorType">The ColorType to use.</param>
-    /// <param name="componentType">Data type of the pixels components.</param>
+    /// <param name="colorType">The color type to use.</param>
+    /// <param name="componentType">The data type of the pixel components.</param>
     /// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
     /// <exception cref="ImageFormatException">
     /// Thrown if an invalid combination of setting is requested.
@@ -70,6 +72,15 @@ public static void WritePixels<TPixel>(
         }
     }
 
+    /// <summary>
+    /// Encodes 8-bit binary grayscale (PGM) pixel data.
+    /// Each pixel is written as a single byte that holds its luminance value.
+    /// </summary>
+    /// <typeparam name="TPixel">The type of input pixel.</typeparam>
+    /// <param name="configuration">The configuration.</param>
+    /// <param name="stream">The byte stream to write to.</param>
+    /// <param name="image">The input image.</param>
+    /// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
     private static void WriteGrayscale<TPixel>(
         Configuration configuration,
         Stream stream,
@@ -100,6 +111,15 @@ private static void WriteGrayscale<TPixel>(
         }
     }
 
+    /// <summary>
+    /// Encodes 16-bit binary grayscale (PGM) pixel data.
+    /// Each pixel is written as one 16-bit sample, most significant byte first.
+    /// </summary>
+    /// <typeparam name="TPixel">The type of input pixel.</typeparam>
+    /// <param name="configuration">The configuration.</param>
+    /// <param name="stream">The byte stream to write to.</param>
+    /// <param name="image">The input image.</param>
+    /// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
     private static void WriteWideGrayscale<TPixel>(
         Configuration configuration,
         Stream stream,
@@ -127,10 +147,23 @@ private static void WriteWideGrayscale<TPixel>(
                 rowSpan,
                 width);
 
+            // The binary format stores 16-bit samples most significant byte first,
+            // but ToL16Bytes produces native (little-endian) byte order.
+            SwapSampleBytes(rowSpan);
+
             stream.Write(rowSpan);
         }
     }
 
+    /// <summary>
+    /// Encodes 8-bit binary color (PPM) pixel data.
+    /// Each pixel is written as three bytes in red, green, blue order.
+    /// </summary>
+    /// <typeparam name="TPixel">The type of input pixel.</typeparam>
+    /// <param name="configuration">The configuration.</param>
+    /// <param name="stream">The byte stream to write to.</param>
+    /// <param name="image">The input image.</param>
+    /// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
     private static void WriteRgb<TPixel>(
         Configuration configuration,
         Stream stream,
@@ -162,6 +195,15 @@ private static void WriteRgb<TPixel>(
         }
     }
 
+    /// <summary>
+    /// Encodes 16-bit binary color (PPM) pixel data.
+    /// Each pixel is written as three 16-bit samples in red, green, blue order, most significant byte first.
+    /// </summary>
+    /// <typeparam name="TPixel">The type of input pixel.</typeparam>
+    /// <param name="configuration">The configuration.</param>
+    /// <param name="stream">The byte stream to write to.</param>
+    /// <param name="image">The input image.</param>
+    /// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
     private static void WriteWideRgb<TPixel>(
         Configuration configuration,
         Stream stream,
@@ -189,10 +231,39 @@ private static void WriteWideRgb<TPixel>(
                 rowSpan,
                 width);
 
+            // The binary format stores 16-bit samples most significant byte first,
+            // but ToRgb48Bytes produces native (little-endian) byte order.
+            SwapSampleBytes(rowSpan);
+
             stream.Write(rowSpan);
         }
     }
 
+    /// <summary>
+    /// Reverses the byte order of each 16-bit sample in the given row when the host is little-endian.
+    /// The binary PGM and PPM formats store multi-byte samples most significant byte first.
+    /// </summary>
+    /// <param name="rowSpan">The row of native-endian sample data to convert in place.</param>
+    private static void SwapSampleBytes(Span<byte> rowSpan)
+    {
+        if (BitConverter.IsLittleEndian)
+        {
+            Span<ushort> samples = MemoryMarshal.Cast<byte, ushort>(rowSpan);
+            BinaryPrimitives.ReverseEndianness(samples, samples);
+        }
+    }
+
+    /// <summary>
+    /// Encodes binary black and white (PBM) pixel data.
+    /// Each byte holds eight pixels, most significant bit first, and a set bit means black.
+    /// A pixel with a luminance value less than 128 is written as black.
+    /// Each row starts on a byte boundary, so the last byte of a row can hold unused bits.
+    /// </summary>
+    /// <typeparam name="TPixel">The type of input pixel.</typeparam>
+    /// <param name="configuration">The configuration.</param>
+    /// <param name="stream">The byte stream to write to.</param>
+    /// <param name="image">The input image.</param>
+    /// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
     private static void WriteBlackAndWhite<TPixel>(
         Configuration
         configuration,

src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs

@@ -53,9 +53,7 @@ internal sealed class PbmDecoderCore : ImageDecoderCore
     /// <param name="options">The decoder options.</param>
     public PbmDecoderCore(DecoderOptions options)
         : base(options)
-    {
-        this.configuration = options.Configuration;
-    }
+        => this.configuration = options.Configuration;
 
     /// <inheritdoc/>
     protected override Image<TPixel> Decode<TPixel>(BufferedReadStream stream, CancellationToken cancellationToken)
@@ -205,7 +203,7 @@ private void ProcessUpscaling<TPixel>(Image<TPixel> image)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         int maxAllocationValue = this.componentType == PbmComponentType.Short ? 65535 : 255;
-        float factor = maxAllocationValue / this.maxPixelValue;
+        float factor = maxAllocationValue / (float)this.maxPixelValue;
         image.Mutate(x => x.Brightness(factor));
     }