Update OpenType feature detection to hb_ot_layout_table_choose_script API call |
||
Issue descriptionIn https://codereview.chromium.org/1882063002 Behdad suggests to change the used HarfBuzz API for implementing the OpenType feature detection: https://codereview.chromium.org/1882063002/diff/1/third_party/WebKit/Source/platform/fonts/opentype/OpenTypeCapsSupportMPL.cpp#newcode65 > third_party/WebKit/Source/platform/fonts/opentype/OpenTypeCapsSupportMPL.cpp:65: if (hb_ot_layout_table_find_script(face, > Using hb_ot_layout_table_choose_script instead of hb_ot_layout_table_find_script simplifies the logic a bit, if you are up to updating for that. Behdad, could you explain this in a bit more detail, how you suggest to modify the code?
,
May 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c51ee10d5afd2a657ed357cda9caf31c788790c1 commit c51ee10d5afd2a657ed357cda9caf31c788790c1 Author: drott <drott@chromium.org> Date: Fri May 27 22:00:25 2016 Simplify OpenType caps feature detection logic We can rely on HarfBuzz API for finding the required OpenType features, instead of implementing our own find loops. Thanks to Behdad for the suggestion. TEST=LayoutTest/fast/text/font-features/* BUG= 603137 R=behdad,eae Review-Url: https://codereview.chromium.org/2017533002 Cr-Commit-Position: refs/heads/master@{#396579} [modify] https://crrev.com/c51ee10d5afd2a657ed357cda9caf31c788790c1/third_party/WebKit/Source/platform/fonts/opentype/OpenTypeCapsSupportMPL.cpp
,
May 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/09e8b8a7b8da00f9595df458606c2a133f99a74f commit 09e8b8a7b8da00f9595df458606c2a133f99a74f Author: drott <drott@chromium.org> Date: Mon May 30 12:00:32 2016 Addendum to OpenType feature detection simplification Addressing Behdad's review comment. BUG= 603137 TBR=behdad Review-Url: https://codereview.chromium.org/2026433002 Cr-Commit-Position: refs/heads/master@{#396714} [modify] https://crrev.com/09e8b8a7b8da00f9595df458606c2a133f99a74f/third_party/WebKit/Source/platform/fonts/opentype/OpenTypeCapsSupportMPL.cpp
,
May 30 2016
,
May 31 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3d9eeeefe535a25f319998c3d2cd848452bf5a03 commit 3d9eeeefe535a25f319998c3d2cd848452bf5a03 Author: guidou <guidou@chromium.org> Date: Tue May 31 15:52:42 2016 Revert of Addendum to OpenType feature detection simplification (patchset #1 id:1 of https://codereview.chromium.org/2026433002/ ) Reason for revert: This CL is suspect of causing fast/text/font-features/caps-native-synthesis.html to fail in the WebKit Win10 bot. See: https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win10/builds/10722 https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win10?numbuilds=200 I will reland if the revert doesn't fix the bot. Original issue's description: > Addendum to OpenType feature detection simplification > > Addressing Behdad's review comment. > > BUG= 603137 > TBR=behdad > > Committed: https://crrev.com/09e8b8a7b8da00f9595df458606c2a133f99a74f > Cr-Commit-Position: refs/heads/master@{#396714} TBR=behdad@chromium.org,drott@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 603137 Review-Url: https://codereview.chromium.org/2027623002 Cr-Commit-Position: refs/heads/master@{#396835} [modify] https://crrev.com/3d9eeeefe535a25f319998c3d2cd848452bf5a03/third_party/WebKit/Source/platform/fonts/opentype/OpenTypeCapsSupportMPL.cpp
,
Jun 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4095e6cfa57acce24b79f4c3ec9d7f2f70e34643 commit 4095e6cfa57acce24b79f4c3ec9d7f2f70e34643 Author: drott <drott@chromium.org> Date: Mon Jun 06 11:21:41 2016 Addendum to OpenType feature detection simplification Addressing Behdad's review comment. Reland after revert in 3d9eeeefe, test expectation can be rebaselined. Landing TBR, since previously LGTM'ed in https://codereview.chromium.org/2026433002 BUG= 603137 TBR=behdad Review-Url: https://codereview.chromium.org/2040923002 Cr-Commit-Position: refs/heads/master@{#398004} [modify] https://crrev.com/4095e6cfa57acce24b79f4c3ec9d7f2f70e34643/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/4095e6cfa57acce24b79f4c3ec9d7f2f70e34643/third_party/WebKit/Source/platform/fonts/opentype/OpenTypeCapsSupportMPL.cpp
,
Jun 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/50d3da951f1ff02ad21b8072b9ffb976884127ff commit 50d3da951f1ff02ad21b8072b9ffb976884127ff Author: Rebaseline Bot <blink-rebaseline-bot@chromium.org> Date: Mon Jun 06 12:38:45 2016 Auto-rebaseline for r398004 https://chromium.googlesource.com/chromium/src/+/4095e6cfa BUG= 603137 TBR=drott@chromium.org Review URL: https://codereview.chromium.org/2046513002 . Cr-Commit-Position: refs/heads/master@{#398012} [modify] https://crrev.com/50d3da951f1ff02ad21b8072b9ffb976884127ff/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/50d3da951f1ff02ad21b8072b9ffb976884127ff/third_party/WebKit/LayoutTests/platform/win/fast/text/font-features/caps-native-synthesis-expected.png [modify] https://crrev.com/50d3da951f1ff02ad21b8072b9ffb976884127ff/third_party/WebKit/LayoutTests/platform/win/fast/text/font-features/caps-native-synthesis-expected.txt |
||
►
Sign in to add a comment |
||
Comment 1 by behdad@chromium.org
, May 25 2016Basically this block: 35 if (!hb_ot_layout_has_substitution(face)) 36 return false; 37 38 // Get the OpenType tag(s) that match this script code 39 const size_t kMaxScriptTags = 4; 40 hb_tag_t scriptTags[kMaxScriptTags] = { 41 HB_TAG_NONE, 42 HB_TAG_NONE, 43 HB_TAG_NONE, 44 HB_TAG_NONE 45 }; 46 hb_ot_tags_from_script(static_cast<hb_script_t>(script), 47 &scriptTags[0], 48 &scriptTags[1]); 49 50 // Replace the first remaining NONE with DEFAULT 51 for (size_t i = 0; i < kMaxScriptTags; ++i) { 52 if (scriptTags[i] == HB_TAG_NONE) { 53 scriptTags[i] = HB_OT_TAG_DEFAULT_SCRIPT; 54 break; 55 } 56 } 57 58 // Now check for 'smcp' under the first of those scripts that is present 59 const hb_tag_t kGSUB = HB_TAG('G', 'S', 'U', 'B'); 60 for (size_t j = 0; j < kMaxScriptTags; ++j) { 61 if (scriptTags[j] == HB_TAG_NONE) 62 break; 63 64 unsigned scriptIndex; 65 if (hb_ot_layout_table_find_script(face, 66 kGSUB, 67 scriptTags[j], 68 &scriptIndex)) { 69 if (hb_ot_layout_language_find_feature(face, kGSUB, 70 scriptIndex, 71 HB_OT_LAYOUT_DEFAULT_LANGUAGE_INDEX, 72 tag, nullptr)) { 73 result = true; 74 } 75 break; 76 } 77 } can become: 40 hb_tag_t scriptTags[3] = {HB_TAG_NONE, HB_TAG_NONE, HB_TAG_NONE}; 46 hb_ot_tags_from_script(static_cast<hb_script_t>(script), 47 &scriptTags[0], 48 &scriptTags[1]); hb_ot_layout_table_choose_script (face, 66 kGSUB, 67 scriptTags[j], 68 &scriptIndex, nullptr); if (hb_ot_layout_language_find_feature(face, kGSUB, 70 scriptIndex, 71 HB_OT_LAYOUT_DEFAULT_LANGUAGE_INDEX, 72 tag, nullptr)) { 73 result = true; 74 }