[Windows] Subscribe pointer events only when needed

[MartyIX]

Description of Change

The idea of the PR is that subscriptions

container.PointerEntered += OnPgrPointerEntered;
container.PointerExited += OnPgrPointerExited;
container.PointerMoved += OnPgrPointerMoved;
container.PointerPressed += OnPgrPointerPressed;
container.PointerReleased += OnPgrPointerReleased;

are only necessary if

1. the associated element has some PointerGestureRecognizer, or
2. these events are subscribed.

This arguments should suffice to claim that this PR is a refactoring after an element is initialization.

Now it is necessary to prove that the PR works even if

1. Pointer gesture events change -> should be safe because of this
2. Element's container change -> should be safe because of this

Performance impact

• MAIN: 1720557197.MAIN.speedscope.MAIN.json
• PR: 1720556818.PR.speedscope.PR.json

-> 98% improvement¹.

Issues Fixed

Contributes to #21787

Footnotes

1. Note that this improvement is for view where there are no PointerGestureRecognizers assigned to elements as in my use case. ↩

[dotnet-policy-service]

Hey there @MartyIX! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[PureWeen]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[daltzctr]

This yields nice improvements in performance in a complex app. However, it does seem to break Visual States.

Repro:

Apply the following on an element and observe it not working.

            <VisualStateManager.VisualStateGroups>
                <VisualStateGroup x:Name="CommonStates">
                    <VisualState x:Name="PointerOver">
                        <VisualState.Setters>
                            <Setter Property="Scale" Value="1.01" />
                        </VisualState.Setters>
                    </VisualState>
                    <VisualState x:Name="Normal">
                        <VisualState.Setters>
                            <Setter Property="Scale" Value="1" />
                        </VisualState.Setters>
                    </VisualState>
                </VisualStateGroup>
            </VisualStateManager.VisualStateGroups>

[MartyIX]

This yields nice improvements in performance in a complex app. However, it does seem to break Visual States.

Repro:

Apply the following on an element and observe it not working.

            <VisualStateManager.VisualStateGroups>
                <VisualStateGroup x:Name="CommonStates">
                    <VisualState x:Name="PointerOver">
                        <VisualState.Setters>
                            <Setter Property="Scale" Value="1.01" />
                        </VisualState.Setters>
                    </VisualState>
                    <VisualState x:Name="Normal">
                        <VisualState.Setters>
                            <Setter Property="Scale" Value="1" />
                        </VisualState.Setters>
                    </VisualState>
                </VisualStateGroup>
            </VisualStateManager.VisualStateGroups>

Thank you for testing.

I thought that this

maui/src/Controls/src/Core/Platform/GestureManager/GesturePlatformManager.Windows.cs

Lines 463 to 466 in f323832

	void ModelGestureRecognizersOnCollectionChanged(object? sender, NotifyCollectionChangedEventArgs notifyCollectionChangedEventArgs)
	{
	UpdatingGestureRecognizers();
	}

would take care of it. But perhaps it doesn't. I need to take a closer look.

[albyrock87]

@MartyIX Here https://github.com/dotnet/maui/pull/23515/files#diff-779a3cbf8053213a2331cbf6a85a82af5c41cd858702c4fa4c4efdf16d7a6b13R772 use CompositeGestureRecognizers instead.

[MartyIX]

Just modernize a bit

[albyrock87]

What about a simpler if (ElementGestureRecognizers is not {} gestures) return; ?
I think that view.GestureRecognizer is just wrong (not enough) in this situation.
Or do you think we would have strange side effects?

[MartyIX]

This is a different method. I don't want to change behavior here.

[MartyIX]

Perhaps it can be done. I'm not sure. However, if I start with it - i.e. changing IList<IGestureRecognizer> to ElementGestureRecognizers - then nobody will want to review this PR and it will lead to delays.

So I sort of prefer as little change as possible.

[albyrock87]

For reference, CompositeGesureRecognizers is basically a proxy of the GestureRecognizers with the addition of the pointer over gesture recognizer which is (un)set by this method here who checks the VSM for pointer over states.

That said, and considering the only piece of code here caring about the pointer gesture recognizer is the one you changed, I feel this is a safe change.

Anyway I'll leave this to the MAUI team: my purpose is just to give more context information.

[MartyIX]

Changed according to: #23515 (comment)

[MartyIX]

I tested with https://github.com/MartyIX/maui/tree/feature/2024-07-09-Optional-pointer-event-subscription-TEST (MartyIX@62dc732)

Test code

MainPage.xaml

<ContentPage
    xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
    xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
    x:Class="Maui.Controls.Sample.MainPage"
    xmlns:local="clr-namespace:Maui.Controls.Sample">

  <VerticalStackLayout>
    <Label Text="First button has assigned a VisualStateManager with a pointer-over state."/>
    <Button x:Name="button1" Text="Button 1">
      <VisualStateManager.VisualStateGroups>
        <VisualStateGroupList>
          <VisualStateGroup>
            <VisualState x:Name="Normal" />
            <VisualState x:Name="Pressed">
              <VisualState.Setters>
                <Setter Property="BackgroundColor" Value="{AppThemeBinding Light=Yellow, Dark=Orange}" />
              </VisualState.Setters>
            </VisualState>
            <VisualState x:Name="PointerOver">
              <VisualState.Setters>
                <Setter Property="BackgroundColor" Value="Red" />
              </VisualState.Setters>
            </VisualState>
          </VisualStateGroup>
        </VisualStateGroupList>
      </VisualStateManager.VisualStateGroups>
    </Button>

    <Label Text="Second button has assigned PointerEntered and PointerExited gesture recognizers."/>
    <Button x:Name="button2" Text="Button 2">
      <Button.GestureRecognizers>
        <PointerGestureRecognizer PointerEntered="PointerGestureRecognizer_PointerEntered" PointerExited="PointerGestureRecognizer_PointerExited" />
      </Button.GestureRecognizers>
    </Button>

    <Label Text="Third button has assigned a pointer-over event in code behind."/>
    <Button x:Name="button3" Text="Button 3"/>

    <Label Text="Fourth button has assigned no pointer events."/>
    <Button x:Name="button4" Text="Button 4"/>

  </VerticalStackLayout>
</ContentPage>

MainPage.xaml.cs

namespace Maui.Controls.Sample;

public partial class MainPage : ContentPage
{
    public MainPage()
    {
  	  InitializeComponent();

  	  PointerGestureRecognizer pointerGestureRecognizer = new();
  	  pointerGestureRecognizer.PointerEnteredCommand = new Command(() =>
  	  {
  		  button3.Text = "Button 3 (pointer over)";
  		  button3.BackgroundColor = Colors.Red;
  	  });

  	  pointerGestureRecognizer.PointerExitedCommand = new Command(() =>
  	  {
  		  button3.Text = "Button 3";
  		  button3.BackgroundColor = null;
  	  });

  	  button3.GestureRecognizers.Add(pointerGestureRecognizer);
    }

    private void PointerGestureRecognizer_PointerEntered(object sender, PointerEventArgs e)
    {
  	  button2.Text = "Button 2 (pointer over)";
  	  button2.BackgroundColor = Colors.Red;
    }

    private void PointerGestureRecognizer_PointerExited(object sender, PointerEventArgs e)
    {
  	  button2.Text = "Button 2";
  	  button2.BackgroundColor = null;
    }
}

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[jonathanpeppers]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).