[PATCH 1/1] ui/gtk: Launching GTK UI with OpenGL on causes the refreshrate update to not run

[PATCH 1/1] ui/gtk: Launching GTK UI with OpenGL on causes the refreshrate update to not run

[Nikola Pavlica]

[-- Attachment #1: Type: text/plain, Size: 1735 bytes --]

I've discussed this issue back January and September. But it still 
occurs on my machine, despite the two patches.
This time, the issue is that the UI refresh rate doesn't get updated 
when I launch QEMU with gl=on.
My fix for this issue is to move the code for updating the refresh rate 
above the code that checks for OpenGL.

Or because OpenGL is meant to be called back with the "render" 
callback, should we instead add the refresh
rate checking code there?

Anyway, regardless of method, I'm happy with any solution that just 
fixes the issue.

Signed-off-by: Nikola Pavlica (pavlica.nikola@gmail.com)
---
diff --git a/ui/gtk.c b/ui/gtk.c
index a752aa22be..74287edde8 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -776,18 +776,6 @@ static gboolean gd_draw_event(GtkWidget *widget, 
cairo_t *cr, void *opaque)
     int fbw, fbh;
     int refresh_rate_millihz;

-#if defined(CONFIG_OPENGL)
-    if (vc->gfx.gls) {
-        if (gtk_use_gl_area) {
-            /* invoke render callback please */
-            return FALSE;
-        } else {
-            gd_egl_draw(vc);
-            return TRUE;
-        }
-    }
-#endif
-
     if (!gtk_widget_get_realized(widget)) {
         return FALSE;
     }
@@ -801,6 +789,18 @@ static gboolean gd_draw_event(GtkWidget *widget, 
cairo_t *cr, void *opaque)
         vc->gfx.dcl.update_interval = MILLISEC_PER_SEC / 
refresh_rate_millihz;
     }

+#if defined(CONFIG_OPENGL)
+    if (vc->gfx.gls) {
+        if (gtk_use_gl_area) {
+            /* invoke render callback please */
+            return FALSE;
+        } else {
+            gd_egl_draw(vc);
+            return TRUE;
+        }
+    }
+#endif
+
     fbw = surface_width(vc->gfx.ds);
     fbh = surface_height(vc->gfx.ds);
-- 
2.21.1


[-- Attachment #2: Type: text/html, Size: 3369 bytes --]

Re: [PATCH 1/1] ui/gtk: Launching GTK UI with OpenGL on causes the refreshrate update to not run

[Philippe Mathieu-Daudé]

Hi Nikola,

On 12/6/20 11:43 AM, Nikola Pavlica wrote:
> I've discussed this issue back January and September. But it still
> occurs on my machine, despite the two patches. This time, the issue is
> that the UI refresh rate doesn't get updated when I launch QEMU with
> gl=on. My fix for this issue is to move the code for updating the
> refresh rate above the code that checks for OpenGL.
> 
> Or because OpenGL is meant to be called back with the "render" callback,
> should we instead add the refresh 
> rate checking code there?

This is not my area, but I think you are right, the the refresh rate
should be updated in the "render" callback.

To speed things you can send another patch, and Gerd will pick the
correct/best one :)

> Anyway, regardless of method, I'm happy with any solution that just
> fixes the issue.
> Signed-off-by: Nikola Pavlica (pavlica.nikola@gmail.com)---
> diff --git a/ui/gtk.c b/ui/gtk.c
> index a752aa22be..74287edde8 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -776,18 +776,6 @@ static gboolean gd_draw_event(GtkWidget *widget,
> cairo_t *cr, void *opaque)
>      int fbw, fbh;
>      int refresh_rate_millihz;
>  
> -#if defined(CONFIG_OPENGL)
> -    if (vc->gfx.gls) {
> -        if (gtk_use_gl_area) {
> -            /* invoke render callback please */
> -            return FALSE;
> -        } else {
> -            gd_egl_draw(vc);
> -            return TRUE;
> -        }
> -    }
> -#endif
> -
>      if (!gtk_widget_get_realized(widget)) {
>          return FALSE;
>      }
> @@ -801,6 +789,18 @@ static gboolean gd_draw_event(GtkWidget *widget,
> cairo_t *cr, void *opaque)
>          vc->gfx.dcl.update_interval = MILLISEC_PER_SEC /
> refresh_rate_millihz;
>      }
>  
> +#if defined(CONFIG_OPENGL)
> +    if (vc->gfx.gls) {
> +        if (gtk_use_gl_area) {
> +            /* invoke render callback please */
> +            return FALSE;
> +        } else {
> +            gd_egl_draw(vc);
> +            return TRUE;
> +        }
> +    }
> +#endif
> +
>      fbw = surface_width(vc->gfx.ds);
>      fbh = surface_height(vc->gfx.ds);
> -- 
> 2.21.1

Re: [PATCH 1/1] ui/gtk: Launching GTK UI with OpenGL on causes the refreshrate update to not run

[Philippe Mathieu-Daudé]

On 12/6/20 12:42 PM, Philippe Mathieu-Daudé wrote:
> Hi Nikola,
> 
> On 12/6/20 11:43 AM, Nikola Pavlica wrote:
>> I've discussed this issue back January and September. But it still
>> occurs on my machine, despite the two patches.

Oh, also, instead of "my machine", please provide more information about
your setup (installed versions), so we might eventually reproduce.

> This time, the issue is
>> that the UI refresh rate doesn't get updated when I launch QEMU with
>> gl=on. My fix for this issue is to move the code for updating the
>> refresh rate above the code that checks for OpenGL.
>>
>> Or because OpenGL is meant to be called back with the "render" callback,
>> should we instead add the refresh 
>> rate checking code there?
> 
> This is not my area, but I think you are right, the the refresh rate
> should be updated in the "render" callback.
> 
> To speed things you can send another patch, and Gerd will pick the
> correct/best one :)
> 
>> Anyway, regardless of method, I'm happy with any solution that just
>> fixes the issue.
>> Signed-off-by: Nikola Pavlica (pavlica.nikola@gmail.com)---

Re: [PATCH 1/1] ui/gtk: Launching GTK UI with OpenGL on causes the refreshrate update to not run

[Michael Tokarev]

06.12.2020 13:43, Nikola Pavlica wrote:
> I've discussed this issue back January and September. But it still occurs on my machine, despite the two patches. This time, the issue is that the UI 
> refresh rate doesn't get updated when I launch QEMU with gl=on. My fix for this issue is to move the code for updating the refresh rate above the code 
> that checks for OpenGL.
> 
> Or because OpenGL is meant to be called back with the "render" callback, should we instead add the refresh
> rate checking code there?
> 
> Anyway, regardless of method, I'm happy with any solution that just fixes the issue.

Is this change not needed anymore?

Nikola, your patch email is html-garbled, at the very least you have to send it as
plain text.

Thanks,

/mjt

> Signed-off-by: Nikola Pavlica (pavlica.nikola@gmail.com)---
> diff --git a/ui/gtk.c b/ui/gtk.c
> index a752aa22be..74287edde8 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -776,18 +776,6 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
>       int fbw, fbh;
>       int refresh_rate_millihz;
> -#if defined(CONFIG_OPENGL)
> -    if (vc->gfx.gls) {
> -        if (gtk_use_gl_area) {
> -            /* invoke render callback please */
> -            return FALSE;
> -        } else {
> -            gd_egl_draw(vc);
> -            return TRUE;
> -        }
> -    }
> -#endif
> -
>       if (!gtk_widget_get_realized(widget)) {
>           return FALSE;
>       }
> @@ -801,6 +789,18 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
>           vc->gfx.dcl.update_interval = MILLISEC_PER_SEC / refresh_rate_millihz;
>       }
> +#if defined(CONFIG_OPENGL)
> +    if (vc->gfx.gls) {
> +        if (gtk_use_gl_area) {
> +            /* invoke render callback please */
> +            return FALSE;
> +        } else {
> +            gd_egl_draw(vc);
> +            return TRUE;
> +        }
> +    }
> +#endif
> +
>       fbw = surface_width(vc->gfx.ds);
>       fbh = surface_height(vc->gfx.ds);
> -- 
> 2.21.1