[V4] Preserve PGM and PPM sample precision

src/ImageSharp/Formats/Pbm/BinaryDecoder.cs

@@ -3,6 +3,7 @@
 
 using System.Buffers;
 using System.Buffers.Binary;
+using System.Numerics;
 using System.Runtime.InteropServices;
 using SixLabors.ImageSharp.IO;
 using SixLabors.ImageSharp.Memory;
@@ -34,9 +35,25 @@ internal class BinaryDecoder
     /// <param name="stream">The stream to read the data from.</param>
     /// <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)
+    /// <param name="maxPixelValue">The maximum sample value declared in the header. Unused for black and white.</param>
+    public static void Process<TPixel>(
+        Configuration configuration,
+        Buffer2D<TPixel> pixels,
+        BufferedReadStream stream,
+        PbmColorType colorType,
+        PbmComponentType componentType,
+        int maxPixelValue)
         where TPixel : unmanaged, IPixel<TPixel>
     {
+        // Full-range inputs keep the existing byte and ushort conversion paths.
+        // Other ranges need normalized floats until the destination pixel format performs its final packing.
+        int fullRange = componentType == PbmComponentType.Short ? ushort.MaxValue : byte.MaxValue;
+        if (colorType != PbmColorType.BlackAndWhite && maxPixelValue != fullRange)
+        {
+            ProcessScaled(configuration, pixels, stream, colorType, componentType, maxPixelValue);
+            return;
+        }
+
         if (colorType == PbmColorType.Grayscale)
         {
             if (componentType == PbmComponentType.Byte)
@@ -65,6 +82,77 @@ public static void Process<TPixel>(Configuration configuration, Buffer2D<TPixel>
         }
     }
 
+    /// <summary>
+    /// Normalizes binary samples before the destination pixel format packs them.
+    /// </summary>
+    /// <typeparam name="TPixel">The destination pixel format.</typeparam>
+    /// <param name="configuration">The configuration.</param>
+    /// <param name="pixels">The destination pixels.</param>
+    /// <param name="stream">The input stream at the raster.</param>
+    /// <param name="colorType">The encoded color type.</param>
+    /// <param name="componentType">The encoded sample size.</param>
+    /// <param name="maxPixelValue">The declared maximum sample value.</param>
+    private static void ProcessScaled<TPixel>(
+        Configuration configuration,
+        Buffer2D<TPixel> pixels,
+        BufferedReadStream stream,
+        PbmColorType colorType,
+        PbmComponentType componentType,
+        int maxPixelValue)
+        where TPixel : unmanaged, IPixel<TPixel>
+    {
+        bool isRgb = colorType == PbmColorType.Rgb;
+        bool isWide = componentType == PbmComponentType.Short;
+        int sampleSize = isWide ? sizeof(ushort) : sizeof(byte);
+        int bytesPerPixel = sampleSize * (isRgb ? 3 : 1);
+        int width = pixels.Width;
+
+        // Rent both rows once. Encoded bytes stay separate from the float row, so widening cannot overwrite unread samples.
+        using IMemoryOwner<byte> encoded = configuration.MemoryAllocator.Allocate<byte>(width * bytesPerPixel);
+        using IMemoryOwner<Vector4> vectors = configuration.MemoryAllocator.Allocate<Vector4>(width);
+        Span<byte> encodedRow = encoded.GetSpan();
+        Span<Vector4> vectorRow = vectors.GetSpan();
+
+        for (int y = 0; y < pixels.Height; y++)
+        {
+            int count = ReadRow(stream, encodedRow, bytesPerPixel);
+            if (count == 0)
+            {
+                return;
+            }
+
+            for (int x = 0; x < count; x++)
+            {
+                int offset = x * bytesPerPixel;
+                int red = isWide ? BinaryPrimitives.ReadUInt16BigEndian(encodedRow.Slice(offset, sampleSize)) : encodedRow[offset];
+                int green = red;
+                int blue = red;
+                if (isRgb)
+                {
+                    offset += sampleSize;
+                    green = isWide ? BinaryPrimitives.ReadUInt16BigEndian(encodedRow.Slice(offset, sampleSize)) : encodedRow[offset];
+                    offset += sampleSize;
+                    blue = isWide ? BinaryPrimitives.ReadUInt16BigEndian(encodedRow.Slice(offset, sampleSize)) : encodedRow[offset];
+                }
+
+                // RGB holds the encoded magnitudes. Alpha uses Maxval so normalization produces exactly opaque alpha.
+                vectorRow[x] = PbmSampleScaler.Scale(new Vector4(red, green, blue, maxPixelValue), maxPixelValue);
+            }
+
+            // Convert only complete pixels. A short final row must not copy unused pooled memory into the image.
+            PixelOperations<TPixel>.Instance.FromVector4Destructive(
+                configuration,
+                vectorRow[..count],
+                pixels.DangerousGetRowSpan(y)[..count],
+                PixelConversionModifiers.Scale);
+
+            if (count < width)
+            {
+                return;
+            }
+        }
+    }
+
     /// <summary>
     /// Decodes 8-bit binary grayscale (PGM) pixel data.
     /// Each pixel is a single byte that holds its luminance value.
@@ -86,17 +174,20 @@ private static void ProcessGrayscale<TPixel>(Configuration configuration, Buffer
 
         for (int y = 0; y < height; y++)
         {
-            if (stream.Read(rowSpan) < rowSpan.Length)
+            int count = ReadRow(stream, rowSpan, bytesPerPixel);
+            if (count == 0)
             {
                 return;
             }
 
+            Span<byte> samples = rowSpan[..(count * bytesPerPixel)];
+
             Span<TPixel> pixelSpan = pixels.DangerousGetRowSpan(y);
             PixelOperations<TPixel>.Instance.FromL8Bytes(
                 configuration,
-                rowSpan,
-                pixelSpan,
-                width);
+                samples,
+                pixelSpan[..count],
+                count);
         }
     }
 
@@ -121,21 +212,24 @@ private static void ProcessWideGrayscale<TPixel>(Configuration configuration, Bu
 
         for (int y = 0; y < height; y++)
         {
-            if (stream.Read(rowSpan) < rowSpan.Length)
+            int count = ReadRow(stream, rowSpan, bytesPerPixel);
+            if (count == 0)
             {
                 return;
             }
 
+            Span<byte> samples = rowSpan[..(count * bytesPerPixel)];
+
             // The binary format stores 16-bit samples most significant byte first,
             // but L16 expects native (little-endian) byte order.
-            SwapSampleBytes(rowSpan);
+            SwapSampleBytes(samples);
 
             Span<TPixel> pixelSpan = pixels.DangerousGetRowSpan(y);
             PixelOperations<TPixel>.Instance.FromL16Bytes(
                 configuration,
-                rowSpan,
-                pixelSpan,
-                width);
+                samples,
+                pixelSpan[..count],
+                count);
         }
     }
 
@@ -160,17 +254,20 @@ private static void ProcessRgb<TPixel>(Configuration configuration, Buffer2D<TPi
 
         for (int y = 0; y < height; y++)
         {
-            if (stream.Read(rowSpan) < rowSpan.Length)
+            int count = ReadRow(stream, rowSpan, bytesPerPixel);
+            if (count == 0)
             {
                 return;
             }
 
+            Span<byte> samples = rowSpan[..(count * bytesPerPixel)];
+
             Span<TPixel> pixelSpan = pixels.DangerousGetRowSpan(y);
             PixelOperations<TPixel>.Instance.FromRgb24Bytes(
                 configuration,
-                rowSpan,
-                pixelSpan,
-                width);
+                samples,
+                pixelSpan[..count],
+                count);
         }
     }
 
@@ -195,24 +292,48 @@ private static void ProcessWideRgb<TPixel>(Configuration configuration, Buffer2D
 
         for (int y = 0; y < height; y++)
         {
-            if (stream.Read(rowSpan) < rowSpan.Length)
+            int count = ReadRow(stream, rowSpan, bytesPerPixel);
+            if (count == 0)
             {
                 return;
             }
 
+            Span<byte> samples = rowSpan[..(count * bytesPerPixel)];
+
             // The binary format stores 16-bit samples most significant byte first,
             // but Rgb48 expects native (little-endian) byte order.
-            SwapSampleBytes(rowSpan);
+            SwapSampleBytes(samples);
 
             Span<TPixel> pixelSpan = pixels.DangerousGetRowSpan(y);
             PixelOperations<TPixel>.Instance.FromRgb48Bytes(
                 configuration,
-                rowSpan,
-                pixelSpan,
-                width);
+                samples,
+                pixelSpan[..count],
+                count);
         }
     }
 
+    /// <summary>
+    /// Reads one encoded row and zero-fills only missing bytes in its last incomplete pixel.
+    /// </summary>
+    /// <param name="stream">The input stream.</param>
+    /// <param name="row">The reusable encoded row.</param>
+    /// <param name="bytesPerPixel">The encoded size of one pixel.</param>
+    /// <returns>The number of available pixels, including an incomplete final pixel.</returns>
+    private static int ReadRow(BufferedReadStream stream, Span<byte> row, int bytesPerPixel)
+    {
+        int bytesRead = stream.Read(row);
+        int count = bytesRead / bytesPerPixel;
+        if (bytesRead % bytesPerPixel != 0)
+        {
+            // Keep the available channels and high sample bytes. Missing bytes must not come from pooled storage.
+            count++;
+            row.Slice(bytesRead, (count * bytesPerPixel) - bytesRead).Clear();
+        }
+
+        return count;
+    }
+
     /// <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.

src/ImageSharp/Formats/Pbm/BufferedReadStreamExtensions.cs

@@ -62,8 +62,19 @@ public static bool SkipWhitespaceAndComments(this BufferedReadStream stream)
     /// It's up to the call site to handle such a situation.
     /// </remarks>
     public static bool ReadDecimal(this BufferedReadStream stream, out int value)
+        => stream.ReadDecimal(out value, out _);
+
+    /// <summary>
+    /// Reads a decimal value and reports values above the supported integer range without wrapping.
+    /// </summary>
+    /// <param name="stream">The input stream.</param>
+    /// <param name="value">The parsed value, saturated at <see cref="int.MaxValue"/>.</param>
+    /// <param name="overflow">True if the decimal value exceeds <see cref="int.MaxValue"/>.</param>
+    /// <returns>False if EOF was reached. The final digits are retained in <paramref name="value"/>.</returns>
+    public static bool ReadDecimal(this BufferedReadStream stream, out int value, out bool overflow)
     {
         value = 0;
+        overflow = false;
         while (true)
         {
             int current = stream.ReadByte();
@@ -78,7 +89,17 @@ public static bool ReadDecimal(this BufferedReadStream stream, out int value)
                 break;
             }
 
-            value = (value * 10) + current;
+            // Keep consuming digits after overflow so the next sample starts at the correct byte.
+            // Sample conversion can saturate this value. Header parsing rejects an overflowing field.
+            if (value > (int.MaxValue - current) / 10)
+            {
+                value = int.MaxValue;
+                overflow = true;
+            }
+            else
+            {
+                value = (value * 10) + current;
+            }
         }
 
         return true;

src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs

@@ -6,7 +6,6 @@
 using SixLabors.ImageSharp.Memory;
 using SixLabors.ImageSharp.Metadata;
 using SixLabors.ImageSharp.PixelFormats;
-using SixLabors.ImageSharp.Processing;
 
 namespace SixLabors.ImageSharp.Formats.Pbm;
 
@@ -65,10 +64,6 @@ protected override Image<TPixel> Decode<TPixel>(BufferedReadStream stream, Cance
         Buffer2D<TPixel> pixels = image.GetRootFramePixelBuffer();
 
         this.ProcessPixels(stream, pixels);
-        if (this.NeedsUpscaling())
-        {
-            this.ProcessUpscaling(image);
-        }
 
         return image;
     }
@@ -138,19 +133,25 @@ private void ProcessHeader(BufferedReadStream stream)
         }
 
         if (!stream.SkipWhitespaceAndComments() ||
-            !stream.ReadDecimal(out int width) ||
+            !stream.ReadDecimal(out int width, out bool widthOverflow) || widthOverflow ||
             !stream.SkipWhitespaceAndComments() ||
-            !stream.ReadDecimal(out int height) ||
-            !stream.SkipWhitespaceAndComments())
+            !stream.ReadDecimal(out int height, out bool heightOverflow) || heightOverflow)
+        {
+            ThrowInvalidHeader();
+        }
+
+        // ReadDecimal consumed the separator after height. Only PBM's binary raster starts here;
+        // other formats still need a Maxval token, and plain PBM still needs to skip token whitespace.
+        if ((this.colorType != PbmColorType.BlackAndWhite || this.encoding == PbmEncoding.Plain) && !stream.SkipWhitespaceAndComments())
         {
-            ThrowPrematureEof();
+            ThrowInvalidHeader();
         }
 
         if (this.colorType != PbmColorType.BlackAndWhite)
         {
-            if (!stream.ReadDecimal(out this.maxPixelValue))
+            if (!stream.ReadDecimal(out this.maxPixelValue, out bool maxValueOverflow) || maxValueOverflow)
             {
-                ThrowPrematureEof();
+                ThrowInvalidHeader();
             }
 
             if (this.maxPixelValue <= 0 || this.maxPixelValue >= 65536)
@@ -167,7 +168,12 @@ private void ProcessHeader(BufferedReadStream stream)
                 this.componentType = PbmComponentType.Byte;
             }
 
-            stream.SkipWhitespaceAndComments();
+            // The binary formats have exactly one separator before the raster. Its bytes can equal whitespace or '#'.
+            // Plain samples are decimal tokens, so their leading whitespace and comments still need to be skipped.
+            if (this.encoding == PbmEncoding.Plain)
+            {
+                stream.SkipWhitespaceAndComments();
+            }
         }
         else
         {
@@ -182,30 +188,21 @@ private void ProcessHeader(BufferedReadStream stream)
         meta.ColorType = this.colorType;
         meta.ComponentType = this.componentType;
 
+        // Reject an incomplete header or a decimal field that exceeds the supported integer range.
         [DoesNotReturn]
-        static void ThrowPrematureEof() => throw new InvalidImageContentException("Reached EOF while reading the header.");
+        static void ThrowInvalidHeader() => throw new InvalidImageContentException("Invalid or incomplete PBM header.");
     }
 
     private void ProcessPixels<TPixel>(BufferedReadStream stream, Buffer2D<TPixel> pixels)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         if (this.encoding == PbmEncoding.Binary)
         {
-            BinaryDecoder.Process(this.configuration, pixels, stream, this.colorType, this.componentType);
+            BinaryDecoder.Process(this.configuration, pixels, stream, this.colorType, this.componentType, this.maxPixelValue);
         }
         else
         {
-            PlainDecoder.Process(this.configuration, pixels, stream, this.colorType, this.componentType);
+            PlainDecoder.Process(this.configuration, pixels, stream, this.colorType, this.componentType, this.maxPixelValue);
         }
     }
-
-    private void ProcessUpscaling<TPixel>(Image<TPixel> image)
-        where TPixel : unmanaged, IPixel<TPixel>
-    {
-        int maxAllocationValue = this.componentType == PbmComponentType.Short ? 65535 : 255;
-        float factor = maxAllocationValue / (float)this.maxPixelValue;
-        image.Mutate(x => x.Brightness(factor));
-    }
-
-    private bool NeedsUpscaling() => this.colorType != PbmColorType.BlackAndWhite && this.maxPixelValue is not 255 and not 65535;
 }