|
From: | Philippe Mathieu-Daudé |
Subject: | Re: [PATCH] display/gtk: get proper refreshrate |
Date: | Tue, 31 Dec 2019 14:48:33 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 |
Hi Nikola, Cc'ing the qemu-devel list. On 12/31/19 1:38 AM, Nikola Pavlica wrote:
On Mon, 2019-12-30 at 23:59 +0100, Philippe Mathieu-Daudé wrote:Hi Nikola, Thanks for your patch! On 12/30/19 6:28 PM, Nikola Pavlica wrote:From 70c95b18fa056b2dd0ecc202ab517bc775b986da Mon Sep 17 00:00:00 2001 From: Nikola Pavlica <address@hidden> Date: Mon, 30 Dec 2019 18:17:35 +0100 Subject: [PATCH] display/gtk: get proper refreshrateCan you describe here the problem you encountered, and how your patch fixes it?Signed-off-by: Nikola Pavlica <address@hidden> --- ui/gtk.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ui/gtk.c b/ui/gtk.c index 692ccc7bbb..7a041457f2 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -2259,6 +2259,11 @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) opts->u.gtk.grab_on_hover) { gtk_menu_item_activate(GTK_MENU_ITEM(s-grab_on_hover_item));} + + GdkDisplay *display = gdk_display_get_default();Can we use window_display declared earlier instead? window_display = gtk_widget_get_display(s->window); If you look at the CODING_STYLE.rst file referenced here: https://wiki.qemu.org/Contribute/SubmitAPatch#Use_the_QEMU_coding_style It states: Declarations ============ Mixed declarations (interleaving statements and declarations within blocks) are generally not allowed; declarations should be at the beginning of blocks. So you should move the declaration of both display/monitor variables earlier, around line 2192.+ GdkMonitor *monitor = gdk_display_get_primary_monitor(display); + vc->gfx.dcl.update_interval = 1000000 / + gdk_monitor_get_refresh_rate(monitor);Now looking at this line, I think this should be done in the gd_vc_gfx_init() function (line 2029, before the register_displaychangelistener() call).}static void early_gtk_display_init(DisplayOptions *opts)As suggested on IRC, your patch have more chances to get reviewed if you Cc its maintainers. See this help here: https://wiki.qemu.org/Contribute/SubmitAPatch#CC_the_relevant_maintainer In this case we get: $ ./scripts/get_maintainer.pl -f ui/gtk.c Gerd Hoffmann <address@hidden> (odd fixer:Graphics) address@hidden (open list:All patches CC here) I'm Cc'ing Gerd for you. Regards, Phil.
This patch is hard to review because it follows another patch...See: https://wiki.qemu.org/Contribute/SubmitAPatch#Do_not_send_as_an_attachment
This is v2, next time post a v3 as a new thread please.
From f4934509ac2da1bbb747422990587433ecc44a0b Mon Sep 17 00:00:00 2001 From: Nikola Pavlica <address@hidden> Date: Tue, 31 Dec 2019 01:12:42 +0100 Subject: [PATCH v2] display/gtk: get proper refreshrate
<here goes the description of problem and fix>
Signed-off-by: Nikola Pavlica <address@hidden> --- ui/gtk.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ui/gtk.c b/ui/gtk.c index 692ccc7bbb..2055dcc03d 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -1967,6 +1967,10 @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, VirtualConsole *vc, { bool zoom_to_fit = false;+ GdkDisplay *dpy = gtk_widget_get_display(s->window);+ GdkWindow *win = gtk_widget_get_window(s->window); + GdkMonitor *monitor = gdk_display_get_monitor_at_window(dpy, win); + vc->label = qemu_console_get_label(con); vc->s = s; vc->gfx.scale_x = 1.0; @@ -2026,6 +2030,8 @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, VirtualConsole *vc,vc->gfx.kbd = qkbd_state_init(con);vc->gfx.dcl.con = con; + vc->gfx.dcl.update_interval = 1000000 / + gdk_monitor_get_refresh_rate(monitor);
From the documentation of gdk_monitor_get_refresh_rate(): The value is in milli-Hertz, so a refresh rate of 60Hz is returned as 60000. Returns the refresh rate in milli-Hertz, or 0Watch out we don't want to crash on division by zero. Maybe we don't want to set the update_interval if the refresh rate is null.
Also, I'd try to avoid the magic value and define something more obvious to review, such:
#define MSEC_PER_MILLIHZ 1000000 ... int refresh_rate_millihz; ... refresh_rate_millihz = gdk_monitor_get_refresh_rate() if (refresh_rate_millihz) {vc->gfx.dcl.update_interval = MSEC_PER_MILLIHZ / refresh_rate_millihz;
}
register_displaychangelistener(&vc->gfx.dcl);gd_connect_vc_gfx_signals(vc);
[Prev in Thread] | Current Thread | [Next in Thread] |