Title: [90798] trunk
Revision
90798
Author
[email protected]
Date
2011-07-11 17:39:46 -0700 (Mon, 11 Jul 2011)

Log Message

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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (90797 => 90798)


--- trunk/LayoutTests/ChangeLog	2011-07-12 00:30:54 UTC (rev 90797)
+++ trunk/LayoutTests/ChangeLog	2011-07-12 00:39:46 UTC (rev 90798)
@@ -1,3 +1,14 @@
+2011-07-11  Dan Bernstein  <[email protected]>
+
+        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  <[email protected]>
 
         Win7 rebaselines after http://trac.webkit.org/changeset/90701.

Added: trunk/LayoutTests/platform/mac/fast/text/rounding-hacks-expansion.html (0 => 90798)


--- trunk/LayoutTests/platform/mac/fast/text/rounding-hacks-expansion.html	                        (rev 0)
+++ trunk/LayoutTests/platform/mac/fast/text/rounding-hacks-expansion.html	2011-07-12 00:39:46 UTC (rev 90798)
@@ -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>

Added: trunk/LayoutTests/platform/mac/platform/mac/fast/text/rounding-hacks-expansion-expected.png


(Binary files differ)
Property changes on: trunk/LayoutTests/platform/mac/platform/mac/fast/text/rounding-hacks-expansion-expected.png ___________________________________________________________________

Added: svn:mime-type

Added: trunk/LayoutTests/platform/mac/platform/mac/fast/text/rounding-hacks-expansion-expected.txt (0 => 90798)


--- trunk/LayoutTests/platform/mac/platform/mac/fast/text/rounding-hacks-expansion-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac/platform/mac/fast/text/rounding-hacks-expansion-expected.txt	2011-07-12 00:39:46 UTC (rev 90798)
@@ -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"

Modified: trunk/Source/WebCore/ChangeLog (90797 => 90798)


--- trunk/Source/WebCore/ChangeLog	2011-07-12 00:30:54 UTC (rev 90797)
+++ trunk/Source/WebCore/ChangeLog	2011-07-12 00:39:46 UTC (rev 90798)
@@ -1,3 +1,20 @@
+2011-07-11  Dan Bernstein  <[email protected]>
+
+        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  <[email protected]>
 
         HTML5 video controller in fullscreen is partly off-screen (at least on youtube) using ClickToFlash

Modified: trunk/Source/WebCore/platform/graphics/WidthIterator.cpp (90797 => 90798)


--- trunk/Source/WebCore/platform/graphics/WidthIterator.cpp	2011-07-12 00:30:54 UTC (rev 90797)
+++ trunk/Source/WebCore/platform/graphics/WidthIterator.cpp	2011-07-12 00:39:46 UTC (rev 90798)
@@ -161,21 +161,24 @@
             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

Modified: trunk/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp (90797 => 90798)


--- trunk/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp	2011-07-12 00:30:54 UTC (rev 90797)
+++ trunk/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp	2011-07-12 00:39:46 UTC (rev 90798)
@@ -489,18 +489,21 @@
                 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
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to