Changeset 230746 in webkit

Timestamp:

Apr 17, 2018, 7:50:25 PM (8 years ago)

Author:

Wenson Hsieh

Message:

[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.

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):

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• LayoutTests/fast/events/extrazoom (added)
• LayoutTests/fast/events/extrazoom/double-tap-to-zoom-on-full-width-text-expected.txt (added)
• LayoutTests/fast/events/extrazoom/double-tap-to-zoom-on-full-width-text.html (added)
• LayoutTests/resources/basic-gestures.js (modified) (1 diff)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/WebProcess/WebPage/ViewGestureGeometryCollector.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/TestExpectations

@@ -26 +26 @@
 fast/visual-viewport/ios/ [ Skip ]
 fast/events/ios [ Skip ]
+fast/events/extrazoom [ Skip ]
 fast/events/touch/ios [ Skip ]
 fast/history/ios [ Skip ]

trunk/LayoutTests/resources/basic-gestures.js

@@ -7 +7 @@
                     uiController.uiScriptComplete();
                 }
+            })();`, resolve);
+    });
+}
+
+function doubleTapToZoomAtPoint(x, y)
+{
+    return new Promise(resolve => {
+        testRunner.runUIScript(`
+            (function() {
+                uiController.didEndZoomingCallback = () => uiController.uiScriptComplete();
+                uiController.singleTapAtPoint(${x}, ${y}, () => { });
+                uiController.singleTapAtPoint(${x}, ${y}, () => { });
             })();`, resolve);
     });

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebKit/WebProcess/WebPage/ViewGestureGeometryCollector.cpp

@@ -27 +27 @@
 #include "ViewGestureGeometryCollector.h"
 
+#include "Logging.h"
 #include "ViewGestureGeometryCollectorMessages.h"
 #include "WebCoreArgumentCoders.h"
@@ -32 +33 @@
 #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)
@@ -129 +133 @@
 #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);
@@ -143 +159 @@
     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 }};