From: Weifeng Liu <weifeng.liu.z@gmail.com>

When gl=on, scale_x and scale_y were set to 1 on startup that didn't
reflect the real situation of the scan-out in free scale mode, resulting
in incorrect cursor coordinates to be sent when moving the mouse
pointer. Simply updating the scales before rendering the image fixes
this issue.

Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
Message-ID: <20250511073337.876650-5-weifeng.liu.z@gmail.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/gtk-gl-area.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index ba9fbec432..db93cd6204 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -43,6 +43,8 @@ void gd_gl_area_draw(VirtualConsole *vc)
     QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf;
 #endif
     int pw, ph, gs, y1, y2;
+    int ww, wh;
+    int fbw, fbh;
 
     if (!vc->gfx.gls) {
         return;
@@ -50,8 +52,14 @@ void gd_gl_area_draw(VirtualConsole *vc)
 
     gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
     gs = gdk_window_get_scale_factor(gtk_widget_get_window(vc->gfx.drawing_area));
-    pw = gtk_widget_get_allocated_width(vc->gfx.drawing_area) * gs;
-    ph = gtk_widget_get_allocated_height(vc->gfx.drawing_area) * gs;
+    fbw = surface_width(vc->gfx.ds);
+    fbh = surface_height(vc->gfx.ds);
+    ww = gtk_widget_get_allocated_width(vc->gfx.drawing_area);
+    wh = gtk_widget_get_allocated_height(vc->gfx.drawing_area);
+    pw = ww * gs;
+    ph = wh * gs;
+
+    gd_update_scale(vc, ww, wh, fbw, fbh);
 
     if (vc->gfx.scanout_mode) {
         if (!vc->gfx.guest_fb.framebuffer) {
-- 
2.49.0

On Sat, 24 May 2025 at 18:37, <marcandre.lureau@redhat.com> wrote:
>
> From: Weifeng Liu <weifeng.liu.z@gmail.com>
>
> When gl=on, scale_x and scale_y were set to 1 on startup that didn't
> reflect the real situation of the scan-out in free scale mode, resulting
> in incorrect cursor coordinates to be sent when moving the mouse
> pointer. Simply updating the scales before rendering the image fixes
> this issue.
>
> Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
> Message-ID: <20250511073337.876650-5-weifeng.liu.z@gmail.com>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Hi; Coverity complains about this change CID 1610328):

> @@ -50,8 +52,14 @@ void gd_gl_area_draw(VirtualConsole *vc)
>
>      gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
>      gs = gdk_window_get_scale_factor(gtk_widget_get_window(vc->gfx.drawing_area));
> -    pw = gtk_widget_get_allocated_width(vc->gfx.drawing_area) * gs;
> -    ph = gtk_widget_get_allocated_height(vc->gfx.drawing_area) * gs;
> +    fbw = surface_width(vc->gfx.ds);
> +    fbh = surface_height(vc->gfx.ds);

Here we now unconditionally dereference vc->gfx.ds at the start of
gd_gl_area_draw().

But towards the end of this function we have a NULL check:

        if (!vc->gfx.ds) {
            return;
        }

Either vc->gfx.ds can be NULL, in which case we need some
kind of guard on these surface_width() and surface_height()
calls; or else it can't, and the NULL check later is dead code.
Which is correct ?

thanks
-- PMM

Hi

On Thu, Jul 10, 2025 at 4:24 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Sat, 24 May 2025 at 18:37, <marcandre.lureau@redhat.com> wrote:
> >
> > From: Weifeng Liu <weifeng.liu.z@gmail.com>
> >
> > When gl=on, scale_x and scale_y were set to 1 on startup that didn't
> > reflect the real situation of the scan-out in free scale mode, resulting
> > in incorrect cursor coordinates to be sent when moving the mouse
> > pointer. Simply updating the scales before rendering the image fixes
> > this issue.
> >
> > Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
> > Message-ID: <20250511073337.876650-5-weifeng.liu.z@gmail.com>
> > Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> > Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Hi; Coverity complains about this change CID 1610328):
>
> > @@ -50,8 +52,14 @@ void gd_gl_area_draw(VirtualConsole *vc)
> >
> >      gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
> >      gs = gdk_window_get_scale_factor(gtk_widget_get_window(vc->gfx.drawing_area));
> > -    pw = gtk_widget_get_allocated_width(vc->gfx.drawing_area) * gs;
> > -    ph = gtk_widget_get_allocated_height(vc->gfx.drawing_area) * gs;
> > +    fbw = surface_width(vc->gfx.ds);
> > +    fbh = surface_height(vc->gfx.ds);
>
> Here we now unconditionally dereference vc->gfx.ds at the start of
> gd_gl_area_draw().
>
> But towards the end of this function we have a NULL check:
>
>         if (!vc->gfx.ds) {
>             return;
>         }
>
> Either vc->gfx.ds can be NULL, in which case we need some
> kind of guard on these surface_width() and surface_height()
> calls; or else it can't, and the NULL check later is dead code.
> Which is correct ?

Given that it's simply called from a GTK callback, it can be NULL. I
think we should simply return in this case, or perhaps use the older
code path as a fallback. Weifeng, wdyt?

-- 
Marc-André Lureau

Hi,

On Fri, 2025-07-11 at 11:01 +0400, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Jul 10, 2025 at 4:24 PM Peter Maydell
> <peter.maydell@linaro.org> wrote:
> > 
> > On Sat, 24 May 2025 at 18:37, <marcandre.lureau@redhat.com> wrote:
> > > 
> > > From: Weifeng Liu <weifeng.liu.z@gmail.com>
> > > 
> > > When gl=on, scale_x and scale_y were set to 1 on startup that
> > > didn't
> > > reflect the real situation of the scan-out in free scale mode,
> > > resulting
> > > in incorrect cursor coordinates to be sent when moving the mouse
> > > pointer. Simply updating the scales before rendering the image
> > > fixes
> > > this issue.
> > > 
> > > Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
> > > Message-ID: <20250511073337.876650-5-weifeng.liu.z@gmail.com>
> > > Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> > > Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > 
> > Hi; Coverity complains about this change CID 1610328):
> > 
> > > @@ -50,8 +52,14 @@ void gd_gl_area_draw(VirtualConsole *vc)
> > > 
> > >      gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
> > >      gs = gdk_window_get_scale_factor(gtk_widget_get_window(vc-
> > > >gfx.drawing_area));
> > > -    pw = gtk_widget_get_allocated_width(vc->gfx.drawing_area) *
> > > gs;
> > > -    ph = gtk_widget_get_allocated_height(vc->gfx.drawing_area) *
> > > gs;
> > > +    fbw = surface_width(vc->gfx.ds);
> > > +    fbh = surface_height(vc->gfx.ds);
> > 
> > Here we now unconditionally dereference vc->gfx.ds at the start of
> > gd_gl_area_draw().
> > 
> > But towards the end of this function we have a NULL check:
> > 
> >         if (!vc->gfx.ds) {
> >             return;
> >         }
> > 
> > Either vc->gfx.ds can be NULL, in which case we need some
> > kind of guard on these surface_width() and surface_height()
> > calls; or else it can't, and the NULL check later is dead code.
> > Which is correct ?
> 
> Given that it's simply called from a GTK callback, it can be NULL. I
> think we should simply return in this case, or perhaps use the older
> code path as a fallback. Weifeng, wdyt?

It makes sense to skip drawing when ds is NULL, although that shouldn’t
happen since dpy_gfx_replace_surface installs a dummy DisplaySurface
for any NULL input. I’ll draft a patch shortly to add this check (and
possibly other refinements).

Best regards,
Weifeng