Change Android client desktop panning and zooming behavior

remoting/android/java/src/org/chromium/chromoting/DesktopView.java

Index: remoting/android/java/src/org/chromium/chromoting/DesktopView.java
diff --git a/remoting/android/java/src/org/chromium/chromoting/DesktopView.java b/remoting/android/java/src/org/chromium/chromoting/DesktopView.java
index c7222577c78ca576a9656fff46ac66c10cd7774d..c2a727176cb173aa1b46abf6b553a5979df59f77 100644
--- a/remoting/android/java/src/org/chromium/chromoting/DesktopView.java
+++ b/remoting/android/java/src/org/chromium/chromoting/DesktopView.java
@@ -57,6 +57,12 @@ public class DesktopView extends SurfaceView implements Runnable, SurfaceHolder.
     /** Specifies the dimension by which the zoom level is being lower-bounded. */
     private Constraint mConstraint;
 
+    /** Whether the right edge of the image was visible on-screen during the last render. */
+    private boolean mRightUsedToBeOut;
+
+    /** Whether the bottom edge of the image was visible on-screen during the last render. */
+    private boolean mBottomUsedToBeOut;
+
     /** Whether the device has just been rotated, necessitating a canvas redraw. */
     private boolean mJustRotated;
 
@@ -71,6 +77,11 @@ public class DesktopView extends SurfaceView implements Runnable, SurfaceHolder.
         mScreenWidth = 0;
         mScreenHeight = 0;
         mConstraint = Constraint.UNDEFINED;
+
+        mRightUsedToBeOut = false;
+        mBottomUsedToBeOut = false;
+
+        mJustRotated = false;
     }
 
     /**
@@ -80,46 +91,83 @@ public class DesktopView extends SurfaceView implements Runnable, SurfaceHolder.
      */
     @Override
     public void run() {
-        if (Looper.myLooper()==Looper.getMainLooper()) {
+        if (Looper.myLooper() == Looper.getMainLooper()) {
             Log.w("deskview", "Canvas being redrawn on UI thread");
         }
 
-        Canvas canvas = getHolder().lockCanvas();
         Bitmap image = JniInterface.retrieveVideoFrame();
+        Canvas canvas = getHolder().lockCanvas();
         synchronized (mTransform) {
             canvas.setMatrix(mTransform);
 
+            // Internal parameters of the transformation matrix.
             float[] values = new float[9];
             mTransform.getValues(values);
+
+            // Screen coordinates of two defining points of the image.
             float[] topleft = {0, 0};
             mTransform.mapPoints(topleft);
             float[] bottomright = {image.getWidth(), image.getHeight()};
             mTransform.mapPoints(bottomright);
 
-            if (mConstraint==Constraint.UNDEFINED) {
+            if (mConstraint == Constraint.UNDEFINED) {
                 mConstraint = image.getWidth()/image.getHeight() > mScreenWidth/mScreenHeight ?
-                        Constraint.HEIGHT : Constraint.WIDTH;
+                        Constraint.WIDTH : Constraint.HEIGHT;
             }
 
-            if (mConstraint==Constraint.WIDTH && bottomright[0] - topleft[0] < mScreenWidth) {
-                mTransform.setPolyToPoly(new float[] {0, 0, image.getWidth(), 0}, 0,
-                        new float[] {0, 0, mScreenWidth, 0}, 0, 2);
-            } else if (mConstraint==Constraint.HEIGHT &&
-                    bottomright[1] - topleft[1] < mScreenHeight) {
-                mTransform.setPolyToPoly(new float[] {0, 0, 0, image.getHeight()}, 0,
-                        new float[] {0, 0, 0, mScreenHeight}, 0, 2);
+            if (mConstraint == Constraint.WIDTH &&
+                    (int)(bottomright[0] - topleft[0] + 0.5) < mScreenWidth) {
+                    // The vertical edges of the image are flush against the device's screen edges
+                    // when the entire host screen is visible, and the user has zoomed out too far.
+                float imageMiddle = (float)image.getHeight() / 2;
+                float screenMiddle = (float)mScreenHeight / 2;
+                mTransform.setPolyToPoly(
+                        new float[] {0, imageMiddle, image.getWidth(), imageMiddle}, 0,
+                        new float[] {0, screenMiddle, mScreenWidth, screenMiddle}, 0, 2);
+            } else if (mConstraint == Constraint.HEIGHT &&
+                    (int)(bottomright[1] - topleft[1] + 0.5) < mScreenHeight) {
+                    // The horizontal image edges are flush against the device's screen edges when
+                    // the entire host screen is visible, and the user has zoomed out too far.
+                float imageCenter = (float)image.getWidth() / 2;
+                float screenCenter = (float)mScreenWidth / 2;
+                mTransform.setPolyToPoly(
+                        new float[] {imageCenter, 0, imageCenter, image.getHeight()}, 0,
+                        new float[] {screenCenter, 0, screenCenter, mScreenHeight}, 0, 2);
             } else {
-                if (values[Matrix.MTRANS_X] > 0) {
-                    values[Matrix.MTRANS_X] = 0;
+                    // It's fine for both members of a pair of image edges to be within the screen
+                    // edges (or "out of bounds"); that simply means that the image is zoomed out as
+                    // far as permissible. And both members of a pair can obviously be outside the
+                    // screen's edges, which indicates that the image is zoomed in to far to see the
+                    // whole host screen. However, if only one of a pair of edges has entered the
+                    // screen, the user is attempting to scroll into a blank area of the canvas.

[garykac, 2013/07/20 21:10:44]

These comments (here and above) seem indented strangely.

[solb, 2013/07/21 08:18:07]

Okay; I'll bump them out one level.

+                // A value of true means the corresponding edge has entered the screen's borders.
+                boolean leftEdgeOutOfBounds = values[Matrix.MTRANS_X] > 0;
+                boolean topEdgeOutOfBounds = values[Matrix.MTRANS_Y] > 0;
+                boolean rightEdgeOutOfBounds = bottomright[0] < mScreenWidth;
+                boolean bottomEdgeOutOfBounds = bottomright[1] < mScreenHeight;
+
+                if (leftEdgeOutOfBounds != rightEdgeOutOfBounds) {
+                    if (leftEdgeOutOfBounds != mRightUsedToBeOut) {
+                        values[Matrix.MTRANS_X] = 0;
+                    }
+                    else {
+                        values[Matrix.MTRANS_X] += mScreenWidth - bottomright[0];
+                    }
                 }
-                if (values[Matrix.MTRANS_Y] > 0) {
-                    values[Matrix.MTRANS_Y] = 0;
+                else {  // Updating this value during an attempt to scroll away causes oscillation.

[garykac, 2013/07/20 21:10:44]

If these comments apply to the line below, then they shouldn't be a line comment
on the "else" token.

Also, the comment seems to imply a TODO to address the oscillation.

[solb, 2013/07/21 08:18:07]

Actually, this comment was intended to explain the placement of that line inside
an else as opposed to simply following the if block, where it would exhibit the
described behavior. I'll try to reword it in a way that expresses this more
clearly.

+                    mRightUsedToBeOut = rightEdgeOutOfBounds;
                 }
-                if (bottomright[0] < mScreenWidth) {
-                    values[Matrix.MTRANS_X] += mScreenWidth - bottomright[0];
+
+                if (topEdgeOutOfBounds != bottomEdgeOutOfBounds) {
+                    if (topEdgeOutOfBounds != mBottomUsedToBeOut) {
+                        values[Matrix.MTRANS_Y] = 0;
+                    }
+                    else {
+                        values[Matrix.MTRANS_Y] += mScreenHeight - bottomright[1];
+                    }
                 }
-                if (bottomright[1] < mScreenHeight) {
-                    values[Matrix.MTRANS_Y] += mScreenHeight - bottomright[1];
+                else {  // Updating this value during an attempt to scroll away causes oscillation.
+                    mBottomUsedToBeOut = bottomEdgeOutOfBounds;
                 }
 
                 mTransform.setValues(values);