Hi,
On 2022/12/6 22:17, Markus Scholz wrote:
Hi Zhang,
now I need to apologize for my very late reply.. sorry!
I saw that you also went ahead as you said with committing the highdpi
patches.
Anyways, regarding the high DPI question:
until now we have mostly focused on different screen resolutions in our
customer offerings and simply chose the font
size based on visual testing. So we did not attempt to solve this
"mathematically".
E.g. right now use:
- x_res >= 1024; bootmenu font ubuntu regular 20 (large theme)
- x_res == 1204; bootmenu font ubuntu regular 17 (medium theme)
- x_res < 1024 & unknown ; bootmenu font ubuntu regular 15 (small theme)
I don’t know if this helpful for you but that’s the current state we are
using to keep
the text readable given different resolutions. Of course, for the
different fonts/themes
(small, medium, large) the screens still look different i.e. the user
experience is not consistent.
Maybe using your highDPI patch it will become much easier to
make the appearance consistent and more appealing across different
screen resolutions.
Thanks for explaining your solution. My font scaling patches can't give
user consistent experience, because it only accept integer scaling factors.
By the way, Debian CDs use a fixed 800x600 resolution as far as I know,
and the user experience is consistent. However, its appearance is blurry
and this doesn't work on buggy firmwares which do not allow to switch to
800x600 resolution.
Note that we exclusively use gfxmode for our application, avoiding text
mode.
We believe that, for our application, this gives a much more polished
and mature look&feel.
Unfortunately my patches only work on gfxterm and there is no support
for gfxmenu yet (at least for now).
Nevertheless, I think it's good to have both my patches and your
patches, because I think we are focusing on different things.
Best Regards,
Zhang Boyang
Also thank you for your recommendation for the patch submission.
I will attempt to send another reply to this thread with the help of git
patch asap.
Best
Markus
-----Ursprüngliche Nachricht-----
Von: grub-devel-bounces+scholz=novelsense.com@gnu.org
<grub-devel-bounces+scholz=novelsense.com@gnu.org> Im Auftrag von Zhang
Boyang
Gesendet: Montag, 7. November 2022 08:56
An: Markus Scholz <scholz@novelsense.com>; grub-devel@gnu.org
Cc: phcoder@gmail.com; pmenzel@molgen.mpg.de; 'Daniel Kiper' <
dkiper@net-space.pl>
Betreff: Re: AW: Adding --set to videoinfo to retrieve current video
resolution
Hi,
Sorry for late reply... I think my patch is almost independent of
Markus's, and it would be good to have both. However, I think there are
some point we can share opinions. I'd like to ask your opinions of the
mechanism of choosing font size automatically. In my patch (preliminary),
it scales Unifont(16x16) M times on a N x 1080p screen, with M =
pow(2,ceil(log2(N))), e.g. 2x when (1080p, 2160p], 4x when (2160p, 4320p],
8x when (4320p, 8640p]. This approach seems not good enough, and it might
not suitable for custom themes. What's your opinion on this from your
perspective? I will probably go back to work on my HiDPI patch after few
weeks.
By the way, if I understand correctly, you patch seems reversed, and
your mail client seems corrupted your patch. You might find "git
format-patch" and "git send-email" useful :)
Best Regards,
Zhang Boyang
On 2022/10/24 19:23, Markus Scholz wrote:
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
}
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel