diff --git a/CHANGELOG.md b/CHANGELOG.md index dd91fb38b..7070f9385 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed - [UUM-149791] Fixed the First Vertex pivot ending up offset from the shape after resizing an existing shape or shift-duplicating one with the Shape tool. +- [UUM-148935] Fixed `VectorHash`/`IntVec2`/`IntVec3`/`IntVec4` producing different hash codes on different runtimes and architectures (e.g. Mono vs CoreCLR), and fixed a `GetHashCode` contract violation where positions considered equal by `IntVec3.Equals` could still hash differently, silently breaking vertex welding. - [UUM-150702] Fixed the ProBuilderDefault and Checker materials referencing a stale shader ID, causing ProBuilder meshes to render magenta when a scene was opened. - [UUM-148237] Fixed the "Lightmap UVs Settings" foldout in ProBuilder Preferences not opening when clicking its title. - [UUM-133528] Fixed an issue where using some UV Editor actions would clear a mesh's Lightmap UVs without regenerating them. diff --git a/Runtime/Core/IntVec2.cs b/Runtime/Core/IntVec2.cs index 4d93b224b..3e8a6e766 100644 --- a/Runtime/Core/IntVec2.cs +++ b/Runtime/Core/IntVec2.cs @@ -57,7 +57,7 @@ public override int GetHashCode() private static int round(float v) { - return System.Convert.ToInt32(v * VectorHash.FltCompareResolution); + return VectorHash.RoundToInt(v); } public static implicit operator Vector2(IntVec2 p) diff --git a/Runtime/Core/IntVec3.cs b/Runtime/Core/IntVec3.cs index 49fb12be8..324beb72e 100644 --- a/Runtime/Core/IntVec3.cs +++ b/Runtime/Core/IntVec3.cs @@ -60,7 +60,7 @@ public override int GetHashCode() private static int round(float v) { - return System.Convert.ToInt32(v * VectorHash.FltCompareResolution); + return VectorHash.RoundToInt(v); } public static implicit operator Vector3(IntVec3 p) diff --git a/Runtime/Core/IntVec4.cs b/Runtime/Core/IntVec4.cs index 69ec38960..e2152da77 100644 --- a/Runtime/Core/IntVec4.cs +++ b/Runtime/Core/IntVec4.cs @@ -63,7 +63,7 @@ public override int GetHashCode() private static int round(float v) { - return System.Convert.ToInt32(v * VectorHash.FltCompareResolution); + return VectorHash.RoundToInt(v); } public static implicit operator Vector4(IntVec4 p) diff --git a/Runtime/Core/VectorHash.cs b/Runtime/Core/VectorHash.cs index 03246d360..40f9e486d 100644 --- a/Runtime/Core/VectorHash.cs +++ b/Runtime/Core/VectorHash.cs @@ -11,10 +11,28 @@ static class VectorHash { public const float FltCompareResolution = 1000f; - static int HashFloat(float f) + /// + /// Quantize a float to FltCompareResolution. Shared by IntVec2/IntVec3/IntVec4 equality and + /// by the hash functions below, so that values which compare equal always hash equally. + /// + internal static int RoundToInt(float f) { - ulong u = (ulong)(f * FltCompareResolution); - return (int)(u % int.MaxValue); + if (float.IsNaN(f)) + return 0; + + float scaled = f * FltCompareResolution; + + // Casting an out-of-range float to an integer type is unspecified in ECMA-335 - Mono on + // x64 wraps around while CoreCLR and Mono on Arm64 saturate, which also collapses every + // negative component to 0 - so clamp explicitly (UUM-148935, UUM-111993). + if (scaled <= int.MinValue) + return int.MinValue; + + if (scaled >= int.MaxValue) + return int.MaxValue; + + // System.Math, not UnityEngine.ProBuilder.Math + return (int)System.Math.Round(scaled, MidpointRounding.ToEven); } /// @@ -29,8 +47,8 @@ public static int GetHashCode(Vector2 v) unchecked { - hash = hash * 29 + HashFloat(v.x); - hash = hash * 29 + HashFloat(v.y); + hash = hash * 29 + RoundToInt(v.x); + hash = hash * 29 + RoundToInt(v.y); } return hash; @@ -48,9 +66,9 @@ public static int GetHashCode(Vector3 v) unchecked { - hash = hash * 29 + HashFloat(v.x); - hash = hash * 29 + HashFloat(v.y); - hash = hash * 29 + HashFloat(v.z); + hash = hash * 29 + RoundToInt(v.x); + hash = hash * 29 + RoundToInt(v.y); + hash = hash * 29 + RoundToInt(v.z); } return hash; @@ -68,10 +86,10 @@ public static int GetHashCode(Vector4 v) unchecked { - hash = hash * 29 + HashFloat(v.x); - hash = hash * 29 + HashFloat(v.y); - hash = hash * 29 + HashFloat(v.z); - hash = hash * 29 + HashFloat(v.w); + hash = hash * 29 + RoundToInt(v.x); + hash = hash * 29 + RoundToInt(v.y); + hash = hash * 29 + RoundToInt(v.z); + hash = hash * 29 + RoundToInt(v.w); } return hash; diff --git a/Tests/Runtime/Type/VertexTests.cs b/Tests/Runtime/Type/VertexTests.cs index 6b7b06ace..210115d45 100644 --- a/Tests/Runtime/Type/VertexTests.cs +++ b/Tests/Runtime/Type/VertexTests.cs @@ -4,9 +4,6 @@ using System.Linq; using System; using UnityEngine.ProBuilder; -#if UNITY_6000_7_OR_NEWER -using UnityEngine.TestTools; -#endif static class TestHashUtility { @@ -44,6 +41,11 @@ static float RandFlt() return UnityEngine.Random.Range(0f, 100f) * .001f; } + static float RandJitter() + { + return UnityEngine.Random.Range(-.001f, .001f); + } + static Vertex RandVertex() { Vertex v = new Vertex(); @@ -59,54 +61,33 @@ static Vertex RandVertex() } [Test] -#if UNITY_6000_7_OR_NEWER - [UnityCoreClrExplicitDisabled("https://jira.unity3d.com/browse/UUM-148935", "ProBuilder IntVec3/VectorHash float hashing produces different values on CoreCLR")] -#endif public static void TestHashCollisions_IVEC3() { -#if UNITY_EDITOR_OSX - if (System.Runtime.InteropServices.RuntimeInformation.OSArchitecture == System.Runtime.InteropServices.Architecture.Arm64) - Assert.Ignore("Fails on macOS13 Arm64 https://jira.unity3d.com/browse/UUM-111993"); -#endif - IntVec3[] ivec3 = ArrayUtility.Fill(TestIterationCount, (i) => { return (IntVec3)RandVec3(); }); Assert.IsTrue(TestHashUtility.GetCollisionsCount(ivec3) < TestIterationCount * .05f); } [Test] -#if UNITY_6000_7_OR_NEWER - [UnityCoreClrExplicitDisabled("https://jira.unity3d.com/browse/UUM-148935", "ProBuilder IntVec3/VectorHash float hashing produces different values on CoreCLR")] -#endif public static void TestVectorHashOverflow() { -#if UNITY_EDITOR_OSX - if (System.Runtime.InteropServices.RuntimeInformation.OSArchitecture == System.Runtime.InteropServices.Architecture.Arm64) - Assert.Ignore("Fails on macOS13 Arm64 https://jira.unity3d.com/browse/UUM-111993"); -#endif - Vector3 over = new Vector3(((float)int.MaxValue) + 10f, 0f, 0f); Vector3 under = new Vector3(((float)-int.MaxValue) - 10f, 0f, 0f); Vector3 inf = new Vector3(Mathf.Infinity, 0f, 0f); + Vector3 negInf = new Vector3(Mathf.NegativeInfinity, 0f, 0f); Vector3 nan = new Vector3(float.NaN, 0f, 0f); - // mostly checking that GetHashCode doesn't throw an error when converting bad float values - Assert.AreEqual(VectorHash.GetHashCode(over), 1499503, "Over"); - Assert.AreEqual(VectorHash.GetHashCode(under), 2147303674, "Under"); - Assert.AreNotEqual(VectorHash.GetHashCode(inf), 0, "Inf"); - Assert.AreNotEqual(VectorHash.GetHashCode(nan), 0, "NaN"); + // Out-of-range components saturate and NaN quantizes to zero, so these hash codes are + // identical on every runtime and architecture. See UUM-148935. + Assert.AreEqual(-2146825986, VectorHash.GetHashCode(over), "Over"); + Assert.AreEqual(-2146825145, VectorHash.GetHashCode(under), "Under"); + Assert.AreEqual(-2146825986, VectorHash.GetHashCode(inf), "Inf"); + Assert.AreEqual(-2146825145, VectorHash.GetHashCode(negInf), "NegInf"); + Assert.AreEqual(VectorHash.GetHashCode(Vector3.zero), VectorHash.GetHashCode(nan), "NaN"); } [Test] -#if UNITY_6000_7_OR_NEWER - [UnityCoreClrExplicitDisabled("https://jira.unity3d.com/browse/UUM-148935", "ProBuilder IntVec3/VectorHash float hashing produces different values on CoreCLR")] -#endif public static void TestComparison_IVEC3() { -#if UNITY_EDITOR_OSX - if (System.Runtime.InteropServices.RuntimeInformation.OSArchitecture == System.Runtime.InteropServices.Architecture.Arm64) - Assert.Ignore("Fails on macOS13 Arm64 https://jira.unity3d.com/browse/UUM-111993"); -#endif - IntVec3 a = (IntVec3)RandVec3(); IntVec3 b = (IntVec3)(a.value * 2.3f); IntVec3 c = (IntVec3) new Vector3(a.x, a.y + .001f, a.z); @@ -123,6 +104,21 @@ public static void TestComparison_IVEC3() Assert.AreEqual(13, arr.Distinct().Count()); } + [Test] + public static void TestEqualIntVec3SharesHashCode() + { + // IntVec3.Equals and VectorHash must quantize identically, otherwise positions that + // ProBuilder considers coincident land in different dictionary buckets and never weld. + for (int i = 0; i < TestIterationCount; ++i) + { + IntVec3 a = (IntVec3)RandVec3(); + IntVec3 b = (IntVec3)(a.value + new Vector3(RandJitter(), RandJitter(), RandJitter())); + + if (a == b) + Assert.AreEqual(a.GetHashCode(), b.GetHashCode(), a + " == " + b); + } + } + [Test] public static void TestComparison_VERTEX() {