Skip to content

Commit

Permalink
Landing font change again. I don't believe early flakiness was the
Browse files Browse the repository at this point in the history
result of my change. test_shell doesn't use font at all. This is
exactly the same change as was earlier landed.

BUG=22791
TEST=see bug
TBR=agl
Review URL: http://codereview.chromium.org/242111

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@27790 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
sky@chromium.org committed Oct 1, 2009
1 parent 2141597 commit 00e7029
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 16 deletions.
40 changes: 27 additions & 13 deletions app/gfx/font_skia.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

#include "app/gfx/font.h"

#include <gdk/gdk.h>
#include <pango/pango.h>

#include "app/gfx/canvas.h"
#include "base/logging.h"
#include "base/string_piece.h"
Expand All @@ -18,6 +21,27 @@ namespace {
// IsFallbackFontAllowed function in skia/ext/SkFontHost_fontconfig_direct.cpp.
const char* kFallbackFontFamilyName = "sans";

// Pango scales font sizes. This returns the scale factor. See
// pango_cairo_context_set_resolution for details.
// NOTE: this isn't entirely accurate, in that Pango also consults the
// FC_PIXEL_SIZE first (see get_font_size in pangocairo-fcfont), but this
// seems to give us the same sizes as used by Pango for all our fonts in both
// English and Thia.
static double GetPangoScaleFactor() {
static float scale_factor = 0;
static bool determined_scale = false;
if (!determined_scale) {
PangoContext* context = gdk_pango_context_get();
scale_factor = pango_cairo_context_get_resolution(context);
g_object_unref(context);
if (scale_factor <= 0)
scale_factor = 1;
else
scale_factor /= 72.0;
}
return scale_factor;
}

} // namespace

namespace gfx {
Expand Down Expand Up @@ -49,18 +73,8 @@ void Font::calculateMetrics() {
PaintSetup(&paint);
paint.getFontMetrics(&metrics);

// NOTE: we don't use the ascent/descent as it doesn't match with how pango
// ends up drawing the text, in particular if we clip to the ascent/descent
// the text is clipped. This algorithm doesn't give us an exact match with
// the numbers returned from pango (we are off by 1 in some cases), but it
// is close enough that you won't notice clipping.
//
// NOTE2: I tried converting this to use Pango exclusively for measuring the
// text but it causes a startup regression. The best I could get it was
// ~10% slow down. Slow down appeared to be entirely in libfontconfig.
ascent_ = SkScalarCeil(-metrics.fTop);
height_ = SkScalarCeil(-metrics.fTop) + SkScalarCeil(metrics.fBottom) +
SkScalarCeil(metrics.fLeading);
ascent_ = SkScalarCeil(-metrics.fAscent);
height_ = ascent_ + SkScalarCeil(metrics.fDescent);

if (metrics.fAvgCharWidth) {
avg_width_ = SkScalarRound(metrics.fAvgCharWidth);
Expand Down Expand Up @@ -148,7 +162,7 @@ Font Font::DeriveFont(int size_delta, int style) const {
void Font::PaintSetup(SkPaint* paint) const {
paint->setAntiAlias(false);
paint->setSubpixelText(false);
paint->setTextSize(SkFloatToScalar(font_size_));
paint->setTextSize(SkFloatToScalar(font_size_ * GetPangoScaleFactor()));
paint->setTypeface(typeface_);
paint->setFakeBoldText((BOLD & style_) && !typeface_->isBold());
paint->setTextSkewX((ITALIC & style_) && !typeface_->isItalic() ?
Expand Down
4 changes: 1 addition & 3 deletions app/gfx/font_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,11 @@ TEST_F(FontTest, LoadArialBold) {
TEST_F(FontTest, Ascent) {
Font cf(Font::CreateFont(L"Arial", 16));
ASSERT_GT(cf.baseline(), 2);
ASSERT_LT(cf.baseline(), 20);
}

TEST_F(FontTest, Height) {
Font cf(Font::CreateFont(L"Arial", 16));
ASSERT_GT(cf.baseline(), 2);
ASSERT_LT(cf.baseline(), 20);
ASSERT_LT(cf.baseline(), 22);
}

TEST_F(FontTest, AvgWidths) {
Expand Down

0 comments on commit 00e7029

Please sign in to comment.