[PATCH 0/9] ui: Improve scale handling

[PATCH 0/9] ui: Improve scale handling

[Weifeng Liu]

Hi all,

Now we have quite a lot of display backends for different use cases.
Even in the context of gtk, we have various implementations (e.g., gl=on
vs gl=off, X11 vs Wayland). However, behaviors to users are not aligned
across the backends, especially in the part of scale handling. This
patch set attempts to improve scale handling.

We have to deal with various coordinates due to the existence of scaling
in different level. Firstly, in desktop level, we could have a global
window scale factor. Secondly, users might set a zooming factor to
adjust the size of guest content in scan-out level. Consequently, 1) the
buffer from guest, 2) the host window and 3) OpenGl drawing area inside
the host window are in distinct coordinates. It's important to define
these coordinates and scales unambiguously and use a consistent naming
convention for variables representing different concepts. The first
patch in this set tries to achieve this goal by adding a document in
gtk.c, and the next patch (PATCH 2) attempts to align the code with the
document.

PATCH 3 - 5 fix bugs in mouse position calculation due to not handling
scale properly, for both gtk and sdl.

PATCH 6 align scale update logic in gtk-egl with other implementations.

PATCH 7 fix an issue that gtk window might keep enlarging/shrinking because 
ui info propagating to guest not considering scale.

PATCH 8 and 9 align fixed-scale mode behavior in gtk-gl-area and gtk-egl with
other implementations by adding appropriate padding to the window to preserve
the scale.

Tested cases:

- qemu-kvm -display gtk,gl=on (free scale mode, fixed scale mode, full
  screen mode)
- qemu-kvm -display gtk,gl=off (free scale mode, fixed scale mode, full
  screen mode)
- GDK_BACKEND=x11 qemu-kvm -display gtk,gl=off (free scale mode, fixed
  scale mode, full screen mode)
- qemu-kvm -display sdl,gl=off
- qemu-kvm -display sdl,gl=on

Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: Alex Bennée <alex.bennee@linaro.org>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Cc: Dongwon Kim <dongwon.kim@intel.com>

Best regards,
Weifeng

Weifeng Liu (9):
  ui/gtk: Document scale and coordinate handling
  ui/gtk: Use consistent naming for variables in different coordinates
  gtk/ui: Introduce helper gd_update_scale
  ui/gtk: Update scales in fixed-scale mode when rendering GL area
  ui/sdl: Consider scaling in mouse event handling
  ui/gtk: Don't update scale in fixed scale mode in gtk-egl.c
  ui/gtk: Consider scaling when propagating ui info
  ui/gtk-gl-area: Render guest content with padding in fixed-scale mode
  ui/gtk-egl: Render guest content with padding in fixed-scale mode

 include/ui/egl-helpers.h |   4 +-
 include/ui/gtk.h         |   2 +
 ui/egl-helpers.c         |  10 +-
 ui/gtk-egl.c             |  58 ++++++---
 ui/gtk-gl-area.c         |  53 +++++++--
 ui/gtk.c                 | 245 +++++++++++++++++++++++++++------------
 ui/sdl2-gl.c             |   2 +-
 ui/sdl2.c                |  20 +++-
 8 files changed, 290 insertions(+), 104 deletions(-)

-- 
2.49.0

[PATCH 1/9] ui/gtk: Document scale and coordinate handling

[Weifeng Liu]

The existence of multiple scaling factors forces us to deal with various
coordinate systems and this would be confusing. It would be beneficial
to define the concepts clearly and use consistent representation for
variables in different coordinates.

Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
---
 ui/gtk.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index 982037b2c0..9f3171abc5 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -800,6 +800,71 @@ void gd_update_monitor_refresh_rate(VirtualConsole *vc, GtkWidget *widget)
 #endif
 }
 
+/**
+ * DOC: Coordinate handling.
+ *
+ * We are coping with sizes and positions in various coordinates and the
+ * handling of these coordinates is somewhat confusing. It would benefit us
+ * all if we define these coordinates explicitly and clearly. Besides, it's
+ * also helpful to follow the same naming convention for variables
+ * representing values in different coordinates.
+ *
+ * I. Definitions
+ *
+ * - (guest) buffer coordinate: this is the coordinates that the guest will
+ *   see. The x/y offsets and width/height specified in commands sent by
+ *   guest is basically in buffer coordinate.
+ *
+ * - (host) pixel coordinate: this is the coordinate in pixel level on the
+ *   host destop. A window/widget of width 300 in pixel coordinate means it
+ *   occupies 300 pixels horizontally.
+ *
+ * - (host) logical window coordinate: the existence of global scaling
+ *   factor in desktop level makes this kind of coordinate play a role. It
+ *   always holds that (logical window size) * (global scale factor) =
+ *   (pixel size).
+ *
+ * - global scale factor: this is specified in desktop level and is
+ *   typically invariant during the life cycle of the process. Users with
+ *   high-DPI monitors might set this scale, for example, to 2, in order to
+ *   make the UI look larger.
+ *
+ * - zooming scale: this can be freely controlled by the QEMU user to zoom
+ *   in/out the guest content.
+ *
+ * II. Representation
+ *
+ * We'd like to use consistent representation for variables in different
+ * coordinates:
+ * - buffer coordinate: prefix fb
+ * - pixel coordinate: prefix p
+ * - logical window coordinate: prefix w
+ *
+ * For scales:
+ * - global scale factor: prefix gs
+ * - zooming scale: prefix scale/s
+ *
+ * Example: fbw, pw, ww for width in different coordinates
+ *
+ * III. Equation
+ *
+ * - fbw * gs * scale_x = pw
+ * - pw = gs * ww
+ *
+ * Consequently we have
+ *
+ * - fbw * scale_x = ww
+ *
+ * Example: assuming we are running QEMU on a 3840x2160 screen and have set
+ * global scaling factor to 2, if the guest buffer size is 1920x1080 and the
+ * zooming scale is 0.5, then we have:
+ * - fbw = 1920, fbh = 1080
+ * - pw  = 1920, ph  = 1080
+ * - ww  = 960,  wh  = 540
+ * A bonus of this configuration is that we can achieve pixel to pixel
+ * presentation of the guest content.
+ */
+
 static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
 {
     VirtualConsole *vc = opaque;
-- 
2.49.0

[PATCH 2/9] ui/gtk: Use consistent naming for variables in different coordinates

[Weifeng Liu]

Now that we've documented definitions and presentation of various
coordinates, let's enforce the rules.

Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
---
 ui/gtk-egl.c     |  12 +++--
 ui/gtk-gl-area.c |  14 ++---
 ui/gtk.c         | 133 ++++++++++++++++++++++++-----------------------
 3 files changed, 82 insertions(+), 77 deletions(-)

diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index f7a428c86a..947c99334b 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -70,16 +70,18 @@ void gd_egl_draw(VirtualConsole *vc)
     QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf;
     int fence_fd;
 #endif
-    int ww, wh, ws;
+    int ww, wh, pw, ph, gs;
 
     if (!vc->gfx.gls) {
         return;
     }
 
     window = gtk_widget_get_window(vc->gfx.drawing_area);
-    ws = gdk_window_get_scale_factor(window);
-    ww = gdk_window_get_width(window) * ws;
-    wh = gdk_window_get_height(window) * ws;
+    gs = gdk_window_get_scale_factor(window);
+    ww = gdk_window_get_width(window);
+    wh = gdk_window_get_height(window);
+    pw = ww * gs;
+    ph = wh * gs;
 
     if (vc->gfx.scanout_mode) {
 #ifdef CONFIG_GBM
@@ -115,7 +117,7 @@ void gd_egl_draw(VirtualConsole *vc)
         eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
                        vc->gfx.esurface, vc->gfx.ectx);
 
-        surface_gl_setup_viewport(vc->gfx.gls, vc->gfx.ds, ww, wh);
+        surface_gl_setup_viewport(vc->gfx.gls, vc->gfx.ds, pw, ph);
         surface_gl_render_texture(vc->gfx.gls, vc->gfx.ds);
 
         eglSwapBuffers(qemu_egl_display, vc->gfx.esurface);
diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index 2c9a0db425..ba9fbec432 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -42,16 +42,16 @@ void gd_gl_area_draw(VirtualConsole *vc)
 #ifdef CONFIG_GBM
     QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf;
 #endif
-    int ww, wh, ws, y1, y2;
+    int pw, ph, gs, y1, y2;
 
     if (!vc->gfx.gls) {
         return;
     }
 
     gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
-    ws = gdk_window_get_scale_factor(gtk_widget_get_window(vc->gfx.drawing_area));
-    ww = gtk_widget_get_allocated_width(vc->gfx.drawing_area) * ws;
-    wh = gtk_widget_get_allocated_height(vc->gfx.drawing_area) * ws;
+    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;
 
     if (vc->gfx.scanout_mode) {
         if (!vc->gfx.guest_fb.framebuffer) {
@@ -71,11 +71,11 @@ void gd_gl_area_draw(VirtualConsole *vc)
         glBindFramebuffer(GL_READ_FRAMEBUFFER, vc->gfx.guest_fb.framebuffer);
         /* GtkGLArea sets GL_DRAW_FRAMEBUFFER for us */
 
-        glViewport(0, 0, ww, wh);
+        glViewport(0, 0, pw, ph);
         y1 = vc->gfx.y0_top ? 0 : vc->gfx.h;
         y2 = vc->gfx.y0_top ? vc->gfx.h : 0;
         glBlitFramebuffer(0, y1, vc->gfx.w, y2,
-                          0, 0, ww, wh,
+                          0, 0, pw, ph,
                           GL_COLOR_BUFFER_BIT, GL_NEAREST);
 #ifdef CONFIG_GBM
         if (dmabuf) {
@@ -101,7 +101,7 @@ void gd_gl_area_draw(VirtualConsole *vc)
         }
         gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
 
-        surface_gl_setup_viewport(vc->gfx.gls, vc->gfx.ds, ww, wh);
+        surface_gl_setup_viewport(vc->gfx.gls, vc->gfx.ds, pw, ph);
         surface_gl_render_texture(vc->gfx.gls, vc->gfx.ds);
     }
 }
diff --git a/ui/gtk.c b/ui/gtk.c
index 9f3171abc5..8f5bb4b62e 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -387,16 +387,16 @@ static void *gd_win32_get_hwnd(VirtualConsole *vc)
 /** DisplayState Callbacks **/
 
 static void gd_update(DisplayChangeListener *dcl,
-                      int x, int y, int w, int h)
+                      int fbx, int fby, int fbw, int fbh)
 {
     VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
     GdkWindow *win;
-    int x1, x2, y1, y2;
-    int mx, my;
-    int fbw, fbh;
-    int ww, wh;
+    int wx1, wx2, wy1, wy2;
+    int wx_offset, wy_offset;
+    int ww_surface, wh_surface;
+    int ww_widget, wh_widget;
 
-    trace_gd_update(vc->label, x, y, w, h);
+    trace_gd_update(vc->label, fbx, fby, fbw, fbh);
 
     if (!gtk_widget_get_realized(vc->gfx.drawing_area)) {
         return;
@@ -405,35 +405,36 @@ static void gd_update(DisplayChangeListener *dcl,
     if (vc->gfx.convert) {
         pixman_image_composite(PIXMAN_OP_SRC, vc->gfx.ds->image,
                                NULL, vc->gfx.convert,
-                               x, y, 0, 0, x, y, w, h);
+                               fbx, fby, 0, 0, fbx, fby, fbw, fbh);
     }
 
-    x1 = floor(x * vc->gfx.scale_x);
-    y1 = floor(y * vc->gfx.scale_y);
+    wx1 = floor(fbx * vc->gfx.scale_x);
+    wy1 = floor(fby * vc->gfx.scale_y);
 
-    x2 = ceil(x * vc->gfx.scale_x + w * vc->gfx.scale_x);
-    y2 = ceil(y * vc->gfx.scale_y + h * vc->gfx.scale_y);
+    wx2 = ceil(fbx * vc->gfx.scale_x + fbw * vc->gfx.scale_x);
+    wy2 = ceil(fby * vc->gfx.scale_y + fbh * vc->gfx.scale_y);
 
-    fbw = surface_width(vc->gfx.ds) * vc->gfx.scale_x;
-    fbh = surface_height(vc->gfx.ds) * vc->gfx.scale_y;
+    ww_surface = surface_width(vc->gfx.ds) * vc->gfx.scale_x;
+    wh_surface = surface_height(vc->gfx.ds) * vc->gfx.scale_y;
 
     win = gtk_widget_get_window(vc->gfx.drawing_area);
     if (!win) {
         return;
     }
-    ww = gdk_window_get_width(win);
-    wh = gdk_window_get_height(win);
+    ww_widget = gdk_window_get_width(win);
+    wh_widget = gdk_window_get_height(win);
 
-    mx = my = 0;
-    if (ww > fbw) {
-        mx = (ww - fbw) / 2;
+    wx_offset = wy_offset = 0;
+    if (ww_widget > ww_surface) {
+        wx_offset = (ww_widget - ww_surface) / 2;
     }
-    if (wh > fbh) {
-        my = (wh - fbh) / 2;
+    if (wh_widget > wh_surface) {
+        wy_offset = (wh_widget - wh_surface) / 2;
     }
 
     gtk_widget_queue_draw_area(vc->gfx.drawing_area,
-                               mx + x1, my + y1, (x2 - x1), (y2 - y1));
+                               wx_offset + wx1, wy_offset + wy1,
+                               (wx2 - wx1), (wy2 - wy1));
 }
 
 static void gd_refresh(DisplayChangeListener *dcl)
@@ -869,8 +870,8 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
 {
     VirtualConsole *vc = opaque;
     GtkDisplayState *s = vc->s;
-    int mx, my;
-    int ww, wh;
+    int wx_offset, wy_offset;
+    int ww_widget, wh_widget, ww_surface, wh_surface;
     int fbw, fbh;
 
 #if defined(CONFIG_OPENGL)
@@ -904,46 +905,47 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
     fbw = surface_width(vc->gfx.ds);
     fbh = surface_height(vc->gfx.ds);
 
-    ww = gdk_window_get_width(gtk_widget_get_window(widget));
-    wh = gdk_window_get_height(gtk_widget_get_window(widget));
+    ww_widget = gdk_window_get_width(gtk_widget_get_window(widget));
+    wh_widget = gdk_window_get_height(gtk_widget_get_window(widget));
 
     if (s->full_screen) {
-        vc->gfx.scale_x = (double)ww / fbw;
-        vc->gfx.scale_y = (double)wh / fbh;
+        vc->gfx.scale_x = (double)ww_widget / fbw;
+        vc->gfx.scale_y = (double)wh_widget / fbh;
     } else if (s->free_scale) {
         double sx, sy;
 
-        sx = (double)ww / fbw;
-        sy = (double)wh / fbh;
+        sx = (double)ww_widget / fbw;
+        sy = (double)wh_widget / fbh;
 
         vc->gfx.scale_x = vc->gfx.scale_y = MIN(sx, sy);
     }
 
-    fbw *= vc->gfx.scale_x;
-    fbh *= vc->gfx.scale_y;
+    ww_surface = fbw * vc->gfx.scale_x;
+    wh_surface = fbh * vc->gfx.scale_y;
 
-    mx = my = 0;
-    if (ww > fbw) {
-        mx = (ww - fbw) / 2;
+    wx_offset = wy_offset = 0;
+    if (ww_widget > ww_surface) {
+        wx_offset = (ww_widget - ww_surface) / 2;
     }
-    if (wh > fbh) {
-        my = (wh - fbh) / 2;
+    if (wh_widget > wh_surface) {
+        wy_offset = (wh_widget - wh_surface) / 2;
     }
 
-    cairo_rectangle(cr, 0, 0, ww, wh);
+    cairo_rectangle(cr, 0, 0, ww_widget, wh_widget);
 
     /* Optionally cut out the inner area where the pixmap
        will be drawn. This avoids 'flashing' since we're
        not double-buffering. Note we're using the undocumented
        behaviour of drawing the rectangle from right to left
        to cut out the whole */
-    cairo_rectangle(cr, mx + fbw, my,
-                    -1 * fbw, fbh);
+    cairo_rectangle(cr, wx_offset + ww_surface, wy_offset,
+                    -1 * ww_surface, wh_surface);
     cairo_fill(cr);
 
     cairo_scale(cr, vc->gfx.scale_x, vc->gfx.scale_y);
     cairo_set_source_surface(cr, vc->gfx.surface,
-                             mx / vc->gfx.scale_x, my / vc->gfx.scale_y);
+                             wx_offset / vc->gfx.scale_x,
+                             wy_offset / vc->gfx.scale_y);
     cairo_paint(cr);
 
     return TRUE;
@@ -954,19 +956,19 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion,
 {
     VirtualConsole *vc = opaque;
     GtkDisplayState *s = vc->s;
-    int x, y;
-    int mx, my;
-    int fbh, fbw;
-    int ww, wh;
+    int fbx, fby;
+    int wx_offset, wy_offset;
+    int wh_surface, ww_surface;
+    int ww_widget, wh_widget;
 
     if (!vc->gfx.ds) {
         return TRUE;
     }
 
-    fbw = surface_width(vc->gfx.ds) * vc->gfx.scale_x;
-    fbh = surface_height(vc->gfx.ds) * vc->gfx.scale_y;
-    ww = gtk_widget_get_allocated_width(widget);
-    wh = gtk_widget_get_allocated_height(widget);
+    ww_surface = surface_width(vc->gfx.ds) * vc->gfx.scale_x;
+    wh_surface = surface_height(vc->gfx.ds) * vc->gfx.scale_y;
+    ww_widget = gtk_widget_get_allocated_width(widget);
+    wh_widget = gtk_widget_get_allocated_height(widget);
 
     /*
      * `widget` may not have the same size with the frame buffer.
@@ -974,41 +976,42 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion,
      * To achieve that, `vc` will be displayed at (mx, my)
      * so that it is displayed at the center of the widget.
      */
-    mx = my = 0;
-    if (ww > fbw) {
-        mx = (ww - fbw) / 2;
+    wx_offset = wy_offset = 0;
+    if (ww_widget > ww_surface) {
+        wx_offset = (ww_widget - ww_surface) / 2;
     }
-    if (wh > fbh) {
-        my = (wh - fbh) / 2;
+    if (wh_widget > wh_surface) {
+        wy_offset = (wh_widget - wh_surface) / 2;
     }
 
     /*
      * `motion` is reported in `widget` coordinates
      * so translating it to the coordinates in `vc`.
      */
-    x = (motion->x - mx) / vc->gfx.scale_x;
-    y = (motion->y - my) / vc->gfx.scale_y;
+    fbx = (motion->x - wx_offset) / vc->gfx.scale_x;
+    fby = (motion->y - wy_offset) / vc->gfx.scale_y;
 
-    trace_gd_motion_event(ww, wh, gtk_widget_get_scale_factor(widget), x, y);
+    trace_gd_motion_event(ww_widget, wh_widget,
+                          gtk_widget_get_scale_factor(widget), fbx, fby);
 
     if (qemu_input_is_absolute(vc->gfx.dcl.con)) {
-        if (x < 0 || y < 0 ||
-            x >= surface_width(vc->gfx.ds) ||
-            y >= surface_height(vc->gfx.ds)) {
+        if (fbx < 0 || fby < 0 ||
+            fbx >= surface_width(vc->gfx.ds) ||
+            fby >= surface_height(vc->gfx.ds)) {
             return TRUE;
         }
-        qemu_input_queue_abs(vc->gfx.dcl.con, INPUT_AXIS_X, x,
+        qemu_input_queue_abs(vc->gfx.dcl.con, INPUT_AXIS_X, fbx,
                              0, surface_width(vc->gfx.ds));
-        qemu_input_queue_abs(vc->gfx.dcl.con, INPUT_AXIS_Y, y,
+        qemu_input_queue_abs(vc->gfx.dcl.con, INPUT_AXIS_Y, fby,
                              0, surface_height(vc->gfx.ds));
         qemu_input_event_sync();
     } else if (s->last_set && s->ptr_owner == vc) {
-        qemu_input_queue_rel(vc->gfx.dcl.con, INPUT_AXIS_X, x - s->last_x);
-        qemu_input_queue_rel(vc->gfx.dcl.con, INPUT_AXIS_Y, y - s->last_y);
+        qemu_input_queue_rel(vc->gfx.dcl.con, INPUT_AXIS_X, fbx - s->last_x);
+        qemu_input_queue_rel(vc->gfx.dcl.con, INPUT_AXIS_Y, fby - s->last_y);
         qemu_input_event_sync();
     }
-    s->last_x = x;
-    s->last_y = y;
+    s->last_x = fbx;
+    s->last_y = fby;
     s->last_set = TRUE;
 
     if (!qemu_input_is_absolute(vc->gfx.dcl.con) && s->ptr_owner == vc) {
-- 
2.49.0

[PATCH 3/9] gtk/ui: Introduce helper gd_update_scale

[Weifeng Liu]

The code snippet updating scale_x/scale_y is general and will be used in
next patch. Make it a function.

Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
---
 include/ui/gtk.h |  2 ++
 ui/gtk.c         | 30 +++++++++++++++++++-----------
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index aa3d637029..d3944046db 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -224,4 +224,6 @@ int gd_gl_area_make_current(DisplayGLCtx *dgc,
 /* gtk-clipboard.c */
 void gd_clipboard_init(GtkDisplayState *gd);
 
+void gd_update_scale(VirtualConsole *vc, int ww, int wh, int fbw, int fbh);
+
 #endif /* UI_GTK_H */
diff --git a/ui/gtk.c b/ui/gtk.c
index 8f5bb4b62e..47af49e387 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -801,6 +801,24 @@ void gd_update_monitor_refresh_rate(VirtualConsole *vc, GtkWidget *widget)
 #endif
 }
 
+void gd_update_scale(VirtualConsole *vc, int ww, int wh, int fbw, int fbh)
+{
+    if (!vc) {
+        return;
+    }
+
+    if (vc->s->full_screen) {
+        vc->gfx.scale_x = (double)ww / fbw;
+        vc->gfx.scale_y = (double)wh / fbh;
+    } else if (vc->s->free_scale) {
+        double sx, sy;
+
+        sx = (double)ww / fbw;
+        sy = (double)wh / fbh;
+
+        vc->gfx.scale_x = vc->gfx.scale_y = MIN(sx, sy);
+    }
+}
 /**
  * DOC: Coordinate handling.
  *
@@ -908,17 +926,7 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
     ww_widget = gdk_window_get_width(gtk_widget_get_window(widget));
     wh_widget = gdk_window_get_height(gtk_widget_get_window(widget));
 
-    if (s->full_screen) {
-        vc->gfx.scale_x = (double)ww_widget / fbw;
-        vc->gfx.scale_y = (double)wh_widget / fbh;
-    } else if (s->free_scale) {
-        double sx, sy;
-
-        sx = (double)ww_widget / fbw;
-        sy = (double)wh_widget / fbh;
-
-        vc->gfx.scale_x = vc->gfx.scale_y = MIN(sx, sy);
-    }
+    gd_update_scale(vc, ww_widget, wh_widget, fbw, fbh);
 
     ww_surface = fbw * vc->gfx.scale_x;
     wh_surface = fbh * vc->gfx.scale_y;
-- 
2.49.0

[PATCH 4/9] ui/gtk: Update scales in fixed-scale mode when rendering GL area

[Weifeng Liu]

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>
---
 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

[PATCH 5/9] ui/sdl: Consider scaling in mouse event handling

[Weifeng Liu]

From: Weifeng Liu <weifeng.liu@intel.com>

When using sdl display backend, if the window is scaled, incorrect mouse
positions will be reported since scaling is not properly handled. Fix it
by transforming the positions from window coordinate to guest buffer
coordinate.

Signed-off-by: Weifeng Liu <weifeng.liu@intel.com>
---
 ui/sdl2.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index cda4293a53..b00e421f7f 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -488,14 +488,14 @@ static void handle_mousemotion(SDL_Event *ev)
 {
     int max_x, max_y;
     struct sdl2_console *scon = get_scon_from_window(ev->motion.windowID);
+    int scr_w, scr_h, surf_w, surf_h, x, y, dx, dy;
 
     if (!scon || !qemu_console_is_graphic(scon->dcl.con)) {
         return;
     }
 
+    SDL_GetWindowSize(scon->real_window, &scr_w, &scr_h);
     if (qemu_input_is_absolute(scon->dcl.con) || absolute_enabled) {
-        int scr_w, scr_h;
-        SDL_GetWindowSize(scon->real_window, &scr_w, &scr_h);
         max_x = scr_w - 1;
         max_y = scr_h - 1;
         if (gui_grab && !gui_fullscreen
@@ -509,9 +509,14 @@ static void handle_mousemotion(SDL_Event *ev)
             sdl_grab_start(scon);
         }
     }
+    surf_w = surface_width(scon->surface);
+    surf_h = surface_height(scon->surface);
+    x = (int64_t)ev->motion.x * surf_w / scr_w;
+    y = (int64_t)ev->motion.y * surf_h / scr_h;
+    dx = (int64_t)ev->motion.xrel * surf_w / scr_w;
+    dy = (int64_t)ev->motion.yrel * surf_h / scr_h;
     if (gui_grab || qemu_input_is_absolute(scon->dcl.con) || absolute_enabled) {
-        sdl_send_mouse_event(scon, ev->motion.xrel, ev->motion.yrel,
-                             ev->motion.x, ev->motion.y, ev->motion.state);
+        sdl_send_mouse_event(scon, dx, dy, x, y, ev->motion.state);
     }
 }
 
@@ -520,12 +525,17 @@ static void handle_mousebutton(SDL_Event *ev)
     int buttonstate = SDL_GetMouseState(NULL, NULL);
     SDL_MouseButtonEvent *bev;
     struct sdl2_console *scon = get_scon_from_window(ev->button.windowID);
+    int scr_w, scr_h, x, y;
 
     if (!scon || !qemu_console_is_graphic(scon->dcl.con)) {
         return;
     }
 
     bev = &ev->button;
+    SDL_GetWindowSize(scon->real_window, &scr_w, &scr_h);
+    x = (int64_t)bev->x * surface_width(scon->surface) / scr_w;
+    y = (int64_t)bev->y * surface_height(scon->surface) / scr_h;
+
     if (!gui_grab && !qemu_input_is_absolute(scon->dcl.con)) {
         if (ev->type == SDL_MOUSEBUTTONUP && bev->button == SDL_BUTTON_LEFT) {
             /* start grabbing all events */
@@ -537,7 +547,7 @@ static void handle_mousebutton(SDL_Event *ev)
         } else {
             buttonstate &= ~SDL_BUTTON(bev->button);
         }
-        sdl_send_mouse_event(scon, 0, 0, bev->x, bev->y, buttonstate);
+        sdl_send_mouse_event(scon, 0, 0, x, y, buttonstate);
     }
 }
 
-- 
2.49.0

[PATCH 6/9] ui/gtk: Don't update scale in fixed scale mode in gtk-egl.c

[Weifeng Liu]

Scale shouldn't be changed until user explicitly requests it in fixed
scale mode (full-screen=false and free-scale=false). Use function
gd_update_scale to complete scale updating instead.

Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
---
 ui/gtk-egl.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index 947c99334b..f8e4f4bc70 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -95,8 +95,9 @@ void gd_egl_draw(VirtualConsole *vc)
 #endif
         gd_egl_scanout_flush(&vc->gfx.dcl, 0, 0, vc->gfx.w, vc->gfx.h);
 
-        vc->gfx.scale_x = (double)ww / surface_width(vc->gfx.ds);
-        vc->gfx.scale_y = (double)wh / surface_height(vc->gfx.ds);
+        gd_update_scale(vc, ww, wh,
+                        surface_width(vc->gfx.ds),
+                        surface_height(vc->gfx.ds));
 
         glFlush();
 #ifdef CONFIG_GBM
@@ -122,8 +123,9 @@ void gd_egl_draw(VirtualConsole *vc)
 
         eglSwapBuffers(qemu_egl_display, vc->gfx.esurface);
 
-        vc->gfx.scale_x = (double)ww / surface_width(vc->gfx.ds);
-        vc->gfx.scale_y = (double)wh / surface_height(vc->gfx.ds);
+        gd_update_scale(vc, ww, wh,
+                        surface_width(vc->gfx.ds),
+                        surface_height(vc->gfx.ds));
 
         glFlush();
     }
-- 
2.49.0

[PATCH 7/9] ui/gtk: Consider scaling when propagating ui info

[Weifeng Liu]

The ui width and height sent to guest is supposed to be in buffer
coordinate. Hence conversion is required.

If scaling (global window scale and zooming scale) is not respected in
non-free-scale mode, window size could keep changing because of the
existence of the iteration of the following steps:

1. In resize event or configure event, a size larger (or smaller) than
   the currently used one might be calculated due to not considering
   scaling.
2. On reception of the display size change event in guest, the guest
   might decide to do a mode setting and use the larger (or smaller)
   mode.
3. When the new guest scan-out command arrives, QEMU would request the
   window size to change to fit the new buffer size. This will trigger a
   resize event or a configure event, making us go back to step 1.

Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
---
 ui/gtk.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 47af49e387..8c4a94c8f6 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -772,8 +772,21 @@ static void gd_resize_event(GtkGLArea *area,
                             gint width, gint height, gpointer *opaque)
 {
     VirtualConsole *vc = (void *)opaque;
+    double pw = width, ph = height;
+    double sx = vc->gfx.scale_x, sy = vc->gfx.scale_y;
+    GdkWindow *window = gtk_widget_get_window(GTK_WIDGET(area));
+    const int gs = gdk_window_get_scale_factor(window);
 
-    gd_set_ui_size(vc, width, height);
+    if (!vc->s->free_scale && !vc->s->full_screen) {
+        pw /= sx;
+        ph /= sy;
+    }
+
+    /**
+     * width and height here are in pixel coordinate, so we must divide it
+     * by global window scale (gs)
+     */
+    gd_set_ui_size(vc, pw / gs, ph / gs);
 }
 
 #endif
@@ -1836,8 +1849,16 @@ static gboolean gd_configure(GtkWidget *widget,
                              GdkEventConfigure *cfg, gpointer opaque)
 {
     VirtualConsole *vc = opaque;
+    const double sx = vc->gfx.scale_x, sy = vc->gfx.scale_y;
+    double width = cfg->width, height = cfg->height;
+
+    if (!vc->s->free_scale && !vc->s->full_screen) {
+        width /= sx;
+        height /= sy;
+    }
+
+    gd_set_ui_size(vc, width, height);
 
-    gd_set_ui_size(vc, cfg->width, cfg->height);
     return FALSE;
 }
 
-- 
2.49.0

[PATCH 8/9] ui/gtk-gl-area: Render guest content with padding in fixed-scale mode

[Weifeng Liu]

In fixed-scale mode (zoom-to-fit=false), we expect that scale should not
change, meaning that if window size is larger than guest surface,
padding is supposed to be added to preserve the scale. However, in
OpenGL mode (gl=on), guest surface is always painted to the whole canvas
without any padding. This change tries to fix this bug by adding
appropriate padding when drawing surfaces.

Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
---
 ui/gtk-gl-area.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index db93cd6204..8151cc413c 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -44,7 +44,9 @@ void gd_gl_area_draw(VirtualConsole *vc)
 #endif
     int pw, ph, gs, y1, y2;
     int ww, wh;
+    int ww_surface, wh_surface;
     int fbw, fbh;
+    int wx_offset, wy_offset;
 
     if (!vc->gfx.gls) {
         return;
@@ -61,6 +63,17 @@ void gd_gl_area_draw(VirtualConsole *vc)
 
     gd_update_scale(vc, ww, wh, fbw, fbh);
 
+    ww_surface = fbw * vc->gfx.scale_x;
+    wh_surface = fbh * vc->gfx.scale_y;
+
+    wx_offset = wy_offset = 0;
+    if (ww > ww_surface) {
+        wx_offset = (ww - ww_surface) / 2;
+    }
+    if (wh > wh_surface) {
+        wy_offset = (wh - wh_surface) / 2;
+    }
+
     if (vc->gfx.scanout_mode) {
         if (!vc->gfx.guest_fb.framebuffer) {
             return;
@@ -79,11 +92,29 @@ void gd_gl_area_draw(VirtualConsole *vc)
         glBindFramebuffer(GL_READ_FRAMEBUFFER, vc->gfx.guest_fb.framebuffer);
         /* GtkGLArea sets GL_DRAW_FRAMEBUFFER for us */
 
+        if (wx_offset > 0) {
+            glEnable(GL_SCISSOR_TEST);
+            glScissor(0, 0, wx_offset * gs, wh * gs);
+            glClear(GL_COLOR_BUFFER_BIT);
+            glScissor((ww - wx_offset) * gs, 0, wx_offset * gs, wh * gs);
+            glClear(GL_COLOR_BUFFER_BIT);
+            glDisable(GL_SCISSOR_TEST);
+        }
+        if (wy_offset > 0) {
+            glEnable(GL_SCISSOR_TEST);
+            glScissor(0, 0, ww * gs, wy_offset * gs);
+            glClear(GL_COLOR_BUFFER_BIT);
+            glScissor(0, (wh - wy_offset) * gs, ww * gs, wy_offset * gs);
+            glClear(GL_COLOR_BUFFER_BIT);
+            glDisable(GL_SCISSOR_TEST);
+        }
+
         glViewport(0, 0, pw, ph);
         y1 = vc->gfx.y0_top ? 0 : vc->gfx.h;
         y2 = vc->gfx.y0_top ? vc->gfx.h : 0;
         glBlitFramebuffer(0, y1, vc->gfx.w, y2,
-                          0, 0, pw, ph,
+                          wx_offset * gs, wy_offset * gs,
+                          (ww - wx_offset) * gs, (wh - wy_offset) * gs,
                           GL_COLOR_BUFFER_BIT, GL_NEAREST);
 #ifdef CONFIG_GBM
         if (dmabuf) {
-- 
2.49.0

[PATCH 9/9] ui/gtk-egl: Render guest content with padding in fixed-scale mode

[Weifeng Liu]

Scaling was not respected when rendering frames in gtk-egl.c (used if
gl=on and X11 mode). To fix this, add fields x and y to struct egl_fb
for x offset and y offset so we can add padding to window.

Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
---
 include/ui/egl-helpers.h |  4 +++-
 ui/egl-helpers.c         | 10 ++++++++--
 ui/gtk-egl.c             | 36 +++++++++++++++++++++++++++++++-----
 ui/sdl2-gl.c             |  2 +-
 4 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
index 4b8c0d2281..8f1670fd26 100644
--- a/include/ui/egl-helpers.h
+++ b/include/ui/egl-helpers.h
@@ -17,6 +17,8 @@ extern bool qemu_egl_angle_d3d;
 typedef struct egl_fb {
     int width;
     int height;
+    int x;
+    int y;
     GLuint texture;
     GLuint framebuffer;
     bool delete_texture;
@@ -26,7 +28,7 @@ typedef struct egl_fb {
 #define EGL_FB_INIT { 0, }
 
 void egl_fb_destroy(egl_fb *fb);
-void egl_fb_setup_default(egl_fb *fb, int width, int height);
+void egl_fb_setup_default(egl_fb *fb, int width, int height, int x, int y);
 void egl_fb_setup_for_tex(egl_fb *fb, int width, int height,
                           GLuint texture, bool delete);
 void egl_fb_setup_new_tex(egl_fb *fb, int width, int height);
diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index d591159480..8b60eb1062 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -92,14 +92,18 @@ void egl_fb_destroy(egl_fb *fb)
 
     fb->width = 0;
     fb->height = 0;
+    fb->x = 0;
+    fb->y = 0;
     fb->texture = 0;
     fb->framebuffer = 0;
 }
 
-void egl_fb_setup_default(egl_fb *fb, int width, int height)
+void egl_fb_setup_default(egl_fb *fb, int width, int height, int x, int y)
 {
     fb->width = width;
     fb->height = height;
+    fb->x = x;
+    fb->y = y;
     fb->framebuffer = 0; /* default framebuffer */
 }
 
@@ -144,6 +148,7 @@ void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip)
     glBindFramebuffer(GL_READ_FRAMEBUFFER, src->framebuffer);
     glBindFramebuffer(GL_DRAW_FRAMEBUFFER, dst->framebuffer);
     glViewport(0, 0, dst->width, dst->height);
+    glClear(GL_COLOR_BUFFER_BIT);
 
     if (src->dmabuf) {
         x1 = qemu_dmabuf_get_x(src->dmabuf);
@@ -160,7 +165,8 @@ void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip)
     x2 = x1 + w;
 
     glBlitFramebuffer(x1, y1, x2, y2,
-                      0, 0, dst->width, dst->height,
+                      dst->x, dst->y,
+                      dst->x + dst->width, dst->y + dst->height,
                       GL_COLOR_BUFFER_BIT, GL_LINEAR);
 }
 
diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index f8e4f4bc70..0b787bea25 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -340,7 +340,11 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl,
 {
     VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
     GdkWindow *window;
-    int ww, wh, ws;
+    int px_offset, py_offset;
+    int gs;
+    int pw_widget, ph_widget, pw_surface, ph_surface;
+    int ww_widget, wh_widget, ww_surface, wh_surface;
+    int fbw, fbh;
 
     if (!vc->gfx.scanout_mode) {
         return;
@@ -353,10 +357,32 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl,
                    vc->gfx.esurface, vc->gfx.ectx);
 
     window = gtk_widget_get_window(vc->gfx.drawing_area);
-    ws = gdk_window_get_scale_factor(window);
-    ww = gdk_window_get_width(window) * ws;
-    wh = gdk_window_get_height(window) * ws;
-    egl_fb_setup_default(&vc->gfx.win_fb, ww, wh);
+    gs = gdk_window_get_scale_factor(window);
+    ww_widget = gdk_window_get_width(window);
+    wh_widget = gdk_window_get_height(window);
+    fbw = surface_width(vc->gfx.ds);
+    fbh = surface_height(vc->gfx.ds);
+
+    gd_update_scale(vc, ww_widget, wh_widget, fbw, fbh);
+
+    ww_surface = fbw * vc->gfx.scale_x;
+    wh_surface = fbh * vc->gfx.scale_y;
+    pw_widget = ww_widget * gs;
+    ph_widget = wh_widget * gs;
+    pw_surface = ww_surface * gs;
+    ph_surface = wh_surface * gs;
+
+    px_offset = 0;
+    py_offset = 0;
+    if (pw_widget > pw_surface) {
+        px_offset = (pw_widget - pw_surface) / 2;
+    }
+    if (ph_widget > ph_surface) {
+        py_offset = (ph_widget - ph_surface) / 2;
+    }
+
+    egl_fb_setup_default(&vc->gfx.win_fb, pw_surface, ph_surface,
+                         px_offset, py_offset);
     if (vc->gfx.cursor_fb.texture) {
         egl_texture_blit(vc->gfx.gls, &vc->gfx.win_fb, &vc->gfx.guest_fb,
                          vc->gfx.y0_top);
diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
index e01d9ab0c7..3be17d1079 100644
--- a/ui/sdl2-gl.c
+++ b/ui/sdl2-gl.c
@@ -241,7 +241,7 @@ void sdl2_gl_scanout_flush(DisplayChangeListener *dcl,
     SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
 
     SDL_GetWindowSize(scon->real_window, &ww, &wh);
-    egl_fb_setup_default(&scon->win_fb, ww, wh);
+    egl_fb_setup_default(&scon->win_fb, ww, wh, 0, 0);
     egl_fb_blit(&scon->win_fb, &scon->guest_fb, !scon->y0_top);
 
     SDL_GL_SwapWindow(scon->real_window);
-- 
2.49.0

Re: [PATCH 1/9] ui/gtk: Document scale and coordinate handling

[Gerd Hoffmann]

On Sun, May 11, 2025 at 03:33:11PM +0800, Weifeng Liu wrote:
> The existence of multiple scaling factors forces us to deal with various
> coordinate systems and this would be confusing. It would be beneficial
> to define the concepts clearly and use consistent representation for
> variables in different coordinates.
> 
> Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
> ---
>  ui/gtk.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 982037b2c0..9f3171abc5 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -800,6 +800,71 @@ void gd_update_monitor_refresh_rate(VirtualConsole *vc, GtkWidget *widget)
>  #endif
>  }
>  
> +/**
> + * DOC: Coordinate handling.
> + *
> + * We are coping with sizes and positions in various coordinates and the
> + * handling of these coordinates is somewhat confusing. It would benefit us
> + * all if we define these coordinates explicitly and clearly. Besides, it's
> + * also helpful to follow the same naming convention for variables
> + * representing values in different coordinates.
> + *
> + * I. Definitions
> + *
> + * - (guest) buffer coordinate: this is the coordinates that the guest will
> + *   see. The x/y offsets and width/height specified in commands sent by
> + *   guest is basically in buffer coordinate.
> + *
> + * - (host) pixel coordinate: this is the coordinate in pixel level on the
> + *   host destop. A window/widget of width 300 in pixel coordinate means it
> + *   occupies 300 pixels horizontally.
> + *
> + * - (host) logical window coordinate: the existence of global scaling
> + *   factor in desktop level makes this kind of coordinate play a role. It
> + *   always holds that (logical window size) * (global scale factor) =
> + *   (pixel size).
> + *
> + * - global scale factor: this is specified in desktop level and is
> + *   typically invariant during the life cycle of the process. Users with
> + *   high-DPI monitors might set this scale, for example, to 2, in order to
> + *   make the UI look larger.
> + *
> + * - zooming scale: this can be freely controlled by the QEMU user to zoom
> + *   in/out the guest content.
> + *
> + * II. Representation
> + *
> + * We'd like to use consistent representation for variables in different
> + * coordinates:
> + * - buffer coordinate: prefix fb
> + * - pixel coordinate: prefix p
> + * - logical window coordinate: prefix w
> + *
> + * For scales:
> + * - global scale factor: prefix gs
> + * - zooming scale: prefix scale/s
> + *
> + * Example: fbw, pw, ww for width in different coordinates
> + *
> + * III. Equation
> + *
> + * - fbw * gs * scale_x = pw

Well.  That is one possible approach (and this is what qemu is doing
today, for historical reasons, because most code dates back to pre
high-dpi days).

A possible alternative would be to go for fbw * scale_x = pw, i.e. let
the guest run in pixel coordinates instead of window coordinates.  The
guest would do the high-dpi scaling then.  That requires setting
physical display width and height in ui_info, so the guest can figure
what the display resolution is and go into high-dpi mode if needed.

We probably also need a non-high-dpi compatibility mode for old guests.
That mode would start with "zooming scale = global scale" instead of
"zooming scale = 1", and the dpi calculation would have to consider
that too.

(maybe best done on top of this nice cleanup).

take care,
  Gerd

Re: [PATCH 0/9] ui: Improve scale handling

[Gerd Hoffmann]

On Sun, May 11, 2025 at 03:33:10PM +0800, Weifeng Liu wrote:
> Hi all,
> 
> Now we have quite a lot of display backends for different use cases.
> Even in the context of gtk, we have various implementations (e.g., gl=on
> vs gl=off, X11 vs Wayland). However, behaviors to users are not aligned
> across the backends, especially in the part of scale handling. This
> patch set attempts to improve scale handling.
> 
> We have to deal with various coordinates due to the existence of scaling
> in different level. Firstly, in desktop level, we could have a global
> window scale factor. Secondly, users might set a zooming factor to
> adjust the size of guest content in scan-out level. Consequently, 1) the
> buffer from guest, 2) the host window and 3) OpenGl drawing area inside
> the host window are in distinct coordinates. It's important to define
> these coordinates and scales unambiguously and use a consistent naming
> convention for variables representing different concepts. The first
> patch in this set tries to achieve this goal by adding a document in
> gtk.c, and the next patch (PATCH 2) attempts to align the code with the
> document.
> 
> PATCH 3 - 5 fix bugs in mouse position calculation due to not handling
> scale properly, for both gtk and sdl.
> 
> PATCH 6 align scale update logic in gtk-egl with other implementations.
> 
> PATCH 7 fix an issue that gtk window might keep enlarging/shrinking because 
> ui info propagating to guest not considering scale.
> 
> PATCH 8 and 9 align fixed-scale mode behavior in gtk-gl-area and gtk-egl with
> other implementations by adding appropriate padding to the window to preserve
> the scale.

Series:
Acked-by: Gerd Hoffmann <kraxel@redhat.com>

take care,
  Gerd

RE: [PATCH 8/9] ui/gtk-gl-area: Render guest content with padding in fixed-scale mode

[Kim, Dongwon]

Hi,

> Subject: [PATCH 8/9] ui/gtk-gl-area: Render guest content with padding in
> fixed-scale mode
> 
> In fixed-scale mode (zoom-to-fit=false), we expect that scale should not
> change, meaning that if window size is larger than guest surface, padding is

# @zoom-to-fit: Zoom guest display to fit into the host window.  When
#     turned off the host window will be resized instead.  In case the
#     display device can notify the guest on window resizes
#     (virtio-gpu) this will default to "on", assuming the guest will
#     resize the display to match the window size then.  Otherwise it
#     defaults to "off".  (Since 3.1)

Current definition says the host window should be resized to fit the
size of the guest surface instead. Wouldn't padding accommodate this? 

> supposed to be added to preserve the scale. However, in OpenGL mode
> (gl=on), guest surface is always painted to the whole canvas without any
> padding. This change tries to fix this bug by adding appropriate padding
> when drawing surfaces.
> 
> Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
> ---
>  ui/gtk-gl-area.c | 33 ++++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c index db93cd6204..8151cc413c
> 100644
> --- a/ui/gtk-gl-area.c
> +++ b/ui/gtk-gl-area.c
> @@ -44,7 +44,9 @@ void gd_gl_area_draw(VirtualConsole *vc)  #endif
>      int pw, ph, gs, y1, y2;
>      int ww, wh;
> +    int ww_surface, wh_surface;
>      int fbw, fbh;
> +    int wx_offset, wy_offset;
> 
>      if (!vc->gfx.gls) {
>          return;
> @@ -61,6 +63,17 @@ void gd_gl_area_draw(VirtualConsole *vc)
> 
>      gd_update_scale(vc, ww, wh, fbw, fbh);
> 
> +    ww_surface = fbw * vc->gfx.scale_x;
> +    wh_surface = fbh * vc->gfx.scale_y;
> +
> +    wx_offset = wy_offset = 0;
> +    if (ww > ww_surface) {
> +        wx_offset = (ww - ww_surface) / 2;
> +    }
> +    if (wh > wh_surface) {
> +        wy_offset = (wh - wh_surface) / 2;
> +    }
> +
>      if (vc->gfx.scanout_mode) {
>          if (!vc->gfx.guest_fb.framebuffer) {
>              return;
> @@ -79,11 +92,29 @@ void gd_gl_area_draw(VirtualConsole *vc)
>          glBindFramebuffer(GL_READ_FRAMEBUFFER, vc-
> >gfx.guest_fb.framebuffer);
>          /* GtkGLArea sets GL_DRAW_FRAMEBUFFER for us */
> 
> +        if (wx_offset > 0) {
> +            glEnable(GL_SCISSOR_TEST);
> +            glScissor(0, 0, wx_offset * gs, wh * gs);
> +            glClear(GL_COLOR_BUFFER_BIT);
> +            glScissor((ww - wx_offset) * gs, 0, wx_offset * gs, wh * gs);
> +            glClear(GL_COLOR_BUFFER_BIT);
> +            glDisable(GL_SCISSOR_TEST);
> +        }
> +        if (wy_offset > 0) {
> +            glEnable(GL_SCISSOR_TEST);
> +            glScissor(0, 0, ww * gs, wy_offset * gs);
> +            glClear(GL_COLOR_BUFFER_BIT);
> +            glScissor(0, (wh - wy_offset) * gs, ww * gs, wy_offset * gs);
> +            glClear(GL_COLOR_BUFFER_BIT);
> +            glDisable(GL_SCISSOR_TEST);
> +        }
> +
>          glViewport(0, 0, pw, ph);
>          y1 = vc->gfx.y0_top ? 0 : vc->gfx.h;
>          y2 = vc->gfx.y0_top ? vc->gfx.h : 0;
>          glBlitFramebuffer(0, y1, vc->gfx.w, y2,
> -                          0, 0, pw, ph,
> +                          wx_offset * gs, wy_offset * gs,
> +                          (ww - wx_offset) * gs, (wh - wy_offset) * gs,
>                            GL_COLOR_BUFFER_BIT, GL_NEAREST);  #ifdef CONFIG_GBM
>          if (dmabuf) {
> --
> 2.49.0
> 

Thanks

RE: [PATCH 3/9] gtk/ui: Introduce helper gd_update_scale

[Kim, Dongwon]

Hi,

> Subject: [PATCH 3/9] gtk/ui: Introduce helper gd_update_scale
> 
> The code snippet updating scale_x/scale_y is general and will be used in next
> patch. Make it a function.
> 
> Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
> ---
>  include/ui/gtk.h |  2 ++
>  ui/gtk.c         | 30 +++++++++++++++++++-----------
>  2 files changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/include/ui/gtk.h b/include/ui/gtk.h index
> aa3d637029..d3944046db 100644
> --- a/include/ui/gtk.h
> +++ b/include/ui/gtk.h
> @@ -224,4 +224,6 @@ int gd_gl_area_make_current(DisplayGLCtx *dgc,
>  /* gtk-clipboard.c */
>  void gd_clipboard_init(GtkDisplayState *gd);
> 
> +void gd_update_scale(VirtualConsole *vc, int ww, int wh, int fbw, int
> +fbh);
> +
>  #endif /* UI_GTK_H */
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 8f5bb4b62e..47af49e387 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -801,6 +801,24 @@ void
> gd_update_monitor_refresh_rate(VirtualConsole *vc, GtkWidget *widget)
> #endif  }
> 
> +void gd_update_scale(VirtualConsole *vc, int ww, int wh, int fbw, int
> +fbh) {
> +    if (!vc) {
> +        return;
> +    }
> +
> +    if (vc->s->full_screen) {
> +        vc->gfx.scale_x = (double)ww / fbw;
> +        vc->gfx.scale_y = (double)wh / fbh;
> +    } else if (vc->s->free_scale) {
> +        double sx, sy;
> +
> +        sx = (double)ww / fbw;
> +        sy = (double)wh / fbh;
> +
> +        vc->gfx.scale_x = vc->gfx.scale_y = MIN(sx, sy);
 
I assume you are trying to keep the w/h ratio same here in case free-scale == true.
Why would we do that? We can easily stretch the host window to any direction then the scale-x and scale-y
could be different any time.

> +    }
> +}
>  /**
>   * DOC: Coordinate handling.
>   *
> @@ -908,17 +926,7 @@ static gboolean gd_draw_event(GtkWidget *widget,
> cairo_t *cr, void *opaque)
>      ww_widget = gdk_window_get_width(gtk_widget_get_window(widget));
>      wh_widget = gdk_window_get_height(gtk_widget_get_window(widget));
> 
> -    if (s->full_screen) {
> -        vc->gfx.scale_x = (double)ww_widget / fbw;
> -        vc->gfx.scale_y = (double)wh_widget / fbh;
> -    } else if (s->free_scale) {
> -        double sx, sy;
> -
> -        sx = (double)ww_widget / fbw;
> -        sy = (double)wh_widget / fbh;
> -
> -        vc->gfx.scale_x = vc->gfx.scale_y = MIN(sx, sy);
> -    }
> +    gd_update_scale(vc, ww_widget, wh_widget, fbw, fbh);
> 
>      ww_surface = fbw * vc->gfx.scale_x;
>      wh_surface = fbh * vc->gfx.scale_y;
> --
> 2.49.0
> 

Re: [PATCH 3/9] gtk/ui: Introduce helper gd_update_scale

[Weifeng Liu]

Hi Dongwon,

On Tue, 2025-05-13 at 01:26 +0000, Kim, Dongwon wrote:
> Hi,
> 
> > Subject: [PATCH 3/9] gtk/ui: Introduce helper gd_update_scale
> > 
> > The code snippet updating scale_x/scale_y is general and will be
> > used in next
> > patch. Make it a function.
> > 
> > Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
> > ---
> >  include/ui/gtk.h |  2 ++
> >  ui/gtk.c         | 30 +++++++++++++++++++-----------
> >  2 files changed, 21 insertions(+), 11 deletions(-)
> > 
> > diff --git a/include/ui/gtk.h b/include/ui/gtk.h index
> > aa3d637029..d3944046db 100644
> > --- a/include/ui/gtk.h
> > +++ b/include/ui/gtk.h
> > @@ -224,4 +224,6 @@ int gd_gl_area_make_current(DisplayGLCtx *dgc,
> >  /* gtk-clipboard.c */
> >  void gd_clipboard_init(GtkDisplayState *gd);
> > 
> > +void gd_update_scale(VirtualConsole *vc, int ww, int wh, int fbw,
> > int
> > +fbh);
> > +
> >  #endif /* UI_GTK_H */
> > diff --git a/ui/gtk.c b/ui/gtk.c
> > index 8f5bb4b62e..47af49e387 100644
> > --- a/ui/gtk.c
> > +++ b/ui/gtk.c
> > @@ -801,6 +801,24 @@ void
> > gd_update_monitor_refresh_rate(VirtualConsole *vc, GtkWidget
> > *widget)
> > #endif  }
> > 
> > +void gd_update_scale(VirtualConsole *vc, int ww, int wh, int fbw,
> > int
> > +fbh) {
> > +    if (!vc) {
> > +        return;
> > +    }
> > +
> > +    if (vc->s->full_screen) {
> > +        vc->gfx.scale_x = (double)ww / fbw;
> > +        vc->gfx.scale_y = (double)wh / fbh;
> > +    } else if (vc->s->free_scale) {
> > +        double sx, sy;
> > +
> > +        sx = (double)ww / fbw;
> > +        sy = (double)wh / fbh;
> > +
> > +        vc->gfx.scale_x = vc->gfx.scale_y = MIN(sx, sy);
>  
> I assume you are trying to keep the w/h ratio same here in case free-
> scale == true.
> Why would we do that? We can easily stretch the host window to any
> direction then the scale-x and scale-y
> could be different any time.
> 
Currently, the code doesn’t clarify how we should handle aspect ratios.
However, I noticed that in the gd_draw_event function, when free-
scale=true, it preserves a fixed aspect ratio. I believe this is a
reasonable approach (in my humble opinion, it's unlikely that people
want to see distorted images), so I’ve decided to retain this behavior
for now and align other parts to follow the same logic, ensuring a
consistent experience for users.

Best regards,
Weifeng

> > +    }
> > +}
> >  /**
> >   * DOC: Coordinate handling.
> >   *
> > @@ -908,17 +926,7 @@ static gboolean gd_draw_event(GtkWidget
> > *widget,
> > cairo_t *cr, void *opaque)
> >      ww_widget =
> > gdk_window_get_width(gtk_widget_get_window(widget));
> >      wh_widget =
> > gdk_window_get_height(gtk_widget_get_window(widget));
> > 
> > -    if (s->full_screen) {
> > -        vc->gfx.scale_x = (double)ww_widget / fbw;
> > -        vc->gfx.scale_y = (double)wh_widget / fbh;
> > -    } else if (s->free_scale) {
> > -        double sx, sy;
> > -
> > -        sx = (double)ww_widget / fbw;
> > -        sy = (double)wh_widget / fbh;
> > -
> > -        vc->gfx.scale_x = vc->gfx.scale_y = MIN(sx, sy);
> > -    }
> > +    gd_update_scale(vc, ww_widget, wh_widget, fbw, fbh);
> > 
> >      ww_surface = fbw * vc->gfx.scale_x;
> >      wh_surface = fbh * vc->gfx.scale_y;
> > --
> > 2.49.0
> > 

Re: [PATCH 8/9] ui/gtk-gl-area: Render guest content with padding in fixed-scale mode

[Weifeng Liu]

Hi,

On Tue, 2025-05-13 at 00:37 +0000, Kim, Dongwon wrote:
> Hi,
> 
> > Subject: [PATCH 8/9] ui/gtk-gl-area: Render guest content with
> > padding in
> > fixed-scale mode
> > 
> > In fixed-scale mode (zoom-to-fit=false), we expect that scale
> > should not
> > change, meaning that if window size is larger than guest surface,
> > padding is
> 
> # @zoom-to-fit: Zoom guest display to fit into the host window.  When
> #     turned off the host window will be resized instead.  In case
> the
> #     display device can notify the guest on window resizes
> #     (virtio-gpu) this will default to "on", assuming the guest will
> #     resize the display to match the window size then.  Otherwise it
> #     defaults to "off".  (Since 3.1)
> 
> Current definition says the host window should be resized to fit the
> size of the guest surface instead. Wouldn't padding accommodate this?
> 

Yeah, window will be resized to fit the size of guest surface in fixed-
scale mode. However, users are still allowed to resize the window to a
larger size and this is case where padding is required, otherwise the
fixed-scale assumption is broken. In fact, gl=off mode employs padding
to preserve scale already but gl=on mode doesn't follow this behavior,
which, IMO, is a defect that this patch is trying to correct.

Best regards,
Weifeng

> > supposed to be added to preserve the scale. However, in OpenGL mode
> > (gl=on), guest surface is always painted to the whole canvas
> > without any
> > padding. This change tries to fix this bug by adding appropriate
> > padding
> > when drawing surfaces.
> > 
> > Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
> > ---
> >  ui/gtk-gl-area.c | 33 ++++++++++++++++++++++++++++++++-
> >  1 file changed, 32 insertions(+), 1 deletion(-)
> > 
> > diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c index
> > db93cd6204..8151cc413c
> > 100644
> > --- a/ui/gtk-gl-area.c
> > +++ b/ui/gtk-gl-area.c
> > @@ -44,7 +44,9 @@ void gd_gl_area_draw(VirtualConsole *vc)  #endif
> >      int pw, ph, gs, y1, y2;
> >      int ww, wh;
> > +    int ww_surface, wh_surface;
> >      int fbw, fbh;
> > +    int wx_offset, wy_offset;
> > 
> >      if (!vc->gfx.gls) {
> >          return;
> > @@ -61,6 +63,17 @@ void gd_gl_area_draw(VirtualConsole *vc)
> > 
> >      gd_update_scale(vc, ww, wh, fbw, fbh);
> > 
> > +    ww_surface = fbw * vc->gfx.scale_x;
> > +    wh_surface = fbh * vc->gfx.scale_y;
> > +
> > +    wx_offset = wy_offset = 0;
> > +    if (ww > ww_surface) {
> > +        wx_offset = (ww - ww_surface) / 2;
> > +    }
> > +    if (wh > wh_surface) {
> > +        wy_offset = (wh - wh_surface) / 2;
> > +    }
> > +
> >      if (vc->gfx.scanout_mode) {
> >          if (!vc->gfx.guest_fb.framebuffer) {
> >              return;
> > @@ -79,11 +92,29 @@ void gd_gl_area_draw(VirtualConsole *vc)
> >          glBindFramebuffer(GL_READ_FRAMEBUFFER, vc-
> > > gfx.guest_fb.framebuffer);
> >          /* GtkGLArea sets GL_DRAW_FRAMEBUFFER for us */
> > 
> > +        if (wx_offset > 0) {
> > +            glEnable(GL_SCISSOR_TEST);
> > +            glScissor(0, 0, wx_offset * gs, wh * gs);
> > +            glClear(GL_COLOR_BUFFER_BIT);
> > +            glScissor((ww - wx_offset) * gs, 0, wx_offset * gs, wh
> > * gs);
> > +            glClear(GL_COLOR_BUFFER_BIT);
> > +            glDisable(GL_SCISSOR_TEST);
> > +        }
> > +        if (wy_offset > 0) {
> > +            glEnable(GL_SCISSOR_TEST);
> > +            glScissor(0, 0, ww * gs, wy_offset * gs);
> > +            glClear(GL_COLOR_BUFFER_BIT);
> > +            glScissor(0, (wh - wy_offset) * gs, ww * gs, wy_offset
> > * gs);
> > +            glClear(GL_COLOR_BUFFER_BIT);
> > +            glDisable(GL_SCISSOR_TEST);
> > +        }
> > +
> >          glViewport(0, 0, pw, ph);
> >          y1 = vc->gfx.y0_top ? 0 : vc->gfx.h;
> >          y2 = vc->gfx.y0_top ? vc->gfx.h : 0;
> >          glBlitFramebuffer(0, y1, vc->gfx.w, y2,
> > -                          0, 0, pw, ph,
> > +                          wx_offset * gs, wy_offset * gs,
> > +                          (ww - wx_offset) * gs, (wh - wy_offset)
> > * gs,
> >                            GL_COLOR_BUFFER_BIT, GL_NEAREST); 
> > #ifdef CONFIG_GBM
> >          if (dmabuf) {
> > --
> > 2.49.0
> > 
> 
> Thanks

Re: [PATCH 8/9] ui/gtk-gl-area: Render guest content with padding in fixed-scale mode

[BALATON Zoltan]

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

On Tue, 13 May 2025, Weifeng Liu wrote:
> Hi,
>
> On Tue, 2025-05-13 at 00:37 +0000, Kim, Dongwon wrote:
>> Hi,
>> 
>> > Subject: [PATCH 8/9] ui/gtk-gl-area: Render guest content with
>> > padding in
>> > fixed-scale mode
>> > 
>> > In fixed-scale mode (zoom-to-fit=false), we expect that scale
>> > should not
>> > change, meaning that if window size is larger than guest surface,
>> > padding is
>> 
>> # @zoom-to-fit: Zoom guest display to fit into the host window.  When
>> #     turned off the host window will be resized instead.  In case
>> the
>> #     display device can notify the guest on window resizes
>> #     (virtio-gpu) this will default to "on", assuming the guest will
>> #     resize the display to match the window size then.  Otherwise it
>> #     defaults to "off".  (Since 3.1)
>> 
>> Current definition says the host window should be resized to fit the
>> size of the guest surface instead. Wouldn't padding accommodate this?
>> 
>
> Yeah, window will be resized to fit the size of guest surface in fixed-
> scale mode. However, users are still allowed to resize the window to a
> larger size and this is case where padding is required, otherwise the
> fixed-scale assumption is broken. In fact, gl=off mode employs padding
> to preserve scale already but gl=on mode doesn't follow this behavior,
> which, IMO, is a defect that this patch is trying to correct.

I think current set of switches is not enough to describe all possible 
configs and this leads to inconsistency between display backends. Each 
display backend has different idea on how zoom-to-fit should work now. 
Maybe we need a new keep-aspect=off or similar option to make it explicit 
then these can be set independently to decide if a full-screen zoom-to-fit 
window should be stretched or padded. Currently it behaves differently 
depending on display backend or even options of one display backend as you 
say above. Fixing just one place won't solve the problem with other 
backends so maybe separating this option into a new one would end this 
inconsistency. I got requests from people for both padded or stretched 
behaviour so it seems some prefer one or the other and just zoom-to-fit 
can't set both.

Regards,
BALATON Zoltan

Re: [PATCH 8/9] ui/gtk-gl-area: Render guest content with padding in fixed-scale mode

[Weifeng Liu]

Hi,

On Tue, 2025-05-13 at 11:52 +0200, BALATON Zoltan wrote:
> On Tue, 13 May 2025, Weifeng Liu wrote:
> > Hi,
> > 
> > On Tue, 2025-05-13 at 00:37 +0000, Kim, Dongwon wrote:
> > > Hi,
> > > 
> > > > Subject: [PATCH 8/9] ui/gtk-gl-area: Render guest content with
> > > > padding in
> > > > fixed-scale mode
> > > > 
> > > > In fixed-scale mode (zoom-to-fit=false), we expect that scale
> > > > should not
> > > > change, meaning that if window size is larger than guest surface,
> > > > padding is
> > > 
> > > # @zoom-to-fit: Zoom guest display to fit into the host window.  When
> > > #     turned off the host window will be resized instead.  In case
> > > the
> > > #     display device can notify the guest on window resizes
> > > #     (virtio-gpu) this will default to "on", assuming the guest will
> > > #     resize the display to match the window size then.  Otherwise it
> > > #     defaults to "off".  (Since 3.1)
> > > 
> > > Current definition says the host window should be resized to fit the
> > > size of the guest surface instead. Wouldn't padding accommodate this?
> > > 
> > 
> > Yeah, window will be resized to fit the size of guest surface in fixed-
> > scale mode. However, users are still allowed to resize the window to a
> > larger size and this is case where padding is required, otherwise the
> > fixed-scale assumption is broken. In fact, gl=off mode employs padding
> > to preserve scale already but gl=on mode doesn't follow this behavior,
> > which, IMO, is a defect that this patch is trying to correct.
> 
> I think current set of switches is not enough to describe all possible 
> configs and this leads to inconsistency between display backends. Each 
> display backend has different idea on how zoom-to-fit should work now. 
> Maybe we need a new keep-aspect=off or similar option to make it explicit 
> then these can be set independently to decide if a full-screen zoom-to-fit 
> window should be stretched or padded. Currently it behaves differently 
> depending on display backend or even options of one display backend as you 
> say above. Fixing just one place won't solve the problem with other 
> backends so maybe separating this option into a new one would end this 
> inconsistency. I got requests from people for both padded or stretched 
> behaviour so it seems some prefer one or the other and just zoom-to-fit 
> can't set both.
> 

Thank you for pointing out the demand for both stretched and padded
behavior — allowing users to choose their preferred display makes a lot
of sense. With the changes in this patch set, we can at least ensure
that all GTK-based backends behave consistently with regard to aspect
ratio. I’ll follow up with a separate patch set to introduce the new
keep-aspect=off (or similar) option you suggested.

By the way, I’ve also been working on a “scale” option to let users
specify an exact zoom level. I deliberately left it out of this patch
set because I wanted to keep its scope narrowly focused on refactoring.
Once this set is merged, I’ll submit the new patch set with these
changes.

Best regards,
Weifeng

> Regards,
> BALATON Zoltan

Re: [PATCH 8/9] ui/gtk-gl-area: Render guest content with padding in fixed-scale mode

[BALATON Zoltan]

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

On Tue, 13 May 2025, Weifeng Liu wrote:
> Hi,
>
> On Tue, 2025-05-13 at 11:52 +0200, BALATON Zoltan wrote:
>> On Tue, 13 May 2025, Weifeng Liu wrote:
>>> Hi,
>>>
>>> On Tue, 2025-05-13 at 00:37 +0000, Kim, Dongwon wrote:
>>>> Hi,
>>>>
>>>>> Subject: [PATCH 8/9] ui/gtk-gl-area: Render guest content with
>>>>> padding in
>>>>> fixed-scale mode
>>>>>
>>>>> In fixed-scale mode (zoom-to-fit=false), we expect that scale
>>>>> should not
>>>>> change, meaning that if window size is larger than guest surface,
>>>>> padding is
>>>>
>>>> # @zoom-to-fit: Zoom guest display to fit into the host window.  When
>>>> #     turned off the host window will be resized instead.  In case
>>>> the
>>>> #     display device can notify the guest on window resizes
>>>> #     (virtio-gpu) this will default to "on", assuming the guest will
>>>> #     resize the display to match the window size then.  Otherwise it
>>>> #     defaults to "off".  (Since 3.1)
>>>>
>>>> Current definition says the host window should be resized to fit the
>>>> size of the guest surface instead. Wouldn't padding accommodate this?
>>>>
>>>
>>> Yeah, window will be resized to fit the size of guest surface in fixed-
>>> scale mode. However, users are still allowed to resize the window to a
>>> larger size and this is case where padding is required, otherwise the
>>> fixed-scale assumption is broken. In fact, gl=off mode employs padding
>>> to preserve scale already but gl=on mode doesn't follow this behavior,
>>> which, IMO, is a defect that this patch is trying to correct.
>>
>> I think current set of switches is not enough to describe all possible
>> configs and this leads to inconsistency between display backends. Each
>> display backend has different idea on how zoom-to-fit should work now.
>> Maybe we need a new keep-aspect=off or similar option to make it explicit
>> then these can be set independently to decide if a full-screen zoom-to-fit
>> window should be stretched or padded. Currently it behaves differently
>> depending on display backend or even options of one display backend as you
>> say above. Fixing just one place won't solve the problem with other
>> backends so maybe separating this option into a new one would end this
>> inconsistency. I got requests from people for both padded or stretched
>> behaviour so it seems some prefer one or the other and just zoom-to-fit
>> can't set both.
>>
>
> Thank you for pointing out the demand for both stretched and padded
> behavior — allowing users to choose their preferred display makes a lot
> of sense. With the changes in this patch set, we can at least ensure
> that all GTK-based backends behave consistently with regard to aspect
> ratio. I’ll follow up with a separate patch set to introduce the new
> keep-aspect=off (or similar) option you suggested.
>
> By the way, I’ve also been working on a “scale” option to let users
> specify an exact zoom level. I deliberately left it out of this patch
> set because I wanted to keep its scope narrowly focused on refactoring.
> Once this set is merged, I’ll submit the new patch set with these
> changes.

A scale option would be a welcome change as well. I thought about that too 
but did not try to implement it. One problem left may be that different 
backends may be still inconsistent so if you only fix gtk others may need 
to be adapted as well at some point (not necessarily by you but if you can 
look at that and keep it consistent when making changes that would be 
nice).

Regards,
BALATON Zoltan

RE: [PATCH 3/9] gtk/ui: Introduce helper gd_update_scale

[Kim, Dongwon]

> Hi Dongwon,

> On Tue, 2025-05-13 at 01:26 +0000, Kim, Dongwon wrote:
> > Hi,
> >
> > > Subject: [PATCH 3/9] gtk/ui: Introduce helper gd_update_scale
> > > 
> > > The code snippet updating scale_x/scale_y is general and will be
> > > used in next
> > > patch. Make it a function.
> > > 
> > > Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
> > > ---
> > >  include/ui/gtk.h |  2 ++
> > >  ui/gtk.c         | 30 +++++++++++++++++++-----------
> > >  2 files changed, 21 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/include/ui/gtk.h b/include/ui/gtk.h index
> > > aa3d637029..d3944046db 100644
> > > --- a/include/ui/gtk.h
> > > +++ b/include/ui/gtk.h
> > > @@ -224,4 +224,6 @@ int gd_gl_area_make_current(DisplayGLCtx *dgc,
> > >  /* gtk-clipboard.c */
> > >  void gd_clipboard_init(GtkDisplayState *gd);
> > > 
> > > +void gd_update_scale(VirtualConsole *vc, int ww, int wh, int fbw,
> > > int
> > > +fbh);
> > > +
> > >  #endif /* UI_GTK_H */
> > > diff --git a/ui/gtk.c b/ui/gtk.c
> > > index 8f5bb4b62e..47af49e387 100644
> > > --- a/ui/gtk.c
> > > +++ b/ui/gtk.c
> > > @@ -801,6 +801,24 @@ void
> > > gd_update_monitor_refresh_rate(VirtualConsole *vc, GtkWidget
> > > *widget)
> > > #endif  }
> > > 
> > > +void gd_update_scale(VirtualConsole *vc, int ww, int wh, int fbw,
> > > int
> > > +fbh) {
> > > +    if (!vc) {
> > > +        return;
> > > +    }
> > > +
> > > +    if (vc->s->full_screen) {
> > > +        vc->gfx.scale_x = (double)ww / fbw;
> > > +        vc->gfx.scale_y = (double)wh / fbh;
> > > +    } else if (vc->s->free_scale) {
> > > +        double sx, sy;
> > > +
> > > +        sx = (double)ww / fbw;
> > > +        sy = (double)wh / fbh;
> > > +
> > > +        vc->gfx.scale_x = vc->gfx.scale_y = MIN(sx, sy);
> > 
> > I assume you are trying to keep the w/h ratio same here in case free-
> > scale == true.
> > Why would we do that? We can easily stretch the host window to any
> > direction then the scale-x and scale-y
> > could be different any time.
> >
> Currently, the code doesn't clarify how we should handle aspect ratios.
> However, I noticed that in the gd_draw_event function, when free-
> scale=true, it preserves a fixed aspect ratio. I believe this is a
> reasonable approach (in my humble opinion, it's unlikely that people
> want to see distorted images), so I've decided to retain this behavior
> for now and align other parts to follow the same logic, ensuring a
> consistent experience for users.

Yeah, I didn't realize it does keep the ratio in case of gl=off in the original code.
The reason I brought up this is because as you may Have noticed, scale_x and scale_y
have been individually configured based on width and height of the current window
and guest surface in gtk-egl and gtk-gl-area, which is being changed with your patches.
I am not sure which one is the best either but this would definitely require some discussion.

> Best regards,
> Weifeng

> > > +    }
> > > +}
> > >  /**
> > >   * DOC: Coordinate handling.
> > >   *
> > > @@ -908,17 +926,7 @@ static gboolean gd_draw_event(GtkWidget
> > > *widget,
> > > cairo_t *cr, void *opaque)
> > >      ww_widget =
> > > gdk_window_get_width(gtk_widget_get_window(widget));
> > >      wh_widget =
> > > gdk_window_get_height(gtk_widget_get_window(widget));
> > > 
> > > -    if (s->full_screen) {
> > > -        vc->gfx.scale_x = (double)ww_widget / fbw;
> > > -        vc->gfx.scale_y = (double)wh_widget / fbh;
> > > -    } else if (s->free_scale) {
> > > -        double sx, sy;
> > > -
> > > -        sx = (double)ww_widget / fbw;
> > > -        sy = (double)wh_widget / fbh;
> > > -
> > > -        vc->gfx.scale_x = vc->gfx.scale_y = MIN(sx, sy);
> > > -    }
> > > +    gd_update_scale(vc, ww_widget, wh_widget, fbw, fbh);
> > > 
> > >      ww_surface = fbw * vc->gfx.scale_x;
> > >      wh_surface = fbh * vc->gfx.scale_y;
> > > --
> > > 2.49.0
> > >

Re: [PATCH 3/9] gtk/ui: Introduce helper gd_update_scale

[Weifeng Liu]

On Tue, 2025-05-13 at 20:01 +0000, Kim, Dongwon wrote:
> > Hi Dongwon,
> 
> > On Tue, 2025-05-13 at 01:26 +0000, Kim, Dongwon wrote:
> > > Hi,
> > > 
> > > > Subject: [PATCH 3/9] gtk/ui: Introduce helper gd_update_scale
> > > > 
> > > > The code snippet updating scale_x/scale_y is general and will be
> > > > used in next
> > > > patch. Make it a function.
> > > > 
> > > > Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
> > > > ---
> > > >  include/ui/gtk.h |  2 ++
> > > >  ui/gtk.c         | 30 +++++++++++++++++++-----------
> > > >  2 files changed, 21 insertions(+), 11 deletions(-)
> > > > 
> > > > diff --git a/include/ui/gtk.h b/include/ui/gtk.h index
> > > > aa3d637029..d3944046db 100644
> > > > --- a/include/ui/gtk.h
> > > > +++ b/include/ui/gtk.h
> > > > @@ -224,4 +224,6 @@ int gd_gl_area_make_current(DisplayGLCtx *dgc,
> > > >  /* gtk-clipboard.c */
> > > >  void gd_clipboard_init(GtkDisplayState *gd);
> > > > 
> > > > +void gd_update_scale(VirtualConsole *vc, int ww, int wh, int fbw,
> > > > int
> > > > +fbh);
> > > > +
> > > >  #endif /* UI_GTK_H */
> > > > diff --git a/ui/gtk.c b/ui/gtk.c
> > > > index 8f5bb4b62e..47af49e387 100644
> > > > --- a/ui/gtk.c
> > > > +++ b/ui/gtk.c
> > > > @@ -801,6 +801,24 @@ void
> > > > gd_update_monitor_refresh_rate(VirtualConsole *vc, GtkWidget
> > > > *widget)
> > > > #endif  }
> > > > 
> > > > +void gd_update_scale(VirtualConsole *vc, int ww, int wh, int fbw,
> > > > int
> > > > +fbh) {
> > > > +    if (!vc) {
> > > > +        return;
> > > > +    }
> > > > +
> > > > +    if (vc->s->full_screen) {
> > > > +        vc->gfx.scale_x = (double)ww / fbw;
> > > > +        vc->gfx.scale_y = (double)wh / fbh;
> > > > +    } else if (vc->s->free_scale) {
> > > > +        double sx, sy;
> > > > +
> > > > +        sx = (double)ww / fbw;
> > > > +        sy = (double)wh / fbh;
> > > > +
> > > > +        vc->gfx.scale_x = vc->gfx.scale_y = MIN(sx, sy);
> > > 
> > > I assume you are trying to keep the w/h ratio same here in case free-
> > > scale == true.
> > > Why would we do that? We can easily stretch the host window to any
> > > direction then the scale-x and scale-y
> > > could be different any time.
> > > 
> > Currently, the code doesn't clarify how we should handle aspect ratios.
> > However, I noticed that in the gd_draw_event function, when free-
> > scale=true, it preserves a fixed aspect ratio. I believe this is a
> > reasonable approach (in my humble opinion, it's unlikely that people
> > want to see distorted images), so I've decided to retain this behavior
> > for now and align other parts to follow the same logic, ensuring a
> > consistent experience for users.
> 
> Yeah, I didn't realize it does keep the ratio in case of gl=off in the original code.
> The reason I brought up this is because as you may Have noticed, scale_x and scale_y
> have been individually configured based on width and height of the current window
> and guest surface in gtk-egl and gtk-gl-area, which is being changed with your patches.
> I am not sure which one is the best either but this would definitely require some discussion.
> 

IMO, neither preserving nor ignoring the aspect ratio is objectively
“better” — both have its own supporters. I suggest we start by defining
a reasonable default and making all implementations conform to it (even
if it means changing the behaviors of a few backends). Once that’s in
place, I’ll add an option like "keep-aspect" or similar (per my
discussion with Balaton in the other thread) so users can select the
behavior they prefer. Does that sound acceptable?

> > Best regards,
> > Weifeng
> 
> > > > +    }
> > > > +}
> > > >  /**
> > > >   * DOC: Coordinate handling.
> > > >   *
> > > > @@ -908,17 +926,7 @@ static gboolean gd_draw_event(GtkWidget
> > > > *widget,
> > > > cairo_t *cr, void *opaque)
> > > >      ww_widget =
> > > > gdk_window_get_width(gtk_widget_get_window(widget));
> > > >      wh_widget =
> > > > gdk_window_get_height(gtk_widget_get_window(widget));
> > > > 
> > > > -    if (s->full_screen) {
> > > > -        vc->gfx.scale_x = (double)ww_widget / fbw;
> > > > -        vc->gfx.scale_y = (double)wh_widget / fbh;
> > > > -    } else if (s->free_scale) {
> > > > -        double sx, sy;
> > > > -
> > > > -        sx = (double)ww_widget / fbw;
> > > > -        sy = (double)wh_widget / fbh;
> > > > -
> > > > -        vc->gfx.scale_x = vc->gfx.scale_y = MIN(sx, sy);
> > > > -    }
> > > > +    gd_update_scale(vc, ww_widget, wh_widget, fbw, fbh);
> > > > 
> > > >      ww_surface = fbw * vc->gfx.scale_x;
> > > >      wh_surface = fbh * vc->gfx.scale_y;
> > > > --
> > > > 2.49.0
> > > > 

Re: [PATCH 1/9] ui/gtk: Document scale and coordinate handling

[Weifeng Liu]

Hi Gerd,

On Mon, 2025-05-12 at 13:46 +0200, Gerd Hoffmann wrote:
> On Sun, May 11, 2025 at 03:33:11PM +0800, Weifeng Liu wrote:
> > The existence of multiple scaling factors forces us to deal with
> > various
> > coordinate systems and this would be confusing. It would be
> > beneficial
> > to define the concepts clearly and use consistent representation
> > for
> > variables in different coordinates.
> > 
> > Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
> > ---
> >  ui/gtk.c | 65
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 65 insertions(+)
> > 
> > diff --git a/ui/gtk.c b/ui/gtk.c
> > index 982037b2c0..9f3171abc5 100644
> > --- a/ui/gtk.c
> > +++ b/ui/gtk.c
> > @@ -800,6 +800,71 @@ void
> > gd_update_monitor_refresh_rate(VirtualConsole *vc, GtkWidget
> > *widget)
> >  #endif
> >  }
> >  
> > +/**
> > + * DOC: Coordinate handling.
> > + *
> > + * We are coping with sizes and positions in various coordinates
> > and the
> > + * handling of these coordinates is somewhat confusing. It would
> > benefit us
> > + * all if we define these coordinates explicitly and clearly.
> > Besides, it's
> > + * also helpful to follow the same naming convention for variables
> > + * representing values in different coordinates.
> > + *
> > + * I. Definitions
> > + *
> > + * - (guest) buffer coordinate: this is the coordinates that the
> > guest will
> > + *   see. The x/y offsets and width/height specified in commands
> > sent by
> > + *   guest is basically in buffer coordinate.
> > + *
> > + * - (host) pixel coordinate: this is the coordinate in pixel
> > level on the
> > + *   host destop. A window/widget of width 300 in pixel coordinate
> > means it
> > + *   occupies 300 pixels horizontally.
> > + *
> > + * - (host) logical window coordinate: the existence of global
> > scaling
> > + *   factor in desktop level makes this kind of coordinate play a
> > role. It
> > + *   always holds that (logical window size) * (global scale
> > factor) =
> > + *   (pixel size).
> > + *
> > + * - global scale factor: this is specified in desktop level and
> > is
> > + *   typically invariant during the life cycle of the process.
> > Users with
> > + *   high-DPI monitors might set this scale, for example, to 2, in
> > order to
> > + *   make the UI look larger.
> > + *
> > + * - zooming scale: this can be freely controlled by the QEMU user
> > to zoom
> > + *   in/out the guest content.
> > + *
> > + * II. Representation
> > + *
> > + * We'd like to use consistent representation for variables in
> > different
> > + * coordinates:
> > + * - buffer coordinate: prefix fb
> > + * - pixel coordinate: prefix p
> > + * - logical window coordinate: prefix w
> > + *
> > + * For scales:
> > + * - global scale factor: prefix gs
> > + * - zooming scale: prefix scale/s
> > + *
> > + * Example: fbw, pw, ww for width in different coordinates
> > + *
> > + * III. Equation
> > + *
> > + * - fbw * gs * scale_x = pw
> 
> Well.  That is one possible approach (and this is what qemu is doing
> today, for historical reasons, because most code dates back to pre
> high-dpi days).
> 
> A possible alternative would be to go for fbw * scale_x = pw, i.e.
> let
> the guest run in pixel coordinates instead of window coordinates. 
> The
> guest would do the high-dpi scaling then.  That requires setting
> physical display width and height in ui_info, so the guest can figure
> what the display resolution is and go into high-dpi mode if needed.
> 

Thanks for your suggestion. Sounds like code could be simplified and be
much easier to understand in this way. I will investigate it on top of
this change.

Best regards,
Weifeng

> We probably also need a non-high-dpi compatibility mode for old
> guests.
> That mode would start with "zooming scale = global scale" instead of
> "zooming scale = 1", and the dpi calculation would have to consider
> that too.
> 
> (maybe best done on top of this nice cleanup).
> 
> take care,
>   Gerd

Re: [PATCH 1/9] ui/gtk: Document scale and coordinate handling

[BALATON Zoltan]

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

On Wed, 14 May 2025, Weifeng Liu wrote:
> Hi Gerd,
> On Mon, 2025-05-12 at 13:46 +0200, Gerd Hoffmann wrote:
>> On Sun, May 11, 2025 at 03:33:11PM +0800, Weifeng Liu wrote:
>>> The existence of multiple scaling factors forces us to deal with
>>> various
>>> coordinate systems and this would be confusing. It would be
>>> beneficial
>>> to define the concepts clearly and use consistent representation
>>> for
>>> variables in different coordinates.
>>>
>>> Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
>>> ---
>>>  ui/gtk.c | 65
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 65 insertions(+)
>>>
>>> diff --git a/ui/gtk.c b/ui/gtk.c
>>> index 982037b2c0..9f3171abc5 100644
>>> --- a/ui/gtk.c
>>> +++ b/ui/gtk.c
>>> @@ -800,6 +800,71 @@ void
>>> gd_update_monitor_refresh_rate(VirtualConsole *vc, GtkWidget
>>> *widget)
>>>  #endif
>>>  }
>>>  
>>> +/**
>>> + * DOC: Coordinate handling.
>>> + *
>>> + * We are coping with sizes and positions in various coordinates
>>> and the
>>> + * handling of these coordinates is somewhat confusing. It would
>>> benefit us
>>> + * all if we define these coordinates explicitly and clearly.
>>> Besides, it's
>>> + * also helpful to follow the same naming convention for variables
>>> + * representing values in different coordinates.
>>> + *
>>> + * I. Definitions
>>> + *
>>> + * - (guest) buffer coordinate: this is the coordinates that the
>>> guest will
>>> + *   see. The x/y offsets and width/height specified in commands
>>> sent by
>>> + *   guest is basically in buffer coordinate.
>>> + *
>>> + * - (host) pixel coordinate: this is the coordinate in pixel
>>> level on the
>>> + *   host destop. A window/widget of width 300 in pixel coordinate
>>> means it
>>> + *   occupies 300 pixels horizontally.
>>> + *
>>> + * - (host) logical window coordinate: the existence of global
>>> scaling
>>> + *   factor in desktop level makes this kind of coordinate play a
>>> role. It
>>> + *   always holds that (logical window size) * (global scale
>>> factor) =
>>> + *   (pixel size).
>>> + *
>>> + * - global scale factor: this is specified in desktop level and
>>> is
>>> + *   typically invariant during the life cycle of the process.
>>> Users with
>>> + *   high-DPI monitors might set this scale, for example, to 2, in
>>> order to
>>> + *   make the UI look larger.
>>> + *
>>> + * - zooming scale: this can be freely controlled by the QEMU user
>>> to zoom
>>> + *   in/out the guest content.
>>> + *
>>> + * II. Representation
>>> + *
>>> + * We'd like to use consistent representation for variables in
>>> different
>>> + * coordinates:
>>> + * - buffer coordinate: prefix fb
>>> + * - pixel coordinate: prefix p
>>> + * - logical window coordinate: prefix w
>>> + *
>>> + * For scales:
>>> + * - global scale factor: prefix gs
>>> + * - zooming scale: prefix scale/s
>>> + *
>>> + * Example: fbw, pw, ww for width in different coordinates
>>> + *
>>> + * III. Equation
>>> + *
>>> + * - fbw * gs * scale_x = pw
>>
>> Well.  That is one possible approach (and this is what qemu is doing
>> today, for historical reasons, because most code dates back to pre
>> high-dpi days).
>>
>> A possible alternative would be to go for fbw * scale_x = pw, i.e.
>> let
>> the guest run in pixel coordinates instead of window coordinates. 
>> The
>> guest would do the high-dpi scaling then.  That requires setting
>> physical display width and height in ui_info, so the guest can figure
>> what the display resolution is and go into high-dpi mode if needed.

Does that assume the guest knows about hidpi and has its own scale factor? 
What if I want to run an old guest that cannot do hidpi on a modern host. 
Can I still specify a scale factor to scale it up to usable size? That's a 
use case I care about which might be unusual but does exist.

Regards,
BALATON Zoltan

>
> Thanks for your suggestion. Sounds like code could be simplified and be
> much easier to understand in this way. I will investigate it on top of
> this change.
>
> Best regards,
> Weifeng
>
>> We probably also need a non-high-dpi compatibility mode for old
>> guests.
>> That mode would start with "zooming scale = global scale" instead of
>> "zooming scale = 1", and the dpi calculation would have to consider
>> that too.
>>
>> (maybe best done on top of this nice cleanup).
>>
>> take care,
>>   Gerd
>
>

Re: [PATCH 1/9] ui/gtk: Document scale and coordinate handling

[Weifeng Liu]

On Wed, 2025-05-14 at 13:50 +0200, BALATON Zoltan wrote:
> On Wed, 14 May 2025, Weifeng Liu wrote:
> > Hi Gerd,
> > On Mon, 2025-05-12 at 13:46 +0200, Gerd Hoffmann wrote:
> > > On Sun, May 11, 2025 at 03:33:11PM +0800, Weifeng Liu wrote:
> > > > The existence of multiple scaling factors forces us to deal
> > > > with
> > > > various
> > > > coordinate systems and this would be confusing. It would be
> > > > beneficial
> > > > to define the concepts clearly and use consistent
> > > > representation
> > > > for
> > > > variables in different coordinates.
> > > > 
> > > > Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
> > > > ---
> > > >  ui/gtk.c | 65
> > > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 65 insertions(+)
> > > > 
> > > > diff --git a/ui/gtk.c b/ui/gtk.c
> > > > index 982037b2c0..9f3171abc5 100644
> > > > --- a/ui/gtk.c
> > > > +++ b/ui/gtk.c
> > > > @@ -800,6 +800,71 @@ void
> > > > gd_update_monitor_refresh_rate(VirtualConsole *vc, GtkWidget
> > > > *widget)
> > > >  #endif
> > > >  }
> > > >  
> > > > +/**
> > > > + * DOC: Coordinate handling.
> > > > + *
> > > > + * We are coping with sizes and positions in various
> > > > coordinates
> > > > and the
> > > > + * handling of these coordinates is somewhat confusing. It
> > > > would
> > > > benefit us
> > > > + * all if we define these coordinates explicitly and clearly.
> > > > Besides, it's
> > > > + * also helpful to follow the same naming convention for
> > > > variables
> > > > + * representing values in different coordinates.
> > > > + *
> > > > + * I. Definitions
> > > > + *
> > > > + * - (guest) buffer coordinate: this is the coordinates that
> > > > the
> > > > guest will
> > > > + *   see. The x/y offsets and width/height specified in
> > > > commands
> > > > sent by
> > > > + *   guest is basically in buffer coordinate.
> > > > + *
> > > > + * - (host) pixel coordinate: this is the coordinate in pixel
> > > > level on the
> > > > + *   host destop. A window/widget of width 300 in pixel
> > > > coordinate
> > > > means it
> > > > + *   occupies 300 pixels horizontally.
> > > > + *
> > > > + * - (host) logical window coordinate: the existence of global
> > > > scaling
> > > > + *   factor in desktop level makes this kind of coordinate
> > > > play a
> > > > role. It
> > > > + *   always holds that (logical window size) * (global scale
> > > > factor) =
> > > > + *   (pixel size).
> > > > + *
> > > > + * - global scale factor: this is specified in desktop level
> > > > and
> > > > is
> > > > + *   typically invariant during the life cycle of the process.
> > > > Users with
> > > > + *   high-DPI monitors might set this scale, for example, to
> > > > 2, in
> > > > order to
> > > > + *   make the UI look larger.
> > > > + *
> > > > + * - zooming scale: this can be freely controlled by the QEMU
> > > > user
> > > > to zoom
> > > > + *   in/out the guest content.
> > > > + *
> > > > + * II. Representation
> > > > + *
> > > > + * We'd like to use consistent representation for variables in
> > > > different
> > > > + * coordinates:
> > > > + * - buffer coordinate: prefix fb
> > > > + * - pixel coordinate: prefix p
> > > > + * - logical window coordinate: prefix w
> > > > + *
> > > > + * For scales:
> > > > + * - global scale factor: prefix gs
> > > > + * - zooming scale: prefix scale/s
> > > > + *
> > > > + * Example: fbw, pw, ww for width in different coordinates
> > > > + *
> > > > + * III. Equation
> > > > + *
> > > > + * - fbw * gs * scale_x = pw
> > > 
> > > Well.  That is one possible approach (and this is what qemu is
> > > doing
> > > today, for historical reasons, because most code dates back to
> > > pre
> > > high-dpi days).
> > > 
> > > A possible alternative would be to go for fbw * scale_x = pw,
> > > i.e.
> > > let
> > > the guest run in pixel coordinates instead of window
> > > coordinates. 
> > > The
> > > guest would do the high-dpi scaling then.  That requires setting
> > > physical display width and height in ui_info, so the guest can
> > > figure
> > > what the display resolution is and go into high-dpi mode if
> > > needed.
> 
> Does that assume the guest knows about hidpi and has its own scale
> factor? 
> What if I want to run an old guest that cannot do hidpi on a modern
> host. 
> Can I still specify a scale factor to scale it up to usable size?
> That's a 
> use case I care about which might be unusual but does exist.
> 

Yeah, that use case will be supported by introducing a non-high-dpi
compatibility mode as suggested by Gerd. In this mode, the guest images
will be scaled by global desktop scale factor.

Best regards,
Weifeng

> Regards,
> BALATON Zoltan
> 
> > 
> > Thanks for your suggestion. Sounds like code could be simplified
> > and be
> > much easier to understand in this way. I will investigate it on top
> > of
> > this change.
> > 
> > Best regards,
> > Weifeng
> > 
> > > We probably also need a non-high-dpi compatibility mode for old
> > > guests.
> > > That mode would start with "zooming scale = global scale" instead
> > > of
> > > "zooming scale = 1", and the dpi calculation would have to
> > > consider
> > > that too.
> > > 
> > > (maybe best done on top of this nice cleanup).
> > > 
> > > take care,
> > >   Gerd
> > 

Re: [PATCH 0/9] ui: Improve scale handling

[Michael Tokarev]

On 11.05.2025 10:33, Weifeng Liu wrote:
> Hi all,
> 
> Now we have quite a lot of display backends for different use cases.
> Even in the context of gtk, we have various implementations (e.g., gl=on
> vs gl=off, X11 vs Wayland). However, behaviors to users are not aligned
> across the backends, especially in the part of scale handling. This
> patch set attempts to improve scale handling.
> 
> We have to deal with various coordinates due to the existence of scaling
> in different level. Firstly, in desktop level, we could have a global
> window scale factor. Secondly, users might set a zooming factor to
> adjust the size of guest content in scan-out level. Consequently, 1) the
> buffer from guest, 2) the host window and 3) OpenGl drawing area inside
> the host window are in distinct coordinates. It's important to define
> these coordinates and scales unambiguously and use a consistent naming
> convention for variables representing different concepts. The first
> patch in this set tries to achieve this goal by adding a document in
> gtk.c, and the next patch (PATCH 2) attempts to align the code with the
> document.
> 
> PATCH 3 - 5 fix bugs in mouse position calculation due to not handling
> scale properly, for both gtk and sdl.
> 
> PATCH 6 align scale update logic in gtk-egl with other implementations.
> 
> PATCH 7 fix an issue that gtk window might keep enlarging/shrinking because
> ui info propagating to guest not considering scale.
> 
> PATCH 8 and 9 align fixed-scale mode behavior in gtk-gl-area and gtk-egl with
> other implementations by adding appropriate padding to the window to preserve
> the scale.
...
> Weifeng Liu (9):
>    ui/gtk: Document scale and coordinate handling
>    ui/gtk: Use consistent naming for variables in different coordinates
>    gtk/ui: Introduce helper gd_update_scale
>    ui/gtk: Update scales in fixed-scale mode when rendering GL area
>    ui/sdl: Consider scaling in mouse event handling
>    ui/gtk: Don't update scale in fixed scale mode in gtk-egl.c
>    ui/gtk: Consider scaling when propagating ui info
>    ui/gtk-gl-area: Render guest content with padding in fixed-scale mode
>    ui/gtk-egl: Render guest content with padding in fixed-scale mode

Is there anything here which should be picked up for qemu-stable
(current active branches: 7.2 and 10.0)?

Thanks,

/mjt

Re: [PATCH 0/9] ui: Improve scale handling

[Weifeng Liu]

Hi Michael,

On Thu, 2025-05-29 at 10:23 +0300, Michael Tokarev wrote:
> On 11.05.2025 10:33, Weifeng Liu wrote:
> > Hi all,
> > 
> > Now we have quite a lot of display backends for different use
> > cases.
> > Even in the context of gtk, we have various implementations (e.g.,
> > gl=on
> > vs gl=off, X11 vs Wayland). However, behaviors to users are not
> > aligned
> > across the backends, especially in the part of scale handling. This
> > patch set attempts to improve scale handling.
> > 
> > We have to deal with various coordinates due to the existence of
> > scaling
> > in different level. Firstly, in desktop level, we could have a
> > global
> > window scale factor. Secondly, users might set a zooming factor to
> > adjust the size of guest content in scan-out level. Consequently,
> > 1) the
> > buffer from guest, 2) the host window and 3) OpenGl drawing area
> > inside
> > the host window are in distinct coordinates. It's important to
> > define
> > these coordinates and scales unambiguously and use a consistent
> > naming
> > convention for variables representing different concepts. The first
> > patch in this set tries to achieve this goal by adding a document
> > in
> > gtk.c, and the next patch (PATCH 2) attempts to align the code with
> > the
> > document.
> > 
> > PATCH 3 - 5 fix bugs in mouse position calculation due to not
> > handling
> > scale properly, for both gtk and sdl.
> > 
> > PATCH 6 align scale update logic in gtk-egl with other
> > implementations.
> > 
> > PATCH 7 fix an issue that gtk window might keep enlarging/shrinking
> > because
> > ui info propagating to guest not considering scale.
> > 
> > PATCH 8 and 9 align fixed-scale mode behavior in gtk-gl-area and
> > gtk-egl with
> > other implementations by adding appropriate padding to the window
> > to preserve
> > the scale.
> ...
> > Weifeng Liu (9):
> >    ui/gtk: Document scale and coordinate handling
> >    ui/gtk: Use consistent naming for variables in different
> > coordinates
> >    gtk/ui: Introduce helper gd_update_scale
> >    ui/gtk: Update scales in fixed-scale mode when rendering GL area
> >    ui/sdl: Consider scaling in mouse event handling
> >    ui/gtk: Don't update scale in fixed scale mode in gtk-egl.c
> >    ui/gtk: Consider scaling when propagating ui info
> >    ui/gtk-gl-area: Render guest content with padding in fixed-scale
> > mode
> >    ui/gtk-egl: Render guest content with padding in fixed-scale
> > mode
> 
> Is there anything here which should be picked up for qemu-stable
> (current active branches: 7.2 and 10.0)?
> 

I think the first five patches are good candidates for backporting to
the stable branches, as they only address bugs without altering
existing behavior. I was able to apply them cleanly to stable-10.0, but
porting them to 7.2 will require some additional effort. I'll send out
a new patch set once the backporting work is complete.

Best regards,
Weifeng

> Thanks,
> 
> /mjt

Re: [PATCH 0/9] ui: Improve scale handling

[Michael Tokarev]

Hi!

Thank you for your attention!

On 30.05.2025 09:39, Weifeng Liu wrote:
> On Thu, 2025-05-29 at 10:23 +0300, Michael Tokarev wrote:
>> Is there anything here which should be picked up for qemu-stable
>> (current active branches: 7.2 and 10.0)?
>>
> 
> I think the first five patches are good candidates for backporting to
> the stable branches, as they only address bugs without altering
> existing behavior. I was able to apply them cleanly to stable-10.0, but
> porting them to 7.2 will require some additional effort. I'll send out
> a new patch set once the backporting work is complete.

Please don't, - just forget about it in 7.2.  It's been here for quite
long already, and there was no complaints.  Let's keep it this way.
I'll apply it to 10.0.x series though.

Thank you!

/mjt

Re: [PATCH 0/9] ui: Improve scale handling

[Michael Tokarev]

On 30.05.2025 09:39, Weifeng Liu wrote:
> On Thu, 2025-05-29 at 10:23 +0300, Michael Tokarev wrote:

>>> PATCH 3 - 5 fix bugs in mouse position calculation due to not
>>> handling
>>> scale properly, for both gtk and sdl.
>>>
>>> PATCH 6 align scale update logic in gtk-egl with other
>>> implementations.
>>>
>>> PATCH 7 fix an issue that gtk window might keep enlarging/shrinking
>>> because
>>> ui info propagating to guest not considering scale.
>>>
>>> PATCH 8 and 9 align fixed-scale mode behavior in gtk-gl-area and
>>> gtk-egl with
>>> other implementations by adding appropriate padding to the window
>>> to preserve
>>> the scale.
>> ...
>>> Weifeng Liu (9):
>>>     ui/gtk: Document scale and coordinate handling
>>>     ui/gtk: Use consistent naming for variables in different
>>> coordinates
>>>     gtk/ui: Introduce helper gd_update_scale
>>>     ui/gtk: Update scales in fixed-scale mode when rendering GL area
>>>     ui/sdl: Consider scaling in mouse event handling
>>>     ui/gtk: Don't update scale in fixed scale mode in gtk-egl.c
>>>     ui/gtk: Consider scaling when propagating ui info
>>>     ui/gtk-gl-area: Render guest content with padding in fixed-scale
>>> mode
>>>     ui/gtk-egl: Render guest content with padding in fixed-scale
>>> mode
...
> I think the first five patches are good candidates for backporting to
> the stable branches, as they only address bugs without altering
> existing behavior. [...]

It somehow feels like *all* patches should be picked up, not just first
5 of them :)  BTW, the first one (Document scale handling) does not fix
any bugs ;)

Thanks,

/mjt

Re: [PATCH 0/9] ui: Improve scale handling

[Weifeng Liu]

On 5/30/25 6:56 PM, Michael Tokarev wrote:
> On 30.05.2025 09:39, Weifeng Liu wrote:
>> On Thu, 2025-05-29 at 10:23 +0300, Michael Tokarev wrote:
>
>>>> PATCH 3 - 5 fix bugs in mouse position calculation due to not
>>>> handling
>>>> scale properly, for both gtk and sdl.
>>>>
>>>> PATCH 6 align scale update logic in gtk-egl with other
>>>> implementations.
>>>>
>>>> PATCH 7 fix an issue that gtk window might keep enlarging/shrinking
>>>> because
>>>> ui info propagating to guest not considering scale.
>>>>
>>>> PATCH 8 and 9 align fixed-scale mode behavior in gtk-gl-area and
>>>> gtk-egl with
>>>> other implementations by adding appropriate padding to the window
>>>> to preserve
>>>> the scale.
>>> ...
>>>> Weifeng Liu (9):
>>>>     ui/gtk: Document scale and coordinate handling
>>>>     ui/gtk: Use consistent naming for variables in different
>>>> coordinates
>>>>     gtk/ui: Introduce helper gd_update_scale
>>>>     ui/gtk: Update scales in fixed-scale mode when rendering GL area
>>>>     ui/sdl: Consider scaling in mouse event handling
>>>>     ui/gtk: Don't update scale in fixed scale mode in gtk-egl.c
>>>>     ui/gtk: Consider scaling when propagating ui info
>>>>     ui/gtk-gl-area: Render guest content with padding in fixed-scale
>>>> mode
>>>>     ui/gtk-egl: Render guest content with padding in fixed-scale
>>>> mode
> ...
>> I think the first five patches are good candidates for backporting to
>> the stable branches, as they only address bugs without altering
>> existing behavior. [...]
>
> It somehow feels like *all* patches should be picked up, not just first
> 5 of them :)  BTW, the first one (Document scale handling) does not fix
> any bugs ;)
>

Great! Please apply all these patches to 10.0 branch. Let me know if
there's anything I can help with.

Best regards,
Weifeng