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 > >
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel