Re: [PATCH 1/2] ui/gtk: use widget size for cursor motion event

[Marc-André Lureau]

Hi Erico

On Thu, Mar 23, 2023 at 9:02 AM Kasireddy, Vivek
<vivek.kasireddy@intel.com> wrote:
>
> Hi Erico,
>
> > >
> > >>
> > >> The gd_motion_event size has some calculations for the cursor position,
> > >> which also take into account things like different size of the
> > >> framebuffer compared to the window size.
> > >> The use of window size makes things more difficult though, as at least
> > >> in the case of Wayland includes the size of ui elements like a menu bar
> > >> at the top of the window. This leads to a wrong position calculation by
> > >> a few pixels.
> > >> Fix it by using the size of the widget, which already returns the size
> > >> of the actual space to render the framebuffer.
> > >>
> > >> Signed-off-by: Erico Nunes <ernunes@redhat.com>
> > >> ---
> > >>  ui/gtk.c | 8 +++-----
> > >>  1 file changed, 3 insertions(+), 5 deletions(-)
> > >>
> > >> diff --git a/ui/gtk.c b/ui/gtk.c
> > >> index fd82e9b1ca..d1b2a80c2b 100644
> > >> --- a/ui/gtk.c
> > >> +++ b/ui/gtk.c
> > >> @@ -868,7 +868,6 @@ static gboolean gd_motion_event(GtkWidget *widget,
> > >> GdkEventMotion *motion,
> > >>  {
> > >>      VirtualConsole *vc = opaque;
> > >>      GtkDisplayState *s = vc->s;
> > >> -    GdkWindow *window;
> > >>      int x, y;
> > >>      int mx, my;
> > >>      int fbh, fbw;
> > >> @@ -881,10 +880,9 @@ 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;
> > >>
> > >> -    window = gtk_widget_get_window(vc->gfx.drawing_area);
> > >> -    ww = gdk_window_get_width(window);
> > >> -    wh = gdk_window_get_height(window);
> > >> -    ws = gdk_window_get_scale_factor(window);
> > >> +    ww = gtk_widget_get_allocated_width(widget);
> > >> +    wh = gtk_widget_get_allocated_height(widget);
> > > [Kasireddy, Vivek] Could you please confirm if this works in X-based 
> > > compositor
> > > environments as well? Last time I checked (with Fedora 36 and Gnome + X), 
> > > the
> > > get_allocated_xxx APIs were not accurate in X-based environments. 
> > > Therefore,
> > > I restricted the above change to Wayland-based environments only:
> > > https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03100.html
> >
> > Yes, I tested again and it seems to work fine for me even with the gtk
> > ui running on X. I'm using Fedora 37.
> [Kasireddy, Vivek] Ok, in that case, this patch is
> Acked-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>
> >
> > I was not aware of that patch series though and just spent some time
> > debugging these ui issues. It looks like your series was missed?
> [Kasireddy, Vivek] Yeah, not sure why my series was missed but in
> retrospect, I probably should have separated out bug fix patches
> from new feature enablement patches.
>
> >
> > I'm still debugging additional issues with cursor position calculation,
> > especially in wayland environments (and in particular with
> > vhost-user-gpu now). Do those patches address more cursor issues?
> [Kasireddy, Vivek] They do address more cursor issues but not sure how
> helpful they would be for you as most of them deal with relative mode +
> Wayland environment. However, there is another one that deals with
> cursor/pointer in absolute mode + multiple monitors:
> https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03097.html
>

Should we queue the 2 patches from this series? (note that they were
not correctly handled by patchew, probably because you dropped the
cover letter).

For me -display gtk is unusable on hidpi & wayland anyway, because the
cursor position given to the guest does not match the dimensions given
for the monitor.

Also relative mouse support is broken as well (mouse wrapping and
confinement/grab is not supported by gdk/gtk on wayland).

I am not actively looking at these problems, they are "solved" with
spice (use -display spice-app). And I am also regularly working on a
gtk4/rust widget, using -display dbus
(https://gitlab.gnome.org/malureau/rdw). There is also
https://gitlab.gnome.org/chergert/libmks as a gtk4/C alternative. I am
not sure we should keep maintaining the gtk3 backend going forward.
And as a Gtk4/-display dbus client mature, I hope we can offer a
better alternative than ui/sdl or ui/cocoa on other platforms as well.


-- 
Marc-André Lureau