[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
45fd9424c7, which make face destroy access
font tables.

Now, I fixed that issue in 395b35903e 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
This commit is contained in:
Behdad Esfahbod 2014-12-28 17:44:26 -08:00
parent 9a3b74884b
commit 350f3a02ce
3 changed files with 92 additions and 3 deletions

View File

@ -297,8 +297,8 @@ AC_ARG_WITH(freetype,
[with_freetype=auto]) [with_freetype=auto])
have_freetype=false have_freetype=false
if test "x$with_freetype" = "xyes" -o "x$with_freetype" = "xauto"; then 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 # See freetype/docs/VERSION.DLL; 12.0.6 means freetype-2.4.2
PKG_CHECK_MODULES(FREETYPE, freetype2 >= 9.19.3, have_freetype=true, :) PKG_CHECK_MODULES(FREETYPE, freetype2 >= 12.0.6, have_freetype=true, :)
fi fi
if test "x$with_freetype" = "xyes" -a "x$have_freetype" != "xtrue"; then if test "x$with_freetype" = "xyes" -a "x$have_freetype" != "xtrue"; then
AC_MSG_ERROR([FreeType support requested but libfreetype2 not found]) AC_MSG_ERROR([FreeType support requested but libfreetype2 not found])

View File

@ -352,6 +352,22 @@ hb_ft_face_create (FT_Face ft_face,
return 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 static void
hb_ft_face_finalize (FT_Face ft_face) hb_ft_face_finalize (FT_Face ft_face)
{ {
@ -421,6 +437,22 @@ hb_ft_font_create (FT_Face ft_face,
return font; 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 */ /* Thread-safe, lock-free, FT_Library */

View File

@ -34,19 +34,76 @@
HB_BEGIN_DECLS 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_face_t *
hb_ft_face_create (FT_Face ft_face, hb_ft_face_create (FT_Face ft_face,
hb_destroy_func_t destroy); 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_face_t *
hb_ft_face_create_cached (FT_Face ft_face); 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_font_t *
hb_ft_font_create (FT_Face ft_face, hb_ft_font_create (FT_Face ft_face,
hb_destroy_func_t destroy); 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. */ /* Makes an hb_font_t use FreeType internally to implement font functions. */