Changeset 26696 in webkit

Timestamp:

Oct 16, 2007, 9:51:50 PM (17 years ago)

Author:

darin

Message:

Reviewed by Mitz.

• fix ​http://bugs.webkit.org/show_bug.cgi?id=15536 need to cache missing glyph so we're not slow on pages that show missing glyphs

• <rdar://problem/5404359> UI thread stall (>60sec) in MLANG running stress test (related to font data caching)

• platform/Font.cpp: (WebCore::Font::glyphDataForCharacter): Check for a null value for fontData rather than for glyph to detect non-cached entries in the glyph data.

• platform/gtk/GlyphPageTreeNodeGtk.cpp: (WebCore::GlyphPage::fill):
• platform/mac/GlyphPageTreeNodeMac.cpp: (WebCore::GlyphPage::fill):
• platform/win/GlyphPageTreeNodeWin.cpp: (WebCore::GlyphPage::fill): Set fontData to 0 for missing glyph entries. Also fixed the Windows and GTK versions to return the proper value for haveGlyphs.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/Font.cpp (modified) (3 diffs)
• platform/gtk/GlyphPageTreeNodeGtk.cpp (modified) (2 diffs)
• platform/mac/GlyphPageTreeNodeMac.cpp (modified) (2 diffs)
• platform/win/GlyphPageTreeNodeWin.cpp (modified) (2 diffs)

trunk/WebCore/ChangeLog ¶

@@ +1 @@
+2007-10-16  Darin Adler  <darin@apple.com>
+
+        Reviewed by Mitz.
+
+        - fix http://bugs.webkit.org/show_bug.cgi?id=15536
+          need to cache missing glyph so we're not slow on pages that show missing glyphs
+
+        - <rdar://problem/5404359> UI thread stall (>60sec) in MLANG running stress test
+          (related to font data caching)
+
+        * platform/Font.cpp: (WebCore::Font::glyphDataForCharacter): Check for a null value
+        for fontData rather than for glyph to detect non-cached entries in the glyph data.
+
+        * platform/gtk/GlyphPageTreeNodeGtk.cpp: (WebCore::GlyphPage::fill):
+        * platform/mac/GlyphPageTreeNodeMac.cpp: (WebCore::GlyphPage::fill):
+        * platform/win/GlyphPageTreeNodeWin.cpp: (WebCore::GlyphPage::fill):
+        Set fontData to 0 for missing glyph entries. Also fixed the Windows and GTK versions
+        to return the proper value for haveGlyphs.
+
 2007-10-16  Mark Rowe  <mrowe@apple.com>
 

trunk/WebCore/platform/Font.cpp ¶

@@ -386 +386 @@
             if (page) {
                 const GlyphData& data = page->glyphDataForCharacter(c);
-                if (data.glyph)
+                if (data.fontData)
                     return data;
                 if (node->isSystemFallback())
@@ -404 +404 @@
             if (page) {
                 const GlyphData& data = page->glyphDataForCharacter(c);
-                if (data.glyph) {
+                if (data.fontData) {
                     // The smallCapsFontData function should not normally return 0.
                     // But if it does, we will just render the capital letter big.
@@ -413 +413 @@
                     GlyphPageTreeNode* smallCapsNode = GlyphPageTreeNode::getRootChild(smallCapsFontData, pageNumber);
                     const GlyphData& data = smallCapsNode->page()->glyphDataForCharacter(c);
-                    if (data.glyph)
+                    if (data.fontData)
                         return data;
 

trunk/WebCore/platform/gtk/GlyphPageTreeNodeGtk.cpp ¶

@@ -1 +1 @@
 /*
- * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
+ * Copyright (C) 2006, 2007 Apple Inc. All rights reserved.
  * Copyright (C) 2006 Michael Emmel mike.emmel@gmail.com 
  * Copyright (C) 2007 Alp Toker <alp.toker@collabora.co.uk>
- * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -40 +39 @@
 {
     FT_Face face = cairo_ft_scaled_font_lock_face(fontData->m_font.m_scaledFont);
-
     if (!face)
         return false;
 
-    for (unsigned i = 0; i < bufferLength; i++)
-        setGlyphDataForIndex(i, FcFreeTypeCharIndex(face, buffer[i]), fontData);
+    bool haveGlyphs = false;
+    for (unsigned i = 0; i < bufferLength; i++) {
+        Glyph glyph = FcFreeTypeCharIndex(face, buffer[i]);
+        if (!glyph)
+            setGlyphDataForIndex(i, 0, 0);
+        else {
+            setGlyphDataForIndex(i, glyph, fontData);
+            haveGlyphs = true;
+        }
+    }
 
     cairo_ft_scaled_font_unlock_face(fontData->m_font.m_scaledFont);
 
-    return true;
+    return haveGlyphs;
 }
 

trunk/WebCore/platform/mac/GlyphPageTreeNodeMac.cpp ¶

@@ -1 +1 @@
 /*
- * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
+ * Copyright (C) 2006, 2007 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -61 +61 @@
     for (unsigned i = 0; i < GlyphPage::size; i++) {
         Glyph glyph = glyphRecord->glyphID;
-        setGlyphDataForIndex(i, glyph, fontData);
-        if (!haveGlyphs && glyph)
+        if (!glyph)
+            setGlyphDataForIndex(i, 0, 0);
+        else {
+            setGlyphDataForIndex(i, glyph, fontData);
             haveGlyphs = true;
+        }
         glyphRecord = (ATSLayoutRecord *)((char *)glyphRecord + wkGetGlyphVectorRecordSize(glyphVector));
     }

trunk/WebCore/platform/win/GlyphPageTreeNodeWin.cpp ¶

@@ -1 +1 @@
 /*
- * Copyright (C) 2006, 2007 Apple Inc.  All rights reserved.
+ * Copyright (C) 2006, 2007 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -29 +29 @@
 #include "config.h"
 #include "FontData.h"
+
 #include <WebKitSystemInterface/WebKitSystemInterface.h>
 
-namespace WebCore
-{
+namespace WebCore {
 
 bool GlyphPage::fill(UChar* buffer, unsigned bufferLength, const FontData* fontData)
 {
-    // bufferLength will be greater than the glyph page size if the buffer has Unicode supplementary characters.
+    // The bufferLength will be greater than the glyph page size if the buffer has Unicode supplementary characters.
     // We won't support this for now.
     if (bufferLength > GlyphPage::size)
         return false;
 
+    bool haveGlyphs = false;
     CGGlyph localGlyphBuffer[GlyphPage::size];
     wkGetGlyphs(fontData->platformData().cgFont(), buffer, localGlyphBuffer, bufferLength);
-    for (unsigned i = 0; i < GlyphPage::size; i++)
-        setGlyphDataForIndex(i, localGlyphBuffer[i], fontData);
-    return true;
+    for (unsigned i = 0; i < GlyphPage::size; i++) {
+        Glyph glyph = localGlyphBuffer[i];
+        if (!glyph)
+            setGlyphDataForIndex(i, 0, 0);
+        else {
+            setGlyphDataForIndex(i, glyph, fontData);
+            haveGlyphs = true;
+        }
+    }
+    return haveGlyphs;
 }
 
 }
-