Re: [spice-gtk] widget: avoid gdk_seat_grab/ungrab() API temporarily

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Quick update, this will not be needed anymore, yay.
https://gitlab.gnome.org/GNOME/gtk/merge_requests/166

Upstream bug with lots of information:
https://gitlab.gnome.org/GNOME/gtk/issues/1073

Cheers,
        toso

On Thu, May 24, 2018 at 08:41:02AM +0200, Victor Toso wrote:
> Hi Pavel,
> 
> Thanks for taking time to review this one.
> 
> On Tue, May 22, 2018 at 08:44:16PM +0200, Pavel Grunt wrote:
> > The intention of those commits was to be ready for new things
> > (wayland, gtk4... wait, actually gtk3). That side effect was
> > not desired...  I'm not sure whether is better to revert it
> > completely and go back to gtk2 api.
> 
> The change to get into gtk3 functions is good IMHO as we should
> be getting better support for wayland. I can only reproduce this
> bug on X11 so there is that :)
> 
> > Ack,
> 
> Thanks, I don't really plan to apply this patch upstream unless
> we are about to do a release. I hope to have time to handle
> gdk_seat_grab failures soonish.
> 
> > Pavel
> >
> > If there is a bug/regression in gtk+, it'd be good to have
> > reference to it in the commit ;)
> 
> I have both bugs in the bottom of the commit log, they are:
> 
> Fedora: https://bugzilla.redhat.com/show_bug.cgi?id=1485968
> RHEL7: https://bugzilla.redhat.com/show_bug.cgi?id=1571422
> 
> > Maybe it's easier for you to come up with a simple gtk
> > reproducer since you analyzed the issue properly.
> 
> Yes, I should have done that already indeed, thanks for the
> suggestion.
> 
> > If the comment is still valid, pity that the gtk+ bug did not
> > get any reaction (in the bug report).
> 
> Indeed.
> 
> Cheers,
>         toso
> >
> > 2018-05-22 8:42 GMT+02:00 Victor Toso <victortoso@xxxxxxxxxx>:
> >
> > > From: Victor Toso <me@xxxxxxxxxxxxxx>
> > >
> > > We are using those APIs to avoid the deprecated ones from GTK but
> > > unexpected events are handled different by them introducing a
> > > regression that is easy to reproduce when Spice client holds the
> > > focus and user locks the screen.
> > >
> > > We need to better handle the situation where gdk_seat_grab() fails,
> > > specifically with GDK_GRAB_ALREADY_GRABBED which is the situation
> > > for the bugs mentioned below. The grab does not fail with older API
> > > in the same situation.
> > >
> > > Note that gdk_seat_grab() returning GDK_GRAB_ALREADY_GRABBED is due
> > > the locking screen holding the keyboard device which also means that
> > > remote-viewer isn't visible/show but gdk_window_is_visible() returns
> > > true till gdk_seat_grab() is called. The failure in gtk+ function
> > > call sets the visibility to false and the next gdk_seat_grab() call
> > > does return GDK_GRAB_NOT_VIEWABLE and gdk_window_is_visible() false
> > > as it should.
> > >
> > > So, gtk+ needs to fix some behavior and spice-gtk improve error
> > > handle but till then, let's avoid the regression for users.
> > >
> > > This patch basically avoid the changes introduced by following
> > > commits:
> > >
> > >  | commit 283f41cd289084689fbdf1151da55aa451ba343c
> > >  | gtk: Use gdk_window_get_device_position
> > >  |
> > >  | commit a718aec66658ba6bb3bb45a9af81a3aa2a652d18
> > >  | widget: Use deprecated function to ungrab device
> > >  |
> > >  | commit 3f4c5bcc88ca5db125ec48ebf696cb23a8e6339a
> > >  | gtk: Avoid deprecated gdk_keyboard_grab
> > >  |
> > >  | commit ef7a6fa1798c8e53c0b77330b398a78181cf90e5
> > >  | gtk: Avoid deprecated gdk_pointer_grab
> > >
> > > Related: https://bugzilla.redhat.com/show_bug.cgi?id=1485968
> > > Related: https://bugzilla.redhat.com/show_bug.cgi?id=1571422
> > >
> > > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> > > ---
> > >  src/spice-widget.c | 12 ++++++------
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/src/spice-widget.c b/src/spice-widget.c
> > > index 72f5334..ec6a197 100644
> > > --- a/src/spice-widget.c
> > > +++ b/src/spice-widget.c
> > > @@ -811,7 +811,7 @@ SpiceGrabSequence *spice_display_get_grab_keys(SpiceDisplay
> > > *display)
> > >      return d->grabseq;
> > >  }
> > >
> > > -#if GTK_CHECK_VERSION(3, 20, 0)
> > > +#if 0 && GTK_CHECK_VERSION(3, 20, 0)
> > >  static GdkSeat *spice_display_get_default_seat(SpiceDisplay *display)
> > >  {
> > >      GdkWindow *window = gtk_widget_get_window(GTK_WIDGET(display));
> > > @@ -867,7 +867,7 @@ static void try_keyboard_grab(SpiceDisplay *display)
> > >                                              GetModuleHandle(NULL), 0);
> > >      g_warn_if_fail(d->keyboard_hook != NULL);
> > >  #endif
> > > -#if GTK_CHECK_VERSION(3, 20, 0)
> > > +#if 0 && GTK_CHECK_VERSION(3, 20, 0)
> > >      status = gdk_seat_grab(spice_display_get_default_seat(display),
> > >                             gtk_widget_get_window(widget),
> > >                             GDK_SEAT_CAPABILITY_KEYBOARD,
> > > @@ -892,7 +892,7 @@ static void try_keyboard_grab(SpiceDisplay *display)
> > >  static void ungrab_keyboard(G_GNUC_UNUSED SpiceDisplay *display)
> > >  {
> > >      G_GNUC_BEGIN_IGNORE_DEPRECATIONS
> > > -#if GTK_CHECK_VERSION(3, 20, 0)
> > > +#if 0 && GTK_CHECK_VERSION(3, 20, 0)
> > >      /* we want to ungrab just the keyboard - it is not possible using
> > > gdk_seat_ungrab().
> > >         See also https://bugzilla.gnome.org/show_bug.cgi?id=780133 */
> > >      GdkDevice *keyboard = gdk_seat_get_keyboard(spice_
> > > display_get_default_seat(display));
> > > @@ -1042,7 +1042,7 @@ static gboolean do_pointer_grab(SpiceDisplay
> > > *display)
> > >
> > >      try_keyboard_grab(display);
> > >      G_GNUC_BEGIN_IGNORE_DEPRECATIONS
> > > -#if GTK_CHECK_VERSION(3, 20, 0)
> > > +#if 0 && GTK_CHECK_VERSION(3, 20, 0)
> > >      status = gdk_seat_grab(spice_display_get_default_seat(display),
> > >                             window,
> > >                             GDK_SEAT_CAPABILITY_ALL_POINTING,
> > > @@ -1174,7 +1174,7 @@ static void mouse_wrap(SpiceDisplay *display,
> > > GdkEventMotion *motion)
> > >  static void ungrab_pointer(G_GNUC_UNUSED SpiceDisplay *display)
> > >  {
> > >      G_GNUC_BEGIN_IGNORE_DEPRECATIONS
> > > -#if GTK_CHECK_VERSION(3, 20, 0)
> > > +#if 0 && GTK_CHECK_VERSION(3, 20, 0)
> > >      /* we want to ungrab just the pointer - it is not possible using
> > > gdk_seat_ungrab().
> > >         See also https://bugzilla.gnome.org/show_bug.cgi?id=780133 */
> > >      GdkDevice *pointer = gdk_seat_get_pointer(spice_
> > > display_get_default_seat(display));
> > > @@ -2431,7 +2431,7 @@ static GdkDevice *spice_gdk_window_get_pointing_device(GdkWindow
> > > *window)
> > >  {
> > >      GdkDisplay *gdk_display = gdk_window_get_display(window);
> > >      G_GNUC_BEGIN_IGNORE_DEPRECATIONS
> > > -#if GTK_CHECK_VERSION(3, 20, 0)
> > > +#if 0 && GTK_CHECK_VERSION(3, 20, 0)
> > >      return gdk_seat_get_pointer(gdk_display_get_default_seat(gdk_
> > > display));
> > >  #else
> > >      return gdk_device_manager_get_client_pointer(gdk_display_get_
> > > device_manager(gdk_display));
> > > --
> > > 2.17.0
> > >
> > > _______________________________________________
> > > Spice-devel mailing list
> > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > >



> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]