[PATCH] fix menu size calculation in presence of selected item pixmap

[PATCH] fix menu size calculation in presence of selected item pixmap

[Andrey Borzenkov]

draw_menu() acconted for upper border of selected item pixmap, but not
for lower. So it could happen that lower border was visually cut off.

Also various calculations of menu vertical size did not account for
space for lower border

Signed-off-by: Andrey Borzenkov <arvidjaar@gmail.com>

---
 grub-core/gfxmenu/gui_list.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/grub-core/gfxmenu/gui_list.c b/grub-core/gfxmenu/gui_list.c
index 1982d9a..5c83383 100644
--- a/grub-core/gfxmenu/gui_list.c
+++ b/grub-core/gfxmenu/gui_list.c
@@ -97,8 +97,14 @@ get_num_shown_items (list_impl_t self)
   grub_gfxmenu_box_t box = self->menu_box;
   int box_top_pad = box->get_top_pad (box);
   int box_bottom_pad = box->get_bottom_pad (box);
+
+  grub_gfxmenu_box_t selbox = self->selected_item_box;
+  int sel_top_pad = selbox->get_top_pad (selbox);
+  int sel_bottom_pad = selbox->get_bottom_pad (selbox);
+      
       
   return (self->bounds.height + item_vspace - 2 * boxpad
+	  - sel_top_pad - sel_bottom_pad
 	  - box_top_pad - box_bottom_pad) / (item_height + item_vspace);
 }
 
@@ -392,7 +398,8 @@ list_get_minimal_size (void *vself, unsigned *width, unsigned *height)
       unsigned width_s;
 
       grub_gfxmenu_box_t selbox = self->selected_item_box;
-      int sel_toppad = selbox->get_top_pad (selbox);
+      int sel_top_pad = selbox->get_top_pad (selbox);
+      int sel_bottom_pad = selbox->get_bottom_pad (selbox);
       
       *width = grub_font_get_string_width (self->item_font, "Typical OS");
       width_s = grub_font_get_string_width (self->selected_item_font,
@@ -406,7 +413,7 @@ list_get_minimal_size (void *vself, unsigned *width, unsigned *height)
       *height = (item_height * num_items
                  + item_vspace * (num_items - 1)
                  + 2 * boxpad
-                 + box_top_pad + box_bottom_pad + sel_toppad);
+                 + box_top_pad + box_bottom_pad + sel_top_pad + sel_bottom_pad);
     }
   else
     {
-- 
tg: (cc35b49..) e/menu-selected-pad (depends on: master)

Re: [PATCH] fix menu size calculation in presence of selected item pixmap

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 21.02.2013 18:36, Andrey Borzenkov wrote:

> draw_menu() acconted for upper border of selected item pixmap, but not
> for lower. So it could happen that lower border was visually cut off.
> 
> Also various calculations of menu vertical size did not account for
> space for lower border
> 
> Signed-off-by: Andrey Borzenkov <arvidjaar@gmail.com>
> 
> ---
>  grub-core/gfxmenu/gui_list.c |   11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/grub-core/gfxmenu/gui_list.c b/grub-core/gfxmenu/gui_list.c
> index 1982d9a..5c83383 100644
> --- a/grub-core/gfxmenu/gui_list.c
> +++ b/grub-core/gfxmenu/gui_list.c
> @@ -97,8 +97,14 @@ get_num_shown_items (list_impl_t self)
>    grub_gfxmenu_box_t box = self->menu_box;
>    int box_top_pad = box->get_top_pad (box);
>    int box_bottom_pad = box->get_bottom_pad (box);
> +
> +  grub_gfxmenu_box_t selbox = self->selected_item_box;
> +  int sel_top_pad = selbox->get_top_pad (selbox);
> +  int sel_bottom_pad = selbox->get_bottom_pad (selbox);
> +      
>        
>    return (self->bounds.height + item_vspace - 2 * boxpad
> +	  - sel_top_pad - sel_bottom_pad

I don't see the logic behind this. item_top in draw_menu doesn't
increase more on selected item than on any other one.

>  	  - box_top_pad - box_bottom_pad) / (item_height + item_vspace);
>  }
>  
> @@ -392,7 +398,8 @@ list_get_minimal_size (void *vself, unsigned *width, unsigned *height)
>        unsigned width_s;
>  
>        grub_gfxmenu_box_t selbox = self->selected_item_box;
> -      int sel_toppad = selbox->get_top_pad (selbox);
> +      int sel_top_pad = selbox->get_top_pad (selbox);
> +      int sel_bottom_pad = selbox->get_bottom_pad (selbox);
>        
>        *width = grub_font_get_string_width (self->item_font, "Typical OS");
>        width_s = grub_font_get_string_width (self->selected_item_font,
> @@ -406,7 +413,7 @@ list_get_minimal_size (void *vself, unsigned *width, unsigned *height)
>        *height = (item_height * num_items
>                   + item_vspace * (num_items - 1)
>                   + 2 * boxpad
> -                 + box_top_pad + box_bottom_pad + sel_toppad);
> +                 + box_top_pad + box_bottom_pad + sel_top_pad + sel_bottom_pad);
>      }
>    else
>      {




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: [PATCH] fix menu size calculation in presence of selected item pixmap

[Andrey Borzenkov]

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

В Mon, 08 Apr 2013 20:51:17 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:

> On 21.02.2013 18:36, Andrey Borzenkov wrote:
> 
> > draw_menu() acconted for upper border of selected item pixmap, but not
> > for lower. So it could happen that lower border was visually cut off.
> > 
> > Also various calculations of menu vertical size did not account for
> > space for lower border
> > 
> > Signed-off-by: Andrey Borzenkov <arvidjaar@gmail.com>
> > 
> > ---
> >  grub-core/gfxmenu/gui_list.c |   11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/grub-core/gfxmenu/gui_list.c b/grub-core/gfxmenu/gui_list.c
> > index 1982d9a..5c83383 100644
> > --- a/grub-core/gfxmenu/gui_list.c
> > +++ b/grub-core/gfxmenu/gui_list.c
> > @@ -97,8 +97,14 @@ get_num_shown_items (list_impl_t self)
> >    grub_gfxmenu_box_t box = self->menu_box;
> >    int box_top_pad = box->get_top_pad (box);
> >    int box_bottom_pad = box->get_bottom_pad (box);
> > +
> > +  grub_gfxmenu_box_t selbox = self->selected_item_box;
> > +  int sel_top_pad = selbox->get_top_pad (selbox);
> > +  int sel_bottom_pad = selbox->get_bottom_pad (selbox);
> > +      
> >        
> >    return (self->bounds.height + item_vspace - 2 * boxpad
> > +	  - sel_top_pad - sel_bottom_pad
> 
> I don't see the logic behind this. item_top in draw_menu doesn't
> increase more on selected item than on any other one.
> 

Layout of menu entries used by draw_menu

sel_top_pad
entry 1
item_vspace
entry 2
item_vspace
...
entry n
sel_bottom_pad

draw_menu() explicitly offsets start of the first menu entry by
sel_top_pad (int item_top = sel_toppad;)

Currently get_num_shown_items can return more items than would fit
using above scheme. Which results in cut off image below last visible
entry. The updated formula ensures all elements are always displayed in
full.

Vladimir already demonstrated it a while back with screenshots.

> >  	  - box_top_pad - box_bottom_pad) / (item_height + item_vspace);
> >  }
> >  
> > @@ -392,7 +398,8 @@ list_get_minimal_size (void *vself, unsigned *width, unsigned *height)
> >        unsigned width_s;
> >  
> >        grub_gfxmenu_box_t selbox = self->selected_item_box;
> > -      int sel_toppad = selbox->get_top_pad (selbox);
> > +      int sel_top_pad = selbox->get_top_pad (selbox);
> > +      int sel_bottom_pad = selbox->get_bottom_pad (selbox);
> >        
> >        *width = grub_font_get_string_width (self->item_font, "Typical OS");
> >        width_s = grub_font_get_string_width (self->selected_item_font,
> > @@ -406,7 +413,7 @@ list_get_minimal_size (void *vself, unsigned *width, unsigned *height)
> >        *height = (item_height * num_items
> >                   + item_vspace * (num_items - 1)
> >                   + 2 * boxpad
> > -                 + box_top_pad + box_bottom_pad + sel_toppad);
> > +                 + box_top_pad + box_bottom_pad + sel_top_pad + sel_bottom_pad);
> >      }
> >    else
> >      {
> 
> 
> 


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] fix menu size calculation in presence of selected item pixmap

[Vladimir Testov]

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

>I don't see the logic behind this. item_top in draw_menu doesn't
>increase more on selected item than on any other one.

Current function does not take into account pixmap style of the selected item.
This pixmap style needs for additional space to be drawn. 
Overall height of the selected item is:
sel_top_pad + sel_bottom_pad + item_height - 1

So the number of shown items is miscalculated.
And in some cases we will have the bottom item not fully drawn. I can sent a screenshot.
-- 
With best regards,
_______________________________
Vladimir Testov, ROSA Laboratory.
www.rosalab.ru

[-- Attachment #2: Type: text/html, Size: 3665 bytes --]