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

[Kasireddy, Vivek]

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

Thanks,
Vivek
> 
> Thank you
> 
> Erico
>