Re: [ft-devel] Enhancements for Subpixel Rendering

[Werner LEMBERG]

Hello Erik!


> Here are the latest updates I've been working on for the subpixel
> hinting stuff.

Thanks!  Looks good.

> I attempted to commit this with David's latest patch but ran into
> some issues, so this doesn't include his C++ compatibility stuff.

Well, I will soon commit David's patch, so please adjust your code,
then commit.

Below are some minor comments.


   Werner


======================================================================


> +      else grayscale = grayscale_hinting = FALSE;

Please use

   else
     grayscale = grayscale_hinting = FALSE;

> +       if ( CUR.ignore_x_mode                         &&  \
> +            ( ( I == 24                           &&      \
> +                ( CUR.face->sph_found_func_flags &        \
> +                  ( SPH_FDEF_SPACING_1 |                  \
> +                    SPH_FDEF_SPACING_2 ) ) )        ||    \
> +              ( I == 22                           &&      \
> +                ( CUR.sph_in_func_flags &                 \
> +                  SPH_FDEF_TYPEMAN_STROKES ) )      ||    \
> +              ( I == 8                            &&      \
> +                ( CUR.face->sph_found_func_flags &        \
> +                  SPH_FDEF_VACUFORM_ROUND_1 )     &&      \
> +                  CUR.iup_called ) ) )                    \

Please align the closing parentheses with the logical operator one
line above:

      if ( CUR.ignore_x_mode                          &&  \
           ( ( I == 24                            &&      \
               ( CUR.face->sph_found_func_flags &         \
                 ( SPH_FDEF_SPACING_1 |                   \
                   SPH_FDEF_SPACING_2 )         ) ) ||    \
             ( I == 22                      &&            \
               ( CUR.sph_in_func_flags    &               \
                 SPH_FDEF_TYPEMAN_STROKES ) )       ||    \
             ( I == 8                             &&      \
               ( CUR.face->sph_found_func_flags &         \
                 SPH_FDEF_VACUFORM_ROUND_1      ) &&      \
                 CUR.iup_called                   ) ) )   \

otherwise our use of trailing operators becomes hard to read.

> +            case 5:
> +              switch ( n )
> +              {
> +              case 0: case 1: case 2: case 4: case 7: case 8:

Please do

  case 0:
  case 1:
  ...

>          if ( CUR.ignore_x_mode )
>          {
> +
>            /* save point for later comparison */

No need for an empty line here IMHO.


> +#ifdef TT_CONFIG_OPTION_SUBPIXEL_HINTING
> +    {
> +      /* always do cvt cut-in always in MIRP for sph */

Grammar :-)

> +      if ( CUR.ignore_x_mode && CUR.GS.gep0 == CUR.GS.gep1 )
> +      {
> +        if ( FT_ABS( cvt_dist - org_dist ) > control_value_cutin )
> +        cvt_dist = org_dist;

Looks like wrong indentation...

> +            /* *** messes up dejavu *** may not be needed */
> +            /*if ( !CUR.face->sph_compatibility_mode &&
> +                   CUR.GS.freeVector.y != 0           )
> +              CUR_Func_move( &CUR.zp0, A, B );*/
> +            /*else*/

Please use `#if 0 ... #endif' instead commenting out code.

> +  /* Force special legady fixes for fonts;                                 */
> +#define COMPATIBILITY_MODE_RULES_SIZE  1

s/legady/legacy/.