Commit cb43fc3c authored by Etienne Bergeron's avatar Etienne Bergeron Committed by Commit Bot

Keep typeface on PlatformFontWin for fallback fonts

The PlatformFontWin is not able to represent a specific typeface.
A recent bug (http://crbug.com/1003829) shows that fallback fonts
should be kept as a typeface to avoid losing information and doing
wrong rematch.

This CL is a continuation of.
  https://chromium-review.googlesource.com/c/chromium/src/+/1824104
The above CL is fixing the problem but rely on the migration of
PlatformFontSkiaOnWindows to be completed. The current CL proposed
to keep a path working either way. This way the shipping of
PlatformFontSkiaOnWindows is independent of the fallback font fix
and the Georgian fonts should work fine in both PlatformFont.

R=robliao@chromium.org
CC=​​​drott@chromium.org

Bug: 1003829
Change-Id: If6b9872da01cadaf4f538db2ebac42e887f106d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1848913
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Reviewed-by: default avatarRobert Liao <robliao@chromium.org>
Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704653}
parent 4811d89e
...@@ -275,11 +275,27 @@ HRESULT GetFamilyNameFromDirectWriteFont(IDWriteFont* dwrite_font, ...@@ -275,11 +275,27 @@ HRESULT GetFamilyNameFromDirectWriteFont(IDWriteFont* dwrite_font,
PlatformFontWin::PlatformFontWin() : font_ref_(GetBaseFontRef()) { PlatformFontWin::PlatformFontWin() : font_ref_(GetBaseFontRef()) {
} }
PlatformFontWin::PlatformFontWin(const std::string& font_name, PlatformFontWin::PlatformFontWin(const std::string& font_name, int font_size) {
int font_size) {
InitWithFontNameAndSize(font_name, font_size); InitWithFontNameAndSize(font_name, font_size);
} }
PlatformFontWin::PlatformFontWin(sk_sp<SkTypeface> typeface,
int font_size_pixels,
const base::Optional<FontRenderParams>& params)
: typeface_(std::move(typeface)) {
DCHECK(typeface_);
// TODO(http://crbug.com/944227): This is a transitional code path until we
// complete migrating to PlatformFontSkia on Windows. Being unable to wrap the
// SkTypeface into a PlatformFontSkia and performing a rematching by font
// family name instead loses platform font handles encapsulated in SkTypeface,
// and in turn leads to instantiating a different font than what was returned
// by font fallback, compare https://crbug.com/1003829.
SkString family_name;
typeface_->getFamilyName(&family_name);
InitWithFontNameAndSize(family_name.c_str(), font_size_pixels);
}
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
// PlatformFontWin, PlatformFont implementation: // PlatformFontWin, PlatformFont implementation:
...@@ -360,7 +376,7 @@ const FontRenderParams& PlatformFontWin::GetFontRenderParams() { ...@@ -360,7 +376,7 @@ const FontRenderParams& PlatformFontWin::GetFontRenderParams() {
} }
sk_sp<SkTypeface> PlatformFontWin::GetNativeSkTypefaceIfAvailable() const { sk_sp<SkTypeface> PlatformFontWin::GetNativeSkTypefaceIfAvailable() const {
return nullptr; return sk_sp<SkTypeface>(typeface_);
} }
NativeFont PlatformFontWin::GetNativeFont() const { NativeFont PlatformFontWin::GetNativeFont() const {
...@@ -657,16 +673,7 @@ PlatformFont* PlatformFont::CreateFromSkTypeface( ...@@ -657,16 +673,7 @@ PlatformFont* PlatformFont::CreateFromSkTypeface(
TRACE_EVENT0("fonts", "PlatformFont::CreateFromSkTypeface"); TRACE_EVENT0("fonts", "PlatformFont::CreateFromSkTypeface");
if (base::FeatureList::IsEnabled(kPlatformFontSkiaOnWindows)) if (base::FeatureList::IsEnabled(kPlatformFontSkiaOnWindows))
return new PlatformFontSkia(typeface, font_size, params); return new PlatformFontSkia(typeface, font_size, params);
return new PlatformFontWin(typeface, font_size, params);
// This is a transitional code path until we complete migrating to
// PlatformFontSkia on Windows. Being unable to wrap the SkTypeface into a
// PlatformFontSkia and performing a rematching by font family name instead
// loses platform font handles encapsulated in SkTypeface, and in turn leads
// to instantiating a different font than what was returned by font fallback,
// compare https://crbug.com/1003829.
SkString family_name;
typeface->getFamilyName(&family_name);
return new PlatformFontWin(family_name.c_str(), font_size);
} }
} // namespace gfx } // namespace gfx
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/gtest_prod_util.h" #include "base/gtest_prod_util.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "third_party/skia/include/core/SkTypeface.h"
#include "ui/gfx/gfx_export.h" #include "ui/gfx/gfx_export.h"
#include "ui/gfx/platform_font.h" #include "ui/gfx/platform_font.h"
...@@ -26,6 +27,11 @@ class GFX_EXPORT PlatformFontWin : public PlatformFont { ...@@ -26,6 +27,11 @@ class GFX_EXPORT PlatformFontWin : public PlatformFont {
PlatformFontWin(); PlatformFontWin();
PlatformFontWin(const std::string& font_name, int font_size); PlatformFontWin(const std::string& font_name, int font_size);
// Wraps the provided SkTypeface without triggering a font rematch.
PlatformFontWin(sk_sp<SkTypeface> typeface,
int font_size_pixels,
const base::Optional<FontRenderParams>& params);
// Dialog units to pixels conversion. // Dialog units to pixels conversion.
// See http://support.microsoft.com/kb/145994 for details. // See http://support.microsoft.com/kb/145994 for details.
int horizontal_dlus_to_pixels(int dlus) const { int horizontal_dlus_to_pixels(int dlus) const {
...@@ -191,6 +197,9 @@ class GFX_EXPORT PlatformFontWin : public PlatformFont { ...@@ -191,6 +197,9 @@ class GFX_EXPORT PlatformFontWin : public PlatformFont {
// Indirect reference to the HFontRef, which references the underlying HFONT. // Indirect reference to the HFontRef, which references the underlying HFONT.
scoped_refptr<HFontRef> font_ref_; scoped_refptr<HFontRef> font_ref_;
// An optional typeface when the font is constructed from a typeface.
sk_sp<SkTypeface> typeface_;
DISALLOW_COPY_AND_ASSIGN(PlatformFontWin); DISALLOW_COPY_AND_ASSIGN(PlatformFontWin);
}; };
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "base/win/scoped_hdc.h" #include "base/win/scoped_hdc.h"
#include "base/win/scoped_select_object.h" #include "base/win/scoped_select_object.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "third_party/skia/include/core/SkFontMgr.h"
#include "ui/gfx/font.h" #include "ui/gfx/font.h"
#include "ui/gfx/font_render_params.h" #include "ui/gfx/font_render_params.h"
#include "ui/gfx/platform_font_skia.h" #include "ui/gfx/platform_font_skia.h"
...@@ -143,4 +144,18 @@ TEST(PlatformFontWinTest, DefaultFontRenderParams) { ...@@ -143,4 +144,18 @@ TEST(PlatformFontWinTest, DefaultFontRenderParams) {
named_font->GetFontRenderParams()); named_font->GetFontRenderParams());
} }
TEST(PlatformFontWinTest, SkiaTypefaceConstructor) {
gfx::Font default_font;
EXPECT_EQ(default_font.platform_font()->GetNativeSkTypefaceIfAvailable(),
nullptr);
sk_sp<SkFontMgr> font_mgr = SkFontMgr::RefDefault();
sk_sp<SkTypeface> typeface(
font_mgr->matchFamilyStyle("Segoe UI", SkFontStyle()));
ASSERT_TRUE(typeface);
gfx::Font fallback_font(new PlatformFontWin(typeface, 13, base::nullopt));
EXPECT_EQ(fallback_font.platform_font()->GetNativeSkTypefaceIfAvailable(),
typeface);
}
} // namespace gfx } // namespace gfx
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment