Skip to content

GitLab

T
tangled
Commit 1b8f4236 authored by mek's avatar mek Committed by Commit bot
Browse files
Options

Revert of Improve fallback for Burmese with leading punctuation + spacing mark... 

Revert of Improve fallback for Burmese with leading punctuation + spacing mark (patchset #3 id:30009 of https://codereview.chromium.org/2530153002/ )

Reason for revert:
This seems to be causing test failures for fast/text/international/text-spliced-font.html on https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win10/builds/17622

Original issue's description:
> Improve fallback for Burmese with leading punctuation + spacing mark
>
> Issue 618178 describes an example where a Burmese text run starts with a
> leading punctuation character followed by a combining spacing mark. This
> grapheme cannot be shaped with the default font, since Times for example
> cannot display the combination of a left quote with a Burmese combining
> mark. Our fallback code attempts to find a fallback font based on the
> first character at the beginning of an extracted unshaped sub-run, which
> does not lead to finding a font suitable for Myanmar text in this case.
> So in a way it runs into a fallback trap, where no fallback hint helps
> to find the right fallback font and the whole run ends up as notdef
> glyphs.
>
> This CL attempts to resolve this by looking for a better fallback hint
> character, which is not script common or inherited, if such is
> available. This improves the situation for the Burmese text from the
> issue report.
>
> In addition, as a better fix we should give higher importance to the
> locale information in font fallback, filed as issue 668706.
>
> BUG=618178
> R=eae,kojii,behdad
>
> Committed: https://crrev.com/d9280a5e951415e1b2c6c7958adda19b731a99fa
> Cr-Commit-Position: refs/heads/master@{#434665}

TBR=behdad@chromium.org,eae@chromium.org,kojii@chromium.org,drott@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=618178

Review-Url: https://codereview.chromium.org/2532253002
Cr-Commit-Position: refs/heads/master@{#434719}
parent 5c30282c
Hide whitespace changes
Inline Side-by-side
Showing with 6 additions and 93 deletions
third_party/WebKit/LayoutTests/NeverFixTests
View file @ 1b8f4236
......@@ -80,9 +80,6 @@ fast/harness/sample-fail-mismatch-reftest.html [ WontFix ]
# AAT/mortx shaping not supported on platforms other than Mac
[ Linux Win Android ] fast/text/aat-morx.html [ WontFix ]
# Linux layout tests do not have a Myanmar fallback font.
[ Linux ] inspector-protocol/layout-fonts/fallback-myanmar.html [ WontFix ]
# Tests in media/stable are only supposed to be run as virtual test (see virtual/stable/media).
media/stable [ WontFix ]
......
third_party/WebKit/LayoutTests/inspector-protocol/layout-fonts/fallback-myanmar.html deleted 100644 → 0
View file @ 5c30282c
<!DOCTYPE html>
<html>
<meta charset="UTF-8">
<head>
<script type="text/javascript" src="../../http/tests/inspector-protocol/inspector-protocol-test.js"></script>
<script type="text/javascript" src="../../http/tests/inspector-protocol/css-protocol-test.js"></script>
<script type="text/javascript" src="../../http/tests/inspector-protocol/dom-protocol-test.js"></script>
<script type="text/javascript" src="resources/layout-font-test.js"></script>
</head>
<script>
function postTestHookWithFontResults(results) {
var el = document.createElement("div");
var passed = (results['#myanmar'].length == 1 &&
results['#myanmar'][0].familyName.includes("Myanmar")) ||
(results['#myanmar'].length == 2 &&
results['#myanmar'][0].glyphCount == 2 &&
results['#myanmar'][1].glyphCount > 10 &&
results['#myanmar'][1].familyName.includes("Myanmar"));
el.innerHTML = passed ? "PASS" : "FAIL";
document.body.appendChild(el);
}
</script>
<body>
Test passes if a maxmium of the two first glyphs are notdef's (for Myanmar fonts that do not combine a left quote
with a Myanmar spacing mark and the rest of the run is shaped, given a system Myanmar font is available.
<div class="test">
<div lang="my" id="myanmar">‘ေရွးျမန္မာမင္းေတြလက္ထက္က</div>
</div>
</body>
</html>
third_party/WebKit/LayoutTests/inspector-protocol/layout-fonts/resources/layout-font-test.js
View file @ 1b8f4236
......@@ -51,10 +51,6 @@ function test()
InspectorTest.evaluateInInspectedPage("injectCollectedResultsInPage(" +
JSON.stringify(collectedFontUsage) +
")");
InspectorTest.evaluateInInspectedPage("postTestHookWithFontResults(" +
JSON.stringify(collectedFontUsage) +
")");
}
function platformFontsForElementWithSelector(selector)
......
third_party/WebKit/LayoutTests/platform/mac/inspector-protocol/layout-fonts/fallback-myanmar-expected.txt deleted 100644 → 0
View file @ 5c30282c
Test passes if a maxmium of the two first glyphs are notdef's (for Myanmar fonts that do not combine a left quote with a Myanmar spacing mark and the rest of the run is shaped, given a system Myanmar font is available.
‘ေရွးျမန္မာမင္းေတြလက္ထက္က
#myanmar:
"Myanmar MN" : 22
PASS
third_party/WebKit/LayoutTests/platform/win/inspector-protocol/layout-fonts/fallback-myanmar-expected.txt deleted 100644 → 0
View file @ 5c30282c
Test passes if a maxmium of the two first glyphs are notdef's (for Myanmar fonts that do not combine a left quote with a Myanmar spacing mark and the rest of the run is shaped, given a system Myanmar font is available.
‘ေရွးျမန္မာမင္းေတြလက္ထက္က
#myanmar:
"Myanmar Text" : 26
PASS
third_party/WebKit/LayoutTests/platform/win7/inspector-protocol/layout-fonts/fallback-myanmar-expected.txt deleted 100644 → 0
View file @ 5c30282c
Test passes if a maxmium of the two first glyphs are notdef's (for Myanmar fonts that do not combine a left quote with a Myanmar spacing mark and the rest of the run is shaped, given a system Myanmar font is available.
‘ေရွးျမန္မာမင္းေတြလက္ထက္က
#myanmar:
"Arial" : 25
FAIL
third_party/WebKit/Source/platform/fonts/FontFallbackIterator.cpp
View file @ 1b8f4236
......@@ -99,7 +99,8 @@ PassRefPtr<FontDataForRangeSet> FontFallbackIterator::next(
if (m_fallbackStage == SystemFonts) {
// We've reached pref + system fallback.
RefPtr<SimpleFontData> systemFont = uniqueSystemFontForHintList(hintList);
ASSERT(hintList.size());
RefPtr<SimpleFontData> systemFont = uniqueSystemFontForHint(hintList[0]);
if (systemFont) {
// Fallback fonts are not retained in the FontDataCache.
return uniqueOrNext(adoptRef(new FontDataForRangeSet(systemFont)),
......@@ -192,47 +193,15 @@ PassRefPtr<SimpleFontData> FontFallbackIterator::fallbackPriorityFont(
m_fontFallbackPriority);
}
static inline unsigned chooseHintIndex(const Vector<UChar32>& hintList) {
// crbug.com/618178 has a test case where no Myanmar font is ever found,
// because the run starts with a punctuation character with a script value of
// common. Our current font fallback code does not find a very meaningful
// result for this.
// TODO crbug.com/668706 - Improve this situation.
// So if we have multiple hint characters (which indicates that a
// multi-character grapheme or more failed to shape, then we can try to be
// smarter and select the first character that has an actual script value.
DCHECK(hintList.size());
if (hintList.size() <= 1)
return 0;
UErrorCode err = U_ZERO_ERROR;
UScriptCode hintCharScript = uscript_getScript(hintList[0], &err);
if (!U_SUCCESS(err) || hintCharScript > USCRIPT_INHERITED)
return 0;
for (size_t i = 1; i < hintList.size(); ++i) {
UScriptCode newHintScript = uscript_getScript(hintList[i], &err);
if (!U_SUCCESS(err))
return 0;
if (newHintScript > USCRIPT_INHERITED)
return i;
}
return 0;
}
PassRefPtr<SimpleFontData> FontFallbackIterator::uniqueSystemFontForHintList(
const Vector<UChar32>& hintList) {
PassRefPtr<SimpleFontData> FontFallbackIterator::uniqueSystemFontForHint(
UChar32 hint) {
// When we're asked for a fallback for the same characters again, we give up
// because the shaper must have previously tried shaping with the font
// already.
if (!hintList.size())
if (!hint || m_previouslyAskedForHint.contains(hint))
return nullptr;
FontCache* fontCache = FontCache::fontCache();
UChar32 hint = hintList[chooseHintIndex(hintList)];
if (!hint || m_previouslyAskedForHint.contains(hint))
return nullptr;
m_previouslyAskedForHint.add(hint);
return fontCache->fallbackFontForCharacter(
m_fontDescription, hint,
......
third_party/WebKit/Source/platform/fonts/FontFallbackIterator.h
View file @ 1b8f4236
......@@ -51,8 +51,7 @@ class FontFallbackIterator : public RefCounted<FontFallbackIterator> {
const Vector<UChar32>& hintList);
PassRefPtr<SimpleFontData> fallbackPriorityFont(UChar32 hint);
PassRefPtr<SimpleFontData> uniqueSystemFontForHintList(
const Vector<UChar32>& hintList);
PassRefPtr<SimpleFontData> uniqueSystemFontForHint(UChar32 hint);
const FontDescription& m_fontDescription;
RefPtr<FontFallbackList> m_fontFallbackList;
......
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