[Sensors] OrientationSensor.populateMatrix() works also with Float64Array and DOMMatrix types

third_party/WebKit/Source/modules/sensor/OrientationSensor.cpp

Index: third_party/WebKit/Source/modules/sensor/OrientationSensor.cpp
diff --git a/third_party/WebKit/Source/modules/sensor/OrientationSensor.cpp b/third_party/WebKit/Source/modules/sensor/OrientationSensor.cpp
index 7a4ea244f1319c811d9e28831af458ee08d72e69..c04e065940004d6f215b10dbb863e467d8e5104d 100644
--- a/third_party/WebKit/Source/modules/sensor/OrientationSensor.cpp
+++ b/third_party/WebKit/Source/modules/sensor/OrientationSensor.cpp
@@ -5,6 +5,7 @@
 #include "modules/sensor/OrientationSensor.h"
 
 #include "bindings/core/v8/ExceptionState.h"
+#include "core/geometry/DOMMatrix.h"
 
 using device::mojom::blink::SensorType;
 
@@ -20,9 +21,65 @@ Vector<double> OrientationSensor::quaternion(bool& isNull) {
                                   readingValueUnchecked(2)});  // Vz
 }
 
-void OrientationSensor::populateMatrix(DOMFloat32Array* buffer,
-                                       ExceptionState& exceptionState) {
-  if (buffer->length() < 16) {
+template <typename T>
+void doPopulateMatrix(T* targetMatrix, double x, double y, double z, double w) {
+  auto out = targetMatrix->data();
+  out[0] = 1.0 - 2 * (y * y + z * z);
+  out[1] = 2 * (x * y - z * w);
+  out[2] = 2 * (x * z + y * w);
+  out[3] = 0.0;
+  out[4] = 2 * (x * y + z * w);
+  out[5] = 1.0 - 2 * (x * x + z * z);
+  out[6] = 2 * (y * z - x * w);
+  out[7] = 0.0;
+  out[8] = 2 * (x * z - y * w);
+  out[9] = 2 * (y * z + x * w);
+  out[10] = 1.0 - 2 * (x * x + y * y);
+  out[11] = 0.0;
+  out[12] = 0.0;
+  out[13] = 0.0;
+  out[14] = 0.0;
+  out[15] = 1.0;
+}
+
+template <>
+void doPopulateMatrix(DOMMatrix* targetMatrix,
+                      double x,
+                      double y,
+                      double z,
+                      double w) {
+  targetMatrix->setM11(1.0 - 2 * (y * y + z * z));

[Reilly Grant (use Gerrit), 2017/03/28 18:23:03]

It would be nice if DOMMatrix provided a way to call m_matrix->setMatrix() so
you could set all these values in a single call.

+  targetMatrix->setM12(2 * (x * y - z * w));
+  targetMatrix->setM13(2 * (x * z + y * w));
+  targetMatrix->setM14(0.0);
+  targetMatrix->setM21(2 * (x * y + z * w));
+  targetMatrix->setM22(1.0 - 2 * (x * x + z * z));
+  targetMatrix->setM23(2 * y * z - 2 * x * w);
+  targetMatrix->setM24(0.0);
+  targetMatrix->setM31(2 * (x * z - y * w));
+  targetMatrix->setM32(2 * (y * z + x * w));
+  targetMatrix->setM33(1.0 - 2 * (x * x + y * y));
+  targetMatrix->setM34(0.0);
+  targetMatrix->setM41(0.0);
+  targetMatrix->setM42(0.0);
+  targetMatrix->setM43(0.0);
+  targetMatrix->setM44(1.0);
+}
+
+template <typename T>
+bool checkBufferLength(T* buffer) {
+  return buffer->length() >= 16;
+}
+
+template <>
+bool checkBufferLength(DOMMatrix*) {
+  return true;
+}
+
+template <typename Matrix>
+void OrientationSensor::populateMatrixInternal(Matrix* targetMatrix,

[Reilly Grant (use Gerrit), 2017/03/28 18:23:03]

I would merge this function into populateMatrix because otherwise you are
duplicating a bunch of code that is identical regardless of the matrix type.

[Mikhail, 2017/03/29 08:27:58]

Merging looks a bit problematic as there are two template calls in this
function: in the beginning (checkBufferLength) and in the end
(doPopulateMatrix), so extracting somehow the common code in the middle would
IMO affect readability.
Given that all these template functions are not exported and only used inside
this .cpp I do not think binary size will be an issue here, wdyt?

[Reilly Grant (use Gerrit), 2017/03/29 15:31:22]

I would write populateMatrix like this, which only duplicates one of the error
checks:

void OrientationSensor::populateMatrix(
    Float32ArrayOrFloat64ArrayOrDOMMatrix& matrix,
    ExceptionState& exceptionState) {
  if (!isActivated()) {
    exceptionState.throwDOMException(
        InvalidStateError, "The sensor must be in 'connected' state.");
    return;
  }
  if (!canReturnReadings())
    return;

  double x = readingValueUnchecked(0);
  double y = readingValueUnchecked(1);
  double z = readingValueUnchecked(2);
  double w = readingValueUnchecked(3);

  // You may even pre-compute the 3x3 matrix here.

  if (matrix.isFloat32Array()) {
    if (matrix.getAsFloat32Array()->length() < 16) {
      exceptionState.throwTypeError(
          "Target buffer must have at least 16 elements.");
      return;
    }
    populateMatrixInternal(matrix.getAsFloat32Array(), x, y, z, w);
  } else if (matrix.isFloat64Array()) {
    if (matrix.getAsFloat64Array()->length() < 16) {
      exceptionState.throwTypeError(
          "Target buffer must have at least 16 elements.");
      return;
    }
    populateMatrixInternal(matrix.getAsFloat64Array(), x, y, z, w);
  } else if (matrix.isDOMMatrix()) {
    populateMatrixInternal(matrix.getAsDOMMatrix(), x, y, z, w);
  } else {
    NOTREACHED() << "Unexpected rotation matrix type."
  }
}

+                                               ExceptionState& exceptionState) {
+  if (!checkBufferLength(targetMatrix)) {
     exceptionState.throwTypeError(
         "Target buffer must have at least 16 elements.");
     return;
@@ -35,22 +92,25 @@ void OrientationSensor::populateMatrix(DOMFloat32Array* buffer,
   if (!canReturnReadings())
     return;
 
-  float x = readingValueUnchecked(0);
-  float y = readingValueUnchecked(1);
-  float z = readingValueUnchecked(2);
-  float w = readingValueUnchecked(3);
+  double x = readingValueUnchecked(0);
+  double y = readingValueUnchecked(1);
+  double z = readingValueUnchecked(2);
+  double w = readingValueUnchecked(3);
 
-  float* out = buffer->data();
-  out[0] = 1.0 - 2 * (y * y - z * z);
-  out[1] = 2 * (x * y - z * w);
-  out[2] = 2 * (x * z + y * w);
-  out[4] = 2 * (x * y + z * w);
-  out[5] = 1.0 - 2 * (x * x - z * z);
-  out[6] = 2 * (y * z - x * w);
-  out[8] = 2 * (x * z - y * w);
-  out[9] = 2 * (y * z + x * w);
-  out[10] = 1.0 - 2 * (x * x - y * y);
-  out[15] = 1.0;
+  doPopulateMatrix(targetMatrix, x, y, z, w);
+}
+
+void OrientationSensor::populateMatrix(
+    Float32ArrayOrFloat64ArrayOrDOMMatrix& matrix,
+    ExceptionState& exceptionState) {
+  if (matrix.isFloat32Array())
+    populateMatrixInternal(matrix.getAsFloat32Array(), exceptionState);
+  else if (matrix.isFloat64Array())
+    populateMatrixInternal(matrix.getAsFloat64Array(), exceptionState);
+  else if (matrix.isDOMMatrix())
+    populateMatrixInternal(matrix.getAsDOMMatrix(), exceptionState);
+  else
+    NOTREACHED() << "Unexpected rotation matrix type.";
 }
 
 bool OrientationSensor::isReadingDirty() const {