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.

Sign up for GitHub

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

[WinUI] Allocate less when updating gestures #21450

Merged
merged 7 commits into from
Apr 6, 2024
Merged

[WinUI] Allocate less when updating gestures #21450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
acabea4
`EnumerableExtensions`: Enable nullable
MartyIX Mar 26, 2024
19d7b69
Add `EnumerableExtensions.HasAnyGesturesFor`
MartyIX Mar 26, 2024
e984695
Sandbox measure code
MartyIX Mar 26, 2024
1b3612b
Revert "Sandbox measure code"
MartyIX Mar 26, 2024
39b7ec6
Do not create lambdas + apply MSVS suggestion regarding pattern match…
MartyIX Mar 26, 2024
edd66ef
Make `HasAnyGesturesFor` `internal`
MartyIX Mar 26, 2024
acadf48
Reimplement by adding `FirstGestureOrDefault`
MartyIX Mar 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 45 additions & 23 deletions src/Controls/src/Core/EnumerableExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,64 +1,86 @@
#nullable disable
using System;
using System.Collections.Generic;
using System.ComponentModel;

namespace Microsoft.Maui.Controls.Internals
{
static class EnumerableExtensions
{
public static bool HasChildGesturesFor<T>(this IEnumerable<GestureElement> elements, Func<T, bool> predicate = null) where T : GestureRecognizer
public static bool HasChildGesturesFor<T>(this IEnumerable<GestureElement>? elements, Func<T, bool>? predicate = null) where T : GestureRecognizer
{
if (elements == null)
if (elements is null)
{
return false;

if (predicate == null)
predicate = x => true;
}

foreach (var element in elements)
{
foreach (var item in element.GestureRecognizers)
{
var gesture = item as T;
if (gesture != null && predicate(gesture))
if (item is T gesture && (predicate is null || predicate(gesture)))
{
return true;
}
}
}

return false;
}

public static IEnumerable<T> GetChildGesturesFor<T>(this IEnumerable<GestureElement> elements, Func<T, bool> predicate = null) where T : GestureRecognizer
public static IEnumerable<T> GetChildGesturesFor<T>(this IEnumerable<GestureElement>? elements, Func<T, bool>? predicate = null) where T : GestureRecognizer
{
if (elements == null)
if (elements is null)
{
yield break;

if (predicate == null)
predicate = x => true;
}

foreach (var element in elements)
{
foreach (var item in element.GestureRecognizers)
{
var gesture = item as T;
if (gesture != null && predicate(gesture))
if (item is T gesture && (predicate is null || predicate(gesture)))
{
yield return gesture;
}
}
}
}

public static IEnumerable<T> GetGesturesFor<T>(this IEnumerable<IGestureRecognizer> gestures, Func<T, bool> predicate = null) where T : GestureRecognizer
/// <remarks>The method makes a defensive copy of the gestures.</remarks>
public static IEnumerable<T> GetGesturesFor<T>(this IEnumerable<IGestureRecognizer>? gestures, Func<T, bool>? predicate = null) where T : GestureRecognizer
{
if (gestures == null)
if (gestures is null)
{
yield break;

if (predicate == null)
predicate = x => true;
}

foreach (IGestureRecognizer item in new List<IGestureRecognizer>(gestures))
{
var gesture = item as T;
if (gesture != null && predicate(gesture))
if (item is T gesture && (predicate is null || predicate(gesture)))
{
yield return gesture;
}
}
}

internal static bool HasAnyGesturesFor<T>(this IEnumerable<IGestureRecognizer>? gestures, Func<T, bool>? predicate = null) where T : GestureRecognizer
=> FirstGestureOrDefault(gestures, predicate) is not null;

internal static T? FirstGestureOrDefault<T>(this IEnumerable<IGestureRecognizer>? gestures, Func<T, bool>? predicate = null) where T : GestureRecognizer
{
if (gestures is null)
{
return null;
}

foreach (IGestureRecognizer item in gestures)
{
if (item is T gesture && (predicate is null || predicate(gesture)))
{
return gesture;
}
}

return null;
}
}
}
32 changes: 19 additions & 13 deletions src/Controls/src/Core/Platform/GestureManager/GesturePlatformManager.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,28 +670,32 @@ void PinchComplete(bool success)

void UpdateDragAndDropGestureRecognizers()
{
if (_container == null)
if (_container is null)
{
return;
}

var view = Element as View;
IList<IGestureRecognizer>? gestures = view?.GestureRecognizers;

if (gestures == null)
if (gestures is null)
{
return;
}

_container.CanDrag = gestures.GetGesturesFor<DragGestureRecognizer>()
.FirstOrDefault()?.CanDrag ?? false;
bool canDrag = gestures.FirstGestureOrDefault<DragGestureRecognizer>()?.CanDrag ?? false;
_container.CanDrag = canDrag;

_container.AllowDrop = gestures.GetGesturesFor<DropGestureRecognizer>()
.FirstOrDefault()?.AllowDrop ?? false;
bool allowDrop = gestures.FirstGestureOrDefault<DropGestureRecognizer>()?.AllowDrop ?? false;
_container.AllowDrop = allowDrop;

if (_container.CanDrag)
if (canDrag)
{
_container.DragStarting += HandleDragStarting;
_container.DropCompleted += HandleDropCompleted;
}

if (_container.AllowDrop)
if (allowDrop)
{
_container.DragOver += HandleDragOver;
_container.Drop += HandleDrop;
Expand All @@ -714,7 +718,7 @@ void UpdatingGestureRecognizers()
IList<TapGestureRecognizer>? childGestures =
children?.GetChildGesturesFor<TapGestureRecognizer>().ToList();

if (gestures.GetGesturesFor<TapGestureRecognizer>(g => g.NumberOfTapsRequired == 1).Any()
if (gestures.HasAnyGesturesFor<TapGestureRecognizer>(g => g.NumberOfTapsRequired == 1)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should save 1 allocation.

|| children?.GetChildGesturesFor<TapGestureRecognizer>(g => g.NumberOfTapsRequired == 1).Any() == true)
{
_container.Tapped += OnTap;
Expand All @@ -728,7 +732,7 @@ void UpdatingGestureRecognizers()
}
}

if (gestures.GetGesturesFor<TapGestureRecognizer>(g => g.NumberOfTapsRequired == 1 || g.NumberOfTapsRequired == 2).Any()
if (gestures.HasAnyGesturesFor<TapGestureRecognizer>(g => g.NumberOfTapsRequired == 1 || g.NumberOfTapsRequired == 2)
|| children?.GetChildGesturesFor<TapGestureRecognizer>(g => g.NumberOfTapsRequired == 1 || g.NumberOfTapsRequired == 2).Any() == true)
{
_container.DoubleTapped += OnTap;
Expand All @@ -747,11 +751,13 @@ void UpdatingGestureRecognizers()
_container.PointerPressed += OnPgrPointerPressed;
_container.PointerReleased += OnPgrPointerReleased;

bool hasSwipeGesture = gestures.GetGesturesFor<SwipeGestureRecognizer>().GetEnumerator().MoveNext();
bool hasPinchGesture = gestures.GetGesturesFor<PinchGestureRecognizer>().GetEnumerator().MoveNext();
bool hasPanGesture = gestures.GetGesturesFor<PanGestureRecognizer>().GetEnumerator().MoveNext();
bool hasSwipeGesture = gestures.HasAnyGesturesFor<SwipeGestureRecognizer>();
bool hasPinchGesture = gestures.HasAnyGesturesFor<PinchGestureRecognizer>();
bool hasPanGesture = gestures.HasAnyGesturesFor<PanGestureRecognizer>();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 allocations here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So one thing to consider is this would still iterate over the gestures list three times. How many are normally in this list? If it's usually like 1-5, that might be OK.

Copy link
Contributor Author

@MartyIX MartyIX Mar 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the list contains only a handful of items as you said.

if (!hasSwipeGesture && !hasPinchGesture && !hasPanGesture)
{
return;
}

//We can't handle ManipulationMode.Scale and System , so we don't support pinch/pan on a scrollview
if (Element is ScrollView)
Expand Down
Loading