Fix PlotBase lifecycle and input handling

Source/OxyPlot.Avalonia/PlotBase.Events.cs

@@ -12,7 +12,6 @@
 namespace OxyPlot.Avalonia
 {
     using global::Avalonia.Input;
-    using System;
     using System.Collections.Generic;
     using System.Linq;
 
@@ -57,6 +56,16 @@ protected override void OnPointerWheelChanged(PointerWheelEventArgs e)
         /// </summary>
         private SortedDictionary<int, ScreenPoint> TouchPositions { get; } = new SortedDictionary<int, ScreenPoint>();
 
+        /// <summary>
+        /// The last known position of the pointer.
+        /// </summary>
+        private ScreenPoint lastPointerPosition;
+
+        /// <summary>
+        /// Indicates that the capture is released by <see cref="OnPointerReleased"/> itself, not by the system.
+        /// </summary>
+        private bool isReleasingPointer;
+
         /// <summary>
         /// Invoked when an unhandled MouseDown attached event reaches an element in its route that is derived from this class. Implement this method to add class handling for this event.
         /// </summary>
@@ -72,6 +81,12 @@ protected override void OnPointerPressed(PointerPressedEventArgs e)
             Focus();
             e.Pointer.Capture(this);
 
+            // The press point is remembered for any pointer: it decides whether the context menu is shown.
+            mouseDownPoint = e.GetPosition(this).ToScreenPoint();
+            mouseDownPointerType = e.Pointer.Type;
+            hasMouseDownPoint = true;
+            lastPointerPosition = mouseDownPoint;
+
             if (e.Pointer.Type == PointerType.Touch)
             {
                 var position = e.GetPosition(this).ToScreenPoint();
@@ -93,9 +108,6 @@ protected override void OnPointerPressed(PointerPressedEventArgs e)
             }
             else
             {
-                // store the mouse down point, check it when mouse button is released to determine if the context menu should be shown
-                mouseDownPoint = e.GetPosition(this).ToScreenPoint();
-
                 e.Handled = ActualController.HandleMouseDown(this, e.ToMouseDownEventArgs(this));
             }
         }
@@ -112,6 +124,8 @@ protected override void OnPointerMoved(PointerEventArgs e)
                 return;
             }
 
+            lastPointerPosition = e.GetPosition(this).ToScreenPoint();
+
             if (e.Pointer.Type == PointerType.Touch)
             {
                 var point = e.GetPosition(this).ToScreenPoint();
@@ -135,13 +149,30 @@ protected override void OnPointerMoved(PointerEventArgs e)
         /// <param name="e">The <see cref="T:System.Windows.Input.MouseButtonEventArgs" /> that contains the event data. The event data reports that the mouse button was released.</param>
         protected override void OnPointerReleased(PointerReleasedEventArgs e)
         {
+            // The state is remembered before the base method: it raises ContextRequested
+            // and copies its Handled here. Otherwise, when a context menu is set,
+            // the controller would not receive the release and the manipulator would stay active.
+            var handledBefore = e.Handled;
             base.OnPointerReleased(e);
-            if (e.Handled)
+
+            // The capture is always released, even if the event has already been marked as handled.
+            // Otherwise the pointer stays captured after a right click and panning gets stuck.
+            // Releasing the capture synchronously raises PointerCaptureLost: the flag distinguishes it from a loss of
+            // capture by the system, otherwise the manipulator would be completed twice.
+            isReleasingPointer = true;
+            try
             {
-                return;
+                e.Pointer.Capture(null);
+            }
+            finally
+            {
+                isReleasingPointer = false;
             }
 
-            e.Pointer.Capture(null);
+            if (handledBefore)
+            {
+                return;
+            }
 
             if (e.Pointer.Type == PointerType.Touch)
             {
@@ -165,24 +196,70 @@ protected override void OnPointerReleased(PointerReleasedEventArgs e)
             else
             {
                 e.Handled = ActualController.HandleMouseUp(this, e.ToMouseReleasedEventArgs(this));
+            }
+        }
 
-                // Open the context menu
-                var p = e.GetPosition(this).ToScreenPoint();
-                var d = p.DistanceTo(mouseDownPoint);
+        /// <summary>
+        /// Completes the action started by a press if the pointer capture is lost without a release:
+        /// the window lost activation, the system cancelled the touch, etc. Otherwise the manipulator of the controller
+        /// stays active and the plot is panned by plain mouse movement, and the touch stays forever
+        /// in <see cref="TouchPositions"/>, so the following gestures do not start.
+        /// </summary>
+        /// <param name="e">The event data.</param>
+        protected override void OnPointerCaptureLost(PointerCaptureLostEventArgs e)
+        {
+            base.OnPointerCaptureLost(e);
+            if (isReleasingPointer)
+            {
+                return;
+            }
 
-                if (ContextMenu != null)
+            if (e.Pointer.Type == PointerType.Touch)
+            {
+                if (TouchPositions.TryGetValue(e.Pointer.Id, out var position))
                 {
-                    if (Math.Abs(d) < 1e-8 && e.InitialPressMouseButton == MouseButton.Right)
+                    TouchPositions.Remove(e.Pointer.Id);
+                    if (TouchPositions.Count == 0)
                     {
-                        ContextMenu.DataContext = DataContext;
-                        ContextMenu.IsVisible = true;
-                    }
-                    else
-                    {
-                        ContextMenu.IsVisible = false;
+                        ActualController.HandleTouchCompleted(this, new OxyTouchEventArgs
+                        {
+                            Position = position,
+                            DeltaTranslation = new ScreenVector(0, 0),
+                            DeltaScale = new ScreenVector(1, 1),
+                        });
                     }
                 }
             }
+            else
+            {
+                ActualController.HandleMouseUp(this, new OxyMouseEventArgs { Position = lastPointerPosition });
+            }
+        }
+
+        /// <summary>
+        /// Prepares the context menu for display. The menu is opened by Avalonia itself on the
+        /// <see cref="InputElement.ContextRequestedEvent"/> event, so it does not need to be opened manually.
+        /// If the pointer was moved after the button was pressed (a zoom rectangle was drawn), the menu is not shown.
+        /// </summary>
+        /// <param name="sender">The event source.</param>
+        /// <param name="e">The event data.</param>
+        private void OnPlotContextRequested(object sender, ContextRequestedEventArgs e)
+        {
+            // The menu is suppressed only for the mouse: for touch and pen it is opened by a long press,
+            // and the press point is irrelevant to that.
+            if (hasMouseDownPoint && mouseDownPointerType == PointerType.Mouse
+                && e.TryGetPosition(this, out var position)
+                && position.ToScreenPoint().DistanceTo(mouseDownPoint) > 1e-8)
+            {
+                // A handled event suppresses opening of the menu.
+                e.Handled = true;
+                return;
+            }
+
+            if (ContextMenu != null)
+            {
+                ContextMenu.DataContext = DataContext;
+            }
         }
 
         /// <summary>

Source/OxyPlot.Avalonia/PlotBase.Properties.cs

@@ -55,6 +55,11 @@ public partial class PlotBase
 
         static PlotBase()
         {
+            // Without this the control does not take focus, and the keyboard commands of the plot do not work.
+            FocusableProperty.OverrideDefaultValue<PlotBase>(true);
+
+            // The plot must not be part of the Tab order, it takes focus only by a click.
+            IsTabStopProperty.OverrideDefaultValue<PlotBase>(false);
         }
 
         /// <summary>

Source/OxyPlot.Avalonia/PlotBase.cs

@@ -75,24 +75,24 @@ public abstract partial class PlotBase : TemplatedControl, IPlotView
         private ScreenPoint mouseDownPoint;
 
         /// <summary>
-        /// The overlays.
+        /// Indicates that the press point has been filled in at least once.
         /// </summary>
-        private Canvas overlays;
+        private bool hasMouseDownPoint;
 
         /// <summary>
-        /// The zoom control.
+        /// The type of the pointer that made the last press.
         /// </summary>
-        private ContentControl zoomControl;
+        private PointerType mouseDownPointerType;
 
         /// <summary>
-        /// The is visible to user cache.
+        /// The overlays.
         /// </summary>
-        private bool isVisibleToUserCache;
+        private Canvas overlays;
 
         /// <summary>
-        /// The cached parent.
+        /// The zoom control.
         /// </summary>
-        private Control containerCache;
+        private ContentControl zoomControl;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="PlotBase" /> class.
@@ -101,8 +101,9 @@ protected PlotBase()
         {
             DisconnectCanvasWhileUpdating = true;
             trackerDefinitions = new ObservableCollection<TrackerDefinition>();
-            
+
             this.GetObservable(BoundsProperty).Subscribe(new AnonymousObserver<Rect>(OnSizeChanged));
+            AddHandler(ContextRequestedEvent, OnPlotContextRequested);
         }
 
         /// <summary>
@@ -366,9 +367,20 @@ public void ShowZoomRectangle(OxyRect r)
         /// <param name="text">The text.</param>
         public async void SetClipboardText(string text)
         {
-            if (TopLevel.GetTopLevel(this) is { Clipboard: { } clipboard })
+            // The method is declared as void in IPlotView, so there is nobody to catch an exception from here.
+            // Without try/catch an unavailable or busy clipboard would crash the whole application.
+            try
             {
-                await clipboard.SetTextAsync(text).ConfigureAwait(true);
+                if (TopLevel.GetTopLevel(this) is { Clipboard: { } clipboard })
+                {
+                    await clipboard.SetTextAsync(text).ConfigureAwait(true);
+                }
+            }
+            catch (Exception ex)
+            {
+                global::Avalonia.Logging.Logger
+                    .TryGet(global::Avalonia.Logging.LogEventLevel.Warning, global::Avalonia.Logging.LogArea.Control)
+                    ?.Log(this, "Failed to write text to the clipboard: {0}", ex.Message);
             }
         }
 
@@ -397,7 +409,20 @@ protected override Size ArrangeOverride(Size finalSize)
         /// <param name="updateData">The update Data.</param>
         protected void UpdateModel(bool updateData = true)
         {
-            if (Width <= 0 || Height <= 0 || ActualModel == null)
+            if (ActualModel == null)
+            {
+                // The model was removed: the old plot is erased by UpdateVisuals, but it is only called by
+                // the layout pass triggered by the isPlotInvalidated flag. Without the flag the picture would stay on the screen.
+                isUpdateRequired = 0;
+                if (Interlocked.CompareExchange(ref isPlotInvalidated, 1, 0) == 0)
+                {
+                    BeginInvoke(InvalidateArrange);
+                }
+
+                return;
+            }
+
+            if (Width <= 0 || Height <= 0)
             {
                 isUpdateRequired = 0;
                 return;
@@ -551,15 +576,17 @@ private void UpdateVisuals()
         /// Invokes the specified action on the dispatcher, if necessary.
         /// </summary>
         /// <param name="action">The action.</param>
-        private static void BeginInvoke(Action action)
+        private void BeginInvoke(Action action)
         {
-            if (Dispatcher.UIThread.CheckAccess())
+            // The dispatcher of the control, not Dispatcher.UIThread: in Avalonia 12 a control can live
+            // in a window on another UI thread, and then the main thread has no access to it.
+            if (CheckAccess())
             {
                 action?.Invoke();
             }
             else
             {
-                Dispatcher.UIThread.InvokeAsync(action, DispatcherPriority.Loaded);
+                Dispatcher.InvokeAsync(action, DispatcherPriority.Loaded);
             }
         }
     }

Source/OxyPlot.Avalonia/Utilities/ConverterExtensions.cs

@@ -432,11 +432,16 @@ public static OxyModifierKeys ToModifierKeys(this KeyModifiers modifiers)
         /// <returns>A <see cref="OxyMouseWheelEventArgs" /> containing the converted event arguments.</returns>
         public static OxyMouseWheelEventArgs ToMouseWheelEventArgs(this PointerWheelEventArgs e, IInputElement relativeTo)
         {
+            // Delta is a fraction of a wheel step, and for a touchpad it is less than one.
+            // Casting to int before the multiplication zeroed such scrolling, so the rounding is done afterwards.
+            // The horizontal delta is used only when the vertical one is zero, otherwise a diagonal gesture doubled the step.
+            var wheel = e.Delta.Y != 0 ? e.Delta.Y : e.Delta.X;
+
             return new OxyMouseWheelEventArgs
             {
                 Position = e.GetPosition(relativeTo as Visual).ToScreenPoint(),
                 ModifierKeys = e.KeyModifiers.ToModifierKeys(),
-                Delta = (int)(e.Delta.Y + e.Delta.X) * 120
+                Delta = (int)Math.Round(wheel * 120, MidpointRounding.AwayFromZero)
             };
         }