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

feat(PinchGestureRecognizer): Report Pinch angle in degrees #13244

Merged
merged 8 commits into from
Nov 23, 2023
Merged

feat(PinchGestureRecognizer): Report Pinch angle in degrees #13244

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bcb24df
feat(PinchGestureRecognizer): Report Pinch angle in degrees
workgroupengineering Oct 12, 2023
f7bdd65
fix: Address review
workgroupengineering Oct 13, 2023
f39c585
fix: Angle calculation
workgroupengineering Oct 14, 2023
38d84f0
fix: Prevent Gesture Recognition from other Recognition
workgroupengineering Oct 14, 2023
98bfceb
feat: Add Sample
workgroupengineering Oct 14, 2023
e92bd22
fix: make GetDistance static
workgroupengineering Oct 16, 2023
3dbaf93
feat(PinchGestureRecognizer): AngleDelta
workgroupengineering Oct 16, 2023
7f3e8ae
Merge branch 'master' into features/Core/Gestures/PinchGesture-Angle
workgroupengineering Oct 18, 2023
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
16 changes: 14 additions & 2 deletions src/Avalonia.Base/Input/GestureRecognizers/PinchGestureRecognizer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ protected override void PointerMoved(PointerEventArgs e)
{
if (Target != null && Target is Visual visual)
{
if(_firstContact == e.Pointer)
if (_firstContact == e.Pointer)
{
_firstPoint = e.GetPosition(visual);
}
Expand All @@ -50,7 +50,19 @@ protected override void PointerMoved(PointerEventArgs e)

var scale = distance / _initialDistance;

var pinchEventArgs = new PinchEventArgs(scale, _origin);
var point = _firstPoint;
if (point.X > _origin.X)
{
point = _secondPoint;
}

// https://stackoverflow.com/a/15994225/20894223
var deltaX = point.X - _origin.X;
var deltaY = -(point.Y - _origin.Y); // I reverse the sign, because on the screen the Y axes are reversed with respect to the Cartesian plane.
var rad = System.Math.Atan2(deltaX, deltaY); // radiant
Copy link
Contributor

Choose a reason for hiding this comment

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

"radiant" is "radians" in English

var degree = rad * (180 / System.Math.PI);
degree += 90; // Shift angle of 90°
Copy link
Contributor

@robloo robloo Oct 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

  1. Why is this +90 degrees done?
  2. I wonder if the angle should be normalized between 0 and 180 degrees. This would remove the directionality of the start/end points. I'm not sure a start and end point even make sense in the context of a pinch. In a pinch how is 300 degrees different from 120 degrees?
    • Edit: It is common for pinch to be used on object directly when interacting with them on screen. In this case you may want to manipulate the object using pinch AND rotate it at the same time. In this context the vector of the pinch event and the full 360deg rotation is important info to have.

Also, I'm assuming WPF's coordinates system for measure degrees:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

90° is wrog, the corrent 180. Atan2 returns a radian value between -π to +π, in degrees -180 to +180. To get the angle between 0 and 360 degrees you need to add 180 degrees.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if the angle should be normalized between 0 and 180 degrees. This would remove the directionality of the start/end points. I'm not sure a start and end point even make sense in the context of a pinch. In a pinch how is 300 degrees different from 120 degrees?

* Edit: It is common for pinch to be used on object directly when interacting with them on screen. In this case you may want to manipulate the object using pinch AND rotate it at the same time. In this context the vector of the pinch event and the full 360deg rotation is important info to have.

Do you suggest adding a Sing or AngleDelta property?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let me think about this some more and compare to other platforms:

adding a Sign

Actually, yes, I think is probably a good idea. + is clockwise. - is counterclockwise.

AngleDelta

Yea... that might be useful. I also wonder if we should track (or if it is even possible to track) rotations past 360 degrees. If a user wants to just keep rotating something during a pin/zoom/rotate operation triggered by this gesture it might be useful.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've been thinking about this more. I realize it is a larger topic. We really are starting to add features that should be in a generic ManipulationStarted/Completed event as in WinUI.

Specifically for the angle delta, ManipulationDelta in WinUI is a generic representation of everything you would want to know about the gesture. We probably should be using that.

I still think Pinch is a useful special-case of the fully generic Manipulation gesture (which doesn't exist yet). But I guess we either 1) should exclude delta in pinch for now or 2) design the manipulation gesture as well so event args and data types can be shared with the pinch gesture.

The most future-proof is to implement ManipulationDelta now within Pinch. Maybe even a ManipulationCompletedEventArgs-similar class.

Looking at other platforms, https://developer.apple.com/documentation/uikit/uirotationgesturerecognizer, I notice the name Rotation as well. This should be the name of the property instead of Angle. It is also what is done in ManipulationDelta and WinUI.

var pinchEventArgs = new PinchEventArgs(scale, _origin, degree);
Target?.RaiseEvent(pinchEventArgs);

e.Handled = pinchEventArgs.Handled;
Expand Down
16 changes: 14 additions & 2 deletions src/Avalonia.Base/Input/PinchEventArgs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,32 @@ namespace Avalonia.Input
{
public class PinchEventArgs : RoutedEventArgs
{
public PinchEventArgs(double scale, Point scaleOrigin) : base(Gestures.PinchEvent)
public PinchEventArgs(double scale, Point scaleOrigin) : base(Gestures.PinchEvent)
{
Scale = scale;
ScaleOrigin = scaleOrigin;
}

public PinchEventArgs(double scale, Point scaleOrigin, double degree) : base(Gestures.PinchEvent)
Copy link
Contributor

Choose a reason for hiding this comment

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

double degree should really be double angle to follow standard parameter naming conventions. Degrees would then be in comments.

{
Scale = scale;
ScaleOrigin = scaleOrigin;
Angle = degree;
}

public double Scale { get; } = 1;

public Point ScaleOrigin { get; }

/// <summary>
/// Pinch angle in degrees
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be a lot more descriptive. Also needs to start with "Gets" and have a "." at the end to avoid analyzer warnings and follow conventions.

/// <summary>
/// Gets the angle of the pinch gesture, in degrees.
/// <summary>
/// <remarks>
/// A pinch gesture is the movement of two pressed points closer together. This property is the measured angle of the line between those two points. Remember zero degrees is a line pointing up.
/// </remarks>

/// </summary>
public double Angle { get; }
}

public class PinchEndedEventArgs : RoutedEventArgs
{
public PinchEndedEventArgs() : base(Gestures.PinchEndedEvent)
public PinchEndedEventArgs() : base(Gestures.PinchEndedEvent)
{
}
}
Expand Down