[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Font antialiasing v2

From: Evgeny Kolesnikov
Subject: Re: [PATCH] Font antialiasing v2
Date: Mon, 05 Apr 2010 12:50:06 +0700

On Fri, 2010-04-02 at 22:23 +0200, Vladimir 'φ-coder/phcoder' Serbinenko

> -     for (j = font_info->ranges[i * 2]; j <= font_info->ranges[i * 2 + 1];
> -          j++)
> -       add_char (font_info, face, j);
> +           for (j = font_info->ranges[i * 2]; j <= font_info->ranges[i * 2 + 
> 1]; j++)
> +             add_char (font_info, face, j);
> Can you fix the style of your patch to avoid hunks like these? (Hint:
> GNU indent)

Most of sources related to font functionality contains awful mix of
spaces and tabs. Should I fix this? Strictly use spaces everywhere? 

> -  glyph_bitmap.mode_info.blit_format = GRUB_VIDEO_BLIT_FORMAT_1BIT_PACKED;
> -  glyph_bitmap.mode_info.bpp = 1;
> -
> -  /* Really 1 bit per pixel.  */
> -  glyph_bitmap.mode_info.bytes_per_pixel = 0;
> -
> These bits need to be filled even if blit_format is set


> Using entire byte for this is quite a waste. It's better to use 2 or 3 last 

This byte is already wasted with packed/unpacked bit. 
3 bits actually, others as reserved. And I used reserved ones.
See (PFF2 spec, CHIX, item 2).

> Also I doubt usefulness of a font in which only some glyphs are anti-aliased. 
> Perhaps we could move antialiasing flags to file header and shave off few 
> bytes?

Yes this makes sense. I.e. substitution glyph for unknown symbol 
have no AA. Also frames and other geometry can benefit from this.
Anyway this byte is already wasted in original spec.

Moreover I thinking about transparent gz or xz reader for font
file. This will drastically reduce the size.

> Another point is that although storage_flags are present since some time 
> trying to use antialised fonts in older grub will result in garbage. I prefer 
> to put PFF3 signature if any flag is used. It will also allow more freedom it 
> fields specification,

Well, generally I agree. This will be more evident in case of troubles.

> +  fgcolor = grub_video_fb_map_rgba (src->mode_info->fg_red,
> +                                 src->mode_info->fg_green,
> +                                 src->mode_info->fg_blue,
> +                                 src->mode_info->fg_alpha);
> background color isn't handled correctly (it's not always transparent)
> +  grub_uint8_t fa, a;

Actually it always 0x00000000. See grub_font_draw_glyph: 
there is only FG color in declaration.

Moreover I don't think that bg color is useful because
we handle alpha channel correctly for fg and mask itself,
so anyone can blit glyph onto everything he want.

Also filling rectangle with color below text and then
rendering only fg mask will be way more fast.

But if you insist we should fix grub_font_draw_glyph and all
it's callers at first place. How exactly?

> Please avoid mixing declarations and code


reply via email to

[Prev in Thread] Current Thread [Next in Thread]