[Webkit-unassigned] [Bug 20542] Adding EOT Font Rendering capability along the 'GDI olny' path

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 27 06:20:08 PDT 2008


eric at webkit.org changed:

           What    |Removed                     |Added
  Attachment #23022|review?                     |review-
               Flag|                            |

------- Comment #2 from eric at webkit.org  2008-08-27 06:20 PDT -------
(From update of attachment 23022)
It seems there are missing files?  Can this little amount of code really
support EOT fonts?

This will not compile in release mode:
+    if(!m_isEOT || renderingMode != AlternateRenderingMode)
+        ASSERT(m_cgFont);

ASSERT disappears in release mode.

Also, be sure to read:

There are lots of little style violations in this code.  A few which come to

// comment
( generally we have a space after // before the comment)

if ( instead of if(

We don't use else after a return statement:
+    if(!m_isEOT)
+        return FontPlatformData(hfont, m_cgFont, size, bold, italic,
renderingMode == AlternateRenderingMode);
+    else
+        return FontPlatformData(hfont, size, bold, italic, renderingMode ==

{ goes on the next line after a function definition:

+void EOTStream::setInHeader(bool inHeader) {
+    m_inHeader = inHeader;

Again, won't compile in Release mode (afaik):

+            if(!m_useGDI)
+                ASSERT(m_cgFont);

+bool isEOTFont(SharedBuffer* fontData) 

should have some minimum length check.  Otherwise it will crash if passed data
shorter than sizeof(EOTPrefix)

According to our style guidelines, we don't use c-style casts in C++, this
should be a reinterpret_cast<EOTPrefix*>:

+    EOTPrefix* prefix = (EOTPrefix*)(data);

No inner { } here:
+            if (localGlyphBuffer[i] == invalid_glyph) {
+                localGlyphBuffer[i] = 0;
+            }

r- for all the style violations.  I've CC'd hyatt and hixie, who are the two
people who you would need to convince that EOT support is a good thing for
WebKit before we could accept such a patch.

Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

More information about the webkit-unassigned mailing list