Commit 8d07a719 authored by Meredith Lane's avatar Meredith Lane Committed by Commit Bot

Revert "Remove ignorable codepoints for FontFallback"

This reverts commit 143d21fd.

Reason for revert: Deterministically fails gfx_unittests on Mac 10.12
First failure: https://ci.chromium.org/p/chromium/builders/ci/Mac10.12%20Tests/31106

Original change's description:
> Remove ignorable codepoints for FontFallback
> 
> Default ignorable codepoints should not be considered for
> choosing a fallback font.
> 
> see:
>  http://www.unicode.org/L2/L2002/02368-default-ignorable.pdf
> """
>   Default-ignorable codepoints are those that should be ignored
>   by default in rendering (unless explicitly supported). They have
>   no visible glyph or advance width in and of themselves, although
>   they may affect the display, positioning, or adornment of
>   adjacent or surrounding characters.
> """
> 
> The API used to determine fallback fonts on Mac is
> CTFontCreateForString(...)
>   1) This API can returns .LastResort for an unknown text.
>      We observed this behavior with Emoji (e.g. regional flags).
>    see:
>      https://unicode.org/policies/lastresortfont_eula.html
> 
>   2) The API is not able to match default ignorable codepoints
>      like ZWJ and NZWJ. GetFallbackFont(...) was failing when
>      an ignorable codepoints is present and the expensive
>      GetFallbackFonts(...) is used instead.
> 
> 
> Bug: 1036652
> Change-Id: I12a81a0130b072f662547abd0d82f73c24a7b20d
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1995501
> Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
> Reviewed-by: Robert Liao <robliao@chromium.org>
> Reviewed-by: Dominik Röttsches <drott@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#732133}

TBR=robliao@chromium.org,drott@chromium.org,etienneb@chromium.org

Change-Id: I5c9fa4078a1519fc044536befd5d8a6ed0ea2604
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1036652
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2004168Reviewed-by: default avatarMeredith Lane <meredithl@chromium.org>
Commit-Queue: Meredith Lane <meredithl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732252}
parent f9cbf3a9
......@@ -16,8 +16,6 @@
namespace gfx {
namespace {
constexpr char kLastResortFontName[] = ".LastResort";
// CTFontCreateForString() sometimes re-wraps its result in a new CTFontRef with
// identical attributes. This wastes time shaping the text run and confounds
// Skia's internal typeface cache.
......@@ -87,7 +85,7 @@ bool GetFallbackFont(const Font& font,
return false;
*result = Font(base::mac::CFToNSCast(ct_result.get()));
return result->GetFontName() != kLastResortFontName;
return true;
}
} // namespace gfx
......@@ -108,24 +108,6 @@ bool IsBracket(UChar32 codepoint) {
U_BPT_NONE;
}
// Remove ignorables codepoint from text.
base::string16 RemoveIgnorableCodepoints(const base::StringPiece16 text) {
base::string16 result;
result.resize(text.size());
size_t offset = 0;
bool is_error = false;
for (base::i18n::UTF16CharIterator iter(text.data(), text.length());
!iter.end(); iter.Advance()) {
const UChar32 codepoint = iter.get();
if (!u_hasBinaryProperty(codepoint, UCHAR_DEFAULT_IGNORABLE_CODE_POINT)) {
U16_APPEND(&result[0], offset, result.length(), codepoint, is_error);
}
}
DCHECK(!is_error);
result.resize(offset);
return result;
}
// Writes the script and the script extensions of the Unicode |codepoint|.
// Returns the number of written scripts.
size_t GetScriptExtensions(UChar32 codepoint, UScriptCode* scripts) {
......@@ -2020,16 +2002,6 @@ void RenderTextHarfBuzz::ShapeRuns(
current_run->range.length());
fallback_found =
GetFallbackFont(primary_font, locale_, run_text, &fallback_font);
if (!fallback_found) {
// Ignorable codepoints are handled by Harfbuzz and can be safely
// ignored. The default behavior of harfbuzz is to hide these glyphs by
// using the space glyph and zeroing the advance width.
const base::string16 fallback_run_text =
RemoveIgnorableCodepoints(run_text);
fallback_found = GetFallbackFont(primary_font, locale_,
fallback_run_text, &fallback_font);
}
}
if (fallback_found) {
......
......@@ -4584,24 +4584,6 @@ TEST_F(RenderTextTest, SameFontForParentheses) {
}
}
TEST_F(RenderTextTest, SameFontAccrossIgnorableCodepoints) {
RenderText* render_text = GetRenderText();
render_text->SetText(WideToUTF16(L"\u060F"));
const std::vector<FontSpan> spans1 = GetFontSpans();
ASSERT_EQ(1u, spans1.size());
Font font1 = spans1[0].first;
render_text->SetText(WideToUTF16(L"\u060F\u200C\u060F"));
const std::vector<FontSpan> spans2 = GetFontSpans();
ASSERT_EQ(1u, spans2.size());
Font font2 = spans2[0].first;
// Ensures the same font is used with or without the joiners
// (see http://cdbug.com/1036652).
EXPECT_EQ(font1.GetFontName(), font2.GetFontName());
}
// Make sure the caret width is always >=1 so that the correct
// caret is drawn at high DPI. crbug.com/164100.
TEST_F(RenderTextTest, CaretWidth) {
......
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