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.