From bb19f1b9b18e24919adcd945bca02b1fdf7a17d3 Mon Sep 17 00:00:00 2001 From: ds5678 <49847914+ds5678@users.noreply.github.com> Date: Sat, 3 Feb 2024 15:13:27 -0500 Subject: [PATCH] BoneWeight Improvements * Fix bug where there were extra skin weights extracted. The extra values were all 0 because there was never any data to fill them. * Make compiler warn for incorrect use of Pure methods * Refactor BoneWeight4 to use InlineArray * Support bit packing ReadOnlySpan * Support setting the compressed mesh weights --- Source/.editorconfig | 3 +- .../BoneWeight4Tests.cs | 18 ++ Source/AssetRipper.Numerics/BoneWeight4.cs | 218 ++++++++++++++---- .../CompressedMeshExtensions.cs | 59 ++++- .../PackedIntVectorExtensions.cs | 6 + .../VertexDataExtensions.cs | 4 +- .../AssetRipper.Tests/CompressedMeshTests.cs | 75 +++++- 7 files changed, 326 insertions(+), 57 deletions(-) create mode 100644 Source/AssetRipper.Numerics.Tests/BoneWeight4Tests.cs diff --git a/Source/.editorconfig b/Source/.editorconfig index 4055f2fb4..47fffdc8c 100644 --- a/Source/.editorconfig +++ b/Source/.editorconfig @@ -255,4 +255,5 @@ dotnet_style_qualification_for_field = false:silent dotnet_style_qualification_for_property = false:silent dotnet_style_qualification_for_method = false:silent dotnet_style_qualification_for_event = false:silent -dotnet_style_prefer_collection_expression = true:suggestion \ No newline at end of file +dotnet_style_prefer_collection_expression = when_types_exactly_match:suggestion +dotnet_diagnostic.CA1806.severity = warning \ No newline at end of file diff --git a/Source/AssetRipper.Numerics.Tests/BoneWeight4Tests.cs b/Source/AssetRipper.Numerics.Tests/BoneWeight4Tests.cs new file mode 100644 index 000000000..9016aa015 --- /dev/null +++ b/Source/AssetRipper.Numerics.Tests/BoneWeight4Tests.cs @@ -0,0 +1,18 @@ +namespace AssetRipper.Numerics.Tests; + +public class BoneWeight4Tests +{ + [Test] + public void DefaultCanBeNormalized() + { + BoneWeight4 boneWeight = new BoneWeight4().NormalizeWeights(); + Assert.Multiple(() => + { + Assert.That(boneWeight.Weight0, Is.EqualTo(0.25f)); + Assert.That(boneWeight.Weight1, Is.EqualTo(0.25f)); + Assert.That(boneWeight.Weight2, Is.EqualTo(0.25f)); + Assert.That(boneWeight.Weight3, Is.EqualTo(0.25f)); + Assert.That(boneWeight.Normalized); + }); + } +} diff --git a/Source/AssetRipper.Numerics/BoneWeight4.cs b/Source/AssetRipper.Numerics/BoneWeight4.cs index 400618386..fddf32368 100644 --- a/Source/AssetRipper.Numerics/BoneWeight4.cs +++ b/Source/AssetRipper.Numerics/BoneWeight4.cs @@ -1,55 +1,80 @@ using System.Diagnostics.Contracts; +using System.Runtime.CompilerServices; namespace AssetRipper.Numerics { - public record struct BoneWeight4( - float Weight0, - float Weight1, - float Weight2, - float Weight3, - int Index0, - int Index1, - int Index2, - int Index3) + public record struct BoneWeight4 { + public const int Count = 4; + public WeightArray Weights; + public IndexArray Indices; + + public BoneWeight4(float weight0, float weight1, float weight2, float weight3, int index0, int index1, int index2, int index3) + : this(new WeightArray(weight0, weight1, weight2, weight3), new IndexArray(index0, index1, index2, index3)) + { + } + + public BoneWeight4(WeightArray weights, IndexArray indices) + { + Weights = weights; + Indices = indices; + } + + public float Weight0 + { + readonly get => Weights[0]; + set => Weights[0] = value; + } + + public float Weight1 + { + readonly get => Weights[1]; + set => Weights[1] = value; + } + + public float Weight2 + { + readonly get => Weights[2]; + set => Weights[2] = value; + } + + public float Weight3 + { + readonly get => Weights[3]; + set => Weights[3] = value; + } + + public int Index0 + { + readonly get => Indices[0]; + set => Indices[0] = value; + } + + public int Index1 + { + readonly get => Indices[1]; + set => Indices[1] = value; + } + + public int Index2 + { + readonly get => Indices[2]; + set => Indices[2] = value; + } + + public int Index3 + { + readonly get => Indices[3]; + set => Indices[3] = value; + } + public readonly bool AnyWeightsNegative => Weight0 < 0f || Weight1 < 0f || Weight2 < 0f || Weight3 < 0f; public readonly float Sum => Weight0 + Weight1 + Weight2 + Weight3; - public void SetWeight(int index, float value) - { - switch (index) - { - case 0: - Weight0 = value; break; - case 1: - Weight1 = value; break; - case 2: - Weight2 = value; break; - case 3: - Weight3 = value; break; - default: - throw new ArgumentOutOfRangeException(nameof(index), value, null); - } - } - - public void SetIndex(int index, int value) - { - switch (index) - { - case 0: - Index0 = value; break; - case 1: - Index1 = value; break; - case 2: - Index2 = value; break; - case 3: - Index3 = value; break; - default: - throw new ArgumentOutOfRangeException(nameof(index), value, null); - } - } + public readonly bool Normalized => Sum == 1f; + [Pure] public readonly BoneWeight4 NormalizeWeights() { float sum = Sum; @@ -63,5 +88,114 @@ namespace AssetRipper.Numerics return new BoneWeight4(Weight0 * invSum, Weight1 * invSum, Weight2 * invSum, Weight3 * invSum, Index0, Index1, Index2, Index3); } } + + public override readonly string ToString() + { + return $"{nameof(BoneWeight4)}: {{ {nameof(Weights)} = {Weights}, {nameof(Indices)} = {Indices} }}"; + } + + [InlineArray(Count)] + public struct WeightArray : IEquatable + { + private float _element0; + + public WeightArray(float weight0, float weight1, float weight2, float weight3) + { + this[0] = weight0; + this[1] = weight1; + this[2] = weight2; + this[3] = weight3; + } + + public override readonly bool Equals(object? obj) + { + return obj is WeightArray array && Equals(array); + } + + public readonly bool Equals(WeightArray other) + { + return ((ReadOnlySpan)this).SequenceEqual(other); + } + + public override readonly int GetHashCode() + { + return HashCode.Combine(this[0], this[1], this[2], this[3]); + } + + public static bool operator ==(WeightArray left, WeightArray right) + { + return left.Equals(right); + } + + public static bool operator !=(WeightArray left, WeightArray right) + { + return !(left == right); + } + + public readonly void Deconstruct(out float weight0, out float weight1, out float weight2, out float weight3) + { + weight0 = this[0]; + weight1 = this[1]; + weight2 = this[2]; + weight3 = this[3]; + } + + public override readonly string ToString() + { + return $"[{this[0]}, {this[1]}, {this[2]}, {this[3]}]"; + } + } + + [InlineArray(Count)] + public struct IndexArray : IEquatable + { + private int _element0; + + public IndexArray(int index0, int index1, int index2, int index3) + { + this[0] = index0; + this[1] = index1; + this[2] = index2; + this[3] = index3; + } + + public override readonly bool Equals(object? obj) + { + return obj is IndexArray array && Equals(array); + } + + public readonly bool Equals(IndexArray other) + { + return ((ReadOnlySpan)this).SequenceEqual(other); + } + + public override readonly int GetHashCode() + { + return HashCode.Combine(this[0], this[1], this[2], this[3]); + } + + public static bool operator ==(IndexArray left, IndexArray right) + { + return left.Equals(right); + } + + public static bool operator !=(IndexArray left, IndexArray right) + { + return !(left == right); + } + + public readonly void Deconstruct(out int index0, out int index1, out int index2, out int index3) + { + index0 = this[0]; + index1 = this[1]; + index2 = this[2]; + index3 = this[3]; + } + + public override readonly string ToString() + { + return $"[{this[0]}, {this[1]}, {this[2]}, {this[3]}]"; + } + } } } diff --git a/Source/AssetRipper.SourceGenerated.Extensions/CompressedMeshExtensions.cs b/Source/AssetRipper.SourceGenerated.Extensions/CompressedMeshExtensions.cs index 10100f483..ebe61c069 100644 --- a/Source/AssetRipper.SourceGenerated.Extensions/CompressedMeshExtensions.cs +++ b/Source/AssetRipper.SourceGenerated.Extensions/CompressedMeshExtensions.cs @@ -212,7 +212,8 @@ namespace AssetRipper.SourceGenerated.Extensions int[] weights = compressedMesh.Weights.UnpackInts(); int[] boneIndices = compressedMesh.BoneIndices.UnpackInts(); - BoneWeight4[] skin = new BoneWeight4[compressedMesh.Weights.NumItems]; + //In theory, the array length should be exactly the same as the number of vertices, but it's better to be safe. + BoneWeight4[] skin = ArrayPool.Shared.Rent((int)compressedMesh.Weights.NumItems); int bonePos = 0; int boneIndexPos = 0; @@ -224,8 +225,8 @@ namespace AssetRipper.SourceGenerated.Extensions //read bone index and weight. { BoneWeight4 boneWeight = skin[bonePos]; - boneWeight.SetWeight(j, weights[i] / 31f); - boneWeight.SetIndex(j, boneIndices[boneIndexPos++]); + boneWeight.Weights[j] = weights[i] / 31f; + boneWeight.Indices[j] = boneIndices[boneIndexPos++]; skin[bonePos] = boneWeight; } j++; @@ -237,8 +238,8 @@ namespace AssetRipper.SourceGenerated.Extensions for (; j < 4; j++) { BoneWeight4 boneWeight = skin[bonePos]; - boneWeight.SetWeight(j, 0); - boneWeight.SetIndex(j, 0); + boneWeight.Weights[j] = 0; + boneWeight.Indices[j] = 0; skin[bonePos] = boneWeight; } bonePos++; @@ -250,8 +251,8 @@ namespace AssetRipper.SourceGenerated.Extensions else if (j == 3) { BoneWeight4 boneWeight = skin[bonePos]; - boneWeight.SetWeight(j, (31 - sum) / 31f); - boneWeight.SetIndex(j, boneIndices[boneIndexPos++]); + boneWeight.Weights[j] = (31 - sum) / 31f; + boneWeight.Indices[j] = boneIndices[boneIndexPos++]; skin[bonePos] = boneWeight; bonePos++; j = 0; @@ -259,18 +260,54 @@ namespace AssetRipper.SourceGenerated.Extensions } } - return skin; + BoneWeight4[] result = skin.AsSpan(0, bonePos).ToArray(); + ArrayPool.Shared.Return(skin); + return result; } - public static void SetWeights(this ICompressedMesh compressedMesh, ReadOnlySpan weights) + public static void SetWeights(this ICompressedMesh compressedMesh, ReadOnlySpan skin) { - if (weights.Length > 0) + if (skin.Length > 0) { - throw new NotImplementedException(); + int i_weight = 0; + int i_boneIndex = 0; + int[] weightList = ArrayPool.Shared.Rent(skin.Length * 3); + int[] boneIndexList = ArrayPool.Shared.Rent(skin.Length * 4); + + foreach (BoneWeight4 boneWeight in skin) + { + int sum = 0; + for (int j = 0; j < 4; j++) + { + int weight = (int)(boneWeight.Weights[j] * 31); + sum += weight; + if (j != 3) + { + //We never store the last weight because it can be calculated from the sum of the other weights. + weightList[i_weight] = weight; + i_weight++; + } + + boneIndexList[i_boneIndex] = boneWeight.Indices[j]; + i_boneIndex++; + + if (sum >= 31) + { + break; + } + } + } + + compressedMesh.Weights.PackInts(weightList.AsSpan(0, i_weight)); + compressedMesh.BoneIndices.PackInts(boneIndexList.AsSpan(0, i_boneIndex)); + + ArrayPool.Shared.Return(weightList); + ArrayPool.Shared.Return(boneIndexList); } else { compressedMesh.Weights.Reset(); + compressedMesh.BoneIndices.Reset(); } } diff --git a/Source/AssetRipper.SourceGenerated.Extensions/PackedIntVectorExtensions.cs b/Source/AssetRipper.SourceGenerated.Extensions/PackedIntVectorExtensions.cs index 216f9405a..16c276a83 100644 --- a/Source/AssetRipper.SourceGenerated.Extensions/PackedIntVectorExtensions.cs +++ b/Source/AssetRipper.SourceGenerated.Extensions/PackedIntVectorExtensions.cs @@ -1,4 +1,5 @@ using AssetRipper.SourceGenerated.Subclasses.PackedBitVector_Int32; +using System.Runtime.InteropServices; namespace AssetRipper.SourceGenerated.Extensions { @@ -13,6 +14,11 @@ namespace AssetRipper.SourceGenerated.Extensions instance.BitSize = source.BitSize; } + public static void PackInts(this PackedBitVector_Int32 packedVector, ReadOnlySpan data) + { + packedVector.PackUInts(MemoryMarshal.Cast(data)); + } + public static void PackUInts(this PackedBitVector_Int32 packedVector, ReadOnlySpan data) { uint maxDataValue = 0; diff --git a/Source/AssetRipper.SourceGenerated.Extensions/VertexDataExtensions.cs b/Source/AssetRipper.SourceGenerated.Extensions/VertexDataExtensions.cs index f6e570a88..3c61e7e64 100644 --- a/Source/AssetRipper.SourceGenerated.Extensions/VertexDataExtensions.cs +++ b/Source/AssetRipper.SourceGenerated.Extensions/VertexDataExtensions.cs @@ -221,7 +221,7 @@ namespace AssetRipper.SourceGenerated.Extensions BoneWeight4 boneWeight = skin[i]; for (int j = 0; j < dimension; j++) { - boneWeight.SetWeight(j, componentsFloatArray[i * dimension + j]); + boneWeight.Weights[j] = componentsFloatArray[i * dimension + j]; } skin[i] = boneWeight; } @@ -233,7 +233,7 @@ namespace AssetRipper.SourceGenerated.Extensions BoneWeight4 boneWeight = skin[i]; for (int j = 0; j < dimension; j++) { - boneWeight.SetIndex(j, componentsIntArray[i * dimension + j]); + boneWeight.Indices[j] = componentsIntArray[i * dimension + j]; } skin[i] = boneWeight; } diff --git a/Source/AssetRipper.Tests/CompressedMeshTests.cs b/Source/AssetRipper.Tests/CompressedMeshTests.cs index 79408fffc..dcc5830b3 100644 --- a/Source/AssetRipper.Tests/CompressedMeshTests.cs +++ b/Source/AssetRipper.Tests/CompressedMeshTests.cs @@ -16,6 +16,7 @@ namespace AssetRipper.Tests private static readonly uint[] integers = MakeUInts(24); private static readonly Vector2[] uv0 = MakeUV(VertexCount); private static readonly Vector2[] uv1 = MakeUV(VertexCount); + private static readonly BoneWeight4[] boneWeights = MakeBoneWeights(VertexCount); private static Vector3[] MakeUnitVectors(int count) { @@ -88,6 +89,44 @@ namespace AssetRipper.Tests return result; } + private static BoneWeight4[] MakeBoneWeights(int count) + { + BoneWeight4[] result = new BoneWeight4[count]; + for (int i = 0; i < count; i++) + { + BoneWeight4 item = new(); + const int MaxSum = 31; + const float MaxSumF = MaxSum; + int sum = 0; + for (int j = 0; j < 4; j++) + { + if (sum == MaxSum) + { + item.Weights[j] = 0; + item.Indices[j] = 0; + } + else + { + int weight; + if (j == 3) + { + weight = MaxSum - sum; + } + else + { + weight = random.Next(1, MaxSum + 1 - sum); + sum += weight; + } + item.Weights[j] = weight / MaxSumF; + item.Indices[j] = random.Next(0, count); + //This might not be the correct range for the index, but it doesn't matter for the tests. + } + } + result[i] = item; + } + return result; + } + [Test] public void VertexAssignmentSymmetry() { @@ -219,6 +258,15 @@ namespace AssetRipper.Tests AreAlmostEqual(uv0, unpackedUV6, 0.000001f); } + [Test] + public void BoneWeightAssignmentSymmetry() + { + CompressedMesh_5 compressedMesh = new(); + compressedMesh.SetWeights(boneWeights); + BoneWeight4[] unpackedValues = compressedMesh.GetWeights(); + AreAlmostEqual(boneWeights, unpackedValues, 0.000001f); + } + private static void AreAlmostEqual(ReadOnlySpan expected, ReadOnlySpan actual, float maxDeviation) { if (expected.Length != actual.Length) @@ -267,6 +315,31 @@ namespace AssetRipper.Tests } } + private static void AreAlmostEqual(ReadOnlySpan expectedSpan, ReadOnlySpan actualSpan, float maxDeviation) + { + if (expectedSpan.Length != actualSpan.Length) + { + Assert.Fail($"Lengths were inequal.\nExpected: {expectedSpan.Length}\nBut was: {actualSpan.Length}"); + } + + for (int i = 0; i < expectedSpan.Length; i++) + { + BoneWeight4 expected = expectedSpan[i]; + BoneWeight4 actual = actualSpan[i]; + if (expected.Indices != actual.Indices) + { + Assert.Fail($"Bone Indices significantly differ at span index {i}\nExpected: {expected.Indices}\nBut was: {actual.Indices}"); + } + for (int j = 0; j < BoneWeight4.Count; j++) + { + if (float.Abs(expected.Weights[j] - actual.Weights[j]) > maxDeviation) + { + Assert.Fail($"Weights significantly differ at span index {i}, weight index {j}\nExpected: {expected.Weights[j]}\nBut was: {actual.Weights[j]}"); + } + } + } + } + private static void AreAlmostEqual(ReadOnlySpan expected, ReadOnlySpan actual, float maxDeviation) { if (expected.Length != actual.Length) @@ -276,7 +349,7 @@ namespace AssetRipper.Tests for (int i = 0; i < expected.Length; i++) { - if (MathF.Abs(expected[i] - actual[i]) > maxDeviation) + if (float.Abs(expected[i] - actual[i]) > maxDeviation) { Assert.Fail($"Values significantly differ at index {i}\nExpected: {expected[i]}\nBut was: {actual[i]}"); }