Prevent scrolling of finite results.

Bug: 20562484

Inhibit scrolling past the last nonzero digit when we can identify it.
Increase length of immediate result a bit, to make it closer to the
L calculator.  On "=", expand less for scrollable results, more for
short non-scrollable results.  This allows us to use larger fonts for
short finite results.

Fix up animation logic when displaying result.  The old version was
never quite right, and became more visibly wrong with variable
enlargement.  This version is simpler and seems to give much better
results.

This still does not add ellipsis at the right end of a result.  But
it is now easily possible to tell whether a result is known finite by
attempting to scroll it.  That may be good enough.

Remove some obsolete TODO entries.

Change-Id: I25a842a743e1c27191ca18ac69aa9eef0f0ea9b1

4 files changed, 90 insertions, 52 deletions

diff --git a/res/layout/display.xml b/res/layout/display.xml
index 1fe2cf7..d50d09f 100644
--- a/res/layout/display.xml
+++ b/res/layout/display.xml
@@ -53,7 +53,7 @@
 
     <!--
       We lay the result out to full width, but are careful to use only
-      2/3 of the space, so that we have room when we expand.
+      4/5 of the space, so that we have room when we expand.
       -->
     <com.android.calculator2.CalculatorResult
         android:id="@+id/result"
diff --git a/src/com/android/calculator2/Calculator.java b/src/com/android/calculator2/Calculator.java
index d5242fa..e2c006c 100644
--- a/src/com/android/calculator2/Calculator.java
+++ b/src/com/android/calculator2/Calculator.java
@@ -19,11 +19,7 @@
 //        Other menus are not handled brilliantly either.
 // TODO: Revisit handling of "Help" menu, so that it's more consistent
 //       with our conventions.
-// TODO: See if we can make scrolling look better, especially on small
-//       displays. Fix evaluation interface so the evaluator returns entire
-//       result, and formatting of exponent etc. is done separately.
 // TODO: Better indication of when the result is known to be exact.
-// TODO: Fix placement of inverse trig buttons.
 // TODO: Check and possibly fix accessability issues.
 // TODO: Copy & more general paste in formula?  Note that this requires
 //       great care: Currently the text version of a displayed formula
@@ -466,15 +462,12 @@ public class Calculator extends Activity
     }
 
     // Initial evaluation completed successfully.  Initiate display.
-    public void onEvaluate(int initDisplayPrec, String truncatedWholeNumber) {
+    public void onEvaluate(int initDisplayPrec, int leastDigPos, String truncatedWholeNumber) {
         // Invalidate any options that may depend on the current result.
         invalidateOptionsMenu();
 
-        if (mCurrentState == CalculatorState.INPUT) {
-            // Just update small result display.
-            mResult.displayResult(initDisplayPrec, truncatedWholeNumber);
-        } else { // in EVALUATE or INIT state
-            mResult.displayResult(initDisplayPrec, truncatedWholeNumber);
+        mResult.displayResult(initDisplayPrec, leastDigPos, truncatedWholeNumber);
+        if (mCurrentState != CalculatorState.INPUT) { // in EVALUATE or INIT state
             onResult(mCurrentState != CalculatorState.INIT);
         }
     }
@@ -646,20 +639,19 @@ public class Calculator extends Activity
     // We assume the result already contains the text to be expanded.
     private void onResult(boolean animate) {
         // Calculate the values needed to perform the scale and translation animations.
-        // We now fix the character size in the display to avoid weird effects
-        // when we scroll.
-        // Display.xml is designed to ensure exactly a 3/2 ratio between the formula
-        // slot and small result slot.
-        final float resultScale = 1.5f;
-        final float resultTranslationX = -mResult.getWidth() * (resultScale - 1)/2;
-                // mFormulaText is aligned with mResult on the right.
-                // When we enlarge it around its center, the right side
-                // moves to the right.  This compensates.
-        float resultTranslationY = -mResult.getHeight();
-        // This is how much we want to move the bottom.
-        // Now compensate for the fact that we're
-        // simultaenously expanding it around its center by half its height
-        resultTranslationY += mResult.getHeight() * (resultScale - 1)/2;
+        // The nominal font size in the result display is fixed.  But the magnification we
+        // use when the user hits "=" is variable, with a scrollable result always getting
+        // minimum magnification.
+        // Display.xml is designed to ensure that a 5/4 increase is always possible.
+        // More is possible if the display is not fully occupied.
+        // Pivot the result around the bottom of the text.
+        final float resultScale = (float)Math.min(1.25f / mResult.getOccupancy(), 2.0);
+        // Keep the right end of text fixed as we scale.
+        mResult.setPivotX(mResult.getWidth() - mResult.getPaddingRight());
+        // Move result up to take place of formula.  Scale around top of formula.
+        mResult.setPivotY(mResult.getPaddingTop());
+        float resultTranslationY = -mFormulaText.getHeight();
+        // Move formula off screen.
         final float formulaTranslationY = -mFormulaText.getBottom();
 
         // TODO: Reintroduce textColorAnimator?
@@ -672,7 +664,6 @@ public class Calculator extends Activity
             animatorSet.playTogether(
                     ObjectAnimator.ofFloat(mResult, View.SCALE_X, resultScale),
                     ObjectAnimator.ofFloat(mResult, View.SCALE_Y, resultScale),
-                    ObjectAnimator.ofFloat(mResult, View.TRANSLATION_X, resultTranslationX),
                     ObjectAnimator.ofFloat(mResult, View.TRANSLATION_Y, resultTranslationY),
                     ObjectAnimator.ofFloat(mFormulaText, View.TRANSLATION_Y,
                                            formulaTranslationY));
@@ -697,7 +688,6 @@ public class Calculator extends Activity
         } else /* No animation desired; get there fast, e.g. when restarting */ {
             mResult.setScaleX(resultScale);
             mResult.setScaleY(resultScale);
-            mResult.setTranslationX(resultTranslationX);
             mResult.setTranslationY(resultTranslationY);
             mFormulaText.setTranslationY(formulaTranslationY);
             setState(CalculatorState.RESULT);
diff --git a/src/com/android/calculator2/CalculatorResult.java b/src/com/android/calculator2/CalculatorResult.java
index d14135c..a916c30 100644
--- a/src/com/android/calculator2/CalculatorResult.java
+++ b/src/com/android/calculator2/CalculatorResult.java
@@ -48,7 +48,7 @@ import android.support.v4.view.ViewCompat;
 // A text widget that is "infinitely" scrollable to the right,
 // and obtains the text to display via a callback to Logic.
 public class CalculatorResult extends TextView {
-    static final int MAX_RIGHT_SCROLL = 100000000;
+    static final int MAX_RIGHT_SCROLL = 10000000;
     static final int INVALID = MAX_RIGHT_SCROLL + 10000;
         // A larger value is unlikely to avoid running out of space
     final OverScroller mScroller;
@@ -71,8 +71,10 @@ public class CalculatorResult extends TextView {
                             // Large positive values mean the decimal point is scrolled off the
                             // left of the display.  Zero means decimal point is barely displayed
                             // on the right.
-    private int mLastPos;   // Position already reflected in display.
-    private int mMinPos;    // Maximum position before all digits disappear of the right.
+    private int mLastPos;   // Position already reflected in display. Pixels.
+    private int mMinPos;    // Minimum position before all digits disappear off the right. Pixels.
+    private int mMaxPos;    // Maximum position before we start displaying the infinite
+                            // sequence of trailing zeroes on the right. Pixels.
     private Object mWidthLock = new Object();
                             // Protects the next two fields.
     private int mWidthConstraint = -1;
@@ -109,14 +111,14 @@ public class CalculatorResult extends TextView {
                     // Ignore scrolls of error string, etc.
                     if (!mScrollable) return true;
                     mScroller.fling(mCurrentPos, 0, - (int) velocityX, 0  /* horizontal only */,
-                                    mMinPos, MAX_RIGHT_SCROLL, 0, 0);
+                                    mMinPos, mMaxPos, 0, 0);
                     ViewCompat.postInvalidateOnAnimation(CalculatorResult.this);
                     return true;
                 }
                 @Override
                 public boolean onScroll(MotionEvent e1, MotionEvent e2,
                                         float distanceX, float distanceY) {
-                    // TODO: Should we be dealing with any edge effects here?
+                    int distance = (int)distanceX;
                     if (!mScroller.isFinished()) {
                         mCurrentPos = mScroller.getFinalX();
                     }
@@ -124,9 +126,14 @@ public class CalculatorResult extends TextView {
                     stopActionMode();
                     CalculatorResult.this.cancelLongPress();
                     if (!mScrollable) return true;
+                    if (mCurrentPos + distance < mMinPos) {
+                        distance = mMinPos - mCurrentPos;
+                    } else if (mCurrentPos + distance > mMaxPos) {
+                        distance = mMaxPos - mCurrentPos;
+                    }
                     int duration = (int)(e2.getEventTime() - e1.getEventTime());
                     if (duration < 1 || duration > 100) duration = 10;
-                    mScroller.startScroll(mCurrentPos, 0, (int)distanceX, 0, (int)duration);
+                    mScroller.startScroll(mCurrentPos, 0, distance, 0, (int)duration);
                     ViewCompat.postInvalidateOnAnimation(CalculatorResult.this);
                     return true;
                 }
@@ -177,18 +184,45 @@ public class CalculatorResult extends TextView {
         }
     }
 
-    // Display a new result, given initial displayed
-    // precision and the string representing the whole part of
-    // the number to be displayed.
-    // We pass the string, instead of just the length, so we have
-    // one less place to fix in case we ever decide to
-    // correctly use a variable width font.
-    void displayResult(int initPrec, String truncatedWholePart) {
+    // Given that the last non-zero digit is at pos, compute the precision we have to ask
+    // ask for to actually get the digit at pos displayed.  This is not an identity
+    // function, since we may need to drop digits to the right to make room for the exponent.
+    private int addExpSpace(int lastDigit) {
+        if (lastDigit < getMaxChars() - 1) {
+            // The decimal point will be in view when displaying the rightmost digit.
+            // no exponent needed.
+            // TODO: This will change if we stop scrolling to the left of the decimal
+            // point, which might be desirable in the traditional scientific notation case.
+            return lastDigit;
+        }
+        // When the last digit is displayed, the exponent will look like "e-<lastDigit>".
+        // The length of that string is the extra precision we need.
+        return lastDigit + (int)Math.ceil(Math.log10((double)lastDigit)) + 2;
+    }
+
+    // Display a new result, given initial displayed precision, position of the rightmost
+    // nonzero digit (or Integer.MAX_VALUE if non-terminating), and the string representing
+    // the whole part of the number to be displayed.
+    // We pass the string, instead of just the length, so we have one less place to fix in case
+    // we ever decide to fully handle a variable width font.
+    void displayResult(int initPrec, int leastDigPos, String truncatedWholePart) {
         mLastPos = INVALID;
         synchronized(mWidthLock) {
             mCurrentPos = initPrec * mCharWidth;
         }
-        mMinPos = - (int) Math.ceil(getPaint().measureText(truncatedWholePart));
+        // Should logically be
+        // mMinPos = - (int) Math.ceil(getPaint().measureText(truncatedWholePart)), but
+        // we eventually transalate to a character position by dividing by mCharWidth.
+        // To avoid rounding issues, we use the analogous computation here.
+        mMinPos = - truncatedWholePart.length() * mCharWidth;
+        if (leastDigPos < MAX_RIGHT_SCROLL) {
+            mMaxPos = Math.min(addExpSpace(leastDigPos) * mCharWidth, MAX_RIGHT_SCROLL);
+        } else {
+            mMaxPos = MAX_RIGHT_SCROLL;
+        }
+        mScrollable = (leastDigPos != (initPrec == -1 ? 0 : initPrec));
+                // We assume that initPrec allows most significant digit to be displayed.
+                // If there is nothing to the right of initPrec, there is no point in scrolling.
         redisplay();
     }
 
@@ -212,8 +246,6 @@ public class CalculatorResult extends TextView {
     // would have been in had we not done so.
     // This minimizes jumps as a result of scrolling.  Result is NOT internationalized,
     // uses "e" for exponent.
-    // last_included[0] is set to the position of the last digit we actually include;
-    // thus caller can tell whether result is exact.
     public String formatResult(String res, int digs,
                                int maxDigs, boolean truncated,
                                boolean negative) {
@@ -313,13 +345,16 @@ public class CalculatorResult extends TextView {
         return (currentCharPos >= BoundedRational.digitsRequired(rat));
     }
 
-    // May be called asynchronously from non-UI thread.
+    /**
+     * Return the maximum number of characters that will fit in the result display.
+     * May be called asynchronously from non-UI thread.
+     */
     int getMaxChars() {
-        // We only use 2/3 of the available space, since the left 1/3 of the result is not
-        // visible when it is shown in large size.
+        // We only use 4/5 of the available space, since at least the left 4/5 of the result
+        // is not visible when it is shown in large size.
         int result;
         synchronized(mWidthLock) {
-            result = 2 * mWidthConstraint / (3 * mCharWidth);
+            result = 4 * mWidthConstraint / (5 * mCharWidth);
             // We can apparently finish evaluating before onMeasure in CalculatorText has been
             // called, in which case we get 0 or -1 as the width constraint.
         }
@@ -331,6 +366,19 @@ public class CalculatorResult extends TextView {
         }
     }
 
+    /**
+     * Return the fraction of the available character space occupied by the
+     * current result.
+     * Should be called only with a valid result displayed.
+     */
+    float getOccupancy() {
+        if (mScrollable) {
+            return 1.0f;
+        } else {
+            return (float)getText().length() / getMaxChars();
+        }
+    }
+
     int getCurrentCharPos() {
         synchronized(mWidthLock) {
             return mCurrentPos/mCharWidth;
@@ -359,7 +407,6 @@ public class CalculatorResult extends TextView {
             setText(result);
         }
         mValid = true;
-        mScrollable = true;
     }
 
     @Override
diff --git a/src/com/android/calculator2/Evaluator.java b/src/com/android/calculator2/Evaluator.java
index 6f508c4..8764a99 100644
--- a/src/com/android/calculator2/Evaluator.java
+++ b/src/com/android/calculator2/Evaluator.java
@@ -425,8 +425,8 @@ class Evaluator {
             // checking for change.
             int init_prec = result.mInitDisplayPrec;
             int msd = getMsdPos(mCache);
-            int new_init_prec = getPreferredPrec(mCache, msd,
-                                BoundedRational.digitsRequired(mRatVal));
+            int leastDigPos = BoundedRational.digitsRequired(mRatVal);
+            int new_init_prec = getPreferredPrec(mCache, msd, leastDigPos);
             if (new_init_prec < init_prec) {
                 init_prec = new_init_prec;
             } else {
@@ -434,7 +434,7 @@ class Evaluator {
                 // happen if they're not. e.g. because
                 // CalculatorResult.MAX_WIDTH was too small.
             }
-            mCalculator.onEvaluate(init_prec,truncatedWholePart);
+            mCalculator.onEvaluate(init_prec, leastDigPos, truncatedWholePart);
         }
         @Override
         protected void onCancelled(InitialResult result) {
@@ -712,7 +712,8 @@ class Evaluator {
             // Notify immediately, reusing existing result.
             int dotPos = mCache.indexOf('.');
             String truncatedWholePart = mCache.substring(0, dotPos);
-            mCalculator.onEvaluate(mLastDigs,truncatedWholePart);
+            int leastDigPos = BoundedRational.digitsRequired(mRatVal);
+            mCalculator.onEvaluate(mLastDigs, leastDigPos, truncatedWholePart);
         }
     }