[ft] Add hb_ft_face_create_referenced() and hb_ft_font_create_referenced()

When I originally wrote hb-ft, FreeType objects did not support reference
counting.  As such, hb_ft_face_create() and hb_ft_font_create() had a
"destroy" callback and client was responsible for making sure FT_Face is
kept around as long as the hb-font/face are alive.

However, since this was not clearly documented, some clienets didn't
correctly did that.  In particular, some clients assumed that it's safe
to destroy FT_Face and then hb_face_t.  This, indeed, used to work, until
45fd9424c723f115ca98995b8f8a25185a6fc71d, which make face destroy access
font tables.

Now, I fixed that issue in 395b35903e052aecc97d0807e4f813c64c0d2b0b since
the access was not needed, but the problem remains that not all clients
handle this correctly.  See:

  https://bugs.freedesktop.org/show_bug.cgi?id=86300

Fortunately, FT_Reference_Face() was added to FreeType in 2010, and so we
can use it now.  Originally I wanted to change hb_ft_face_create() and
hb_ft_font_create() to reference the face if destroy==NULL was passed in.
That would improve pretty much all clients, with little undesired effects.
Except that FreeType itself, when compiled with HarfBuzz support, calls
hb_ft_font_create() with destroy==NULL and saves the resulting hb-font on
the ft-face (why does it not free it immediately?).  Making hb-face
reference ft-face causes a cycling reference there.  At least, that's my
current understanding.

At any rate, a cleaner approach, even if it means all clients will need a
change, is to introduce brand new API.  Which this commit does.

Some comments added to hb-ft.h, hoping to make future clients make better
choices.

Fixes https://bugs.freedesktop.org/show_bug.cgi?id=75299

diff --git a/configure.ac b/configure.ac
index 8f894a8..4dc562c 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -297,8 +297,8 @@ AC_ARG_WITH(freetype,
        [with_freetype=auto])
 have_freetype=false
 if test "x$with_freetype" = "xyes" -o "x$with_freetype" = "xauto"; then
-       # See freetype/docs/VERSION.DLL; 9.19.13 means freetype-2.3.8
-       PKG_CHECK_MODULES(FREETYPE, freetype2 >= 9.19.3, have_freetype=true, :)
+       # See freetype/docs/VERSION.DLL; 12.0.6 means freetype-2.4.2
+       PKG_CHECK_MODULES(FREETYPE, freetype2 >= 12.0.6, have_freetype=true, :)
 fi
 if test "x$with_freetype" = "xyes" -a "x$have_freetype" != "xtrue"; then
        AC_MSG_ERROR([FreeType support requested but libfreetype2 not found])

diff --git a/src/hb-ft.cc b/src/hb-ft.cc
index 0e21573..9b2a908 100644 (file)
--- a/src/hb-ft.cc
+++ b/src/hb-ft.cc
@@ -352,6 +352,22 @@ hb_ft_face_create (FT_Face           ft_face,
   return face;
 }
 
+/**
+ * hb_ft_face_create_referenced:
+ * @ft_face:
+ *
+ * 
+ *
+ * Return value: (transfer full): 
+ * Since: 1.0
+ **/
+hb_face_t *
+hb_ft_face_create_referenced (FT_Face ft_face)
+{
+  FT_Reference_Face (ft_face);
+  return hb_ft_face_create (ft_face, (hb_destroy_func_t) FT_Done_Face);
+}
+
 static void
 hb_ft_face_finalize (FT_Face ft_face)
 {
@@ -421,6 +437,22 @@ hb_ft_font_create (FT_Face           ft_face,
   return font;
 }
 
+/**
+ * hb_ft_font_create_referenced:
+ * @ft_face:
+ *
+ * 
+ *
+ * Return value: (transfer full): 
+ * Since: 1.0
+ **/
+hb_font_t *
+hb_ft_font_create_referenced (FT_Face ft_face)
+{
+  FT_Reference_Face (ft_face);
+  return hb_ft_font_create (ft_face, (hb_destroy_func_t) FT_Done_Face);
+}
+
 
 /* Thread-safe, lock-free, FT_Library */
 

diff --git a/src/hb-ft.h b/src/hb-ft.h
index 696251e..92f4b36 100644 (file)
--- a/src/hb-ft.h
+++ b/src/hb-ft.h
@@ -34,19 +34,76 @@
 
 HB_BEGIN_DECLS
 
-/* Note: FreeType is not thread-safe.  Hence, these functions are not either. */
+/*
+ * Note: FreeType is not thread-safe.
+ * Hence, these functions are not either.
+ */
+
+/*
+ * hb-face from ft-face.
+ */
 
+/* This one creates a new hb-face for given ft-face.
+ * When the returned hb-face is destroyed, the destroy
+ * callback is called (if not NULL), with the ft-face passed
+ * to it.
+ *
+ * The client is responsible to make sure that ft-face is
+ * destroyed after hb-face is destroyed.
+ *
+ * Most often you don't want this function.  You should use either
+ * hb_ft_face_create_cached(), or hb_ft_face_create_referenced().
+ * In particular, if you are going to pass NULL as destroy, you
+ * probably should use (the more recent) hb_ft_face_create_referenced()
+ * instead.
+ */
 hb_face_t *
 hb_ft_face_create (FT_Face           ft_face,
                   hb_destroy_func_t destroy);
 
+/* This version is like hb_ft_face_create(), except that it caches
+ * the hb-face using the generic pointer of the ft-face.  This means
+ * that subsequent calls to this function with the same ft-face will
+ * return the same hb-face (correctly referenced).
+ *
+ * Client is still responsible for making sure that ft-face is destroyed
+ * after hb-face is.
+ */
 hb_face_t *
 hb_ft_face_create_cached (FT_Face ft_face);
 
+/* This version is like hb_ft_face_create(), except that it calls
+ * FT_Reference_Face() on ft-face, as such keeping ft-face alive
+ * as long as the hb-face is.
+ *
+ * This is the most convenient version to use.  Use it unless you have
+ * very good reasons not to.
+ */
+hb_face_t *
+hb_ft_face_create_referenced (FT_Face ft_face);
+
+
+/*
+ * hb-font from ft-face.
+ */
+
+/*
+ * Note:
+ *
+ * Set face size on ft-face before creating hb-font from it.
+ * Otherwise hb-ft would NOT pick up the font size correctly.
+ */
+
+/* See notes on hb_ft_face_create().  Same issues re lifecycle-management
+ * apply here.  Use hb_ft_font_create_referenced() if you can. */
 hb_font_t *
 hb_ft_font_create (FT_Face           ft_face,
                   hb_destroy_func_t destroy);
 
+/* See notes on hb_ft_face_create_referenced() re lifecycle-management
+ * issues. */
+hb_font_t *
+hb_ft_font_create_referenced (FT_Face ft_face);
 
 
 /* Makes an hb_font_t use FreeType internally to implement font functions. */