Remove gtk_widget_get_scale_factor() usage from the calculation of
the motion events in the GTK backend to make it work correctly on
environments that have `gtk_widget_get_scale_factor() != 1`.

This scale factor usage had been introduced in the commit f14aab420c and
at that time the window size was used for calculating the things and it
was working correctly. However, in the commit 2f31663ed4 the logic
switched to use the widget size instead of window size and because of
the change the usage of scale factor becomes invalid (since widgets use
`vc->gfx.scale_{x, y}` for scaling).

Tested on Crostini on ChromeOS (15823.51.0) with an external display.

Fixes: 2f31663ed4 ("ui/gtk: use widget size for cursor motion event")
Fixes: f14aab420c ("ui: fix incorrect pointer position on highdpi with
gtk")

Signed-off-by: hikalium <hikalium@hikalium.com>
---
 ui/gtk.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index ebae888d4f..4386198c95 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -887,7 +887,7 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion,
     int x, y;
     int mx, my;
     int fbh, fbw;
-    int ww, wh, ws;
+    int ww, wh;
 
     if (!vc->gfx.ds) {
         return TRUE;
@@ -895,11 +895,15 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion,
 
     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);
-    ws = gtk_widget_get_scale_factor(widget);
 
+    /*
+     * `widget` may not have the same size with the frame buffer.
+     * In such cases, some paddings are needed around the `vc`.
+     * 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;
@@ -908,8 +912,12 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion,
         my = (wh - fbh) / 2;
     }
 
-    x = (motion->x - mx) / vc->gfx.scale_x * ws;
-    y = (motion->y - my) / vc->gfx.scale_y * ws;
+    /*
+     * `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;
 
     trace_gd_motion_event(ww, wh, gtk_widget_get_scale_factor(widget), x, y);
 
-- 
2.39.2

Hi

On Sun, May 12, 2024 at 3:16 PM hikalium <hikalium@hikalium.com> wrote:
>
> Remove gtk_widget_get_scale_factor() usage from the calculation of
> the motion events in the GTK backend to make it work correctly on
> environments that have `gtk_widget_get_scale_factor() != 1`.
>
> This scale factor usage had been introduced in the commit f14aab420c and
> at that time the window size was used for calculating the things and it
> was working correctly. However, in the commit 2f31663ed4 the logic
> switched to use the widget size instead of window size and because of
> the change the usage of scale factor becomes invalid (since widgets use
> `vc->gfx.scale_{x, y}` for scaling).
>
> Tested on Crostini on ChromeOS (15823.51.0) with an external display.
>
> Fixes: 2f31663ed4 ("ui/gtk: use widget size for cursor motion event")
> Fixes: f14aab420c ("ui: fix incorrect pointer position on highdpi with
> gtk")
>

Thanks for the fix, I am okay with it.

But the QEMU displays are not working well with HiDPI. By treating
size & position with logical units, we can't let the guest handle
HiDPI. Imho, we should fix the code differently so the guest has the
non-scaled units.

This is not as trivial, since widget geometry and drawing code is a
bit more involved. And in theory we should adopt the same behaviour
for other display backends.

> Signed-off-by: hikalium <hikalium@hikalium.com>
> ---
>  ui/gtk.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/ui/gtk.c b/ui/gtk.c
> index ebae888d4f..4386198c95 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -887,7 +887,7 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion,
>      int x, y;
>      int mx, my;
>      int fbh, fbw;
> -    int ww, wh, ws;
> +    int ww, wh;
>
>      if (!vc->gfx.ds) {
>          return TRUE;
> @@ -895,11 +895,15 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion,
>
>      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);
> -    ws = gtk_widget_get_scale_factor(widget);
>
> +    /*
> +     * `widget` may not have the same size with the frame buffer.
> +     * In such cases, some paddings are needed around the `vc`.
> +     * 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;
> @@ -908,8 +912,12 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion,
>          my = (wh - fbh) / 2;
>      }
>
> -    x = (motion->x - mx) / vc->gfx.scale_x * ws;
> -    y = (motion->y - my) / vc->gfx.scale_y * ws;
> +    /*
> +     * `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;
>
>      trace_gd_motion_event(ww, wh, gtk_widget_get_scale_factor(widget), x, y);
>
> --
> 2.39.2
>
>


-- 
Marc-André Lureau