Display images in multiline Braille

chrome/browser/resources/chromeos/chromevox/braille/braille_display_manager.js

Index: chrome/browser/resources/chromeos/chromevox/braille/braille_display_manager.js
diff --git a/chrome/browser/resources/chromeos/chromevox/braille/braille_display_manager.js b/chrome/browser/resources/chromeos/chromevox/braille/braille_display_manager.js
index ab17296192861b8d25991ed2463da568109a1638..313df159bfa0444ecd1919d900d438c90d78d868 100644
--- a/chrome/browser/resources/chromeos/chromevox/braille/braille_display_manager.js
+++ b/chrome/browser/resources/chromeos/chromevox/braille/braille_display_manager.js
@@ -57,8 +57,8 @@ cvox.BrailleDisplayManager = function(translatorManager) {
    * @type {!cvox.BrailleDisplayState}
    * @private
    */
-  this.displayState_ = {available: false, textRowCount: undefined,
-      textColumnCount: undefined};
+  this.displayState_ = {available: false, textRowCount: 0,
+      textColumnCount: 0};
   /**
    * State reported from the chrome api, reflecting a real hardware
    * display.
@@ -67,6 +67,13 @@ cvox.BrailleDisplayManager = function(translatorManager) {
    */
   this.realDisplayState_ = this.displayState_;
 
+  /**
+   * @type {boolean} True if we're currently displaying an image, false if
+   *     we're displaying text.

[David Tseng, 2017/02/24 05:39:46]

No longer needed.

[dmazzoni, 2017/02/27 06:56:24]

Done.

+   * @private
+   */
+  this.displayingImage_ = false;
+
   translatorManager.addChangeListener(function() {
     this.translateContent_(this.content_, this.expansionType_);
   }.bind(this));
@@ -118,11 +125,80 @@ cvox.BrailleDisplayManager.CURSOR_DOTS_ = 1 << 6 | 1 << 7;
  */
 cvox.BrailleDisplayManager.prototype.setContent = function(
     content, expansionType) {
+  this.displayingImage_ = false;
   this.translateContent_(content, expansionType);
 };
 
 
 /**
+ * Takes an image, in the form of a data url, and displays it in braille

[David Tseng, 2017/02/24 05:39:45]

Would be helpful to include a note of how this is done e.g. maps a pixel to a
braille dot so we "increase" the image to a multiple of the braille cell's
dimensions. Good to remember, for example, when someone wants to do 2x3 cells.

+ * onto the physical braille display and the virtual braille captions display.
+ * @param {!string} imageUrl The image, in the form of a data url.
+ */
+cvox.BrailleDisplayManager.prototype.setImageContent = function(imageUrl) {
+  this.displayingImage_ = true;

[David Tseng, 2017/02/24 05:39:45]

Move this below the check for availability.

[dmazzoni, 2017/02/27 06:56:23]

Done.

+  if (!this.displayState_.available) {
+    return;
+  }
+
+  var rows = this.displayState_.textRowCount;
+  var columns = this.displayState_.textColumnCount;
+  var imageDataUrl = imageUrl;
+  var imgElement = document.createElement('img');
+  imgElement.src = imageDataUrl;
+  imgElement.onload = (function() {
+    var canvas = document.createElement('canvas');
+    var context = canvas.getContext('2d');
+    canvas.width = columns * 2;
+    canvas.height = rows * 4;

[David Tseng, 2017/02/24 05:39:45]

It would be helpful to name these constants or to add a comment why these values
were picked. i.e. BRAILLE_CELL_WIDTH and BRAILLE_CELL_HEIGHT (and use them
below). 

[dmazzoni, 2017/02/27 06:56:24]

Done.

+    context.drawImage(imgElement, 0, 0, canvas.width, canvas.height);
+    var imageData = context.getImageData(0, 0, canvas.width, canvas.height);
+    var data = imageData.data;
+    var outputData = [];
+
+    // Convert image to black and white.
+    for (var i = 0; i < data.length; i += 4) {
+      // Show if the alpha value is visible (above 20) and the pixel is not

[David Tseng, 2017/02/24 05:39:46]

This doesn't seem to match the below check. They're also reversed with the order
they're checked.

[dmazzoni, 2017/02/27 06:56:23]

Cleaned up, and also made it more easy to justify by
using a standard formula for luminance and thresholding
at 50%.

+      // white (under 600).
+      var show = (data[i] +

[David Tseng, 2017/02/24 05:39:46]

Name each of these offsets (e.g. i + RED_OFFSET, i + GREEN_OFFSET, etc).

[dmazzoni, 2017/02/27 06:56:23]

I did it slightly differently but should be just as
readable.

+                  data[i + 1] +
+                  data[i + 2] <= 200 * 3) &&

[David Tseng, 2017/02/24 05:39:45]

600 please (so it matches the comment above) and better yet, name it.

[dmazzoni, 2017/02/27 06:56:23]

Should be clear now, let me know.

+                  (data[i + 3]) == 255;
+      outputData.push(show);
+    }
+
+    // Convert to Braille.
+    var buf = new ArrayBuffer(rows * columns);

[David Tseng, 2017/02/24 05:39:45]

Maybe brailleBuff?

[dmazzoni, 2017/02/27 06:56:23]

brailleBuf to match elsewhere in the file

+    var view = new Uint8Array(buf);
+    var coordsToBrailleDot = [0x1, 0x2, 0x4, 0x40, 0x8, 0x10, 0x20, 0x80];

[David Tseng, 2017/02/24 05:39:46]

This is just static data; promote to a static class member.

[dmazzoni, 2017/02/27 06:56:24]

Done.

+    for (var i = 0; i < rows; i++) {
+      for (var j = 0; j < columns; j++) {
+        //Index in braille array
+        var index = i * columns + j;

[David Tseng, 2017/02/24 05:39:46]

brailleIndex

[dmazzoni, 2017/02/27 06:56:23]

Done.

+        view[index] = 0;

[David Tseng, 2017/02/24 05:39:46]

Not need; ArrayBuffer contents are initialized to 0.

[dmazzoni, 2017/02/27 06:56:24]

Done.

+        for (var x = 0; x < 2; x++) {

[David Tseng, 2017/02/24 05:39:46]

x is cell width so maybe brailleCellRow

[dmazzoni, 2017/02/27 06:56:24]

Done.

+          for (var y = 0; y < 4; y++) {

[David Tseng, 2017/02/24 05:39:46]

Use constants (see other comment).

[dmazzoni, 2017/02/27 06:56:23]

Done.

+            if (outputData[((i * columns * 4 + j + y * columns) * 2 + x)])

[David Tseng, 2017/02/24 05:39:46]

Can you distribute the index here and add a comment.

Also, be consistent with bracing (use them here since the rest of the file
does).

[dmazzoni, 2017/02/27 06:56:23]

Cleaned up, how's this?

+              view[index] += coordsToBrailleDot[x * 4 + y];
+          }
+        }
+      }
+    }
+
+    if (this.realDisplayState_.available) {
+      chrome.brailleDisplayPrivate.writeDots(
+          buf,
+          this.displayState_.textColumnCount,
+          this.displayState_.textRowCount);
+    }
+    if (cvox.BrailleCaptionsBackground.isEnabled()) {
+      cvox.BrailleCaptionsBackground.setImageContent(buf, rows, columns);
+    }
+  }).bind(this);
+};
+
+
+/**
  * Sets the command listener.  When a command is invoked, the listener will be
  * called with the BrailleKeyEvent corresponding to the command and the content
  * that was present on the display when the command was invoked.  The content
@@ -136,8 +212,19 @@ cvox.BrailleDisplayManager.prototype.setCommandListener = function(func) {
 
 
 /**
- * @param {!cvox.BrailleDisplayState} newState Display state reported
- *     by the extension API.
+ * @return {!cvox.BrailleDisplayState} The current display state.
+ */
+cvox.BrailleDisplayManager.prototype.getDisplayState = function() {
+  return this.displayState_;
+};
+
+
+/**
+ * @param {{available: boolean, textRowCount: (number|undefined),
+ *     textColumnCount: (number|undefined)}} newState Display state reported
+ *     by the extension API. Note that the type is almost the same as
+ *     cvox.BrailleDisplayState except that the extension API allows
+ *     some fields to be undefined, while cvox.BrailleDisplayState does not.
  * @private
  */
 cvox.BrailleDisplayManager.prototype.refreshDisplayState_ = function(
@@ -154,7 +241,11 @@ cvox.BrailleDisplayManager.prototype.refreshDisplayState_ = function(
     }
     this.refresh_();
   }.bind(this);
-  this.realDisplayState_ = newState;
+  this.realDisplayState_ = {
+    available: newState.available,
+    textRowCount: newState.textRowCount || 0,
+    textColumnCount: newState.textColumnCount || 0
+  };
   if (newState.available) {
     processDisplayState(newState);
     // Update the dimensions of the virtual braille captions display to those
@@ -183,7 +274,7 @@ cvox.BrailleDisplayManager.prototype.onCaptionsStateChanged_ = function() {
  * @private
  */
 cvox.BrailleDisplayManager.prototype.refresh_ = function() {
-  if (!this.displayState_.available) {
+  if (!this.displayState_.available || this.displayingImage_) {

[David Tseng, 2017/02/24 05:39:46]

Returns also if braille is frozen.

[dmazzoni, 2017/02/27 06:56:23]

Freezing is handled at a higher level than this, though...

     return;
   }
   var brailleBuf = this.panStrategy_.getCurrentBrailleViewportContents();