Re: conditionals for older freetype2 without color font feature

On Wed, Apr 04, 2018 at 01:15:41AM +0900, suzuki toshiya wrote:
> Dear Bryce,
> 
> Here is revised patch. Please let me explain for your review

> diff --git a/configure.ac b/configure.ac
> index d78b2ed..ebc31d9 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 nothing if freetype2 supports
> color font])
> +  AC_MSG_RESULT([no, disable color font (freetype2 >= 2.5.1 is required)])
> +])
> +
>    LIBS="$_save_libs"
>    CFLAGS="$_save_cflags"
>  fi

Hi Toshiyasan,

Your patch applies cleanly and configure passes for me with no issue,
thank you.  I noticed a small error in the comment, shouldn't it be
"define nothing if freetype2 does not support color fonts"?  I can fix
that locally.

> * the patched part tries to compile & link a source including FT_HAS_COLOR()
> which is defined as a macro function. basically, the functions whose names
> are in all upper cases in FreeType2 are macro functions, so the availability
> check could be simplified to the check of C preprocessor macro, but I did
> like this, to minimize the assumption.
> 
> * for newer FreeType2 with FT_HAS_COLOR(), config.h defines nothing about it.
> (note: config.h.in looks like as if "#undef FT_HAS_COLOR" is executed
> for newer FreeType2, but it is not - please check the result on the
> platform with newer FreeType2, it would be commented out)
> 
> * for older FreeType2 without FT_HAS_COLOR(), config.h defines as
> #define FT_HAS_COLOR(x) (0)

Yes, makes sense.  I've written a commit message incorporating the above
details, and crediting you for the work.

Thanks again,
Bryce

> suzuki toshiya wrote:
> > Dear Bryce,
> > 
> > Thank you for prompt review!
> > 
> >  >> #ifndef FT_HAS_COLOR
> >  >> # define FT_HAS_COLOR(x) ( 0 )
> >  >> #endif
> >  >>
> >  >> in config header is better?
> >  >
> >  > Offhand I prefer this latter approach, just feels a bit cleaner.  Would
> >  > you mind respinning your patch with this approach?
> > 
> > OK! I would revise and resubmit.
> > 
> > Regards,
> > mpsuzuki
> > 
> > On 4/3/2018 12:30 PM, Bryce Harrington wrote:
> >> On Tue, Apr 03, 2018 at 10:34:20AM +0900, suzuki toshiya wrote:
> >>> Hi,
> >>>
> >>> FT_HAS_COLOR() macro is unavailable in older freetype2
> >>> without color font feature. attached is a quick fix.
> >>>
> >>> or, something like
> >>>
> >>> #ifndef FT_HAS_COLOR
> >>> # define FT_HAS_COLOR(x) ( 0 )
> >>> #endif
> >>>
> >>> in config header is better?
> >> Offhand I prefer this latter approach, just feels a bit cleaner.  Would
> >> you mind respinning your patch with this approach?
> >>
> >>> it seems that fontconfig decided to drop old freetype2,
> >>> but I wish if cairo can provide (limited) support for
> >>> older freetype2.
> >> Sounds ok, I don't see a problem with doing this.
> >>
> >> Thanks,
> >> Bryce
> >>
> > 

> sh: 1: highlight: not found

-- 
cairo mailing list
[email protected]
https://lists.cairographics.org/mailman/listinfo/cairo