From 00eb1f8b92225a0ba7b42bbf618261760fb0bfd0 Mon Sep 17 00:00:00 2001 From: Kurtis Date: Mon, 8 Jun 2026 15:50:21 -0700 Subject: [PATCH 1/2] Add tests --- .../MRTKDevTemplate/Packages/manifest.json | 1 + org.mixedrealitytoolkit.theming/CHANGELOG.md | 1 + org.mixedrealitytoolkit.theming/Tests.meta | 8 ++ .../Tests/Editor.meta | 8 ++ .../Tests/Editor/AssemblyInfo.cs | 12 ++ .../Tests/Editor/AssemblyInfo.cs.meta | 11 ++ .../Tests/Editor/BaseThemeBinderTests.cs | 125 ++++++++++++++++++ .../Tests/Editor/BaseThemeBinderTests.cs.meta | 11 ++ .../Editor/MRTK.Theming.EditorTests.asmdef | 31 +++++ .../MRTK.Theming.EditorTests.asmdef.meta | 7 + .../Tests/Editor/PackageValidationTest.cs | 32 +++++ .../Editor/PackageValidationTest.cs.meta | 11 ++ 12 files changed, 258 insertions(+) create mode 100644 org.mixedrealitytoolkit.theming/Tests.meta create mode 100644 org.mixedrealitytoolkit.theming/Tests/Editor.meta create mode 100644 org.mixedrealitytoolkit.theming/Tests/Editor/AssemblyInfo.cs create mode 100644 org.mixedrealitytoolkit.theming/Tests/Editor/AssemblyInfo.cs.meta create mode 100644 org.mixedrealitytoolkit.theming/Tests/Editor/BaseThemeBinderTests.cs create mode 100644 org.mixedrealitytoolkit.theming/Tests/Editor/BaseThemeBinderTests.cs.meta create mode 100644 org.mixedrealitytoolkit.theming/Tests/Editor/MRTK.Theming.EditorTests.asmdef create mode 100644 org.mixedrealitytoolkit.theming/Tests/Editor/MRTK.Theming.EditorTests.asmdef.meta create mode 100644 org.mixedrealitytoolkit.theming/Tests/Editor/PackageValidationTest.cs create mode 100644 org.mixedrealitytoolkit.theming/Tests/Editor/PackageValidationTest.cs.meta diff --git a/UnityProjects/MRTKDevTemplate/Packages/manifest.json b/UnityProjects/MRTKDevTemplate/Packages/manifest.json index 16cea28ea..fc096fc1a 100644 --- a/UnityProjects/MRTKDevTemplate/Packages/manifest.json +++ b/UnityProjects/MRTKDevTemplate/Packages/manifest.json @@ -82,6 +82,7 @@ "org.mixedrealitytoolkit.input", "org.mixedrealitytoolkit.spatialmanipulation", "org.mixedrealitytoolkit.standardassets", + "org.mixedrealitytoolkit.theming", "org.mixedrealitytoolkit.uxcomponents", "org.mixedrealitytoolkit.uxcomponents.noncanvas", "org.mixedrealitytoolkit.uxcore", diff --git a/org.mixedrealitytoolkit.theming/CHANGELOG.md b/org.mixedrealitytoolkit.theming/CHANGELOG.md index 52880eb4c..febb779dc 100644 --- a/org.mixedrealitytoolkit.theming/CHANGELOG.md +++ b/org.mixedrealitytoolkit.theming/CHANGELOG.md @@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). * Added serializable theme item data types for booleans. [PR #1130](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1130) * Added built-in binders for skybox materials and behaviour enabled states. [PR #1130](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1130) +* Added edit mode tests for validating custom theming types and their dependencies. [PR #1133](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1133) ## [1.0.0-pre.1] - 2026-05-20 diff --git a/org.mixedrealitytoolkit.theming/Tests.meta b/org.mixedrealitytoolkit.theming/Tests.meta new file mode 100644 index 000000000..0aa4b53d4 --- /dev/null +++ b/org.mixedrealitytoolkit.theming/Tests.meta @@ -0,0 +1,8 @@ +fileFormatVersion: 2 +guid: f15c71cd4449f464592b9317d8df9037 +folderAsset: yes +DefaultImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/org.mixedrealitytoolkit.theming/Tests/Editor.meta b/org.mixedrealitytoolkit.theming/Tests/Editor.meta new file mode 100644 index 000000000..28c724e82 --- /dev/null +++ b/org.mixedrealitytoolkit.theming/Tests/Editor.meta @@ -0,0 +1,8 @@ +fileFormatVersion: 2 +guid: 05341ca71d86b4749837802d9272b087 +folderAsset: yes +DefaultImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/org.mixedrealitytoolkit.theming/Tests/Editor/AssemblyInfo.cs b/org.mixedrealitytoolkit.theming/Tests/Editor/AssemblyInfo.cs new file mode 100644 index 000000000..a01411c1b --- /dev/null +++ b/org.mixedrealitytoolkit.theming/Tests/Editor/AssemblyInfo.cs @@ -0,0 +1,12 @@ +// Copyright (c) Mixed Reality Toolkit Contributors +// Licensed under the BSD 3-Clause + +using System.Reflection; + +[assembly: AssemblyProduct("Mixed Reality Toolkit UX Theming Editor Tests")] +[assembly: AssemblyCopyright("Copyright (c) Mixed Reality Toolkit Contributors")] + +// The AssemblyVersion attribute is checked-in and is recommended not to be changed often. +// https://docs.microsoft.com/troubleshoot/visualstudio/general/assembly-version-assembly-file-version +// AssemblyFileVersion and AssemblyInformationalVersion are added by pack-upm.ps1 to match the current MRTK build version. +[assembly: AssemblyVersion("1.0.0.0")] diff --git a/org.mixedrealitytoolkit.theming/Tests/Editor/AssemblyInfo.cs.meta b/org.mixedrealitytoolkit.theming/Tests/Editor/AssemblyInfo.cs.meta new file mode 100644 index 000000000..d5ad5babc --- /dev/null +++ b/org.mixedrealitytoolkit.theming/Tests/Editor/AssemblyInfo.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 7b447f4a2c57e514ca8ccb9ee04e1d45 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/org.mixedrealitytoolkit.theming/Tests/Editor/BaseThemeBinderTests.cs b/org.mixedrealitytoolkit.theming/Tests/Editor/BaseThemeBinderTests.cs new file mode 100644 index 000000000..dad7846e6 --- /dev/null +++ b/org.mixedrealitytoolkit.theming/Tests/Editor/BaseThemeBinderTests.cs @@ -0,0 +1,125 @@ +// Copyright (c) Mixed Reality Toolkit Contributors +// Licensed under the BSD 3-Clause + +using NUnit.Framework; +using System; +using System.Linq; +using UnityEditor; + +namespace MixedReality.Toolkit.Theming.Tests.EditMode +{ + /// + /// Tests to evaluate the architectural integrity of Theme Binders. + /// + public class BaseThemeBinderTests + { + /// + /// Verifies that all non-abstract theme binders have fully resolved their generic arguments (T and K). + /// + [Test] + public void ConcreteBindersFullyResolveGenericArguments() + { + // Quickly grab every class in the project that implements IBinder + var binderTypes = TypeCache.GetTypesDerivedFrom(typeof(IBinder)) + .Where(t => !t.IsAbstract && !t.IsInterface); + + int testedCount = 0; + + foreach (Type type in binderTypes) + { + Assert.IsFalse(type.ContainsGenericParameters, + $"Binder type '{type.Name}' is not abstract, but contains open generic parameters. " + + "Concrete binders must fully resolve their BaseThemeBinder data types."); + + Type baseBinderType = type; + while (baseBinderType != null && (!baseBinderType.IsGenericType || baseBinderType.GetGenericTypeDefinition() != typeof(BaseThemeBinder<,>))) + { + baseBinderType = baseBinderType.BaseType; + } + + if (baseBinderType != null) + { + Type valueType = baseBinderType.GenericTypeArguments[0]; + Type targetType = baseBinderType.GenericTypeArguments[1]; + + Assert.IsNotNull(valueType, $"Value type (T) for '{type.Name}' could not be resolved."); + Assert.IsNotNull(targetType, $"Target type (K) for '{type.Name}' could not be resolved."); + + testedCount++; + } + } + + Assert.Greater(testedCount, 0, "No concrete BaseThemeBinder implementations were found to test."); + } + + /// + /// Verifies that all non-abstract theme item data classes have the [Serializable] attribute. + /// + [Test] + public void ConcreteThemeItemDataClassesAreSerializable() + { + var itemDataTypes = TypeCache.GetTypesDerivedFrom(typeof(BaseThemeItemData<>)) + .Where(t => !t.IsAbstract && !t.IsInterface); + + int testedCount = 0; + foreach (Type type in itemDataTypes) + { + Assert.IsTrue(type.IsSerializable, + $"Theme item data type '{type.Name}' must have the [Serializable] attribute to be saved in a Theme asset."); + testedCount++; + } + + Assert.Greater(testedCount, 0, "No concrete BaseThemeItemData implementations were found to test."); + } + + /// + /// Verifies that every generic data type targeted by a binder has a corresponding concrete ThemeItemData class to hold it. + /// + [Test] + public void BindersHaveCorrespondingThemeItemData() + { + var binderTypes = TypeCache.GetTypesDerivedFrom(typeof(IBinder)) + .Where(t => !t.IsAbstract && !t.IsInterface); + + var itemDataTypes = TypeCache.GetTypesDerivedFrom(typeof(BaseThemeItemData<>)) + .Where(t => !t.IsAbstract && !t.IsInterface).ToList(); + + int testedCount = 0; + foreach (Type binderType in binderTypes) + { + Type baseBinderType = binderType; + while (baseBinderType != null && (!baseBinderType.IsGenericType || baseBinderType.GetGenericTypeDefinition() != typeof(BaseThemeBinder<,>))) + { + baseBinderType = baseBinderType.BaseType; + } + + if (baseBinderType != null) + { + Type valueType = baseBinderType.GenericTypeArguments[0]; + + bool hasMatchingItemData = false; + foreach (Type itemDataType in itemDataTypes) + { + Type baseItemDataType = itemDataType; + while (baseItemDataType != null && (!baseItemDataType.IsGenericType || baseItemDataType.GetGenericTypeDefinition() != typeof(BaseThemeItemData<>))) + { + baseItemDataType = baseItemDataType.BaseType; + } + + if (baseItemDataType != null && baseItemDataType.GenericTypeArguments[0] == valueType) + { + hasMatchingItemData = true; + break; + } + } + + Assert.IsTrue(hasMatchingItemData, + $"Binder '{binderType.Name}' binds to '{valueType.Name}', but no concrete BaseThemeItemData<{valueType.Name}> class exists to serialize it in the Theme asset."); + testedCount++; + } + } + + Assert.Greater(testedCount, 0, "No concrete BaseThemeBinder implementations were found to test."); + } + } +} diff --git a/org.mixedrealitytoolkit.theming/Tests/Editor/BaseThemeBinderTests.cs.meta b/org.mixedrealitytoolkit.theming/Tests/Editor/BaseThemeBinderTests.cs.meta new file mode 100644 index 000000000..98239b300 --- /dev/null +++ b/org.mixedrealitytoolkit.theming/Tests/Editor/BaseThemeBinderTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 033e10d7e483c314386bb9b93fb16ed9 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/org.mixedrealitytoolkit.theming/Tests/Editor/MRTK.Theming.EditorTests.asmdef b/org.mixedrealitytoolkit.theming/Tests/Editor/MRTK.Theming.EditorTests.asmdef new file mode 100644 index 000000000..0aaf086e0 --- /dev/null +++ b/org.mixedrealitytoolkit.theming/Tests/Editor/MRTK.Theming.EditorTests.asmdef @@ -0,0 +1,31 @@ +{ + "name": "MixedReality.Toolkit.Theming.Editor.Tests", + "rootNamespace": "MixedReality.Toolkit.Theming.Tests.EditMode", + "references": [ + "MixedReality.Toolkit.Theming", + "MixedReality.Toolkit.UXCore", + "UnityEngine.TestRunner", + "UnityEditor.TestRunner" + ], + "includePlatforms": [ + "Editor" + ], + "excludePlatforms": [], + "allowUnsafeCode": false, + "overrideReferences": true, + "precompiledReferences": [ + "nunit.framework.dll" + ], + "autoReferenced": false, + "defineConstraints": [ + "UNITY_INCLUDE_TESTS" + ], + "versionDefines": [ + { + "name": "com.unity.asset-store-validation", + "expression": "", + "define": "HAS_ASSET_STORE_VALIDATION" + } + ], + "noEngineReferences": false +} \ No newline at end of file diff --git a/org.mixedrealitytoolkit.theming/Tests/Editor/MRTK.Theming.EditorTests.asmdef.meta b/org.mixedrealitytoolkit.theming/Tests/Editor/MRTK.Theming.EditorTests.asmdef.meta new file mode 100644 index 000000000..a0bda6f68 --- /dev/null +++ b/org.mixedrealitytoolkit.theming/Tests/Editor/MRTK.Theming.EditorTests.asmdef.meta @@ -0,0 +1,7 @@ +fileFormatVersion: 2 +guid: 07bb2e8be351e584b80703fb9df9603d +AssemblyDefinitionImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/org.mixedrealitytoolkit.theming/Tests/Editor/PackageValidationTest.cs b/org.mixedrealitytoolkit.theming/Tests/Editor/PackageValidationTest.cs new file mode 100644 index 000000000..4c9561e3f --- /dev/null +++ b/org.mixedrealitytoolkit.theming/Tests/Editor/PackageValidationTest.cs @@ -0,0 +1,32 @@ +// Copyright (c) Mixed Reality Toolkit Contributors +// Licensed under the BSD 3-Clause + +#if HAS_ASSET_STORE_VALIDATION + +using MixedReality.Toolkit.Core.Tests.EditMode; +using NUnit.Framework; +using System; + +namespace MixedReality.Toolkit.Theming.Tests.EditMode +{ + /// + /// This class is used to validate the package for the Mixed Reality Toolkit UX Theming package, verifying that all + /// requirements are met for publishing the package to the Unity Asset Store. + /// + internal class PackageValidationTest + { + /// + /// Test to validate the package for the Mixed Reality Toolkit UX Theming package, verifying that all + /// requirements are met for publishing the package to the Unity Asset Store. + /// + [Test] + public void PackageTest() + { + PackageValidatorResults results = PackageValidator.Validate("org.mixedrealitytoolkit.theming"); + Assert.AreEqual(0, results.FailedCount, $"Failed tests found.{Environment.NewLine}{results.ToString(PackageValidatorResults.MessageType.Failed)}"); + Assert.IsTrue(0 < results.SucceededCount, "No tests succeeded"); + } + } +} + +#endif // HAS_ASSET_STORE_VALIDATION diff --git a/org.mixedrealitytoolkit.theming/Tests/Editor/PackageValidationTest.cs.meta b/org.mixedrealitytoolkit.theming/Tests/Editor/PackageValidationTest.cs.meta new file mode 100644 index 000000000..4a16aba46 --- /dev/null +++ b/org.mixedrealitytoolkit.theming/Tests/Editor/PackageValidationTest.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 78b18fd01cb9ed64a97e55d09ba534e6 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: From 1df3ef05f1f033fd39fca484ba2be989beb0bd04 Mon Sep 17 00:00:00 2001 From: Kurtis Date: Thu, 15 Oct 2026 12:41:09 -0700 Subject: [PATCH 2/2] Optimize Theming hot paths: O(1) hash cache in Theme, delegate caching in BaseThemeBinder, Zero-GC dynamic listeners in ThemeDataSource, and add comprehensive test suite --- .../AssemblyInfo.cs | 2 + .../Binders/BaseThemeBinder.cs | 9 +- org.mixedrealitytoolkit.theming/CHANGELOG.md | 7 + .../Tests/Runtime.meta | 8 + .../Tests/Runtime/AssemblyInfo.cs | 12 + .../Tests/Runtime/AssemblyInfo.cs.meta | 2 + .../Runtime/MRTK.Theming.RuntimeTests.asmdef | 27 ++ .../MRTK.Theming.RuntimeTests.asmdef.meta | 7 + .../Tests/Runtime/ThemingHotPathTests.cs | 310 ++++++++++++++++++ .../Tests/Runtime/ThemingHotPathTests.cs.meta | 7 + .../Theming/Theme.cs | 66 +++- .../Theming/ThemeDataSource.cs | 28 +- org.mixedrealitytoolkit.uxcore/CHANGELOG.md | 6 + .../Theming/ThemeBinding.cs | 12 +- org.mixedrealitytoolkit.uxcore/package.json | 2 +- 15 files changed, 487 insertions(+), 18 deletions(-) create mode 100644 org.mixedrealitytoolkit.theming/Tests/Runtime.meta create mode 100644 org.mixedrealitytoolkit.theming/Tests/Runtime/AssemblyInfo.cs create mode 100644 org.mixedrealitytoolkit.theming/Tests/Runtime/AssemblyInfo.cs.meta create mode 100644 org.mixedrealitytoolkit.theming/Tests/Runtime/MRTK.Theming.RuntimeTests.asmdef create mode 100644 org.mixedrealitytoolkit.theming/Tests/Runtime/MRTK.Theming.RuntimeTests.asmdef.meta create mode 100644 org.mixedrealitytoolkit.theming/Tests/Runtime/ThemingHotPathTests.cs create mode 100644 org.mixedrealitytoolkit.theming/Tests/Runtime/ThemingHotPathTests.cs.meta diff --git a/org.mixedrealitytoolkit.theming/AssemblyInfo.cs b/org.mixedrealitytoolkit.theming/AssemblyInfo.cs index 218ffdc9c..c1046e1bd 100644 --- a/org.mixedrealitytoolkit.theming/AssemblyInfo.cs +++ b/org.mixedrealitytoolkit.theming/AssemblyInfo.cs @@ -8,6 +8,8 @@ [assembly: AssemblyCopyright("Copyright (c) Mixed Reality Toolkit Contributors")] [assembly: InternalsVisibleTo("MixedReality.Toolkit.Theming.Editor")] +[assembly: InternalsVisibleTo("MixedReality.Toolkit.Theming.Editor.Tests")] +[assembly: InternalsVisibleTo("MixedReality.Toolkit.Theming.Runtime.Tests")] // The AssemblyVersion attribute is checked-in and is recommended not to be changed often. // https://docs.microsoft.com/troubleshoot/visualstudio/general/assembly-version-assembly-file-version diff --git a/org.mixedrealitytoolkit.theming/Binders/BaseThemeBinder.cs b/org.mixedrealitytoolkit.theming/Binders/BaseThemeBinder.cs index 4f80c710b..3ffc9acb9 100644 --- a/org.mixedrealitytoolkit.theming/Binders/BaseThemeBinder.cs +++ b/org.mixedrealitytoolkit.theming/Binders/BaseThemeBinder.cs @@ -48,19 +48,22 @@ protected void OnThemeChanged(ChangeEvent changeEvent) Apply(value); } + private UnityEngine.Events.UnityAction> cachedOnThemeChanged; + void IBinder.Subscribe(ThemeDataSource themeDataSource) { if (themeDataSource != null) { - themeDataSource.AddListener(OnThemeChanged); + cachedOnThemeChanged ??= OnThemeChanged; + themeDataSource.AddListener(cachedOnThemeChanged); } } void IBinder.Unsubscribe(ThemeDataSource themeDataSource) { - if (themeDataSource != null) + if (themeDataSource != null && cachedOnThemeChanged != null) { - themeDataSource.RemoveListener(OnThemeChanged); + themeDataSource.RemoveListener(cachedOnThemeChanged); } } } diff --git a/org.mixedrealitytoolkit.theming/CHANGELOG.md b/org.mixedrealitytoolkit.theming/CHANGELOG.md index febb779dc..7a2ce897a 100644 --- a/org.mixedrealitytoolkit.theming/CHANGELOG.md +++ b/org.mixedrealitytoolkit.theming/CHANGELOG.md @@ -9,6 +9,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). * Added serializable theme item data types for booleans. [PR #1130](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1130) * Added built-in binders for skybox materials and behaviour enabled states. [PR #1130](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1130) * Added edit mode tests for validating custom theming types and their dependencies. [PR #1133](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1133) +* Added runtime performance benchmark and regression tests for theming hot paths. [PR #1133](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1133) + +### Changed + +* Optimized `Theme.TryGetItemData` from an $O(N)$ linear list scan to an $O(1)$ hash table lookup with automatic cache invalidation on deserialization, inspector modifications, and enable events. [PR #1133](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1133) +* Optimized `BaseThemeBinder` by caching `UnityAction>` delegates, eliminating heap allocations during repeated subscribe/unsubscribe cycles. [PR #1133](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1133) +* Optimized `ThemeDataSource` to route programmatic subscribers through a Zero-GC dynamic listener list, avoiding delegate allocations while preserving inspector `UnityEvent` invocations. [PR #1133](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1133) ## [1.0.0-pre.1] - 2026-05-20 diff --git a/org.mixedrealitytoolkit.theming/Tests/Runtime.meta b/org.mixedrealitytoolkit.theming/Tests/Runtime.meta new file mode 100644 index 000000000..421cf4753 --- /dev/null +++ b/org.mixedrealitytoolkit.theming/Tests/Runtime.meta @@ -0,0 +1,8 @@ +fileFormatVersion: 2 +guid: a1c96e41a1ca4ee9b00d9bee52cefad5 +folderAsset: yes +DefaultImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/org.mixedrealitytoolkit.theming/Tests/Runtime/AssemblyInfo.cs b/org.mixedrealitytoolkit.theming/Tests/Runtime/AssemblyInfo.cs new file mode 100644 index 000000000..ad7db7000 --- /dev/null +++ b/org.mixedrealitytoolkit.theming/Tests/Runtime/AssemblyInfo.cs @@ -0,0 +1,12 @@ +// Copyright (c) Mixed Reality Toolkit Contributors +// Licensed under the BSD 3-Clause + +using System.Reflection; + +[assembly: AssemblyProduct("Mixed Reality Toolkit UX Theming Runtime Tests")] +[assembly: AssemblyCopyright("Copyright (c) Mixed Reality Toolkit Contributors")] + +// The AssemblyVersion attribute is checked-in and is recommended not to be changed often. +// https://docs.microsoft.com/troubleshoot/visualstudio/general/assembly-version-assembly-file-version +// AssemblyFileVersion and AssemblyInformationalVersion are added by pack-upm.ps1 to match the current MRTK build version. +[assembly: AssemblyVersion("1.0.0.0")] diff --git a/org.mixedrealitytoolkit.theming/Tests/Runtime/AssemblyInfo.cs.meta b/org.mixedrealitytoolkit.theming/Tests/Runtime/AssemblyInfo.cs.meta new file mode 100644 index 000000000..ce6bc3a48 --- /dev/null +++ b/org.mixedrealitytoolkit.theming/Tests/Runtime/AssemblyInfo.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 41ae53e38d474d543af527c6ef40570c \ No newline at end of file diff --git a/org.mixedrealitytoolkit.theming/Tests/Runtime/MRTK.Theming.RuntimeTests.asmdef b/org.mixedrealitytoolkit.theming/Tests/Runtime/MRTK.Theming.RuntimeTests.asmdef new file mode 100644 index 000000000..978cada25 --- /dev/null +++ b/org.mixedrealitytoolkit.theming/Tests/Runtime/MRTK.Theming.RuntimeTests.asmdef @@ -0,0 +1,27 @@ +{ + "name": "MixedReality.Toolkit.Theming.Runtime.Tests", + "rootNamespace": "MixedReality.Toolkit.Theming.Tests.Runtime", + "references": [ + "MixedReality.Toolkit.Core", + "MixedReality.Toolkit.Core.TestUtilities", + "MixedReality.Toolkit.Theming", + "MixedReality.Toolkit.UXCore", + "MixedReality.Toolkit.UXCore.Theming", + "UnityEngine.TestRunner", + "UnityEditor.TestRunner", + "Unity.TextMeshPro" + ], + "includePlatforms": [], + "excludePlatforms": [], + "allowUnsafeCode": false, + "overrideReferences": true, + "precompiledReferences": [ + "nunit.framework.dll" + ], + "autoReferenced": false, + "defineConstraints": [ + "UNITY_INCLUDE_TESTS" + ], + "versionDefines": [], + "noEngineReferences": false +} diff --git a/org.mixedrealitytoolkit.theming/Tests/Runtime/MRTK.Theming.RuntimeTests.asmdef.meta b/org.mixedrealitytoolkit.theming/Tests/Runtime/MRTK.Theming.RuntimeTests.asmdef.meta new file mode 100644 index 000000000..710c0aaf8 --- /dev/null +++ b/org.mixedrealitytoolkit.theming/Tests/Runtime/MRTK.Theming.RuntimeTests.asmdef.meta @@ -0,0 +1,7 @@ +fileFormatVersion: 2 +guid: c00b9e56f23f49aa9397b2cbba219d74 +TextScriptImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/org.mixedrealitytoolkit.theming/Tests/Runtime/ThemingHotPathTests.cs b/org.mixedrealitytoolkit.theming/Tests/Runtime/ThemingHotPathTests.cs new file mode 100644 index 000000000..6680ede63 --- /dev/null +++ b/org.mixedrealitytoolkit.theming/Tests/Runtime/ThemingHotPathTests.cs @@ -0,0 +1,310 @@ +// Copyright (c) Mixed Reality Toolkit Contributors +// Licensed under the BSD 3-Clause + +#pragma warning disable CS1591 + +using MixedReality.Toolkit.Core.Tests; +using MixedReality.Toolkit.UX; +using NUnit.Framework; +using System.Collections; +using System.Collections.Generic; +using System.Diagnostics; +using System.Reflection; +using UnityEngine; +using UnityEngine.TestTools; +using UnityEngine.TestTools.Constraints; +using Debug = UnityEngine.Debug; +using Is = UnityEngine.TestTools.Constraints.Is; + +namespace MixedReality.Toolkit.Theming.Tests.Runtime +{ + /// + /// Runtime performance and regression tests verifying lookup latency, + /// cache correctness, delegate caching, and Zero-GC invariants across Theming hot paths. + /// + public class ThemingHotPathTests : BaseRuntimeTests + { + private class TestBinder : BaseThemeBinder + { + public bool AppliedValue { get; private set; } + public int ApplyCount { get; private set; } + + public void Initialize(string itemName, Behaviour target) + { + PropertyInfo nameProp = typeof(BaseThemeBinder).GetProperty( + "ThemeDefinitionItemName", + BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance); + nameProp?.GetSetMethod(true)?.Invoke(this, new object[] { itemName }); + + PropertyInfo targetProp = typeof(BaseThemeBinder).GetProperty( + "Target", + BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance); + targetProp?.GetSetMethod(true)?.Invoke(this, new object[] { target }); + } + + protected override void Apply(BaseThemeItemData themeItemData) + { + if (themeItemData != null) + { + AppliedValue = themeItemData.Value; + ApplyCount++; + } + } + } + + private static Theme CreateTestTheme(int totalItemCount = 100) + { + Theme theme = ScriptableObject.CreateInstance(); + FieldInfo themeItemsField = typeof(Theme).GetField("themeItems", BindingFlags.NonPublic | BindingFlags.Instance); + List items = (List)themeItemsField.GetValue(theme); + if (items == null) + { + items = new List(); + themeItemsField.SetValue(theme, items); + } + + items.Add(new Theme.ThemeItem("PrimaryColor", new ColorThemeItemData { Value = Color.red })); + items.Add(new Theme.ThemeItem("SecondaryColor", new ColorThemeItemData { Value = Color.blue })); + items.Add(new Theme.ThemeItem("IsEnabled", new BoolItemData { Value = true })); + items.Add(new Theme.ThemeItem("FontSize", new FloatItemData { Value = 14.5f })); + items.Add(new Theme.ThemeItem("ScaleVector", new Vector3ItemData { Value = Vector3.one * 2f })); + + for (int i = 5; i < totalItemCount; i++) + { + items.Add(new Theme.ThemeItem($"Item_{i}", new FloatItemData { Value = i * 1.5f })); + } + + return theme; + } + + [UnityTest] + public IEnumerator Theme_TryGetItemData_AccuracyAndTypes() + { + Theme theme = CreateTestTheme(20); + + // 1. Correct typed retrieval + Assert.IsTrue(theme.TryGetItemData("PrimaryColor", out ColorThemeItemData colorData)); + Assert.AreEqual(Color.red, colorData.Value); + + Assert.IsTrue(theme.TryGetItemData("IsEnabled", out BoolItemData boolData)); + Assert.IsTrue(boolData.Value); + + Assert.IsTrue(theme.TryGetItemData("FontSize", out FloatItemData floatData)); + Assert.AreEqual(14.5f, floatData.Value); + + Assert.IsTrue(theme.TryGetItemData("ScaleVector", out Vector3ItemData vecData)); + Assert.AreEqual(Vector3.one * 2f, vecData.Value); + + // 2. Type mismatch returns false + Assert.IsFalse(theme.TryGetItemData("PrimaryColor", out BoolItemData _)); + + // 3. Nonexistent item returns false + Assert.IsFalse(theme.TryGetItemData("NonExistentItem", out ColorThemeItemData _)); + + // 4. Null and empty keys return false without throwing + Assert.IsFalse(theme.TryGetItemData(null, out ColorThemeItemData _)); + Assert.IsFalse(theme.TryGetItemData(string.Empty, out ColorThemeItemData _)); + + Object.Destroy(theme); + yield return null; + } + + [UnityTest] + public IEnumerator Theme_TryGetItemData_BenchmarkAndAllocation() + { + const int totalItems = 100; + const int lookupIterations = 50000; + Theme theme = CreateTestTheme(totalItems); + + // Warmup + for (int i = 0; i < 500; i++) + { + theme.TryGetItemData("PrimaryColor", out ColorThemeItemData _); + theme.TryGetItemData("Item_50", out FloatItemData _); + theme.TryGetItemData("Item_99", out FloatItemData _); + } + + // Benchmark + Stopwatch sw = Stopwatch.StartNew(); + for (int i = 0; i < lookupIterations; i++) + { + theme.TryGetItemData("PrimaryColor", out ColorThemeItemData _); + theme.TryGetItemData("Item_50", out FloatItemData _); + theme.TryGetItemData("Item_99", out FloatItemData _); + theme.TryGetItemData("NonExistent", out FloatItemData _); + } + sw.Stop(); + long totalLookups = (long)lookupIterations * 4; + double msPerLookup = (double)sw.ElapsedTicks / Stopwatch.Frequency * 1000.0 / totalLookups; + Debug.Log($"[Theming Perf] {totalLookups} lookups in {sw.ElapsedMilliseconds} ms ({msPerLookup * 1000.0:F3} us/lookup) for a {totalItems}-item theme."); + + // Zero-GC verification for steady-state lookups + TestDelegate lookupAction = () => + { + for (int i = 0; i < 1000; i++) + { + theme.TryGetItemData("PrimaryColor", out ColorThemeItemData _); + theme.TryGetItemData("Item_50", out FloatItemData _); + theme.TryGetItemData("Item_99", out FloatItemData _); + theme.TryGetItemData("NonExistent", out FloatItemData _); + } + }; + lookupAction(); + Assert.That(lookupAction, Is.Not.AllocatingGCMemory()); + + Object.Destroy(theme); + yield return null; + } + + [UnityTest] + public IEnumerator Theme_CacheInvalidation_ReflectsUpdates() + { + Theme theme = CreateTestTheme(20); + + Assert.IsTrue(theme.TryGetItemData("Item_10", out FloatItemData initialData)); + Assert.AreEqual(15.0f, initialData.Value); + + // Mutate item value in list + FieldInfo themeItemsField = typeof(Theme).GetField("themeItems", BindingFlags.NonPublic | BindingFlags.Instance); + List items = (List)themeItemsField.GetValue(theme); + int idx = items.FindIndex(it => it.Name == "Item_10"); + items[idx] = new Theme.ThemeItem("Item_10", new FloatItemData { Value = 999.0f }); + + // Invalidate cache if method exists + MethodInfo invalidateMethod = typeof(Theme).GetMethod("InvalidateCache", BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance); + invalidateMethod?.Invoke(theme, null); + + Assert.IsTrue(theme.TryGetItemData("Item_10", out FloatItemData updatedData)); + Assert.AreEqual(999.0f, updatedData.Value); + + Object.Destroy(theme); + yield return null; + } + + [UnityTest] + public IEnumerator BaseThemeBinder_SubscribeUnsubscribe_AllocationTest() + { + ThemeDataSource dataSource = ScriptableObject.CreateInstance(); + Theme theme = CreateTestTheme(10); + dataSource.value = theme; + + GameObject go = new GameObject("TestBinderTarget"); + Behaviour dummyBehaviour = go.AddComponent(); + TestBinder binder = new TestBinder(); + binder.Initialize("IsEnabled", dummyBehaviour); + + IBinder iBinder = binder; + + // Warmup initial subscription + iBinder.Subscribe(dataSource); + iBinder.Unsubscribe(dataSource); + + TestDelegate subscribeCycle = () => + { + for (int i = 0; i < 50; i++) + { + iBinder.Subscribe(dataSource); + iBinder.Unsubscribe(dataSource); + } + }; + subscribeCycle(); + Assert.That(subscribeCycle, Is.Not.AllocatingGCMemory()); + + Object.Destroy(go); + Object.Destroy(theme); + Object.Destroy(dataSource); + yield return null; + } + + [UnityTest] + public IEnumerator BaseThemeBinder_ThemeChange_AppliesNewValue() + { + ThemeDataSource dataSource = ScriptableObject.CreateInstance(); + + Theme themeA = CreateTestTheme(10); + FieldInfo themeItemsField = typeof(Theme).GetField("themeItems", BindingFlags.NonPublic | BindingFlags.Instance); + List itemsA = (List)themeItemsField.GetValue(themeA); + int idxA = itemsA.FindIndex(it => it.Name == "IsEnabled"); + itemsA[idxA] = new Theme.ThemeItem("IsEnabled", new BoolItemData { Value = true }); + + Theme themeB = CreateTestTheme(10); + List itemsB = (List)themeItemsField.GetValue(themeB); + int idxB = itemsB.FindIndex(it => it.Name == "IsEnabled"); + itemsB[idxB] = new Theme.ThemeItem("IsEnabled", new BoolItemData { Value = false }); + + dataSource.value = themeA; + + GameObject go = new GameObject("TestBinderTarget"); + Behaviour dummyBehaviour = go.AddComponent(); + TestBinder binder = new TestBinder(); + binder.Initialize("IsEnabled", dummyBehaviour); + + IBinder iBinder = binder; + iBinder.Subscribe(dataSource); + + // AddListener should immediately trigger apply for activeTheme + Assert.AreEqual(1, binder.ApplyCount); + Assert.IsTrue(binder.AppliedValue); + + // Switch to themeB + dataSource.value = themeB; + Assert.AreEqual(2, binder.ApplyCount); + Assert.IsFalse(binder.AppliedValue); + + // Unsubscribe and switch back to themeA; binder should not receive it + iBinder.Unsubscribe(dataSource); + dataSource.value = themeA; + Assert.AreEqual(2, binder.ApplyCount); + Assert.IsFalse(binder.AppliedValue); + + Object.Destroy(go); + Object.Destroy(themeA); + Object.Destroy(themeB); + Object.Destroy(dataSource); + yield return null; + } + + [UnityTest] + public IEnumerator ThemeBinding_Lifecycle_ZeroGC() + { + ThemeDataSource dataSource = ScriptableObject.CreateInstance(); + Theme theme = CreateTestTheme(10); + dataSource.value = theme; + + GameObject go = new GameObject("ThemeBindingHolder"); + go.SetActive(false); + Behaviour dummyBehaviour = go.AddComponent(); + ThemeBinding themeBinding = go.AddComponent(); + + TestBinder binder = new TestBinder(); + binder.Initialize("IsEnabled", dummyBehaviour); + + FieldInfo dataSourceField = typeof(ThemeBinding).GetField("themeDataSource", BindingFlags.NonPublic | BindingFlags.Instance); + dataSourceField?.SetValue(themeBinding, dataSource); + + FieldInfo bindersField = typeof(ThemeBinding).GetField("binders", BindingFlags.NonPublic | BindingFlags.Instance); + bindersField?.SetValue(themeBinding, new IBinder[] { binder }); + + // Warmup initial enable/disable + go.SetActive(true); + go.SetActive(false); + + // Lifecycle Zero-GC test across repeated disable/enable + TestDelegate lifecycleCycle = () => + { + for (int i = 0; i < 20; i++) + { + go.SetActive(true); + go.SetActive(false); + } + }; + lifecycleCycle(); + Assert.That(lifecycleCycle, Is.Not.AllocatingGCMemory()); + + Object.Destroy(go); + Object.Destroy(theme); + Object.Destroy(dataSource); + yield return null; + } + } +} diff --git a/org.mixedrealitytoolkit.theming/Tests/Runtime/ThemingHotPathTests.cs.meta b/org.mixedrealitytoolkit.theming/Tests/Runtime/ThemingHotPathTests.cs.meta new file mode 100644 index 000000000..92634d00e --- /dev/null +++ b/org.mixedrealitytoolkit.theming/Tests/Runtime/ThemingHotPathTests.cs.meta @@ -0,0 +1,7 @@ +fileFormatVersion: 2 +guid: 917003354433477292d3cfee9873ea65 +TextScriptImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/org.mixedrealitytoolkit.theming/Theming/Theme.cs b/org.mixedrealitytoolkit.theming/Theming/Theme.cs index f369fcd3f..b555dad70 100644 --- a/org.mixedrealitytoolkit.theming/Theming/Theme.cs +++ b/org.mixedrealitytoolkit.theming/Theming/Theme.cs @@ -11,7 +11,7 @@ namespace MixedReality.Toolkit.Theming /// A ScriptableObject that provides concrete values for every item declared in a 's definition. /// [CreateAssetMenu(fileName = "Theme", menuName = "MRTK/Theming/Theme", order = 0)] - public class Theme : ScriptableObject + public class Theme : ScriptableObject, ISerializationCallbackReceiver { [SerializeField] [Tooltip("The schema that this theme conforms to. Every theme item must map to an item defined here.")] @@ -26,6 +26,58 @@ public class Theme : ScriptableObject [Tooltip("The items defining this theme's data mapped to the definition's items.")] private List themeItems = new List(); + [NonSerialized] + private Dictionary itemCache; + + private void EnsureCache() + { + if (itemCache != null) + { + return; + } + + if (themeItems == null) + { + itemCache = new Dictionary(0); + return; + } + + itemCache = new Dictionary(themeItems.Count); + for (int i = 0; i < themeItems.Count; i++) + { + ThemeItem item = themeItems[i]; + if (item != null && !string.IsNullOrEmpty(item.Name) && !itemCache.ContainsKey(item.Name)) + { + itemCache.Add(item.Name, item.Data); + } + } + } + + /// + /// Invalidates the internal theme item lookup cache. + /// + public void InvalidateCache() + { + itemCache = null; + } + + private void OnEnable() + { + InvalidateCache(); + } + + private void OnValidate() + { + InvalidateCache(); + } + + void ISerializationCallbackReceiver.OnBeforeSerialize() { } + + void ISerializationCallbackReceiver.OnAfterDeserialize() + { + InvalidateCache(); + } + /// /// Attempts to retrieve the data for a specific theme item by its name and expected type. /// @@ -35,15 +87,13 @@ public class Theme : ScriptableObject /// if an item with the specified name and type is found; otherwise, . public bool TryGetItemData(string itemName, out T itemValue) { - if (themeItems != null) + if (!string.IsNullOrEmpty(itemName)) { - foreach (var themeItem in themeItems) + EnsureCache(); + if (itemCache.TryGetValue(itemName, out object data) && data is T themeItemData) { - if (themeItem != null && themeItem.Name == itemName && themeItem.Data is T themeItemData) - { - itemValue = themeItemData; - return true; - } + itemValue = themeItemData; + return true; } } diff --git a/org.mixedrealitytoolkit.theming/Theming/ThemeDataSource.cs b/org.mixedrealitytoolkit.theming/Theming/ThemeDataSource.cs index e99ec94f4..757d7ba7f 100644 --- a/org.mixedrealitytoolkit.theming/Theming/ThemeDataSource.cs +++ b/org.mixedrealitytoolkit.theming/Theming/ThemeDataSource.cs @@ -1,6 +1,8 @@ // Copyright (c) Mixed Reality Toolkit Contributors // Licensed under the BSD 3-Clause +using System; +using System.Collections.Generic; using UnityEngine; using UnityEngine.Events; using UnityEngine.UIElements; @@ -21,6 +23,14 @@ public class ThemeDataSource : ScriptableObject, INotifyValueChanged public ThemeDefinition Definition => themeDefinition; + [NonSerialized] + private List>> dynamicListeners = new List>>(16); + + private void OnEnable() + { + dynamicListeners ??= new List>>(16); + } + #region INotifyValueChanged /// @@ -41,6 +51,14 @@ public Theme value // and object pooling SetValueWithoutNotify(value); onThemeChanged.Invoke(changeEvent); + + if (dynamicListeners != null) + { + for (int i = 0; i < dynamicListeners.Count; i++) + { + dynamicListeners[i]?.Invoke(changeEvent); + } + } } } } @@ -56,7 +74,11 @@ public void SetValueWithoutNotify(Theme newValue) public void AddListener(UnityAction> action) { if (action == null) { return; } - onThemeChanged.AddListener(action); + dynamicListeners ??= new List>>(16); + if (!dynamicListeners.Contains(action)) + { + dynamicListeners.Add(action); + } if (activeTheme == null) { return; } using ChangeEvent changeEvent = ChangeEvent.GetPooled(null, activeTheme); action.Invoke(changeEvent); @@ -64,8 +86,8 @@ public void AddListener(UnityAction> action) public void RemoveListener(UnityAction> action) { - if (action == null) { return; } - onThemeChanged.RemoveListener(action); + if (action == null || dynamicListeners == null) { return; } + dynamicListeners.Remove(action); } } } diff --git a/org.mixedrealitytoolkit.uxcore/CHANGELOG.md b/org.mixedrealitytoolkit.uxcore/CHANGELOG.md index 514019778..69e9e4c5d 100644 --- a/org.mixedrealitytoolkit.uxcore/CHANGELOG.md +++ b/org.mixedrealitytoolkit.uxcore/CHANGELOG.md @@ -2,6 +2,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). +## Unreleased + +### Changed + +* Optimized `ThemeBinding` lifecycle to pre-allocate list capacity and use indexed loops, eliminating heap allocations during repeated `OnEnable` and `OnDisable` cycles. [PR #1133](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1133) + ## [4.0.0-pre.4] - 2026-06-10 ### Added diff --git a/org.mixedrealitytoolkit.uxcore/Theming/ThemeBinding.cs b/org.mixedrealitytoolkit.uxcore/Theming/ThemeBinding.cs index b07482816..00485775a 100644 --- a/org.mixedrealitytoolkit.uxcore/Theming/ThemeBinding.cs +++ b/org.mixedrealitytoolkit.uxcore/Theming/ThemeBinding.cs @@ -35,8 +35,14 @@ protected void OnEnable() if (binders != null) { - foreach (IBinder binder in binders) + if (subscribedBinders.Capacity < binders.Length) { + subscribedBinders.Capacity = binders.Length; + } + + for (int i = 0; i < binders.Length; i++) + { + IBinder binder = binders[i]; if (binder == null) { Debug.LogWarning($"{nameof(ThemeBinding)} on '{gameObject.name}' has a null binder entry.", this); @@ -58,9 +64,9 @@ protected void OnDisable() { if (subscribedDataSource != null) { - foreach (IBinder binder in subscribedBinders) + for (int i = 0; i < subscribedBinders.Count; i++) { - binder?.Unsubscribe(subscribedDataSource); + subscribedBinders[i]?.Unsubscribe(subscribedDataSource); } subscribedBinders.Clear(); subscribedDataSource = null; diff --git a/org.mixedrealitytoolkit.uxcore/package.json b/org.mixedrealitytoolkit.uxcore/package.json index 95a6fa50a..22e1cfc76 100644 --- a/org.mixedrealitytoolkit.uxcore/package.json +++ b/org.mixedrealitytoolkit.uxcore/package.json @@ -1,6 +1,6 @@ { "name": "org.mixedrealitytoolkit.uxcore", - "version": "4.0.0-development.pre.4", + "version": "4.0.0-development.pre.5", "description": "Core interaction and visualization scripts for building MR UI components. Intended to be consumed when building UX libraries. For pre-existing library of components, see the UX Components package.", "displayName": "MRTK UX Core Scripts", "msftFeatureCategory": "MRTK3",