Changeset 40471 in webkit

Timestamp:

Jan 31, 2009 10:23:39 PM (15 years ago)

Author:

mitz@apple.com

Message:

Reviewed by Oliver Hunt.

• fix <rdar://problem/6546549> Eliminate some RenderText::width() overhead from findNextLineBreak()

On the PLT, this results in 0.36x the number of virtual function calls
to RenderText::width() and 0.69x the number of calls to
Font::isFixedPitch(), but makes 1.0004x the number of calls to
Font::width().

• rendering/RenderText.cpp: (WebCore::RenderText::width): Replaced bounds checks on 'from' and 'len' with an assertion.
• rendering/bidi.cpp: (WebCore::textWidth): Added this inlined helper function which calls Font::width() directly in the non-fixed-pitch, non-full-range case, and otherwise calls RenderText::width(). (WebCore::RenderBlock::findNextLineBreak): Cache whether the font has fixed pitch (in which case RenderText::width() will be called in order to take advantage of the widthFromCache() optimization for fixed-pitch fonts). Replaced all calls to RenderText::width() with calls to the textWidth() helper function.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/RenderText.cpp (modified) (2 diffs)
• rendering/bidi.cpp (modified) (8 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2009-01-31  Dan Bernstein  <mitz@apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        - fix <rdar://problem/6546549> Eliminate some RenderText::width() overhead from findNextLineBreak()
+
+        On the PLT, this results in 0.36x the number of virtual function calls
+        to RenderText::width() and 0.69x the number of calls to
+        Font::isFixedPitch(), but makes 1.0004x the number of calls to
+        Font::width().
+
+        * rendering/RenderText.cpp:
+        (WebCore::RenderText::width): Replaced bounds checks on 'from' and 'len'
+        with an assertion.
+        * rendering/bidi.cpp:
+        (WebCore::textWidth): Added this inlined helper function which calls
+        Font::width() directly in the non-fixed-pitch, non-full-range case, and
+        otherwise calls RenderText::width().
+        (WebCore::RenderBlock::findNextLineBreak): Cache whether the font has
+        fixed pitch (in which case RenderText::width() will be called in order
+        to take advantage of the widthFromCache() optimization for fixed-pitch
+        fonts). Replaced all calls to RenderText::width() with calls to the
+        textWidth() helper function.
+
 2009-01-31  David Hyatt  <hyatt@apple.com>
 

trunk/WebCore/rendering/RenderText.cpp

@@ -1015 +1015 @@
 }
 
-unsigned int RenderText::width(unsigned int from, unsigned int len, int xPos, bool firstLine) const
+unsigned RenderText::width(unsigned from, unsigned len, int xPos, bool firstLine) const
 {
     if (from >= textLength())
@@ -1026 +1026 @@
 }
 
-unsigned int RenderText::width(unsigned int from, unsigned int len, const Font& f, int xPos) const
-{
-    if (!characters() || from > textLength())
+unsigned RenderText::width(unsigned from, unsigned len, const Font& f, int xPos) const
+{
+    ASSERT(from + len <= textLength());
+    if (!characters())
         return 0;
-
-    if (from + len > textLength())
-        len = textLength() - from;
 
     int w;

trunk/WebCore/rendering/bidi.cpp

@@ -1535 +1535 @@
 }
 
+static inline unsigned textWidth(RenderText* text, unsigned from, unsigned len, const Font& font, int xPos, bool isFixedPitch, bool collapseWhiteSpace)
+{
+    if (isFixedPitch || !from && len == text->textLength())
+        return text->width(from, len, font, xPos);
+    return font.width(TextRun(text->characters() + from, len, !collapseWhiteSpace, xPos));
+}
+
 InlineIterator RenderBlock::findNextLineBreak(InlineBidiResolver& resolver, bool firstLine, EClear* clear)
 {
@@ -1743 +1750 @@
 
             const Font& f = t->style(firstLine)->font();
+            bool isFixedPitch = f.isFixedPitch();
 
             int lastSpace = pos;
@@ -1791 +1799 @@
 
                         // Add the width up to but not including the hyphen.
-                        tmpW += t->width(lastSpace, pos - lastSpace, f, w + tmpW) + lastSpaceWordSpacing;
+                        tmpW += textWidth(t, lastSpace, pos - lastSpace, f, w + tmpW, isFixedPitch, collapseWhiteSpace) + lastSpaceWordSpacing;
 
                         // For wrapping text only, include the hyphen.  We need to ensure it will fit
                         // on the line if it shows when we break.
                         if (autoWrap)
-                            tmpW += t->width(pos, 1, f, w + tmpW);
+                            tmpW += textWidth(t, pos, 1, f, w + tmpW, isFixedPitch, collapseWhiteSpace);
 
                         InlineIterator afterSoftHyphen(0, o, pos);
@@ -1816 +1824 @@
                 if ((breakAll || breakWords) && !midWordBreak) {
                     wrapW += charWidth;
-                    charWidth = t->width(pos, 1, f, w + wrapW);
+                    charWidth = textWidth(t, pos, 1, f, w + wrapW, isFixedPitch, collapseWhiteSpace);
                     midWordBreak = w + wrapW + charWidth > width;
                 }
@@ -1841 +1849 @@
                     }
 
-                    int additionalTmpW = t->width(lastSpace, pos - lastSpace, f, w+tmpW) + lastSpaceWordSpacing;
+                    int additionalTmpW = textWidth(t, lastSpace, pos - lastSpace, f, w + tmpW, isFixedPitch, collapseWhiteSpace) + lastSpaceWordSpacing;
                     tmpW += additionalTmpW;
                     if (!appliedStartWidth) {
@@ -1858 +1866 @@
                         bool lineWasTooWide = false;
                         if (w + tmpW <= width && currentCharacterIsWS && o->style()->breakOnlyAfterWhiteSpace() && !midWordBreak) {
-                            int charWidth = t->width(pos, 1, f, w + tmpW) + (applyWordSpacing ? wordSpacing : 0);
+                            int charWidth = textWidth(t, pos, 1, f, w + tmpW, isFixedPitch, collapseWhiteSpace) + (applyWordSpacing ? wordSpacing : 0);
                             // Check if line is too big even without the extra space
                             // at the end of the line. If it is not, do nothing. 
@@ -1888 +1896 @@
                             if (pos > 0 && str[pos-1] == softHyphen)
                                 // Subtract the width of the soft hyphen out since we fit on a line.
-                                tmpW -= t->width(pos-1, 1, f, w+tmpW);
+                                tmpW -= textWidth(t, pos - 1, 1, f, w + tmpW, isFixedPitch, collapseWhiteSpace);
                         }
                     }
@@ -1979 +1987 @@
             // IMPORTANT: pos is > length here!
             if (!ignoringSpaces)
-                tmpW += t->width(lastSpace, pos - lastSpace, f, w+tmpW) + lastSpaceWordSpacing;
+                tmpW += textWidth(t, lastSpace, pos - lastSpace, f, w + tmpW, isFixedPitch, collapseWhiteSpace) + lastSpaceWordSpacing;
             tmpW += inlineWidth(o, !appliedStartWidth, true);
         } else