[Text autosizing] [iPadOS] Revise our heuristics to determine idempotent text autosiz...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Jun 2019 04:06:51 +0000 (04:06 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Jun 2019 04:06:51 +0000 (04:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=198763
<rdar://problem/51826266>

Reviewed by Simon Fraser.

Source/WebCore:

This patch adjusts existing text autosizing heuristics, based on a survey of text on websites in the Alexa top
500 that shrink down to fit the viewport when requesting the desktop version of the site. The new heuristic is
derived from training decision trees against the dataset obtained from this survey, and balances false positives
(cases where layout is broken due to autosizing) against overall accuracy (measured using cross-validation).

See below for more details. Additionally, please refer to the link in the radar for more details, as well as
resources used to generate, validate, and analyze these decision trees.

Test: fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates.html

* css/StyleResolver.cpp:
(WebCore::StyleResolver::adjustRenderStyleForTextAutosizing):
* rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::isIdempotentTextAutosizingCandidate const):

Rename AutosizeStatus::shouldSkipSubtree to RenderStyle::isIdempotentTextAutosizingCandidate. We relocate this
logic to RenderStyle, such that we're able to ask the element's RenderStyle questions when determining whether
the element should be autosized.

Of course, this patch additionally revamps the heuristic used to determine whether it is safe to autosize an
element. Our current heuristic in trunk simply checks for the presence of inline block display, out of flow
positioning and a fixed height ancestor; if any of these conditions are satisfied, we opt the element out of
text autosizing. This is an excellent strategy for boosting some runs of text while avoiding autosizing in the
vast majority of cases where increasing font size may lead to layout breakage (e.g. overlapping or clipped text,
content unexpectedly flowing to the next line, etc.). However, it also avoids boosting font sizes in many
scenarios where boosting font sizes is desired; for concrete examples, see the (currently 24) radars about small
font sizes that are duped to <rdar://problem/51826266>.

To help analyze and identify trends in autosizable and non-autosizable text, we assembled a dataset of elements
with text from the Alexa top 500 that either: (1) were too small and could be boosted safely, or (2) would break
layout if boosted. With this labeled dataset, we then trained binary decision trees to classify the data. Each
decision tree was trained with a number of hyperparameters: namely, maximum depth, minimum leaf size, and the
amount of bias towards negative samples (i.e. the ratio of the weight of a non-autosizable sample relative to
the weight of an autosizable sample).

For each 3-tuple of these hyperparameters (800 in total: max depth between 3 and 10, min leaf size between 1 and
10 and bias between 1 and 10), for 5000 iterations each, we split the full dataset into a training dataset and
a cross-validation dataset, trained a decision tree using the training set, and tested against the cross-
validation set to compute average precision, recall, and overall accuracy for each tuple of hyperparameters.

The decision tree introduced in this patch was generated using a hand-picked set of hyperparameters (max depth
10, min leaf size 4, and negative bias 2) to provide a balance between precision scores (limiting layout
breakage) and recall score (ensuring that small text is mostly autosized), while optimizing for overall
accuracy. Cross-validation scores predict that the overall accuracy of this classifier is approximately 70%, up
from the current accuracy in trunk (~53%).

* rendering/style/RenderStyle.h:

Grow the width of `autosizeStatus` from 4 to 8 (notably, this does not increase the size of RenderStyle).

* rendering/style/TextSizeAdjustment.cpp:
(WebCore::AutosizeStatus::updateStatus):
(WebCore::AutosizeStatus::shouldSkipSubtree const): Deleted.
* rendering/style/TextSizeAdjustment.h:

Introduce new text autosizing state flags, and remove some existing ones.

LayoutTests:

Rebaseline an existing text autosizing test, and introduce some new test cases that correspond to several common
patterns of autosizable (or non-autosizable) text on websites that were surveyed.

* fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-after-changing-initial-scale.html:
* fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates-expected.txt: Added.
* fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates.html: Renamed from LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip.html.

Rename this existing layout test too, to avoid using the term "skip" in the name of a layout test.

* fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip-expected.txt: Removed.

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

LayoutTests/ChangeLog
LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-after-changing-initial-scale.html
LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates-expected.txt [new file with mode: 0644]
LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates.html [moved from LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip.html with 54% similarity]
LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip-expected.txt [deleted file]
Source/WebCore/ChangeLog
Source/WebCore/css/StyleResolver.cpp
Source/WebCore/rendering/style/RenderStyle.cpp
Source/WebCore/rendering/style/RenderStyle.h
Source/WebCore/rendering/style/TextSizeAdjustment.cpp
Source/WebCore/rendering/style/TextSizeAdjustment.h

index 2a5c776..8a9fa8f 100644 (file)
@@ -1,3 +1,22 @@
+2019-06-24  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Text autosizing] [iPadOS] Revise our heuristics to determine idempotent text autosizing candidates
+        https://bugs.webkit.org/show_bug.cgi?id=198763
+        <rdar://problem/51826266>
+
+        Reviewed by Simon Fraser.
+
+        Rebaseline an existing text autosizing test, and introduce some new test cases that correspond to several common
+        patterns of autosizable (or non-autosizable) text on websites that were surveyed.
+
+        * fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-after-changing-initial-scale.html:
+        * fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates-expected.txt: Added.
+        * fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates.html: Renamed from LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip.html.
+
+        Rename this existing layout test too, to avoid using the term "skip" in the name of a layout test.
+
+        * fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip-expected.txt: Removed.
+
 2019-06-24  Simon Fraser  <simon.fraser@apple.com>
 
         REGRESSION (r246725 ): Crashes on twitch.tv
diff --git a/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates-expected.txt b/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates-expected.txt
new file mode 100644 (file)
index 0000000..0a6c89e
--- /dev/null
@@ -0,0 +1,50 @@
+Checking target1:
+PASS result is >= 13
+Checking target2:
+PASS result is >= 13
+Checking target3:
+PASS result is 12
+Checking target4:
+PASS result is >= 13
+Checking target5:
+PASS result is >= 13
+Checking target6:
+PASS result is 12
+Checking target7:
+PASS result is >= result2
+Checking target8:
+PASS result is >= 13
+Checking target9:
+PASS result is >= 13
+Checking target10:
+PASS result is >= 13
+Checking target11:
+PASS result is >= 13
+Checking target12:
+PASS result is 12
+Checking target13:
+PASS result is >= 13
+Checking target14:
+PASS result is 12
+Checking target15:
+PASS result is >= 13
+Checking target16:
+PASS result is >= 13
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Test
+Test
+Test
+Test
+Test
+TestTestTestTest
+Test
+Test
+Test
+Test
+Test
+Test
+Test
+Test
+Test
@@ -11,32 +11,42 @@ if (window.internals) {
 <script src="../../../../resources/js-test-pre.js"></script>
 </head>
 <body>
-<div style="background: green;"><span id="target" style="font-size: 12px;">Test</span></div>
+<div style="background: green;"><span id="target1" style="font-size: 12px;">Test</span></div>
 <div style="background: green; overflow: auto;"><span id="target2" style="float: right; font-size: 12px;">Test</span></div>
-<div style="background: green;"><span id="target3" style="display: inline-block; font-size: 12px;">Test</span></div>
-<div style="background: green;"><span style="display: inline-block; font-size: 12px;"><span id="target4">Test</span></span></div>
+<div style="background: green;"><span id="target3" style="display: inline-block; font-size: 12px; width: 20px;">Test</span></div>
+<div style="background: green;"><span style="display: inline-block; height: 12px; font-size: 12px;"><span id="target4">Test</span></span></div>
 <div style="background: green;"><span id="target5" style="position: absolute; left: 0px; top: 0px; font-size: 12px;">Test</span></div>
 <div style="background: green;"><span id="target6" style="display: none; font-size: 12px;">Test</span></div>
 <div style="background: green;"><span id="comparison" style="font-size: 12px;">Test<span>Test<span>Test<span id="target7">Test</span></span></span></span></div>
 <div style="background: green;"><span id="target8" style="font-size: 12px; -webkit-text-size-adjust: 100%">Test</span></div>
+<div style="background: green;"><span id="target9" style="font-size: 12px; display: inline-block;">Test</span></div>
+<div style="background: green;"><span id="target10" style="font-size: 12px; width: 20px; height: 12px;">Test</span></div>
+<div style="background: green;"><span id="target11" style="font-size: 12px; height: 20px; position: fixed; white-space: nowrap;">Test</span></div>
+<div style="background: green;"><span id="target12" style="font-size: 12px; height: 20px; position: fixed; float: right;">Test</span></div>
+<div style="background: green;"><span id="target13" style="font-size: 12px; height: 20px; position: fixed; float: right; overflow-x: hidden; width: 100px;">Test</span></div>
+<div style="background: green;"><span id="target14" style="font-size: 12px; height: 20px; width: 100px; float: right;">Test</span></div>
+<div style="background: green;"><span id="target15" style="overflow-y: hidden; float: right;">Test</span></div>
+<div style="background: green;"><span id="target16" style="float: right;">Test</span></div>
 <script>
 let result;
 function check(name, shouldGetAutosized) {
     let target = document.getElementById(name);
     target.offsetWidth;
     result = Number.parseInt(window.getComputedStyle(target).getPropertyValue("font-size"));
+    debug(`Checking ${name}:`);
     if (shouldGetAutosized)
         shouldBeGreaterThanOrEqual("result", "13");
     else
         shouldBe("result", "12");
 }
-check("target", true);
+check("target1", true);
 check("target2", true);
 check("target3", false);
-check("target4", false);
-check("target5", false);
+check("target4", true);
+check("target5", true);
 check("target6", false);
 
+debug(`Checking target7:`);
 let target = document.getElementById("target7");
 target.offsetWidth;
 let comparison = document.getElementById("comparison");
@@ -46,6 +56,17 @@ let result2 = Number.parseInt(window.getComputedStyle(comparison).getPropertyVal
 shouldBeGreaterThanOrEqual("result", "result2");
 
 check("target8", true);
+check("target9", true);
+check("target10", true);
+
+// Below are some common scenarios where we prefer (or prefer to not) adjust text size. These examples are inspired by
+// common patterns in real webpages.
+check("target11", true);
+check("target12", false);
+check("target13", true);
+check("target14", false);
+check("target15", true);
+check("target16", true);
 </script>
 <script src="../../../../resources/js-test-post.js"></script>
 </body>
diff --git a/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip-expected.txt b/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip-expected.txt
deleted file mode 100644 (file)
index 339d190..0000000
+++ /dev/null
@@ -1,18 +0,0 @@
-PASS result is >= 13
-PASS result is >= 13
-PASS result is 12
-PASS result is 12
-PASS result is 12
-PASS result is 12
-PASS result is >= result2
-PASS result is >= 13
-PASS successfullyParsed is true
-
-TEST COMPLETE
-Test
-Test
-Test
-Test
-Test
-TestTestTestTest
-Test
index 3ef19a3..e54e326 100644 (file)
@@ -1,3 +1,68 @@
+2019-06-24  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Text autosizing] [iPadOS] Revise our heuristics to determine idempotent text autosizing candidates
+        https://bugs.webkit.org/show_bug.cgi?id=198763
+        <rdar://problem/51826266>
+
+        Reviewed by Simon Fraser.
+
+        This patch adjusts existing text autosizing heuristics, based on a survey of text on websites in the Alexa top
+        500 that shrink down to fit the viewport when requesting the desktop version of the site. The new heuristic is
+        derived from training decision trees against the dataset obtained from this survey, and balances false positives
+        (cases where layout is broken due to autosizing) against overall accuracy (measured using cross-validation).
+
+        See below for more details. Additionally, please refer to the link in the radar for more details, as well as
+        resources used to generate, validate, and analyze these decision trees.
+
+        Test: fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates.html
+
+        * css/StyleResolver.cpp:
+        (WebCore::StyleResolver::adjustRenderStyleForTextAutosizing):
+        * rendering/style/RenderStyle.cpp:
+        (WebCore::RenderStyle::isIdempotentTextAutosizingCandidate const):
+
+        Rename AutosizeStatus::shouldSkipSubtree to RenderStyle::isIdempotentTextAutosizingCandidate. We relocate this
+        logic to RenderStyle, such that we're able to ask the element's RenderStyle questions when determining whether
+        the element should be autosized.
+
+        Of course, this patch additionally revamps the heuristic used to determine whether it is safe to autosize an
+        element. Our current heuristic in trunk simply checks for the presence of inline block display, out of flow
+        positioning and a fixed height ancestor; if any of these conditions are satisfied, we opt the element out of
+        text autosizing. This is an excellent strategy for boosting some runs of text while avoiding autosizing in the
+        vast majority of cases where increasing font size may lead to layout breakage (e.g. overlapping or clipped text,
+        content unexpectedly flowing to the next line, etc.). However, it also avoids boosting font sizes in many
+        scenarios where boosting font sizes is desired; for concrete examples, see the (currently 24) radars about small
+        font sizes that are duped to <rdar://problem/51826266>.
+
+        To help analyze and identify trends in autosizable and non-autosizable text, we assembled a dataset of elements
+        with text from the Alexa top 500 that either: (1) were too small and could be boosted safely, or (2) would break
+        layout if boosted. With this labeled dataset, we then trained binary decision trees to classify the data. Each
+        decision tree was trained with a number of hyperparameters: namely, maximum depth, minimum leaf size, and the
+        amount of bias towards negative samples (i.e. the ratio of the weight of a non-autosizable sample relative to
+        the weight of an autosizable sample).
+
+        For each 3-tuple of these hyperparameters (800 in total: max depth between 3 and 10, min leaf size between 1 and
+        10 and bias between 1 and 10), for 5000 iterations each, we split the full dataset into a training dataset and
+        a cross-validation dataset, trained a decision tree using the training set, and tested against the cross-
+        validation set to compute average precision, recall, and overall accuracy for each tuple of hyperparameters.
+
+        The decision tree introduced in this patch was generated using a hand-picked set of hyperparameters (max depth
+        10, min leaf size 4, and negative bias 2) to provide a balance between precision scores (limiting layout
+        breakage) and recall score (ensuring that small text is mostly autosized), while optimizing for overall
+        accuracy. Cross-validation scores predict that the overall accuracy of this classifier is approximately 70%, up
+        from the current accuracy in trunk (~53%).
+
+        * rendering/style/RenderStyle.h:
+
+        Grow the width of `autosizeStatus` from 4 to 8 (notably, this does not increase the size of RenderStyle).
+
+        * rendering/style/TextSizeAdjustment.cpp:
+        (WebCore::AutosizeStatus::updateStatus):
+        (WebCore::AutosizeStatus::shouldSkipSubtree const): Deleted.
+        * rendering/style/TextSizeAdjustment.h:
+
+        Introduce new text autosizing state flags, and remove some existing ones.
+
 2019-06-24  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r246714.
index 0248790..b3e6e14 100644 (file)
@@ -879,17 +879,17 @@ static bool hasTextChildren(const Element& element)
 
 void StyleResolver::adjustRenderStyleForTextAutosizing(RenderStyle& style, const Element& element)
 {
-    auto newAutosizeStatus = AutosizeStatus::updateStatus(style);
     if (!settings().textAutosizingEnabled() || !settings().textAutosizingUsesIdempotentMode())
         return;
 
+    AutosizeStatus::updateStatus(style);
     if (!hasTextChildren(element))
         return;
 
     if (style.textSizeAdjust().isNone())
         return;
 
-    if (newAutosizeStatus.shouldSkipSubtree())
+    if (!style.isIdempotentTextAutosizingCandidate())
         return;
 
     float initialScale = document().page() ? document().page()->initialScale() : 1;
index 14b1ca0..b0e288b 100644 (file)
@@ -492,6 +492,33 @@ bool RenderStyle::equalForTextAutosizing(const RenderStyle& other) const
         && m_rareNonInheritedData->textOverflow == other.m_rareNonInheritedData->textOverflow;
 }
 
+bool RenderStyle::isIdempotentTextAutosizingCandidate() const
+{
+    // Refer to <rdar://problem/51826266> for more information regarding how this function was generated.
+    auto fields = OptionSet<AutosizeStatus::Fields>::fromRaw(m_inheritedFlags.autosizeStatus);
+    if (fields.contains(AutosizeStatus::Fields::DisplayNone))
+        return false;
+
+    if (fields.contains(AutosizeStatus::Fields::FixedHeight)) {
+        if (whiteSpace() == WhiteSpace::NoWrap)
+            return true;
+
+        if (fields.contains(AutosizeStatus::Fields::Floating))
+            return fields.contains(AutosizeStatus::Fields::OutOfFlowPosition) && fields.contains(AutosizeStatus::Fields::OverflowXHidden);
+
+        if (fields.contains(AutosizeStatus::Fields::FixedWidth))
+            return !fields.contains(AutosizeStatus::Fields::OutOfFlowPosition);
+    }
+
+    if (fields.contains(AutosizeStatus::Fields::Floating))
+        return true;
+
+    if (fields.contains(AutosizeStatus::Fields::FixedWidth))
+        return fields.contains(AutosizeStatus::Fields::OverflowYHidden);
+
+    return !fields.contains(AutosizeStatus::Fields::OverflowYHidden) && !fields.contains(AutosizeStatus::Fields::FixedMaxWidth);
+}
+
 AutosizeStatus RenderStyle::autosizeStatus() const
 {
     return OptionSet<AutosizeStatus::Fields>::fromRaw(m_inheritedFlags.autosizeStatus);
index ec30ea6..c0d837d 100644 (file)
@@ -741,6 +741,7 @@ public:
 #if ENABLE(TEXT_AUTOSIZING)
     TextSizeAdjustment textSizeAdjust() const { return m_rareInheritedData->textSizeAdjust; }
     AutosizeStatus autosizeStatus() const;
+    bool isIdempotentTextAutosizingCandidate() const;
 #endif
 
     TextSecurity textSecurity() const { return static_cast<TextSecurity>(m_rareInheritedData->textSecurity); }
@@ -1834,9 +1835,9 @@ private:
         // 48 bits
 
 #if ENABLE(TEXT_AUTOSIZING)
-        unsigned autosizeStatus : 4;
+        unsigned autosizeStatus : 8;
 #endif
-        // 52 bits
+        // 56 bits
     };
 
     // This constructor is used to implement the replace operation.
index 0200278..cc1da80 100644 (file)
@@ -42,30 +42,35 @@ bool AutosizeStatus::contains(Fields fields) const
     return m_fields.contains(fields);
 }
 
-AutosizeStatus AutosizeStatus::updateStatus(RenderStyle& style)
+void AutosizeStatus::updateStatus(RenderStyle& style)
 {
-    OptionSet<Fields> result = style.autosizeStatus().fields();
+    auto result = style.autosizeStatus().fields();
+
+    if (style.display() == DisplayType::None)
+        result.add(Fields::DisplayNone);
+
     if (style.hasOutOfFlowPosition())
-        result.add(Fields::FoundOutOfFlowPosition);
-    switch (style.display()) {
-    case DisplayType::InlineBlock:
-        result.add(Fields::FoundInlineBlock);
-        break;
-    case DisplayType::None:
-        result.add(Fields::FoundDisplayNone);
-        break;
-    default: // FIXME: Add more cases.
-        break;
-    }
+        result.add(Fields::OutOfFlowPosition);
+
     if (style.height().isFixed())
-        result.add(Fields::FoundFixedHeight);
-    style.setAutosizeStatus(result);
-    return result;
-}
+        result.add(Fields::FixedHeight);
 
-bool AutosizeStatus::shouldSkipSubtree() const
-{
-    return m_fields.containsAny({ Fields::FoundOutOfFlowPosition, Fields::FoundInlineBlock, Fields::FoundFixedHeight, Fields::FoundDisplayNone });
+    if (style.width().isFixed())
+        result.add(Fields::FixedWidth);
+
+    if (style.maxWidth().isFixed())
+        result.add(Fields::FixedMaxWidth);
+
+    if (style.overflowX() == Overflow::Hidden)
+        result.add(Fields::OverflowXHidden);
+
+    if (style.overflowY() == Overflow::Hidden)
+        result.add(Fields::OverflowYHidden);
+
+    if (style.isFloating())
+        result.add(Fields::Floating);
+
+    style.setAutosizeStatus(result);
 }
 
 float AutosizeStatus::idempotentTextSize(float specifiedSize, float pageScale)
index 9f6b3fb..ffc0344 100644 (file)
@@ -52,10 +52,14 @@ private:
 class AutosizeStatus {
 public:
     enum class Fields : uint8_t {
-        FoundOutOfFlowPosition = 1 << 0,
-        FoundInlineBlock = 1 << 1,
-        FoundFixedHeight = 1 << 2,
-        FoundDisplayNone = 1 << 3
+        DisplayNone = 1 << 0,
+        FixedHeight = 1 << 1,
+        FixedWidth = 1 << 2,
+        Floating = 1 << 3,
+        OverflowXHidden = 1 << 4,
+        OverflowYHidden = 1 << 5,
+        OutOfFlowPosition = 1 << 6,
+        FixedMaxWidth = 1 << 7
         // Adding new values requires giving RenderStyle::InheritedFlags::autosizeStatus additional bits.
     };
 
@@ -63,10 +67,9 @@ public:
     OptionSet<Fields> fields() const { return m_fields; }
 
     bool contains(Fields) const;
-    bool shouldSkipSubtree() const;
 
     static float idempotentTextSize(float specifiedSize, float pageScale);
-    static AutosizeStatus updateStatus(RenderStyle&);
+    static void updateStatus(RenderStyle&);
 
 private:
     OptionSet<Fields> m_fields;