[kmscon-devel] [PATCH] font: pango: fix a rendering problem of certain CJK fonts

[Chang Liu]

Hi David

2013/3/9 David Herrmann <dh.herrmann at gmail.com>

> Hi
>
> On Sat, Mar 9, 2013 at 8:41 AM, Chang Liu <cl91tp at gmail.com> wrote:
> > This patch fixes a rendering problem of certain CJK fonts which causes
> > the rendered CJK characters to appear downward-shifted.
> >
> > For some CJK fonts, pango_layout_get_pixel_extent() returns different
> > logical_rect for CJK glyphs and Latin glyphs. Therefore, using -rec.y
> > as the y position of the baseline will render the CJK glyphs downward-
> > shifted. face.baseline will be a better choice.
> >
> > ---
> >  src/font_pango.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/font_pango.c b/src/font_pango.c
> > index c2954c6..0964123 100644
> > --- a/src/font_pango.c
> > +++ b/src/font_pango.c
> > @@ -199,7 +199,7 @@ static int get_glyph(struct face *face, struct
> kmscon_glyph **out,
> >         bitmap.pixel_mode = FT_PIXEL_MODE_GRAY;
> >         bitmap.buffer = glyph->buf.data;
> >
> > -       pango_ft2_render_layout_line(&bitmap, line, -rec.x, -rec.y);
> > +       pango_ft2_render_layout_line(&bitmap, line, -rec.x,
> face->baseline);
>
> Ok, lets summarize how this is supposed to work (as I myself don't
> remember it correctly):
> We calculate the width/height of a cell by drawing the basic ASCII
> characters in a cache and measuring them. Every other glyph is then
> supposed to fit into multiples of these cells (multiples only in
> horizontal direction).
> "face->baseline" is set according to the baseline of this measurement
> (that is, the ASCII characters).
>
> So what this basically does is calculating a bounding box for our
> glyphs and computing the logical origin. Every glyph that we draw
> should be drawn at the logical origin (which is face->baseline) and
> clipped according to this box.
> So we do have to use face->baseline here, indeed. Using -rec.y aligns
> to the bottom edge instead, which is obviously wrong.

Yes, my thoughts exactly.

Could you tell me the font-name so I can reproduce this?

For Latin languages I am using  Lucida Console and for Chinese I am using
Microsoft YaHei. Both of them are default fonts in MS Win 8 which I
illegally ^_^
downloaded from the Internet. Not sure if you could do this in Europe but I
guess
you might need a valid license of Win 8 before you could legally use these
fonts.
However these fonts are available in Win 7 too and I don't think they
underwent
substantial changes since then. Since most laptop came with a pre-installed
Win 7
I think you can legally copy these fonts from a Win 7 installation and use
them
for testing. They are located in C:\Windows\Fonts and named lucida.ttf and
msyh{,bd}.ttc.

There are also a bunch of CJK fonts in MS Win 7 which I tested. Look for
.ttf or .ttc
files which are larger than 1 MB, they are most certainly CJK fonts. They
are rendered
correctly after the patch is applied.


>
I changed your commit-message to mention why this fix is correct and
> then applied the patch. I hope you don't mind.
>
No, not at all.

> Commit: e874518153e509b74aa8e3e52b02642718eb5575
>

There's also ./docs/unicode-test.txt. Could you check whether there
> are symbols in it that show this wrong behavior? If not, could you
> tell me what symbols you used in your screenshots so I can add them.
>
> Yes. The 4th line (empty line included), the line with a `Unicode' in it,
is Chinese
for `What is Unicode?'. All the Chinese characters in that line show this
behavior.


> Thanks a lot for reporting this!
> David
>
> >         pthread_mutex_lock(&face->glyph_lock);
> >         ret = shl_hashtable_insert(face->glyphs, (void*)(long)id, glyph);
> > --
> > 1.8.1.5
> >
> > _______________________________________________
> > kmscon-devel mailing list
> > kmscon-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/kmscon-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/kmscon-devel/attachments/20130310/6420bb79/attachment-0001.html>