Changeset 131410 in webkit

Timestamp:

Oct 15, 2012 10:07:56 PM (12 years ago)

Author:

mitz@apple.com

Message:

REGRESSION (r131365): WidthIterator::advance() is needlessly passed a GlyphBuffer in many cases
​https://bugs.webkit.org/show_bug.cgi?id=99413

Reviewed by Adele Peterson.

r131365 made Font::width() always pass a GlyphBuffer pointer to floatWidthForSimpleText and
from there down to WidthIterator::advance(). However, when measuring the width of a run, a
GlyphBuffer is only needed if font transforms (kerning and ligatures) need to be applied.

No new test, because there is no change in behavior.

• platform/graphics/Font.cpp:

(WebCore::Font::width): Removed the local GlyphBuffer that was passed down to
floatWidthForSimpleText().

• platform/graphics/Font.h: Removed the GlyphBuffer* parameter to floatWidthForSimpleText.

All but the above caller were passing 0.

• platform/graphics/FontFastPath.cpp:

(WebCore::Font::floatWidthForSimpleText): Removed the GlyphBuffer* parameter and added a
local GlyphBuffer, which is passed by reference to WidthIterator::advance() only if
typesetting features require it.
(WebCore::Font::offsetForPositionForSimpleText): Updated for change to
floatWidthForSimpleText.

• platform/graphics/pango/FontPango.cpp:

(WebCore::Font::floatWidthForComplexText): Ditto.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/Font.cpp (modified) (2 diffs)
• platform/graphics/Font.h (modified) (1 diff)
• platform/graphics/FontFastPath.cpp (modified) (2 diffs)
• platform/graphics/pango/FontPango.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-10-15  Dan Bernstein  <mitz@apple.com>
+
+        REGRESSION (r131365): WidthIterator::advance() is needlessly passed a GlyphBuffer in many cases
+        https://bugs.webkit.org/show_bug.cgi?id=99413
+
+        Reviewed by Adele Peterson.
+
+        r131365 made Font::width() always pass a GlyphBuffer pointer to floatWidthForSimpleText and
+        from there down to WidthIterator::advance(). However, when measuring the width of a run, a
+        GlyphBuffer is only needed if font transforms (kerning and ligatures) need to be applied.
+
+        No new test, because there is no change in behavior.
+
+        * platform/graphics/Font.cpp:
+        (WebCore::Font::width): Removed the local GlyphBuffer that was passed down to
+        floatWidthForSimpleText().
+        * platform/graphics/Font.h: Removed the GlyphBuffer* parameter to floatWidthForSimpleText.
+        All but the above caller were passing 0.
+        * platform/graphics/FontFastPath.cpp:
+        (WebCore::Font::floatWidthForSimpleText): Removed the GlyphBuffer* parameter and added a
+        local GlyphBuffer, which is passed by reference to WidthIterator::advance() only if
+        typesetting features require it.
+        (WebCore::Font::offsetForPositionForSimpleText): Updated for change to
+        floatWidthForSimpleText.
+        * platform/graphics/pango/FontPango.cpp:
+        (WebCore::Font::floatWidthForComplexText): Ditto.
+
 2012-10-15  Pablo Flouret  <pablof@motorola.com>
 

trunk/Source/WebCore/platform/graphics/Font.cpp

@@ -188 +188 @@
         // returning them for simple text as well.
         static bool returnFallbackFonts = canReturnFallbackFontsForComplexText();
-        GlyphBuffer glyphBuffer;
-        return floatWidthForSimpleText(run, &glyphBuffer, returnFallbackFonts ? fallbackFonts : 0, codePathToUse == SimpleWithGlyphOverflow || (glyphOverflow && glyphOverflow->computeBounds) ? glyphOverflow : 0);
+        return floatWidthForSimpleText(run, returnFallbackFonts ? fallbackFonts : 0, codePathToUse == SimpleWithGlyphOverflow || (glyphOverflow && glyphOverflow->computeBounds) ? glyphOverflow : 0);
     }
 
@@ -206 +205 @@
 
     if (codePath(run) != Complex)
-        return floatWidthForSimpleText(run, 0);
+        return floatWidthForSimpleText(run);
 
     return floatWidthForComplexText(run);

trunk/Source/WebCore/platform/graphics/Font.h

@@ -224 +224 @@
     void drawGlyphBuffer(GraphicsContext*, const TextRun&, const GlyphBuffer&, const FloatPoint&) const;
     void drawEmphasisMarks(GraphicsContext*, const TextRun&, const GlyphBuffer&, const AtomicString&, const FloatPoint&) const;
-    float floatWidthForSimpleText(const TextRun&, GlyphBuffer*, HashSet<const SimpleFontData*>* fallbackFonts = 0, GlyphOverflow* = 0) const;
+    float floatWidthForSimpleText(const TextRun&, HashSet<const SimpleFontData*>* fallbackFonts = 0, GlyphOverflow* = 0) const;
     int offsetForPositionForSimpleText(const TextRun&, float position, bool includePartialGlyphs) const;
     FloatRect selectionRectForSimpleText(const TextRun&, const FloatPoint&, int h, int from, int to) const;

trunk/Source/WebCore/platform/graphics/FontFastPath.cpp

@@ -470 +470 @@
 }
 
-float Font::floatWidthForSimpleText(const TextRun& run, GlyphBuffer* glyphBuffer, HashSet<const SimpleFontData*>* fallbackFonts, GlyphOverflow* glyphOverflow) const
+float Font::floatWidthForSimpleText(const TextRun& run, HashSet<const SimpleFontData*>* fallbackFonts, GlyphOverflow* glyphOverflow) const
 {
     WidthIterator it(this, run, fallbackFonts, glyphOverflow);
-    it.advance(run.length(), glyphBuffer);
+    GlyphBuffer glyphBuffer;
+    it.advance(run.length(), (typesettingFeatures() & (Kerning | Ligatures)) ? &glyphBuffer : 0);
 
     if (glyphOverflow) {
@@ -512 +513 @@
     unsigned offset;
     if (run.rtl()) {
-        delta -= floatWidthForSimpleText(run, 0);
+        delta -= floatWidthForSimpleText(run);
         while (1) {
             offset = it.m_currentCharacter;

trunk/Source/WebCore/platform/graphics/pango/FontPango.cpp

@@ -372 +372 @@
 #if USE(FREETYPE)
     if (!primaryFont()->platformData().m_pattern)
-        return floatWidthForSimpleText(run, 0, fallbackFonts, overflow);
+        return floatWidthForSimpleText(run, fallbackFonts, overflow);
 #endif