Re: [PATCH RESEND v3 2/2] term/gfxterm: Preliminary HiDPI support

[Paul Menzel]

Dear Zhang,


Thank you for your patches for a much needed feature.

Maybe make the commit message summary/title a statement:

term/gfxterm: Add preliminary HiDPI support

Am 27.06.22 um 11:35 schrieb Zhang Boyang:
> Currently GRUB's default font is too small to see on a HiDPI monitor.
> This patch adds preliminary HiDPI support to gfxterm. It introduces a
> new environment variable 'gfxterm_scale'. If set to 0, and a high
> resolution monitor is detected, it will scale the font size
> automatically. If set to other number, that number will be the scale
> factor, overriding automatic scale factor calculation.

Trying to work around the issue, I found the answer to *How can I change 
the resolution of the GRUB menu?* [1] saying to create bigger fonts, 
referencing *Increase Font in GRUB for High DPI Screens* [2]:

    sudo grub-mkfont --output=/boot/grub/fonts/DejaVuSansMono24.pf2 \
  --size=24 /usr/share/fonts/truetype/dejavu/DejaVuSansMono.ttf

    GRUB_FONT=/boot/grub/fonts/DejaVuSansMono24.pf2

Can you elaborate how your scaling approach works quality wise? (I have 
no idea about the font format (pixel, vector?).)

> Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
> ---
>   docs/grub.texi           | 11 ++++++
>   grub-core/gfxmenu/view.c |  1 +
>   grub-core/term/gfxterm.c | 72 ++++++++++++++++++++++++++++------------
>   include/grub/gfxterm.h   |  3 +-
>   4 files changed, 64 insertions(+), 23 deletions(-)
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 9b902273c..8f82061f6 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -3274,6 +3274,7 @@ These variables have special meaning to GRUB.
>   * gfxmode::
>   * gfxpayload::
>   * gfxterm_font::
> +* gfxterm_scale::
>   * grub_cpu::
>   * grub_platform::
>   * icondir::
> @@ -3548,6 +3549,16 @@ If this variable is set, it names a font to use for text 
> on the
>   available font.
>   
>   
> +@node gfxterm_scale
> +@subsection gfxterm_scale
> +
> +If this variable is not set, or set to @samp{0}, the @samp{gfxterm}
> +graphical terminal will scale the font automatically when a high resolution

*the* graphical terminal

How will it be scaled?

> +monitor is detected.  If set to other number, the font scale factor will be
> +forced to that number.  Set this to @samp{1} if user don't want

… if the user doesn’t want …

> +@samp{gfxterm} to scale the font on screen.
> +
> +
>   @node grub_cpu
>   @subsection grub_cpu
>   
> diff --git a/grub-core/gfxmenu/view.c b/grub-core/gfxmenu/view.c
> index 6358004b2..94b9ef4db 100644
> --- a/grub-core/gfxmenu/view.c
> +++ b/grub-core/gfxmenu/view.c
> @@ -546,6 +546,7 @@ init_terminal (grub_gfxmenu_view_t view)
>                              view->terminal_rect.height,
>                              view->double_repaint,
>                              terminal_font,
> +                           1,
>                              view->terminal_border);
>     grub_gfxterm_decorator_hook = grub_gfxmenu_draw_terminal_box;
>   }
> diff --git a/grub-core/term/gfxterm.c b/grub-core/term/gfxterm.c
> index 4512dee6f..df2d3f86b 100644
> --- a/grub-core/term/gfxterm.c
> +++ b/grub-core/term/gfxterm.c
> @@ -82,6 +82,7 @@ struct grub_virtual_screen
>   
>     /* Font settings. */
>     grub_font_t font;
> +  unsigned int scale;
>   
>     /* Terminal color settings.  */
>     grub_uint8_t standard_color_setting;
> @@ -204,7 +205,7 @@ grub_virtual_screen_free (void)
>   static grub_err_t
>   grub_virtual_screen_setup (unsigned int x, unsigned int y,
>                              unsigned int width, unsigned int height,
> -                          grub_font_t font)
> +                          grub_font_t font, unsigned int scale)
>   {
>     unsigned int i;
>   
> @@ -213,6 +214,7 @@ grub_virtual_screen_setup (unsigned int x, unsigned int y,
>   
>     /* Initialize with default data.  */
>     virtual_screen.font = font;
> +  virtual_screen.scale = scale;
>     virtual_screen.width = width;
>     virtual_screen.height = height;
>     virtual_screen.offset_x = x;
> @@ -220,9 +222,9 @@ grub_virtual_screen_setup (unsigned int x, unsigned int y,
>     virtual_screen.normal_char_width =
>       calculate_normal_character_width (virtual_screen.font);
>     virtual_screen.normal_char_height =
> -    grub_font_get_max_char_height (virtual_screen.font);
> +    grub_font_get_max_char_height (virtual_screen.font) * virtual_screen.scale;
>     if (virtual_screen.normal_char_height == 0)
> -    virtual_screen.normal_char_height = 16;
> +    virtual_screen.normal_char_height = 16 * virtual_screen.scale;
>     virtual_screen.cursor_x = 0;
>     virtual_screen.cursor_y = 0;
>     virtual_screen.cursor_state = 1;
> @@ -297,7 +299,8 @@ grub_err_t
>   grub_gfxterm_set_window (struct grub_video_render_target *target,
>                          int x, int y, int width, int height,
>                          int double_repaint,
> -                        grub_font_t font, int border_width)
> +                        grub_font_t font, int scale,
> +                        int border_width)
>   {
>     /* Clean up any prior instance.  */
>     destroy_window ();
> @@ -306,10 +309,10 @@ grub_gfxterm_set_window (struct grub_video_render_target 
> *target,
>     render_target = target;
>   
>     /* Create virtual screen.  */
> -  if (grub_virtual_screen_setup (border_width, border_width,
> -                                 width - 2 * border_width,
> -                                 height - 2 * border_width,
> -                                 font)
> +  if (grub_virtual_screen_setup (border_width * scale, border_width * scale,
> +                                 width - 2 * border_width * scale,
> +                                 height - 2 * border_width * scale,
> +                                 font, scale)
>         != GRUB_ERR_NONE)
>       {
>         return grub_errno;
> @@ -332,11 +335,13 @@ static grub_err_t
>   grub_gfxterm_fullscreen (void)
>   {
>     const char *font_name;
> +  const char *scale_conf;
>     struct grub_video_mode_info mode_info;
>     grub_video_color_t color;
>     grub_err_t err;
>     int double_redraw;
>     grub_font_t font;
> +  int scale;
>   
>     err = grub_video_get_info (&mode_info);
>     /* Figure out what mode we ended up.  */
> @@ -366,12 +371,34 @@ grub_gfxterm_fullscreen (void)
>     if (!font)
>       return grub_error (GRUB_ERR_BAD_FONT, "no font loaded");
>   
> +  /* Decide font scale factor. */
> +  scale_conf = grub_env_get ("gfxterm_scale");
> +  scale = 0;
> +  if (scale_conf)
> +    scale = (int) grub_strtoul (scale_conf, 0, 0);
> +  if (scale < 0 || scale > GRUB_FONT_MAX_SCALE)
> +    scale = 0;
> +  if (scale == 0)
> +    {
> +      if (mode_info.width > 7680 && mode_info.height > 4320)
> +       scale = 8;
> +      else if (mode_info.width > 3840 && mode_info.height > 2160)
> +       scale = 4;
> +      else if (mode_info.width > 1920 && mode_info.height > 1080)
> +       scale = 2;
> +      else
> +       scale = 1;
> +    }

Would checking the width be enough, and then set scale to 
mode_info.width % 1920?

> +  if (grub_font_get_max_char_width(font) * scale > GRUB_FONT_MAX_DIMENSION
> +      || grub_font_get_max_char_height(font) * scale > GRUB_FONT_MAX_DIMENSION)
> +    scale = 1;
> +
>     grub_gfxterm_decorator_hook = NULL;
>   
>     return grub_gfxterm_set_window (GRUB_VIDEO_RENDER_TARGET_DISPLAY,
>                                   0, 0, mode_info.width, mode_info.height,
>                                   double_redraw,
> -                                 font, DEFAULT_BORDER_WIDTH);
> +                                 font, scale, DEFAULT_BORDER_WIDTH);
>   }
>   
>   static grub_err_t
> @@ -642,7 +669,7 @@ paint_char (unsigned cx, unsigned cy)
>         grub_errno = GRUB_ERR_NONE;
>         return;
>       }
> -  ascent = grub_font_get_ascent (virtual_screen.font);
> +  ascent = grub_font_get_ascent (virtual_screen.font) * virtual_screen.scale;
>   
>     width = virtual_screen.normal_char_width * calculate_character_width(glyph);
>     height = virtual_screen.normal_char_height;
> @@ -656,7 +683,7 @@ paint_char (unsigned cx, unsigned cy)
>     /* Render glyph to text layer.  */
>     grub_video_set_active_render_target (text_layer);
>     grub_video_fill_rect (bgcolor, x, y, width, height);
> -  grub_font_draw_glyph (glyph, color, x, y + ascent, 1);
> +  grub_font_draw_glyph (glyph, color, x, y + ascent, virtual_screen.scale);
>     grub_video_set_active_render_target (render_target);
>   
>     /* Mark character to be drawn.  */
> @@ -690,9 +717,9 @@ draw_cursor (int show)
>       return;
>   
>     /* Ensure that cursor doesn't go outside of character box.  */
> -  ascent = grub_font_get_ascent(virtual_screen.font);
> -  if (ascent > virtual_screen.normal_char_height - 2)
> -    ascent = virtual_screen.normal_char_height - 2;
> +  ascent = grub_font_get_ascent(virtual_screen.font) * virtual_screen.scale;
> +  if (ascent > virtual_screen.normal_char_height - 2 * virtual_screen.scale)
> +    ascent = virtual_screen.normal_char_height - 2 * virtual_screen.scale;
>   
>     /* Determine cursor properties and position on text layer. */
>     x = virtual_screen.cursor_x * virtual_screen.normal_char_width;
> @@ -701,7 +728,7 @@ draw_cursor (int show)
>     y = ((virtual_screen.cursor_y + virtual_screen.total_scroll)
>          * virtual_screen.normal_char_height
>          + ascent);
> -  height = 2;
> +  height = 2 * virtual_screen.scale;
>   
>     /* Render cursor to text layer.  */
>     grub_video_set_active_render_target (text_layer);
> @@ -957,18 +984,18 @@ calculate_normal_character_width (grub_font_t font)
>         width = glyph->device_width;
>       }
>     if (!width)
> -    return 8;
> +    return 8 * virtual_screen.scale;
>   
> -  return width;
> +  return width * virtual_screen.scale;
>   }
>   
>   static unsigned char
>   calculate_character_width (struct grub_font_glyph *glyph)
>   {
>     if (! glyph || glyph->device_width == 0)
> -    return 1;
> +    return 1 * virtual_screen.scale;
>   
> -  return (glyph->device_width
> +  return (glyph->device_width * virtual_screen.scale
>             + (virtual_screen.normal_char_width - 1))
>            / virtual_screen.normal_char_width;
>   }
> @@ -981,10 +1008,11 @@ grub_gfxterm_getcharwidth (struct grub_term_output *term 
> __attribute__ ((unused)
>     dev_width = grub_font_get_constructed_device_width (virtual_screen.font, c);
>   
>     if (dev_width == 0)
> -    return 1;
> +    return 1 * virtual_screen.scale;
>   
> -  return (dev_width + (virtual_screen.normal_char_width - 1))
> -    / virtual_screen.normal_char_width;
> +  return (dev_width * virtual_screen.scale
> +          + (virtual_screen.normal_char_width - 1))
> +         / virtual_screen.normal_char_width;
>   }
>   
>   static struct grub_term_coordinate
> diff --git a/include/grub/gfxterm.h b/include/grub/gfxterm.h
> index 7e1ff6dfc..73735a26e 100644
> --- a/include/grub/gfxterm.h
> +++ b/include/grub/gfxterm.h
> @@ -29,7 +29,8 @@ grub_err_t
>   EXPORT_FUNC (grub_gfxterm_set_window) (struct grub_video_render_target 
> *target,
>                                        int x, int y, int width, int height,
>                                        int double_repaint,
> -                                      grub_font_t font, int border_width);
> +                                      grub_font_t font, int scale,
> +                                      int border_width);
>   
>   void EXPORT_FUNC (grub_gfxterm_schedule_repaint) (void);
>   


Kind regards,

Paul


[1]: 
https://askubuntu.com/questions/1091778/how-can-i-change-the-resolution-of-the-grub-menu
[2]: https://blog.wxm.be/2014/08/29/increase-font-in-grub-for-high-dpi.html