fix(pointers): Ensure that a manipulation cancelled by ManipStarting event is cleanup

Author: dr1rrb

src/Uno.UWP/UI/Input/GestureRecognizer.Manipulation.cs

@@ -77,7 +77,35 @@ private enum ManipulationState
 			public bool IsRotateEnabled	  => _isRotateEnabled;
 			public bool IsScaleEnabled	  => _isScaleEnabled;
 
-			public Manipulation(GestureRecognizer recognizer, PointerPoint pointer1)
+			internal static void AddPointer(GestureRecognizer recognizer, PointerPoint pointer)
+			{
+				var current = recognizer._manipulation;
+				if (current != null)
+				{
+					if (current.TryAdd(pointer))
+					{
+						// The pending manipulation which can either handle the pointer, either is still active with another pointer,
+						// just ignore the new pointer.
+						return;
+					}
+
+					// The current manipulation should be discarded
+					current.Complete(); // Will removed 'current' from the 'recognizer._manipulation' field.
+				}
+
+				var manipulation = new Manipulation(recognizer, pointer);
+				if (manipulation._state == ManipulationState.Completed)
+				{
+					// The new manipulation has been cancelled in the ManipStarting handler, throw it away.
+					manipulation.Complete(); // Should not do anything, safety only
+				}
+				else
+				{
+					recognizer._manipulation = manipulation;
+				}
+			}
+
+			private Manipulation(GestureRecognizer recognizer, PointerPoint pointer1)
 			{
 				_recognizer = recognizer;
 				_deviceType = pointer1.PointerDevice.PointerDeviceType;
@@ -114,35 +142,37 @@ public Manipulation(GestureRecognizer recognizer, PointerPoint pointer1)
 
 				if ((_settings & (GestureSettingsHelper.Manipulations | GestureSettingsHelper.DragAndDrop)) == 0)
 				{
+					// The manipulation has been cancelled (all possible manip has been removed)
+					// WARNING: The _gestureRecognizer._manipulation has not been set yet! We cannot invoke the Complete right now (cf. AddPointer)
+
 					_state = ManipulationState.Completed;
+					return;
 				}
-				else
-				{
-					_isDraggingEnable = (_settings & GestureSettings.Drag) != 0;
-					_isTranslateXEnabled = (_settings & (GestureSettings.ManipulationTranslateX | GestureSettings.ManipulationTranslateRailsX)) != 0;
-					_isTranslateYEnabled = (_settings & (GestureSettings.ManipulationTranslateY | GestureSettings.ManipulationTranslateRailsY)) != 0;
-					_isRotateEnabled = (_settings & GestureSettings.ManipulationRotate) != 0;
-					_isScaleEnabled = (_settings & GestureSettings.ManipulationScale) != 0;
-				}
+
+				_isDraggingEnable = (_settings & GestureSettings.Drag) != 0;
+				_isTranslateXEnabled = (_settings & (GestureSettings.ManipulationTranslateX | GestureSettings.ManipulationTranslateRailsX)) != 0;
+				_isTranslateYEnabled = (_settings & (GestureSettings.ManipulationTranslateY | GestureSettings.ManipulationTranslateRailsY)) != 0;
+				_isRotateEnabled = (_settings & GestureSettings.ManipulationRotate) != 0;
+				_isScaleEnabled = (_settings & GestureSettings.ManipulationScale) != 0;
 
 				_recognizer.ManipulationConfigured?.Invoke(_recognizer, this);
 				StartDragTimer();
 			}
 
-			public bool IsActive(PointerDeviceType type, uint id)
+			public bool IsActive(PointerIdentifier pointer)
 				=> _state != ManipulationState.Completed
-					&& _deviceType == type
-					&& _origins.ContainsPointer(id);
+					&& _deviceType == pointer.Type
+					&& _origins.ContainsPointer(pointer.Id);
 
-			public bool TryAdd(PointerPoint point)
+			private bool TryAdd(PointerPoint point)
 			{
 				if (point.Pointer == _origins.Pointer1.Pointer)
 				{
 					this.Log().Error(
 						"Invalid manipulation state: We are receiving a down for the second time for the same pointer!"
 						+ "This is however common when using iOS emulator with VNC where we might miss some pointer events "
 						+ "due to focus being stole by debugger, in that case you can safely ignore this message.");
-					return false; // Request to create a new manipualtion
+					return false; // Request to create a new manipulation
 				}
 				else if (_state >= ManipulationState.Inertia)
 				{
@@ -236,12 +266,17 @@ public void Complete()
 							new ManipulationCompletedEventArgs(_currents.Identifiers, position, cumulative, velocities, _state == ManipulationState.Inertia, _contacts.onStart, _contacts.current));
 						break;
 
-					default:
+					case ManipulationState.Starting:
 						_inertia?.Dispose();
 						_state = ManipulationState.Completed;
 
 						_recognizer.ManipulationAborted?.Invoke(_recognizer, this);
 						break;
+
+					default: // Safety only
+						_inertia?.Dispose();
+						_state = ManipulationState.Completed;
+						break;
 				}
 
 				// Self scavenge our self from the _recognizer ... yes it's a bit strange,
@@ -524,6 +559,7 @@ private void StopDragTimer()
 			{
 				_dragHoldTimer?.Stop();
 			}
+
 			// For pen and mouse this only means down -> * moves out of tap range;
 			// For touch it means down -> * moves close to origin for DragUsingFingerMinDelayTicks -> * moves far from the origin 
 			private bool IsBeginningOfDragManipulation()

src/Uno.UWP/UI/Input/GestureRecognizer.cs

@@ -90,18 +90,10 @@ public void ProcessDownEvent(PointerPoint value)
 			}
 			_gestures[value.PointerId] = gesture;
 
-			// Create of update a Manipulation responsible to recognize multi-pointer and drag gestures
+			// Create or update a Manipulation responsible to recognize multi-pointer and drag gestures
 			if (_isManipulationOrDragEnabled)
 			{
-				if (_manipulation == null)
-				{
-					_manipulation = new Manipulation(this, value);
-				}
-				else if (!_manipulation.TryAdd(value))
-				{
-					_manipulation.Complete();
-					_manipulation = new Manipulation(this, value);
-				}
+				Manipulation.AddPointer(this, value);
 			}
 		}
 
@@ -191,8 +183,8 @@ internal void PreventHolding(uint pointerId)
 
 		#region Manipulations
 		internal event TypedEventHandler<GestureRecognizer, ManipulationStartingEventArgs> ManipulationStarting; // This is not on the public API!
-		internal event TypedEventHandler<GestureRecognizer, Manipulation> ManipulationConfigured; // Right after the ManipulationStarting, once application has configured settings
-		internal event TypedEventHandler<GestureRecognizer, Manipulation> ManipulationAborted; // The manipulation has been aborted while in starting state
+		internal event TypedEventHandler<GestureRecognizer, Manipulation> ManipulationConfigured; // Right after the ManipulationStarting, once application has configured settings ** ONLY if not cancelled in starting **
+		internal event TypedEventHandler<GestureRecognizer, Manipulation> ManipulationAborted; // The manipulation has been aborted while in starting state ** ONLY if received a ManipulationConfigured **
 		public event TypedEventHandler<GestureRecognizer, ManipulationCompletedEventArgs> ManipulationCompleted;
 		public event TypedEventHandler<GestureRecognizer, ManipulationInertiaStartingEventArgs> ManipulationInertiaStarting;
 		public event TypedEventHandler<GestureRecognizer, ManipulationStartedEventArgs> ManipulationStarted;

src/Uno.UWP/UI/Input/ManipulationCompletedEventArgs.cs

@@ -31,7 +31,7 @@ internal ManipulationCompletedEventArgs(
 		/// <summary>
 		/// Gets identifiers of all pointer that has been involved in that manipulation (cf. Remarks).
 		/// </summary>
-		/// <remarks> This collection might contains pointers that has been released. <see cref="CurrentContactCount"/> gives the actual number of active pointers.</remarks>
+		/// <remarks>This collection might contains pointers that has been released. <see cref="CurrentContactCount"/> gives the actual number of active pointers.</remarks>
 		/// <remarks>All pointers are expected to have the same <see cref="PointerIdentifier.Type"/>.</remarks>
 		internal PointerIdentifier[] Pointers { get; }