Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flickering occurs while updating the width of ContentView through PanGestureRecognizer. #20772

Open
RasikaPalanisamy opened this issue Feb 22, 2024 · 11 comments · May be fixed by #21547
Open

Flickering occurs while updating the width of ContentView through PanGestureRecognizer. #20772

RasikaPalanisamy opened this issue Feb 22, 2024 · 11 comments · May be fixed by #21547
Labels
area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter platform/android 🤖 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Milestone

Comments

@RasikaPalanisamy
Copy link

RasikaPalanisamy commented Feb 22, 2024

Description

I have created the ContomView, which is derived from CustomLayout. CustomView has 2 children that are derived from contentView. The second child has the PanGestureRecognizer. While the PanGesture is running, I have changed the width of the first child. Now, flickering occurs on the control because the PanGestureRecognizerEventArgs.TotalX value is updated incorrectly.

Steps to Reproduce

1.Run the attached sample.
2. Drag the second child from left to right.

Observed behavior:
flickering occurs on the control.

Flickering_Issue.mp4

Link to public reproduction project repository

https://github.com/RasikaPalanisamy/CustomSample

Version with bug

Unknown/Other

Is this a regression from previous behavior?

Not sure, did not test other versions

Last version that worked well

Unknown/Other

Affected platforms

Android, Windows

Affected platform versions

No response

Did you find any workaround?

No response

Relevant log output

No response

@RasikaPalanisamy RasikaPalanisamy added the t/bug Something isn't working label Feb 22, 2024
@PureWeen PureWeen added area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter platform/android 🤖 labels Feb 22, 2024
@PureWeen
Copy link
Member

@RasikaPalanisamy it might be useful to ask this in discussions or StackOverflow to see if there's a way to optimize how you are updating.

@PureWeen PureWeen added this to the Backlog milestone Feb 22, 2024
@ghost
Copy link

ghost commented Feb 22, 2024

We've added this issue to our backlog, and we will work to address it as time and resources allow. If you have any additional information or questions about this issue, please leave a comment. For additional info about issue management, please read our Triage Process.

@RasikaPalanisamy
Copy link
Author

@PureWeen We need the correct PanGestureRecognizerEventArgs.TotalX value. However, the PanGestureRecognizerEventArgs.TotalX value is returning an incorrect value.

@danielftz
Copy link

danielftz commented Mar 17, 2024

Can confirm PanGestureRecognizerEventArgs.TotalX is returning nonsensical value on Android, and needs to be taken out of the backlog

@Pairoba
Copy link

Pairoba commented Mar 21, 2024

I ran into the same issue and used a workaround for those who are interested. Feel free to provide suggestions for improvement.

private double _lastPanX1 = 0;
private double _lastPanX2 = 0;
private double _lastPanX3 = 0;

private bool IsValidPanEvent(object? sender, PanUpdatedEventArgs e)
{
    bool valid = false;
    if (e.StatusType == GestureStatus.Started)
    {
        _lastPanX1 = 0;
        _lastPanX2 = 0;
        _lastPanX3 = 0;

        valid = true;
    }
    else if (e.StatusType == GestureStatus.Running)
    {
        var lastPanX4 = _lastPanX3;
        _lastPanX3 = _lastPanX2;
        _lastPanX2 = _lastPanX1;
        _lastPanX1 = e.TotalX;

        // detect whether user is increasing or decreasing
        // therefore, we will compare 1 with 3 and 2 with 4:
        // if different, we expect an invalid call
        // due to update speed I see no reason to skip some of the events

        bool totalXIncreases1 = _lastPanX1 > _lastPanX3;
        bool totalXIncreases2 = _lastPanX2 > lastPanX4;

        if (totalXIncreases1 == totalXIncreases2)
        {
            if (totalXIncreases1)
            {
                // if we expect increase, we want to make sure, that the new value is actually higher
                if (_lastPanX1 > _lastPanX2)
                {
                    valid = true;
                }
            }
            else
            {
                if (_lastPanX1 < _lastPanX2)
                {
                    valid = true;
                }
            }
        }
    }

    return valid;
}

@XamlTest XamlTest added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Mar 27, 2024
@XamlTest
Copy link

Verified this on VS 17.10.0 Preview 2.0(8.0.14). Repro on Android 14.0-API34, not repro on iOS 17.2 and MacCatalyst.
Project: CustomSample.zip
Android:
DragAndroid

Different behavior on Windows 11:
Drag

@BurningLights
Copy link
Contributor

I noticed in src/Controls/src/Core/Platform/GestureManager/GesturePlatformManager.Android.cs in OnPlatformViewTouched, the event consumed boolean value returned from OnTouchEvent isn't propagated out. The lines

if (e.Event != null)
	OnTouchEvent(e.Event);

should be

if (e.Event != null)
	e.Handled = OnTouchEvent(e.Event);

@BurningLights
Copy link
Contributor

BurningLights commented Mar 31, 2024

I think I've figured it out. It looks like InnerGestureListener should use RawX and RawY instead of GetX() and GetY(). The raw versions return absolute screen coordinates, while the non-raw versions return the coordinates relative to the view. If the view is being translated, that shifts the view so the relative coordinates get messed up for calculating the length of the swipe, but the absoulte coordinates would still give the correct length of the swipe.

@BurningLights BurningLights linked a pull request Mar 31, 2024 that will close this issue
@BurningLights
Copy link
Contributor

Here is the workaround I'm using to create smooth panning on Android. I've subclassed PanGestureRecognizer and re-implemented the IPanGestureController. Just be sure to set the PanUpdated handler using either XAML or a variable of the inherited type, due to having to use the new keyword to create an event handler the inherited class can trigger.

internal sealed class CorrectedPanGestureRecognizer : PanGestureRecognizer, IPanGestureController
{
#if ANDROID
    // Index 0 is X, index 1 is Y
    private readonly int[] startingLocation = new int[2];
    private readonly int[] currentLocation = new int[2];
#endif

    void IPanGestureController.SendPan(Element sender, double totalX, double totalY, int gestureId)
    {
#if ANDROID
        ArgumentNullException.ThrowIfNull(sender.Handler.MauiContext?.Context);
        Android.Views.View view = sender.ToPlatform(sender.Handler.MauiContext);
        view.GetLocationOnScreen(currentLocation);
        totalX += sender.Handler.MauiContext.Context.FromPixels(currentLocation[0] - startingLocation[0]);
        totalY += sender.Handler.MauiContext.Context.FromPixels(currentLocation[1] - startingLocation[1]);

#endif
        PanUpdated?.Invoke(sender, new PanUpdatedEventArgs(GestureStatus.Running, gestureId, totalX, totalY));
    }

    void IPanGestureController.SendPanCanceled(Element sender, int gestureId)
    {
        PanUpdated?.Invoke(sender, new PanUpdatedEventArgs(GestureStatus.Canceled, gestureId));
    }

    void IPanGestureController.SendPanCompleted(Element sender, int gestureId)
    {
        PanUpdated?.Invoke(sender, new PanUpdatedEventArgs(GestureStatus.Completed, gestureId));
    }

    void IPanGestureController.SendPanStarted(Element sender, int gestureId)
    {
#if ANDROID
        ArgumentNullException.ThrowIfNull(sender.Handler.MauiContext);
        Android.Views.View view = sender.ToPlatform(sender.Handler.MauiContext);
        view.GetLocationOnScreen(startingLocation);
#endif
        PanUpdated?.Invoke(sender, new PanUpdatedEventArgs(GestureStatus.Started, gestureId));
    }

    public new event EventHandler<PanUpdatedEventArgs>? PanUpdated;
}

@samhouts samhouts removed s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Jul 3, 2024
@samhouts samhouts added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Jul 10, 2024
@alistair-ocad
Copy link

I've also noticed that if dragging across the whole screen the values returned don't reflect the actual position of the finger/pen. This is only noticeable if dragging a larger distance, feels almost as if it's a factor of 2 out. Again, only on Android.

@beeradmoore
Copy link
Contributor

beeradmoore commented Aug 15, 2024

I was looking into a different pan issue (#24252) and when I tested on Android I came across this problem as well.

The CorrectedPanGestureRecognizer from BurningLights above does fix the issue for me.

Broken:

android-broken.mp4

Working:

android-working.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter platform/android 🤖 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants