[Extra zoom mode] Double tap to zoom should account for text legibility in extra...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Apr 2018 02:50:25 +0000 (02:50 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Apr 2018 02:50:25 +0000 (02:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184631
<rdar://problem/39303706>

Reviewed by Tim Horton.

Source/WebKit:

Implement the text legibility heuristic alluded to in r230506 by iterating through text runs in the document (up
to a maximum of 200) and building a histogram of font sizes that appear in the document, where each tally
represents a character.

The first and second text legibility zoom scales are then computed based on the zoom scales needed to
make 50% and 90% of the text legible, respectively. Here, a zoom scale that makes text legible is such that the
text would have an apparent font size of a hard-coded constant (currently, 12) after zooming. This means the
first and second text legibility scales may end up being close to one another, or even the same (in the case
where there is only a single font size in the entire document). In this case, we just snap the first scale to
the second, so that double tapping will only toggle between two zoom scales. In another case where the document
has no text (e.g. an image document), we just fall back to a zoom scale of 1.

Test: fast/events/extrazoom/double-tap-to-zoom-on-full-width-text.html

* WebProcess/WebPage/ViewGestureGeometryCollector.cpp:
(WebKit::ViewGestureGeometryCollector::computeTextLegibilityScales):

LayoutTests:

Add a layout test to check that double tap to zoom works in extra zoom mode, even when text spans the entire
width of the document.

* TestExpectations:
* fast/events/extrazoom/double-tap-to-zoom-on-full-width-text-expected.txt: Added.
* fast/events/extrazoom/double-tap-to-zoom-on-full-width-text.html: Added.
* resources/basic-gestures.js:

Add a helper method to double tap at a given location, and wait for zooming to finish.

(return.new.Promise):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230746 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/fast/events/extrazoom/double-tap-to-zoom-on-full-width-text-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/extrazoom/double-tap-to-zoom-on-full-width-text.html [new file with mode: 0644]
LayoutTests/resources/basic-gestures.js
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebPage/ViewGestureGeometryCollector.cpp

index 5fd683c..8e65ed8 100644 (file)
@@ -1,3 +1,23 @@
+2018-04-17  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Extra zoom mode] Double tap to zoom should account for text legibility in extra zoom mode
+        https://bugs.webkit.org/show_bug.cgi?id=184631
+        <rdar://problem/39303706>
+
+        Reviewed by Tim Horton.
+
+        Add a layout test to check that double tap to zoom works in extra zoom mode, even when text spans the entire
+        width of the document.
+
+        * TestExpectations:
+        * fast/events/extrazoom/double-tap-to-zoom-on-full-width-text-expected.txt: Added.
+        * fast/events/extrazoom/double-tap-to-zoom-on-full-width-text.html: Added.
+        * resources/basic-gestures.js:
+
+        Add a helper method to double tap at a given location, and wait for zooming to finish.
+
+        (return.new.Promise):
+
 2018-04-17  Tadeu Zagallo  <tzagallo@apple.com>
 
         Retain MessagePortChannel for transfer when disentangling ports
index 315df32..bc6e9b8 100644 (file)
@@ -25,6 +25,7 @@ fast/forms/ios [ Skip ]
 fast/viewport/ios [ Skip ]
 fast/visual-viewport/ios/ [ Skip ]
 fast/events/ios [ Skip ]
+fast/events/extrazoom [ Skip ]
 fast/events/touch/ios [ Skip ]
 fast/history/ios [ Skip ]
 fast/scrolling/ios [ Skip ]
diff --git a/LayoutTests/fast/events/extrazoom/double-tap-to-zoom-on-full-width-text-expected.txt b/LayoutTests/fast/events/extrazoom/double-tap-to-zoom-on-full-width-text-expected.txt
new file mode 100644 (file)
index 0000000..a2b242d
--- /dev/null
@@ -0,0 +1,5 @@
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Even though this text is laid out at the full document width, we should still be able to double tap to zoom in "extra zoom mode", such that the text becomes legible.
+Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
diff --git a/LayoutTests/fast/events/extrazoom/double-tap-to-zoom-on-full-width-text.html b/LayoutTests/fast/events/extrazoom/double-tap-to-zoom-on-full-width-text.html
new file mode 100644 (file)
index 0000000..0e17b86
--- /dev/null
@@ -0,0 +1,25 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<script src="../../../resources/js-test.js"></script>
+<script src="../../../resources/basic-gestures.js"></script>
+<meta name="viewport" content="width=device-width">
+<body style="margin: 0; font-size: 10px;">
+    <div>
+        Even though this text is laid out at the full document width, we should still
+        be able to double tap to zoom in "extra zoom mode", such that the text becomes
+        legible.
+    </div>
+    <div>
+        Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor
+        incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis
+        nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
+        Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu
+        fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in
+        culpa qui officia deserunt mollit anim id est laborum.
+    </div>
+</body>
+<script>
+    jsTestIsAsync = true;
+    doubleTapToZoomAtPoint(75, 75).then(finishJSTest);
+</script>
+</html>
index c9659a4..9c90b8d 100644 (file)
@@ -10,6 +10,18 @@ function didShowKeyboard()
     });
 }
 
+function doubleTapToZoomAtPoint(x, y)
+{
+    return new Promise(resolve => {
+        testRunner.runUIScript(`
+            (function() {
+                uiController.didEndZoomingCallback = () => uiController.uiScriptComplete();
+                uiController.singleTapAtPoint(${x}, ${y}, () => { });
+                uiController.singleTapAtPoint(${x}, ${y}, () => { });
+            })();`, resolve);
+    });
+}
+
 function longPressAtPoint(x, y)
 {
     return new Promise(resolve => {
index cc7e744..ba0852e 100644 (file)
@@ -1,3 +1,28 @@
+2018-04-17  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Extra zoom mode] Double tap to zoom should account for text legibility in extra zoom mode
+        https://bugs.webkit.org/show_bug.cgi?id=184631
+        <rdar://problem/39303706>
+
+        Reviewed by Tim Horton.
+
+        Implement the text legibility heuristic alluded to in r230506 by iterating through text runs in the document (up
+        to a maximum of 200) and building a histogram of font sizes that appear in the document, where each tally
+        represents a character.
+
+        The first and second text legibility zoom scales are then computed based on the zoom scales needed to
+        make 50% and 90% of the text legible, respectively. Here, a zoom scale that makes text legible is such that the
+        text would have an apparent font size of a hard-coded constant (currently, 12) after zooming. This means the
+        first and second text legibility scales may end up being close to one another, or even the same (in the case
+        where there is only a single font size in the entire document). In this case, we just snap the first scale to
+        the second, so that double tapping will only toggle between two zoom scales. In another case where the document
+        has no text (e.g. an image document), we just fall back to a zoom scale of 1.
+
+        Test: fast/events/extrazoom/double-tap-to-zoom-on-full-width-text.html
+
+        * WebProcess/WebPage/ViewGestureGeometryCollector.cpp:
+        (WebKit::ViewGestureGeometryCollector::computeTextLegibilityScales):
+
 2018-04-17  Megan Gardner  <megan_gardner@apple.com>
 
         Don't activate selection on become first responder
index b00aa9e..a6db0e5 100644 (file)
 #include "config.h"
 #include "ViewGestureGeometryCollector.h"
 
+#include "Logging.h"
 #include "ViewGestureGeometryCollectorMessages.h"
 #include "WebCoreArgumentCoders.h"
 #include "WebFrame.h"
 #include "WebPage.h"
 #include "WebProcess.h"
+#include <WebCore/FontCascade.h>
 #include <WebCore/Frame.h>
 #include <WebCore/FrameView.h>
 #include <WebCore/HTMLImageElement.h>
+#include <WebCore/HTMLTextFormControlElement.h>
 #include <WebCore/HitTestResult.h>
 #include <WebCore/ImageDocument.h>
 #include <WebCore/RenderView.h>
+#include <WebCore/TextIterator.h>
 
 #if PLATFORM(IOS)
 #include "SmartMagnificationControllerMessages.h"
@@ -128,9 +132,21 @@ void ViewGestureGeometryCollector::collectGeometryForSmartMagnificationGesture(F
 
 #if PLATFORM(IOS)
 
+struct FontSizeAndCount {
+    unsigned fontSize;
+    unsigned count;
+};
+
 std::optional<std::pair<double, double>> ViewGestureGeometryCollector::computeTextLegibilityScales(double& viewportMinimumScale, double& viewportMaximumScale)
 {
-    static const double defaultMaximumTextLegibilityScale = 1;
+    static const unsigned fontSizeBinningInterval = 2;
+    static const double maximumNumberOfTextRunsToConsider = 200;
+
+    static const double targetLegibilityFontSize = 12;
+    static const double firstTextLegibilityScaleRatio = 0.5;
+    static const double secondTextLegibilityScaleRatio = 0.1;
+    static const double minimumDifferenceBetweenTextLegibilityScales = 0.2;
+    static const double fallbackTextLegibilityScale = 1;
 
     computeMinimumAndMaximumViewportScales(viewportMinimumScale, viewportMaximumScale);
     if (m_cachedTextLegibilityScales)
@@ -142,10 +158,64 @@ std::optional<std::pair<double, double>> ViewGestureGeometryCollector::computeTe
 
     document->updateLayoutIgnorePendingStylesheets();
 
-    // FIXME: Determine appropriate text legibility scales by examining text runs in the document. For now, hard code the second text legibility scale to be 1,
-    // and set the first text legibility scale to be the halfway point between the initial scale and 1.
-    double firstTextLegibilityScale = clampTo<double>((m_webPage.viewportConfiguration().initialScale() + defaultMaximumTextLegibilityScale) / 2, viewportMinimumScale, viewportMaximumScale);
-    double secondTextLegibilityScale = clampTo<double>(defaultMaximumTextLegibilityScale, viewportMinimumScale, viewportMaximumScale);
+    auto documentRange = Range::create(*document, {{ document->documentElement(), Position::PositionIsBeforeAnchor }}, {{ document->documentElement(), Position::PositionIsAfterAnchor }});
+    HashSet<Node*> allTextNodes;
+    HashMap<unsigned, unsigned> fontSizeToCountMap;
+    unsigned numberOfIterations = 0;
+    unsigned totalSampledTextLength = 0;
+
+    for (TextIterator documentTextIterator { documentRange.ptr(), TextIteratorEntersTextControls }; !documentTextIterator.atEnd(); documentTextIterator.advance()) {
+        if (++numberOfIterations >= maximumNumberOfTextRunsToConsider)
+            break;
+
+        if (!is<Text>(documentTextIterator.node()))
+            continue;
+
+        auto& textNode = downcast<Text>(*documentTextIterator.node());
+        auto textLength = textNode.length();
+        if (!textLength || !textNode.renderer() || allTextNodes.contains(&textNode))
+            continue;
+
+        allTextNodes.add(&textNode);
+
+        unsigned fontSizeBin = fontSizeBinningInterval * round(textNode.renderer()->style().fontCascade().size() / fontSizeBinningInterval);
+        auto entry = fontSizeToCountMap.find(fontSizeBin);
+        fontSizeToCountMap.set(fontSizeBin, textLength + (entry == fontSizeToCountMap.end() ? 0 : entry->value));
+        totalSampledTextLength += textLength;
+    }
+
+    Vector<FontSizeAndCount> sortedFontSizesAndCounts;
+    sortedFontSizesAndCounts.reserveCapacity(fontSizeToCountMap.size());
+    for (auto& entry : fontSizeToCountMap)
+        sortedFontSizesAndCounts.append({ entry.key, entry.value });
+
+    std::sort(sortedFontSizesAndCounts.begin(), sortedFontSizesAndCounts.end(), [] (auto& first, auto& second) {
+        return first.fontSize < second.fontSize;
+    });
+
+    double firstTextLegibilityScale = 0;
+    double secondTextLegibilityScale = 0;
+    double currentSampledTextLength = 0;
+    for (auto& fontSizeAndCount : sortedFontSizesAndCounts) {
+        currentSampledTextLength += fontSizeAndCount.count;
+        double ratioOfTextUnderCurrentFontSize = currentSampledTextLength / totalSampledTextLength;
+        LOG(ViewGestures, "About %.2f%% of text is smaller than font size %tu", ratioOfTextUnderCurrentFontSize * 100, fontSizeAndCount.fontSize);
+        if (!firstTextLegibilityScale && ratioOfTextUnderCurrentFontSize >= firstTextLegibilityScaleRatio)
+            firstTextLegibilityScale = targetLegibilityFontSize / fontSizeAndCount.fontSize;
+        if (!secondTextLegibilityScale && ratioOfTextUnderCurrentFontSize >= secondTextLegibilityScaleRatio)
+            secondTextLegibilityScale = targetLegibilityFontSize / fontSizeAndCount.fontSize;
+    }
+
+    if (sortedFontSizesAndCounts.isEmpty()) {
+        firstTextLegibilityScale = fallbackTextLegibilityScale;
+        secondTextLegibilityScale = fallbackTextLegibilityScale;
+    } else if (secondTextLegibilityScale - firstTextLegibilityScale < minimumDifferenceBetweenTextLegibilityScales)
+        firstTextLegibilityScale = secondTextLegibilityScale;
+
+    secondTextLegibilityScale = clampTo<double>(secondTextLegibilityScale, viewportMinimumScale, viewportMaximumScale);
+    firstTextLegibilityScale = clampTo<double>(firstTextLegibilityScale, viewportMinimumScale, viewportMaximumScale);
+
+    LOG(ViewGestures, "The computed text legibility scales are: (%.2f, %.2f)", firstTextLegibilityScale, secondTextLegibilityScale);
 
     m_cachedTextLegibilityScales = std::optional<std::pair<double, double>> {{ firstTextLegibilityScale, secondTextLegibilityScale }};
     return m_cachedTextLegibilityScales;