Fix motion event orientation handling for styluses.

ui/events/blink/blink_event_util.cc

Index: ui/events/blink/blink_event_util.cc
diff --git a/ui/events/blink/blink_event_util.cc b/ui/events/blink/blink_event_util.cc
index 9a3a648ae3306e5517b87255acd1c5d0f1fac847..75b9e0da97ff92191282a3c895fb38d8ce3edaaf 100644
--- a/ui/events/blink/blink_event_util.cc
+++ b/ui/events/blink/blink_event_util.cc
@@ -103,9 +103,23 @@ WebTouchPoint CreateWebTouchPoint(const MotionEvent& event,
   DCHECK_GE(major_radius, 0);
   DCHECK_GE(minor_radius, 0);
   DCHECK_GE(major_radius, minor_radius);
-  // Allow a small bound tolerance to account for floating point conversion.
-  DCHECK_GT(orientation_deg, -90.01f);
-  DCHECK_LT(orientation_deg, 90.01f);
+  if (event.GetToolType(pointer_index) == MotionEvent::TOOL_TYPE_STYLUS) {

[jdduke (slow), 2015/05/29 14:36:14]

I guess at some point we should add some basic test coverage to these conversion
routines, but I won't ask you to add those.

+    // Orientation lies in [-180, 180] for a stylus.

[tdresser, 2015/05/29 15:01:21]

Use () for open ranges.

[USE eero AT chromium.org, 2015/05/29 16:17:47]

It is not an open but a closed range.

From
http://developer.android.com/reference/android/view/MotionEvent.html#AXIS_ORI...

[tdresser, 2015/05/29 16:45:44]

Sorry, you're right.

+    // Allow a small bound tolerance to account for floating point conversion.
+    DCHECK_GT(orientation_deg, -180.01f);
+    DCHECK_LT(orientation_deg, 180.01f);

[mustaq, 2015/05/29 15:11:51]

Please create a bug saying that "When Touch.rotationAngle represents a stylus
orientation, because we are forced to limit the angle to [0,90) as per the TE
spec, the angle is exact in one quadrant, off by 90 degree in two quadrants, and
off by 180 degrees in one quadrant". Also add a TODO here pointing to the bug.

[USE eero AT chromium.org, 2015/05/29 16:17:47]

Done.

+    if (orientation_deg >= 90.f)

[jdduke (slow), 2015/05/29 14:36:14]

Where does the spec say it's strictly less than 90 degrees? Can't seem to find
it.

[USE eero AT chromium.org, 2015/05/29 16:17:47]

In section 2.1 the spec says that than rotationAngle must be greater than or
equal to (i.e. >=) 0 and less than (i.e. <) 90.

+      orientation_deg -= 180.f;
+    else if (orientation_deg < -90.f)
+      orientation_deg += 180.f;
+  } else {
+    // Orientation lies in [-90, 90] for a stylus.

[tdresser, 2015/05/29 15:01:21]

stylus -> touch.

[USE eero AT chromium.org, 2015/05/29 16:17:47]

Done.

+    // Allow a small bound tolerance to account for floating point conversion.
+    DCHECK_GT(orientation_deg, -90.01f);
+    DCHECK_LT(orientation_deg, 90.01f);
+    if (orientation_deg >= 90.f)

[jdduke (slow), 2015/05/29 14:36:14]

Nit: If we DCHECK, we shouldn't actually fix it if it's wrong, so I would remove
this >= adjustment branch.

[USE eero AT chromium.org, 2015/05/29 16:17:47]

There is nothing wrong for orientation_deg to be equal to 90. We also allow that
in DCHECKs. We also allow a small tolerance, thus there is nothing wrong for
orientation_deg to be in [90, 90.01).

Here I normalise orientation_deg from a closed range [-90, 90] to a half-open
range [-90, 90).
That is because the set {-90, 90} forms an equivalence class thus these values
should be handled in the same way. The easiest way to do that (IMO) is to
normalise these values to -90. Another way would be add a third branch to the if
block on the line 123.

+      orientation_deg -= 180.f;
+  }
   if (orientation_deg >= 0) {
     // The case orientation_deg == 0 is handled here on purpose: although the
     // 'else' block is equivalent in this case, we want to pass the 0 value