[HarfBuzz] harfbuzz: Branch 'master' - 2 commits

Behdad Esfahbod behdad at kemper.freedesktop.org
Tue Oct 9 20:26:10 UTC 2018


 src/hb-ft.cc                |   25 ++++++++++++++++++-------
 src/hb-mutex.hh             |    8 ++++++++
 test/api/test-multithread.c |    7 ++-----
 3 files changed, 28 insertions(+), 12 deletions(-)

New commits:
commit be2f148da474d6dd30132c22dd467ea33a942edf
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Tue Oct 9 16:24:50 2018 -0400

    [ft] Use mutex to lock access to FT_Face
    
    Makes our FT-backed hb_font_t safe to use from multiple threads.  Still,
    the underlying FT_Face should NOT be used from other threads by client
    or other libraries.
    
    Maybe I add a lock()/unlock() public API ala PangoFT2 and cairo-ft.
    Maybe not.

diff --git a/src/hb-ft.cc b/src/hb-ft.cc
index 90e8073b..18fb72a7 100644
--- a/src/hb-ft.cc
+++ b/src/hb-ft.cc
@@ -60,6 +60,7 @@
 
 struct hb_ft_font_t
 {
+  mutable hb_mutex_t lock;
   FT_Face ft_face;
   int load_flags;
   bool symbol; /* Whether selected cmap is symbol cmap. */
@@ -77,6 +78,7 @@ _hb_ft_font_create (FT_Face ft_face, bool symbol, bool unref)
   if (unlikely (!ft_font))
     return nullptr;
 
+  ft_font->lock.init ();
   ft_font->ft_face = ft_face;
   ft_font->symbol = symbol;
   ft_font->unref = unref;
@@ -105,6 +107,8 @@ _hb_ft_font_destroy (void *data)
   if (ft_font->unref)
     _hb_ft_face_destroy (ft_font->ft_face);
 
+  ft_font->lock.fini ();
+
   free (ft_font);
 }
 
@@ -172,6 +176,7 @@ hb_ft_get_nominal_glyph (hb_font_t *font HB_UNUSED,
 			 void *user_data HB_UNUSED)
 {
   const hb_ft_font_t *ft_font = (const hb_ft_font_t *) font_data;
+  hb_lock_t lock (ft_font->lock);
   unsigned int g = FT_Get_Char_Index (ft_font->ft_face, unicode);
 
   if (unlikely (!g))
@@ -206,6 +211,7 @@ hb_ft_get_nominal_glyphs (hb_font_t *font HB_UNUSED,
 			  void *user_data HB_UNUSED)
 {
   const hb_ft_font_t *ft_font = (const hb_ft_font_t *) font_data;
+  hb_lock_t lock (ft_font->lock);
   unsigned int done;
   for (done = 0;
        done < count && (*first_glyph = FT_Get_Char_Index (ft_font->ft_face, *first_unicode));
@@ -229,6 +235,7 @@ hb_ft_get_variation_glyph (hb_font_t *font HB_UNUSED,
 			   void *user_data HB_UNUSED)
 {
   const hb_ft_font_t *ft_font = (const hb_ft_font_t *) font_data;
+  hb_lock_t lock (ft_font->lock);
   unsigned int g = FT_Face_GetCharVariantIndex (ft_font->ft_face, unicode, variation_selector);
 
   if (unlikely (!g))
@@ -248,6 +255,7 @@ hb_ft_get_glyph_h_advances (hb_font_t* font, void* font_data,
 			    void *user_data HB_UNUSED)
 {
   const hb_ft_font_t *ft_font = (const hb_ft_font_t *) font_data;
+  hb_lock_t lock (ft_font->lock);
   FT_Face ft_face = ft_font->ft_face;
   int load_flags = ft_font->load_flags;
   int mult = font->x_scale < 0 ? -1 : +1;
@@ -285,6 +293,7 @@ hb_ft_get_glyph_v_advance (hb_font_t *font,
 			   void *user_data HB_UNUSED)
 {
   const hb_ft_font_t *ft_font = (const hb_ft_font_t *) font_data;
+  hb_lock_t lock (ft_font->lock);
   FT_Fixed v;
 
   if (unlikely (FT_Get_Advance (ft_font->ft_face, glyph, ft_font->load_flags | FT_LOAD_VERTICAL_LAYOUT, &v)))
@@ -307,6 +316,7 @@ hb_ft_get_glyph_v_origin (hb_font_t *font,
 			  void *user_data HB_UNUSED)
 {
   const hb_ft_font_t *ft_font = (const hb_ft_font_t *) font_data;
+  hb_lock_t lock (ft_font->lock);
   FT_Face ft_face = ft_font->ft_face;
 
   if (unlikely (FT_Load_Glyph (ft_face, glyph, ft_font->load_flags)))
@@ -333,6 +343,7 @@ hb_ft_get_glyph_extents (hb_font_t *font,
 			 void *user_data HB_UNUSED)
 {
   const hb_ft_font_t *ft_font = (const hb_ft_font_t *) font_data;
+  hb_lock_t lock (ft_font->lock);
   FT_Face ft_face = ft_font->ft_face;
 
   if (unlikely (FT_Load_Glyph (ft_face, glyph, ft_font->load_flags)))
@@ -365,6 +376,7 @@ hb_ft_get_glyph_contour_point (hb_font_t *font HB_UNUSED,
 			       void *user_data HB_UNUSED)
 {
   const hb_ft_font_t *ft_font = (const hb_ft_font_t *) font_data;
+  hb_lock_t lock (ft_font->lock);
   FT_Face ft_face = ft_font->ft_face;
 
   if (unlikely (FT_Load_Glyph (ft_face, glyph, ft_font->load_flags)))
@@ -390,8 +402,10 @@ hb_ft_get_glyph_name (hb_font_t *font HB_UNUSED,
 		      void *user_data HB_UNUSED)
 {
   const hb_ft_font_t *ft_font = (const hb_ft_font_t *) font_data;
+  hb_lock_t lock (ft_font->lock);
+  FT_Face ft_face = ft_font->ft_face;
 
-  hb_bool_t ret = !FT_Get_Glyph_Name (ft_font->ft_face, glyph, name, size);
+  hb_bool_t ret = !FT_Get_Glyph_Name (ft_face, glyph, name, size);
   if (ret && (size && !*name))
     ret = false;
 
@@ -406,6 +420,7 @@ hb_ft_get_glyph_from_name (hb_font_t *font HB_UNUSED,
 			   void *user_data HB_UNUSED)
 {
   const hb_ft_font_t *ft_font = (const hb_ft_font_t *) font_data;
+  hb_lock_t lock (ft_font->lock);
   FT_Face ft_face = ft_font->ft_face;
 
   if (len < 0)
@@ -438,6 +453,7 @@ hb_ft_get_font_h_extents (hb_font_t *font HB_UNUSED,
 			  void *user_data HB_UNUSED)
 {
   const hb_ft_font_t *ft_font = (const hb_ft_font_t *) font_data;
+  hb_lock_t lock (ft_font->lock);
   FT_Face ft_face = ft_font->ft_face;
   metrics->ascender = ft_face->size->metrics.ascender;
   metrics->descender = ft_face->size->metrics.descender;
diff --git a/src/hb-mutex.hh b/src/hb-mutex.hh
index d8cdf4b6..c49d7a9e 100644
--- a/src/hb-mutex.hh
+++ b/src/hb-mutex.hh
@@ -137,5 +137,13 @@ struct hb_mutex_t
   inline void fini (void) { hb_mutex_impl_finish (&m); }
 };
 
+struct hb_lock_t
+{
+  hb_lock_t (hb_mutex_t &mutex_) : mutex (mutex_) { mutex.lock (); }
+  ~hb_lock_t (void) { mutex.unlock (); }
+  private:
+  hb_mutex_t &mutex;
+};
+
 
 #endif /* HB_MUTEX_HH */
diff --git a/test/api/test-multithread.c b/test/api/test-multithread.c
index af5e8f9a..779b762d 100644
--- a/test/api/test-multithread.c
+++ b/test/api/test-multithread.c
@@ -164,11 +164,8 @@ main (int argc, char **argv)
 
   test_body ();
 
-  /* hb-font backed by FreeType functions can only be used from
-   * one thread at a time, because that's FT_Face's MT guarantee.
-   * So, disable this, even though it works "most of the time". */
-  //hb_ft_font_set_funcs (font);
-  //test_body ();
+  hb_ft_font_set_funcs (font);
+  test_body ();
 
   hb_buffer_destroy (ref_buffer);
 
commit d18c3c5861d40291077eb8b8667dc2f12b649cf2
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Tue Oct 9 16:07:55 2018 -0400

    [ft] Remove (probably) stale comment

diff --git a/src/hb-ft.cc b/src/hb-ft.cc
index d293693c..90e8073b 100644
--- a/src/hb-ft.cc
+++ b/src/hb-ft.cc
@@ -45,21 +45,16 @@
  * In general, this file does a fine job of what it's supposed to do.
  * There are, however, things that need more work:
  *
- *   - I remember seeing FT_Get_Advance() without the NO_HINTING flag to be buggy.
- *     Have not investigated.
- *
  *   - FreeType works in 26.6 mode.  Clients can decide to use that mode, and everything
  *     would work fine.  However, we also abuse this API for performing in font-space,
  *     but don't pass the correct flags to FreeType.  We just abuse the no-hinting mode
  *     for that, such that no rounding etc happens.  As such, we don't set ppem, and
  *     pass NO_HINTING as load_flags.  Would be much better to use NO_SCALE, and scale
- *     ourselves, like we do in uniscribe, etc.
+ *     ourselves.
  *
  *   - We don't handle / allow for emboldening / obliqueing.
  *
  *   - In the future, we should add constructors to create fonts in font space?
- *
- *   - FT_Load_Glyph() is extremely costly.  Do something about it?
  */
 
 


More information about the HarfBuzz mailing list