Skip to content

Commit 3fba916

Browse files
dgoodnow-MLkeveleigh
dgoodnow-ML
and
keveleigh
authored
Fix direct-grab sliders getting stuck at maximum value (#945)
* Fixed for sliders with min-max values outside range \[0-1\] when slider is configured for grab interaction (IsTouchable = false) * Move tests to org.mixedrealitytoolkit.uxcomponents * Update SliderTests.cs * Update new tests to assemble on the fly instead of adding more prefabs --------- Signed-off-by: Kurtis <[email protected]> Co-authored-by: Kurtis <[email protected]> Co-authored-by: Kurtis <[email protected]>
1 parent 0ab1671 commit 3fba916

File tree

5 files changed

+219
-12
lines changed

5 files changed

+219
-12
lines changed

‎org.mixedrealitytoolkit.uxcomponents.noncanvas/Tests/Runtime/SliderTests.cs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ namespace MixedReality.Toolkit.UX.Runtime.Tests
1919
public class SliderTests : BaseRuntimeInputTests
2020
{
2121
// UXComponents/Sliders/Prefabs/NonCanvasSliderBase.prefab
22-
private const string defaultSliderPrefabGuid = "5cf5d5d5cc7fe184a93c388dbab66bb9";
23-
private static readonly string defaultSliderPrefabPath = AssetDatabase.GUIDToAssetPath(defaultSliderPrefabGuid);
24-
private const float sliderValueDelta = 0.02f;
22+
private const string DefaultSliderPrefabGuid = "5cf5d5d5cc7fe184a93c388dbab66bb9";
23+
private static readonly string DefaultSliderPrefabPath = AssetDatabase.GUIDToAssetPath(DefaultSliderPrefabGuid);
24+
private const float SliderValueDelta = 0.02f;
2525

2626
public override IEnumerator Setup()
2727
{
@@ -57,10 +57,10 @@ public IEnumerator TestAssembleInteractableAndNearManip()
5757
// This should not throw exception
5858
AssembleSlider(InputTestUtilities.InFrontOfUser(Vector3.forward), Vector3.zero, out GameObject sliderObject, out Slider slider, out _);
5959

60-
Assert.AreEqual(0.5f, slider.Value, sliderValueDelta, "Slider should have value 0.5 at start");
60+
Assert.AreEqual(0.5f, slider.Value, SliderValueDelta, "Slider should have value 0.5 at start");
6161
yield return DirectPinchAndMoveSlider(slider, 1.0f);
6262
// Allow some leeway due to grab positions shifting on open
63-
Assert.AreEqual(1.0f, slider.Value, sliderValueDelta, "Slider should have value 1.0 after being manipulated at start");
63+
Assert.AreEqual(1.0f, slider.Value, SliderValueDelta, "Slider should have value 1.0 after being manipulated at start");
6464

6565
//// clean up
6666
Object.Destroy(sliderObject);
@@ -75,10 +75,10 @@ public IEnumerator TestLoadPrefabAndNearManipNoSnap()
7575
slider.SnapToPosition = false;
7676
slider.IsTouchable = false;
7777

78-
Assert.AreEqual(0.5f, slider.Value, sliderValueDelta, "Slider should have value 0.5 at start");
78+
Assert.AreEqual(0.5f, slider.Value, SliderValueDelta, "Slider should have value 0.5 at start");
7979
yield return DirectPinchAndMoveSlider(slider, 1.0f);
8080
// Allow some leeway due to grab positions shifting on open
81-
Assert.AreEqual(1.0f, slider.Value, sliderValueDelta, "Slider should have value 1.0 after being manipulated at start");
81+
Assert.AreEqual(1.0f, slider.Value, SliderValueDelta, "Slider should have value 1.0 after being manipulated at start");
8282

8383
// clean up
8484
Object.Destroy(sliderObject);
@@ -435,7 +435,7 @@ public IEnumerator TestNullVisualsNoSnap()
435435

436436
// Test that the slider still works and no errors are thrown
437437
yield return RuntimeTestUtilities.WaitForUpdates();
438-
Assert.AreEqual(0.5f, slider.Value, sliderValueDelta, "Slider should have value 0.5 at start");
438+
Assert.AreEqual(0.5f, slider.Value, SliderValueDelta, "Slider should have value 0.5 at start");
439439

440440
// clean up
441441
Object.Destroy(sliderObject);
@@ -669,7 +669,7 @@ private void AssembleSlider(Vector3 position, Vector3 rotation, out GameObject s
669669
private void InstantiateDefaultSliderPrefab(Vector3 position, Vector3 rotation, out GameObject sliderObject, out Slider slider, out SliderVisuals sliderVisuals)
670670
{
671671
// Load interactable prefab
672-
Object sliderPrefab = AssetDatabase.LoadAssetAtPath(defaultSliderPrefabPath, typeof(Object));
672+
Object sliderPrefab = AssetDatabase.LoadAssetAtPath(DefaultSliderPrefabPath, typeof(Object));
673673
sliderObject = Object.Instantiate(sliderPrefab) as GameObject;
674674
slider = sliderObject.GetComponent<Slider>();
675675
sliderVisuals = sliderObject.GetComponent<SliderVisuals>();

‎org.mixedrealitytoolkit.uxcomponents/Tests/Runtime/SliderTests.cs‎

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
// Copyright (c) Mixed Reality Toolkit Contributors
2+
// Licensed under the BSD 3-Clause
3+
4+
// Disable "missing XML comment" warning for tests. While nice to have, this documentation is not required.
5+
#pragma warning disable CS1591
6+
7+
using MixedReality.Toolkit.Core.Tests;
8+
using MixedReality.Toolkit.Input;
9+
using MixedReality.Toolkit.Input.Tests;
10+
using NUnit.Framework;
11+
using System.Collections;
12+
using UnityEditor;
13+
using UnityEngine;
14+
using UnityEngine.TestTools;
15+
using UnityEngine.UI;
16+
using Object = UnityEngine.Object;
17+
18+
namespace MixedReality.Toolkit.UX.Runtime.Tests
19+
{
20+
public class SliderTests : BaseRuntimeInputTests
21+
{
22+
// Slider/CanvasSlider.prefab
23+
private const string DefaultSliderPrefabGuid = "f64620d502cdf0f429efa27703913cb7";
24+
private static readonly string DefaultSliderPrefabPath = AssetDatabase.GUIDToAssetPath(DefaultSliderPrefabGuid);
25+
26+
private const int HandMovementFrames = 10;
27+
28+
private TestHand hand;
29+
30+
[SetUp]
31+
public void SetUp()
32+
{
33+
hand = new TestHand(Handedness.Right);
34+
}
35+
36+
[UnityTest]
37+
public IEnumerator TouchSlider_MoveRight_ValueIncreasesCorrectly([ValueSource(nameof(MoveRightTestCases))] TestCase testCase)
38+
{
39+
InputTestUtilities.InitializeCameraToOriginAndForward();
40+
41+
var testPrefab = InstantiateSlider(DefaultSliderPrefabPath);
42+
testPrefab.transform.position = InputTestUtilities.InFrontOfUser(new Vector3(0, 0, 1));
43+
44+
var slider = testPrefab.GetComponentInChildren<Slider>();
45+
slider.MinValue = testCase.MinValue;
46+
slider.MaxValue = testCase.MaxValue;
47+
slider.Value = ((testCase.MaxValue - testCase.MinValue) / 2) + testCase.MinValue;
48+
49+
yield return ShowHand();
50+
yield return hand.MoveTo(testPrefab.transform.position - new Vector3(0, 0, 0.00f), HandMovementFrames);
51+
yield return RuntimeTestUtilities.WaitForUpdates();
52+
yield return hand.MoveTo(testPrefab.transform.position - new Vector3(-0.04f, 0f, 0.00f), HandMovementFrames);
53+
yield return RuntimeTestUtilities.WaitForUpdates();
54+
Assert.That(slider.Value, Is.EqualTo(testCase.Expected).Within(0.00001f));
55+
56+
Object.Destroy(testPrefab);
57+
}
58+
59+
[UnityTest]
60+
public IEnumerator GrabSlider_MoveRight_ValueIncreasesCorrectly([ValueSource(nameof(MoveRightTestCases))] TestCase testCase)
61+
{
62+
InputTestUtilities.InitializeCameraToOriginAndForward();
63+
64+
var testPrefab = InstantiateSlider(DefaultSliderPrefabPath);
65+
testPrefab.transform.position = InputTestUtilities.InFrontOfUser(new Vector3(0, 0, 1));
66+
67+
var slider = testPrefab.GetComponentInChildren<Slider>();
68+
slider.IsTouchable = false;
69+
slider.MinValue = testCase.MinValue;
70+
slider.MaxValue = testCase.MaxValue;
71+
slider.Value = ((testCase.MaxValue - testCase.MinValue) / 2) + testCase.MinValue;
72+
73+
yield return ShowHand();
74+
yield return hand.MoveTo(testPrefab.transform.position - new Vector3(0, 0, 0.00f), HandMovementFrames);
75+
yield return RuntimeTestUtilities.WaitForUpdates();
76+
yield return hand.SetHandshape(HandshapeTypes.HandshapeId.Grab);
77+
yield return RuntimeTestUtilities.WaitForUpdates();
78+
yield return hand.MoveTo(testPrefab.transform.position - new Vector3(-0.04f, 0f, 0.00f), HandMovementFrames);
79+
yield return RuntimeTestUtilities.WaitForUpdates();
80+
yield return hand.SetHandshape(HandshapeTypes.HandshapeId.Open);
81+
yield return RuntimeTestUtilities.WaitForUpdates();
82+
83+
Assert.That(slider.Value, Is.EqualTo(testCase.Expected).Within(0.00001f));
84+
85+
Object.Destroy(testPrefab);
86+
}
87+
[UnityTest]
88+
public IEnumerator TouchSlider_MoveLeft_ValueIncreasesCorrectly([ValueSource(nameof(MoveLeftTestCases))] TestCase testCase)
89+
{
90+
InputTestUtilities.InitializeCameraToOriginAndForward();
91+
92+
var testPrefab = InstantiateSlider(DefaultSliderPrefabPath);
93+
testPrefab.transform.position = InputTestUtilities.InFrontOfUser(new Vector3(0, 0, 1));
94+
95+
var slider = testPrefab.GetComponentInChildren<Slider>();
96+
slider.MinValue = testCase.MinValue;
97+
slider.MaxValue = testCase.MaxValue;
98+
slider.Value = ((testCase.MaxValue - testCase.MinValue) / 2) + testCase.MinValue;
99+
100+
yield return ShowHand();
101+
yield return hand.MoveTo(testPrefab.transform.position - new Vector3(0, 0, 0.00f), HandMovementFrames);
102+
yield return RuntimeTestUtilities.WaitForUpdates();
103+
yield return hand.MoveTo(testPrefab.transform.position + new Vector3(-0.04f, 0f, 0.00f), HandMovementFrames);
104+
yield return RuntimeTestUtilities.WaitForUpdates();
105+
Assert.That(slider.Value, Is.EqualTo(testCase.Expected).Within(0.00001f));
106+
107+
Object.Destroy(testPrefab);
108+
}
109+
110+
[UnityTest]
111+
public IEnumerator GrabSlider_MoveLeft_ValueIncreasesCorrectly([ValueSource(nameof(MoveLeftTestCases))] TestCase testCase)
112+
{
113+
InputTestUtilities.InitializeCameraToOriginAndForward();
114+
115+
var testPrefab = InstantiateSlider(DefaultSliderPrefabPath);
116+
testPrefab.transform.position = InputTestUtilities.InFrontOfUser(new Vector3(0, 0, 1));
117+
118+
var slider = testPrefab.GetComponentInChildren<Slider>();
119+
slider.IsTouchable = false;
120+
slider.MinValue = testCase.MinValue;
121+
slider.MaxValue = testCase.MaxValue;
122+
slider.Value = ((testCase.MaxValue - testCase.MinValue) / 2) + testCase.MinValue;
123+
124+
yield return ShowHand();
125+
yield return hand.MoveTo(testPrefab.transform.position - new Vector3(0, 0, 0.00f), HandMovementFrames);
126+
yield return RuntimeTestUtilities.WaitForUpdates();
127+
yield return hand.SetHandshape(HandshapeTypes.HandshapeId.Grab);
128+
yield return RuntimeTestUtilities.WaitForUpdates();
129+
yield return hand.MoveTo(testPrefab.transform.position + new Vector3(-0.04f, 0f, 0.00f), HandMovementFrames);
130+
yield return RuntimeTestUtilities.WaitForUpdates();
131+
yield return hand.SetHandshape(HandshapeTypes.HandshapeId.Open);
132+
yield return RuntimeTestUtilities.WaitForUpdates();
133+
134+
Assert.That(slider.Value, Is.EqualTo(testCase.Expected).Within(0.00001f));
135+
136+
Object.Destroy(testPrefab);
137+
}
138+
139+
public readonly struct TestCase
140+
{
141+
public float MinValue { get; }
142+
public float MaxValue { get; }
143+
public float Expected { get; }
144+
145+
public TestCase(float minValue, float maxValue, float expected)
146+
{
147+
MinValue = minValue;
148+
MaxValue = maxValue;
149+
Expected = expected;
150+
}
151+
}
152+
153+
private static IEnumerable MoveRightTestCases()
154+
{
155+
yield return new TestCase(minValue: 0, maxValue: 1, expected: 0.7f);
156+
yield return new TestCase(minValue: 0, maxValue: 10, expected: 7);
157+
yield return new TestCase(minValue: 0, maxValue: 0.1f, expected: 0.07f);
158+
yield return new TestCase(minValue: -1, maxValue: 1, expected: 0.4f);
159+
yield return new TestCase(minValue: -1, maxValue: 0, expected: -0.3f);
160+
}
161+
162+
private static IEnumerable MoveLeftTestCases()
163+
{
164+
yield return new TestCase(minValue: 0, maxValue: 1, expected: 0.3f);
165+
yield return new TestCase(minValue: 0, maxValue: 10, expected: 3);
166+
yield return new TestCase(minValue: 0, maxValue: 0.1f, expected: 0.03f);
167+
yield return new TestCase(minValue: -1, maxValue: 1, expected: -0.4f);
168+
yield return new TestCase(minValue: -1, maxValue: 0, expected: -0.7f);
169+
}
170+
171+
private IEnumerator ShowHand()
172+
{
173+
Vector3 initialHandPosition = InputTestUtilities.InFrontOfUser(new Vector3(0.05f, -0.05f, 0.3f));
174+
yield return hand.Show(initialHandPosition);
175+
yield return RuntimeTestUtilities.WaitForUpdates();
176+
}
177+
178+
private GameObject InstantiateSlider(string prefabPath)
179+
{
180+
GameObject canvas = new GameObject("SliderParent", typeof(RectTransform), typeof(Canvas), typeof(CanvasScaler));
181+
canvas.transform.localScale = Vector3.one * 0.001f;
182+
(canvas.transform as RectTransform).sizeDelta = Vector2.one * 200;
183+
GameObject slider = Object.Instantiate(AssetDatabase.LoadAssetAtPath<GameObject>(prefabPath));
184+
slider.transform.SetParent(canvas.transform, worldPositionStays: false);
185+
slider.transform.position = Vector3.zero;
186+
if (slider.transform is RectTransform rectTransform)
187+
{
188+
rectTransform.sizeDelta = new Vector2(200f, rectTransform.sizeDelta.y);
189+
}
190+
return canvas;
191+
}
192+
}
193+
}
194+
#pragma warning restore CS1591

‎org.mixedrealitytoolkit.uxcomponents/Tests/Runtime/SliderTests.cs.meta‎

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎org.mixedrealitytoolkit.uxcore/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
1515
* Prevent simultaneous editing of multiple input fields when using Non-Native Keyboard. [PR #942](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/942)
1616
* Don't try to start a coroutine in VirtualizedScrollRectList when the GameObject is inactive. [PR #972](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/972)
1717
* Fix SliderSounds playing sound even when disabled. [PR #1007](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1007)
18+
* Fix for sliders with min-max values outside range \[0-1\] when slider is configured for grab interaction (IsTouchable = false) [Issue 944](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/issues/944)
1819

1920
## [3.2.2] - 2024-09-18
2021

‎org.mixedrealitytoolkit.uxcore/Slider/Slider.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,8 @@ private void UpdateSliderValue()
370370

371371
var handDelta = Vector3.Dot(SliderTrackDirection.normalized, interactorDelta);
372372

373-
float normalizedValue = Mathf.Clamp(StartSliderValue + handDelta / SliderTrackDirection.magnitude, 0f, 1.0f);
373+
var normalizedStartValue = (StartSliderValue - MinValue) / (MaxValue - MinValue);
374+
float normalizedValue = Mathf.Clamp(normalizedStartValue + handDelta / SliderTrackDirection.magnitude, 0f, 1.0f);
374375

375376
var unsnappedValue = normalizedValue * (MaxValue - MinValue) + MinValue;
376377
Value = useSliderStepDivisions ? SnapSliderToStepPositions(unsnappedValue) : unsnappedValue;
@@ -386,12 +387,12 @@ protected override void OnSelectEntered(SelectEnterEventArgs args)
386387
base.OnSelectEntered(args);
387388

388389
// Snap to position by setting the startPosition
389-
// to the slider start, and start value to zero.
390+
// to the slider start, and start value to MinValue.
390391
// However, don't snap when using grabs.
391392
if (snapToPosition && !(args.interactorObject is IGrabInteractor))
392393
{
393394
StartInteractionPoint = SliderStart.position;
394-
StartSliderValue = 0.0f;
395+
StartSliderValue = MinValue;
395396
}
396397
else
397398
{

0 commit comments

Comments
 (0)