[Ffmpeg-devel] [PATCH] drawtext.c: 09 fix draw_glyph

[Michael Niedermayer]

Hi

On Sun, Sep 10, 2006 at 04:29:56PM -0300, Gustavo Sverzut Barbieri wrote:
> This fix the other major remaining bug with drawtext.c: drawing glyphs.
> 
> The previous code was flawed due over-complicated outline detection,
> this is much more simple: while visiting a glyph pixel, check if
> neighbours are not pixels, in this case they're border and should be
> draw as outline if this is the case.
> 
> I believe this is the last patch to fix bugs and improve code. If
> they're all ok then I'll post a "code cleanup" series of patches.
[...]
> @@ -317,65 +305,55 @@
>  }
>  
>  
> +static inline int bitmap_is_set(const FT_Bitmap *bitmap, const int x, const int y)
> +{
> +  const int idx = bitmap->pitch * y + x / 8;
> +  const int is_pixel = bitmap->buffer[idx] & (0x80 >> (x % 8));

/8 and %8 of signed integers is slow, you should use >>3 and &7


> +
> +  return is_pixel;
> +}
> +
> +static inline int bitmap_is_border(const FT_Bitmap *bitmap, const int x, const int y, const int dx, const int dy)
> +{
> +  if (x == 0 || x >= bitmap->width || y == 0 || y >= bitmap->rows)
> +    return 1;
> +  return !bitmap_is_set(bitmap, x + dx, y + dy);
> +}
>  
>  
>  static inline void draw_glyph(AVPicture *picture, FT_Bitmap *bitmap, unsigned int x, unsigned int y, unsigned int width, unsigned int height, unsigned char yuv_fgcolor[3], unsigned char yuv_bgcolor[3], int outline)
>  {
>    int r, c;
> -  int spixel, dpixel[3], in_glyph=0;
>  
>    if (bitmap->pixel_mode == ft_pixel_mode_mono)
>      {
> -      in_glyph = 0;
> -      for (r=0; (r < bitmap->rows) && (r+y < height); r++)
> +      for (r=0; r < bitmap->rows; r++)
>          {
> -          for (c=0; (c < bitmap->width) && (c+x < width); c++)
> +          for (c=0; c < bitmap->width; c++)
>              {
> -              /* pixel in the picture (destination) */
> -              GET_PIXEL(picture, dpixel, (c+x), (y+r));
> -
> -              /* pixel in the glyph bitmap (source) */
> -              spixel = bitmap->buffer[r*bitmap->pitch +c/8] & (0x80>>(c%8));
> -
> -              if (spixel)
> -                COPY_3(dpixel, yuv_fgcolor);
> -
> -              if (outline)
> +              if (bitmap_is_set(bitmap, c, r))
>                  {
> -                  /* border detection: */
> -                  if ( (!in_glyph) && (spixel) )
> -                    /* left border detected */
> -                    {
> -                      in_glyph = 1;
> -                      /* draw left pixel border */
> -                      if (c-1 >= 0)
> -                        SET_PIXEL(picture, yuv_bgcolor, (c+x-1), (y+r));
> -                    }
> -                  else if ( (in_glyph) && (!spixel) )
> -                    /* right border detected */
> +                  if (outline)
>                      {
> -                      in_glyph = 0;
> -                      /* 'draw' right pixel border */
> -                      COPY_3(dpixel, yuv_bgcolor);
> +                      if (bitmap_is_border(bitmap, c, r, -1, 0) &&
> +                          c - 1 + x >= 0)
> +                        SET_PIXEL(picture, yuv_bgcolor, c - 1 + x, r + y);
> +
> +                      if (bitmap_is_border(bitmap, c, r, +1, 0) &&
> +                          c + 1 + x < width)
> +                        SET_PIXEL(picture, yuv_bgcolor, c + 1 + x, r + y);
> +
> +                      if (bitmap_is_border(bitmap, c, r, 0, -1) &&
> +                          r - 1 + y >= 0)
> +                        SET_PIXEL(picture, yuv_bgcolor, c + x, r - 1 + y);
> +
> +                      if (bitmap_is_border(bitmap, c, r, 0, +1) &&
> +                          r + 1 + y < height)
> +                        SET_PIXEL(picture, yuv_bgcolor, c + x, r + 1 + y);
>                      }

the outline should be precalculated, instead of redoing the calculation for
every pixel, also the code should be split into a inner optimized part and
one for the areas close to the borders of the image to avoid the 
0/width/height checks


>  
> -                  if (in_glyph)
> -                    /* see if we have a top/bottom border */
> -                    {
> -                      /* top */
> -                      if ( (r-1 >= 0) && (! bitmap->buffer[(r-1)*bitmap->pitch +c/8] & (0x80>>(c%8))) )
> -                        /* we have a top border */
> -                        SET_PIXEL(picture, yuv_bgcolor, (c+x), (y+r-1));
> -
> -                      /* bottom */
> -                      if ( (r+1 < height) && (! bitmap->buffer[(r+1)*bitmap->pitch +c/8] & (0x80>>(c%8))) )
> -                        /* we have a bottom border */
> -                        SET_PIXEL(picture, yuv_bgcolor, (c+x), (y+r+1));
> -
> -                    }
> +                  SET_PIXEL(picture, yuv_fgcolor, c + x, r + y);

arent the chroma values totally wrong here? they are subsampled and just
setting them to the color of the font if any of the 4 luma samples is part
of the text isnt correct
and actually the whole should be "antialiased" instead of the binary
transparent vs. opaque

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is