Skip to content

Remove CanvasSliderVisuals Update polling, refactor UpdateValue helper, and add tests #1144

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
keveleigh wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Remove CanvasSliderVisuals Update polling, refactor UpdateValue helper, and add tests #1144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions org.mixedrealitytoolkit.uxcore/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).

* Added `AlphaBlend` tint mode to `TintEffect`. [PR #1131](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1131)

### Changed

* Removed redundant `Update()` from `CanvasSliderVisuals` in favor of event-driven updates and `OnValidate()`. [PR #1144](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1144)

### Fixed

* Fixed "leaked managed shell" issue in `UGUIInputAdapter`. [PR #1096](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1096)
* Fixed "Attribute 'SerializeField' is not valid on this declaration type. It is only valid on 'field' declarations" error on `DialogButton` in Unity 6.3. [PR #1108](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1108)
* Fixed slider handle visuals not updating when slider value is modified from Inspector in play mode. [PR #1144](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1144)

## [3.3.0] - 2025-11-12

Expand Down
91 changes: 54 additions & 37 deletions org.mixedrealitytoolkit.uxcore/Slider/Slider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Mixed Reality Toolkit Contributors
// Copyright (c) Mixed Reality Toolkit Contributors
// Licensed under the BSD 3-Clause

using System;
Expand Down Expand Up @@ -103,14 +103,8 @@ public Collider TrackCollider
/// </summary>
public float MinValue
{
get
{
return minValue;
}
set
{
minValue = Mathf.Min(value, maxValue);
}
get => minValue;
set => minValue = Mathf.Min(value, maxValue);
}

[SerializeField]
Expand All @@ -122,14 +116,8 @@ public float MinValue
/// </summary>
public float MaxValue
{
get
{
return maxValue;
}
set
{
maxValue = Mathf.Max(minValue, value);
}
get => maxValue;
set => maxValue = Mathf.Max(minValue, value);
}

[VariableRange("minValue", "maxValue")]
Expand All @@ -149,16 +137,7 @@ public float MaxValue
public float Value
{
get => value;
set
{
if (this.value != value)
{
value = Mathf.Clamp(value, minValue, maxValue);
var oldSliderValue = this.value;
this.value = value;
OnValueUpdated.Invoke(new SliderEventData(oldSliderValue, value));
}
}
set => UpdateValue(value);
}

/// <summary>
Expand All @@ -176,7 +155,14 @@ public float Value
public bool UseSliderStepDivisions
{
get => useSliderStepDivisions;
set => useSliderStepDivisions = value;
set
{
useSliderStepDivisions = value;
if (useSliderStepDivisions)
{
UpdateValue(this.value);
}
}
}

[SerializeField]
Expand All @@ -190,7 +176,14 @@ public bool UseSliderStepDivisions
public int SliderStepDivisions
{
get => sliderStepDivisions;
set => sliderStepDivisions = value;
set
{
sliderStepDivisions = value;
if (useSliderStepDivisions)
{
UpdateValue(this.value);
}
}
}

[Header("Layout")]
Expand Down Expand Up @@ -240,6 +233,7 @@ public Transform SliderEnd
/// A Unity event that is invoked when <see cref="Value"/> changes.
/// </summary>
public SliderEvent OnValueUpdated => onValueUpdated;

#endregion

#region Private Fields
Expand Down Expand Up @@ -285,7 +279,7 @@ protected override void Reset()

/// <summary>
/// A Unity event function that is called on the frame when a script is enabled just before any of the update methods are called the first time.
/// </summary>
/// </summary>
protected virtual void Start()
{
// Turn on/off colliders at Start() to avoid bugs with
Expand Down Expand Up @@ -324,14 +318,15 @@ private void OnValidate()
// Ensure that the proper constraints are applied to the possible values of the slider
MinValue = minValue;
MaxValue = maxValue;
Value = value;
UpdateValue(value, forceUpdate: true);
}

#endregion

#region Protected Methods

/// <summary>
/// Invoked on <see cref="Start"/>, <see cref="Awake"/>, and <see cref="Reset"/> to apply required
/// Invoked on <see cref="Start"/>, <see cref="Awake"/>, and <see cref="Reset"/> to apply required
/// settings to this <see cref="Slider"/> instance.
/// </summary>
/// <remarks>
Expand All @@ -344,22 +339,45 @@ protected virtual void ApplyRequiredSettings()
// been acquired.
selectMode = InteractableSelectMode.Single;
}

#endregion Protected Methods

#region Private Methods
/// <summary>

/// <summary>
/// Private method used to adjust initial slider value to stepwise values
/// </summary>
private void InitializeStepDivisions()
{
Value = SnapSliderToStepPositions(Value);
UpdateValue(value);
}

private float ClampAndSnapValue(float rawValue)
{
float clamped = Mathf.Clamp(rawValue, minValue, maxValue);
if (useSliderStepDivisions)
{
clamped = SnapSliderToStepPositions(clamped);
}
return Mathf.Clamp(clamped, minValue, maxValue);
}

private void UpdateValue(float newValue, bool forceUpdate = false)
{
float clampedValue = ClampAndSnapValue(newValue);

if (forceUpdate || this.value != clampedValue)
{
var oldSliderValue = this.value;
this.value = clampedValue;
OnValueUpdated.Invoke(new SliderEventData(oldSliderValue, clampedValue));
}
}

private float SnapSliderToStepPositions(float value)
{
var stepCount = value / SliderStepVal;
var snappedValue = SliderStepVal * Mathf.RoundToInt(stepCount);
Mathf.Clamp(snappedValue, 0f, 1.0f);
return snappedValue;
}

Expand All @@ -373,8 +391,7 @@ private void UpdateSliderValue()
var normalizedStartValue = (StartSliderValue - MinValue) / (MaxValue - MinValue);
float normalizedValue = Mathf.Clamp(normalizedStartValue + handDelta / SliderTrackDirection.magnitude, 0f, 1.0f);

var unsnappedValue = normalizedValue * (MaxValue - MinValue) + MinValue;
Value = useSliderStepDivisions ? SnapSliderToStepPositions(unsnappedValue) : unsnappedValue;
UpdateValue(normalizedValue * (MaxValue - MinValue) + MinValue);
}

#endregion
Expand Down
106 changes: 71 additions & 35 deletions org.mixedrealitytoolkit.uxcore/Slider/Visuals/CanvasSliderVisuals.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,20 +130,28 @@ public Direction SliderDirection
/// </summary>
private void OnEnable()
{
sliderStart = SliderState.SliderStart.EnsureComponent<RectTransform>();
sliderEnd = SliderState.SliderEnd.EnsureComponent<RectTransform>();
touchableFitter = TrackArea.GetComponent<RectTransformColliderFitter>();
EnsureSliderEndpoints();

if (TrackArea != null)
{
touchableFitter = TrackArea.GetComponent<RectTransformColliderFitter>();
}

uguiInputAdapter = GetComponent<UGUIInputAdapter>();

if (sliderStart == null || sliderEnd == null)
{
Debug.LogError("Slider is missing start/end transforms.");
}

SliderState.OnValueUpdated.AddListener(UpdateHandle);
if (SliderState != null)
{
SliderState.OnValueUpdated.AddListener(UpdateHandle);

// Initial update. We may miss the first OnValueUpdated, depending on execution order.
UpdateHandle(SliderState.NormalizedValue);
}

// Initial update. We may miss the first OnValueUpdated, depending on execution order.
UpdateHandle(SliderState.NormalizedValue);
SetLayout(sliderDirection);
}

Expand All @@ -156,17 +164,22 @@ private void OnDisable()
}

#if UNITY_EDITOR

// Keep track of the last direction, so we can re-layout if needed.
private Direction prevDirection;

/// <summary>
/// A Unity event function that is called every frame, if this object is enabled.
/// A Unity Editor only event function that is called when the script is loaded or a value changes in the Unity Inspector.
/// </summary>
private void Update()
private void OnValidate()
{
// Only do this in edit mode, for performance.
if (Application.isPlaying)
if (trackArea == null || handle == null || fillVisual == null)
{
return;
}

EnsureSliderEndpoints();

if (sliderStart == null || sliderEnd == null)
{
return;
}
Expand All @@ -179,14 +192,17 @@ private void Update()
trackArea.anchoredPosition = Vector3.zero;

// Helper to apply layout if set from inspector.
if (SliderDirection != prevDirection)
if (sliderDirection != prevDirection)
{
SetLayout(SliderDirection);
prevDirection = SliderDirection;
SetLayout(sliderDirection);
prevDirection = sliderDirection;
}

// Update handle in editor view.
UpdateHandle(SliderState.NormalizedValue);
if (SliderState != null)
{
UpdateHandle(SliderState.NormalizedValue);
}
}
#endif // UNITY_EDITOR

Expand All @@ -199,6 +215,11 @@ void UpdateHandle(SliderEventData data)
// Update the things that depend on the slider value.
void UpdateHandle(float value)
{
if (handle == null || fillVisual == null)
{
return;
}

switch (SliderDirection)
{
case Direction.LeftToRight:
Expand All @@ -225,17 +246,47 @@ void UpdateHandle(float value)
handle.anchoredPosition = Vector3.zero;
}

private void EnsureSliderEndpoints()
{
if (sliderStart == null && SliderState != null && SliderState.SliderStart != null)
{
sliderStart = Application.isPlaying
? SliderState.SliderStart.EnsureComponent<RectTransform>()
: SliderState.SliderStart.GetComponent<RectTransform>();
}

if (sliderEnd == null && SliderState != null && SliderState.SliderEnd != null)
{
sliderEnd = Application.isPlaying
? SliderState.SliderEnd.EnsureComponent<RectTransform>()
: SliderState.SliderEnd.GetComponent<RectTransform>();
}
}

void SetLayout(Direction direction)
{
// Ensure sliderStart and sliderEnd are rooted at 0, we change the positions of these points via their anchors
sliderStart.anchoredPosition = Vector3.zero;
sliderEnd.anchoredPosition = Vector3.zero;

if (uguiInputAdapter == null)
{
uguiInputAdapter = GetComponent<UGUIInputAdapter>();
}

// Make sure UGUI understands which axis we can
// slide on. (Affects dpad/gamepad/etc)
if (uguiInputAdapter != null)
{
uguiInputAdapter.MovableAxes = (direction == Direction.LeftToRight || direction == Direction.RightToLeft)
? AxisFlags.XAxis
: AxisFlags.YAxis;
}

float trackWidth = Mathf.Max(trackArea.sizeDelta.x, trackArea.sizeDelta.y);
switch (direction)
{
case Direction.LeftToRight:

// Make sure UGUI understands which axis we can
// slide on. (Affects dpad/gamepad/etc)
uguiInputAdapter.MovableAxes = AxisFlags.XAxis;

sliderStart.anchorMin = new Vector2(0.0f, 0.5f);
sliderStart.anchorMax = new Vector2(0.0f, 0.5f);
sliderEnd.anchorMin = new Vector2(1.0f, 0.5f);
Expand All @@ -251,11 +302,6 @@ void SetLayout(Direction direction)
break;

case Direction.RightToLeft:

// Make sure UGUI understands which axis we can
// slide on. (Affects dpad/gamepad/etc)
uguiInputAdapter.MovableAxes = AxisFlags.XAxis;

sliderStart.anchorMin = new Vector2(1.0f, 0.5f);
sliderStart.anchorMax = new Vector2(1.0f, 0.5f);
sliderEnd.anchorMin = new Vector2(0.0f, 0.5f);
Expand All @@ -271,11 +317,6 @@ void SetLayout(Direction direction)
break;

case Direction.BottomToTop:

// Make sure UGUI understands which axis we can
// slide on. (Affects dpad/gamepad/etc)
uguiInputAdapter.MovableAxes = AxisFlags.YAxis;

sliderStart.anchorMin = new Vector2(0.5f, 0.0f);
sliderStart.anchorMax = new Vector2(0.5f, 0.0f);
sliderEnd.anchorMin = new Vector2(0.5f, 1.0f);
Expand All @@ -291,11 +332,6 @@ void SetLayout(Direction direction)
break;

case Direction.TopToBottom:

// Make sure UGUI understands which axis we can
// slide on. (Affects dpad/gamepad/etc)
uguiInputAdapter.MovableAxes = AxisFlags.YAxis;

sliderStart.anchorMin = new Vector2(0.5f, 1);
sliderStart.anchorMax = new Vector2(0.5f, 1);
sliderEnd.anchorMin = new Vector2(0.5f, 0);
Expand Down
Loading
Loading