Re: [PATCH] multistring support in gui_label

[Vladimir Testov <vladimir.testov@rosalab.ru>]

[-- Attachment #1: Type: text/plain, Size: 3496 bytes --]

On Wednesday, April 17, 2013 09:01:47 AM Vladimir 'φ-coder/phcoder' Serbinenko 
wrote:

> Don't write any line-breaking at all yourself.

I did not write any line-breaking. It is existing line-breaking. And it have 
one algorithmic problem. Do not see any problem if I will fix the problem.

The problem persists when we have some long word which cannot be printed 
entirely on one line...

> > 2) There is funny handling of UTF-8 symbols. Each symbol have
> > "device_width" parameter,
> > which is used in calculation of string's length.
> 
> Where is it used? Show exactly. It must be some leftover code.

> We don't do any kerning. No need to change algorithm.

O.k. Here it is.

from grub-core/font/font.c  line 1387 function blit_comb :
>            do_blit (combining_glyphs[i],
>                    main_glyph->device_width
>                     + combining_glyphs[i]->offset_x,
>                     -(combining_glyphs[i]->height
>                       + combining_glyphs[i]->offset_y), &ctx);
>            add_device_width (combining_glyphs[i]->device_width, &ctx);
>          }
>        }
>    }
>  add_device_width ((above_rightx >
>                     below_rightx ? above_rightx : below_rightx) -
>                    (main_glyph->offset_x + main_glyph->width), &ctx);
>  add_device_width (above_leftx - main_glyph->offset_x, &ctx);
>  if (glyph && glyph->device_width < min_devwidth)
>    glyph->device_width = min_devwidth;
>  if (device_width && *device_width < min_devwidth)
>    *device_width = min_devwidth;

***device_width is changed sometimes.

from grub-core/font/font.c line 1458 function  grub_font_construct_dry_run :
>  if (device_width)
>    *device_width = main_glyph->device_width;
>
>  if (!glyph_id->ncomb && !glyph_id->attributes)
>    return main_glyph;
>
>  combining_glyphs = grub_malloc (sizeof (combining_glyphs[0])
>                                  * glyph_id->ncomb);
>  if (glyph_id->ncomb && !combining_glyphs)
>    {
>      grub_errno = GRUB_ERR_NONE;
>      return main_glyph;
>    }
>
>  for (i = 0; i < glyph_id->ncomb; i++)
>    combining_glyphs[i]
>      = grub_font_get_glyph_with_fallback (main_glyph->font,
>                                           glyph_id->combining[i].code);
>
>  blit_comb (glyph_id, NULL, bounds, main_glyph, combining_glyphs,
>             device_width);
>  if (combining_glyphs_out)
>    *combining_glyphs_out = combining_glyphs;
>  else
>    grub_free (combining_glyphs);
>
>  return main_glyph;

***return value *device_width set to main_glyph->device_width before 
main_glyph->device_width is changed during execution of blit_comb

If we want to get device_width of some glyph - we use function 
get_constructed_device_width. It returns device_width parameter of the asked 
glyph with parameter *device_width of the function 
grub_font_construct_dry_run.

When we actually construct glyph, we take device_width from glyph-
>device_width.

The most adequate way to find device_width of grub_unicode_glyph is to call 
grub_font_get_constructed_device_width. But it is miscalculated in some cases.

So these two values are different in some cases (This is all about problem 2)

Suggested patch to fix this problem (second one) is attached.

I will remake the patch for multistring support for the upstream version.

-- 
With best regards,
_______________________________
Vladimir Testov, ROSA Laboratory.
www.rosalab.ru

[-- Attachment #2: grub-get_constructed_device_width-fixed.patch --]
[-- Type: text/x-patch, Size: 521 bytes --]

diff -Naur grub/grub-core/font/font.c grub-new/grub-core/font/font.c
--- grub/grub-core/font/font.c	2013-03-28 23:28:47.784178000 +0400
+++ grub-new/grub-core/font/font.c	2013-04-17 12:21:53.682367388 +0400
@@ -1476,6 +1476,11 @@
 
   blit_comb (glyph_id, NULL, bounds, main_glyph, combining_glyphs,
 	     device_width);
+
+  /* The value could have been changed */
+  if (device_width)
+    *device_width = main_glyph->device_width;
+
   if (combining_glyphs_out)
     *combining_glyphs_out = combining_glyphs;
   else