Attachment 23200 Details for Bug 20237 – Updated: ZWS fix only

[patch] Updated: ZWS fix only

20237-3.patch (text/plain), 8.27 KB, created by Dimitri Glazkov (Google) on 2008-09-05 14:18:46 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Dimitri Glazkov (Google)

Created: 2008-09-05 14:18:46 PDT

Size: 8.27 KB

patch

obsolete

>Index: WebCore/ChangeLog
>===================================================================
>--- WebCore/ChangeLog	(revision 36115)
>+++ WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,29 @@
>+2008-09-05  Dimitri Glazkov  <dglazkov@chromium.org>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        WARNING: NO TEST CASES ADDED OR CHANGED
>+       
>+        Fixes a layout test (fast/text/zero-width-characters.html) and addresses
>+        mitz comments in response to original patch for
>+        https://bugs.webkit.org/show_bug.cgi?id=20237
>+
>+        This change brings handling of the ZWS character width down to
>+        the level of SimpleFontData by creating special (sub-classed)
>+        SimpleFontData objects that are used in GlyphData. These instances are
>+        created when the glyph cache is being filled (GlyphPage::fill).
>+
>+        * platform/graphics/Font.cpp:
>+        * platform/graphics/Font.h:
>+        * platform/graphics/SimpleFontData.cpp:
>+        (WebCore::SimpleFontData::SimpleFontData):
>+        (WebCore::SimpleFontData::zeroWidthFontData):
>+        (WebCore::ZeroWidthFontData::init):
>+        * platform/graphics/SimpleFontData.h:
>+        (WebCore::ZeroWidthFontData::widthForGlyph):
>+        * platform/graphics/win/GlyphPageTreeNodeCGWin.cpp:
>+        (WebCore::GlyphPage::fill):
>+
> 2008-09-05  Antti Koivisto  <antti@apple.com>
> 
>         Qt build fix.
>Index: WebCore/platform/graphics/SimpleFontData.cpp
>===================================================================
>--- WebCore/platform/graphics/SimpleFontData.cpp	(revision 36115)
>+++ WebCore/platform/graphics/SimpleFontData.cpp	(working copy)
>@@ -49,6 +49,7 @@ SimpleFontData::SimpleFontData(const Fon
>     , m_isCustomFont(customFont)
>     , m_isLoading(loading)
>     , m_smallCapsFontData(0)
>+    , m_zeroWidthFontData(new ZeroWidthFontData())
> {
> #if ENABLE(SVG_FONTS) && !PLATFORM(QT)
>     if (SVGFontFaceElement* svgFontFaceElement = svgFontData ? svgFontData->svgFontFaceElement() : 0) {
>@@ -70,6 +71,7 @@ SimpleFontData::SimpleFontData(const Fon
>         determinePitch();
>         m_missingGlyphData.fontData = this;
>         m_missingGlyphData.glyph = 0;
>+        m_zeroWidthFontData->init(this);
>         return;
>     }
> #endif
>@@ -85,6 +87,7 @@ SimpleFontData::SimpleFontData(const Fon
>         determinePitch();
>         m_missingGlyphData.fontData = this;
>         m_missingGlyphData.glyph = 0;
>+        m_zeroWidthFontData->init(this);
>         return;
>     }
> 
>@@ -97,6 +100,13 @@ SimpleFontData::SimpleFontData(const Fon
>     determinePitch();
>     m_adjustedSpaceWidth = m_treatAsFixedPitch ? ceilf(width) : roundf(width);
> 
>+    // FIXME: Investigate and implement across platforms, if needed
>+#if PLATFORM(WIN)
>+    // ZERO WIDTH SPACES are explicitly mapped to share the glyph
>+    // with SPACE (with width adjusted to 0) during GlyphPage::fill
>+    // This is currently only implemented for Windows port. The FontData
>+    // remapping may very well be needed for other platforms.
>+#else
>     // Force the glyph for ZERO WIDTH SPACE to have zero width, unless it is shared with SPACE.
>     // Helvetica is an example of a non-zero width ZERO WIDTH SPACE glyph.
>     // See <http://bugs.webkit.org/show_bug.cgi?id=13178>
>@@ -109,9 +119,22 @@ SimpleFontData::SimpleFontData(const Fon
>         else
>             LOG_ERROR("Font maps SPACE and ZERO WIDTH SPACE to the same glyph. Glyph width not overridden.");
>     }
>+#endif
> 
>     m_missingGlyphData.fontData = this;
>     m_missingGlyphData.glyph = 0;
>+    m_zeroWidthFontData->init(this);
>+}
>+
>+SimpleFontData::SimpleFontData()
>+    : m_treatAsFixedPitch(false)
>+#if ENABLE(SVG_FONTS)
>+    , m_svgFontData(0)
>+#endif
>+    , m_isCustomFont(0)
>+    , m_isLoading(0)
>+    , m_smallCapsFontData(0)
>+{
> }
> 
> SimpleFontData::~SimpleFontData()
>@@ -150,4 +173,30 @@ bool SimpleFontData::isSegmented() const
>     return false;
> }
> 
>+const SimpleFontData* SimpleFontData::zeroWidthFontData() const
>+{
>+    return m_zeroWidthFontData.get();
>+}
>+
>+void ZeroWidthFontData::init(SimpleFontData* fontData)
>+{
>+    m_font = fontData->m_font;
>+    m_smallCapsFontData = fontData->m_smallCapsFontData;
>+    m_ascent = fontData->m_ascent;
>+    m_descent = fontData->m_descent;
>+    m_lineSpacing = fontData->m_lineSpacing;
>+    m_lineGap = fontData->m_lineGap;
>+    m_xHeight = fontData->m_xHeight;
>+    m_unitsPerEm = fontData->m_unitsPerEm;
>+    m_spaceWidth = 0;
>+    m_spaceGlyph = 0;
>+    m_adjustedSpaceWidth = fontData->m_adjustedSpaceWidth;
>+#if PLATFORM(WIN)
>+    m_scriptCache = 0;
>+    m_scriptFontProperties = 0;
>+    m_isSystemFont = false;
>+    m_syntheticBoldOffset = 0.0f;
>+#endif
>+}
>+
> } // namespace WebCore
>Index: WebCore/platform/graphics/SimpleFontData.h
>===================================================================
>--- WebCore/platform/graphics/SimpleFontData.h	(revision 36115)
>+++ WebCore/platform/graphics/SimpleFontData.h	(working copy)
>@@ -48,6 +48,7 @@ class FontPlatformData;
> class SharedBuffer;
> class SVGFontData;
> class WidthMap;
>+class ZeroWidthFontData;
> 
> enum Pitch { UnknownPitch, FixedPitch, VariablePitch };
> 
>@@ -56,6 +57,10 @@ public:
>     SimpleFontData(const FontPlatformData&, bool customFont = false, bool loading = false, SVGFontData* data = 0);
>     virtual ~SimpleFontData();
> 
>+protected:
>+    // sub-class constructor
>+    SimpleFontData();
>+
> public:
>     const FontPlatformData& platformData() const { return m_font; }
>     SimpleFontData* smallCapsFontData(const FontDescription& fontDescription) const;
>@@ -68,7 +73,7 @@ public:
>     float xHeight() const { return m_xHeight; }
>     unsigned unitsPerEm() const { return m_unitsPerEm; }
> 
>-    float widthForGlyph(Glyph) const;
>+    virtual float widthForGlyph(Glyph) const;
>     float platformWidthForGlyph(Glyph) const;
> 
>     virtual const SimpleFontData* fontDataForCharacter(UChar32) const;
>@@ -118,6 +123,8 @@ public:
>     wxFont getWxFont() const { return m_font.font(); }
> #endif
> 
>+    const SimpleFontData* zeroWidthFontData() const;
>+
> private:
>     void platformInit();
>     void platformDestroy();
>@@ -177,6 +184,18 @@ public:
>     mutable SCRIPT_CACHE m_scriptCache;
>     mutable SCRIPT_FONTPROPERTIES* m_scriptFontProperties;
> #endif
>+
>+private:
>+    OwnPtr<ZeroWidthFontData> m_zeroWidthFontData;
>+};
>+
>+// SimpleFontData sub-classes:
>+
>+// Has a single zero-width glyph, used for ZWS characters
>+class ZeroWidthFontData : public SimpleFontData {
>+public:
>+    void init(SimpleFontData*);
>+    virtual float widthForGlyph(Glyph) const { return 0.0f; }
> };
> 
> } // namespace WebCore
>Index: WebCore/platform/graphics/win/GlyphPageTreeNodeCGWin.cpp
>===================================================================
>--- WebCore/platform/graphics/win/GlyphPageTreeNodeCGWin.cpp	(revision 36115)
>+++ WebCore/platform/graphics/win/GlyphPageTreeNodeCGWin.cpp	(working copy)
>@@ -29,6 +29,8 @@
> #include "config.h"
> #include "GlyphPageTreeNode.h"
> 
>+#include "CharacterNames.h"
>+#include "Font.h"
> #include "SimpleFontData.h"
> #include <WebKitSystemInterface/WebKitSystemInterface.h>
> 
>@@ -42,16 +44,31 @@ bool GlyphPage::fill(unsigned offset, un
>         return false;
> 
>     bool haveGlyphs = false;
>+    CGGlyph spaceGlyph = 0;
>     CGGlyph localGlyphBuffer[GlyphPage::size];
>     wkGetGlyphs(fontData->platformData().cgFont(), buffer, localGlyphBuffer, bufferLength);
>     for (unsigned i = 0; i < length; i++) {
>-        Glyph glyph = localGlyphBuffer[i];
>-        if (!glyph)
>-            setGlyphDataForIndex(offset + i, 0, 0);
>-        else {
>-            setGlyphDataForIndex(offset + i, glyph, fontData);
>+        CGGlyph glyph = localGlyphBuffer[i];
>+        UChar c = buffer[i];
>+        const SimpleFontData* glyphFontData = fontData;
>+        if (c == zeroWidthSpace) {
>+            if (!glyph) {
>+                static UChar space = ' ';
>+                if (!spaceGlyph)
>+                    wkGetGlyphs(fontData->platformData().cgFont(), &space,
>+                                &spaceGlyph, 1);
>+                glyph = spaceGlyph;
>+            }
>+            glyphFontData = fontData->zeroWidthFontData();
>+            haveGlyphs = true;
>+        } else if (!glyph) {
>+            // WebKit expects both the glyph index and FontData
>+            // pointer to be NULL if the glyph is not present
>+            glyphFontData = 0;
>+        } else {
>             haveGlyphs = true;
>         }
>+        setGlyphDataForIndex(offset + i, glyph, glyphFontData);
>     }
>     return haveGlyphs;
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 20237: 22570 | 22591 | 23174 | 23200 | 23276