Re: Implement GetKerningPairs for TrueType fonts, take 3

[Huw Davies]

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
huw(a)codeweavers.com