Refine autozoom scaling constants and logic

Source/web/WebViewImpl.cpp

 /*
  * Copyright (C) 2011, 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 164 matching lines @@
 
 static const double multipleTargetsZoomAnimationDurationInSeconds = 0.25;
 static const double findInPageAnimationDurationInSeconds = 0;
 
 // Constants for viewport anchoring on resize.
 static const float viewportAnchorXCoord = 0.5f;
 static const float viewportAnchorYCoord = 0;
 
 // Constants for zooming in on a focused text field.
 static const double scrollAndScaleAnimationDurationInSeconds = 0.2;
-static const int minReadableCaretHeight = 18;
-static const float minScaleChangeToTriggerZoom = 1.05f;
+static const int minReadableCaretHeight = 16;
+static const float minScaleChangeToTriggerZoom = 1.35f;
+static const float minScaleChangeFromBlockScaleToTriggerZoom = 2.25f;
 static const float leftBoxRatio = 0.3f;
 static const int caretPadding = 10;
 
 namespace blink {
 
 // Change the text zoom level by kTextSizeMultiplierRatio each time the user
 // zooms text in or out (ie., change by 20%).  The min and max values limit
 // text zoom to half and 3x the original text size.  These three values match
 // those in Apple's port in WebKit/WebKit/WebView/WebView.mm
 const double WebView::textSizeMultiplierRatio = 1.2;
@@ skipping 2326 matching lines @@
     WebRect caret, unusedEnd;
     selectionBounds(caret, unusedEnd);
     IntRect unscaledCaret = caret;
     unscaledCaret.scale(1 / pageScaleFactor());
     caret = unscaledCaret;
 
     // Pick a scale which is reasonably readable. This is the scale at which
     // the caret height will become minReadableCaretHeight (adjusted for dpi
     // and font scale factor).
     newScale = clampPageScaleFactorToLimits(legibleScale() * minReadableCaretHeight / caret.height);
-    const float deltaScale = newScale / pageScaleFactor();
+
+    // Avoiding zooming in further if already zoomed sufficiently within the focused block.
+    if (newScale > pageScaleFactor()) {
+        WebRect blockBounds = computeBlockBounds(textboxRect, false);
+
+        float blockScale;
+        WebPoint blockScroll;
+        computeScaleAndScrollForBlockRect(textboxRect.location(), blockBounds, touchPointPadding, minimumPageScaleFactor() * doubleTapZoomAlreadyLegibleRatio, blockScale, blockScroll);

[aelias_OOO_until_Jul13, 2014/03/14 07:16:10]

Hmm, this invokes more code than is necessary, particularly the computation of
scroll position which you then ignore.  And I don't like introducing a
dependency on double-tap zoom code because it makes it hard to change double-tap
zoom behavior without unexpected fallout on textbox behavior (or vice versa). 
If you want to keep this, could you rewrite it from first principles instead
(maybe start by copy-pasting the scale-computation code you're relying on and
then trim down any unnecessary parts?).

+        const float deltaScaleFromBlockScale = newScale / blockScale;
+        if (pageScaleFactor() > blockScale && deltaScaleFromBlockScale < minScaleChangeFromBlockScaleToTriggerZoom)
+            newScale = pageScaleFactor();
+    }
+
+    needAnimation = false;
+    // If we're sufficiently far from the target level, zoom in/out as necessary.
+    if (newScale / pageScaleFactor() > minScaleChangeToTriggerZoom
+        || pageScaleFactor() / newScale > minScaleChangeToTriggerZoom) {
+        needAnimation = true;
+    } else {
+        // Avoid changing the scale if it's already sufficiently close to the target.
+        newScale = pageScaleFactor();
+    }
 
     // Convert the rects to absolute space in the new scale.
     IntRect textboxRectInDocumentCoordinates = textboxRect;
     textboxRectInDocumentCoordinates.move(mainFrame()->scrollOffset());
     IntRect caretInDocumentCoordinates = caret;
     caretInDocumentCoordinates.move(mainFrame()->scrollOffset());
 
     int viewWidth = m_size.width / newScale;
     int viewHeight = m_size.height / newScale;
 
+    // If the caret is offscreen, then animate.
+    IntRect sizeRect(0, 0, viewWidth, viewHeight);
+    if (!sizeRect.contains(caret))
+        needAnimation = true;
+
+    // If the box is partially offscreen and it's possible to bring it fully
+    // onscreen, then animate.
+    if (sizeRect.contains(textboxRectInDocumentCoordinates.width(), textboxRectInDocumentCoordinates.height()) && !sizeRect.contains(textboxRect))
+        needAnimation = true;
+
+    if (!needAnimation)
+        return;
+
     if (textboxRectInDocumentCoordinates.width() <= viewWidth) {
         // Field is narrower than screen. Try to leave padding on left so field's
         // label is visible, but it's more important to ensure entire field is
         // onscreen.
         int idealLeftPadding = viewWidth * leftBoxRatio;
         int maxLeftPaddingKeepingBoxOnscreen = viewWidth - textboxRectInDocumentCoordinates.width();
         newScroll.setX(textboxRectInDocumentCoordinates.x() - min<int>(idealLeftPadding, maxLeftPaddingKeepingBoxOnscreen));
     } else {
         // Field is wider than screen. Try to left-align field, unless caret would
         // be offscreen, in which case right-align the caret.
         newScroll.setX(max<int>(textboxRectInDocumentCoordinates.x(), caretInDocumentCoordinates.x() + caretInDocumentCoordinates.width() + caretPadding - viewWidth));
     }
     if (textboxRectInDocumentCoordinates.height() <= viewHeight) {
         // Field is shorter than screen. Vertically center it.
         newScroll.setY(textboxRectInDocumentCoordinates.y() - (viewHeight - textboxRectInDocumentCoordinates.height()) / 2);
     } else {
         // Field is taller than screen. Try to top align field, unless caret would
         // be offscreen, in which case bottom-align the caret.
         newScroll.setY(max<int>(textboxRectInDocumentCoordinates.y(), caretInDocumentCoordinates.y() + caretInDocumentCoordinates.height() + caretPadding - viewHeight));
     }
-
-    needAnimation = false;
-    // If we are at less than the target zoom level, zoom in.
-    if (deltaScale > minScaleChangeToTriggerZoom)
-        needAnimation = true;
-    // If the caret is offscreen, then animate.
-    IntRect sizeRect(0, 0, viewWidth, viewHeight);
-    if (!sizeRect.contains(caret))
-        needAnimation = true;
-    // If the box is partially offscreen and it's possible to bring it fully
-    // onscreen, then animate.
-    if (sizeRect.contains(textboxRectInDocumentCoordinates.width(), textboxRectInDocumentCoordinates.height()) && !sizeRect.contains(textboxRect))
-        needAnimation = true;
 }
 
 void WebViewImpl::advanceFocus(bool reverse)
 {
     page()->focusController().advanceFocus(reverse ? FocusTypeBackward : FocusTypeForward);
 }
 
 double WebViewImpl::zoomLevel()
 {
     return m_zoomLevel;
@@ skipping 1381 matching lines @@
     const PageScaleConstraints& constraints = m_pageScaleConstraintsSet.pageDefinedConstraints();
 
     if (!mainFrameImpl() || !mainFrameImpl()->frameView())
         return false;
 
     return mainFrameImpl()->frameView()->layoutSize().width() == m_size.width
         || (constraints.minimumScale == constraints.maximumScale && constraints.minimumScale != -1);
 }
 
 } // namespace blink