[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
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.
More information about the webkit-reviews
mailing list