Improved Pinch-Zoom for PDF.

chrome/browser/resources/pdf/viewport.js

Index: chrome/browser/resources/pdf/viewport.js
diff --git a/chrome/browser/resources/pdf/viewport.js b/chrome/browser/resources/pdf/viewport.js
index 18fb6c2ad403f4c8a49956f43a37bc776eaece6e..aaa13490e7da0a278eb89d33d030add5887bfe33 100644
--- a/chrome/browser/resources/pdf/viewport.js
+++ b/chrome/browser/resources/pdf/viewport.js
@@ -49,6 +49,10 @@ function Viewport(window,
   this.fittingType_ = Viewport.FittingType.NONE;
   this.defaultZoom_ = defaultZoom;
   this.topToolbarHeight_ = topToolbarHeight;
+  this.prevScale = 1;
+  this.doRender_ = true;
+  this.didPinch_ = false;
+  this.didPinchEnd_ = false;
 
   window.addEventListener('scroll', this.updateViewport_.bind(this));
   window.addEventListener('resize', this.resize_.bind(this));
@@ -171,9 +175,7 @@ Viewport.prototype = {
    * @private
    * Called when the viewport should be updated.
    */
-  updateViewport_: function() {
-    this.viewportChangedCallback_();
-  },
+  updateViewport_: function() { this.viewportChangedCallback_(); },

[Kevin McNee - google account, 2016/10/06 21:53:17]

Unnecessary whitespace change.

[Kevin McNee - google account, 2016/10/13 18:19:24]

Done.

 
   /**
    * @private
@@ -222,9 +224,7 @@ Viewport.prototype = {
   /**
    * @type {number} the zoom level of the viewport.
    */
-  get zoom() {
-    return this.zoom_;
-  },
+  get zoom() { return this.zoom_; },

[Kevin McNee - google account, 2016/10/06 21:53:17]

Unnecessary whitespace change.

[Kevin McNee - google account, 2016/10/13 18:19:23]

Done.

 
   /**
    * @private
@@ -237,6 +237,13 @@ Viewport.prototype = {
     this.beforeZoomCallback_();
     this.allowedToChangeZoom_ = true;
     f();
+    var needsScrollbars = this.documentNeedsScrollbars_(this.zoom_);
+    if (!needsScrollbars.horizontal) {
+      this.pinchCenter_ = {

[Kevin McNee - google account, 2016/10/06 21:53:17]

Comments from previous review:
wjmaclean: Perhaps add a TODO here: at present the transition between using the
window centre and the gesture centre is liable to be a little 'jarring' ...
might be nice to think of some way of doing a smooth transition, perhaps a
weighted average of the window and gesture centres where the weight transitions
smoothly from one to the other over a preset number of frames?

bokan: This is the case where we don't have horizontal scrolling so we want to
keep the document centered. In that case, I think using the window center is
fine (and this feels fine to me). Although we probably don't want to do the
centering vertically.

alessandroa: I've explained today to David how by having the point in the middle
of the view helps us zooming.

[wjmaclean, 2016/10/07 12:30:21]

I'm fine with making this a todo ("maybedo"?), and using the window centre for
now.

[Kevin McNee - google account, 2016/10/13 18:19:23]

Acknowledged.

+        x: this.window_.innerWidth / 2,
+        y: this.window_.innerHeight / 2
+      };
+    }
     this.allowedToChangeZoom_ = false;
     this.afterZoomCallback_();
   },
@@ -266,6 +273,59 @@ Viewport.prototype = {
   },
 
   /**
+   * @private
+   * Sets the zoom of the viewport.
+   * Same function as below but for pinch zoom we have some more operations.
+   * @param {number} scaleDelta the zoom delta.
+   * @param {Object} center the pinch center in content coordinates
+   */
+  setPinchZoomInternal_: function(scaleDelta, center) {
+    if (!this.allowedToChangeZoom_) {
+      throw 'Called Viewport.setZoomInternal_ without calling ' +
+          'Viewport.mightZoom_.';
+    }
+    this.zoom_ = this.clampScale(this.zoom_ * scaleDelta);
+
+    var newCenterInContent = this.frameToContent(center);
+    var delta = {
+      x: (newCenterInContent.x - this.oldCenterInContent.x),
+      y: (newCenterInContent.y - this.oldCenterInContent.y)
+    };
+
+    // Record the scroll position (relative to the pinch center).
+    var currentScrollPos = {
+      x: this.position.x - delta.x * this.zoom_,
+      y: this.position.y - delta.y * this.zoom_
+    };
+
+    this.contentSizeChanged_();
+    // Scroll to the scaled scroll position.
+    this.position = {x: currentScrollPos.x, y: currentScrollPos.y};
+  },
+
+  /**
+   *  @private
+   *  Makes sure that the scale level doesn't get out of the limits.
+   *  @param {number} scale the new scale level
+   *  @return {number} the scale clamped in the limit
+   */
+  clampScale: function(scale) { return Math.min(5, Math.max(0.25, scale)); },
+
+  /**
+   *  @private
+   *  Gets the new center in content.
+   *  @param {Object} pinch center
+   *  @param {Object} zoom level
+   *  @return {Objet} the new center in content
+   */
+  frameToContent: function(framePoint) {
+    return {
+      x: (framePoint.x + this.position.x) / this.zoom_,
+      y: (framePoint.y + this.position.y) / this.zoom_
+    };
+  },
+
+  /**
    * Sets the zoom to the given zoom level.
    * @param {number} newZoom the zoom level to zoom to.
    */
@@ -282,16 +342,12 @@ Viewport.prototype = {
   /**
    * @type {number} the width of scrollbars in the viewport in pixels.
    */
-  get scrollbarWidth() {
-    return this.scrollbarWidth_;
-  },
+  get scrollbarWidth() { return this.scrollbarWidth_; },

[Kevin McNee - google account, 2016/10/06 21:53:17]

Unnecessary whitespace change.

[Kevin McNee - google account, 2016/10/13 18:19:24]

Done.

 
   /**
    * @type {Viewport.FittingType} the fitting type the viewport is currently in.
    */
-  get fittingType() {
-    return this.fittingType_;
-  },
+  get fittingType() { return this.fittingType_; },

[Kevin McNee - google account, 2016/10/06 21:53:17]

Unnecessary whitespace change.

[Kevin McNee - google account, 2016/10/13 18:19:24]

Done.

 
   /**
    * @private
@@ -457,14 +513,26 @@ Viewport.prototype = {
       this.updateViewport_();
     }.bind(this));
   },
+  /**
+   * @private
+   * Computes vector between two points.
+   * @param {Object} First Point
+   * @param {Object} Second Point
+   * @return {Object} The vector
+   */
+  vectorDelta: function(p1, p2) {
+    var vector = {
+      x: p2.x - p1.x,
+      y: p2.y - p1.y
+    };
+    return vector;
+  },
 
   /**
    * Zoom the viewport so that a page consumes the entire viewport. Also scrolls
    * the viewport to the top of the current page.
    */
-  fitToPage: function() {
-    this.fitToPageInternal_(true);
-  },
+  fitToPage: function() { this.fitToPageInternal_(true); },

[Kevin McNee - google account, 2016/10/06 21:53:17]

Unnecessary whitespace change.

[Kevin McNee - google account, 2016/10/13 18:19:23]

Done.

 
   /**
    * Zoom out to the next predefined zoom level.
@@ -499,6 +567,74 @@ Viewport.prototype = {
   },
 
   /**
+   * Pinch zoom event handler
+   * @param {ev} the pinch event
+   */
+  pinchZoom: function(ev) {
+    this.mightZoom_(function() {
+      // We render on pinchin in order to solve the invalid regions that appear
+      // after zooming out.
+      if (ev.additionalEvent == 'pinchin')
+        this.doRender_ = true;
+      else
+        this.doRender_ = false;
+
+      var scaleDelta = ev.scale / this.prevScale;
+      this.pinchPanVector_ = this.vectorDelta(ev.center, this.first_pinch_center_in_frame_);

[Kevin McNee - google account, 2016/10/06 21:53:17]

Line too long.

[Kevin McNee - google account, 2016/10/13 18:19:23]

Done.

+
+      var needsScrollbars = this.documentNeedsScrollbars_(
+          this.clampScale(this.zoom_ * scaleDelta));
+      // If there's no horizontal scrolling, keep the content centered so the
+      // user can't zoom in on the non-content area.
+      if (this.keepContentCentered_ && needsScrollbars.horizontal) {
+        this.oldCenterInContent =
+            this.frameToContent(this.frameToPluginCoordinate(ev.center));
+        this.keepContentCentered_ = false;
+      }
+
+      this.pinchCenter_ = ev.center;
+      this.setPinchZoomInternal_(scaleDelta, this.frameToPluginCoordinate(ev.center));

[Kevin McNee - google account, 2016/10/06 21:53:17]

Line too long.

[Kevin McNee - google account, 2016/10/13 18:19:24]

Done.

+      this.updateViewport_();
+      this.prevScale = ev.scale;
+    }.bind(this));
+  },
+
+  pinchZoomStart: function(ev) {
+    this.prevScale = 1;
+    this.oldCenterInContent =
+        this.frameToContent(this.frameToPluginCoordinate(ev.center));
+
+    var needsScrollbars = this.documentNeedsScrollbars_(this.zoom_);
+    if (!needsScrollbars.horizontal)
+      this.keepContentCentered_ = true;

[Kevin McNee - google account, 2016/10/13 18:04:02]

Verbose

[Kevin McNee - google account, 2016/10/13 18:19:24]

Done.

+    else
+      this.keepContentCentered_ = false;
+    // We keep track of begining of the pinch.
+    // By doing so we will be able to compute the pan distance.
+    this.first_pinch_center_in_frame_ = ev.center;
+  },
+
+  pinchZoomEnd: function(ev) {
+    this.mightZoom_(function() {
+      // We want to render the document on pinch end
+      this.doRender_ = true;
+      var scaleDelta = ev.scale / this.prevScale;
+      this.pinchCenter_ = ev.center;
+
+      this.setPinchZoomInternal_(scaleDelta, this.frameToPluginCoordinate(ev.center));

[Kevin McNee - google account, 2016/10/06 21:53:17]

Line too long.

[Kevin McNee - google account, 2016/10/13 18:19:23]

Done.

+      this.updateViewport_();
+    }.bind(this));
+  },
+
+  frameToPluginCoordinate: function(coordinateInFrame) {
+    var container = $('plugin');
+    return {
+      x: coordinateInFrame.x - container.getBoundingClientRect().left,
+      y: coordinateInFrame.y - container.getBoundingClientRect().top
+    };
+  },
+
+  /**
    * Go to the given page index.
    * @param {number} page the index of the page to go to. zero-based.
    */