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

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Sat Mar 4 00:40:52 PST 2006

Eric Seidel <macdome at opendarwin.org> has granted Eric Seidel
<macdome at opendarwin.org>'s request for review:
Bug 7586: Eliminate QFontMetrics

Attachment 6837: Eliminate QFontMetrics

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


More information about the webkit-reviews mailing list