Re: [PATCH v2 2/2] ui/gtk: Fix mouse/motion event scaling issue with GTK display backend

[Marc-André Lureau]

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