Gallery: Random fixes for the Viewport class.

The CL do:

* Remove the device rect argument from paintDeviceRect because the rectangle can
be calculated from other arguments.

* Remove setByFrame(AndFit) helper functions that are used only once each to simplify
the Viewport class.

* Rename getScreenClipped with getImageBoundsOnScreenClipped. The old name
  sounds it retuns the bounds of screen.

* Add getImageElementBounds to obtain the bounds before applying zoom and
  offset.

BUG=245926
TEST=manually

Review URL: https://codereview.chromium.org/398263002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@283719 0039d316-1c4b-4281-b951-d872f2087c98

ui/file_manager/gallery/js/image_editor/commands.js

@@ -415,9 +415,7 @@ Command.Filter.prototype = { __proto__: Command.prototype };
 Command.Filter.prototype.execute = function(
     document, srcCanvas, callback, uiContext) {
   var result = this.createCanvas_(document, srcCanvas);
-
   var self = this;
-
   var previousRow = 0;
 
   function onProgressVisible(updatedRow, rowCount) {
@@ -438,8 +436,7 @@ Command.Filter.prototype.execute = function(
       screenStrip.height =
           Math.round(viewport.imageToScreenY(updatedRow)) - screenStrip.top;
 
-      uiContext.imageView.paintDeviceRect(
-          viewport.screenToDeviceRect(screenStrip), result, imageStrip);
+      uiContext.imageView.paintDeviceRect(result, imageStrip);
       previousRow = updatedRow;
     }
   }

ui/file_manager/gallery/js/image_editor/image_editor.js

@@ -30,7 +30,8 @@ function ImageEditor(
   ImageUtil.removeChildren(this.container_);
 
   this.viewport_ = viewport;
-  this.viewport_.sizeByFrame(this.container_);
+  this.viewport_.setScreenSize(
+      this.container_.clientWidth, this.container_.clientHeight);
 
   this.imageView_ = imageView;
   this.imageView_.addContentCallback(this.onContentUpdate_.bind(this));

ui/file_manager/gallery/js/image_editor/image_transform.js

@@ -128,7 +128,7 @@ ImageEditor.Mode.Crop.prototype.reset = function() {
  * Updates the position of DOM elements.
  */
 ImageEditor.Mode.Crop.prototype.positionDOM = function() {
-  var screenClipped = this.viewport_.getScreenClipped();
+  var screenClipped = this.viewport_.getImageBoundsOnScreenClipped();
 
   var screenCrop = this.viewport_.imageToScreenRect(this.cropRect_.getRect());
   var delta = ImageEditor.Mode.Crop.MOUSE_GRAB_RADIUS;
@@ -190,7 +190,8 @@ ImageEditor.Mode.Crop.prototype.getCommand = function() {
  * Creates default (initial) crop.
  */
 ImageEditor.Mode.Crop.prototype.createDefaultCrop = function() {
-  var rect = new Rect(this.getViewport().getImageClipped());
+  var rect = this.getViewport().screenToImageRect(
+      new Rect(this.getViewport().getImageBoundsOnScreenClipped()));
   rect = rect.inflate(
       -Math.round(rect.width / 6), -Math.round(rect.height / 6));
   this.cropRect_ = new DraggableRect(rect, this.getViewport());
@@ -465,7 +466,8 @@ DraggableRect.prototype.getDragHandler = function(
   var initialY = this.viewport_.screenToImageY(initialScreenY);
   var initialWidth = this.bounds_.right - this.bounds_.left;
   var initialHeight = this.bounds_.bottom - this.bounds_.top;
-  var clipRect = this.viewport_.getImageClipped();
+  var clipRect = this.viewport_.screenToImageRect(
+      this.viewport_.getImageBoundsOnScreenClipped());
   if (!clipRect.inside(initialX, initialY))
     return null;
 
@@ -557,10 +559,7 @@ DraggableRect.prototype.getDragHandler = function(
  * @return {ImageBuffer.DoubleTapAction} Double tap action.
  */
 DraggableRect.prototype.getDoubleTapAction = function(x, y, touch) {
-  x = this.viewport_.screenToImageX(x);
-  y = this.viewport_.screenToImageY(y);
-
-  var clipRect = this.viewport_.getImageClipped();
+  var clipRect = this.viewport_.getImageBoundsOnScreenClipped();
   if (clipRect.inside(x, y))
     return ImageBuffer.DoubleTapAction.COMMIT;
   else

ui/file_manager/gallery/js/image_editor/image_util.js

@@ -139,6 +139,28 @@ function Rect() {
        Array.apply(null, arguments));
 }
 
+Rect.prototype = {
+  /**
+   * Obtains the x coordinate of right edge. The most right pixels in the
+   * rectangle are (x = right - 1) and the pixels (x = right) are not included
+   * in the rectangle.
+   * @return {number}
+   */
+  get right() {
+    return this.left + this.width;
+  },
+
+  /**
+   * Obtains the y coordinate of bottom edge. The most bottom pixels in the
+   * rectanlge are (y = buttom - 1) and the pixels (y = bottom) are not included
+   * in the rectangle.
+   * @return {number}
+   */
+  get bottom() {
+    return this.top + this.height;
+  }
+};
+
 /**
  * @param {number} factor Factor to scale.
  * @return {Rect} A rectangle with every dimension scaled.

ui/file_manager/gallery/js/image_editor/image_view.js

@@ -119,8 +119,7 @@ ImageView.prototype.draw = function() {
     this.displayedContentGeneration_ = this.contentGeneration_;
 
     ImageUtil.trace.resetTimer('paint');
-    this.paintDeviceRect(this.viewport_.getDeviceClipped(),
-        this.contentCanvas_, this.viewport_.getImageClipped());
+    this.paintDeviceRect(this.contentCanvas_, new Rect(this.contentCanvas_));
     ImageUtil.trace.reportTimer('paint');
   }
 };
@@ -178,22 +177,29 @@ ImageView.prototype.getContentRevision = function() {
  * Copies an image fragment from a full resolution canvas to a device resolution
  * canvas.
  *
- * @param {Rect} deviceRect Rectangle in the device coordinates.
- * @param {HTMLCanvasElement} canvas Full resolution canvas.
- * @param {Rect} imageRect Rectangle in the full resolution canvas.
- */
-ImageView.prototype.paintDeviceRect = function(deviceRect, canvas, imageRect) {
-  // Map screen canvas (0,0) to (deviceBounds.left, deviceBounds.top)
-  var deviceBounds = this.viewport_.getDeviceClipped();
-  deviceRect = deviceRect.shift(-deviceBounds.left, -deviceBounds.top);
-
-  // The source canvas may have different physical size than the image size
-  // set at the viewport. Adjust imageRect accordingly.
-  var bounds = this.viewport_.getImageBounds();
-  var scaleX = canvas.width / bounds.width;
-  var scaleY = canvas.height / bounds.height;
-  imageRect = new Rect(imageRect.left * scaleX, imageRect.top * scaleY,
-                       imageRect.width * scaleX, imageRect.height * scaleY);
+ * @param {HTMLCanvasElement} canvas Canvas containing whole image. The canvas
+ *     may not be full resolution (scaled).
+ * @param {Rect} imageRect Rectangle region of the canvas to be rendered.
+ */
+ImageView.prototype.paintDeviceRect = function(canvas, imageRect) {
+  // Check canvas size.
+  var deviceBounds = this.viewport_.getDeviceBounds();
+  if (this.screenImage_.width != deviceBounds.width ||
+      this.screenImage_.height != deviceBounds.height) {
+    console.error('The size of canvas is invalid.', (new Error).stack);
+    return;
+  }
+
+  // Map the rectangle in full resolution image to the rectangle in the device
+  // canvas.
+  var scaleX = deviceBounds.width / canvas.width;
+  var scaleY = deviceBounds.height / canvas.height;
+  var deviceRect = new Rect(
+      imageRect.left * scaleX,
+      imageRect.top * scaleY,
+      imageRect.width * scaleX,
+      imageRect.height * scaleY);
+
   Rect.drawImage(
       this.screenImage_.getContext('2d'), canvas, deviceRect, imageRect);
 };
@@ -217,17 +223,17 @@ ImageView.prototype.createOverlayCanvas = function() {
  * @param {HTMLCanvasElement} canvas The buffer canvas.
  */
 ImageView.prototype.setupDeviceBuffer = function(canvas) {
-  var deviceRect = this.viewport_.getDeviceClipped();
-
   // Set the canvas position and size in device pixels.
+  var deviceRect = this.viewport_.getDeviceBounds();
   if (canvas.width !== deviceRect.width)
     canvas.width = deviceRect.width;
-
   if (canvas.height !== deviceRect.height)
     canvas.height = deviceRect.height;
 
-  canvas.style.left = deviceRect.left + 'px';
-  canvas.style.top = deviceRect.top + 'px';
+  // Center the image.
+  var imageBoudns = this.viewport_.getImageElementBoundsOnScreen();
+  canvas.style.left = imageBoudns.left + 'px';
+  canvas.style.top = imageBoudns.top + 'px';
 
   // Scale the canvas down to screen pixels.
   this.setTransform(canvas);

ui/file_manager/gallery/js/image_editor/viewport.js

@@ -23,6 +23,27 @@ function Viewport() {
    */
   this.screenBounds_ = new Rect();
 
+  /**
+   * Bounds of the image element on screen without zoom and offset.
+   * @type {Rect}
+   * @private
+   */
+  this.imageElementBoundsOnScreen_ = null;
+
+  /**
+   * Bounds of the image with zoom and offset.
+   * @type {Rect}
+   * @private
+   */
+  this.imageBoundsOnScreen_ = null;
+
+  /**
+   * Image bounds that is clipped with the screen bounds.
+   * @type {Rect}
+   * @private
+   */
+  this.imageBoundsOnScreenClipped_ = null;
+
   /**
    * Scale from the full resolution image to the screen displayed image. This is
    * not zoom operated by users.
@@ -45,12 +66,28 @@ function Viewport() {
    */
   this.zoom_ = 1;
 
+  /**
+   * Offset specified by user operations.
+   * @type {number}
+   */
   this.offsetX_ = 0;
+
+  /**
+   * Offset specified by user operations.
+   * @type {number}
+   */
   this.offsetY_ = 0;
 
+  /**
+   * Generation of the screen size image cache.
+   * This is incremented every time when the size of image cache is changed.
+   * @type {number}
+   * @private
+   */
   this.generation_ = 0;
 
   this.update();
+  Object.seal(this);
 }
 
 /**
@@ -75,6 +112,7 @@ Viewport.ZOOM_RATIOS = Object.freeze({
  */
 Viewport.prototype.setImageSize = function(width, height) {
   this.imageBounds_ = new Rect(width, height);
+  this.update();
   this.invalidateCaches();
 };
 
@@ -84,6 +122,7 @@ Viewport.prototype.setImageSize = function(width, height) {
  */
 Viewport.prototype.setScreenSize = function(width, height) {
   this.screenBounds_ = new Rect(width, height);
+  this.update();
   this.invalidateCaches();
 };
 
@@ -95,9 +134,9 @@ Viewport.prototype.setZoomIndex = function(zoomIndex) {
   // Ignore the invalid zoomIndex.
   if (!Viewport.ZOOM_RATIOS[zoomIndex.toString()])
     return;
-
   this.zoomIndex_ = zoomIndex;
   this.zoom_ = Viewport.ZOOM_RATIOS[zoomIndex];
+  this.update();
 };
 
 /**
@@ -108,29 +147,6 @@ Viewport.prototype.getZoomIndex = function() {
   return this.zoomIndex_;
 };
 
-/**
- * Set the size by an HTML element.
- *
- * @param {HTMLElement} frame The element acting as the "screen".
- */
-Viewport.prototype.sizeByFrame = function(frame) {
-  this.setScreenSize(frame.clientWidth, frame.clientHeight);
-};
-
-/**
- * Set the size and scale to fit an HTML element.
- *
- * @param {HTMLElement} frame The element acting as the "screen".
- */
-Viewport.prototype.sizeByFrameAndFit = function(frame) {
-  var wasFitting = this.getScale() == this.getFittingScale();
-  this.sizeByFrame(frame);
-  var minScale = this.getFittingScale();
-  if (wasFitting || (this.getScale() < minScale)) {
-    this.setScale(minScale, true);
-  }
-};
-
 /**
  * @return {number} Scale.
  */
@@ -138,11 +154,12 @@ Viewport.prototype.getScale = function() { return this.scale_; };
 
 /**
  * @param {number} scale The new scale.
- * @param {boolean} notify True if the change should be reflected in the UI.
  */
-Viewport.prototype.setScale = function(scale, notify) {
-  if (this.scale_ == scale) return;
+Viewport.prototype.setScale = function(scale) {
+  if (this.scale_ == scale)
+    return;
   this.scale_ = scale;
+  this.update();
   this.invalidateCaches();
 };
 
@@ -174,8 +191,7 @@ Viewport.prototype.getFittingScaleForImageSize_ = function(width, height) {
  * Set the scale to fit the image into the screen.
  */
 Viewport.prototype.fitImage = function() {
-  var scale = this.getFittingScale();
-  this.setScale(scale, true);
+  this.setScale(this.getFittingScale());
 };
 
 /**
@@ -216,15 +232,13 @@ Viewport.prototype.getImageBounds = function() { return this.imageBounds_; };
 Viewport.prototype.getScreenBounds = function() { return this.screenBounds_; };
 
 /**
- * @return {Rect} The visible part of the image, in image coordinates.
+ * @return {Rect} The size of screen cache canvas.
  */
-Viewport.prototype.getImageClipped = function() { return this.imageClipped_; };
-
-/**
- * @return {Rect} The visible part of the image, in screen coordinates.
- */
-Viewport.prototype.getScreenClipped = function() {
-  return this.screenClipped_;
+Viewport.prototype.getDeviceBounds = function() {
+  var size = this.getImageElementBoundsOnScreen();
+  return new Rect(
+      size.width * window.devicePixelRatio,
+      size.height * window.devicePixelRatio);
 };
 
 /**
@@ -244,7 +258,24 @@ Viewport.prototype.invalidateCaches = function() { this.generation_++; };
  * @return {Rect} The image bounds in screen coordinates.
  */
 Viewport.prototype.getImageBoundsOnScreen = function() {
-  return this.imageOnScreen_;
+  return this.imageBoundsOnScreen_;
+};
+
+/**
+ * The image bounds in screen coordinates.
+ * This returns the bounds of element before applying zoom and offset.
+ * @return {Rect}
+ */
+Viewport.prototype.getImageElementBoundsOnScreen = function() {
+  return this.imageElementBoundsOnScreen_;
+};
+
+/**
+ * The image bounds on screen, which is clipped with the screen size.
+ * @return {Rect}
+ */
+Viewport.prototype.getImageBoundsOnScreenClipped = function() {
+  return this.imageBoundsOnScreenClipped_;
 };
 
 /**
@@ -260,7 +291,7 @@ Viewport.prototype.screenToImageSize = function(size) {
  * @return {number} X in image coordinates.
  */
 Viewport.prototype.screenToImageX = function(x) {
-  return Math.round((x - this.imageOnScreen_.left) / this.getScale());
+  return Math.round((x - this.imageBoundsOnScreen_.left) / this.getScale());
 };
 
 /**
@@ -268,7 +299,7 @@ Viewport.prototype.screenToImageX = function(x) {
  * @return {number} Y in image coordinates.
  */
 Viewport.prototype.screenToImageY = function(y) {
-  return Math.round((y - this.imageOnScreen_.top) / this.getScale());
+  return Math.round((y - this.imageBoundsOnScreen_.top) / this.getScale());
 };
 
 /**
@@ -296,7 +327,7 @@ Viewport.prototype.imageToScreenSize = function(size) {
  * @return {number} X in screen coordinates.
  */
 Viewport.prototype.imageToScreenX = function(x) {
-  return Math.round(this.imageOnScreen_.left + x * this.getScale());
+  return Math.round(this.imageBoundsOnScreen_.left + x * this.getScale());
 };
 
 /**
@@ -304,7 +335,7 @@ Viewport.prototype.imageToScreenX = function(x) {
  * @return {number} Y in screen coordinates.
  */
 Viewport.prototype.imageToScreenY = function(y) {
-  return Math.round(this.imageOnScreen_.top + y * this.getScale());
+  return Math.round(this.imageBoundsOnScreen_.top + y * this.getScale());
 };
 
 /**
@@ -319,34 +350,6 @@ Viewport.prototype.imageToScreenRect = function(rect) {
       Math.round(this.imageToScreenSize(rect.height)));
 };
 
-/**
- * Convert a rectangle from screen coordinates to 'device' coordinates.
- *
- * This conversion enlarges the original rectangle devicePixelRatio times
- * with the screen center as a fixed point.
- *
- * @param {Rect} rect Rectangle in screen coordinates.
- * @return {Rect} Rectangle in device coordinates.
- */
-Viewport.prototype.screenToDeviceRect = function(rect) {
-  var ratio = window.devicePixelRatio;
-  var screenCenterX = Math.round(
-      this.screenBounds_.left + this.screenBounds_.width / 2);
-  var screenCenterY = Math.round(
-      this.screenBounds_.top + this.screenBounds_.height / 2);
-  return new Rect(screenCenterX + (rect.left - screenCenterX) * ratio,
-                  screenCenterY + (rect.top - screenCenterY) * ratio,
-                  rect.width * ratio,
-                  rect.height * ratio);
-};
-
-/**
- * @return {Rect} The visible part of the image, in device coordinates.
- */
-Viewport.prototype.getDeviceClipped = function() {
-  return this.screenToDeviceRect(this.getScreenClipped());
-};
-
 /**
  * @return {boolean} True if some part of the image is clipped by the screen.
  */
@@ -394,45 +397,47 @@ Viewport.prototype.clampOffsetY_ = function(y) {
   return ImageUtil.clamp(-limit, y, limit);
 };
 
+/**
+ * @private
+ */
+Viewport.prototype.getCenteredRect_ = function(
+    width, height, offsetX, offsetY) {
+  return new Rect(
+      ~~((this.screenBounds_.width - width) / 2) + offsetX,
+      ~~((this.screenBounds_.height - height) / 2) + offsetY,
+      width,
+      height);
+};
+
 /**
  * Recalculate the viewport parameters.
  */
 Viewport.prototype.update = function() {
   var scale = this.getScale();
 
-  // Image bounds in screen coordinates.
-  this.imageOnScreen_ = new Rect(
-      this.getMarginX_(),
-      this.getMarginY_(),
-      Math.round(this.imageBounds_.width * scale),
-      Math.round(this.imageBounds_.height * scale));
-
-  // A visible part of the image in image coordinates.
-  this.imageClipped_ = new Rect(this.imageBounds_);
-
-  // A visible part of the image in screen coordinates.
-  this.screenClipped_ = new Rect(this.screenBounds_);
-
-  // Adjust for the offset.
-  if (this.imageOnScreen_.left < 0) {
-    this.imageOnScreen_.left +=
-        Math.round(this.clampOffsetX_(this.offsetX_) * scale);
-    this.imageClipped_.left = Math.round(-this.imageOnScreen_.left / scale);
-    this.imageClipped_.width = Math.round(this.screenBounds_.width / scale);
-  } else {
-    this.screenClipped_.left = this.imageOnScreen_.left;
-    this.screenClipped_.width = this.imageOnScreen_.width;
-  }
+  // Image bounds on screen.
+  this.imageBoundsOnScreen_ = this.getCenteredRect_(
+      ~~(this.imageBounds_.width * scale * this.zoom_),
+      ~~(this.imageBounds_.height * scale * this.zoom_),
+      this.offsetX_,
+      this.offsetY_);
 
-  if (this.imageOnScreen_.top < 0) {
-    this.imageOnScreen_.top +=
-        Math.round(this.clampOffsetY_(this.offsetY_) * scale);
-    this.imageClipped_.top = Math.round(-this.imageOnScreen_.top / scale);
-    this.imageClipped_.height = Math.round(this.screenBounds_.height / scale);
-  } else {
-    this.screenClipped_.top = this.imageOnScreen_.top;
-    this.screenClipped_.height = this.imageOnScreen_.height;
-  }
+  // Image bounds of element (that is not applied zoom and offset) on screen.
+  this.imageElementBoundsOnScreen_ = this.getCenteredRect_(
+      ~~(this.imageBounds_.width * scale),
+      ~~(this.imageBounds_.height * scale),
+      0,
+      0);
+
+  // Image bounds on screen cliped with the screen bounds.
+  var left = Math.max(this.imageBoundsOnScreen_.left, 0);
+  var top = Math.max(this.imageBoundsOnScreen_.top, 0);
+  var right = Math.min(
+      this.imageBoundsOnScreen_.right, this.screenBounds_.width);
+  var bottom = Math.min(
+      this.imageBoundsOnScreen_.bottom, this.screenBounds_.height);
+  this.imageBoundsOnScreenClipped_ = new Rect(
+      left, top, right - left, bottom - top);
 };
 
 /**
@@ -512,10 +517,9 @@ Viewport.prototype.getInverseTransformForCroppedImage =
  * @return {string} Transformation description.
  */
 Viewport.prototype.getScreenRectTransformForImage = function(screenRect) {
-  var screenImageWidth = this.imageBounds_.width * this.getScale();
-  var screenImageHeight = this.imageBounds_.height * this.getScale();
-  var scaleX = screenRect.width / screenImageWidth;
-  var scaleY = screenRect.height / screenImageHeight;
+  var imageBounds = this.getImageElementBoundsOnScreen();
+  var scaleX = screenRect.width / imageBounds.width;
+  var scaleY = screenRect.height / imageBounds.height;
   var screenWidth = this.screenBounds_.width;
   var screenHeight = this.screenBounds_.height;
   var dx = screenRect.left + screenRect.width / 2 - screenWidth / 2;

ui/file_manager/gallery/js/slide_mode.js

@@ -394,7 +394,7 @@ SlideMode.prototype.getSelectedImageRect = function() {
   if (this.getSelectedIndex() < 0)
     return null;
   else
-    return this.viewport_.getScreenClipped();
+    return this.viewport_.getImageBoundsOnScreen();
 };
 
 /**
@@ -889,8 +889,8 @@ SlideMode.prototype.onKeyDown = function(event) {
  * @private
  */
 SlideMode.prototype.onResize_ = function() {
-  this.viewport_.sizeByFrameAndFit(this.container_);
-  this.viewport_.update();
+  this.viewport_.setScreenSize(
+      this.container_.clientWidth, this.container_.clientHeight);
   this.editor_.getBuffer().draw();
 };
 
@@ -1047,7 +1047,7 @@ SlideMode.prototype.isSlideshowOn_ = function() {
 SlideMode.prototype.startSlideshow = function(opt_interval, opt_event) {
   // Reset zoom.
   this.viewport_.setZoomIndex(0);
-  this.imageView_.draw();
+  this.imageView_.applyViewportChange();
 
   // Set the attribute early to prevent the toolbar from flashing when
   // the slideshow is being started from the mosaic view.
@@ -1202,7 +1202,7 @@ SlideMode.prototype.toggleEditor = function(opt_event) {
   if (this.isEditing()) { // isEditing has just been flipped to a new value.
     // Reset zoom.
     this.viewport_.setZoomIndex(0);
-    this.imageView_.draw();
+    this.imageView_.applyViewportChange();
     if (this.context_.readonlyDirName) {
       this.editor_.getPrompt().showAt(
           'top', 'GALLERY_READONLY_WARNING', 0, this.context_.readonlyDirName);