Re: [PATCH] [autofit] Fix double division in stem darkening.

[Alexei Podtelezhnikov]

Hi Ishi-san

I will apply your patch later today. Since you must have analyzed the
code and understand the bottleneck in the emboldening function, I
would  like to invite you to explore an alternative approach.
https://lists.nongnu.org/archive/html/freetype-devel/2017-09/msg00020.html
is my proff of concept.

The autohinter has a good grasp of stem widths internally and adjusts
them ever so slightly for thin stems. It seems to me that all we need
to do is to increase that adjustment instead of the emboldening
business.

Alexei

On Fri, Dec 18, 2020 at 2:30 AM Tatsuyuki Ishi <ishitatsuyuki@gmail.com> wrote:
>
> The old code used to divide the darkening amount by em_ratio twice,
> leading to unnecessarily bold stems on certain fonts with higher units
> per em (e.g. Inter). This patch fixes it.
>
> The return value of af_loader_compute_darkening was also changed to use
> 16/16 fixed point to get rid of a redundant truncation operation. This
> should slightly improve the precision, although it's still bottlenecked
> by the emboldening function, which uses 26/6 fixed point.
> ---
>  src/autofit/afloader.c | 27 ++++++++++++---------------
>  src/autofit/afloader.h |  2 +-
>  2 files changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/src/autofit/afloader.c b/src/autofit/afloader.c
> index c35d85c4c..bdcecb3b1 100644
> --- a/src/autofit/afloader.c
> +++ b/src/autofit/afloader.c
> @@ -105,7 +105,6 @@
>                                globals->stem_darkening_for_ppem;
>
>      FT_Fixed  em_size  = af_intToFixed( face->units_per_EM );
> -    FT_Fixed  em_ratio = FT_DivFix( af_intToFixed( 1000 ), em_size );
>
>      FT_Matrix  scale_down_matrix = { 0x10000L, 0, 0, 0x10000L };
>
> @@ -142,12 +141,11 @@
>
>
>        darken_by_font_units_x =
> -        af_intToFixed( af_loader_compute_darkening( loader,
> -                                                    face,
> -                                                    stdVW ) );
> -      darken_x = FT_DivFix( FT_MulFix( darken_by_font_units_x,
> -                                       size_metrics->x_scale ),
> -                            em_ratio );
> +         af_loader_compute_darkening( loader,
> +                                      face,
> +                                      stdVW ) ;
> +      darken_x = FT_MulFix( darken_by_font_units_x,
> +                            size_metrics->x_scale );
>
>        globals->standard_vertical_width = stdVW;
>        globals->stem_darkening_for_ppem = size_metrics->x_ppem;
> @@ -161,12 +159,11 @@
>
>
>        darken_by_font_units_y =
> -        af_intToFixed( af_loader_compute_darkening( loader,
> -                                                    face,
> -                                                    stdHW ) );
> -      darken_y = FT_DivFix( FT_MulFix( darken_by_font_units_y,
> -                                       size_metrics->y_scale ),
> -                            em_ratio );
> +         af_loader_compute_darkening( loader,
> +                                      face,
> +                                      stdHW ) ;
> +      darken_y = FT_MulFix( darken_by_font_units_y,
> +                            size_metrics->y_scale );
>
>        globals->standard_horizontal_width = stdHW;
>        globals->stem_darkening_for_ppem   = size_metrics->x_ppem;
> @@ -594,7 +591,7 @@
>     *
>     * XXX: Currently a crude adaption of the original algorithm.  Do
> better?
>     */
> -  FT_LOCAL_DEF( FT_Int32 )
> +  FT_LOCAL_DEF( FT_Fixed )
>    af_loader_compute_darkening( AF_Loader  loader,
>                                 FT_Face    face,
>                                 FT_Pos     standard_width )
> @@ -713,7 +710,7 @@
>      }
>
>      /* Convert darken_amount from per 1000 em to true character space.
> */
> -    return af_fixedToInt( FT_DivFix( darken_amount, em_ratio ) );
> +    return FT_DivFix( darken_amount, em_ratio );
>    }
>
>
> diff --git a/src/autofit/afloader.h b/src/autofit/afloader.h
> index 97282371c..0223a1ea9 100644
> --- a/src/autofit/afloader.h
> +++ b/src/autofit/afloader.h
> @@ -75,7 +75,7 @@ FT_BEGIN_HEADER
>                          FT_UInt    gindex,
>                          FT_Int32   load_flags );
>
> -  FT_LOCAL_DEF( FT_Int32 )
> +  FT_LOCAL_DEF( FT_Fixed )
>    af_loader_compute_darkening( AF_Loader  loader,
>                                 FT_Face    face,
>                                 FT_Pos     standard_width );
> --
> 2.29.2
>
>
>
>


-- 
Alexei A. Podtelezhnikov, PhD