Touch behavior issue after widget tap

[janusw]

Mapsui Version
4.1.0-preview.1

Mapsui Platform
Xamarin.Forms

Device
iOS (v16+17)

Describe the bug
I'm seeing a regression with widget behavior in version 4.1.0-preview.1 (compared to 4.0.0). I use custom widgets in a MapView that trigger a particular action when tapping. The action is still triggered alright with the preview version, but afterwards there are issues with the touch behavior (it seems like a TouchEnded event is missing or something like that): Panning with a single finger results in two-finger pinch behavior, and after a second tap on the widget it does not work at all any more. Probably the MapView keeps some extraneous touch points that don't exist any more.

Expected behavior
Touch gestures should work like usual.

Additional context
It seems the bug was introduced with PR #2133 by @inforithmics (most probably due to the MapView changes in commit 91b24b3).

[pauldendulk]

Thanks for the clear report.

From reading the description I think the underlying problem is the same as in this issue:
#2189
Which should be fixed with this PR:
#2190

Although your description includes some more problems, so I am not sure. Perhaps you can test this with these artifacts:
https://github.com/Mapsui/Mapsui/actions/runs/6365272919

[janusw]

From reading the description I think the underlying problem is the same as in this issue: #2189 Which should be fixed with this PR: #2190

Ah, good. I did not see the other ticket, but it really sounds like it's the same problem.

Although your description includes some more problems, so I am not sure. Perhaps you can test this with these artifacts: https://github.com/Mapsui/Mapsui/actions/runs/6365272919

I tried the merge commit of #2190, but unfortunately none of my widgets show any reaction any more ☹️
(neither on iOS nor on Android, with Mapsui.Forms)

[janusw]

Just tried Mapsui.Samples.Forms.Android on current master. The zoom buttons also don't seem to work there. Was #2190 only tested with Blazor? Do the widgets work there?

[pauldendulk]

I tested the zoom buttons on WPF and Avalonia (tested again just now, works on master), but also, I do not plan to do more testing. We have 12 platform nuget packages. Several of those can be run on different platforms, and then there is also different devices and different browsers. So, testing on all those platforms would be very time consuming. The only way to keep this project manageable is to reduce the amount of platform specific code. Changes in our code base should have the same effect on all platforms. This comes up a lot in discussions. I hope people see why this is very important.

[janusw]

I tested the zoom buttons on WPF and Avalonia (tested again just now, works on master), but also, I do not plan to do more testing. We have 12 platform nuget packages. Several of those can be run on different platforms, and then there is also different devices and different browsers. So, testing on all those platforms would be very time consuming.

Sure, this is the classic dilemma of cross-platform software. And Mapsui really supports a lot of different platforms by now (with the addition of all the Avalonia/Uno/Eto stuff etc).

I certainly did not want to blame you for anything. My intention was just to know which platform the PR was tested on (of course it is not feasible to always test everywhere, but it is good practice to at least document in a review comment where testing was done).

The only way to keep this project manageable is to reduce the amount of platform specific code. Changes in our code base should have the same effect on all platforms. This comes up a lot in discussions. I hope people see why this is very important.

Completely agree. The goal should be to have as much shared/generic code as possible and reduce the platform-specific parts to the minimum.

Then also automated testing helps a lot, but this can be difficult, in particular with UI.

And in the end you still need a lot of manual testing on different platforms as well. With an open source project, this luckily can be 'crowd-sourced' to a large extent 😄

[pauldendulk]

but it is good practice to at least document in a review comment where testing was done).

true, and we could improve on that.

Then also automated testing helps a lot, but this can be difficult, in particular with UI.

Yes, this is better and, yes, this is hard with UI. If we depend mostly on or skia widgets we could use our image regression test to test the rendering of the widgets. Interactions like these will still be hard. UI testing frameworks will be platform specific, so also too much maintenance for us. A bigger part of code underlying the interactions could be unit tested.

With an open source project, this luckily can be 'crowd-sourced' to a large extent 😄

This indeed the way I think, I release a preview and hope others will test, as you did today👍

[charlenni]

We rely on SkiaSharp for the platform specific parts. So it isn't often a problem of Mapsui, but of a problem of SkiaSharp. If SkiaSharp isn't working correct on a platform, then Mapsui isn't working correct. If SkiaSharp doesn't have a tap event, then Mapsui could provide one.

The next is, that different platforms handling things different. On one platform you get a zoom when you double-click, on the next platform you have a gesture for this. Even in Maui or Avalonia you have problems to use the same events for the same actions.

I think, Paul is right: We should move more parts from the platform specific parts into the shared MapControl. I did this long time ago, because many of the MapControls had the same code. But what doesn't work is, to make the effort, to provide the same functionality for all platforms. The shared MapControl should have a function to zoom in or out, but not how this function is called. The MapControl for Maui could call it in the tap event, Avalonia in a click event. And if someone likes to have a special event for zoom in Eto, then this one should implement it in SkiaSharp and connect it with Mapsui in the platform specific part of the MapControl.

[pauldendulk]

I can reproduce it with the Maui MapControl.

When debugging I don't get any SKTouchAction.Released event. This is a bit too weird to be true. So, perhaps there is something wrong with my system, I have been debugging a bit too long today, so I am not very sharp. Perhaps someone else can try to reproduce this.

    private async void OnTouch(object? sender, SKTouchEventArgs e)
    {
        if (e.ActionType == SKTouchAction.Released)
        {
            Console.WriteLine("Touche!"); // <-- The breakpoint here is never hit.
        }
    }

[pauldendulk]

@charlenni There will always be a parts that are platform specific, but there is definitely a lot more we could move to shared code. We should discuss this further in the context of more specific examples.

[janusw]

@charlenni There will always be a parts that are platform specific, but there is definitely a lot more we could move to shared code. We should discuss this further in the context of more specific examples.

... and in another issue 😜

[janusw]

Just tried Mapsui.Samples.Forms.Android on current master. The zoom buttons also don't seem to work there.

Same with Mapsui.Samples.Maui.MapView (on iOS) :/

@inforithmics, it seems like #2190 made all widgets unusable (with Xamarin and Maui).

[inforithmics]

@janusw yes you are right #2190 introduced this behavior.
The reason is here.

 private bool HandleTouch(SKTouchEventArgs e, MPoint location)
 {
     return e.ActionType switch
     {
         SKTouchAction.Pressed when HandleTouching(location, true, Math.Max(1, _numOfTaps), ShiftPessed) => true,
         SKTouchAction.Released when HandleTouched(location, true, 0, ShiftPessed) => true,
         SKTouchAction.Moved when HandleMoving(location, true, Math.Max(1, _numOfTaps), ShiftPessed) => true,
         _ => false
     };
 }

When HandleTouching returns true then HandleTouched is never executed.

Using this code fixes the problem but I have to anlyze it further

 private bool HandleTouch(SKTouchEventArgs e, MPoint location)
 {
     return e.ActionType switch
     {
         SKTouchAction.Pressed when HandleTouching(location, true, Math.Max(1, _numOfTaps), ShiftPessed) => false,
         SKTouchAction.Released when HandleTouched(location, true, 0, ShiftPessed) => true,
         SKTouchAction.Moved when HandleMoving(location, true, Math.Max(1, _numOfTaps), ShiftPessed) => true,
         _ => false
     };
 }

[inforithmics]

after looking at this I discovered that the e.Handled is not set

 if (HandleTouch(e, location))
 {
     return;
 }

Setting it fixed the problem

 if (HandleTouch(e, location))
 {
     e.Handled = true;
     return;
 }