Fix motion event orientation handling for styluses.

ui/events/blink/blink_event_util.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // MSVC++ requires this to be set before any other includes to get M_PI.
 #define _USE_MATH_DEFINES
 
 #include "ui/events/blink/blink_event_util.h"
 
 #include <cmath>
@@ skipping 85 matching lines @@
   float major_radius = event.GetTouchMajor(pointer_index) / 2.f;
   float minor_radius = event.GetTouchMinor(pointer_index) / 2.f;
 
   DCHECK_LE(minor_radius, major_radius);
   DCHECK_IMPLIES(major_radius, minor_radius);
 
   float orientation_deg = event.GetOrientation(pointer_index) * 180.f / M_PI;
   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
     // unchanged (and 0 is the default value for many devices that don't
     // report elliptical touches).
     touch.radiusX = minor_radius;
     touch.radiusY = major_radius;
     touch.rotationAngle = orientation_deg;
   } else {
     touch.radiusX = major_radius;
@@ skipping 175 matching lines @@
 }
 
 WebGestureEvent CreateWebGestureEventFromGestureEventData(
     const GestureEventData& data) {
   return CreateWebGestureEvent(data.details, data.time - base::TimeTicks(),
                                gfx::PointF(data.x, data.y),
                                gfx::PointF(data.raw_x, data.raw_y), data.flags);
 }
 
 }  // namespace ui