From 8d5842fa80060506841e92dee7547e65b5337cf7 Mon Sep 17 00:00:00 2001 From: Jason Finch Date: Sun, 18 Oct 2026 09:18:12 +1000 Subject: [PATCH] Scale PGM and PPM samples to the full range before converting them to the pixel type A non-standard maxval was applied with a brightness pass after the samples had been converted to the destination pixel type. A 16-bit sample with a small maxval was narrowed to 8 bits first, so decoding to an 8-bit pixel type such as the default Rgb24 kept only a few distinct levels (maxval 1000 gave five). The plain and binary decoders now scale every sample from [0, maxval] to the full 8-bit or 16-bit range with rounded integer arithmetic before the pixel conversion, and the extra brightness pass over the image is removed. --- src/ImageSharp/Formats/Pbm/BinaryDecoder.cs | 71 ++++++++++++++++--- src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs | 19 +---- src/ImageSharp/Formats/Pbm/PbmSampleScaler.cs | 30 ++++++++ src/ImageSharp/Formats/Pbm/PlainDecoder.cs | 35 +++++---- .../Formats/Pbm/PbmDecoderTests.cs | 48 +++++++++++++ 5 files changed, 164 insertions(+), 39 deletions(-) create mode 100644 src/ImageSharp/Formats/Pbm/PbmSampleScaler.cs diff --git a/src/ImageSharp/Formats/Pbm/BinaryDecoder.cs b/src/ImageSharp/Formats/Pbm/BinaryDecoder.cs index a465907cf0..f83d3f74b8 100644 --- a/src/ImageSharp/Formats/Pbm/BinaryDecoder.cs +++ b/src/ImageSharp/Formats/Pbm/BinaryDecoder.cs @@ -34,29 +34,30 @@ internal class BinaryDecoder /// The stream to read the data from. /// The color type of the encoded pixels. /// The data type of the pixel components. - public static void Process(Configuration configuration, Buffer2D pixels, BufferedReadStream stream, PbmColorType colorType, PbmComponentType componentType) + /// The maximum sample value declared in the header. Unused for black and white. + public static void Process(Configuration configuration, Buffer2D pixels, BufferedReadStream stream, PbmColorType colorType, PbmComponentType componentType, int maxPixelValue) where TPixel : unmanaged, IPixel { if (colorType == PbmColorType.Grayscale) { if (componentType == PbmComponentType.Byte) { - ProcessGrayscale(configuration, pixels, stream); + ProcessGrayscale(configuration, pixels, stream, maxPixelValue); } else { - ProcessWideGrayscale(configuration, pixels, stream); + ProcessWideGrayscale(configuration, pixels, stream, maxPixelValue); } } else if (colorType == PbmColorType.Rgb) { if (componentType == PbmComponentType.Byte) { - ProcessRgb(configuration, pixels, stream); + ProcessRgb(configuration, pixels, stream, maxPixelValue); } else { - ProcessWideRgb(configuration, pixels, stream); + ProcessWideRgb(configuration, pixels, stream, maxPixelValue); } } else @@ -74,7 +75,8 @@ public static void Process(Configuration configuration, Buffer2D /// The configuration. /// The pixel buffer to decode into. /// The stream to read the data from. - private static void ProcessGrayscale(Configuration configuration, Buffer2D pixels, BufferedReadStream stream) + /// The maximum sample value declared in the header. + private static void ProcessGrayscale(Configuration configuration, Buffer2D pixels, BufferedReadStream stream, int maxPixelValue) where TPixel : unmanaged, IPixel { const int bytesPerPixel = 1; @@ -91,6 +93,8 @@ private static void ProcessGrayscale(Configuration configuration, Buffer return; } + ScaleSamples(rowSpan, maxPixelValue); + Span pixelSpan = pixels.DangerousGetRowSpan(y); PixelOperations.Instance.FromL8Bytes( configuration, @@ -109,7 +113,8 @@ private static void ProcessGrayscale(Configuration configuration, Buffer /// The configuration. /// The pixel buffer to decode into. /// The stream to read the data from. - private static void ProcessWideGrayscale(Configuration configuration, Buffer2D pixels, BufferedReadStream stream) + /// The maximum sample value declared in the header. + private static void ProcessWideGrayscale(Configuration configuration, Buffer2D pixels, BufferedReadStream stream, int maxPixelValue) where TPixel : unmanaged, IPixel { const int bytesPerPixel = 2; @@ -129,6 +134,7 @@ private static void ProcessWideGrayscale(Configuration configuration, Bu // The binary format stores 16-bit samples most significant byte first, // but L16 expects native (little-endian) byte order. SwapSampleBytes(rowSpan); + ScaleSamples(MemoryMarshal.Cast(rowSpan), maxPixelValue); Span pixelSpan = pixels.DangerousGetRowSpan(y); PixelOperations.Instance.FromL16Bytes( @@ -148,7 +154,8 @@ private static void ProcessWideGrayscale(Configuration configuration, Bu /// The configuration. /// The pixel buffer to decode into. /// The stream to read the data from. - private static void ProcessRgb(Configuration configuration, Buffer2D pixels, BufferedReadStream stream) + /// The maximum sample value declared in the header. + private static void ProcessRgb(Configuration configuration, Buffer2D pixels, BufferedReadStream stream, int maxPixelValue) where TPixel : unmanaged, IPixel { const int bytesPerPixel = 3; @@ -165,6 +172,8 @@ private static void ProcessRgb(Configuration configuration, Buffer2D pixelSpan = pixels.DangerousGetRowSpan(y); PixelOperations.Instance.FromRgb24Bytes( configuration, @@ -183,7 +192,8 @@ private static void ProcessRgb(Configuration configuration, Buffer2DThe configuration. /// The pixel buffer to decode into. /// The stream to read the data from. - private static void ProcessWideRgb(Configuration configuration, Buffer2D pixels, BufferedReadStream stream) + /// The maximum sample value declared in the header. + private static void ProcessWideRgb(Configuration configuration, Buffer2D pixels, BufferedReadStream stream, int maxPixelValue) where TPixel : unmanaged, IPixel { const int bytesPerPixel = 6; @@ -203,6 +213,7 @@ private static void ProcessWideRgb(Configuration configuration, Buffer2D // The binary format stores 16-bit samples most significant byte first, // but Rgb48 expects native (little-endian) byte order. SwapSampleBytes(rowSpan); + ScaleSamples(MemoryMarshal.Cast(rowSpan), maxPixelValue); Span pixelSpan = pixels.DangerousGetRowSpan(y); PixelOperations.Instance.FromRgb48Bytes( @@ -227,6 +238,48 @@ private static void SwapSampleBytes(Span rowSpan) } } + /// + /// Scales a row of 8-bit samples in place from the range [0, maxval] to the full range [0, 255]. + /// Scaling happens on the samples, before they are converted to the destination pixel type, + /// so the result does not depend on the precision of that pixel type. + /// + /// The row of samples to scale. + /// The maximum sample value declared in the header. + private static void ScaleSamples(Span samples, int maxPixelValue) + { + // Samples that already use the full range are left as they are, so the common case costs nothing. + if (maxPixelValue == byte.MaxValue) + { + return; + } + + for (int i = 0; i < samples.Length; i++) + { + samples[i] = (byte)PbmSampleScaler.Scale(samples[i], maxPixelValue, byte.MaxValue); + } + } + + /// + /// Scales a row of native-endian 16-bit samples in place from the range [0, maxval] to the full range [0, 65535]. + /// Scaling happens on the samples, before they are converted to the destination pixel type, + /// so a narrower pixel type does not lose the precision of a small maxval before it is scaled. + /// + /// The row of samples to scale. + /// The maximum sample value declared in the header. + private static void ScaleSamples(Span samples, int maxPixelValue) + { + // Samples that already use the full range are left as they are, so the common case costs nothing. + if (maxPixelValue == ushort.MaxValue) + { + return; + } + + for (int i = 0; i < samples.Length; i++) + { + samples[i] = (ushort)PbmSampleScaler.Scale(samples[i], maxPixelValue, ushort.MaxValue); + } + } + /// /// Decodes binary black and white (PBM) pixel data. /// Each byte holds eight pixels, most significant bit first, and a set bit means black. diff --git a/src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs b/src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs index 938be62408..3c25ddc5cf 100644 --- a/src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs +++ b/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 Decode(BufferedReadStream stream, Cance Buffer2D pixels = image.GetRootFramePixelBuffer(); this.ProcessPixels(stream, pixels); - if (this.NeedsUpscaling()) - { - this.ProcessUpscaling(image); - } return image; } @@ -191,21 +186,11 @@ private void ProcessPixels(BufferedReadStream stream, Buffer2D p { 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(Image image) - where TPixel : unmanaged, IPixel - { - 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; } diff --git a/src/ImageSharp/Formats/Pbm/PbmSampleScaler.cs b/src/ImageSharp/Formats/Pbm/PbmSampleScaler.cs new file mode 100644 index 0000000000..e8af1143e6 --- /dev/null +++ b/src/ImageSharp/Formats/Pbm/PbmSampleScaler.cs @@ -0,0 +1,30 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +namespace SixLabors.ImageSharp.Formats.Pbm; + +/// +/// Scales PGM and PPM samples from the range declared by the header maxval to the full range of the component type. +/// +internal static class PbmSampleScaler +{ + /// + /// Scales a sample from the range [0, ] to the range [0, ], + /// rounding to the nearest value. Samples outside the declared range are clamped to . + /// + /// The sample value read from the stream. + /// The maxval declared in the header, between 1 and 65535. + /// The maximum value of the component type, either 255 or 65535. + /// The scaled sample value. + public static int Scale(int value, int maxPixelValue, int targetMaxValue) + { + // Negative values can only come from an overflowing decimal sample; reinterpreting them as unsigned + // makes them clamp to maxval along with every other out-of-range sample. + uint sample = Math.Min((uint)value, (uint)maxPixelValue); + + // Adding half of the divisor before dividing rounds to the nearest value, so maxval maps exactly to + // targetMaxValue and the scale does not depend on maxval dividing the target range. + // The largest intermediate value is 65535 * 65535 + 32767, which fits in an unsigned 32-bit integer. + return (int)(((sample * (uint)targetMaxValue) + ((uint)maxPixelValue >> 1)) / (uint)maxPixelValue); + } +} diff --git a/src/ImageSharp/Formats/Pbm/PlainDecoder.cs b/src/ImageSharp/Formats/Pbm/PlainDecoder.cs index 8748d90fa8..0b468d5e98 100644 --- a/src/ImageSharp/Formats/Pbm/PlainDecoder.cs +++ b/src/ImageSharp/Formats/Pbm/PlainDecoder.cs @@ -25,29 +25,30 @@ internal class PlainDecoder /// The stream to read the data from. /// The ColorType to decode. /// Data type of the pixles components. - public static void Process(Configuration configuration, Buffer2D pixels, BufferedReadStream stream, PbmColorType colorType, PbmComponentType componentType) + /// The maximum sample value declared in the header. Unused for black and white. + public static void Process(Configuration configuration, Buffer2D pixels, BufferedReadStream stream, PbmColorType colorType, PbmComponentType componentType, int maxPixelValue) where TPixel : unmanaged, IPixel { if (colorType == PbmColorType.Grayscale) { if (componentType == PbmComponentType.Byte) { - ProcessGrayscale(configuration, pixels, stream); + ProcessGrayscale(configuration, pixels, stream, maxPixelValue); } else { - ProcessWideGrayscale(configuration, pixels, stream); + ProcessWideGrayscale(configuration, pixels, stream, maxPixelValue); } } else if (colorType == PbmColorType.Rgb) { if (componentType == PbmComponentType.Byte) { - ProcessRgb(configuration, pixels, stream); + ProcessRgb(configuration, pixels, stream, maxPixelValue); } else { - ProcessWideRgb(configuration, pixels, stream); + ProcessWideRgb(configuration, pixels, stream, maxPixelValue); } } else @@ -56,7 +57,7 @@ public static void Process(Configuration configuration, Buffer2D } } - private static void ProcessGrayscale(Configuration configuration, Buffer2D pixels, BufferedReadStream stream) + private static void ProcessGrayscale(Configuration configuration, Buffer2D pixels, BufferedReadStream stream, int maxPixelValue) where TPixel : unmanaged, IPixel { int width = pixels.Width; @@ -71,7 +72,7 @@ private static void ProcessGrayscale(Configuration configuration, Buffer for (int x = 0; x < width; x++) { stream.ReadDecimal(out int value); - rowSpan[x] = new L8((byte)value); + rowSpan[x] = new L8((byte)PbmSampleScaler.Scale(value, maxPixelValue, byte.MaxValue)); eofReached = !stream.SkipWhitespaceAndComments(); if (eofReached) { @@ -92,7 +93,7 @@ private static void ProcessGrayscale(Configuration configuration, Buffer } } - private static void ProcessWideGrayscale(Configuration configuration, Buffer2D pixels, BufferedReadStream stream) + private static void ProcessWideGrayscale(Configuration configuration, Buffer2D pixels, BufferedReadStream stream, int maxPixelValue) where TPixel : unmanaged, IPixel { int width = pixels.Width; @@ -107,7 +108,7 @@ private static void ProcessWideGrayscale(Configuration configuration, Bu for (int x = 0; x < width; x++) { stream.ReadDecimal(out int value); - rowSpan[x] = new L16((ushort)value); + rowSpan[x] = new L16((ushort)PbmSampleScaler.Scale(value, maxPixelValue, ushort.MaxValue)); eofReached = !stream.SkipWhitespaceAndComments(); if (eofReached) { @@ -128,7 +129,7 @@ private static void ProcessWideGrayscale(Configuration configuration, Bu } } - private static void ProcessRgb(Configuration configuration, Buffer2D pixels, BufferedReadStream stream) + private static void ProcessRgb(Configuration configuration, Buffer2D pixels, BufferedReadStream stream, int maxPixelValue) where TPixel : unmanaged, IPixel { int width = pixels.Width; @@ -154,7 +155,11 @@ private static void ProcessRgb(Configuration configuration, Buffer2D(Configuration configuration, Buffer2D(Configuration configuration, Buffer2D pixels, BufferedReadStream stream) + private static void ProcessWideRgb(Configuration configuration, Buffer2D pixels, BufferedReadStream stream, int maxPixelValue) where TPixel : unmanaged, IPixel { int width = pixels.Width; @@ -201,7 +206,11 @@ private static void ProcessWideRgb(Configuration configuration, Buffer2D stream.ReadDecimal(out int blue); - rowSpan[x] = new Rgb48((ushort)red, (ushort)green, (ushort)blue); + rowSpan[x] = new Rgb48( + (ushort)PbmSampleScaler.Scale(red, maxPixelValue, ushort.MaxValue), + (ushort)PbmSampleScaler.Scale(green, maxPixelValue, ushort.MaxValue), + (ushort)PbmSampleScaler.Scale(blue, maxPixelValue, ushort.MaxValue)); + eofReached = !stream.SkipWhitespaceAndComments(); if (eofReached) { diff --git a/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs index 778ad75897..101a28c4ad 100644 --- a/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs @@ -163,6 +163,54 @@ public void Decode_NonStandardShortMaxPixelValue_UpscalesToFullRange() Assert.Equal(65535, image[0, 0].PackedValue); } + [Theory] + [InlineData("P2", 100)] + [InlineData("P3", 100)] + [InlineData("P5", 100)] + [InlineData("P6", 100)] + [InlineData("P2", 1000)] + [InlineData("P3", 1000)] + [InlineData("P5", 1000)] + [InlineData("P6", 1000)] + public void Decode_NonStandardMaxPixelValue_ScalesSamplesToByteRange(string magic, int maxPixelValue) + { + // Six samples make six grayscale pixels or two color pixels. They include both ends of the range + // and values that do not fall on a whole 8-bit step, so a scale that loses precision is detected. + int[] samples = [0, 7, maxPixelValue / 2, maxPixelValue - 1, maxPixelValue, 3]; + bool isRgb = magic is "P3" or "P6"; + int width = isRgb ? samples.Length / 3 : samples.Length; + + byte[] header = Encoding.ASCII.GetBytes($"{magic}\n{width} 1\n{maxPixelValue}\n"); + byte[] payload; + if (magic is "P2" or "P3") + { + payload = Encoding.ASCII.GetBytes(string.Join(' ', samples)); + } + else if (maxPixelValue > 255) + { + // Binary samples above 8 bits are stored as two bytes, most significant byte first. + payload = samples.SelectMany(s => new[] { (byte)(s >> 8), (byte)s }).ToArray(); + } + else + { + payload = samples.Select(s => (byte)s).ToArray(); + } + + using Image image = Image.Load([.. header, .. payload]); + byte[] components = new byte[width * 3]; + image.CopyPixelDataTo(components); + + for (int i = 0; i < samples.Length; i++) + { + // A grayscale sample is decoded to equal red, green and blue components, so compare the red one. + byte actual = isRgb ? components[i] : components[i * 3]; + + // A 16-bit sample is rounded once to 16 bits and again to 8 bits, so allow one step of difference. + int expected = (int)Math.Round(samples[i] * 255D / maxPixelValue, MidpointRounding.AwayFromZero); + Assert.InRange(actual, expected - 1, expected + 1); + } + } + [Theory] [WithFile(RgbPlain, PixelTypes.Rgb24)] public void PbmDecoder_Decode_Resize(TestImageProvider provider)