[webkit-reviews] review granted: [Bug 7586] Eliminate QFontMetrics : [Attachment 6837] Eliminate QFontMetrics

[bugzilla-request-daemon at opendarwin.org]

Eric Seidel <macdome at opendarwin.org> has granted Eric Seidel
<macdome at opendarwin.org>'s request for review:
Bug 7586: Eliminate QFontMetrics
http://bugzilla.opendarwin.org/show_bug.cgi?id=7586

Attachment 6837: Eliminate QFontMetrics
http://bugzilla.opendarwin.org/attachment.cgi?id=6837&action=edit

------- Additional Comments from Eric Seidel <macdome at opendarwin.org>
You disappeared from IRC, so a few more comments here:

+	 if (fontDescription.weight() >= WebCore::cBoldWeight)

Does this still need to be >= now that this is an enum?

+int Font::ascent() const
+{
+    return [m_renderer->getRenderer(fontDescription()) ascent];
+}

Does this need KWQ_BLOCK_EXCEPTIONS?  is there any time in which you might have
a renderer which does not responde to -(int)ascent; for instance?

same goes for -(int)decent;

Maybe some of these methods should be moved into a common Font.cpp file:

+int Font::height() const
+{
+    return ascent() + descent();
+}


Otherwise it looks great!  There are always performance concerns w/ hot code
like this, so you'll need to be sure to autovicki or at least be prepared to
roll out if we regress.

r=me.