Improve fallback logic in PinchInputReader.cs

org.mixedrealitytoolkit.input/CHANGELOG.md

@@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 * Added toggle for frame rate independent smoothing in camera simulation. [PR #1011](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1011)
 * Added implementation for the synthesized TriggerButton, accounting for animation smoothing. [PR #1043](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1043)
 * Added a "squeeze" alias for the grip states, to account for broader input action mapping support. [PR #1043](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1043)
+* Added partial polyfill logic for one of "select" and "select value" being mapped while the other isn't, instead of fully polyfilling based on hand joint data when either is missing. [PR #1041](https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/pull/1041)
 
 ### Fixed
 

org.mixedrealitytoolkit.input/Readers/PinchInputReader.cs

@@ -29,7 +29,6 @@ public class PinchInputReader : MonoBehaviour, IXRInputButtonReader
         /// </summary>
         private struct FallbackState
         {
-            public bool hasPinchData;
             public bool isPerformed;
             public bool wasPerformedThisFrame;
             public bool wasCompletedThisFrame;
@@ -167,10 +166,8 @@ public bool ReadIsPerformed()
                 InputActionPhase phase = action.phase;
                 return phase == InputActionPhase.Performed || (phase != InputActionPhase.Disabled && action.WasPerformedThisFrame());
             }
-            else
-            {
-                return m_fallbackState.isPerformed;
-            }
+
+            return m_fallbackState.isPerformed;
         }
 
         /// <inheritdoc />
@@ -180,10 +177,8 @@ public bool ReadWasPerformedThisFrame()
             {
                 return selectAction.action.WasPerformedThisFrame();
             }
-            else
-            {
-                return m_fallbackState.wasPerformedThisFrame;
-            }
+
+            return m_fallbackState.wasPerformedThisFrame;
         }
 
         /// <inheritdoc />
@@ -193,10 +188,8 @@ public bool ReadWasCompletedThisFrame()
             {
                 return selectAction.action.WasCompletedThisFrame();
             }
-            else
-            {
-                return m_fallbackState.wasCompletedThisFrame;
-            }
+
+            return m_fallbackState.wasCompletedThisFrame;
         }
 
         /// <inheritdoc />
@@ -206,10 +199,8 @@ public float ReadValue()
             {
                 return selectActionValue.action.ReadValue<float>();
             }
-            else
-            {
-                return m_fallbackState.value;
-            }
+
+            return m_fallbackState.value;
         }
 
         /// <inheritdoc />
@@ -221,11 +212,9 @@ public bool TryReadValue(out float value)
                 value = action.ReadValue<float>();
                 return action.IsInProgress();
             }
-            else
-            {
-                value = m_fallbackState.value;
-                return m_fallbackState.hasPinchData;
-            }
+
+            value = m_fallbackState.value;
+            return value > 0;
         }
 
         #endregion IXRInputButtonReader
@@ -242,37 +231,49 @@ private void UpdatePinchSelection()
         {
             using (UpdatePinchSelectionPerfMarker.Auto())
             {
-                // If we still don't have an aggregator, then don't update selects.
-                if (XRSubsystemHelpers.HandsAggregator == null)
+                bool hasPinchData = false;
+                float pinchAmount = 0;
+                float pinchProgress = 0;
+
+                // Workaround for missing select actions on devices without interaction profiles
+                // for hands, such as Varjo and Quest. Should be removed once we have universal
+                // hand interaction profile(s) across vendors.
+                if (XRSubsystemHelpers.HandsAggregator != null
+                    && XRSubsystemHelpers.HandsAggregator.TryGetPinchProgress(handNode, out _, out _, out pinchProgress))
                 {
-                    return;
+                    hasPinchData |= true;
                 }
 
-                // If we got pinch data, write it into our select interaction state.
-                if (XRSubsystemHelpers.HandsAggregator.TryGetPinchProgress(
-                    handNode,
-                    out _,
-                    out _,
-                    out float pinchAmount))
+                // This section accounts for one of "select" and "select value" being bound while the other is polyfilled.
+                // We can use the data from the bound action to synthesize the other better than the hand joint logic will.
+                if (!m_isSelectPolyfilled && !m_isTrackingStatePolyfilled)
                 {
-                    // Workaround for missing select actions on devices without interaction profiles
-                    // for hands, such as Varjo and Quest. Should be removed once we have universal
-                    // hand interaction profile(s) across vendors.
-
-                    // Debounce the polyfill pinch action value.
-                    bool isPinched = pinchAmount >= (m_fallbackState.isPerformed ? 0.9f : 1.0f);
-
-                    m_fallbackState.wasPerformedThisFrame = isPinched && !m_fallbackState.isPerformed;
-                    m_fallbackState.wasCompletedThisFrame = !isPinched && m_fallbackState.isPerformed;
-                    m_fallbackState.isPerformed = isPinched;
-                    m_fallbackState.value = pinchAmount;
-                    m_fallbackState.hasPinchData = true;
+                    // If we successfully read hand joint data, we should use that instead of clamping to 0 or 1
+                    pinchAmount = pinchProgress > 0 ? pinchProgress : ReadIsPerformed() ? 1 : 0;
+                    hasPinchData |= true;
                 }
-                else
+                else if (!m_isSelectValuePolyfilled && !m_isTrackingStatePolyfilled)
+                {
+                    pinchAmount = ReadValue();
+                    hasPinchData |= true;
+                }
+
+                if (!hasPinchData)
                 {
                     // If we didn't get pinch data, reset the fallback state.
                     m_fallbackState = default;
-                }  
+                    return;
+                }
+
+                const float PinchDeactivateThreshold = 0.9f;
+                const float PinchActivateThreshold = 1.0f;
+
+                // Debounce the polyfill pinch action value.
+                bool isPinched = pinchAmount >= (m_fallbackState.isPerformed ? PinchDeactivateThreshold : PinchActivateThreshold);
+                m_fallbackState.wasPerformedThisFrame = isPinched && !m_fallbackState.isPerformed;
+                m_fallbackState.wasCompletedThisFrame = !isPinched && m_fallbackState.isPerformed;
+                m_fallbackState.isPerformed = isPinched;
+                m_fallbackState.value = pinchAmount;
             }
         }