From 867da4b96e8eb591e15951f9fee719c67a0d8638 Mon Sep 17 00:00:00 2001 From: Kevin Lubick Date: Fri, 22 Feb 2019 15:55:39 -0500 Subject: [PATCH] Allow system harfbuzz. This should allow shaper to run on the no-deps bot. Change-Id: I2515875d4e9b428681c20877630b904c3229ecc5 Reviewed-on: https://skia-review.googlesource.com/c/194420 Reviewed-by: Ben Wagner Commit-Queue: Ben Wagner --- experimental/canvaskit/compile.sh | 4 +- gn/skia.gni | 1 + infra/bots/recipe_modules/build/default.py | 1 + ...d-Debian9-Clang-x86_64-Release-NoDEPS.json | 2 +- ...d-Debian9-Clang-x86_64-Release-NoDEPS.json | 2 +- modules/skshaper/BUILD.gn | 38 ++-- modules/skshaper/src/SkShaper_harfbuzz.cpp | 17 ++ third_party/harfbuzz/BUILD.gn | 14 +- third_party/icu/BUILD.gn | 210 +++++++++--------- 9 files changed, 157 insertions(+), 132 deletions(-) diff --git a/experimental/canvaskit/compile.sh b/experimental/canvaskit/compile.sh index 78f1c3fc0c..66c418f607 100755 --- a/experimental/canvaskit/compile.sh +++ b/experimental/canvaskit/compile.sh @@ -110,13 +110,13 @@ else --align 4 fi -GN_SHAPER="skia_use_icu=true skia_use_system_icu=false" +GN_SHAPER="skia_use_icu=true skia_use_system_icu=false skia_use_system_harfbuzz=false" SHAPER_LIB="$BUILD_DIR/libharfbuzz.a \ $BUILD_DIR/libicu.a" SHAPER_TARGETS="libharfbuzz.a libicu.a" if [[ $@ == *primitive_shaper* ]]; then echo "Using the primitive shaper instead of the harfbuzz/icu one" - GN_SHAPER="skia_use_icu=false" + GN_SHAPER="skia_use_icu=false skia_use_harfbuzz=false" SHAPER_LIB="" SHAPER_TARGETS="" fi diff --git a/gn/skia.gni b/gn/skia.gni index d65074a012..e4d5fe5af0 100644 --- a/gn/skia.gni +++ b/gn/skia.gni @@ -12,6 +12,7 @@ declare_args() { skia_enable_skshaper = true skia_enable_tools = is_skia_dev_build skia_use_icu = !is_fuchsia && !is_ios + skia_use_harfbuzz = true } # Our tools require static linking (they use non-exported symbols), and the GPU backend. diff --git a/infra/bots/recipe_modules/build/default.py b/infra/bots/recipe_modules/build/default.py index c4ed4c0456..2877cb78d3 100644 --- a/infra/bots/recipe_modules/build/default.py +++ b/infra/bots/recipe_modules/build/default.py @@ -197,6 +197,7 @@ def compile_fn(api, checkout_root, out_dir): 'skia_enable_pdf': 'false', 'skia_use_expat': 'false', 'skia_use_freetype': 'false', + 'skia_use_harfbuzz': 'false', 'skia_use_libjpeg_turbo': 'false', 'skia_use_libpng': 'false', 'skia_use_libwebp': 'false', diff --git a/infra/bots/recipe_modules/build/examples/full.expected/Build-Debian9-Clang-x86_64-Release-NoDEPS.json b/infra/bots/recipe_modules/build/examples/full.expected/Build-Debian9-Clang-x86_64-Release-NoDEPS.json index 3637071ac3..2745b03969 100644 --- a/infra/bots/recipe_modules/build/examples/full.expected/Build-Debian9-Clang-x86_64-Release-NoDEPS.json +++ b/infra/bots/recipe_modules/build/examples/full.expected/Build-Debian9-Clang-x86_64-Release-NoDEPS.json @@ -32,7 +32,7 @@ "[START_DIR]/cache/work/skia/bin/gn", "gen", "[START_DIR]/cache/work/skia/out/Build-Debian9-Clang-x86_64-Release-NoDEPS/Release", - "--args=cc=\"[START_DIR]/clang_linux/bin/clang\" cxx=\"[START_DIR]/clang_linux/bin/clang++\" extra_cflags=[\"-B[START_DIR]/clang_linux/bin\", \"-DDUMMY_clang_linux_version=42\"] extra_ldflags=[\"-B[START_DIR]/clang_linux/bin\", \"-fuse-ld=lld\"] is_debug=false is_official_build=true skia_enable_fontmgr_empty=true skia_enable_gpu=true skia_enable_pdf=false skia_use_expat=false skia_use_freetype=false skia_use_libjpeg_turbo=false skia_use_libpng=false skia_use_libwebp=false skia_use_vulkan=false skia_use_zlib=false target_cpu=\"x86_64\"" + "--args=cc=\"[START_DIR]/clang_linux/bin/clang\" cxx=\"[START_DIR]/clang_linux/bin/clang++\" extra_cflags=[\"-B[START_DIR]/clang_linux/bin\", \"-DDUMMY_clang_linux_version=42\"] extra_ldflags=[\"-B[START_DIR]/clang_linux/bin\", \"-fuse-ld=lld\"] is_debug=false is_official_build=true skia_enable_fontmgr_empty=true skia_enable_gpu=true skia_enable_pdf=false skia_use_expat=false skia_use_freetype=false skia_use_harfbuzz=false skia_use_libjpeg_turbo=false skia_use_libpng=false skia_use_libwebp=false skia_use_vulkan=false skia_use_zlib=false target_cpu=\"x86_64\"" ], "cwd": "[START_DIR]/cache/work/skia", "env": { diff --git a/infra/bots/recipes/compile.expected/Build-Debian9-Clang-x86_64-Release-NoDEPS.json b/infra/bots/recipes/compile.expected/Build-Debian9-Clang-x86_64-Release-NoDEPS.json index 35e8575284..f2acecd7b2 100644 --- a/infra/bots/recipes/compile.expected/Build-Debian9-Clang-x86_64-Release-NoDEPS.json +++ b/infra/bots/recipes/compile.expected/Build-Debian9-Clang-x86_64-Release-NoDEPS.json @@ -112,7 +112,7 @@ "[START_DIR]/skia/bin/gn", "gen", "[START_DIR]/skia/out/Build-Debian9-Clang-x86_64-Release-NoDEPS/Release", - "--args=cc=\"[START_DIR]/clang_linux/bin/clang\" cxx=\"[START_DIR]/clang_linux/bin/clang++\" extra_cflags=[\"-B[START_DIR]/clang_linux/bin\", \"-DDUMMY_clang_linux_version=42\"] extra_ldflags=[\"-B[START_DIR]/clang_linux/bin\", \"-fuse-ld=lld\"] is_debug=false is_official_build=true skia_enable_fontmgr_empty=true skia_enable_gpu=true skia_enable_pdf=false skia_use_expat=false skia_use_freetype=false skia_use_libjpeg_turbo=false skia_use_libpng=false skia_use_libwebp=false skia_use_vulkan=false skia_use_zlib=false target_cpu=\"x86_64\"" + "--args=cc=\"[START_DIR]/clang_linux/bin/clang\" cxx=\"[START_DIR]/clang_linux/bin/clang++\" extra_cflags=[\"-B[START_DIR]/clang_linux/bin\", \"-DDUMMY_clang_linux_version=42\"] extra_ldflags=[\"-B[START_DIR]/clang_linux/bin\", \"-fuse-ld=lld\"] is_debug=false is_official_build=true skia_enable_fontmgr_empty=true skia_enable_gpu=true skia_enable_pdf=false skia_use_expat=false skia_use_freetype=false skia_use_harfbuzz=false skia_use_libjpeg_turbo=false skia_use_libpng=false skia_use_libwebp=false skia_use_vulkan=false skia_use_zlib=false target_cpu=\"x86_64\"" ], "cwd": "[START_DIR]/skia", "env": { diff --git a/modules/skshaper/BUILD.gn b/modules/skshaper/BUILD.gn index 4330d00cfe..1a55f3947f 100644 --- a/modules/skshaper/BUILD.gn +++ b/modules/skshaper/BUILD.gn @@ -6,31 +6,27 @@ import("../../gn/skia.gni") config("public_config") { - if (skia_enable_skshaper) { - include_dirs = [ "include" ] - defines = [] - if (skia_use_icu) { - defines += [ "SK_SHAPER_HARFBUZZ_AVAILABLE" ] - } + include_dirs = [ "include" ] + defines = [] + if (skia_use_icu) { + defines += [ "SK_SHAPER_HARFBUZZ_AVAILABLE" ] } } component("skshaper") { - if (skia_enable_skshaper) { - import("skshaper.gni") - public_configs = [ ":public_config" ] - public = skia_shaper_public - deps = [ - "../..:skia", + import("skshaper.gni") + public_configs = [ ":public_config" ] + public = skia_shaper_public + deps = [ + "../..:skia", + ] + sources = skia_shaper_primitive_sources + if (skia_use_icu && skia_use_harfbuzz) { + sources += skia_shaper_harfbuzz_sources + deps += [ + "//third_party/harfbuzz", + "//third_party/icu", ] - sources = skia_shaper_primitive_sources - if (skia_use_icu) { - sources += skia_shaper_harfbuzz_sources - deps += [ - "//third_party/harfbuzz", - "//third_party/icu", - ] - } - configs += [ "../../:skia_private" ] } + configs += [ "../../:skia_private" ] } diff --git a/modules/skshaper/src/SkShaper_harfbuzz.cpp b/modules/skshaper/src/SkShaper_harfbuzz.cpp index b2e7116cbd..b5ff497b04 100644 --- a/modules/skshaper/src/SkShaper_harfbuzz.cpp +++ b/modules/skshaper/src/SkShaper_harfbuzz.cpp @@ -215,15 +215,28 @@ hb_bool_t skhb_glyph_extents(hb_font_t* hb_font, return true; } +#define SK_HB_VERSION_CHECK(x, y, z) \ + (HB_VERSION_MAJOR > (x)) || \ + (HB_VERSION_MAJOR == (x) && HB_VERSION_MINOR > (y)) || \ + (HB_VERSION_MAJOR == (x) && HB_VERSION_MINOR == (y) && HB_VERSION_MICRO >= (z)) + hb_font_funcs_t* skhb_get_font_funcs() { static hb_font_funcs_t* const funcs = []{ // HarfBuzz will use the default (parent) implementation if they aren't set. hb_font_funcs_t* const funcs = hb_font_funcs_create(); hb_font_funcs_set_variation_glyph_func(funcs, skhb_glyph, nullptr, nullptr); hb_font_funcs_set_nominal_glyph_func(funcs, skhb_nominal_glyph, nullptr, nullptr); +#if SK_HB_VERSION_CHECK(2, 0, 0) hb_font_funcs_set_nominal_glyphs_func(funcs, skhb_nominal_glyphs, nullptr, nullptr); +#else + sk_ignore_unused_variable(skhb_nominal_glyphs); +#endif hb_font_funcs_set_glyph_h_advance_func(funcs, skhb_glyph_h_advance, nullptr, nullptr); +#if SK_HB_VERSION_CHECK(1, 8, 6) hb_font_funcs_set_glyph_h_advances_func(funcs, skhb_glyph_h_advances, nullptr, nullptr); +#else + sk_ignore_unused_variable(skhb_glyph_h_advances); +#endif hb_font_funcs_set_glyph_extents_func(funcs, skhb_glyph_extents, nullptr, nullptr); hb_font_funcs_make_immutable(funcs); return funcs; @@ -1393,7 +1406,11 @@ ShapedRun SkShaperHarfBuzz::shape(const char* utf8, SkPaint p; run.fFont.getWidthsBounds(&glyph.fID, 1, &advance, &bounds, &p); glyph.fHasVisual = !bounds.isEmpty(); //!font->currentTypeface()->glyphBoundsAreZero(glyph.fID); +#if SK_HB_VERSION_CHECK(1, 5, 0) glyph.fUnsafeToBreak = info[i].mask & HB_GLYPH_FLAG_UNSAFE_TO_BREAK; +#else + glyph.fUnsafeToBreak = false; +#endif glyph.fMustLineBreakBefore = false; runAdvance += glyph.fAdvance; diff --git a/third_party/harfbuzz/BUILD.gn b/third_party/harfbuzz/BUILD.gn index 4cb5df2622..f7b7ad9440 100644 --- a/third_party/harfbuzz/BUILD.gn +++ b/third_party/harfbuzz/BUILD.gn @@ -3,13 +3,19 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. -declare_args() { -} - import("../../gn/skia.gni") import("../third_party.gni") -if (skia_use_icu) { +declare_args() { + skia_use_system_harfbuzz = is_official_build +} + +if (skia_use_system_harfbuzz) { + system("harfbuzz") { + include_dirs = [ "/usr/include/harfbuzz" ] + libs = [ "harfbuzz" ] + } +} else { third_party("harfbuzz") { _src = "../externals/harfbuzz/src" public_include_dirs = [ diff --git a/third_party/icu/BUILD.gn b/third_party/icu/BUILD.gn index 5d3bdc1f8d..50157f0bf1 100644 --- a/third_party/icu/BUILD.gn +++ b/third_party/icu/BUILD.gn @@ -3,120 +3,124 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. -declare_args() { - skia_use_system_icu = is_official_build -} - import("../../gn/skia.gni") import("../third_party.gni") import("icu.gni") -if (skia_use_icu) { - if (skia_use_system_icu) { - system("icu") { - libs = [ "icuuc" ] - defines = [ "U_USING_ICU_NAMESPACE=0" ] - } +declare_args() { + skia_use_system_icu = is_official_build +} + +if (skia_use_system_icu) { + system("icu") { + libs = [ "icuuc" ] + defines = [ "U_USING_ICU_NAMESPACE=0" ] + } +} else { + if (target_cpu == "wasm") { + data_assembly = "$target_gen_dir/icudtl_dat.cpp" } else { + data_assembly = "$target_gen_dir/icudtl_dat.S" + } + data_dir = "../externals/icu/" + if (target_cpu == "wasm") { + # Use a super super super stripped down version for wasm, + # which is the same thing flutter is using. + data_dir += "flutter" + } else if (is_android) { + data_dir += "android" + } else if (is_ios) { + data_dir += "ios" + } else { + data_dir += "common" + } + action("make_data_assembly") { if (target_cpu == "wasm") { - data_assembly = "$target_gen_dir/icudtl_dat.cpp" + _u_icu_version_major_num = "63" # defined in source/common/unicode/uvernum.h + script = "make_data_cpp.py" + inputs = [ + "$data_dir/icudtl.dat", + ] + outputs = [ + data_assembly, + ] + args = [ + "icudt${_u_icu_version_major_num}_dat", + rebase_path(inputs[0], root_build_dir), + rebase_path(data_assembly, root_build_dir), + ] } else { - data_assembly = "$target_gen_dir/icudtl_dat.S" - } - data_dir = "../externals/icu/" - if (target_cpu == "wasm") { - # Use a super super super stripped down version for wasm, - # which is the same thing flutter is using. - data_dir += "flutter" - } else if (is_android) { - data_dir += "android" - } else if (is_ios) { - data_dir += "ios" - } else { - data_dir += "common" - } - action("make_data_assembly") { - if (target_cpu == "wasm") { - _u_icu_version_major_num = "63" # defined in source/common/unicode/uvernum.h - script = "make_data_cpp.py" - inputs = [ - "$data_dir/icudtl.dat", - ] - outputs = [ - data_assembly, - ] - args = [ - "icudt${_u_icu_version_major_num}_dat", - rebase_path(inputs[0], root_build_dir), - rebase_path(data_assembly, root_build_dir), - ] - } else { - script = "../externals/icu/scripts/make_data_assembly.py" - inputs = [ - "$data_dir/icudtl.dat", - ] - outputs = [ - "$data_assembly", - ] - args = [ - rebase_path(inputs[0], root_build_dir), - rebase_path(data_assembly, root_build_dir), - ] - if (is_mac || is_ios) { - args += [ "--mac" ] - } - } - } - - third_party("icu") { - public_include_dirs = [ - "../externals/icu/source/common", - "../externals/icu/source/i18n", - ".", + script = "../externals/icu/scripts/make_data_assembly.py" + inputs = [ + "$data_dir/icudtl.dat", ] - public_defines = [ - "U_USING_ICU_NAMESPACE=0", - "SK_USING_THIRD_PARTY_ICU", + outputs = [ + "$data_assembly", ] - configs -= [ "//gn:no_rtti" ] - defines = [ - # http://userguide.icu-project.org/howtouseicu - "U_COMMON_IMPLEMENTATION", - "U_STATIC_IMPLEMENTATION", - "U_ENABLE_DYLOAD=0", - "U_I18N_IMPLEMENTATION", + args = [ + rebase_path(inputs[0], root_build_dir), + rebase_path(data_assembly, root_build_dir), ] - if (target_cpu == "wasm") { - # Tell ICU that we are a 32 bit platform, otherwise, - # double-conversion-utils.h doesn't know how to operate. - defines += [ "__i386__" ] + if (is_mac || is_ios) { + args += [ "--mac" ] } - sources = icu_sources - if (is_win) { - deps = [ - ":icudata", - ] - public_defines += [ - "U_NOEXCEPT=", - "U_STATIC_IMPLEMENTATION", - ] - libs = [ "Advapi32.lib" ] - sources += [ - "../externals/icu/source/stubdata/stubdata.cpp", - "SkLoadICU.cpp", - ] - } else { - sources += [ "$data_assembly" ] - deps = [ - ":make_data_assembly", - ] - } - } - - copy("icudata") { - sources = [ "../externals/icu/common/icudtl.dat" ] - outputs = [ "$root_out_dir/icudtl.dat" ] - data = [ "$root_out_dir/icudtl.dat" ] } } + + third_party("icu") { + public_include_dirs = [ + "../externals/icu/source/common", + "../externals/icu/source/i18n", + ".", + ] + public_defines = [ + "U_USING_ICU_NAMESPACE=0", + "SK_USING_THIRD_PARTY_ICU", + ] + configs -= [ "//gn:no_rtti" ] + defines = [ + # http://userguide.icu-project.org/howtouseicu + "U_COMMON_IMPLEMENTATION", + "U_STATIC_IMPLEMENTATION", + "U_ENABLE_DYLOAD=0", + "U_I18N_IMPLEMENTATION", + ] + if (target_cpu == "wasm") { + # Tell ICU that we are a 32 bit platform, otherwise, + # double-conversion-utils.h doesn't know how to operate. + defines += [ "__i386__" ] + } + sources = icu_sources + if (is_win) { + deps = [ + ":icudata", + ] + public_defines += [ + "U_NOEXCEPT=", + "U_STATIC_IMPLEMENTATION", + ] + libs = [ "Advapi32.lib" ] + sources += [ + "../externals/icu/source/stubdata/stubdata.cpp", + "SkLoadICU.cpp", + ] + } else { + sources += [ "$data_assembly" ] + deps = [ + ":make_data_assembly", + ] + } + } + + copy("icudata") { + sources = [ + "../externals/icu/common/icudtl.dat", + ] + outputs = [ + "$root_out_dir/icudtl.dat", + ] + data = [ + "$root_out_dir/icudtl.dat", + ] + } }