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

[Tatsuyuki Ishi]

Hi Alexei,

Thanks for merging the patch.

Your linked proposal about stem darkening in auto-hinter looks
interesting. As I'm short on time, I'm afraid I can't dig into that
right now, but I look forward to spending some more time with stem
darkening maybe in one or two months from now.

Tatsuyuki

2020年12月18日(金) 23:27 Alexei Podtelezhnikov <apodtele@gmail.com>:
>
> 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