* [PATCH 1/3] ui/gtk: don't try to redefine SI prefixes
2020-12-13 16:56 [PATCH 0/3] ui/gtk: paper over a gtk bug on Windows Volker Rümelin
@ 2020-12-13 16:57 ` Volker Rümelin
2021-01-14 11:11 ` Philippe Mathieu-Daudé
2020-12-13 16:57 ` [PATCH 2/3] ui/gtk: rename variable window to widget Volker Rümelin
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Volker Rümelin @ 2020-12-13 16:57 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Philippe Mathieu-Daudé, QEMU, Nikola Pavlica
Redefining SI prefixes is always wrong. 1s has per definition
1000ms. Remove the misnamed named constant and replace it with
a comment explaining the frequency to period conversion in two
simple steps. Now you can cancel out the unit mHz in the comment
with the implicit unit mHz in refresh_rate_millihz and see why
the implicit unit ms for update_interval remains.
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
include/ui/gtk.h | 2 --
ui/gtk.c | 3 ++-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index eaeb450f91..80851fb4c7 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -24,8 +24,6 @@
#include "ui/egl-context.h"
#endif
-#define MILLISEC_PER_SEC 1000000
-
typedef struct GtkDisplayState GtkDisplayState;
typedef struct VirtualGfxConsole {
diff --git a/ui/gtk.c b/ui/gtk.c
index a752aa22be..86b386a20d 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -798,7 +798,8 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
refresh_rate_millihz = gd_refresh_rate_millihz(vc->window ?
vc->window : s->window);
if (refresh_rate_millihz) {
- vc->gfx.dcl.update_interval = MILLISEC_PER_SEC / refresh_rate_millihz;
+ /* T = 1 / f = 1 [s*Hz] / f = 1000*1000 [ms*mHz] / f */
+ vc->gfx.dcl.update_interval = 1000 * 1000 / refresh_rate_millihz;
}
fbw = surface_width(vc->gfx.ds);
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/3] ui/gtk: don't try to redefine SI prefixes
2020-12-13 16:57 ` [PATCH 1/3] ui/gtk: don't try to redefine SI prefixes Volker Rümelin
@ 2021-01-14 11:11 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-14 11:11 UTC (permalink / raw)
To: Volker Rümelin, Gerd Hoffmann; +Cc: QEMU, Nikola Pavlica
On 12/13/20 5:57 PM, Volker Rümelin wrote:
> Redefining SI prefixes is always wrong. 1s has per definition
> 1000ms. Remove the misnamed named constant and replace it with
> a comment explaining the frequency to period conversion in two
> simple steps. Now you can cancel out the unit mHz in the comment
> with the implicit unit mHz in refresh_rate_millihz and see why
> the implicit unit ms for update_interval remains.
>
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> ---
> include/ui/gtk.h | 2 --
> ui/gtk.c | 3 ++-
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/include/ui/gtk.h b/include/ui/gtk.h
> index eaeb450f91..80851fb4c7 100644
> --- a/include/ui/gtk.h
> +++ b/include/ui/gtk.h
> @@ -24,8 +24,6 @@
> #include "ui/egl-context.h"
> #endif
>
> -#define MILLISEC_PER_SEC 1000000
Indeed this is MICROSEC_PER_SEC.
> -
> typedef struct GtkDisplayState GtkDisplayState;
>
> typedef struct VirtualGfxConsole {
> diff --git a/ui/gtk.c b/ui/gtk.c
> index a752aa22be..86b386a20d 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -798,7 +798,8 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
> refresh_rate_millihz = gd_refresh_rate_millihz(vc->window ?
> vc->window : s->window);
> if (refresh_rate_millihz) {
> - vc->gfx.dcl.update_interval = MILLISEC_PER_SEC / refresh_rate_millihz;
> + /* T = 1 / f = 1 [s*Hz] / f = 1000*1000 [ms*mHz] / f */
> + vc->gfx.dcl.update_interval = 1000 * 1000 / refresh_rate_millihz;
> }
>
> fbw = surface_width(vc->gfx.ds);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3] ui/gtk: rename variable window to widget
2020-12-13 16:56 [PATCH 0/3] ui/gtk: paper over a gtk bug on Windows Volker Rümelin
2020-12-13 16:57 ` [PATCH 1/3] ui/gtk: don't try to redefine SI prefixes Volker Rümelin
@ 2020-12-13 16:57 ` Volker Rümelin
2020-12-13 16:57 ` [PATCH 3/3] ui/gtk: limit virtual console max update interval Volker Rümelin
2021-01-14 9:38 ` [PATCH 0/3] ui/gtk: paper over a gtk bug on Windows Gerd Hoffmann
3 siblings, 0 replies; 6+ messages in thread
From: Volker Rümelin @ 2020-12-13 16:57 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Philippe Mathieu-Daudé, QEMU, Nikola Pavlica
The type of the variable window is GtkWidget. Rename the variable
from window to widget, because windows and widgets are different
things.
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
ui/gtk.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/ui/gtk.c b/ui/gtk.c
index 86b386a20d..7ff9327b9d 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -752,13 +752,13 @@ static void gd_resize_event(GtkGLArea *area,
* If available, return the refresh rate of the display in milli-Hertz,
* else return 0.
*/
-static int gd_refresh_rate_millihz(GtkWidget *window)
+static int gd_refresh_rate_millihz(GtkWidget *widget)
{
#ifdef GDK_VERSION_3_22
- GdkWindow *win = gtk_widget_get_window(window);
+ GdkWindow *win = gtk_widget_get_window(widget);
if (win) {
- GdkDisplay *dpy = gtk_widget_get_display(window);
+ GdkDisplay *dpy = gtk_widget_get_display(widget);
GdkMonitor *monitor = gdk_display_get_monitor_at_window(dpy, win);
return gdk_monitor_get_refresh_rate(monitor);
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] ui/gtk: limit virtual console max update interval
2020-12-13 16:56 [PATCH 0/3] ui/gtk: paper over a gtk bug on Windows Volker Rümelin
2020-12-13 16:57 ` [PATCH 1/3] ui/gtk: don't try to redefine SI prefixes Volker Rümelin
2020-12-13 16:57 ` [PATCH 2/3] ui/gtk: rename variable window to widget Volker Rümelin
@ 2020-12-13 16:57 ` Volker Rümelin
2021-01-14 9:38 ` [PATCH 0/3] ui/gtk: paper over a gtk bug on Windows Gerd Hoffmann
3 siblings, 0 replies; 6+ messages in thread
From: Volker Rümelin @ 2020-12-13 16:57 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Philippe Mathieu-Daudé, QEMU, Nikola Pavlica
Limit the virtual console maximum update interval to
GUI_REFRESH_INTERVAL_DEFAULT. This papers over a integer
overflow bug in gtk3 on Windows where the reported monitor
refresh frequency can be much smaller than the real refresh
frequency.
The gtk bug report can be found here:
https://gitlab.gnome.org/GNOME/gtk/-/issues/3394
On my Windows 10 system gtk reports a monitor refresh rate of
1.511Hz instead of 60.031Hz and slows down the screen update
rate in qemu to a crawl. Provided you are affected by the gtk
bug on Windows, these are the steps to reproduce the issue:
Start qemu with -display gtk and activate all qemu virtual
consoles and notice the reduced qemu refresh rate. Activating
all virtual consoles is necessary, because gui_update() in
ui/console.c uses the minimum of all display change listeners
update interval and not yet activated virtual consoles report
the default update interval (30ms).
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
ui/gtk.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/ui/gtk.c b/ui/gtk.c
index 7ff9327b9d..78da5902f4 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -749,10 +749,10 @@ static void gd_resize_event(GtkGLArea *area,
#endif
/*
- * If available, return the refresh rate of the display in milli-Hertz,
- * else return 0.
+ * If available, return the update interval of the monitor in ms,
+ * else return 0 (the default update interval).
*/
-static int gd_refresh_rate_millihz(GtkWidget *widget)
+static int gd_monitor_update_interval(GtkWidget *widget)
{
#ifdef GDK_VERSION_3_22
GdkWindow *win = gtk_widget_get_window(widget);
@@ -760,8 +760,13 @@ static int gd_refresh_rate_millihz(GtkWidget *widget)
if (win) {
GdkDisplay *dpy = gtk_widget_get_display(widget);
GdkMonitor *monitor = gdk_display_get_monitor_at_window(dpy, win);
+ int refresh_rate = gdk_monitor_get_refresh_rate(monitor); /* [mHz] */
- return gdk_monitor_get_refresh_rate(monitor);
+ if (refresh_rate) {
+ /* T = 1 / f = 1 [s*Hz] / f = 1000*1000 [ms*mHz] / f */
+ return MIN(1000 * 1000 / refresh_rate,
+ GUI_REFRESH_INTERVAL_DEFAULT);
+ }
}
#endif
return 0;
@@ -774,7 +779,6 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
int mx, my;
int ww, wh;
int fbw, fbh;
- int refresh_rate_millihz;
#if defined(CONFIG_OPENGL)
if (vc->gfx.gls) {
@@ -795,12 +799,8 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
return FALSE;
}
- refresh_rate_millihz = gd_refresh_rate_millihz(vc->window ?
- vc->window : s->window);
- if (refresh_rate_millihz) {
- /* T = 1 / f = 1 [s*Hz] / f = 1000*1000 [ms*mHz] / f */
- vc->gfx.dcl.update_interval = 1000 * 1000 / refresh_rate_millihz;
- }
+ vc->gfx.dcl.update_interval =
+ gd_monitor_update_interval(vc->window ? vc->window : s->window);
fbw = surface_width(vc->gfx.ds);
fbh = surface_height(vc->gfx.ds);
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] ui/gtk: paper over a gtk bug on Windows
2020-12-13 16:56 [PATCH 0/3] ui/gtk: paper over a gtk bug on Windows Volker Rümelin
` (2 preceding siblings ...)
2020-12-13 16:57 ` [PATCH 3/3] ui/gtk: limit virtual console max update interval Volker Rümelin
@ 2021-01-14 9:38 ` Gerd Hoffmann
3 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2021-01-14 9:38 UTC (permalink / raw)
To: Volker Rümelin
Cc: Philippe Mathieu-Daudé, qemu-devel, Nikola Pavlica
On Sun, Dec 13, 2020 at 05:56:28PM +0100, Volker Rümelin wrote:
> Patch dc26435edb "ui/gtk: Update refresh interval after widget
> is realized" exposed a bug in gtk on Windows. The monitor refresh
> rate reported by gtk may be much smaller than the real refresh
> rate leading to an unusable guest screen refresh rate.
Added to ui patch queue.
thanks,
Gerd
^ permalink raw reply [flat|nested] 6+ messages in thread