From be4c833e191a62a17bb73c63a16f0553f2a7f42b Mon Sep 17 00:00:00 2001 From: poi-vrc <77053052+poi-vrc@users.noreply.github.com> Date: Tue, 7 May 2024 01:36:03 +0800 Subject: [PATCH] fix(parameter-slot): prevent flickering by switching states directly --- Editor/Animations/SmartControlComposer.cs | 115 +++++++++++++----- .../VRChat/ComposeSmartControlsPass.cs | 1 + .../Animations/SmartControlComposerTest.cs | 93 ++++++++++---- 3 files changed, 153 insertions(+), 56 deletions(-) diff --git a/Editor/Animations/SmartControlComposer.cs b/Editor/Animations/SmartControlComposer.cs index bde2a4b1..eb2e52b0 100644 --- a/Editor/Animations/SmartControlComposer.cs +++ b/Editor/Animations/SmartControlComposer.cs @@ -34,8 +34,24 @@ internal class SmartControlComposer private const string VRCPhysBoneStretchSuffix = "_Stretch"; private const string VRCPhysBoneSquishSuffix = "_Squish"; + private class ParameterSlotLayerInfo + { + public AnimatorLayerBuilder layer; + public AnimatorStateBuilder entryState; + public Dictionary scEnabledStates; + public Dictionary scPrepareDisabledStates; + + public ParameterSlotLayerInfo() + { + layer = null; + entryState = null; + scEnabledStates = new Dictionary(); + scPrepareDisabledStates = new Dictionary(); + } + } + private readonly HashSet _controls; - private readonly Dictionary> _parameterSlotLayers; + private readonly Dictionary _parameterSlotLayerInfos; private readonly AnimatorOptions _options; private readonly AnimatorController _controller; @@ -44,7 +60,7 @@ public SmartControlComposer(AnimatorOptions options, AnimatorController controll _options = options; _controller = controller; _controls = new HashSet(); - _parameterSlotLayers = new Dictionary>(); + _parameterSlotLayerInfos = new Dictionary(); } private void AddParameterConfig(Transform transform, string parameterName, float defaultValue, bool networkSynced, bool saved) @@ -105,11 +121,11 @@ private void HandleDriverMenuItem(DTSmartControl ctrl) } } - private Tuple PrepareOrGetParameterSlotLayer(DTParameterSlot parameterSlot) + private ParameterSlotLayerInfo PrepareOrGetParameterSlotLayer(DTParameterSlot parameterSlot) { - if (_parameterSlotLayers.ContainsKey(parameterSlot)) + if (_parameterSlotLayerInfos.ContainsKey(parameterSlot)) { - return _parameterSlotLayers[parameterSlot]; + return _parameterSlotLayerInfos[parameterSlot]; } // generate parameter name @@ -143,7 +159,11 @@ private Tuple PrepareOrGetParameterS animator.FloatParameter(parameterSlot.ParameterName, parameterSlot.ParameterDefaultValue); } - return _parameterSlotLayers[parameterSlot] = new Tuple(layer, entryState); + return _parameterSlotLayerInfos[parameterSlot] = new ParameterSlotLayerInfo() + { + layer = layer, + entryState = entryState + }; } private void HandleDriverParameterSlot(DTSmartControl ctrl) @@ -156,9 +176,9 @@ private void HandleDriverParameterSlot(DTSmartControl ctrl) } // prepare and get value slot anystate layer - var tuple = PrepareOrGetParameterSlotLayer(parameterSlot); - var layer = tuple.Item1; - var entryState = tuple.Item2; + var info = PrepareOrGetParameterSlotLayer(parameterSlot); + var layer = info.layer; + var entryState = info.entryState; // generate menu item if (ctrl.ParameterSlotConfig.GenerateMenuItem) @@ -195,17 +215,51 @@ private void HandleDriverParameterSlot(DTSmartControl ctrl) var enabledState = layer.NewState($"{ctrl.name} Enabled"); var prepareDisabledState = layer.NewState($"{ctrl.name} Prepare Disabled"); + info.scEnabledStates[ctrl] = enabledState; + info.scPrepareDisabledStates[ctrl] = prepareDisabledState; entryState.AddTransition(enabledState) .Equals(parameterSlot.ParameterName, ctrl.ParameterSlotConfig.MappedValue); enabledState.AddTransition(prepareDisabledState) .NotEquals(parameterSlot.ParameterName, ctrl.ParameterSlotConfig.MappedValue); - prepareDisabledState.AddTransition(entryState) - .NotEquals(parameterSlot.ParameterName, ctrl.ParameterSlotConfig.MappedValue); + // we add the condition to entry state in FillParameterSlotInterControlTransitions + // to prioritize switching between control states instead ComposeBinaryToggles(_controller, prepareDisabledState, enabledState, ctrl); } + private void FillParameterSlotInterControlTransitions() + { + // this fills conditions in each controls' state to allow + // switching to each other directly without going to entry state + foreach (var psKvp in _parameterSlotLayerInfos) + { + var parameterSlot = psKvp.Key; + var info = psKvp.Value; + foreach (var prepareDisabledKvp in info.scPrepareDisabledStates) + { + var ctrl = prepareDisabledKvp.Key; + var prepareDisabledState = prepareDisabledKvp.Value; + + foreach (var enabledKvp in info.scEnabledStates) + { + var anotherCtrl = enabledKvp.Key; + var enabledState = enabledKvp.Value; + if (anotherCtrl == ctrl) + { + continue; + } + prepareDisabledState.AddTransition(enabledState) + .Equals(parameterSlot.ParameterName, anotherCtrl.ParameterSlotConfig.MappedValue); + } + + // only if no conditions match, it falls back to entry state + prepareDisabledState.AddTransition(info.entryState) + .NotEquals(parameterSlot.ParameterName, ctrl.ParameterSlotConfig.MappedValue); + } + } + } + private string VRCPhysBoneDataSourceToParam(string prefix, DTSmartControl.SCVRCPhysBoneDriverConfig.DataSource source) { if (source == DTSmartControl.SCVRCPhysBoneDriverConfig.DataSource.Angle) @@ -340,6 +394,11 @@ public void Compose(DTSmartControl ctrl) // EditorUtility.SetDirty(ctrl.Controller); } + public void Finish() + { + FillParameterSlotInterControlTransitions(); + } + private void ComposeBinary(DTSmartControl ctrl, List conditionalParams) { if (HasCrossControlCycle(ctrl)) @@ -436,7 +495,7 @@ private static bool TryGetMaterialProperty(Renderer rr, string propertyName, out if (propertyName.StartsWith(MaterialsArrayPrefix)) { // array - var closingBracketIndex = propertyName.IndexOf("]"); + var closingBracketIndex = propertyName.IndexOf(']'); var index = -1; try { @@ -507,7 +566,7 @@ private static bool TryGetComponentProperty(Component comp, string propertyName, return true; } - private void ComposePropertyGroupToggles(AnimationClipBuilder disabledClip, AnimationClipBuilder enabledClip, DTSmartControl.PropertyGroup propGp) + private void ComposePropertyGroupToggles(AnimationClipBuilder prepareDisabledClip, AnimationClipBuilder enabledClip, DTSmartControl.PropertyGroup propGp) { // search through all objects and animate them var searchTrans = propGp.SelectionType == DTSmartControl.PropertyGroup.PropertySelectionType.AvatarWide ? _options.context.AvatarGameObject.transform : propGp.SearchTransform; @@ -534,24 +593,20 @@ private void ComposePropertyGroupToggles(AnimationClipBuilder disabledClip, Anim time = 0.0f, value = propVal.ValueObjectReference }}; - enabledClip.SetCurve(comp.transform, compType, propVal.Name, enabledFrames); - if (!_options.writeDefaults) - { - var disabledFrames = new ObjectReferenceKeyframe[] { new ObjectReferenceKeyframe() { + var disabledFrames = new ObjectReferenceKeyframe[] { new ObjectReferenceKeyframe() { time = 0.0f, value = obj }}; - disabledClip.SetCurve(comp.transform, compType, propVal.Name, disabledFrames); - } + enabledClip.SetCurve(comp.transform, compType, propVal.Name, enabledFrames); + // in write defaults mode, we want to keep the enabled frames in prepare disabled to prevent flickering in parameter slot + prepareDisabledClip.SetCurve(comp.transform, compType, propVal.Name, _options.writeDefaults ? enabledFrames : disabledFrames); } else if (type == typeof(float)) { var f = (float)originalValue; enabledClip.SetCurve(comp.transform, compType, propVal.Name, AnimationCurve.Constant(0.0f, 0.0f, propVal.Value)); - if (!_options.writeDefaults) - { - disabledClip.SetCurve(comp.transform, compType, propVal.Name, AnimationCurve.Constant(0.0f, 0.0f, f)); - } + // in write defaults mode, we want to keep the enabled value in prepare disabled to prevent flickering in parameter slot + prepareDisabledClip.SetCurve(comp.transform, compType, propVal.Name, AnimationCurve.Constant(0.0f, 0.0f, _options.writeDefaults ? propVal.Value : f)); } } } @@ -632,9 +687,9 @@ private void ComposeCrossControlValueActions(AnimatorController controller, Anim } #endif - private void ComposeBinaryToggles(AnimatorController controller, AnimatorStateBuilder disabledState, AnimatorStateBuilder enabledState, DTSmartControl ctrl) + private void ComposeBinaryToggles(AnimatorController controller, AnimatorStateBuilder prepareDisabledState, AnimatorStateBuilder enabledState, DTSmartControl ctrl) { - var disabledClip = disabledState.WithNewAnimation(); + var prepareDisabledClip = prepareDisabledState.WithNewAnimation(); var enabledClip = enabledState.WithNewAnimation(); foreach (var toggle in ctrl.ObjectToggles) @@ -645,20 +700,18 @@ private void ComposeBinaryToggles(AnimatorController controller, AnimatorStateBu } ClipToggle(enabledClip, toggle.Target, toggle.Enabled); - if (!_options.writeDefaults) - { - ClipToggle(disabledClip, toggle.Target, GetComponentOrGameObjectOriginalState(toggle.Target)); - } + // in write defaults mode, we want to keep the enabled value in prepare disabled to prevent flickering in parameter slot + ClipToggle(prepareDisabledClip, toggle.Target, _options.writeDefaults ? toggle.Enabled : GetComponentOrGameObjectOriginalState(toggle.Target)); } foreach (var propGp in ctrl.PropertyGroups) { - ComposePropertyGroupToggles(disabledClip, enabledClip, propGp); + ComposePropertyGroupToggles(prepareDisabledClip, enabledClip, propGp); } #if DT_VRCSDK3A // cross-controls are currently only available in VRC environments - ComposeCrossControlValueActions(controller, disabledState, ctrl.CrossControlActions.ValueActions.ValuesOnDisable); + ComposeCrossControlValueActions(controller, prepareDisabledState, ctrl.CrossControlActions.ValueActions.ValuesOnDisable); ComposeCrossControlValueActions(controller, enabledState, ctrl.CrossControlActions.ValueActions.ValuesOnEnable); #endif } diff --git a/Editor/Passes/Animations/VRChat/ComposeSmartControlsPass.cs b/Editor/Passes/Animations/VRChat/ComposeSmartControlsPass.cs index 8c30e6ff..4cfc55e7 100644 --- a/Editor/Passes/Animations/VRChat/ComposeSmartControlsPass.cs +++ b/Editor/Passes/Animations/VRChat/ComposeSmartControlsPass.cs @@ -98,6 +98,7 @@ public override bool Invoke(Context ctx) { composer.Compose(ctrl); } + composer.Finish(); EditorUtility.SetDirty(fx); diff --git a/Tests~/Editor/Animations/SmartControlComposerTest.cs b/Tests~/Editor/Animations/SmartControlComposerTest.cs index d0087519..0cf08904 100644 --- a/Tests~/Editor/Animations/SmartControlComposerTest.cs +++ b/Tests~/Editor/Animations/SmartControlComposerTest.cs @@ -105,7 +105,7 @@ public void MenuItemDriverTest() } } - private void AssertParameterSlotState(AnimatorController ac, AnimatorOptions options, DTParameterSlot slot, DTSmartControl sc) + private void AssertParameterSlotState(AnimatorController ac, AnimatorOptions options, DTParameterSlot slot, DTSmartControl sc, DTSmartControl another1, DTSmartControl another2) { var entryState = GetLayerState(0, ac, "Entry", options.writeDefaults); Assert.AreEqual(3, entryState.transitions.Length); @@ -128,14 +128,34 @@ private void AssertParameterSlotState(AnimatorController ac, AnimatorOptions opt ).Count() == 1); var prepareDisabledState = GetLayerState(0, ac, $"{sc.name} Prepare Disabled", options.writeDefaults); - Assert.AreEqual(1, prepareDisabledState.transitions.Length); + // expects to have 3 transitions: Entry, and the another two SC + Assert.AreEqual(3, prepareDisabledState.transitions.Length); + Assert.True(prepareDisabledState.transitions.Where(t => + t.conditions.Where(c => + c.parameter == slot.ParameterName && + c.mode == AnimatorConditionMode.Equals && + c.threshold == another1.ParameterSlotConfig.MappedValue + ).Count() == 1 + ).Count() == 1); Assert.True(prepareDisabledState.transitions.Where(t => + t.conditions.Where(c => + c.parameter == slot.ParameterName && + c.mode == AnimatorConditionMode.Equals && + c.threshold == another2.ParameterSlotConfig.MappedValue + ).Count() == 1 + ).Count() == 1); + var notEqualsTransitionQuery = prepareDisabledState.transitions.Where(t => t.conditions.Where(c => c.parameter == slot.ParameterName && c.mode == AnimatorConditionMode.NotEqual && c.threshold == sc.ParameterSlotConfig.MappedValue ).Count() == 1 - ).Count() == 1); + ); + Assert.True(notEqualsTransitionQuery.Count() == 1); + // expects the NotEquals transitions to be the last one + var notEqualsTransition = notEqualsTransitionQuery.FirstOrDefault(); + Assert.NotNull(notEqualsTransition); + Assert.AreEqual(notEqualsTransition, prepareDisabledState.transitions[prepareDisabledState.transitions.Length - 1]); } [Test] @@ -171,14 +191,15 @@ public void ParameterSlotDriverTest() composer.Compose(sc1); composer.Compose(sc2); composer.Compose(sc3); + composer.Finish(); Assert.False(sc1Go.TryGetComponent(out _)); Assert.True(sc2Go.TryGetComponent(out _)); Assert.False(sc3Go.TryGetComponent(out _)); - AssertParameterSlotState(ac, options, slot, sc1); - AssertParameterSlotState(ac, options, slot, sc2); - AssertParameterSlotState(ac, options, slot, sc3); + AssertParameterSlotState(ac, options, slot, sc1, sc2, sc3); + AssertParameterSlotState(ac, options, slot, sc2, sc1, sc3); + AssertParameterSlotState(ac, options, slot, sc3, sc1, sc2); } #if DT_VRCSDK3A @@ -196,6 +217,7 @@ public void VRCPhysBoneDriver_GeneratePrefixTest() ctrl.VRCPhysBoneDriverConfig.Source = DTSmartControl.SCVRCPhysBoneDriverConfig.DataSource.None; composer.Compose(ctrl); + composer.Finish(); Assert.True(string.IsNullOrEmpty(ctrl.AnimatorConfig.ParameterName)); Assert.False(string.IsNullOrEmpty(ctrl.VRCPhysBoneDriverConfig.ParameterPrefix)); @@ -217,6 +239,7 @@ public void VRCPhysBoneDriver_SrcNone_CondGrabbedOrPosedTest() ctrl.VRCPhysBoneDriverConfig.Source = DTSmartControl.SCVRCPhysBoneDriverConfig.DataSource.None; composer.Compose(ctrl); + composer.Finish(); var disabledState = GetLayerState(0, ac, "Disabled", options.writeDefaults); // two separate transitions with one condition @@ -257,6 +280,7 @@ public void VRCPhysBoneDriver_SrcAngle_CondNoneTest() ctrl.VRCPhysBoneDriverConfig.Source = DTSmartControl.SCVRCPhysBoneDriverConfig.DataSource.Angle; composer.Compose(ctrl); + composer.Finish(); var motionTimeState = GetLayerState(0, ac, "Motion Time", options.writeDefaults); Assert.AreEqual(motionTimeState.timeParameter, $"{pb.parameter}_Angle"); @@ -278,6 +302,7 @@ public void VRCPhysBoneDriver_SrcAngle_CondGrabbedOrPosedTest() ctrl.VRCPhysBoneDriverConfig.Source = DTSmartControl.SCVRCPhysBoneDriverConfig.DataSource.Angle; composer.Compose(ctrl); + composer.Finish(); var disabledState = GetLayerState(0, ac, "Disabled", options.writeDefaults); // two separate transitions with one condition @@ -311,6 +336,7 @@ public void AddParameterConfigTest() ctrl.AnimatorConfig.Saved = false; composer.Compose(ctrl); + composer.Finish(); Assert.True(ctrl.TryGetComponent(out var animParams)); var ap = animParams.Configs.Where(p => p.ParameterName == "MyParam").FirstOrDefault(); @@ -349,6 +375,12 @@ public bool HasTogglePropertyCurve(AnimationClip clip, string path, string pr return HasEditorCurve(clip, path, typeof(T), property, curve); } + private void AssertComposeBinaryTogglesEnabledClip(AnimationClip clip) + { + Assert.True(HasToggleComponentCurve(clip, "B", false)); + Assert.True(HasToggleComponentCurve(clip, "C", true)); + } + private void ComposeBinaryTogglesTest(bool writeDefaults) { SetupEnv(out var root, out var options, out var ac); @@ -369,6 +401,7 @@ private void ComposeBinaryTogglesTest(bool writeDefaults) var composer = new SmartControlComposer(options, ac); composer.Compose(ctrl); + composer.Finish(); var disabledClip = GetLayerStateClip(0, ac, "Disabled", writeDefaults); var enabledClip = GetLayerStateClip(0, ac, "Enabled", writeDefaults); @@ -378,15 +411,15 @@ private void ComposeBinaryTogglesTest(bool writeDefaults) if (writeDefaults) { - Assert.True(prepareDisabledClip.empty); + // expects the same as enabled clip + AssertComposeBinaryTogglesEnabledClip(prepareDisabledClip); } else { Assert.True(HasToggleComponentCurve(prepareDisabledClip, "B", true)); Assert.True(HasToggleComponentCurve(prepareDisabledClip, "C", true)); } - Assert.True(HasToggleComponentCurve(enabledClip, "B", false)); - Assert.True(HasToggleComponentCurve(enabledClip, "C", true)); + AssertComposeBinaryTogglesEnabledClip(enabledClip); } [Test] @@ -401,6 +434,25 @@ public void ComposeBinaryTogglesTest_WriteDefaultsOff() ComposeBinaryTogglesTest(false); } + private void AssertComposeBinaryPropertyEnabledClip(AnimationClip clip) + { + Assert.True(HasTogglePropertyCurve(clip, "B/Cube", "blendShape.Key1", MagicBlendshapeFloat)); + + Assert.True(HasTogglePropertyCurve(clip, "B/Cube", "blendShape.Key2", MagicBlendshapeFloat)); + Assert.True(HasTogglePropertyCurve(clip, "D/Cube", "blendShape.Key2", MagicBlendshapeFloat)); + // ignored object + Assert.False(HasTogglePropertyCurve(clip, "C/Cube", "blendShape.Key2", MagicBlendshapeFloat)); + + Assert.True(HasTogglePropertyCurve(clip, "B/Cube", "blendShape.Key3", MagicBlendshapeFloat)); + Assert.True(HasTogglePropertyCurve(clip, "C/Cube", "blendShape.Key3", MagicBlendshapeFloat)); + Assert.True(HasTogglePropertyCurve(clip, "D/Cube", "blendShape.Key3", MagicBlendshapeFloat)); + + Assert.True(HasTogglePropertyCurve(clip, "B/Cube", "material._Metallic", MagicFloat)); + Assert.True(HasTogglePropertyCurve(clip, "C/Cube", "material._Metallic", MagicFloat)); + // avatar-wide ignored + Assert.False(HasTogglePropertyCurve(clip, "D/Cube", "material._Metallic", MagicFloat)); + } + private void ComposeBinaryPropertyTest(bool writeDefaults) { SetupEnv(out var root, out var options, out var ac); @@ -442,6 +494,7 @@ private void ComposeBinaryPropertyTest(bool writeDefaults) var composer = new SmartControlComposer(options, ac); composer.Compose(ctrl); + composer.Finish(); var disabledClip = GetLayerStateClip(0, ac, "Disabled", writeDefaults); var enabledClip = GetLayerStateClip(0, ac, "Enabled", writeDefaults); @@ -451,7 +504,8 @@ private void ComposeBinaryPropertyTest(bool writeDefaults) if (writeDefaults) { - Assert.True(prepareDisabledClip.empty); + // expects the same as enabled clip + AssertComposeBinaryPropertyEnabledClip(prepareDisabledClip); } else { @@ -472,21 +526,7 @@ private void ComposeBinaryPropertyTest(bool writeDefaults) // avatar-wide ignored Assert.False(HasTogglePropertyCurve(prepareDisabledClip, "D/Cube", "material._Metallic", 0.65f)); } - Assert.True(HasTogglePropertyCurve(enabledClip, "B/Cube", "blendShape.Key1", MagicBlendshapeFloat)); - - Assert.True(HasTogglePropertyCurve(enabledClip, "B/Cube", "blendShape.Key2", MagicBlendshapeFloat)); - Assert.True(HasTogglePropertyCurve(enabledClip, "D/Cube", "blendShape.Key2", MagicBlendshapeFloat)); - // ignored object - Assert.False(HasTogglePropertyCurve(enabledClip, "C/Cube", "blendShape.Key2", MagicBlendshapeFloat)); - - Assert.True(HasTogglePropertyCurve(enabledClip, "B/Cube", "blendShape.Key3", MagicBlendshapeFloat)); - Assert.True(HasTogglePropertyCurve(enabledClip, "C/Cube", "blendShape.Key3", MagicBlendshapeFloat)); - Assert.True(HasTogglePropertyCurve(enabledClip, "D/Cube", "blendShape.Key3", MagicBlendshapeFloat)); - - Assert.True(HasTogglePropertyCurve(enabledClip, "B/Cube", "material._Metallic", MagicFloat)); - Assert.True(HasTogglePropertyCurve(enabledClip, "C/Cube", "material._Metallic", MagicFloat)); - // avatar-wide ignored - Assert.False(HasTogglePropertyCurve(enabledClip, "D/Cube", "material._Metallic", MagicFloat)); + AssertComposeBinaryPropertyEnabledClip(enabledClip); } [Test] @@ -531,6 +571,7 @@ private void ComposeMotionTimeTest(bool writeDefaults) var composer = new SmartControlComposer(options, ac); composer.Compose(ctrl); + composer.Finish(); var state = GetLayerState(0, ac, "Motion Time", writeDefaults); Assert.IsInstanceOf(typeof(AnimationClip), state.motion); @@ -575,6 +616,7 @@ public void CrossControlsCycleTest() var composer = new SmartControlComposer(options, ac); composer.Compose(ctrl1); composer.Compose(ctrl2); + composer.Finish(); Assert.True(options.context.Report.HasLogType(DressingFramework.Logging.LogType.Error)); } @@ -615,6 +657,7 @@ public void CrossControlsTest() var composer = new SmartControlComposer(options, ac); composer.Compose(ctrl1); composer.Compose(ctrl2); + composer.Finish(); var enabledState1 = GetLayerState(0, ac, "Enabled", options.writeDefaults); Assert.True(ContainsVRCAvatarParameterDriver(enabledState1, ctrl2.AnimatorConfig.ParameterName, 0.0f));