Excessive expansion of justified text when rounding hacks are enabled

https://bugs.webkit.org/show_bug.cgi?id=64331

Reviewed by Anders Carlsson.

Source/WebCore:

Test: platform/mac/fast/text/rounding-hacks-expansion.html

When rounding hacks are enabled, the expansion at each expansion opportunity should be by an
integer. Restored more of the logic that was removed in r78846 in order to ensure this.

* platform/graphics/WidthIterator.cpp:
(WebCore::WidthIterator::advance):
* platform/graphics/mac/ComplexTextController.cpp:
(WebCore::ComplexTextController::adjustGlyphsAndAdvances):

LayoutTests:

* platform/mac/fast/text/rounding-hacks-expansion.html: Added.
* platform/mac/platform/mac/fast/text/rounding-hacks-expansion-expected.png: Added.
* platform/mac/platform/mac/fast/text/rounding-hacks-expansion-expected.txt: Added.

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index c91a095a20636b33521b4daa06fe47ce830328b1..4c71208bd2039a99a3841bdc0e8a6d8df089e217 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2011-07-11  Dan Bernstein  <mitz@apple.com>
+
+        Excessive expansion of justified text when rounding hacks are enabled
+        https://bugs.webkit.org/show_bug.cgi?id=64331
+
+        Reviewed by Anders Carlsson.
+
+        * platform/mac/fast/text/rounding-hacks-expansion.html: Added.
+        * platform/mac/platform/mac/fast/text/rounding-hacks-expansion-expected.png: Added.
+        * platform/mac/platform/mac/fast/text/rounding-hacks-expansion-expected.txt: Added.
+
 2011-07-11  Ojan Vafai  <ojan@chromium.org>
 
         Win7 rebaselines after http://trac.webkit.org/changeset/90701.

diff --git a/LayoutTests/platform/mac/fast/text/rounding-hacks-expansion.html b/LayoutTests/platform/mac/fast/text/rounding-hacks-expansion.html
new file mode 100644 (file)
index 0000000..8c3b7f9
--- /dev/null
+++ b/LayoutTests/platform/mac/fast/text/rounding-hacks-expansion.html
@@ -0,0 +1,17 @@
+<script>
+    if (window.layoutTestController)
+        layoutTestController.allowRoundingHacks();
+</script>
+<div style="border: solid cyan; font: 16px 'lucida grande'; text-align: justify; width: 300px;">Given an opportunity to make a mistake</div>
+<div style="border: solid cyan; font: 16px 'lucida grande'; text-align: justify; width: 300px;">Given an opportunity to make a mistake</div>
+<div style="border: solid cyan; font: 16px 'lucida grande'; text-align: justify; width: 300px;">Given an opportunity to make a mistake</div>
+<div style="border: solid cyan; font: 16px 'lucida grande'; text-align: justify; width: 300px;">Given an opportunity to make a mistake</div>
+<div style="border: solid cyan; font: 16px 'lucida grande'; text-align: justify; width: 300px;">Given an opportunity to make a mistake</div>
+<div style="border: solid cyan; font: 16px 'lucida grande'; text-align: justify; width: 300px;">Given an opportunity to make a mistake</div>
+<div style="border: solid cyan; font: 16px 'lucida grande'; text-align: justify; width: 300px;">Given an opportunity to make a mistake</div>
+<div style="text-rendering: optimizelegibility; border: solid cyan; font: 16px 'lucida grande'; text-align: justify; width: 300px;">Given an opportunity to make a mistake</div>
+<div style="text-rendering: optimizelegibility; border: solid cyan; font: 16px 'lucida grande'; text-align: justify; width: 300px;">Given an opportunity to make a mistake</div>
+<div style="text-rendering: optimizelegibility; border: solid cyan; font: 16px 'lucida grande'; text-align: justify; width: 300px;">Given an opportunity to make a mistake</div>
+<div style="text-rendering: optimizelegibility; border: solid cyan; font: 16px 'lucida grande'; text-align: justify; width: 300px;">Given an opportunity to make a mistake</div>
+<div style="text-rendering: optimizelegibility; border: solid cyan; font: 16px 'lucida grande'; text-align: justify; width: 300px;">Given an opportunity to make a mistake</div>
+<div style="text-rendering: optimizelegibility; border: solid cyan; font: 16px 'lucida grande'; text-align: justify; width: 300px;">Given an opportunity to make a mistake</div>

diff --git a/LayoutTests/platform/mac/platform/mac/fast/text/rounding-hacks-expansion-expected.png b/LayoutTests/platform/mac/platform/mac/fast/text/rounding-hacks-expansion-expected.png
new file mode 100644 (file)
index 0000000..315bc43
Binary files /dev/null and b/LayoutTests/platform/mac/platform/mac/fast/text/rounding-hacks-expansion-expected.png differ

diff --git a/LayoutTests/platform/mac/platform/mac/fast/text/rounding-hacks-expansion-expected.txt b/LayoutTests/platform/mac/platform/mac/fast/text/rounding-hacks-expansion-expected.txt
new file mode 100644 (file)
index 0000000..239f8a8
--- /dev/null
+++ b/LayoutTests/platform/mac/platform/mac/fast/text/rounding-hacks-expansion-expected.txt
@@ -0,0 +1,57 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {DIV} at (0,0) size 306x42 [border: (3px solid #00FFFF)]
+        RenderText {#text} at (3,3) size 300x36
+          text run at (3,3) width 300: "Given an opportunity to make a"
+          text run at (3,21) width 61: "mistake"
+      RenderBlock {DIV} at (0,42) size 306x42 [border: (3px solid #00FFFF)]
+        RenderText {#text} at (3,3) size 300x36
+          text run at (3,3) width 300: "Given an opportunity to make a"
+          text run at (3,21) width 61: "mistake"
+      RenderBlock {DIV} at (0,84) size 306x42 [border: (3px solid #00FFFF)]
+        RenderText {#text} at (3,3) size 300x36
+          text run at (3,3) width 300: "Given an opportunity to make a"
+          text run at (3,21) width 61: "mistake"
+      RenderBlock {DIV} at (0,126) size 306x42 [border: (3px solid #00FFFF)]
+        RenderText {#text} at (3,3) size 300x36
+          text run at (3,3) width 300: "Given an opportunity to make a"
+          text run at (3,21) width 61: "mistake"
+      RenderBlock {DIV} at (0,168) size 306x42 [border: (3px solid #00FFFF)]
+        RenderText {#text} at (3,3) size 300x36
+          text run at (3,3) width 300: "Given an opportunity to make a"
+          text run at (3,21) width 61: "mistake"
+      RenderBlock {DIV} at (0,210) size 306x42 [border: (3px solid #00FFFF)]
+        RenderText {#text} at (3,3) size 300x36
+          text run at (3,3) width 300: "Given an opportunity to make a"
+          text run at (3,21) width 61: "mistake"
+      RenderBlock {DIV} at (0,252) size 306x42 [border: (3px solid #00FFFF)]
+        RenderText {#text} at (3,3) size 300x36
+          text run at (3,3) width 300: "Given an opportunity to make a"
+          text run at (3,21) width 61: "mistake"
+      RenderBlock {DIV} at (0,294) size 306x42 [border: (3px solid #00FFFF)]
+        RenderText {#text} at (3,3) size 300x36
+          text run at (3,3) width 300: "Given an opportunity to make a"
+          text run at (3,21) width 61: "mistake"
+      RenderBlock {DIV} at (0,336) size 306x42 [border: (3px solid #00FFFF)]
+        RenderText {#text} at (3,3) size 300x36
+          text run at (3,3) width 300: "Given an opportunity to make a"
+          text run at (3,21) width 61: "mistake"
+      RenderBlock {DIV} at (0,378) size 306x42 [border: (3px solid #00FFFF)]
+        RenderText {#text} at (3,3) size 300x36
+          text run at (3,3) width 300: "Given an opportunity to make a"
+          text run at (3,21) width 61: "mistake"
+      RenderBlock {DIV} at (0,420) size 306x42 [border: (3px solid #00FFFF)]
+        RenderText {#text} at (3,3) size 300x36
+          text run at (3,3) width 300: "Given an opportunity to make a"
+          text run at (3,21) width 61: "mistake"
+      RenderBlock {DIV} at (0,462) size 306x42 [border: (3px solid #00FFFF)]
+        RenderText {#text} at (3,3) size 300x36
+          text run at (3,3) width 300: "Given an opportunity to make a"
+          text run at (3,21) width 61: "mistake"
+      RenderBlock {DIV} at (0,504) size 306x42 [border: (3px solid #00FFFF)]
+        RenderText {#text} at (3,3) size 300x36
+          text run at (3,3) width 300: "Given an opportunity to make a"
+          text run at (3,21) width 61: "mistake"

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index e78a1dfe1ee674d2773ea01440905475db371279..209883b4f6708e4b5ebd400988afda8738691c79 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,20 @@
+2011-07-11  Dan Bernstein  <mitz@apple.com>
+
+        Excessive expansion of justified text when rounding hacks are enabled
+        https://bugs.webkit.org/show_bug.cgi?id=64331
+
+        Reviewed by Anders Carlsson.
+
+        Test: platform/mac/fast/text/rounding-hacks-expansion.html
+
+        When rounding hacks are enabled, the expansion at each expansion opportunity should be by an
+        integer. Restored more of the logic that was removed in r78846 in order to ensure this.
+
+        * platform/graphics/WidthIterator.cpp:
+        (WebCore::WidthIterator::advance):
+        * platform/graphics/mac/ComplexTextController.cpp:
+        (WebCore::ComplexTextController::adjustGlyphsAndAdvances):
+
 2011-07-11  Jer Noble  <jer.noble@apple.com>
 
         HTML5 video controller in fullscreen is partly off-screen (at least on youtube) using ClickToFlash

diff --git a/Source/WebCore/platform/graphics/WidthIterator.cpp b/Source/WebCore/platform/graphics/WidthIterator.cpp
index b67b4b25555f5d33c89f01493eda2bfee7c6f904..d749a4f46805bcb42474c37c0547f1abb0d7a70d 100644 (file)
--- a/Source/WebCore/platform/graphics/WidthIterator.cpp
+++ b/Source/WebCore/platform/graphics/WidthIterator.cpp
@@ -161,21 +161,24 @@ unsigned WidthIterator::advance(int offset, GlyphBuffer* glyphBuffer)
             if (treatAsSpace || (expandAroundIdeographs && Font::isCJKIdeographOrSymbol(character))) {
                 // Distribute the run's total expansion evenly over all expansion opportunities in the run.
                 if (m_expansion) {
+                    float previousExpansion = m_expansion;
                     if (!treatAsSpace && !m_isAfterExpansion) {
                         // Take the expansion opportunity before this ideograph.
                         m_expansion -= m_expansionPerOpportunity;
-                        m_runWidthSoFar += m_expansionPerOpportunity;
+                        float expansionAtThisOpportunity = !m_run.applyWordRounding() ? m_expansionPerOpportunity : roundf(previousExpansion) - roundf(m_expansion);
+                        m_runWidthSoFar += expansionAtThisOpportunity;
                         if (glyphBuffer) {
                             if (glyphBuffer->isEmpty())
-                                glyphBuffer->add(fontData->spaceGlyph(), fontData, m_expansionPerOpportunity);
+                                glyphBuffer->add(fontData->spaceGlyph(), fontData, expansionAtThisOpportunity);
                             else
-                                glyphBuffer->expandLastAdvance(m_expansionPerOpportunity);
+                                glyphBuffer->expandLastAdvance(expansionAtThisOpportunity);
                         }
+                        previousExpansion = m_expansion;
                     }
                     if (m_run.allowsTrailingExpansion() || (m_run.ltr() && textIterator.currentCharacter() + advanceLength < static_cast<size_t>(m_run.length()))
                         || (m_run.rtl() && textIterator.currentCharacter())) {
                         m_expansion -= m_expansionPerOpportunity;
-                        width += m_expansionPerOpportunity;
+                        width += !m_run.applyWordRounding() ? m_expansionPerOpportunity : roundf(previousExpansion) - roundf(m_expansion);
                         m_isAfterExpansion = true;
                     }
                 } else

diff --git a/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp b/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp
index 33b2dbe2948b8878fa9da7331363dcb21cd203a1..a324e585a8b530efd8e7b0db1b912d4f28a522e1 100644 (file)
--- a/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp
+++ b/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp
@@ -489,18 +489,21 @@ void ComplexTextController::adjustGlyphsAndAdvances()
                 if (treatAsSpace || Font::isCJKIdeographOrSymbol(ch)) {
                     // Distribute the run's total expansion evenly over all expansion opportunities in the run.
                     if (m_expansion) {
+                        float previousExpansion = m_expansion;
                         if (!treatAsSpace && !m_afterExpansion) {
                             // Take the expansion opportunity before this ideograph.
                             m_expansion -= m_expansionPerOpportunity;
-                            m_totalWidth += m_expansionPerOpportunity;
+                            float expansionAtThisOpportunity = !m_run.applyWordRounding() ? m_expansionPerOpportunity : roundf(previousExpansion) - roundf(m_expansion);
+                            m_totalWidth += expansionAtThisOpportunity;
                             if (m_adjustedAdvances.isEmpty())
-                                m_leadingExpansion = m_expansionPerOpportunity;
+                                m_leadingExpansion = expansionAtThisOpportunity;
                             else
-                                m_adjustedAdvances.last().width += m_expansionPerOpportunity;
+                                m_adjustedAdvances.last().width += expansionAtThisOpportunity;
+                            previousExpansion = m_expansion;
                         }
                         if (!lastGlyph || m_run.allowsTrailingExpansion()) {
                             m_expansion -= m_expansionPerOpportunity;
-                            advance.width += m_expansionPerOpportunity;
+                            advance.width += !m_run.applyWordRounding() ? m_expansionPerOpportunity : roundf(previousExpansion) - roundf(m_expansion);
                             m_afterExpansion = true;
                         }
                     } else