Skip to content

Scale PGM and PPM samples to the full range before converting them to the pixel type #3227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jafin wants to merge 1 commit into
base: main
Choose a base branch
from
+164 −39
Open

Scale PGM and PPM samples to the full range before converting them to the pixel type #3227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 62 additions & 9 deletions src/ImageSharp/Formats/Pbm/BinaryDecoder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,29 +34,30 @@ 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>
{
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
Expand All @@ -74,7 +75,8 @@ public static void Process<TPixel>(Configuration configuration, Buffer2D<TPixel>
/// <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)
/// <param name="maxPixelValue">The maximum sample value declared in the header.</param>
private static void ProcessGrayscale<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream, int maxPixelValue)
where TPixel : unmanaged, IPixel<TPixel>
{
const int bytesPerPixel = 1;
Expand All @@ -91,6 +93,8 @@ private static void ProcessGrayscale<TPixel>(Configuration configuration, Buffer
return;
}

ScaleSamples(rowSpan, maxPixelValue);

Span<TPixel> pixelSpan = pixels.DangerousGetRowSpan(y);
PixelOperations<TPixel>.Instance.FromL8Bytes(
configuration,
Expand All @@ -109,7 +113,8 @@ private static void ProcessGrayscale<TPixel>(Configuration configuration, Buffer
/// <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)
/// <param name="maxPixelValue">The maximum sample value declared in the header.</param>
private static void ProcessWideGrayscale<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream, int maxPixelValue)
where TPixel : unmanaged, IPixel<TPixel>
{
const int bytesPerPixel = 2;
Expand All @@ -129,6 +134,7 @@ private static void ProcessWideGrayscale<TPixel>(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<byte, ushort>(rowSpan), maxPixelValue);

Span<TPixel> pixelSpan = pixels.DangerousGetRowSpan(y);
PixelOperations<TPixel>.Instance.FromL16Bytes(
Expand All @@ -148,7 +154,8 @@ private static void ProcessWideGrayscale<TPixel>(Configuration configuration, Bu
/// <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)
/// <param name="maxPixelValue">The maximum sample value declared in the header.</param>
private static void ProcessRgb<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream, int maxPixelValue)
where TPixel : unmanaged, IPixel<TPixel>
{
const int bytesPerPixel = 3;
Expand All @@ -165,6 +172,8 @@ private static void ProcessRgb<TPixel>(Configuration configuration, Buffer2D<TPi
return;
}

ScaleSamples(rowSpan, maxPixelValue);

Span<TPixel> pixelSpan = pixels.DangerousGetRowSpan(y);
PixelOperations<TPixel>.Instance.FromRgb24Bytes(
configuration,
Expand All @@ -183,7 +192,8 @@ private static void ProcessRgb<TPixel>(Configuration configuration, Buffer2D<TPi
/// <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)
/// <param name="maxPixelValue">The maximum sample value declared in the header.</param>
private static void ProcessWideRgb<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream, int maxPixelValue)
where TPixel : unmanaged, IPixel<TPixel>
{
const int bytesPerPixel = 6;
Expand All @@ -203,6 +213,7 @@ private static void ProcessWideRgb<TPixel>(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<byte, ushort>(rowSpan), maxPixelValue);

Span<TPixel> pixelSpan = pixels.DangerousGetRowSpan(y);
PixelOperations<TPixel>.Instance.FromRgb48Bytes(
Expand All @@ -227,6 +238,48 @@ private static void SwapSampleBytes(Span<byte> rowSpan)
}
}

/// <summary>
/// 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.
/// </summary>
/// <param name="samples">The row of samples to scale.</param>
/// <param name="maxPixelValue">The maximum sample value declared in the header.</param>
private static void ScaleSamples(Span<byte> 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);
}
}

/// <summary>
/// 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.
/// </summary>
/// <param name="samples">The row of samples to scale.</param>
/// <param name="maxPixelValue">The maximum sample value declared in the header.</param>
private static void ScaleSamples(Span<ushort> 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);
}
}

/// <summary>
/// Decodes binary black and white (PBM) pixel data.
/// Each byte holds eight pixels, most significant bit first, and a set bit means black.
Expand Down
19 changes: 2 additions & 17 deletions src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -191,21 +186,11 @@ private void ProcessPixels<TPixel>(BufferedReadStream stream, Buffer2D<TPixel> 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<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;
}
30 changes: 30 additions & 0 deletions src/ImageSharp/Formats/Pbm/PbmSampleScaler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright (c) Six Labors.
// Licensed under the Six Labors Split License.

namespace SixLabors.ImageSharp.Formats.Pbm;

/// <summary>
/// Scales PGM and PPM samples from the range declared by the header maxval to the full range of the component type.
/// </summary>
internal static class PbmSampleScaler
{
/// <summary>
/// Scales a sample from the range [0, <paramref name="maxPixelValue"/>] to the range [0, <paramref name="targetMaxValue"/>],
/// rounding to the nearest value. Samples outside the declared range are clamped to <paramref name="maxPixelValue"/>.
/// </summary>
/// <param name="value">The sample value read from the stream.</param>
/// <param name="maxPixelValue">The maxval declared in the header, between 1 and 65535.</param>
/// <param name="targetMaxValue">The maximum value of the component type, either 255 or 65535.</param>
/// <returns>The scaled sample value.</returns>
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);
}
}
35 changes: 22 additions & 13 deletions src/ImageSharp/Formats/Pbm/PlainDecoder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,29 +25,30 @@ internal class PlainDecoder
/// <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>
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>
{
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
Expand All @@ -56,7 +57,7 @@ public static void Process<TPixel>(Configuration configuration, Buffer2D<TPixel>
}
}

private static void ProcessGrayscale<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream)
private static void ProcessGrayscale<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream, int maxPixelValue)
where TPixel : unmanaged, IPixel<TPixel>
{
int width = pixels.Width;
Expand All @@ -71,7 +72,7 @@ private static void ProcessGrayscale<TPixel>(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)
{
Expand All @@ -92,7 +93,7 @@ private static void ProcessGrayscale<TPixel>(Configuration configuration, Buffer
}
}

private static void ProcessWideGrayscale<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream)
private static void ProcessWideGrayscale<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream, int maxPixelValue)
where TPixel : unmanaged, IPixel<TPixel>
{
int width = pixels.Width;
Expand All @@ -107,7 +108,7 @@ private static void ProcessWideGrayscale<TPixel>(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)
{
Expand All @@ -128,7 +129,7 @@ private static void ProcessWideGrayscale<TPixel>(Configuration configuration, Bu
}
}

private static void ProcessRgb<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream)
private static void ProcessRgb<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream, int maxPixelValue)
where TPixel : unmanaged, IPixel<TPixel>
{
int width = pixels.Width;
Expand All @@ -154,7 +155,11 @@ private static void ProcessRgb<TPixel>(Configuration configuration, Buffer2D<TPi

stream.ReadDecimal(out int blue);

rowSpan[x] = new Rgb24((byte)red, (byte)green, (byte)blue);
rowSpan[x] = new Rgb24(
(byte)PbmSampleScaler.Scale(red, maxPixelValue, byte.MaxValue),
(byte)PbmSampleScaler.Scale(green, maxPixelValue, byte.MaxValue),
(byte)PbmSampleScaler.Scale(blue, maxPixelValue, byte.MaxValue));

eofReached = !stream.SkipWhitespaceAndComments();
if (eofReached)
{
Expand All @@ -175,7 +180,7 @@ private static void ProcessRgb<TPixel>(Configuration configuration, Buffer2D<TPi
}
}

private static void ProcessWideRgb<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream)
private static void ProcessWideRgb<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream, int maxPixelValue)
where TPixel : unmanaged, IPixel<TPixel>
{
int width = pixels.Width;
Expand All @@ -201,7 +206,11 @@ private static void ProcessWideRgb<TPixel>(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)
{
Expand Down
Loading