Skip to contentSkip to conversation

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

PATCH In review Vladimir Testov 2 participants 4 messages Jul 15, 2013 – Jul 22, 2013

lore
Share & tools
lore

Vladimir Testov

author
#0 lore raw

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

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.

Signature · 4 lines
With best regards,
_______________________________
Vladimir Testov, ROSA Laboratory.
www.rosalab.ru
grub-core/gfxmenu/gui_list.c +21 -5
diff -Naur grub-new3/grub-core/gfxmenu/gui_list.c grub-new4/grub-core/gfxmenu/gui_list.c
@@ -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;
author
#1 lore raw

2013-07-16 Vladimir Testov <[email protected]>

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

Quoted 11 lines · Vladimir Testov

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

Signature · 4 lines
With best regards,
_______________________________
Vladimir Testov, ROSA Laboratory.
www.rosalab.ru
#2 lore raw
In reply to Vladimir Testov

В Mon, 15 Jul 2013 17:35:15 +0400
Vladimir Testov <[email protected]> пишет:

Quoted 11 lines

diff -Naur grub-new3/grub-core/gfxmenu/gui_list.c grub-new4/grub-core/gfxmenu/gui_list.c

grub-core/gfxmenu/gui_list.c +2 -2
@@ -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-core/gfxmenu/gui_list.c line 284

Quoted 7 lines
 
   grub_video_rect_t svpsave, sviewport;
grub-core/gfxmenu/gui_list.c +2 -2
-  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.

author
#3 lore raw
Quoted 8 lines · Andrey Borzenkov

grub-core/gfxmenu/gui_list.c line 277

Quoted 5 lines
(quoted diff) +0 -2
-  int string_left_offset = self->icon_width + icon_text_space;
-  int string_top_offset = (item_height - (ascent + descent)) / 2 +

ascent;

grub-core/gfxmenu/gui_list.c +2 -0
+  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.

Signature · 4 lines
With best regards,
_______________________________
Vladimir Testov, ROSA Laboratory.
www.rosalab.ru