Support High Precision TPixel formats via Color

[JimBobSquarePants]

Prerequisites

• I have written a descriptive pull-request title
• I have verified that there are no overlapping pull-requests open
• I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
• I have provided test coverage for my change (where applicable)

Description

Switches out the Rgba64 color backing type for RgbaVector in Color. This allows supporting all the pixel formats in the library without loss of precision.

A new Color.FromPixel<TPixel>(TPixel pixel) method has been provided to support this.

See SixLabors/ImageSharp.Drawing#165

[codecov]

Codecov Report

Merging #1797 (034062d) into master (d021222) will increase coverage by 0.00%.
The diff coverage is 96.87%.

@@           Coverage Diff           @@
##           master    #1797   +/-   ##
=======================================
  Coverage   87.10%   87.10%           
=======================================
  Files         936      936           
  Lines       47832    47861   +29     
  Branches     6009     6009           
=======================================
+ Hits        41662    41690   +28     
+ Misses       5178     5177    -1     
- Partials      992      994    +2     

Flag	Coverage Δ
unittests	87.10% <96.87%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/ImageSharp/Color/Color.cs	95.65% <95.00%> (-0.10%)	⬇️
src/ImageSharp/Color/Color.Conversions.cs	95.55% <97.72%> (+8.88%)	⬆️
...ImageSharp/Formats/Webp/BitReader/Vp8LBitReader.cs	86.95% <0.00%> (-8.70%)	⬇️
...ImageSharp/Formats/Webp/Lossless/Vp8LBitEntropy.cs	100.00% <0.00%> (+1.19%)	⬆️
...rp/PixelFormats/PixelImplementations/RgbaVector.cs	93.18% <0.00%> (+2.27%)	⬆️
...ions/PixelOperations/RgbaVector.PixelOperations.cs	100.00% <0.00%> (+10.00%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d021222...034062d. Read the comment docs.

[antonfirsov]

Let's bring this back to design discussion, and re-evaluate our options:

Replace Rgba64 with Vector4

• Higher precision, covers (somewhat) more cases
• We think it might solve the user wish from SixLabors/ImageSharp.Drawing#165, but no confirmation yet
• Keeps things simple
• Increases Color size from 64 bits to 128 bits
• Floating point flakiness may lead to conversion inaccuracies

Add optional boxed IPixel

• Definitely solves all potential wishes for arbitrary Colors
• Increased complexity / bloat
• Increases Color size from 64 bits to 128 bits
• Always accurate as soon as backing pixel implementation is accurate

I don't see that Vector4 is a clear winner. At least we should see more input from @calrsom. (Use case, actual pixel type, how/why can be Vector4 good enough.)

If there is no consensus, I'd prefer to keep things as they are now.

[JimBobSquarePants]

@antonfirsov one thing I’m struggling with is how bulk operations would work with the boxed value. From a safety perspective you’d have to check each item in the input Color span to ensure they conform to the correct type.

[tocsoft]

@antonfirsov one thing I’m struggling with is how bulk operations would work with the boxed value. From a safety perspective you’d have to check each item in the input Color span to ensure they conform to the correct type.

We don't/shouldn't be using Color in any bulk operations, we always translate to a real IPixel first before any sort of bulk operations happen, also we should never be convert between a Color to IPixel in any sort of loop.

Also do we even have any Span<Color> usage? I didn't think we did. Color isn't an IPixel after all, its just there to provide a way of providing colors in an IPixel agnostic format until we have access to the generic type and as soon as we have the access to the generic TPixel type we immediately convert it over.

[antonfirsov]

I agree with @tocsoft, we don't do (and should never do) bulk conversion of Color on hot path.

Also do we even have any Span<Color> usage?

When we initialize a pixel-specific operation executor, that uses a series of colors, we need to convert Span<Color> to TPixel[] like in case of PaletteQuantizer:

ImageSharp/src/ImageSharp/Processing/Processors/Quantization/PaletteQuantizer.cs

Lines 56 to 60 in 4638965

	int length = Math.Min(this.colorPalette.Length, options.MaxColors);
	var palette = new TPixel[length];
	Color.ToPixel(configuration, this.colorPalette.Span, palette.AsSpan());
	return new PaletteQuantizer<TPixel>(configuration, options, palette);

This happens on a very small batch of data, so the cost is negligible compared to the quantization.

[JimBobSquarePants]

We already expose a bulk method on the Color type.

ImageSharp/src/ImageSharp/Color/Color.cs

Lines 229 to 245 in 527e0fb

	/// <summary>
	/// Bulk converts a span of <see cref="Color"/> to a span of a specified <see cref="IPixel{TSelf}"/> type.
	/// </summary>
	/// <typeparam name="TPixel">The pixel type to convert to.</typeparam>
	/// <param name="configuration">The configuration.</param>
	/// <param name="source">The source color span.</param>
	/// <param name="destination">The destination pixel span.</param>
	[MethodImpl(InliningOptions.ShortMethod)]
	public static void ToPixel<TPixel>(
	Configuration configuration,
	ReadOnlySpan<Color> source,
	Span<TPixel> destination)
	where TPixel : unmanaged, IPixel<TPixel>
	{
	ReadOnlySpan<Rgba64> rgba64Span = MemoryMarshal.Cast<Color, Rgba64>(source);
	PixelOperations<TPixel>.Instance.FromRgba64(configuration, rgba64Span, destination);
	}

[antonfirsov]

@JimBobSquarePants yes, we use that method in PaletteQuantizer. It exists because of an effort to optimize as much as possible, but that doesn't mean the usages are on a hot path, and the conversion efficiency has measurable impact.

[JimBobSquarePants]

I’m just pointing out that we expose is and as such we’d have to lose any optimisation to use the boxes value. We wouldn’t use it on a hot path nor mix color precision but who knows what madness lies beyond our walls!

I’m happy to experiment and convert it to a loop with check but just wanted everyone to be aware.

EDIT. Doing some experimentation here and it's getting messy. Color implements IEquatable<Color>, IPixel doesn't implement any equatable interfaces basically making it impossible to determine whether two high precision instances are equal. The best we can do is convert both to Vector4 and compare.

Have created #1801 for comparison

[JimBobSquarePants]

Closed in favor of #1801