Fix and cleanup GetHashCode and Equals

[ronaldbarendse]

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 matches the existing coding patterns and practise as demonstrated in the repository. These follow strict Stylecop rules 👮.
• I have provided test coverage for my change (where applicable)

Description

This fixes some possible NullReferenceExceptions in the Equals implementations and uses ValueTuple to generate the hash codes, as that's a lot easier, cleaner and heavily optimized:

• https://stackoverflow.com/a/4630550
• https://intellitect.com/overidingobjectusingtuple/
• https://montemagno.com/optimizing-c-struct-equality-with-iequatable/

[JimBobSquarePants]

ValueTuple relies on the default comparer for each property to compare equality. Is that enough, I'd be very careful introducing a patter that can fail in certain cases.

	var a = new List<int> { 1, 2, 3 };
	var b = new List<int> { 1, 2, 3 };

	EqualityComparer<List<int>>.Default.Equals(a, b); // False

[ronaldbarendse]

I can confirm using ValueTuple for equality or hash code generation of collections doesn't work correctly indeed. Even the current GetHashCode implementations return different hash codes when the Equals method returns true, because it uses SequenceEqual to compare the collections (and not the default comparer), but the collections hash code:

var a = new List<int> { 1, 2, 3 };
var b = new List<int> { 1, 2, 3 };
a.Equals(b); // False
a.SequenceEqual(b); // True
a.GetHashCode() == b.GetHashCode(); // False, as different instances return different hash codes, even if the items are the same

Equal items should return the same hash code (although the reverse isn't always true because of hash collisions), which isn't always the case in the current codebase.

Microsoft has added a HashCode helper class in .NET Standard 2.1 (.NET Core 3) and released a NuGet package for .NET Standard 2.0 (.NET Framework 4.6.1) to make this a lot easier, but upgrading the target framework from .NET Framework 4.5.2 to 4.6.1 for this isn't justified.

A quick fix would be to ensure equality is checked individually per property (with support for null values and collections) and just skip the collections in GetHashCode (using the ValueTuple construct).

[JimBobSquarePants]

Microsoft has added a HashCode helper class in .NET Standard 2.1 (.NET Core 3) and released a NuGet package for .NET Standard 2.0 (.NET Framework 4.6.1) to make this a lot easier, but upgrading the target framework from .NET Framework 4.5.2 to 4.6.1 for this isn't justified.

We copied the source for HashCode into the the SixLabors codebase for unsupported frameworks and use type forwarding to avoid conflict. Perhaps we can simply do the same here.

https://github.com/SixLabors/SharedInfrastructure/blob/master/src/SharedInfrastructure/HashCode.cs

[ronaldbarendse]

I've checked all GetHashCode methods and ResizeLayer is the only class that has collections that could be included. Not including all properties shouldn't cause major problems, so I'm fine with the possibility of more hash collisions for non-equal objects.

I've also added some additional comments for explanation and to review 👍

[ronaldbarendse]

I've updated the test to handle the fixed ColorMatrix comparison, as the equality was previously ensured by reusing the same instance (from the ColorMatrixes helper class). I'm not sure what the performance difference will be, but the matrices can now also be initialized within the matrix filters themselves.

@JimBobSquarePants If you can review whether the unresolved comments are okay, I think this is ready to get merged!

[ronaldbarendse]

Note that merging this after PR #796 will result in a conflict on ResizeLayer: make sure this is resolved with the new Equals and GetHashCode methods and that both use the new Center property:

public bool Equals(ResizeLayer other) => other != null
    && this.Size == other.Size
    && this.MaxSize == other.MaxSize
    && (this.RestrictedSizes == null || other.RestrictedSizes == null ? this.RestrictedSizes == other.RestrictedSizes : this.RestrictedSizes.SequenceEqual(other.RestrictedSizes))
    && this.ResizeMode == other.ResizeMode
    && this.AnchorPosition == other.AnchorPosition
    && this.Upscale == other.Upscale
    && this.Center == other.Center
    && this.AnchorPoint == other.AnchorPoint;

public override int GetHashCode() => (this.Size, this.MaxSize, this.ResizeMode, this.AnchorPosition, this.Upscale, this.Center, this.AnchorPoint).GetHashCode();

[JimBobSquarePants]

Nice work. 👍