[PATCH] kernelbase: Partially implement SetCurrentConsoleFontEx

Hugh McMaster hugh.mcmaster at outlook.com
Tue Feb 8 02:51:15 CST 2022


Hi Jacek,

Thanks for the review.

On Sat, 5 Feb 2022 at 04:29, Jacek Caban wrote:
>
> Hi Hugh,
>
> On 2/2/22 12:10, Hugh McMaster wrote:
> > +struct condrv_output_info_params_font {
> > +    struct condrv_output_info_params params;
> > +    WCHAR face_name[LF_FACESIZE];
> > +};
>
>
> Since you use it only for SetCurrentConsoleFontEx, you could just move
> it there. See how GetCurrentConsoleFontEx is handling it.

We could use the struct for GetCurrentConsoleFontEx too, but it
doesn't really matter.

> Also, ioclt()
> size should probably only contain meaningful bytes (no extra
> uninitialized LF_FACESIZE bytes and no null-byte). On conhost side, we
> can calculate string length from params size.
>
>
> >   #define SET_CONSOLE_OUTPUT_INFO_CURSOR_GEOM     0x0001
> >   #define SET_CONSOLE_OUTPUT_INFO_CURSOR_POS      0x0002
> >   #define SET_CONSOLE_OUTPUT_INFO_SIZE            0x0004
> > @@ -155,6 +160,7 @@ struct condrv_output_info_params
> >   #define SET_CONSOLE_OUTPUT_INFO_DISPLAY_WINDOW  0x0010
> >   #define SET_CONSOLE_OUTPUT_INFO_MAX_SIZE        0x0020
> >   #define SET_CONSOLE_OUTPUT_INFO_POPUP_ATTR      0x0040
> > +#define SET_CONSOLE_OUTPUT_INFO_FONT            0x0080
> >
> >   /* IOCTL_CONDRV_FILL_OUTPUT params */
> >   struct condrv_fill_output_params
> > diff --git a/programs/conhost/conhost.c b/programs/conhost/conhost.c
> > index 78f6e345170..456f3a4889e 100644
> > --- a/programs/conhost/conhost.c
> > +++ b/programs/conhost/conhost.c
> > @@ -1913,6 +1913,13 @@ static NTSTATUS set_output_info( struct screen_buffer *screen_buffer,
> >           screen_buffer->max_width  = info->max_width;
> >           screen_buffer->max_height = info->max_height;
> >       }
> > +    if (params->mask & SET_CONSOLE_OUTPUT_INFO_FONT)
> > +    {
> > +        WCHAR *face_name = (WCHAR *)(params + 1);
> > +
> > +        update_console_font( screen_buffer->console, face_name,
> > +                             info->font_height, info->font_weight );
> > +    }
>
>
> Looking at update_console_font, if creating a font with passed arguments
> fails, it will set a first found font, ignoring all passed arguments.
> Should it return a failure instead?

SetCurrentConsoleFontEx only ever fails if the console output handle
is invalid. If it can't match all parameters, it just sets the closest
possible font.

When testing various font parameters, I saw the following behaviour:
  - Font height of 0 returned a font height of 12 on all testbot VMs.
It's not clear where the 12 comes from.
  - Font weight of 0 (invalid) to 500 returned FW_NORMAL, and 600 to
900 returned FW_BOLD.
  - Font family of 0 returned various defaults, 48 or 54.
  - Empty or invalid font face returned either Terminal or a TrueType
font. It's not clear how the font face is chosen. It might be due to a
registry setting.

I think we should leave the font face and font family as they are if
we encounter those edge cases. We can handle the font height and
weight cases though.



More information about the wine-devel mailing list