2D test failing due to change in GetHashCode in Core 3

[grubioe]

Copied from VSO ID: 803447

Test:

Name="Color API Test" Priority="1" Area="2D" SubArea="API Tests"

Fails due to a change in GetHashCode from corefx. What the test code is doing is generating 2 different colors and verifying that the resulting hash codes are not equal, the hashcode is calculated from System.Windows.Media.Color by overriding GetHashCode:

wpf/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Media/Color.cs

Lines 214 to 220 in 9c5dd1a

	///<summary>
	/// GetHashCode
	///</summary>
	public override int GetHashCode()
	{
	return this.scRgbColor.GetHashCode(); //^this.context.GetHashCode();
	}

which essentially only returns the hashcode from its internal struct which is defined as:

wpf/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Media/Color.cs

Lines 1158 to 1161 in 9c5dd1a

	private struct MILColorF // this structure is the "milrendertypes.h" structure and should be identical for performance
	{
	public float a, r, g, b;
	};

in Framework different values for a,r,g,b would result in a different HashCode in Core it seems only the first value is considered for HashCode calculation, our test has the same first value so the hash code is the same.

As per corefx recommendation (https://github.com/dotnet/corefx/issues/35613) we should override and implement a correct HashCode mechanism for our 4 floats in MILColorF, ideally mirroring what Framework used to do

[weltkante]

Was it verified in your original ticket source that this isn't actually a bug in .NET Core? Otherweise I think it'd be worth opening an issue with coreclr to verify that this behavior is intended.

I don't mind proper GetHashCode implementations in WPF but if thats an unintended bug the poor hashcode quality will likely impact users who port their applications as well.

[weltkante]

@grubioe @ojhad actually I cannot reproduce this, I took above definition of MILColor and created instances which differ only in a single component. Regardless which component differs, the hash code always differs too for me.

Debug.WriteLine($"Hash = {new MILColor { a = 0xFF, r = 0xAA, g = 0xBB, b = 0xCC }.GetHashCode()}");
Debug.WriteLine($"Hash = {new MILColor { a = 0x00, r = 0xAA, g = 0xBB, b = 0xCC }.GetHashCode()}");
Debug.WriteLine($"Hash = {new MILColor { a = 0xFF, r = 0x00, g = 0xBB, b = 0xCC }.GetHashCode()}");
Debug.WriteLine($"Hash = {new MILColor { a = 0xFF, r = 0xAA, g = 0x00, b = 0xCC }.GetHashCode()}");
Debug.WriteLine($"Hash = {new MILColor { a = 0xFF, r = 0xAA, g = 0xBB, b = 0x00 }.GetHashCode()}");

// output
//Hash = -669667421
//Hash = -669667492
//Hash = -669710941
//Hash = -659640413
//Hash = 336965539

[grubioe]

Thanks for checking @weltkante

@miguep - is this still valid? It was logged back in February and seems that it no longer repros.

[miguep]

the issue can still be reproduced like this:

            System.Windows.Media.Color ColorA = System.Windows.Media.Color.FromScRgb(1.0f, 0.3f, 0.4f, 1.0f);
            System.Windows.Media.Color ColorB = System.Windows.Media.Color.FromScRgb(0.99999f, 0.3f, 0.4f, 1.0f);
            System.Windows.Media.Color ColorC = System.Windows.Media.Color.FromScRgb(1f, 0.2999f, 0.4f, 1.0f);


            int hashA = ColorA.GetHashCode();
            int hashB = ColorB.GetHashCode();
            int hashC = ColorC.GetHashCode();

In .NET Framework all three hashcodes are different, but in .NET Core, A and C are the same.

an issue against corefx was opened https://github.com/dotnet/corefx/issues/35613, and closed as a by-design behavior, with the recommendation that we implement GetHashCode to our requirements, we'll use this issue to track that work, since we would like Color.GetHashCode to behave as it did in .NET Framework

[weltkante]

I see, GetHashCode actually doesn't use MILColor from the original issue description, but MILColorF, so thats the reason why I couldn't reproduce it, as it only happens with float/double members. Thanks for verifying that its intentional.

[miguep]

@weltkante good observation, the description was actually misleading, I've updated it for clarity, thank you