[MPlayer-cvslog] r23104 - trunk/libass/ass_render.c

[Reimar Döffinger]

Hello,
On Tue, Apr 24, 2007 at 07:36:03PM +0200, eugeni wrote:
> Author: eugeni
> Date: Tue Apr 24 19:36:03 2007
> New Revision: 23104
> 
> Log:
> Do not use FT_Glyph_Copy with NULL glyphs.
> Contrary to the docs, it leaves garbage in *target.
> 
> 
> Modified:
>    trunk/libass/ass_render.c
> 
> Modified: trunk/libass/ass_render.c
> ==============================================================================
> --- trunk/libass/ass_render.c	(original)
> +++ trunk/libass/ass_render.c	Tue Apr 24 19:36:03 2007
> @@ -1239,7 +1239,8 @@ static void get_outline_glyph(int symbol
>  	val = cache_find_glyph(&key);
>  	if (val) {
>  		FT_Glyph_Copy(val->glyph, &info->glyph);
> -		FT_Glyph_Copy(val->outline_glyph, &info->outline_glyph);
> +		if (val->outline_glyph)
> +			FT_Glyph_Copy(val->outline_glyph, &info->outline_glyph);
>  		info->bbox = val->bbox_scaled;
>  		info->advance.x = val->advance.x;
>  		info->advance.y = val->advance.y;
> @@ -1260,8 +1261,10 @@ static void get_outline_glyph(int symbol
>  			}
>  		}
>  
> +		memset(&v, 0, sizeof(v));
>  		FT_Glyph_Copy(info->glyph, &v.glyph);
> -		FT_Glyph_Copy(info->outline_glyph, &v.outline_glyph);
> +		if (info->outline_glyph)
> +			FT_Glyph_Copy(info->outline_glyph, &v.outline_glyph);

If only info->outline_glyph == 0 is the problem, why didn't you put the
memset in the else, IMO that would make it clearer.
And if it is only a problem because FT_Glyph_Copy, you might consider if
it's a good idea to add something like
// HACK: at least in freetype version ... FT_Glyph_Copy does not work as
// specified for info->outline_glyph
Otherwise someone looking at the code in three years will scratch his
head and wonder why you did it that complicated (yes, svn log is there,
but it will be buried very deep...).

Greetings,
Reimar Döffinger