Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1613153002:
Add a SymbolsIterator to aid with Symbols Segmentation (Closed)
|
Created:
4 years, 11 months ago by drott Modified:
4 years, 11 months ago CC:
blink-reviews, blink-reviews-platform-graphics_chromium.org, blink-reviews-wtf_chromium.org, Rik, chromium-reviews, danakj+watch_chromium.org, dshwang, drott+blinkwatch_chromium.org, krit, f(malita), jbroman, Justin Novosad, kinuko+watch, Mikhail, pdr+graphicswatchlist_chromium.org, rwlbuis, Stephen Chennney, vmpstr+blinkwatch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd a SymbolsIterator to aid with Symbols Segmentation
This allows us to have fine-grained control over emoji/text
presentation and helps us with choosing fonts that are better
suited for punctuation, arrows, line drawing, etc. for which
fallback APIs often do not return suitable results.
The categorization into text, symbols, math, emoji text, and
emoji emoji presentation fonts is partially taken from what our
Windows fallback code does for several Unicode blocks. Once this
CL lands, we can simplify and remove these portions of code from
the Windows font fallback code.
The result of the symbol segmentation stage is a value
FontFallbackPriority that guides prioritization of a font
suitable for displaying this type of text in
FontFallbackIterator.
BUG=580074
R=behdad,eae,kojii
Committed: https://crrev.com/906a9c113e5f5f4df8f7005fe0a0d834bacefbcb
Cr-Commit-Position: refs/heads/master@{#370974}
Patch Set 1 #Patch Set 2 : Update emoji data file reference to version 2.0 #
Total comments: 4
Patch Set 3 : Magic constants moved, EmojiEmoji enum value explained, formatting #
Messages
Total messages: 45 (22 generated)
Currently includes https://codereview.chromium.org/1604323002/ until that is landed.
Description was changed from ========== Add a SymbolsIterator to aid with Symbols Segmentation This allows us to have fine-grained control emoji emoji/text presentation and helps us with choosing fonts that are better suited for punctuation, arrows, line drawing, etc. for which fallback APIs often do not return suitable results. The result of the symbol segmentation stage is a value FontFallbackPriority that guides prioritization of a font suitable for displaying this type of text in FontFallbackIterator. BUG=580074 R=behdad,eae,kojii ========== to ========== Add a SymbolsIterator to aid with Symbols Segmentation This allows us to have fine-grained control emoji emoji/text presentation and helps us with choosing fonts that are better suited for punctuation, arrows, line drawing, etc. for which fallback APIs often do not return suitable results. The result of the symbol segmentation stage is a value FontFallbackPriority that guides prioritization of a font suitable for displaying this type of text in FontFallbackIterator. BUG=580074 R=behdad,eae,kojii ==========
The CQ bit was checked by drott@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1613153002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1613153002/1
Update emoji data file reference to version 2.0
Description was changed from ========== Add a SymbolsIterator to aid with Symbols Segmentation This allows us to have fine-grained control emoji emoji/text presentation and helps us with choosing fonts that are better suited for punctuation, arrows, line drawing, etc. for which fallback APIs often do not return suitable results. The result of the symbol segmentation stage is a value FontFallbackPriority that guides prioritization of a font suitable for displaying this type of text in FontFallbackIterator. BUG=580074 R=behdad,eae,kojii ========== to ========== Add a SymbolsIterator to aid with Symbols Segmentation This allows us to have fine-grained control emoji emoji/text presentation and helps us with choosing fonts that are better suited for punctuation, arrows, line drawing, etc. for which fallback APIs often do not return suitable results. The categorization into text, symbols, math, emoji text, and emoji emoji presentation fonts is partially taken from what our Windows fallback code does for several Unicode blocks. Once this CL lands, we can simplify and remove these portions of code from the Windows font fallback code. The result of the symbol segmentation stage is a value FontFallbackPriority that guides prioritization of a font suitable for displaying this type of text in FontFallbackIterator. BUG=580074 R=behdad,eae,kojii ==========
The CQ bit was checked by drott@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1613153002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1613153002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
drott@chromium.org changed reviewers: + leviw@chromium.org, wkorman@chromium.org
very nice, LGTM w/nit https://codereview.chromium.org/1613153002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/platform/fonts/SymbolsIterator.cpp (right): https://codereview.chromium.org/1613153002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/platform/fonts/SymbolsIterator.cpp:30: if (codepoint == 0x20E3 || codepoint == 0x20E0) Could you please add these codepoints to CharacterNames.h and use the constants instead?
lgtm https://codereview.chromium.org/1613153002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/platform/fonts/FontFallbackPriority.cpp (right): https://codereview.chromium.org/1613153002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/platform/fonts/FontFallbackPriority.cpp:11: return fallbackPriority == FontFallbackPriority::Symbols Why not return fallbackPriority != FontFallbackPriority::Text? https://codereview.chromium.org/1613153002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/platform/fonts/FontFallbackPriority.h (right): https://codereview.chromium.org/1613153002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/platform/fonts/FontFallbackPriority.h:22: EmojiEmoji, EmojiImage / EmojiIcon is more what I would expect to see for this one but I may just not be familiar with nomenclature used in emoji related code. Reading 'EmojiEmoji' everywhere looks a bit like a typo to a naive reader. If we keep it the way it is maybe add a comment explaining?
lgtm
Magic constants moved to CharacterNames, EmojiEmoji enum value explained, some formatting
The CQ bit was checked by drott@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from behdad@chromium.org, eae@chromium.org, wkorman@chromium.org Link to the patchset: https://codereview.chromium.org/1613153002/#ps40001 (title: "Magic constants moved to CharacterNames, EmojiEmoji enum value explained, some formatting")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1613153002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1613153002/40001
The CQ bit was unchecked by commit-bot@chromium.org
CLs for remote refs other than refs/pending/heads/master must contain NOTRY=true and NOPRESUBMIT=true in order for the CQ to process them
Description was changed from ========== Add a SymbolsIterator to aid with Symbols Segmentation This allows us to have fine-grained control emoji emoji/text presentation and helps us with choosing fonts that are better suited for punctuation, arrows, line drawing, etc. for which fallback APIs often do not return suitable results. The categorization into text, symbols, math, emoji text, and emoji emoji presentation fonts is partially taken from what our Windows fallback code does for several Unicode blocks. Once this CL lands, we can simplify and remove these portions of code from the Windows font fallback code. The result of the symbol segmentation stage is a value FontFallbackPriority that guides prioritization of a font suitable for displaying this type of text in FontFallbackIterator. BUG=580074 R=behdad,eae,kojii ========== to ========== Add a SymbolsIterator to aid with Symbols Segmentation This allows us to have fine-grained control over emoji/text presentation and helps us with choosing fonts that are better suited for punctuation, arrows, line drawing, etc. for which fallback APIs often do not return suitable results. The categorization into text, symbols, math, emoji text, and emoji emoji presentation fonts is partially taken from what our Windows fallback code does for several Unicode blocks. Once this CL lands, we can simplify and remove these portions of code from the Windows font fallback code. The result of the symbol segmentation stage is a value FontFallbackPriority that guides prioritization of a font suitable for displaying this type of text in FontFallbackIterator. BUG=580074 R=behdad,eae,kojii ==========
https://codereview.chromium.org/1613153002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/platform/fonts/FontFallbackPriority.cpp (right): https://codereview.chromium.org/1613153002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/platform/fonts/FontFallbackPriority.cpp:11: return fallbackPriority == FontFallbackPriority::Symbols On 2016/01/22 00:50:45, wkorman wrote: > Why not return fallbackPriority != FontFallbackPriority::Text? I chose to enumerate them because I would like the next person that adds a value to the enumeration to explicitly decide whether it's a non-text value that requires special handling in FontFallbackIterator and FontCache (the changes to those classes will follow in subsequent CLs).
Patchset #3 (id:40001) has been deleted
Magic constants moved, EmojiEmoji enum value explained, formatting
The CQ bit was checked by drott@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from behdad@chromium.org, eae@chromium.org, wkorman@chromium.org Link to the patchset: https://codereview.chromium.org/1613153002/#ps60001 (title: "Magic constants moved, EmojiEmoji enum value explained, formatting")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1613153002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1613153002/60001
The CQ bit was unchecked by commit-bot@chromium.org
CLs for remote refs other than refs/pending/heads/master must contain NOTRY=true and NOPRESUBMIT=true in order for the CQ to process them
Patchset #3 (id:60001) has been deleted
Magic constants moved, EmojiEmoji enum value explained, formatting
The CQ bit was checked by drott@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from behdad@chromium.org, eae@chromium.org, wkorman@chromium.org Link to the patchset: https://codereview.chromium.org/1613153002/#ps80001 (title: "Magic constants moved, EmojiEmoji enum value explained, formatting")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1613153002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1613153002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
drott@chromium.org changed reviewers: + mikhail.pozdnyakov@intel.com
Mikhail, could you review the additional character literals? I am adding myself as OWNER for this file, but the first attempt failed, see https://codereview.chromium.org/1618823002/ - thank you.
wtf/ lgtm
The CQ bit was checked by drott@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1613153002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1613153002/80001
Message was sent while issue was closed.
Description was changed from ========== Add a SymbolsIterator to aid with Symbols Segmentation This allows us to have fine-grained control over emoji/text presentation and helps us with choosing fonts that are better suited for punctuation, arrows, line drawing, etc. for which fallback APIs often do not return suitable results. The categorization into text, symbols, math, emoji text, and emoji emoji presentation fonts is partially taken from what our Windows fallback code does for several Unicode blocks. Once this CL lands, we can simplify and remove these portions of code from the Windows font fallback code. The result of the symbol segmentation stage is a value FontFallbackPriority that guides prioritization of a font suitable for displaying this type of text in FontFallbackIterator. BUG=580074 R=behdad,eae,kojii ========== to ========== Add a SymbolsIterator to aid with Symbols Segmentation This allows us to have fine-grained control over emoji/text presentation and helps us with choosing fonts that are better suited for punctuation, arrows, line drawing, etc. for which fallback APIs often do not return suitable results. The categorization into text, symbols, math, emoji text, and emoji emoji presentation fonts is partially taken from what our Windows fallback code does for several Unicode blocks. Once this CL lands, we can simplify and remove these portions of code from the Windows font fallback code. The result of the symbol segmentation stage is a value FontFallbackPriority that guides prioritization of a font suitable for displaying this type of text in FontFallbackIterator. BUG=580074 R=behdad,eae,kojii ==========
Message was sent while issue was closed.
Committed patchset #3 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Add a SymbolsIterator to aid with Symbols Segmentation This allows us to have fine-grained control over emoji/text presentation and helps us with choosing fonts that are better suited for punctuation, arrows, line drawing, etc. for which fallback APIs often do not return suitable results. The categorization into text, symbols, math, emoji text, and emoji emoji presentation fonts is partially taken from what our Windows fallback code does for several Unicode blocks. Once this CL lands, we can simplify and remove these portions of code from the Windows font fallback code. The result of the symbol segmentation stage is a value FontFallbackPriority that guides prioritization of a font suitable for displaying this type of text in FontFallbackIterator. BUG=580074 R=behdad,eae,kojii ========== to ========== Add a SymbolsIterator to aid with Symbols Segmentation This allows us to have fine-grained control over emoji/text presentation and helps us with choosing fonts that are better suited for punctuation, arrows, line drawing, etc. for which fallback APIs often do not return suitable results. The categorization into text, symbols, math, emoji text, and emoji emoji presentation fonts is partially taken from what our Windows fallback code does for several Unicode blocks. Once this CL lands, we can simplify and remove these portions of code from the Windows font fallback code. The result of the symbol segmentation stage is a value FontFallbackPriority that guides prioritization of a font suitable for displaying this type of text in FontFallbackIterator. BUG=580074 R=behdad,eae,kojii Committed: https://crrev.com/906a9c113e5f5f4df8f7005fe0a0d834bacefbcb Cr-Commit-Position: refs/heads/master@{#370974} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/906a9c113e5f5f4df8f7005fe0a0d834bacefbcb Cr-Commit-Position: refs/heads/master@{#370974} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
