[Webkit-unassigned] [Bug 15229] [gtk] abstract font management by using pango

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=15229


aroben at apple.com changed:

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




------- Comment #5 from aroben at apple.com  2007-09-21 11:24 PDT -------
(From update of attachment 16340)
+    static PangoFontMap* m_fontMap;
+    static GHashTable  * m_hashTable;

These can just become static globals in FontPlatformDataGdk.cpp.

+    gchar const* families[] = {
+        familyName.domString().utf8().data(),

I'm worried that this will give you a junk pointer. String::utf8 returns a
CString, and CString::data returns a pointer to its internal null-terminated
string, but the temporary CString immediately goes out of scope, which should
free the buffer. I think you'll need to put the CString in a local variable.

+        PangoFontFamily**families = 0;

You're missing a space after the **.

+        int n_families = 0;

We'd normally call this familyCount or something like that.

+        pango_font_map_list_families (m_fontMap, &families, &n_families);
+                g_hash_table_insert (m_hashTable,

Please remove the spaces before the open parenthesis.

+    bool result(pango_font_description_equal(thisDesc, otherDesc));

I think you should just use the assignment operator here.

+static PangoGlyph pango_font_get_glyph (PangoFont* font, PangoContext*
context, gunichar wc)

Please remove the space before the open parenthesis.

+    gint  length = g_unichar_to_utf8(wc, buffer);

Looks like you've got an extra space after gint.

+    GList* items = pango_itemize(context, buffer, 0, length, NULL, NULL);
+    g_list_foreach(items, (GFunc)pango_item_free, NULL);

Since this is C++ code, please use 0 instead of NULL.

+            g_warning ("didn't get 1 glyph but %d", glyphs->num_glyphs);

I think it would be better to use our LOG() macros here.

There are a few other places that have a space before the parameter list of a
function that I haven't explicitly called out here.

I don't think adding braces around the for loops would make the code seem
crammed. I do think it would allow you to make the function calls match our
coding style better, though.

This looks sane overall (though I don't know Pango), but we definitely at least
need to fix the CString issue.


-- 
Configure bugmail: http://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.