configure.ac | 12 +++---------
src/hb-ft.cc | 40 +++++++++++++++++++++++++++++++++-------
src/hb-ft.h | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 94 insertions(+), 17 deletions(-)
New commits:
commit 350f3a02ce225e5d78db8ac96de1351ff9f96dd5
Author: Behdad Esfahbod <beh...@behdad.org>
Date: Sun Dec 28 17:44:26 2014 -0800
[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
--- 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
--- 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
--- 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. */
commit 9a3b74884b2e41c7040611030f4336f13d18fd3e
Author: Behdad Esfahbod <beh...@behdad.org>
Date: Sun Dec 28 17:27:39 2014 -0800
Remove redundant check for FT_Face_GetCharVariantIndex
We require FreeType >= 2.8.3. This symbol was introduced earlier
than that.
diff --git a/configure.ac b/configure.ac
index bf246ee..8f894a8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -285,7 +285,7 @@ if test "x$with_graphite2" = "xyes" -a "x$have_graphite2"
!= "xtrue"; then
AC_MSG_ERROR([graphite2 support requested but libgraphite2 not found])
fi
if $have_graphite2; then
- AC_DEFINE(HAVE_GRAPHITE2, 1, [Have Graphite2 library])
+ AC_DEFINE(HAVE_GRAPHITE2, 1, [Have Graphite2 library])
fi
AM_CONDITIONAL(HAVE_GRAPHITE2, $have_graphite2)
@@ -305,13 +305,6 @@ if test "x$with_freetype" = "xyes" -a "x$have_freetype" !=
"xtrue"; then
fi
if $have_freetype; then
AC_DEFINE(HAVE_FREETYPE, 1, [Have FreeType 2 library])
- _save_libs="$LIBS"
- _save_cflags="$CFLAGS"
- LIBS="$LIBS $FREETYPE_LIBS"
- CFLAGS="$CFLAGS $FREETYPE_CFLAGS"
- AC_CHECK_FUNCS(FT_Face_GetCharVariantIndex)
- LIBS="$_save_libs"
- CFLAGS="$_save_cflags"
fi
AM_CONDITIONAL(HAVE_FREETYPE, $have_freetype)
diff --git a/src/hb-ft.cc b/src/hb-ft.cc
index c3b58f8..0e21573 100644
--- a/src/hb-ft.cc
+++ b/src/hb-ft.cc
@@ -70,12 +70,10 @@ hb_ft_get_glyph (hb_font_t *font HB_UNUSED,
{
FT_Face ft_face = (FT_Face) font_data;
-#ifdef HAVE_FT_FACE_GETCHARVARIANTINDEX
if (unlikely (variation_selector)) {
*glyph = FT_Face_GetCharVariantIndex (ft_face, unicode,
variation_selector);
return *glyph != 0;
}
-#endif
*glyph = FT_Get_Char_Index (ft_face, unicode);
return *glyph != 0;
commit 1226b2e930aa456cc05bbe621c96f4286a95cff6
Author: Behdad Esfahbod <beh...@behdad.org>
Date: Sun Dec 28 17:04:23 2014 -0800
Fix FreeType version check
diff --git a/configure.ac b/configure.ac
index ec41748..bf246ee 100644
--- a/configure.ac
+++ b/configure.ac
@@ -297,7 +297,8 @@ AC_ARG_WITH(freetype,
[with_freetype=auto])
have_freetype=false
if test "x$with_freetype" = "xyes" -o "x$with_freetype" = "xauto"; then
- PKG_CHECK_MODULES(FREETYPE, freetype2 >= 2.3.8, have_freetype=true, :)
+ # See freetype/docs/VERSION.DLL; 9.19.13 means freetype-2.3.8
+ PKG_CHECK_MODULES(FREETYPE, freetype2 >= 9.19.3, 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])
commit affacf2f37db767ab8df7f2db6cd9e0e9b0a2b8a
Author: Behdad Esfahbod <beh...@behdad.org>
Date: Sun Dec 28 16:20:31 2014 -0800
[ft] Open blob in READONLY mode
HB_MEMORY_MODE_READONLY_MAY_MAKE_WRITABLE is deprecated and fairly
useless now.
diff --git a/src/hb-ft.cc b/src/hb-ft.cc
index c42d484..c3b58f8 100644
--- a/src/hb-ft.cc
+++ b/src/hb-ft.cc
@@ -340,11 +340,7 @@ hb_ft_face_create (FT_Face ft_face,
blob = hb_blob_create ((const char *) ft_face->stream->base,
(unsigned int) ft_face->stream->size,
- /* TODO: We assume that it's mmap()'ed, but FreeType
code
- * suggests that there are cases we reach here but
font is
- * not mmapped. For example, when mmap() fails. No
idea
- * how to deal with it better here. */
- HB_MEMORY_MODE_READONLY_MAY_MAKE_WRITABLE,
+ HB_MEMORY_MODE_READONLY,
ft_face, destroy);
face = hb_face_create (blob, ft_face->face_index);
hb_blob_destroy (blob);
_______________________________________________
HarfBuzz mailing list
HarfBuzz@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/harfbuzz
