[PATCH] [4/?] grub-core/gfxmenu/gui_list.c - baseline centering bug

[PATCH] [4/?] grub-core/gfxmenu/gui_list.c - baseline centering bug

[Vladimir Testov]

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

See screenshots included.

If the selected entry font is specified and the font has different size than 
non-selected entry's font size (e.g. selected entry has larger font) then the 
baseline of the font will be misaligned.

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

[-- Attachment #2: before-list-baseline-bug.png --]
[-- Type: image/png, Size: 5890 bytes --]

[-- Attachment #3: after-list-baseline.png --]
[-- Type: image/png, Size: 4011 bytes --]

[-- Attachment #4: grub-list-baseline-fix.patch --]
[-- Type: text/x-patch, Size: 2313 bytes --]

diff -Naur grub-new3/grub-core/gfxmenu/gui_list.c grub-new4/grub-core/gfxmenu/gui_list.c
--- grub-new3/grub-core/gfxmenu/gui_list.c	2013-07-15 15:30:26.585116409 +0400
+++ grub-new4/grub-core/gfxmenu/gui_list.c	2013-07-15 16:36:34.544169956 +0400
@@ -273,16 +273,30 @@
     cwidth -= selbox->get_border_width (selbox);
   selbox->set_content_size (selbox, cwidth, item_height);
 
-  int string_left_offset = self->icon_width + icon_text_space;
-  int string_top_offset = (item_height - (ascent + descent)) / 2 + ascent;
+  int item_left_offset = self->icon_width + icon_text_space;
+  int item_top_offset = (item_height - (ascent + descent)) / 2 + ascent;
+  int sel_item_top_offset;
+  int top_offset;
 
   grub_video_rect_t svpsave, sviewport;
-  sviewport.x = sel_leftpad + string_left_offset;
-  sviewport.width = cwidth - string_left_offset;
+  sviewport.x = sel_leftpad + item_left_offset;
+  sviewport.width = cwidth - item_left_offset;
   sviewport.height = item_height;
 
   check_colors (self);
 
+  if (self->selected_item_font_set)
+    {
+      int sel_ascent = grub_font_get_ascent (self->selected_item_font);
+      int sel_descent = grub_font_get_descent (self->selected_item_font);
+      sel_item_top_offset = (item_height - (sel_ascent + sel_descent)) / 2 
+                        + sel_ascent;
+    }
+  else
+    {
+      sel_item_top_offset = item_top_offset;
+    }
+
   for (visible_index = 0, menu_index = self->first_shown_index;
        visible_index < num_shown_items && menu_index < self->view->menu->size;
        visible_index++, menu_index++)
@@ -297,11 +311,13 @@
           selbox->draw (selbox, 0, item_top - sel_toppad);
           font = self->selected_item_font;
           color = self->selected_item_color;
+          top_offset = sel_item_top_offset;
         }
       else
         {
           font = self->item_font;
           color = self->item_color;
+          top_offset = item_top_offset;
         }
 
       icon = get_item_icon (self, menu_index);
@@ -320,7 +336,7 @@
                              font,
                              color,
                              0,
-                             string_top_offset);
+                             top_offset);
       grub_gui_restore_viewport (&svpsave);
 
       item_top += item_height + item_vspace;

Re: [PATCH] [4/?] grub-core/gfxmenu/gui_list.c - baseline centering bug

[Vladimir Testov]

2013-07-16  Vladimir Testov <vladimir.testov@rosalab.ru>

	* grub-core/gfxmenu/gui_list.c: Baseline misalignment bugfix.

On Monday, July 15, 2013 05:35:15 PM Vladimir Testov wrote:
> See screenshots included.
> 
> If the selected entry font is specified and the font has different size than
> non-selected entry's font size (e.g. selected entry has larger font) then
> the baseline of the font will be misaligned.
> 
> --
> With best regards,
> _______________________________
> Vladimir Testov, ROSA Laboratory.
> www.rosalab.ru
-- 
With best regards,
_______________________________
Vladimir Testov, ROSA Laboratory.
www.rosalab.ru

Re: [PATCH] [4/?] grub-core/gfxmenu/gui_list.c - baseline centering bug

[Andrey Borzenkov]

В Mon, 15 Jul 2013 17:35:15 +0400
Vladimir Testov <vladimir.testov@rosalab.ru> пишет:

> diff -Naur grub-new3/grub-core/gfxmenu/gui_list.c grub-new4/grub-core/gfxmenu/gui_list.c
> --- grub-new3/grub-core/gfxmenu/gui_list.c	2013-07-15 15:30:26.585116409 +0400
> +++ grub-new4/grub-core/gfxmenu/gui_list.c	2013-07-15 16:36:34.544169956 +0400
> @@ -273,16 +273,30 @@
>      cwidth -= selbox->get_border_width (selbox);
>    selbox->set_content_size (selbox, cwidth, item_height);
>  
> -  int string_left_offset = self->icon_width + icon_text_space;
> -  int string_top_offset = (item_height - (ascent + descent)) / 2 + ascent;
> +  int item_left_offset = self->icon_width + icon_text_space;
> +  int item_top_offset = (item_height - (ascent + descent)) / 2 + ascent;
[...]
>  
>    grub_video_rect_t svpsave, sviewport;
> -  sviewport.x = sel_leftpad + string_left_offset;
> -  sviewport.width = cwidth - string_left_offset;
> +  sviewport.x = sel_leftpad + item_left_offset;
> +  sviewport.width = cwidth - item_left_offset;
>    sviewport.height = item_height;

Is there any reason for renaming? Such changes usually obfuscate main
patch content and better done as separate patch with clear explanation.

Re: [PATCH] [4/?] grub-core/gfxmenu/gui_list.c - baseline centering bug

[Vladimir Testov]

On Saturday, July 20, 2013 05:47:57 PM Andrey Borzenkov wrote:

> > -  int string_left_offset = self->icon_width + icon_text_space;
> > -  int string_top_offset = (item_height - (ascent + descent)) / 2 +
> > ascent;
> > +  int item_left_offset = self->icon_width + icon_text_space;
> > +  int item_top_offset = (item_height - (ascent + descent)) / 2 + ascent;

> Is there any reason for renaming? Such changes usually obfuscate main
> patch content and better done as separate patch with clear explanation.

I have renamed these values to differ left and top offsets for non-selected item 
and for selected item.

So the meanings of the values are cleared and easy-to-get.

I can do these changes in a separate patch if it's needed.

Just the patch and the renaming are about the same concept so it should be 
applied together.

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