Implements computation of touch calibration transform using user provided data

ash/touch/touch_transformer_controller.cc

Index: ash/touch/touch_transformer_controller.cc
diff --git a/ash/touch/touch_transformer_controller.cc b/ash/touch/touch_transformer_controller.cc
index 020561faebcaa5d32dc39c2b893cc040f0b41a3e..cf34cf1ac674a24056d8ba22f998e96b96c33c60 100644
--- a/ash/touch/touch_transformer_controller.cc
+++ b/ash/touch/touch_transformer_controller.cc
@@ -8,6 +8,7 @@
 #include "ash/host/ash_window_tree_host.h"
 #include "ash/root_window_controller.h"
 #include "ash/shell.h"
+#include "third_party/skia/include/core/SkMatrix44.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/display/display_layout.h"
 #include "ui/display/manager/chromeos/display_configurator.h"
@@ -35,6 +36,84 @@ ui::TouchscreenDevice FindTouchscreenById(int id) {
   return ui::TouchscreenDevice();
 }
 
+// Given an array of touch point and display point pairs, this function computes
+// and returns the constants(defined below) using a least fit algorithm.
+// If (xt, yt) is a touch point then its corresponding (xd, yd) would be defined
+// by the following 2 equations:
+// xd = xt * A + yt * B + C
+// yd = xt * D + yt * E + F
+// This function computes and returns A, B, C, D, E and F as a pair of
+// SkVector4.
+// See http://crbug.com/672293
+std::pair<SkVector4, SkVector4> SolveCalibrationEquation(
+    std::array<std::pair<gfx::Point, gfx::Point>, 4> touch_point_pairs,
+    const gfx::Transform& pre_calibration_tm) {
+  // If {(xt_1, yt_1), (xt_2, yt_2), (xt_3, yt_3)....} are a set of touch points
+  // received during calibration, then the |touch_point_matrix| would be defined
+  // as:
+  // |xt_1  yt_1  1  0|
+  // |xt_2  yt_2  1  0|
+  // |xt_3  yt_3  1  0|
+  // |xt_4  yt_4  1  0|
+  SkMatrix44 touch_point_matrix;

[kylechar, 2016/12/19 20:00:21]

Can you move variables closer to where they are first used?

[malaykeshav, 2016/12/20 09:56:04]

Done

+  SkMatrix44 touch_point_matrix_transpose;
+
+  SkMatrix44 product_matrix;
+  SkMatrix44 product_matrix_inverse;
+
+  // Transform the display points before solving the equation.
+  // If the calibration was performed at a resolution that is 0.5 times the
+  // current resolution, then the display points (x, y) for a given touch point
+  // now represents a display point at (2 * x, 2 * y). This and other kinds of
+  // similar tranforms can be applied using |pre_calibration_tm|.
+  for (int row = 0; row < 4; row++)
+    pre_calibration_tm.TransformPoint(&touch_point_pairs[row].first);
+
+  // Vector of the X-coordinate of display points corresponding to each of the
+  // touch points.
+  SkVector4 display_points_x(
+      touch_point_pairs[0].first.x(), touch_point_pairs[1].first.x(),
+      touch_point_pairs[2].first.x(), touch_point_pairs[3].first.x());
+  // Vector of the Y-coordinate of display points corresponding to each of the
+  // touch points.
+  SkVector4 display_points_y(
+      touch_point_pairs[0].first.y(), touch_point_pairs[1].first.y(),
+      touch_point_pairs[2].first.y(), touch_point_pairs[3].first.y());
+
+  SkVector4 result_constants_x, result_constants_y;
+
+  // Initialize |touch_point_matrix|
+  for (int row = 0; row < 4; row++) {
+    touch_point_matrix.set(row, 0, touch_point_pairs[row].second.x());
+    touch_point_matrix.set(row, 1, touch_point_pairs[row].second.y());
+    touch_point_matrix.set(row, 2, 1);
+    touch_point_matrix.set(row, 3, 0);
+  }
+  touch_point_matrix_transpose = touch_point_matrix;
+  touch_point_matrix_transpose.transpose();
+
+  product_matrix = touch_point_matrix_transpose * touch_point_matrix;
+
+  // Set (3, 3) = 1 so that |determinent| of the matrix is != 0 and the inverse
+  // can be calculated.
+  product_matrix.set(3, 3, 1);
+
+  // #NOTE: If the determinent is zero then the inverse cannot be computed. The
+  // only solution is to restart touch calibration and get new points from user.
+  CHECK(product_matrix.invert(&product_matrix_inverse));

[kylechar, 2016/12/19 20:00:21]

Is crashing the correct thing to do if the inverse can't be computed the best
solution here? I'd think you would want to invalidate the calibration data and
warn the user or something?

[malaykeshav, 2016/12/20 09:56:04]

This is an extremely rare case since we are using floating points. The inverse
will almost always exist.
The tradeoff to handling this case doesnt seem worth it.

+
+  product_matrix_inverse.set(3, 3, 0);
+
+  product_matrix = product_matrix_inverse * touch_point_matrix_transpose;
+
+  // Vector containing the constants [A, B, C, 0]
+  result_constants_x = product_matrix * display_points_x;
+  // Vector containing the constants [D, E, F, 0]
+  result_constants_y = product_matrix * display_points_y;
+
+  return std::make_pair(result_constants_x, result_constants_y);

[kylechar, 2016/12/19 20:00:21]

Why doesn't this just return the gfx::Transform you build in stored_ctm?

[malaykeshav, 2016/12/20 09:56:04]

Done.

+}
+
 }  // namespace
 
 // This is to compute the scale ratio for the TouchEvent's radius. The
@@ -79,42 +158,103 @@ gfx::Transform TouchTransformerController::GetTouchTransform(
 #endif
 
   gfx::Transform ctm;
+  gfx::Transform stored_ctm;

[kylechar, 2016/12/19 20:00:21]

Define variable where you're going to use it.

[malaykeshav, 2016/12/20 09:56:04]

Done.

 
   if (current_size.IsEmpty() || touch_native_size.IsEmpty() ||
       touch_area.IsEmpty() || touchscreen.id == ui::InputDevice::kInvalidId)
     return ctm;
 
 #if defined(USE_OZONE)
-  // Translate the touch so that it falls within the display bounds.
-  ctm.Translate(display.bounds_in_native().x(), display.bounds_in_native().y());
+  // Translate the touch so that it falls within the display bounds. This should
+  // not be performed if the displays are mirrored.
+  if (display.id() == touch_display.id())
+    ctm.Translate(display.bounds_in_native().x(),
+                  display.bounds_in_native().y());
 #endif
 
-  // Take care of panel fitting only if supported. Panel fitting is emulated in
-  // software mirroring mode (display != touch_display).
-  // If panel fitting is enabled then the aspect ratio is preserved and the
-  // display is scaled acordingly. In this case blank regions would be present
-  // in order to center the displayed area.
-  if (display.is_aspect_preserving_scaling() ||
-      display.id() != touch_display.id()) {
-    float touch_native_ar =
-        touch_native_size.width() / touch_native_size.height();
+  // If the device is currently under calibration, then do not return any
+  // transform as we want to use the raw native touch input data for calibration
+  if (is_calibrating_)
+    return ctm;
+  // If touch calibration data is unavailable, use naive approach.
+  if (!touch_display.has_touch_calibration_data()) {

[kylechar, 2016/12/19 20:00:21]

Could you split this function into something like ComputeUncalibratedTransform()
vs ComputeCalibratedTransform()? It would help with readability.

[malaykeshav, 2016/12/20 09:56:04]

Splitting the uncalibrated transform and moving to a separate function.
Leaving the calibrated transform as is.

+    // Take care of panel fitting only if supported. Panel fitting is emulated
+    // in software mirroring mode (display != touch_display).
+    // If panel fitting is enabled then the aspect ratio is preserved and the
+    // display is scaled acordingly. In this case blank regions would be present
+    // in order to center the displayed area.
+    if (display.is_aspect_preserving_scaling() ||
+        display.id() != touch_display.id()) {
+      float touch_calib_ar =
+          touch_native_size.width() / touch_native_size.height();
+      float current_ar = current_size.width() / current_size.height();
+
+      if (current_ar > touch_calib_ar) {  // Letterboxing
+        ctm.Translate(
+            0, (1 - current_ar / touch_calib_ar) * 0.5 * current_size.height());
+        ctm.Scale(1, current_ar / touch_calib_ar);
+      } else if (touch_calib_ar > current_ar) {  // Pillarboxing
+        ctm.Translate(
+            (1 - touch_calib_ar / current_ar) * 0.5 * current_size.width(), 0);
+        ctm.Scale(touch_calib_ar / current_ar, 1);
+      }
+    }
+    // Take care of scaling between touchscreen area and display resolution.
+    ctm.Scale(current_size.width() / touch_area.width(),
+              current_size.height() / touch_area.height());
+    return ctm;
+  }
+  // The resolution at which the touch calibration was performed.
+  auto touch_calib_size =

[kylechar, 2016/12/19 20:00:21]

gfx::SizeF touch_calib_size(...)

[malaykeshav, 2016/12/20 09:56:04]

Done

+      gfx::SizeF(touch_display.GetTouchCalibrationData().bounds);
+
+  // Any additional tranfomration that needs to be applied to the display

[kylechar, 2016/12/19 20:00:21]

transformation

[malaykeshav, 2016/12/20 09:56:04]

done

+  // points, before we solve for the final transform.
+  gfx::Transform pre_transform;
+
+  if (display.id() != touch_display.id() ||
+      display.is_aspect_preserving_scaling()) {
+    // Case of displays being mirrored or in panel fitting mode.
+    // Aspect ratio of the touch display's resolution during calibration.
+    float calib_ar = touch_calib_size.width() / touch_calib_size.height();
+    // Aspect ratio of the display that is being mirrored.
     float current_ar = current_size.width() / current_size.height();
 
-    if (current_ar > touch_native_ar) {  // Letterboxing
-      ctm.Translate(
-          0, (1 - current_ar / touch_native_ar) * 0.5 * current_size.height());
-      ctm.Scale(1, current_ar / touch_native_ar);
-    } else if (touch_native_ar > current_ar) {  // Pillarboxing
-      ctm.Translate(
-          (1 - touch_native_ar / current_ar) * 0.5 * current_size.width(), 0);
-      ctm.Scale(touch_native_ar / current_ar, 1);
+    if (current_ar < calib_ar) {
+      pre_transform.Scale(current_size.height() / touch_calib_size.height(),
+                          current_size.height() / touch_calib_size.height());
+      pre_transform.Translate(
+          (current_ar / calib_ar - 1.f) * touch_calib_size.width() * 0.5f, 0);
+    } else {
+      pre_transform.Scale(current_size.width() / touch_calib_size.width(),
+                          current_size.width() / touch_calib_size.width());
+      pre_transform.Translate(
+          0, (calib_ar / current_ar - 1.f) * touch_calib_size.height() * 0.5f);
     }
+  } else {
+    // Case of current resolution being different from the resolution when the
+    // touch calibration was performed.
+    pre_transform.Scale(current_size.width() / touch_calib_size.width(),
+                        current_size.height() / touch_calib_size.height());
   }
+  // Calibrate using the calibration data associated with the display.
+  std::pair<SkVector4, SkVector4> result = SolveCalibrationEquation(

[kylechar, 2016/12/19 20:00:21]

Same as above, if this returned the gfx::Transform it would make more sense.

[malaykeshav, 2016/12/20 09:56:04]

done

+      touch_display.GetTouchCalibrationData().point_pairs, pre_transform);
+
+  // Constants [A, B, C, 0] used to calibrate the x-coordinate of touch input.
+  // x_new = x_old * A + y_old * B + C;
+  const SkVector4& x_constants = result.first;
+  // Constants [D, E, F, 0] used to calibrate the y-coordinate of touch input.
+  // y_new = x_old * D + y_old * E + F;
+  const SkVector4& y_constants = result.second;
 
-  // Take care of scaling between touchscreen area and display resolution.
-  ctm.Scale(current_size.width() / touch_area.width(),
-            current_size.height() / touch_area.height());
-  return ctm;
+  // Create a transform matrix using the touch calibration data.
+  stored_ctm = gfx::Transform(x_constants.fData[0], x_constants.fData[1], 0,
+                              x_constants.fData[2], y_constants.fData[0],
+                              y_constants.fData[1], 0, y_constants.fData[2], 0,
+                              0, 1, 0, 0, 0, 0, 1);
+  stored_ctm.ConcatTransform(ctm);
+  return stored_ctm;
 }
 
 TouchTransformerController::TouchTransformerController() {
@@ -226,4 +366,9 @@ void TouchTransformerController::OnDisplayConfigurationChanged() {
   UpdateTouchTransformer();
 }
 
+void TouchTransformerController::SetForCalibration(bool is_calibrating) {
+  is_calibrating_ = is_calibrating;
+  UpdateTouchTransformer();
+}
+
 }  // namespace ash