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

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=20542


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:
http://webkit.org/coding/coding-style.html

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

// 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 ==
AlternateRenderingMode);   

{ 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.