Ensure PixelOperations.To(TPixel) uses scaling. #2050
Conversation
| @@ -94,7 +94,7 @@ public abstract partial class ExifTag | |||
| /// <summary> | |||
| /// Gets the MDFileUnits exif tag. | |||
| /// </summary> | |||
| public static ExifTag<string> MDFileUnits => new ExifTag<string>(ExifTagValue.MDFileUnits); | |||
| public static ExifTag<string> MDFileUnits { get; } = new ExifTag<string>(ExifTagValue.MDFileUnits); | |||
There was a problem hiding this comment.
Pushed this by accident but it was something I found the other day when looking at the EXIF tags.
| <Include Path="..\shared-infrastructure\sixlabors.tests.ruleset" Action="Default" /> | ||
| <Rules AnalyzerId="StyleCop.Analyzers" RuleNamespace="StyleCop.Analyzers"> | ||
| <Rule Id="SA1313" Action="None" /> |
There was a problem hiding this comment.
Allows us to replace "dummy" with discard "_"
|
|
||
| PixelConverterTests.ReferenceImplementations.To<TPixel, TDestPixel>(this.Configuration, source, expected); | ||
|
|
||
| TestOperation(source, expected, (s, d) => this.Operations.To(this.Configuration, s, d.GetSpan())); | ||
| TestOperation(source, expected, (s, d) => this.Operations.To(this.Configuration, s, d.GetSpan()), false); |
There was a problem hiding this comment.
We need approximate comparison here now we've added all the types. Comparison is still very accurate.
| public void Issue2048() | ||
| { | ||
| // https://github.com/SixLabors/ImageSharp/issues/2048 | ||
| RgbaVector green = Color.Green.ToPixel<RgbaVector>(); |
There was a problem hiding this comment.
This is actually covered in the PixelOperation tests but I like to be explicit for serious bugs.
| @@ -16,6 +17,11 @@ public TestPixel() | |||
|
|
|||
| public TestPixel(float red, float green, float blue, float alpha) | |||
| { | |||
| Guard.MustBeBetweenOrEqualTo(red, 0F, 1F, nameof(red)); | |||
There was a problem hiding this comment.
This was unbounded when it shouldn't be.
| @@ -33,14 +39,11 @@ public TestPixel(float red, float green, float blue, float alpha) | |||
| public TPixel AsPixel() | |||
| { | |||
| var pix = default(TPixel); | |||
| pix.FromVector4(new System.Numerics.Vector4(this.Red, this.Green, this.Blue, this.Alpha)); | |||
| pix.FromScaledVector4(new Vector4(this.Red, this.Green, this.Blue, this.Alpha)); | |||
There was a problem hiding this comment.
Best to be explicit despite guard.
| @@ -21,7 +21,7 @@ public partial struct RgbaVector | |||
| internal class PixelOperations : PixelOperations<RgbaVector> | |||
| { | |||
| private static readonly Lazy<PixelTypeInfo> LazyInfo = | |||
| new Lazy<PixelTypeInfo>(() => PixelTypeInfo.Create<RgbaVector>(PixelAlphaRepresentation.Unassociated), true); | |||
| new(() => PixelTypeInfo.Create<RgbaVector>(PixelAlphaRepresentation.Unassociated), true); | |||
There was a problem hiding this comment.
I have to say I am not a fan of this syntax for object creation... I find it a lot harder to read. especially in cases like this:
// in some piece of unchanged code
var list = new List<ObjType>();
.
.
.
// lots of lines later
.
.
.
// in the middle of a PR change block, there is no context available while review what the hell this thing is.
list.Add(new(arg1, "arg2", 34)
{
OptionalProperty = bar
});not going to block any PRs etc over this just wanted to comment on the fact I feel it drastically harms readability in most cases... the only exception I can kind of agree on is property initialization where var is not allowed. Or possibly collection initializers where the Type of clear in the collection type.
There was a problem hiding this comment.
Yeah, definitely something to be aware of. I really like it in some scenarios though
Prerequisites
Description
Fixes #2048