Skip to content

Commit cceab53

Browse files
author
Rene Damm
authored
FIX: InvalidOperationExceptions with InputUser and touch input (case 1196522, #965).
1 parent 2371d05 commit cceab53

File tree

18 files changed

+294
-34
lines changed

18 files changed

+294
-34
lines changed

Assets/Tests/InputSystem/CoreTests_Devices.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -981,6 +981,11 @@ public void OnStateEvent(InputEventPtr eventPtr)
981981
{
982982
InputState.Change(this, eventPtr);
983983
}
984+
985+
public bool GetStateOffsetForEvent(InputControl control, InputEventPtr eventPtr, ref uint offset)
986+
{
987+
return false;
988+
}
984989
}
985990

986991
[Test]
@@ -1074,6 +1079,11 @@ public unsafe void OnStateEvent(InputEventPtr eventPtr)
10741079

10751080
Assert.Fail();
10761081
}
1082+
1083+
public bool GetStateOffsetForEvent(InputControl control, InputEventPtr eventPtr, ref uint offset)
1084+
{
1085+
return false;
1086+
}
10771087
}
10781088

10791089
[Test]

Assets/Tests/InputSystem/CoreTests_Events.cs

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -918,7 +918,7 @@ public void Events_CanSendLargerStateToDeviceWithSmallerState()
918918

919919
[Test]
920920
[Category("Events")]
921-
public unsafe void Events_CanDetectWhetherControlIsPartOfEvent()
921+
public unsafe void Events_CanGetStatePointerFromEventThroughControl()
922922
{
923923
// We use a mouse here as it has several controls that are "parked" outside MouseState.
924924
var mouse = InputSystem.AddDevice<Mouse>();
@@ -948,12 +948,39 @@ public unsafe void Events_CanDetectWhetherControlIsPartOfEvent()
948948
InputSystem.Update();
949949
}
950950

951+
// For devices that implement IStateCallbackReceiver (such as Touchscreen), things get tricky. We may be looking
952+
// at an event in a state format different from that of the device and with only the device knowing how it
953+
// correlates to the state of an individual control.
954+
[Test]
955+
[Category("Events")]
956+
public unsafe void Events_CanGetStatePointerFromEventThroughControl_EvenIfDeviceIsStateCallbackReceiver()
957+
{
958+
var touchscreen = InputSystem.AddDevice<Touchscreen>();
959+
960+
using (var trace = new InputEventTrace { deviceId = touchscreen.deviceId })
961+
{
962+
trace.Enable();
963+
964+
BeginTouch(1, new Vector2(123, 234));
965+
966+
var statePtr = touchscreen.primaryTouch.position.GetStatePtrFromStateEvent(trace.ToArray()[0]);
967+
Assert.That(statePtr != null);
968+
969+
// Attempt reading the position value from the touch event.
970+
Assert.That(touchscreen.primaryTouch.position.ReadValueFromState(statePtr),
971+
Is.EqualTo(new Vector2(123, 234)).Using(Vector2EqualityComparer.Instance));
972+
973+
// It only works with primaryTouch. See Touchscreen.GetStateOffsetForEvent for details.
974+
Assert.That(touchscreen.touches[1].position.GetStatePtrFromStateEvent(trace.ToArray()[0]) == null);
975+
}
976+
}
977+
951978
[Test]
952979
[Category("Events")]
953980
public void Events_CanListenForWhenAllEventsHaveBeenProcessed()
954981
{
955982
var receivedCalls = 0;
956-
Action callback = () => ++ receivedCalls;
983+
void callback() => ++ receivedCalls;
957984

958985
InputSystem.onAfterUpdate += callback;
959986

Assets/Tests/InputSystem/Plugins/UserTests.cs

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
using UnityEngine.InputSystem.Controls;
77
using UnityEngine.InputSystem.LowLevel;
88
using UnityEngine.InputSystem.Users;
9-
using UnityEngine.TestTools.Utils;
109
using Gyroscope = UnityEngine.InputSystem.Gyroscope;
1110

1211
[SuppressMessage("ReSharper", "CheckNamespace")]
@@ -853,6 +852,36 @@ public void Users_CanDetectUseOfUnpairedDevice()
853852
Assert.That(receivedControls, Has.Exactly(1).SameAs(gamepad.aButton));
854853
}
855854

855+
// Touchscreens, because of the unusual TouchState events they receive, are trickier to handle than other
856+
// types of devices. Make use that InputUser.listenForUnpairedDeviceActivity doesn't choke on such events.
857+
// (case 1196522)
858+
[Test]
859+
[Category("Users")]
860+
public void Users_CanDetectUseOfUnpairedDevice_WhenDeviceIsTouchscreen()
861+
{
862+
var touchscreen = InputSystem.AddDevice<Touchscreen>();
863+
864+
var activityWasDetected = false;
865+
866+
++InputUser.listenForUnpairedDeviceActivity;
867+
InputUser.onUnpairedDeviceUsed +=
868+
(control, eventPtr) =>
869+
{
870+
// Because of Touchscreen's state trickery, there's no saying which actual TouchControl
871+
// the event is for until Touchscreen has actually gone and done its thing and processed
872+
// the event. So, all we can say here is that 'control' should be part of any of the
873+
// TouchControls on our Touchscreen.
874+
Assert.That(control.FindInParentChain<TouchControl>(), Is.Not.Null);
875+
Assert.That(control.device, Is.SameAs(touchscreen));
876+
877+
activityWasDetected = true;
878+
};
879+
880+
BeginTouch(1, new Vector2(123, 234));
881+
882+
Assert.That(activityWasDetected);
883+
}
884+
856885
// Make sure that if we pair a device from InputUser.onUnpairedDeviceUsed, we don't get any further
857886
// callbacks.
858887
[Test]

Packages/com.unity.inputsystem/CHANGELOG.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
77
Due to package verification, the latest version below is the unpublished version and the date is meaningless.
88
however, it has to be formatted properly to pass verification tests.
99

10+
## [1.0.0-preview.4] - 2019-12-12
11+
12+
### Changed
13+
14+
- `InputControlExtensions.GetStatePtrFromStateEvent` no longer throws `InvalidOperationException` when the state format for the event does not match that of the device. It simply returns `null` instead (same as when control is found in the event's state).
15+
16+
### Fixed
17+
18+
- `InputUser` in combination with touchscreens no longer throws `InvalidOperationException` complaining about incorrect state format.
19+
* In a related change, `InputControlExtensions.GetStatePtrFromStateEvent` now works with touch events, too.
20+
1021
## [1.0.0-preview.3] - 2019-11-14
1122

1223
### Fixed

Packages/com.unity.inputsystem/InputSystem/Actions/InputAction.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
using System;
2-
using Unity.Collections.LowLevel.Unsafe;
32
using UnityEngine.InputSystem.LowLevel;
43
using UnityEngine.InputSystem.Utilities;
54
using UnityEngine.Serialization;
65

6+
////TODO: add way to retrieve the binding correspond to a control
7+
78
////TODO: add way to retrieve the currently ongoing interaction and also add way to know how long it's been going on
89

910
////FIXME: Whether a control from a binding that's part of a composite appears on an action is currently not consistently enforced.

Packages/com.unity.inputsystem/InputSystem/Controls/InputControlExtensions.cs

Lines changed: 80 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,29 @@ namespace UnityEngine.InputSystem
1515
/// </summary>
1616
public static class InputControlExtensions
1717
{
18+
/// <summary>
19+
/// Find a control of the given type in control hierarchy of <paramref name="control"/>.
20+
/// </summary>
21+
/// <param name="control">Control whose parents to inspect.</param>
22+
/// <typeparam name="TControl">Type of control to look for. Actual control type can be
23+
/// subtype of this.</typeparam>
24+
/// <remarks>The found control of type <typeparamref name="TControl"/> which may be either
25+
/// <paramref name="control"/> itself or one of its parents. If no such control was found,
26+
/// returns <c>null</c>.</remarks>
27+
/// <exception cref="ArgumentNullException"><paramref name="control"/> is <c>null</c>.</exception>
28+
public static TControl FindInParentChain<TControl>(this InputControl control)
29+
where TControl : InputControl
30+
{
31+
if (control == null)
32+
throw new ArgumentNullException(nameof(control));
33+
34+
for (var parent = control; parent != null; parent = parent.parent)
35+
if (parent is TControl parentOfType)
36+
return parentOfType;
37+
38+
return null;
39+
}
40+
1841
/// <summary>
1942
/// Return true if the given control is actuated.
2043
/// </summary>
@@ -113,16 +136,38 @@ public static unsafe object ReadDefaultValueAsObject(this InputControl control)
113136
}
114137

115138
/// <summary>
116-
/// Read the value of the given control from an event.
139+
/// Read the value for the given control from the given event.
117140
/// </summary>
118-
/// <param name="control"></param>
119-
/// <param name="inputEvent">Input event. This must be a <see cref="StateEvent"/> or <seealso cref="DeltaStateEvent"/>.
141+
/// <param name="control">Control to read value for.</param>
142+
/// <param name="inputEvent">Event to read value from. Must be a <see cref="StateEvent"/> or <see cref="DeltaStateEvent"/>.</param>
143+
/// <typeparam name="TValue">Type of value to read.</typeparam>
144+
/// <exception cref="ArgumentNullException"><paramref name="control"/> is <c>null</c>.</exception>
145+
/// <exception cref="ArgumentException"><paramref name="inputEvent"/> is not a <see cref="StateEvent"/> or <see cref="DeltaStateEvent"/>.</exception>
146+
/// <returns>The value for the given control as read out from the given event or <c>default(TValue)</c> if the given
147+
/// event does not contain a value for the control (e.g. if it is a <see cref="DeltaStateEvent"/> not containing the relevant
148+
/// portion of the device's state memory).</returns>
149+
public static TValue ReadValueFromEvent<TValue>(this InputControl<TValue> control, InputEventPtr inputEvent)
150+
where TValue : struct
151+
{
152+
if (control == null)
153+
throw new ArgumentNullException(nameof(control));
154+
if (!ReadValueFromEvent(control, inputEvent, out var value))
155+
return default;
156+
return value;
157+
}
158+
159+
/// <summary>
160+
/// Check if the given event contains a value for the given control and if so, read the value.
161+
/// </summary>
162+
/// <param name="control">Control to read value for.</param>
163+
/// <param name="inputEvent">Input event. This must be a <see cref="StateEvent"/> or <see cref="DeltaStateEvent"/>.
120164
/// Note that in the case of a <see cref="DeltaStateEvent"/>, the control may not actually be part of the event. In this
121165
/// case, the method returns false and stores <c>default(TValue)</c> in <paramref name="value"/>.</param>
122166
/// <param name="value">Variable that receives the control value.</param>
123-
/// <typeparam name="TValue"></typeparam>
167+
/// <typeparam name="TValue">Type of value to read.</typeparam>
124168
/// <returns>True if the value has been successfully read from the event, false otherwise.</returns>
125-
/// <exception cref="ArgumentNullException"><paramref name="control"/> is null.</exception>
169+
/// <exception cref="ArgumentNullException"><paramref name="control"/> is <c>null</c>.</exception>
170+
/// <exception cref="ArgumentException"><paramref name="inputEvent"/> is not a <see cref="StateEvent"/> or <see cref="DeltaStateEvent"/>.</exception>
126171
/// <seealso cref="ReadUnprocessedValueFromEvent{TValue}(InputControl{TValue},InputEventPtr)"/>
127172
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1021:AvoidOutParameters", MessageId = "2#")]
128173
public static unsafe bool ReadValueFromEvent<TValue>(this InputControl<TValue> control, InputEventPtr inputEvent, out TValue value)
@@ -457,6 +502,26 @@ public static unsafe bool HasValueChangeInEvent(this InputControl control, Input
457502
return control.CompareValue(control.currentStatePtr, control.GetStatePtrFromStateEvent(eventPtr));
458503
}
459504

505+
/// <summary>
506+
/// Given a <see cref="StateEvent"/> or <see cref="DeltaStateEvent"/>, return the raw memory pointer that can
507+
/// be used, for example, with <see cref="InputControl{T}.ReadValueFromState"/> to read out the value of <paramref name="control"/>
508+
/// contained in the event.
509+
/// </summary>
510+
/// <param name="control">Control to access state for in the given state event.</param>
511+
/// <param name="eventPtr">A <see cref="StateEvent"/> or <see cref="DeltaStateEvent"/> containing input state.</param>
512+
/// <returns>A pointer that can be used with methods such as <see cref="InputControl{TValue}.ReadValueFromState"/> or <c>null</c>
513+
/// if <paramref name="eventPtr"/> does not contain state for the given <paramref name="control"/>.</returns>
514+
/// <exception cref="ArgumentNullException"><paramref name="control"/> is <c>null</c> -or- <paramref name="eventPtr"/> is invalid.</exception>
515+
/// <exception cref="ArgumentException"><paramref name="eventPtr"/> is not a <see cref="StateEvent"/> or <see cref="DeltaStateEvent"/>.</exception>
516+
/// <remarks>
517+
/// Note that the given state event must have the same state format (see <see cref="InputStateBlock.format"/>) as the device
518+
/// to which <paramref name="control"/> belongs. If this is not the case, the method will invariably return <c>null</c>.
519+
///
520+
/// In practice, this means that the method cannot be used with touch events or, in general, with events sent to devices
521+
/// that implement <see cref="IInputStateCallbackReceiver"/> (which <see cref="Touchscreen"/> does) except if the event
522+
/// is in the same state format as the device. Touch events will generally be sent as state events containing only the
523+
/// state of a single <see cref="Controls.TouchControl"/>, not the state of the entire <see cref="Touchscreen"/>.
524+
/// </remarks>
460525
public static unsafe void* GetStatePtrFromStateEvent(this InputControl control, InputEventPtr eventPtr)
461526
{
462527
if (control == null)
@@ -489,7 +554,7 @@ public static unsafe bool HasValueChangeInEvent(this InputControl control, Input
489554
}
490555
else
491556
{
492-
throw new ArgumentException("Event must be a state or delta state event", "eventPtr");
557+
throw new ArgumentException("Event must be a state or delta state event", nameof(eventPtr));
493558
}
494559

495560
// Make sure we have a state event compatible with our device. The event doesn't
@@ -498,11 +563,18 @@ public static unsafe bool HasValueChangeInEvent(this InputControl control, Input
498563
// to read.
499564
var device = control.device;
500565
if (stateFormat != device.m_StateBlock.format)
501-
throw new InvalidOperationException(
502-
$"Cannot read control '{control.path}' from {eventPtr.type} with format {stateFormat}; device '{device}' expects format {device.m_StateBlock.format}");
566+
{
567+
// If the device is an IInputStateCallbackReceiver, there's a chance it actually recognizes
568+
// the state format in the event and can correlate it to the state as found on the device.
569+
if (!device.hasStateCallbacks ||
570+
!((IInputStateCallbackReceiver)device).GetStateOffsetForEvent(control, eventPtr, ref stateOffset))
571+
return null;
572+
}
503573

504574
// Once a device has been added, global state buffer offsets are baked into control hierarchies.
505575
// We need to unsubtract those offsets here.
576+
// NOTE: If the given device has not actually been added to the system, the offset is simply 0
577+
// and this is a harmless NOP.
506578
stateOffset += device.m_StateBlock.byteOffset;
507579

508580
// Return null if state is out of range.

Packages/com.unity.inputsystem/InputSystem/Controls/InputControlPath.cs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,29 @@ public static bool Matches(string expected, InputControl control)
613613
return MatchesRecursive(ref parser, control);
614614
}
615615

616+
/// <summary>
617+
/// Check whether the given path matches <paramref name="control"/> or any of its parents.
618+
/// </summary>
619+
/// <param name="expected">A control path.</param>
620+
/// <param name="control">An input control.</param>
621+
/// <returns>True if the given path matches at least a partial path to <paramref name="control"/>.</returns>
622+
/// <exception cref="ArgumentNullException"><paramref name="expected"/> is <c>null</c> or empty -or-
623+
/// <paramref name="control"/> is <c>null</c>.</exception>
624+
/// <remarks>
625+
/// <example>
626+
/// <code>
627+
/// // True as the path matches the Keyboard device itself, i.e. the parent of
628+
/// // Keyboard.aKey.
629+
/// InputControlPath.MatchesPrefix("&lt;Keyboard&gt;", Keyboard.current.aKey);
630+
///
631+
/// // False as the path matches none of the controls leading to Keyboard.aKey.
632+
/// InputControlPath.MatchesPrefix("&lt;Gamepad&gt;", Keyboard.current.aKey);
633+
///
634+
/// // True as the path matches Keyboard.aKey itself.
635+
/// InputControlPath.MatchesPrefix("&lt;Keyboard&gt;/a", Keyboard.current.aKey);
636+
/// </code>
637+
/// </example>
638+
/// </remarks>
616639
public static bool MatchesPrefix(string expected, InputControl control)
617640
{
618641
if (string.IsNullOrEmpty(expected))

Packages/com.unity.inputsystem/InputSystem/Devices/InputDevice.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,18 @@ internal bool hasControlsWithDefaultState
594594
}
595595
}
596596

597+
internal bool hasStateCallbacks
598+
{
599+
get => (m_DeviceFlags & DeviceFlags.HasStateCallbacks) == DeviceFlags.HasStateCallbacks;
600+
set
601+
{
602+
if (value)
603+
m_DeviceFlags |= DeviceFlags.HasStateCallbacks;
604+
else
605+
m_DeviceFlags &= ~DeviceFlags.HasStateCallbacks;
606+
}
607+
}
608+
597609
internal void AddDeviceUsage(InternedString usage)
598610
{
599611
var controlUsageCount = m_UsageToControl.LengthSafe();

Packages/com.unity.inputsystem/InputSystem/Devices/Pointer.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,5 +248,10 @@ void IInputStateCallbackReceiver.OnStateEvent(InputEventPtr eventPtr)
248248
{
249249
OnStateEvent(eventPtr);
250250
}
251+
252+
bool IInputStateCallbackReceiver.GetStateOffsetForEvent(InputControl control, InputEventPtr eventPtr, ref uint offset)
253+
{
254+
return false;
255+
}
251256
}
252257
}

0 commit comments

Comments
 (0)