Hi all,
* I checked the links by Daniel (thanks for providing).
* As I understand it Zhangs patches aim to add a high dpi scaling feature
for the fonts
* In contrast, the patch I submitted (albeit having a similar use-case) adds
the possibility
for grub "scripts" / config files to access & act upon the currently used
display resolution
* for me both things would have their merit and independent use-cases
What is your opinion on having both features?
Besides, I am willing to help progress both features - if someone can guide
me /give a hint
how to proceed best?
Thank you
Markus
-----Ursprüngliche Nachricht-----
Von: Daniel Kiper <dkiper@net-space.pl>
Gesendet: Donnerstag, 20. Oktober 2022 19:36
An: Markus Scholz <scholz@novelsense.com>
Cc: grub-devel@gnu.org; phcoder@gmail.com; pmenzel@molgen.mpg.de;
zhangboyang.id@gmail.com
Betreff: Re: Adding --set to videoinfo to retrieve current video resolution
Adding Paul, Vladimir, and Zhang...
On Wed, Oct 12, 2022 at 03:36:13PM +0200, Markus Scholz wrote:
Dear Grub Developers,
recently we faced the problem the author in this old thread faced:
- https://lists.gnu.org/archive/html/grub-devel/2012-10/msg00009.html
Ie. our theme wouldn't display properly due to different screen
resolutions.
Hence, I wanted to change specifically the font size of our theme
given the current resolution.
While the videoinfo module provides info about the current resolution
I wasn't able to discover any way to get the info from the booted
grub.
Following the aforementioned thread, I therefore patched the videoinfo
mod to add the ability of a -set switch just like commands like
"smbinfo" or "search" have.
Ie. the documentation of the videoinfo call could change as follows:
Command: videoinfo [--set var | [WxH]xD]
Retrieve available video modes. If --set is given, assign the
currently active resolution to var.
If --set is not provided the available video modes are listed.
If resolution is given, show only matching modes.
Attached is my patch proposal for the module; I probably made a lot
mistakes wrt.
coding style and guidelines - please bear with me.
I would be grateful regarding feedback what I could do / how I could
improve the patch to make it part of grub? If it possible at all with this
approach?
I think fix for similar problem has been proposed here [1] and I commented
it here [2]. Sadly it ended in limbo. Could you work with Zhang on this
issue?
Daniel
[1] https://lists.gnu.org/archive/html/grub-devel/2022-06/msg00143.html
[2] https://lists.gnu.org/archive/html/grub-devel/2022-07/msg00027.html
Thank you for your reply & best,
Markus
---
--- grub-mod/grub-core/commands/videoinfo.c 2022-10-12
13:30:54.992451000 +0000
+++ grub/grub-core/commands/videoinfo.c 2022-10-12
13:31:37.715512000 +0000
@@ -30,7 +30,6 @@ GRUB_MOD_LICENSE ("GPLv3+"); struct hook_ctx {
unsigned height, width, depth;
- unsigned char set_variable;
struct grub_video_mode_info *current_mode; };
@@ -39,9 +38,6 @@ hook (const struct grub_video_mode_info {
struct hook_ctx *ctx = hook_arg;
- if (ctx->set_variable) /* if variable should be set don't print all
modes */
- return 0;
-
if (ctx->height && ctx->width && (info->width != ctx->width ||
info->height != ctx->height))
return 0;
@@ -136,40 +132,31 @@ grub_cmd_videoinfo (grub_command_t cmd _
grub_video_adapter_t adapter;
grub_video_driver_id_t id;
struct hook_ctx ctx;
-
- ctx.height = ctx.width = ctx.depth = ctx.set_variable = 0;
-
+
+ ctx.height = ctx.width = ctx.depth = 0;
if (argc)
{
- if (argc == 2 && grub_memcmp(args[0], "--set", sizeof ("--set") -
1)
== 0 )
- ctx.set_variable = 1;
- else
- {
const char *ptr;
ptr = args[0];
ctx.width = grub_strtoul (ptr, &ptr, 0);
if (grub_errno)
- return grub_errno;
+ return grub_errno;
if (*ptr != 'x')
- return grub_error (GRUB_ERR_BAD_ARGUMENT,
- N_("invalid video mode specification `%s'"),
- args[0]);
+ return grub_error (GRUB_ERR_BAD_ARGUMENT,
+ N_("invalid video mode specification `%s'"),
+ args[0]);
ptr++;
ctx.height = grub_strtoul (ptr, &ptr, 0);
if (grub_errno)
- return grub_errno;
+ return grub_errno;
if (*ptr == 'x')
- {
- ptr++;
- ctx.depth = grub_strtoul (ptr, &ptr, 0);
- if (grub_errno)
- return grub_errno;
- }
- }
-
+ {
+ ptr++;
+ ctx.depth = grub_strtoul (ptr, &ptr, 0);
+ if (grub_errno)
+ return grub_errno;
+ }
}
-
-
#ifdef GRUB_MACHINE_PCBIOS
if (grub_strcmp (cmd->name, "vbeinfo") == 0) @@ -177,23 +164,20 @@
grub_cmd_videoinfo (grub_command_t cmd _ #endif
id = grub_video_get_driver_id ();
- if (!ctx.set_variable)
- {
-grub_puts_ (N_("List of supported video modes:")); -grub_puts_
(N_("Legend: mask/position=red/green/blue/reserved"));
- }
-
+
+ grub_puts_ (N_("List of supported video modes:")); grub_puts_
+ (N_("Legend: mask/position=red/green/blue/reserved"));
+
FOR_VIDEO_ADAPTERS (adapter)
{
struct grub_video_mode_info info;
struct grub_video_edid_info edid_info;
- if (!ctx.set_variable)
- grub_printf_ (N_("Adapter `%s':\n"), adapter->name);
+
+ grub_printf_ (N_("Adapter `%s':\n"), adapter->name);
if (!adapter->iterate)
{
- if (!ctx.set_variable)
- grub_puts_ (N_(" No info available"));
+ grub_puts_ (N_(" No info available"));
continue;
}
@@ -202,18 +186,7 @@ grub_puts_ (N_("Legend: mask/position=re
if (adapter->id == id)
{
if (grub_video_get_info (&info) == GRUB_ERR_NONE)
- {
- ctx.current_mode = &info;
- if (ctx.set_variable)
- {
- /* set grub env var to current mode */
- char screen_wxh[20];
- unsigned int width = info.width;
- unsigned int height = info.height;
- grub_snprintf (screen_wxh, 20, "%ux%u", width, height);
- grub_env_set (args[1], screen_wxh);
- }
- }
+ ctx.current_mode = &info;
else
/* Don't worry about errors. */
grub_errno = GRUB_ERR_NONE;
@@ -263,26 +236,19 @@ GRUB_MOD_INIT(videoinfo)
/* TRANSLATORS: "x" has to be entered in,
like an identifier, so please don't
use better Unicode codepoints. */
- N_("[--set var | [WxH]xD]"),
- N_("Retrieve available video modes. "
- "--set is given, assign the currently"
- "active resolution to var. If --set "
- "is not provided the available video "
- "modes are listed. If resolution is "
- "given, show only matching modes."));
-
+ N_("[WxH[xD]]"),
+ N_("List available video modes. If "
+ "resolution is given show only modes"
+ " matching it."));
#ifdef GRUB_MACHINE_PCBIOS
cmd_vbe = grub_register_command ("vbeinfo", grub_cmd_videoinfo,
/* TRANSLATORS: "x" has to be entered in,
like an identifier, so please don't
use better Unicode codepoints. */
- N_("[--set var | [WxH]xD]"),
- N_("Retrieve available video modes. "
- "--set is given, assign the currently"
- "active resolution to var. If --set "
- "is not provided the available video "
- "modes are listed. If resolution is "
- "given, show only matching modes."));
+ N_("[WxH[xD]]"),
+ N_("List available video modes. If "
+ "resolution is given show only modes"
+ " matching it."));
#endif
}