* [PATCH] display/gtk: get proper refreshrate
@ 2019-12-30 17:28 Nikola Pavlica
2019-12-30 22:59 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 3+ messages in thread
From: Nikola Pavlica @ 2019-12-30 17:28 UTC (permalink / raw)
To: qemu-devel
From 70c95b18fa056b2dd0ecc202ab517bc775b986da Mon Sep 17 00:00:00 2001
From: Nikola Pavlica <pavlica.nikola@gmail.com>
Date: Mon, 30 Dec 2019 18:17:35 +0100
Subject: [PATCH] display/gtk: get proper refreshrate
Signed-off-by: Nikola Pavlica <pavlica.nikola@gmail.com>
---
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();
+ GdkMonitor *monitor = gdk_display_get_primary_monitor(display);
+ vc->gfx.dcl.update_interval = 1000000 /
+ gdk_monitor_get_refresh_rate(monitor);
}
static void early_gtk_display_init(DisplayOptions *opts)
--
2.24.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] display/gtk: get proper refreshrate
2019-12-30 17:28 [PATCH] display/gtk: get proper refreshrate Nikola Pavlica
@ 2019-12-30 22:59 ` Philippe Mathieu-Daudé
[not found] ` <2ee36cb0103b60a4a0655a4c127469e6f49f8d46.camel@gmail.com>
0 siblings, 1 reply; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-12-30 22:59 UTC (permalink / raw)
To: Nikola Pavlica, qemu-devel, Gerd Hoffmann
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 <pavlica.nikola@gmail.com>
> Date: Mon, 30 Dec 2019 18:17:35 +0100
> Subject: [PATCH] display/gtk: get proper refreshrate
Can you describe here the problem you encountered, and how your patch
fixes it?
>
> Signed-off-by: Nikola Pavlica <pavlica.nikola@gmail.com>
> ---
> 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 <kraxel@redhat.com> (odd fixer:Graphics)
qemu-devel@nongnu.org (open list:All patches CC here)
I'm Cc'ing Gerd for you.
Regards,
Phil.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] display/gtk: get proper refreshrate
[not found] ` <2ee36cb0103b60a4a0655a4c127469e6f49f8d46.camel@gmail.com>
@ 2019-12-31 13:48 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-12-31 13:48 UTC (permalink / raw)
To: Nikola Pavlica; +Cc: kraxel, qemu-devel
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 <pavlica.nikola@gmail.com>
>>> Date: Mon, 30 Dec 2019 18:17:35 +0100
>>> Subject: [PATCH] display/gtk: get proper refreshrate
>>
>> Can you describe here the problem you encountered, and how your
>> patch
>> fixes it?
>>
>>> Signed-off-by: Nikola Pavlica <pavlica.nikola@gmail.com>
>>> ---
>>> 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 <kraxel@redhat.com> (odd fixer:Graphics)
>> qemu-devel@nongnu.org (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 <pavlica.nikola@gmail.com>
> 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 <pavlica.nikola@gmail.com>
> ---
> 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 0
Watch 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);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-12-31 14:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-30 17:28 [PATCH] display/gtk: get proper refreshrate Nikola Pavlica
2019-12-30 22:59 ` Philippe Mathieu-Daudé
[not found] ` <2ee36cb0103b60a4a0655a4c127469e6f49f8d46.camel@gmail.com>
2019-12-31 13:48 ` Philippe Mathieu-Daudé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.