Title: [141976] trunk/Source/WebCore
Revision
141976
Author
commit-qu...@webkit.org
Date
2013-02-06 00:27:21 -0800 (Wed, 06 Feb 2013)

Log Message

Unreviewed, rolling out r141961.
http://trac.webkit.org/changeset/141961
https://bugs.webkit.org/show_bug.cgi?id=109019

assertion failures on svn tests such as fonts-glyph-04-t.svg
(Requested by falken on #webkit).

Patch by Sheriff Bot <webkit.review....@gmail.com> on 2013-02-06

* platform/graphics/GlyphPage.h:
(WebCore::GlyphPage::create):
(WebCore::GlyphPage::glyphDataForCharacter):
(WebCore::GlyphPage::glyphDataForIndex):
(WebCore::GlyphPage::fontDataForCharacter):
(WebCore::GlyphPage::setGlyphDataForIndex):
(GlyphPage):
(WebCore::GlyphPage::copyFrom):
(WebCore::GlyphPage::clear):
(WebCore::GlyphPage::clearForFontData):
(WebCore::GlyphPage::GlyphPage):
* platform/graphics/GlyphPageTreeNode.cpp:
(WebCore::GlyphPageTreeNode::initializePage):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (141975 => 141976)


--- trunk/Source/WebCore/ChangeLog	2013-02-06 08:23:57 UTC (rev 141975)
+++ trunk/Source/WebCore/ChangeLog	2013-02-06 08:27:21 UTC (rev 141976)
@@ -1,3 +1,26 @@
+2013-02-06  Sheriff Bot  <webkit.review....@gmail.com>
+
+        Unreviewed, rolling out r141961.
+        http://trac.webkit.org/changeset/141961
+        https://bugs.webkit.org/show_bug.cgi?id=109019
+
+        assertion failures on svn tests such as fonts-glyph-04-t.svg
+        (Requested by falken on #webkit).
+
+        * platform/graphics/GlyphPage.h:
+        (WebCore::GlyphPage::create):
+        (WebCore::GlyphPage::glyphDataForCharacter):
+        (WebCore::GlyphPage::glyphDataForIndex):
+        (WebCore::GlyphPage::fontDataForCharacter):
+        (WebCore::GlyphPage::setGlyphDataForIndex):
+        (GlyphPage):
+        (WebCore::GlyphPage::copyFrom):
+        (WebCore::GlyphPage::clear):
+        (WebCore::GlyphPage::clearForFontData):
+        (WebCore::GlyphPage::GlyphPage):
+        * platform/graphics/GlyphPageTreeNode.cpp:
+        (WebCore::GlyphPageTreeNode::initializePage):
+
 2013-02-05  Sheriff Bot  <webkit.review....@gmail.com>
 
         Unreviewed, rolling out r141964.

Modified: trunk/Source/WebCore/platform/graphics/GlyphPage.h (141975 => 141976)


--- trunk/Source/WebCore/platform/graphics/GlyphPage.h	2013-02-06 08:23:57 UTC (rev 141975)
+++ trunk/Source/WebCore/platform/graphics/GlyphPage.h	2013-02-06 08:27:21 UTC (rev 141976)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006, 2007, 2008, 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2006, 2007, 2008 Apple Inc. All rights reserved.
  * Copyright (C) Research In Motion Limited 2011. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -33,7 +33,6 @@
 #include "Glyph.h"
 #include <wtf/PassRefPtr.h>
 #include <wtf/RefCounted.h>
-#include <wtf/RefPtr.h>
 #include <wtf/unicode/Unicode.h>
 
 namespace WebCore {
@@ -63,51 +62,24 @@
 // to be overriding the parent's node, but provide no additional information.
 class GlyphPage : public RefCounted<GlyphPage> {
 public:
-    static PassRefPtr<GlyphPage> createUninitialized(GlyphPageTreeNode* owner)
+    static PassRefPtr<GlyphPage> create(GlyphPageTreeNode* owner)
     {
-        return adoptRef(new GlyphPage(owner, false));
+        return adoptRef(new GlyphPage(owner));
     }
 
-    static PassRefPtr<GlyphPage> createZeroedSystemFallbackPage(GlyphPageTreeNode* owner)
-    {
-        return adoptRef(new GlyphPage(owner, true));
-    }
-
-    PassRefPtr<GlyphPage> createCopiedSystemFallbackPage(GlyphPageTreeNode* owner) const
-    {
-        RefPtr<GlyphPage> page = GlyphPage::createUninitialized(owner);
-        memcpy(page->m_glyphs, m_glyphs, sizeof(m_glyphs));
-        page->m_fontDataForAllGlyphs = m_fontDataForAllGlyphs;
-        if (m_perGlyphFontData) {
-            page->m_perGlyphFontData = static_cast<const SimpleFontData**>(fastMalloc(size * sizeof(SimpleFontData*)));
-            memcpy(page->m_perGlyphFontData, m_perGlyphFontData, size * sizeof(SimpleFontData*));
-        }
-        return page.release();
-    }
-
-    ~GlyphPage()
-    {
-        if (m_perGlyphFontData)
-            fastFree(m_perGlyphFontData);
-    }
-
     static const size_t size = 256; // Covers Latin-1 in a single page.
 
     unsigned indexForCharacter(UChar32 c) const { return c % size; }
     GlyphData glyphDataForCharacter(UChar32 c) const
     {
-        return glyphDataForIndex(indexForCharacter(c));
+        unsigned index = indexForCharacter(c);
+        return GlyphData(m_glyphs[index], m_glyphFontData[index]);
     }
 
     GlyphData glyphDataForIndex(unsigned index) const
     {
         ASSERT_WITH_SECURITY_IMPLICATION(index < size);
-        Glyph glyph = m_glyphs[index];
-        if (UNLIKELY(!glyph))
-            return GlyphData(0, 0);
-        if (UNLIKELY(!!m_perGlyphFontData))
-            return GlyphData(glyph, m_perGlyphFontData[index]);
-        return GlyphData(glyph, m_fontDataForAllGlyphs);
+        return GlyphData(m_glyphs[index], m_glyphFontData[index]);
     }
 
     Glyph glyphAt(unsigned index) const
@@ -118,7 +90,7 @@
 
     const SimpleFontData* fontDataForCharacter(UChar32 c) const
     {
-        return glyphDataForIndex(indexForCharacter(c)).fontData;
+        return m_glyphFontData[indexForCharacter(c)];
     }
 
     void setGlyphDataForCharacter(UChar32 c, Glyph g, const SimpleFontData* f)
@@ -126,55 +98,36 @@
         setGlyphDataForIndex(indexForCharacter(c), g, f);
     }
 
-    void setGlyphDataForIndex(unsigned index, Glyph glyph, const SimpleFontData* fontData)
+    void setGlyphDataForIndex(unsigned index, Glyph g, const SimpleFontData* f)
     {
         ASSERT_WITH_SECURITY_IMPLICATION(index < size);
-        m_glyphs[index] = glyph;
-
-        // GlyphPage getters will always return a null SimpleFontData* for glyph #0, so don't worry about the pointer for them.
-        if (!glyph)
-            return;
-
-        // A glyph index without a font data pointer makes no sense.
-        ASSERT(fontData);
-
-        if (m_perGlyphFontData) {
-            m_perGlyphFontData[index] = fontData;
-            return;
-        }
-
-        if (!m_fontDataForAllGlyphs)
-            m_fontDataForAllGlyphs = fontData;
-
-        if (m_fontDataForAllGlyphs == fontData)
-            return;
-
-        // This GlyphPage houses glyphs from multiple fonts, transition to an array of SimpleFontData pointers.
-        const SimpleFontData* oldFontData = m_fontDataForAllGlyphs;
-        m_perGlyphFontData = static_cast<const SimpleFontData**>(fastMalloc(size * sizeof(SimpleFontData*)));
-        for (unsigned i = 0; i < size; ++i)
-            m_perGlyphFontData[i] = oldFontData;
-        m_perGlyphFontData[index] = fontData;
+        m_glyphs[index] = g;
+        m_glyphFontData[index] = f;
     }
 
     void setGlyphDataForIndex(unsigned index, const GlyphData& glyphData)
     {
         setGlyphDataForIndex(index, glyphData.glyph, glyphData.fontData);
     }
+    
+    void copyFrom(const GlyphPage& other)
+    {
+        memcpy(m_glyphs, other.m_glyphs, sizeof(m_glyphs));
+        memcpy(m_glyphFontData, other.m_glyphFontData, sizeof(m_glyphFontData));
+    }
 
+    void clear()
+    {
+        memset(m_glyphs, 0, sizeof(m_glyphs));
+        memset(m_glyphFontData, 0, sizeof(m_glyphFontData));
+    }
+
     void clearForFontData(const SimpleFontData* fontData)
     {
-        if (!m_perGlyphFontData) {
-            if (m_fontDataForAllGlyphs == fontData) {
-                memset(m_glyphs, 0, sizeof(m_glyphs));
-                m_fontDataForAllGlyphs = 0;
-            }
-            return;
-        }
         for (size_t i = 0; i < size; ++i) {
-            if (m_perGlyphFontData[i] == fontData) {
+            if (m_glyphFontData[i] == fontData) {
                 m_glyphs[i] = 0;
-                m_perGlyphFontData[i] = 0;
+                m_glyphFontData[i] = 0;
             }
         }
     }
@@ -185,22 +138,18 @@
     bool fill(unsigned offset, unsigned length, UChar* characterBuffer, unsigned bufferLength, const SimpleFontData*);
 
 private:
-    GlyphPage(GlyphPageTreeNode* owner, bool clearGlyphs)
-        : m_fontDataForAllGlyphs(0)
-        , m_perGlyphFontData(0)
-        , m_owner(owner)
+    GlyphPage(GlyphPageTreeNode* owner)
+        : m_owner(owner)
     {
-        if (clearGlyphs)
-            memset(m_glyphs, 0, sizeof(m_glyphs));
     }
 
-    const SimpleFontData* m_fontDataForAllGlyphs;
-    const SimpleFontData** m_perGlyphFontData;
+    // Separate arrays, rather than array of GlyphData, to save space.
+    Glyph m_glyphs[size];
+    const SimpleFontData* m_glyphFontData[size];
 
     GlyphPageTreeNode* m_owner;
-    Glyph m_glyphs[size];
 };
 
 } // namespace WebCore
 
-#endif // GlyphPage_h
+#endif // GlyphPageTreeNode_h

Modified: trunk/Source/WebCore/platform/graphics/GlyphPageTreeNode.cpp (141975 => 141976)


--- trunk/Source/WebCore/platform/graphics/GlyphPageTreeNode.cpp	2013-02-06 08:23:57 UTC (rev 141975)
+++ trunk/Source/WebCore/platform/graphics/GlyphPageTreeNode.cpp	2013-02-06 08:27:21 UTC (rev 141976)
@@ -200,9 +200,9 @@
                     buffer[i * 2 + 1] = U16_TRAIL(c);
                 }
             }
+            
+            m_page = GlyphPage::create(this);
 
-            m_page = GlyphPage::createUninitialized(this);
-
             // Now that we have a buffer full of characters, we want to get back an array
             // of glyph indices.  This part involves calling into the platform-specific 
             // routine of our glyph map for actually filling in the page with the glyphs.
@@ -225,7 +225,7 @@
                     int to = 1 + min(static_cast<int>(range.to()) - static_cast<int>(start), static_cast<int>(GlyphPage::size) - 1);
                     if (from < static_cast<int>(GlyphPage::size) && to > 0) {
                         if (haveGlyphs && !scratchPage) {
-                            scratchPage = GlyphPage::createUninitialized(this);
+                            scratchPage = GlyphPage::create(this);
                             pageToFill = scratchPage.get();
                         }
 
@@ -279,7 +279,7 @@
                 m_page = parentPage;
             } else {
                 // Combine the parent's glyphs and ours to form a new more complete page.
-                m_page = GlyphPage::createUninitialized(this);
+                m_page = GlyphPage::create(this);
 
                 // Overlay the parent page on the fallback page. Check if the fallback font
                 // has added anything.
@@ -300,14 +300,15 @@
             }
         }
     } else {
+        m_page = GlyphPage::create(this);
         // System fallback. Initialized with the parent's page here, as individual
         // entries may use different fonts depending on character. If the Font
         // ever finds it needs a glyph out of the system fallback page, it will
         // ask the system for the best font to use and fill that glyph in for us.
         if (parentPage)
-            m_page = parentPage->createCopiedSystemFallbackPage(this);
+            m_page->copyFrom(*parentPage);
         else
-            m_page = GlyphPage::createZeroedSystemFallbackPage(this);
+            m_page->clear();
     }
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to