From 43c8dc711a25c98a89a1d131df0294afbcd5c0d5 Mon Sep 17 00:00:00 2001 From: Kyle Tyo <36606155+VerinSenpai@users.noreply.github.com> Date: Sun, 8 Feb 2026 18:47:43 -0500 Subject: [PATCH] Fix(Humanoid): Prevent skin color verification failures due to precision loss (#42836) port over uberrations fixes. --- .../Humanoid/SkinColorationPrototype.cs | 103 +++++-- .../Preferences/Humanoid/SkinTonesTest.cs | 276 +++++++++++++++++- 2 files changed, 349 insertions(+), 30 deletions(-) diff --git a/Content.Shared/Humanoid/SkinColorationPrototype.cs b/Content.Shared/Humanoid/SkinColorationPrototype.cs index e37265cea10..afb90bf39aa 100644 --- a/Content.Shared/Humanoid/SkinColorationPrototype.cs +++ b/Content.Shared/Humanoid/SkinColorationPrototype.cs @@ -1,3 +1,4 @@ +using System; using System.Numerics; using JetBrains.Annotations; using Robust.Shared.Prototypes; @@ -22,7 +23,7 @@ public sealed partial class SkinColorationPrototype : IPrototype } /// -/// The type of input taken by a +/// The type of input taken by a /// [Serializable, NetSerializable] public enum SkinColorationStrategyInput @@ -151,10 +152,12 @@ public sealed partial class HumanTonedSkinColoration : ISkinColorationStrategy if (rangeOffset <= 0) { + // First 20 values adjust hue. hue += Math.Abs(rangeOffset); } else { + // Remaining 80 values adjust saturation and value. sat += rangeOffset; val -= rangeOffset; } @@ -182,14 +185,15 @@ public sealed partial class HumanTonedSkinColoration : ISkinColorationStrategy } /// -/// Unary coloration strategy that clamps the color within the HSV colorspace +/// Coloration strategy that clamps the color within the HSV colorspace. /// [DataDefinition] [Serializable, NetSerializable] public sealed partial class ClampedHsvColoration : ISkinColorationStrategy { /// - /// The (min, max) of the hue channel. + /// Defines the valid (min, max) range for the hue channel (0.0 to 1.0). + /// If min > max, the range wraps around 1.0 (e.g., for reds). /// [DataField] public (float, float)? Hue; @@ -212,13 +216,13 @@ public sealed partial class ClampedHsvColoration : ISkinColorationStrategy { var hsv = Color.ToHsv(color); - if (Hue is (var minHue, var maxHue) && (hsv.X < minHue || hsv.X > maxHue)) + if (Hue is (var minHue, var maxHue) && !SkinColorationUtils.IsHueInRange(hsv.X, minHue, maxHue)) return false; - if (Saturation is (var minSaturation, var maxSaturation) && (hsv.Y < minSaturation || hsv.Y > maxSaturation)) + if (Saturation is (var minSat, var maxSat) && (hsv.Y < minSat - SkinColorationUtils.Epsilon || hsv.Y > maxSat + SkinColorationUtils.Epsilon)) return false; - if (Value is (var minValue, var maxValue) && (hsv.Z < minValue || hsv.Z > maxValue)) + if (Value is (var minVal, var maxVal) && (hsv.Z < minVal - SkinColorationUtils.Epsilon || hsv.Z > maxVal + SkinColorationUtils.Epsilon)) return false; return true; @@ -229,27 +233,26 @@ public sealed partial class ClampedHsvColoration : ISkinColorationStrategy var hsv = Color.ToHsv(color); if (Hue is (var minHue, var maxHue)) - hsv.X = Math.Clamp(hsv.X, minHue, maxHue); - - if (Saturation is (var minSaturation, var maxSaturation)) - hsv.Y = Math.Clamp(hsv.Y, minSaturation, maxSaturation); - - if (Value is (var minValue, var maxValue)) - hsv.Z = Math.Clamp(hsv.Z, minValue, maxValue); + hsv.X = SkinColorationUtils.ClampHue(hsv.X, minHue, maxHue); + if (Saturation is (var minSat, var maxSat)) + hsv.Y = Math.Clamp(hsv.Y, minSat, maxSat); + if (Value is (var minVal, var maxVal)) + hsv.Z = Math.Clamp(hsv.Z, minVal, maxVal); return Color.FromHsv(hsv); } } /// -/// Unary coloration strategy that clamps the color within the HSL colorspace +/// Coloration strategy that clamps the color within the HSL colorspace. /// [DataDefinition] [Serializable, NetSerializable] public sealed partial class ClampedHslColoration : ISkinColorationStrategy { /// - /// The (min, max) of the hue channel. + /// Defines the valid (min, max) range for the hue channel (0.0 to 1.0). + /// If min > max, the range wraps around 1.0 (e.g., for reds). /// [DataField] public (float, float)? Hue; @@ -272,13 +275,13 @@ public sealed partial class ClampedHslColoration : ISkinColorationStrategy { var hsl = Color.ToHsl(color); - if (Hue is (var minHue, var maxHue) && (hsl.X < minHue || hsl.X > maxHue)) + if (Hue is (var minHue, var maxHue) && !SkinColorationUtils.IsHueInRange(hsl.X, minHue, maxHue)) return false; - if (Saturation is (var minSaturation, var maxSaturation) && (hsl.Y < minSaturation || hsl.Y > maxSaturation)) + if (Saturation is (var minSat, var maxSat) && (hsl.Y < minSat - SkinColorationUtils.Epsilon || hsl.Y > maxSat + SkinColorationUtils.Epsilon)) return false; - if (Lightness is (var minValue, var maxValue) && (hsl.Z < minValue || hsl.Z > maxValue)) + if (Lightness is (var minLight, var maxLight) && (hsl.Z < minLight - SkinColorationUtils.Epsilon || hsl.Z > maxLight + SkinColorationUtils.Epsilon)) return false; return true; @@ -289,14 +292,64 @@ public sealed partial class ClampedHslColoration : ISkinColorationStrategy var hsl = Color.ToHsl(color); if (Hue is (var minHue, var maxHue)) - hsl.X = Math.Clamp(hsl.X, minHue, maxHue); - - if (Saturation is (var minSaturation, var maxSaturation)) - hsl.Y = Math.Clamp(hsl.Y, minSaturation, maxSaturation); - - if (Lightness is (var minValue, var maxValue)) - hsl.Z = Math.Clamp(hsl.Z, minValue, maxValue); + hsl.X = SkinColorationUtils.ClampHue(hsl.X, minHue, maxHue); + if (Saturation is (var minSat, var maxSat)) + hsl.Y = Math.Clamp(hsl.Y, minSat, maxSat); + if (Lightness is (var minLight, var maxLight)) + hsl.Z = Math.Clamp(hsl.Z, minLight, maxLight); return Color.FromHsl(hsl); } } + +/// +/// Contains shared utility methods for handling color manipulations in skin coloration strategies. +/// +internal static class SkinColorationUtils +{ + /// + /// An empirically determined epsilon to account for floating-point drift during RGB -> HSL/HSV -> RGB conversions. + /// Based on high-iteration testing (50M+ samples) which showed a max drift of ~4.9E-6 for HSL. + /// A value of 1E-5f provides a robust safety margin. + /// + public const float Epsilon = 1e-5f; // 0.00001 + + /// + /// Checks if a hue value is within a specified range, correctly handling ranges that wrap around 1.0 (e.g., reds). + /// + /// The hue value to check (0.0 to 1.0). + /// The minimum bound of the hue range. + /// The maximum bound of the hue range. + /// True if the hue is within the range; otherwise, false. + public static bool IsHueInRange(float hue, float minHue, float maxHue) + { + if (minHue > maxHue) // Wraps around 1.0 (e.g., reds) + return hue >= minHue - Epsilon || hue <= maxHue + Epsilon; + return hue >= minHue - Epsilon && hue <= maxHue + Epsilon; + } + + /// + /// Clamps a hue value to the closest boundary of a given range, correctly handling ranges that wrap around 1.0. + /// + /// The hue value to clamp (0.0 to 1.0). + /// The minimum bound of the hue range. + /// The maximum bound of the hue range. + /// The clamped hue value, adjusted to the nearest boundary if it was outside the valid range. + public static float ClampHue(float hue, float minHue, float maxHue) + { + if (minHue > maxHue) // Wraps around 1.0 + { + // If it's already in the valid range, do nothing. + if (hue >= minHue || hue <= maxHue) + return hue; + + // It's in the "invalid" gap between maxHue and minHue. Find the closest boundary. + var mid = (maxHue + minHue) / 2f; + if (hue > mid) + return minHue; + return maxHue; + } + + return Math.Clamp(hue, minHue, maxHue); + } +} diff --git a/Content.Tests/Shared/Preferences/Humanoid/SkinTonesTest.cs b/Content.Tests/Shared/Preferences/Humanoid/SkinTonesTest.cs index 63cefac812b..82ef82c5d42 100644 --- a/Content.Tests/Shared/Preferences/Humanoid/SkinTonesTest.cs +++ b/Content.Tests/Shared/Preferences/Humanoid/SkinTonesTest.cs @@ -1,28 +1,294 @@ +using System; +using System.Numerics; using Content.Shared.Humanoid; using NUnit.Framework; +using Robust.Shared.Maths; +using Robust.Shared.Random; namespace Content.Tests.Shared.Preferences.Humanoid; [TestFixture] +[TestOf(typeof(HumanTonedSkinColoration))] +[TestOf(typeof(ClampedHslColoration))] +[TestOf(typeof(ClampedHsvColoration))] public sealed class SkinTonesTest { + // These fields will track the maximum observed floating-point drift across all tests. + // This is for monitoring, even if tests pass due to a sufficiently large Epsilon in production code. + private static float _maxHslDrift; + private static float _maxHsvDrift; + + [OneTimeTearDown] + public void OneTimeTearDown() + { + // After all tests in this fixture run, print the final results. + // This gives insight into the actual precision loss, even if VerifySkinColor passes. + TestContext.Out.WriteLine("\n--- FINAL DRIFT SUMMARY FOR ALL CLAMPING TESTS ---"); + TestContext.Out.WriteLine($"Maximum observed HSL drift: {_maxHslDrift:E}"); // Scientific notation for precision + TestContext.Out.WriteLine($"Maximum observed HSV drift: {_maxHsvDrift:E}"); + TestContext.Out.WriteLine("This indicates the actual max floating-point error observed. Production code's Epsilon should be >= this value."); + TestContext.Out.WriteLine("--------------------------------------------------"); + } + + /// + /// Checks that colors generated by HumanTonedSkinColoration.FromUnary pass verification. + /// [Test] - public void TestHumanSkinToneValidity() + public void TestHumanSkinTonesFromUnaryAreValid() { var strategy = new HumanTonedSkinColoration(); - for (var i = 0; i <= 100; i++) + // Testing across a finer range to hit more edge cases + for (var i = 0; i <= 10000; i++) { - var color = strategy.FromUnary(i); - Assert.That(strategy.VerifySkinColor(color)); + var unaryInput = i / 100f; // Test values like 0.0, 0.01, ..., 100.0 + var color = strategy.FromUnary(unaryInput); + Assert.That(strategy.VerifySkinColor(color), $"Color {color} from unary value {unaryInput} failed verification."); } } + /// + /// Checks that converting a unary value to a color and back results in a similar unary value. + /// [Test] - public void TestDefaultSkinToneValid() + public void TestHumanTonedSkinColoration_RoundTrip() { var strategy = new HumanTonedSkinColoration(); + // Test values across the full range, including transition points + for (var i = 0; i <= 10000; i++) + { + var originalUnary = i / 100f; + var color = strategy.FromUnary(originalUnary); + var resultUnary = strategy.ToUnary(color); + // A small tolerance is expected due to float precision and the nature of HSV conversions + // as well as the rounding logic in ToUnary. + Assert.That(resultUnary, Is.EqualTo(originalUnary).Within(1e-2f), // 1e-2f (0.01) is 1% of the unary range, which is reasonable for rounding and float error. + $"Round trip failed for unary {originalUnary}. Got {resultUnary} back."); + } + } + + /// + /// Checks that the default human skin tone is considered valid. + /// + [Test] + public void TestDefaultHumanSkinToneValid() + { + var strategy = new HumanTonedSkinColoration(); Assert.That(strategy.VerifySkinColor(strategy.ValidHumanSkinTone)); } + + /// + /// Checks that clamping random colors with a low-saturation, high-lightness HSL strategy produces valid colors. + /// This was the primary test case that originally revealed the precision bug. + /// + [Test] + public void TestTintedHuesValidHsl() + { + var random = new RobustRandom(); + var strategy = new ClampedHslColoration() + { + Saturation = (0.0f, 0.1f), + Lightness = (0.85f, 1.0f), + }; + + for (var i = 0; i <= 10000; i++) + { + var color = new Color(random.NextFloat(), random.NextFloat(), random.NextFloat()); + var skinColor = strategy.ClosestSkinColor(color); + LogDriftIfGreater(strategy, color, skinColor, TestContext.CurrentContext.Test.Name); // Monitor drift + + Assert.That(strategy.VerifySkinColor(skinColor), + $"Color {skinColor} (from input {color}) failed verification in {TestContext.CurrentContext.Test.Name} on iteration {i}"); + } + } + + /// + /// Checks that clamping random colors with a low-saturation, high-value HSV strategy produces valid colors. + /// + [Test] + public void TestTintedHuesValidHsv() + { + var random = new RobustRandom(); + var strategy = new ClampedHsvColoration() + { + Saturation = (0.0f, 0.1f), + Value = (0.85f, 1.0f), + }; + + for (var i = 0; i <= 10000; i++) + { + var color = new Color(random.NextFloat(), random.NextFloat(), random.NextFloat()); + var skinColor = strategy.ClosestSkinColor(color); + LogDriftIfGreater(strategy, color, skinColor, TestContext.CurrentContext.Test.Name); // Monitor drift + + Assert.That(strategy.VerifySkinColor(skinColor), + $"Color {skinColor} (from input {color}) failed verification in {TestContext.CurrentContext.Test.Name} on iteration {i}"); + } + } + + /// + /// Checks that clamping random colors with an HSL strategy that limits all three channels produces valid colors. + /// + [Test] + public void TestClampedHslWithAllChannels() + { + var random = new RobustRandom(); + var strategy = new ClampedHslColoration() + { + Hue = (0.1f, 0.3f), + Saturation = (0.2f, 0.8f), + Lightness = (0.3f, 0.7f), + }; + + for (var i = 0; i <= 10000; i++) + { + var color = new Color(random.NextFloat(), random.NextFloat(), random.NextFloat()); + var skinColor = strategy.ClosestSkinColor(color); + LogDriftIfGreater(strategy, color, skinColor, TestContext.CurrentContext.Test.Name); // Monitor drift + + Assert.That(strategy.VerifySkinColor(skinColor), + $"Color {skinColor} (from input {color}) failed verification in {TestContext.CurrentContext.Test.Name} on iteration {i}"); + } + } + + /// + /// Checks that clamping random colors with an HSV strategy that limits all three channels produces valid colors. + /// + [Test] + public void TestClampedHsvWithAllChannels() + { + var random = new RobustRandom(); + var strategy = new ClampedHsvColoration() + { + Hue = (0.1f, 0.3f), + Saturation = (0.2f, 0.8f), + Value = (0.3f, 0.7f), + }; + + for (var i = 0; i <= 10000; i++) + { + var color = new Color(random.NextFloat(), random.NextFloat(), random.NextFloat()); + var skinColor = strategy.ClosestSkinColor(color); + LogDriftIfGreater(strategy, color, skinColor, TestContext.CurrentContext.Test.Name); // Monitor drift + + Assert.That(strategy.VerifySkinColor(skinColor), + $"Color {skinColor} (from input {color}) failed verification in {TestContext.CurrentContext.Test.Name} on iteration {i}"); + } + } + + /// + /// Checks that clamping works correctly for HSL strategies where the hue range wraps around the 0-1 boundary. + /// + [Test] + public void TestClampedHslWithCircularHue() + { + var random = new RobustRandom(); + var strategy = new ClampedHslColoration() + { + Hue = (0.9f, 0.1f), // A range that wraps around 1.0 (e.g., reds) + Saturation = (0.5f, 1.0f), + Lightness = (0.5f, 1.0f), + }; + + for (var i = 0; i <= 10000; i++) + { + var color = new Color(random.NextFloat(), random.NextFloat(), random.NextFloat()); + var skinColor = strategy.ClosestSkinColor(color); + LogDriftIfGreater(strategy, color, skinColor, TestContext.CurrentContext.Test.Name); // Monitor drift + + Assert.That(strategy.VerifySkinColor(skinColor), + $"Color {skinColor} (from input {color}) with circular hue failed verification in {TestContext.CurrentContext.Test.Name} on iteration {i}"); + } + } + + /// + /// Checks that a color that is already valid is not modified. + /// + [Test] + public void TestClosestSkinColorReturnsValidColor() + { + var strategy = new ClampedHslColoration() + { + Saturation = (0.0f, 1.0f), + Lightness = (0.0f, 1.0f), + }; + + var validColor = Color.FromHsl(new Vector4(0.5f, 0.5f, 0.5f, 1.0f)); + var result = strategy.ClosestSkinColor(validColor); + + Assert.That(strategy.VerifySkinColor(result), Is.True); + } + + /// + /// Checks that a color outside the valid range is correctly clamped to a valid color. + /// + [Test] + public void TestClosestSkinColorClampsInvalidColor() + { + var strategy = new ClampedHslColoration() + { + Saturation = (0.0f, 0.1f), + Lightness = (0.85f, 1.0f), + }; + + // This color has high saturation and low lightness, should be clamped + var invalidColor = Color.FromHsl(new Vector4(0.5f, 0.9f, 0.2f, 1.0f)); + var result = strategy.ClosestSkinColor(invalidColor); + + Assert.That(strategy.VerifySkinColor(result), Is.True); + Assert.That(result, Is.Not.EqualTo(invalidColor)); + } + + /// + /// Helper method to calculate and log the maximum floating-point drift observed during clamping. + /// This is for monitoring the behavior of the clamping, not for causing test failures directly. + /// + private void LogDriftIfGreater(ISkinColorationStrategy strategy, Color original, Color clamped, string testName) + { + if (strategy is ClampedHslColoration hslStrategy) + { + var hsl = Color.ToHsl(clamped); + var (minSat, maxSat) = hslStrategy.Saturation ?? (0f, 1f); + var (minLight, maxLight) = hslStrategy.Lightness ?? (0f, 1f); + + // Re-calculate the drift from the original bounds *without* applying Epsilon + // This shows the pure floating-point error relative to the intended boundaries. + var satDrift = Math.Max(minSat - hsl.Y, hsl.Y - maxSat); + var lightDrift = Math.Max(minLight - hsl.Z, hsl.Z - maxLight); + var currentDrift = Math.Max(satDrift, lightDrift); + + if (currentDrift > _maxHslDrift) + { + TestContext.Out.WriteLine($"--- NEW MAX HSL DRIFT DETECTED in {testName} ---"); + TestContext.Out.WriteLine($"Max HSL Drift: {currentDrift:E} (previously {_maxHslDrift:E})"); + TestContext.Out.WriteLine($"Original RGB: {original}"); + TestContext.Out.WriteLine($"Clamped RGB: {clamped}"); + TestContext.Out.WriteLine($"Result HSL: H={hsl.X:F8}, S={hsl.Y:F8}, L={hsl.Z:F8}"); + TestContext.Out.WriteLine($"Bounds: S=({minSat:F8}, {maxSat:F8}), L=({minLight:F8}, {maxLight:F8})"); + _maxHslDrift = currentDrift; + } + } + else if (strategy is ClampedHsvColoration hsvStrategy) + { + var hsv = Color.ToHsv(clamped); + var (minSat, maxSat) = hsvStrategy.Saturation ?? (0f, 1f); + var (minValue, maxValue) = hsvStrategy.Value ?? (0f, 1f); + + var satDrift = Math.Max(minSat - hsv.Y, hsv.Y - maxSat); + var valueDrift = Math.Max(minValue - hsv.Z, hsv.Z - maxValue); + var currentDrift = Math.Max(satDrift, valueDrift); + + if (currentDrift > _maxHsvDrift) + { + TestContext.Out.WriteLine($"--- NEW MAX HSV DRIFT DETECTED in {testName} ---"); + TestContext.Out.WriteLine($"Max HSV Drift: {currentDrift:E} (previously {_maxHsvDrift:E})"); + TestContext.Out.WriteLine($"Original RGB: {original}"); + TestContext.Out.WriteLine($"Clamped RGB: {clamped}"); + TestContext.Out.WriteLine($"Result HSV: H={hsv.X:F8}, S={hsv.Y:F8}, V={hsv.Z:F8}"); + TestContext.Out.WriteLine($"Bounds: S=({minSat:F8}, {maxSat:F8}), V=({minValue:F8}, {maxValue:F8})"); + _maxHsvDrift = currentDrift; + } + } + } }