From 9b9d77f216fac63efd23123ddbad03b08237cd69 Mon Sep 17 00:00:00 2001 From: Thomas Lopez Date: Fri, 11 Sep 2026 16:35:27 -0400 Subject: [PATCH] [UUM-148935] Make VectorHash quantization runtime-independent VectorHash.HashFloat hashed a float by casting it to ulong. ECMA-335 leaves conv.u8 on an out-of-range or NaN float unspecified, so each JIT emits whatever the underlying hardware instruction yields: Mono on x64 wraps around, CoreCLR (and Mono on Arm64) saturate. Saturation clamps every negative product to 0, causing hash collisions and equality/hashcode mismatches that are especially visible on CoreCLR and on macOS13 Arm64. HashFloat is replaced by VectorHash.RoundToInt, which maps NaN to 0, clamps explicitly instead of relying on the cast, and rounds half to even. IntVec2/IntVec3/IntVec4.round now call the same function instead of duplicating the quantization via Convert.ToInt32, which rounded differently than HashFloat truncated and caused GetHashCode to disagree with Equals for some coincident positions. Ported from unity/unity PR 123982, which fixed this in the ShadowPackages vendored snapshot of this package; this is the upstream fix that needs to land here so the next ShadowPackages refresh doesn't silently revert it. --- CHANGELOG.md | 1 + Runtime/Core/IntVec2.cs | 2 +- Runtime/Core/IntVec3.cs | 2 +- Runtime/Core/IntVec4.cs | 2 +- Runtime/Core/VectorHash.cs | 42 +++++++++++++++------- Tests/Runtime/Type/VertexTests.cs | 60 +++++++++++++++---------------- 6 files changed, 62 insertions(+), 47 deletions(-) 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() {