On Wed, Sep 20, 2006 at 03:22:02PM +0900, Dmitry Timoshkov wrote:
> Changelog:
>     Implement GetKerningPairs for TrueType fonts.

Hi Dmitry,

Looks good, apart from one small nitpick below.

I still think it would be nice to add the tests that you have to the
test suite, even if it's just something that checks the first few
pairs of Arial at some fixed font height.

....
> +static DWORD parse_format0_kern_subtable(GdiFont font,
> +                                         const struct 
> TT_format0_kern_subtable *tt_f0_ks,
> +                                         const USHORT *glyph_to_char,
> +                                         KERNINGPAIR *kern_pair, DWORD 
> cPairs)
> +{
....
> +    kern_pairs_copied = 0;
> +
> +    for (i = 0; i < nPairs; i++)
> +    {
> +        kern_pair->wFirst = glyph_to_char[GET_BE_WORD(tt_kern_pair[i].left)];
> +        kern_pair->wSecond = 
> glyph_to_char[GET_BE_WORD(tt_kern_pair[i].right)];
> +        kern_pair->iKernAmount = 
> MulDiv((short)GET_BE_WORD(tt_kern_pair[i].value), font->ppem, 
> font->ft_face->units_per_EM);
> +
> +        TRACE("left %u right %u value %d\n",
> +               kern_pair->wFirst, kern_pair->wSecond, 
> kern_pair->iKernAmount);
> +
> +        kern_pair++;
> +        kern_pairs_copied++;
> +        if (!--cPairs) break;
> +    }

Seems to me that if you set nPairs = min(cPairs, nPairs) at the start
of the loop, then you can eliminate the break and get rid of the
kern_pairs_copied variable that basically mirrors i.

Thanks,
Huw.
-- 
Huw Davies
[EMAIL PROTECTED]


Reply via email to