diff --git a/src/ImageSharp/Common/Helpers/ColorNumerics.cs b/src/ImageSharp/Common/Helpers/ColorNumerics.cs index 735f6bc597..066dbec3dc 100644 --- a/src/ImageSharp/Common/Helpers/ColorNumerics.cs +++ b/src/ImageSharp/Common/Helpers/ColorNumerics.cs @@ -97,7 +97,7 @@ internal static class ColorNumerics /// Scales a value from a 16 bit to an /// 8 bit equivalent. /// - /// The 8 bit component value. + /// The 16 bit component value. /// The [MethodImpl(MethodImplOptions.AggressiveInlining)] public static byte From16BitTo8Bit(ushort component) => @@ -133,13 +133,47 @@ internal static class ColorNumerics (byte)(((component * 255) + 32895) >> 16); /// - /// Scales a value from an 32 bit to - /// an 8 bit equivalent. + /// Scales a value from a 32 bit to an + /// 8 bit equivalent. /// /// The 32 bit component value. /// The value. [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static byte From32BitTo8Bit(uint component) => (byte)(component >> 24); + public static byte From32BitTo8Bit(uint component) => + + // To scale to 8 bits from a 32-bit value V the required value is: + // + // (V * 255) / 4294967295 + // + // Since: + // + // 4294967295 = 255 * 16843009 + // + // this reduces exactly to: + // + // V / 16843009 + // + // To round to nearest using integer arithmetic we add half the divisor + // before dividing: + // + // (V + 16843009 / 2) / 16843009 + // + // where: + // + // 16843009 / 2 = 8421504.5 + // + // Using 8421504 ensures correct round-to-nearest behaviour: + // + // 8421504 -> 0 + // 8421505 -> 1 + // + // The addition must be performed in 64-bit to avoid overflow for large + // input values (for example uint.MaxValue). + // + // Final exact integer implementation: + // + // ((ulong)V + 8421504) / 16843009 + (byte)((component + 8421504UL) / 16843009UL); /// /// Scales a value from an 8 bit to diff --git a/src/ImageSharp/PixelFormats/PixelImplementations/Rgb96.cs b/src/ImageSharp/PixelFormats/PixelImplementations/Rgb96.cs index 5f61d5778b..12275061dd 100644 --- a/src/ImageSharp/PixelFormats/PixelImplementations/Rgb96.cs +++ b/src/ImageSharp/PixelFormats/PixelImplementations/Rgb96.cs @@ -19,7 +19,12 @@ public partial struct Rgb96 : IPixel, IEquatable { private const float InvMax = 1.0f / uint.MaxValue; - private const float Max = uint.MaxValue; + // Use double here because at this magnitude a float cannot represent all 32-bit + // integer values exactly. A float only has 24 bits of precision, so around + // uint.MaxValue it can only represent multiples of 256 and will round + // 4294967295 up to 4294967296. Double has 53 bits of precision and can + // represent all uint values exactly, avoiding precision loss before scaling. + private const double Max = uint.MaxValue; /// /// Gets the red component. @@ -96,8 +101,11 @@ public partial struct Rgb96 : IPixel, IEquatable [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Rgb96 FromVector4(Vector4 source) { - source = Numerics.Clamp(source, Vector4.Zero, Vector4.One) * Max; - return new Rgb96((uint)MathF.Round(source.X), (uint)MathF.Round(source.Y), (uint)MathF.Round(source.Z)); + source = Numerics.Clamp(source, Vector4.Zero, Vector4.One); + return new Rgb96( + (uint)Math.Round(source.X * Max), + (uint)Math.Round(source.Y * Max), + (uint)Math.Round(source.Z * Max)); } /// diff --git a/src/ImageSharp/PixelFormats/PixelImplementations/Rgba128.cs b/src/ImageSharp/PixelFormats/PixelImplementations/Rgba128.cs index 77934eac81..7fc62b7ffb 100644 --- a/src/ImageSharp/PixelFormats/PixelImplementations/Rgba128.cs +++ b/src/ImageSharp/PixelFormats/PixelImplementations/Rgba128.cs @@ -19,7 +19,12 @@ public partial struct Rgba128 : IPixel, IEquatable { private const float InvMax = 1.0f / uint.MaxValue; - private const float Max = uint.MaxValue; + // Use double here because at this magnitude a float cannot represent all 32-bit + // integer values exactly. A float only has 24 bits of precision, so around + // uint.MaxValue it can only represent multiples of 256 and will round + // 4294967295 up to 4294967296. Double has 53 bits of precision and can + // represent all uint values exactly, avoiding precision loss before scaling. + private const double Max = uint.MaxValue; /// /// Gets the red component. @@ -96,8 +101,12 @@ public partial struct Rgba128 : IPixel, IEquatable /// public static Rgba128 FromVector4(Vector4 source) { - source = Numerics.Clamp(source, Vector4.Zero, Vector4.One) * Max; - return new Rgba128((uint)MathF.Round(source.X), (uint)MathF.Round(source.Y), (uint)MathF.Round(source.Z), (uint)MathF.Round(source.W)); + source = Numerics.Clamp(source, Vector4.Zero, Vector4.One); + return new Rgba128( + (uint)Math.Round(source.X * Max), + (uint)Math.Round(source.Y * Max), + (uint)Math.Round(source.Z * Max), + (uint)Math.Round(source.W * Max)); } ///