Change Android client desktop panning and zooming behavior

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

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.chromoting;
 
 import android.content.Context;
 import android.graphics.Bitmap;
 import android.graphics.Canvas;
 import android.graphics.Color;
@@ skipping 39 matching lines @@
 
     /** Stores pan and zoom configuration and converts image coordinates to screen coordinates. */
     private Matrix mTransform;
 
     private int mScreenWidth;
     private int mScreenHeight;
 
     /** 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;
 
     public DesktopView(Context context) {
         super(context);
         getHolder().addCallback(this);
         DesktopListener listener = new DesktopListener();
         mScroller = new GestureDetector(context, listener);
         mZoomer = new ScaleGestureDetector(context, listener);
 
         mTransform = new Matrix();
         mScreenWidth = 0;
         mScreenHeight = 0;
         mConstraint = Constraint.UNDEFINED;
+
+        mRightUsedToBeOut = false;
+        mBottomUsedToBeOut = false;
+
+        mJustRotated = false;
     }
 
     /**
      * Redraws the canvas. This should be done on a non-UI thread or it could
      * cause the UI to lag. Specifically, it is currently invoked on the native
      * graphics thread using a JNI.
      */
     @Override
     public void run() {
-        if (Looper.myLooper()==Looper.getMainLooper()) {
+        if (Looper.myLooper() == Looper.getMainLooper()) {
             Log.w("deskview", "Canvas being redrawn on UI thread");
         }
 
+        Bitmap image = JniInterface.retrieveVideoFrame();
         Canvas canvas = getHolder().lockCanvas();
-        Bitmap image = JniInterface.retrieveVideoFrame();
         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);
             }
 
             canvas.setMatrix(mTransform);
         }
 
         canvas.drawColor(Color.BLACK);
         canvas.drawBitmap(image, 0, 0, new Paint());
@@ skipping 97 matching lines @@
 
         /**
          * Called when the user is done zooming. Defers to onScale()'s judgement.
          */
         @Override
         public void onScaleEnd(ScaleGestureDetector detector) {
             onScale(detector);
         }
     }
 }