oxyplot / oxyplot-maui

OxyPlot for Maui
MIT License
37 stars 7 forks source link

Zoom is not working on iOS #10

Closed RobertStefancic closed 1 month ago

RobertStefancic commented 4 months ago

The TouchRecognizer is not receiving all touch input events, which results in slow and snappy behavior when trying to pan/zoom on the plot.

jaisont07 commented 4 months ago

Does the sample here has a plot where zoom is supported in iOS? @janusw Because, the zoom doesn't seems to be working for pinch to zoom. Instead its panning the plot.

janusw commented 3 months ago

Does the sample here has a plot where zoom is supported in iOS? @janusw

Actually most of the samples in the ExampleBrowser should support zoom, e.g. the first one in the list (Annotations -> Tool tips). Panning and zooming that example works well for me on Android.

Because, the zoom doesn't seems to be working for pinch to zoom. Instead its panning the plot.

Yes, I can confirm that zooming via two-finger pinch gesture does not work at all on iOS. Single-finger panning seems to work well, though.

What @RobertStefancic means by "snappy behavior" is probably that the plot sometimes jumps around wildly when using two-finger pinch.

janusw commented 3 months ago

AFAICS the problem is that iOS always generates TouchAction events with a single point only ...

https://github.com/oxyplot/oxyplot-maui/blob/e2f1eb227e653da2da74766313a1d2ccd0de50b5/Source/OxyPlot.Maui.Skia/Platforms/iOS/Effects/PlatformTouchEffect.cs#L170-L171

... while Android generates touch events with multiple points:

https://github.com/oxyplot/oxyplot-maui/blob/e2f1eb227e653da2da74766313a1d2ccd0de50b5/Source/OxyPlot.Maui.Skia/Platforms/Android/Effects/PlatformTouchEffect.cs#L105-L106

janusw commented 3 months ago

I took a first stab at fixing it on this branch:

https://github.com/janusw/oxyplot-maui/tree/iOS_fix_zoom

It basically makes zooming work in iOS, but in some cases it seems to behave strangely. I'll need to take another look at it soon ...

jaisont07 commented 3 months ago

@janusw, Actually I have made a fix for this which is working fine, If possible to give me the permission, I can raise a PR for the same.

janusw commented 3 months ago

You should be able to create a PR: Just fork the repo, make a new branch, commit your changes, push them, and open a PR for that branch.

RobertStefancic commented 3 months ago

Hello @janusw and @jaisont07. Thank you for taking the time to look into the issue.

I have also developed a solution for the issue that has been accepted and already used in production. Unfortunately I have not been able to create a PR yet due to some issues with GitHub here at work. I planned to do it from my personal account at home, but didn't have the opportunity just yet.

However, I will paste the code here until then. If you find that it's working for you as well, I don't mind you integrating it.

using CoreGraphics;
using Foundation;
using Microsoft.Maui.Controls.Platform;
using OxyPlot.Maui.Skia.Effects;
using UIKit;

namespace OxyPlot.Maui.Skia.ios.Effects;

public class PlatformTouchEffect : PlatformEffect
{
    private UIView view;
    private TouchRecognizer touchRecognizer;

    protected override void OnAttached()
    {
        view = Control ?? Container;

        var touchEffect = Element.Effects.OfType<MyTouchEffect>().FirstOrDefault();

        if (touchEffect != null && view != null)
        {
            touchRecognizer = new TouchRecognizer(Element, touchEffect);
            view.AddGestureRecognizer(touchRecognizer);
        }
    }

    protected override void OnDetached()
    {
        if (touchRecognizer != null)
        {
            touchRecognizer.Detach();
            view.RemoveGestureRecognizer(touchRecognizer);
        }
    }
}

internal class TouchRecognizer : UIGestureRecognizer
{
    private readonly Microsoft.Maui.Controls.Element element;
    private readonly MyTouchEffect touchEffect;
    private uint activeTouchesCount = 0;

    public TouchRecognizer(Microsoft.Maui.Controls.Element element, MyTouchEffect touchEffect)
    {
        this.element = element;
        this.touchEffect = touchEffect;

        ShouldRecognizeSimultaneously = new UIGesturesProbe((_, _) => true);
    }

    public void Detach()
    {
        ShouldRecognizeSimultaneously = null;
    }

    public override void TouchesBegan(NSSet touches, UIEvent evt)
    {
        base.TouchesBegan(touches, evt);
        activeTouchesCount += touches.Count.ToUInt32();
        FireEvent(touches, TouchActionType.Pressed, true);
    }

    public override void TouchesMoved(NSSet touches, UIEvent evt)
    {
        base.TouchesMoved(touches, evt);

        if (activeTouchesCount == touches.Count.ToUInt32())
        {
            FireEvent(touches, TouchActionType.Moved, true);
        }
    }

    public override void TouchesEnded(NSSet touches, UIEvent evt)
    {
        base.TouchesEnded(touches, evt);
        activeTouchesCount -= touches.Count.ToUInt32();
        FireEvent(touches, TouchActionType.Released, false);
    }

    public override void TouchesCancelled(NSSet touches, UIEvent evt)
    {
        base.TouchesCancelled(touches, evt);
    }

    private void FireEvent(NSSet touches, TouchActionType actionType, bool isInContact)
    {
        UITouch[] uiTouches = touches.Cast<UITouch>().ToArray();
        long id = ((IntPtr)uiTouches.First().Handle).ToInt64();
        Point[] points = new Point[uiTouches.Length];

        for (int i = 0; i < uiTouches.Length; i++)
        {
            CGPoint cgPoint = uiTouches[i].LocationInView(View);
            points[i] = new(cgPoint.X, cgPoint.Y);
        }
        touchEffect.OnTouchAction(element, new(id, actionType, points, isInContact));
    }
}
jaisont07 commented 3 months ago

You should be able to create a PR: Just fork the repo, make a new branch, commit your changes, push them, and open a PR for that branch.

Raised a PR

janusw commented 3 months ago

Raised a PR

Awesome, thanks. I'm currently traveling and won't be able to review it before next week.

In the meantime, could you please check the differences to my branch mentioned above:

https://github.com/janusw/oxyplot-maui/tree/iOS_fix_zoom

What are you doing differently? And why do you do it like that?

janusw commented 3 months ago

I have also developed a solution for the issue that has been accepted and already used in production. Unfortunately I have not been able to create a PR yet due to some issues with GitHub here at work. I planned to do it from my personal account at home, but didn't have the opportunity just yet.

However, I will paste the code here until then. If you find that it's working for you as well, I don't mind you integrating it.

If you want this to be considered for inclusion in the repo, please push it to a branch on your fork (and open a PR).

Seems like we have three fixes by now that we can cross-check against each other 😊

janusw commented 1 month ago

This issue is fixed via #15.

janusw commented 1 month ago

The fix is available in version 1.0.1, which was just released.