Re: [PATCH] multistring support in gui_label

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

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

>Please don't use // comments.
O.k. I won't.
>This code completely forgets the cases
>when even the first word doesn't fit in the available space.

Mmm. It can handle this case actually.

>The code as whole breaks some unicode concepts like e.g. bidi stack.

Didn't get what you mean.

>Could you reuse the already available line-vreaking algorithm in
>normal/term.c and normal/charset.c ? Since the line-breaking is
>artificially disabled for labels it should require only minor
>adjustments to be reenabled.

Yep! Here it is (patch included)!

Nevertheless, two problems appeared and I don't sure how exactly should I fix 
them.
1) Handling of some long word. If this word is not first in line and it's 
length is more than label's width
then the length of the first fragment of the word will be counted as if it will 
be drawn on the same line,
but actually it will be printed on the next 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. But, when we are going to 
actually print a symbol,
it's length (with spaces) is calculated in other way. First, all symbols that 
have special connection with
current symbol are looked through. Then device_width is replaced with maximum 
of printed lengths in all cases.
So, for some special symbols (eg "`", used in @KEYMAP_LONG@) actual length 
will be more than pre-counted.

How should I fix these problems?
1st one - for example, I can slightly update line-breaking mechanism.
2nd one - more interesting, harder. I suggest utf-8 printing mechanism 
(charset.c unicode.c etc) should be remade. So symbol connections will be 
counted in more intelligent way (e.g. while counting spaces - take into 
consideration nearby symbols). It is interesting. :) I can do it. Would be 
happy, if someone could give me some advices.

Problem2.png 
text = "@KEYMAP_LONG@"
t is misprinted

Problem1.png
text = "short short short 
HereWeHaveSomeVeryLongWordSoItCannotBePrintedEntirelyOnOneLine"
See how the line-breaking works.

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

[-- Attachment #2: Problem1.png --]
[-- Type: image/png, Size: 2141 bytes --]

[-- Attachment #3: problem2.png --]
[-- Type: image/png, Size: 2954 bytes --]

[-- Attachment #4: grub-2.00-label-multistring.patch --]
[-- Type: text/x-patch, Size: 7526 bytes --]

diff -Naur grub-2.00/grub-core/gfxmenu/font.c grub-new/grub-core/gfxmenu/font.c
--- grub-2.00/grub-core/gfxmenu/font.c	2012-03-27 18:29:43.000000000 +0400
+++ grub-new/grub-core/gfxmenu/font.c	2013-04-15 09:55:26.978883414 +0400
@@ -75,6 +75,94 @@
   return GRUB_ERR_NONE;
 }
 
+/* Differs from grub_font_draw_line only in multiline output */
+grub_err_t
+grub_font_draw_multiline (const char *str, grub_font_t font,
+                          grub_video_color_t color,
+                          int baseline_y, int font_height,
+                          int max_width, int max_height,
+                          int align)
+{
+  int x;
+  struct grub_font_glyph *glyph;
+  grub_uint32_t *logical;
+  grub_ssize_t logical_len, visual_len;
+  struct grub_unicode_glyph *visual, *ptr, *pos;
+
+  grub_err_t print_line ()
+  {
+    while (pos != ptr)
+      {
+        grub_err_t err;
+        glyph = grub_font_construct_glyph (font, pos);
+        if (!glyph)
+          return grub_errno;
+        err = grub_font_draw_glyph (glyph, color, x, baseline_y);
+        x += glyph->device_width;
+        grub_free (glyph);
+        if (err)
+          return err;
+        pos ++;
+      }
+
+    return GRUB_ERR_NONE;
+  }
+
+  logical_len = grub_utf8_to_ucs4_alloc (str, &logical, 0);
+  if (logical_len < 0)
+    return grub_errno;
+
+  auto grub_ssize_t getcharwidth (const struct grub_unicode_glyph *c);
+  grub_ssize_t getcharwidth (const struct grub_unicode_glyph *c)
+  {
+    return grub_font_get_constructed_device_width (font, c);
+  }
+
+  visual_len = grub_bidi_logical_to_visual (logical, logical_len, &visual,
+					    getcharwidth, max_width, 0);
+  grub_free (logical);
+  if (visual_len < 0)
+    return grub_errno;
+
+  pos = visual;
+  for (ptr = visual, x = 0; ptr <= visual + visual_len; ptr++)
+    {
+      if (baseline_y > max_height)
+        break;
+      if (ptr->base == '\n' || ptr == visual + visual_len)
+        {
+          int left_x;
+          if (align == 1)
+            left_x = (max_width - x) / 2;
+          else if (align == 2)
+            left_x = max_width - x;
+          else
+            left_x = 0;
+
+          x = left_x;
+
+          grub_err_t err;
+          err = print_line();
+          if (err != GRUB_ERR_NONE)
+            return err;
+
+          baseline_y += font_height;
+          x = 0;
+          while ((ptr + 1 < visual + visual_len)
+                 && ((ptr + 1)->base == ' '
+                     || (ptr + 1)->base == '\t'))
+            ptr ++;
+          pos = ptr + 1;
+        }
+      else
+        x += grub_font_get_constructed_device_width (font, ptr);
+    }
+
+  grub_free (visual);
+
+  return GRUB_ERR_NONE;
+}
+
 /* Get the width in pixels of the specified UTF-8 string, when rendered in
    in the specified font (but falling back on other fonts for glyphs that
    are missing).  */
diff -Naur grub-2.00/grub-core/gfxmenu/gui_label.c grub-new/grub-core/gfxmenu/gui_label.c
--- grub-2.00/grub-core/gfxmenu/gui_label.c	2012-03-03 16:00:50.000000000 +0400
+++ grub-new/grub-core/gfxmenu/gui_label.c	2013-04-12 16:40:50.151859580 +0400
@@ -46,6 +46,7 @@
   grub_video_rect_t bounds;
   char *id;
   int visible;
+  int multiline;
   char *text;
   char *template;
   grub_font_t font;
@@ -91,28 +92,46 @@
     return;
 
   /* Calculate the starting x coordinate.  */
-  int left_x;
-  if (self->align == align_left)
-    left_x = 0;
-  else if (self->align == align_center)
-    left_x = (self->bounds.width
-	      - grub_font_get_string_width (self->font, self->text)) / 2;
-  else if (self->align == align_right)
-    left_x = (self->bounds.width
-              - grub_font_get_string_width (self->font, self->text));
-  else
-    return;   /* Invalid alignment.  */
+  int left_x = 0;
+  if (!self->multiline)
+    {
+      if (self->align == align_left)
+        left_x = 0;
+      else if (self->align == align_center)
+        left_x = (self->bounds.width
+                  - grub_font_get_string_width (self->font, self->text)) / 2;
+      else if (self->align == align_right)
+        left_x = (self->bounds.width
+                  - grub_font_get_string_width (self->font, self->text));
+      else
+        return;   /* Invalid alignment.  */
 
-  if (left_x < 0 || left_x > (int) self->bounds.width)
-    left_x = 0;
+      if (left_x < 0 || left_x > (int) self->bounds.width)
+        left_x = 0;
+    }
 
   grub_video_rect_t vpsave;
   grub_gui_set_viewport (&self->bounds, &vpsave);
-  grub_font_draw_string (self->text,
-                         self->font,
-                         grub_video_map_rgba_color (self->color),
-                         left_x,
-                         grub_font_get_ascent (self->font));
+  if (!self->multiline)
+    grub_font_draw_string (self->text,
+                           self->font,
+                           grub_video_map_rgba_color (self->color),
+                           left_x,
+                           grub_font_get_ascent (self->font));
+  else
+    {
+      int font_height = grub_font_get_max_char_height (self->font);
+      int max_width = self->bounds.width;
+      int max_height = self->bounds.height;
+      grub_font_draw_multiline (self->text,
+                                self->font,
+                                grub_video_map_rgba_color (self->color),
+                                grub_font_get_ascent (self->font),
+                                font_height,
+                                max_width,
+                                max_height,
+                                self->align);
+    }
   grub_gui_restore_viewport (&vpsave);
 }
 
@@ -148,9 +167,15 @@
 label_get_minimal_size (void *vself, unsigned *width, unsigned *height)
 {
   grub_gui_label_t self = vself;
-  *width = grub_font_get_string_width (self->font, self->text);
-  *height = (grub_font_get_ascent (self->font)
-             + grub_font_get_descent (self->font));
+  if (self->multiline)
+    {
+      *width = grub_font_get_max_char_width (self->font);
+      *height = grub_font_get_max_char_height (self->font);
+    } else {
+      *width = grub_font_get_string_width (self->font, self->text);
+      *height = (grub_font_get_ascent (self->font)
+                 + grub_font_get_descent (self->font));
+    }
 }
 
 static void
@@ -218,6 +243,10 @@
     {
       self->visible = grub_strcmp (value, "false") != 0;
     }
+  else if (grub_strcmp (name, "multiline") == 0)
+    {
+      self->multiline = grub_strcmp (value, "false") != 0;
+    }
   else if (grub_strcmp (name, "id") == 0)
     {
       grub_gfxmenu_timeout_unregister ((grub_gui_component_t) self);
@@ -257,6 +286,7 @@
     return 0;
   label->comp.ops = &label_ops;
   label->visible = 1;
+  label->multiline = 0;
   label->text = grub_strdup ("");
   label->font = grub_font_get ("Unknown Regular 16");
   label->color.red = 0;
diff -Naur grub-2.00/include/grub/gfxmenu_view.h grub-new/include/grub/gfxmenu_view.h
--- grub-2.00/include/grub/gfxmenu_view.h	2012-02-24 14:19:45.000000000 +0400
+++ grub-new/include/grub/gfxmenu_view.h	2013-04-12 16:26:41.869448464 +0400
@@ -66,6 +66,12 @@
 				  grub_font_t font,
 				  grub_video_color_t color,
 				  int left_x, int baseline_y);
+grub_err_t grub_font_draw_multiline (const char *str,
+				     grub_font_t font,
+				     grub_video_color_t color,
+				     int baseline_y, int font_height,
+				     int max_width, int max_height,
+				     int align);
 int grub_font_get_string_width (grub_font_t font,
 				const char *str);