conditionals for older freetype2 without color font feature

Submitted by suzuki toshiya on April 11, 2018, 3:36 p.m.

Details

Message ID 5ACE2B78.7010603@hiroshima-u.ac.jp
State New
Series "conditionals for older freetype2 without color font feature"
Headers show

Commit Message

suzuki toshiya April 11, 2018, 3:36 p.m.
Dear Bryce,

Sorry for the lated response, I appreciate your effort to improve
my confusing comment...

Bryce Harrington wrote:
> /* Define to nothing if freetype2 supports color fonts */
> #define FT_HAS_COLOR(x) (0)
> 
> But actually we only want to define to (0) if freetype2 does *not*
> support color fonts, right?  "Define to nothing" might be a bit
> ambiguous though (does it mean undefine?  define to blank?  define to
> zero?)  So, my suggestion is for it to maybe look more like this:
> 
> /* Define to (0) if freetype2 does not support color fonts */
> #define FT_HAS_COLOR(x) (0)
> 
> What do you think?  I apologize for being nitpicky on a mere comment,
> but thank you for taking the extra time on this.

Please do not apologize! The comments making the developers
confused must be avoided...

Oh, the suggested comment is clearly better. I wonder why I could
not do like this (maybe "Define to XXX if we *have* YYY" style was
deeply hardwired in my brain). Here is the reworked patch.

Regards,
mpsuzuki

Patch hide | download patch | download mbox

diff --git a/configure.ac b/configure.ac
index d78b2ed..59d87eb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -580,6 +580,17 @@  if test "x$use_ft" = "xyes"; then
 
   AC_CHECK_FUNCS(FT_Get_X11_Font_Format FT_GlyphSlot_Embolden FT_GlyphSlot_Oblique FT_Load_Sfnt_Table FT_Library_SetLcdFilter FT_Get_Var_Design_Coordinates FT_Done_MM_Var)
 
+  AC_MSG_CHECKING(for FT_HAS_COLOR)
+  AC_LINK_IFELSE([AC_LANG_PROGRAM([
+#include <ft2build.h>
+#include FT_FREETYPE_H
+],[
+FT_Long has_color = FT_HAS_COLOR( ((FT_Face)NULL) );
+])],[AC_MSG_RESULT([yes])],[
+  AC_DEFINE([FT_HAS_COLOR(x)], [(0)], [Define to (0) if freetype2 does not support color fonts])
+  AC_MSG_RESULT([no, disable color font (freetype2 >= 2.5.1 is required)])
+])
+
   LIBS="$_save_libs"
   CFLAGS="$_save_cflags"
 fi

Comments

Bryce Harrington April 11, 2018, 8:29 p.m.
On Thu, Apr 12, 2018 at 12:36:24AM +0900, suzuki toshiya wrote:
> Dear Bryce,
> 
> Sorry for the lated response, I appreciate your effort to improve
> my confusing comment...
> 
> Bryce Harrington wrote:
> > /* Define to nothing if freetype2 supports color fonts */
> > #define FT_HAS_COLOR(x) (0)
> > 
> > But actually we only want to define to (0) if freetype2 does *not*
> > support color fonts, right?  "Define to nothing" might be a bit
> > ambiguous though (does it mean undefine?  define to blank?  define to
> > zero?)  So, my suggestion is for it to maybe look more like this:
> > 
> > /* Define to (0) if freetype2 does not support color fonts */
> > #define FT_HAS_COLOR(x) (0)
> > 
> > What do you think?  I apologize for being nitpicky on a mere comment,
> > but thank you for taking the extra time on this.
> 
> Please do not apologize! The comments making the developers
> confused must be avoided...
> 
> Oh, the suggested comment is clearly better. I wonder why I could
> not do like this (maybe "Define to XXX if we *have* YYY" style was
> deeply hardwired in my brain). Here is the reworked patch.

Thanks!  Landed and pushed:

To ssh://git.freedesktop.org/git/cairo
   38806bc..caf6f71  master -> master