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 <bungeman@google.com> Commit-Queue: Ben Wagner <bungeman@google.com>
flutter · Feb 23, 2019 · 867da4b · 867da4b
1 parent 12f1fec
commit 867da4b
Show file tree

Hide file tree

Showing 9 changed files with 154 additions and 129 deletions.
diff --git 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
@@ -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
@@ -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/...ecipe_modules/build/examples/full.expected/Build-Debian9-Clang-x86_64-Release-NoDEPS.json b/...ecipe_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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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"
-    } 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"
+      _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_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" ]
-        }
+      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",
-        ".",
-      ]
-      public_defines = [
-        "U_USING_ICU_NAMESPACE=0",
-        "SK_USING_THIRD_PARTY_ICU",
+  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",
       ]
-      configs -= [ "//gn:no_rtti" ]
-      defines = [
-        # http://userguide.icu-project.org/howtouseicu
-        "U_COMMON_IMPLEMENTATION",
+      public_defines += [
+        "U_NOEXCEPT=",
         "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",
-        ]
-      }
+      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" ]
-    }
+  copy("icudata") {
+    sources = [
+      "../externals/icu/common/icudtl.dat",
+    ]
+    outputs = [
+      "$root_out_dir/icudtl.dat",
+    ]
+    data = [
+      "$root_out_dir/icudtl.dat",
+    ]
   }
 }