feat(PinchGestureRecognizer): Report Pinch angle in degrees #13244
Conversation
|
You can test this PR using the following package version. |
| { | ||
| Scale = scale; | ||
| ScaleOrigin = scaleOrigin; | ||
| } | ||
|
|
||
| public PinchEventArgs(double scale, Point scaleOrigin, double degree) : base(Gestures.PinchEvent) |
There was a problem hiding this comment.
double degree should really be double angle to follow standard parameter naming conventions. Degrees would then be in comments.
| public double Scale { get; } = 1; | ||
|
|
||
| public Point ScaleOrigin { get; } | ||
|
|
||
| /// <summary> | ||
| /// Pinch angle in degrees |
There was a problem hiding this comment.
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>
| // 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 |
There was a problem hiding this comment.
"radiant" is "radians" in English
| 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 | ||
| var degree = rad * (180 / System.Math.PI); | ||
| degree += 90; // Shift angle of 90° |
There was a problem hiding this comment.
- Why is this +90 degrees done?
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:
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
You can test this PR using the following package version. |
|
@emmauss do I need to make any other changes? |
workgroupengineering
deleted the
features/Core/Gestures/PinchGesture-Angle
branch
What does the pull request do?
Report Pinch angle in degrees
What is the current behavior?
What is the updated/expected behavior with this PR?
How was the solution implemented (if it's not obvious)?
PinchEventArgsctororverloadPinchEventArgsctororverload read only Agnle propertyChecklist
Breaking changes
Obsoletions / Deprecations
Fixed issues
Fixes #13227