Hey, Sorry for taking some time to review your patches!
On Sun, Jan 21, 2018 at 09:03:12PM +0100, Jakub Janků wrote: > Hi, > these two patches replace the current clipboard handling code, > which uses Xlib, with code utilizing GTK+. The code was moved > from x11.c to clipboard.c > > Although GTK+ can work on Wayland natively, this code currently > works only under X. gdk_set_allowed_backends("x11") in > vdagent.c is called to force GTK+ to always use X11 backend. > > The implementation is partially based on the code in spice-gtk > (spice-gtk-session.c). > > Cheers, Awesome work. Tested and seems to work as expected on F26 guest. I'll be reviewing this inline but I'd like to suggest a different approach to the patches. For the clipboard, GTK+ is going to replace x11 fairly well and I don't expect major problems but we might have some, we never know. I would like to do one last vdagent release with x11 clipboard to explicit say that we will replace x11 with gtk+ after that. I think we can do a release as soon as this gets in. So, removing x11 calls would be a problem. Another issue is that, it is hard to review/compare when one patch removes the feature and the other add it back besides the fact that bisecting would be broken to that feature (although its not a big problem in this case, I think). My suggestion is to have a ./configure --with-gtk or --enable-gtk to wrap new code around #ifdef GTK_ENABLED for the new code otherwise it calls the x11 code as before. I think the clipboard function functions exported by x11.h should be moved to clipboard.[ch] and together with the proposed VDAgentClipboards (interface) changes. What do you think? Cheers, toso > > Makefile.am | 2 + > src/vdagent/clipboard.c | 401 ++++++++++++++++++ > src/vdagent/clipboard.h | 42 ++ > src/vdagent/vdagent.c | 38 +- > src/vdagent/x11-priv.h | 91 ---- > src/vdagent/x11.c | 1074 > +---------------------------------------------- > src/vdagent/x11.h | 10 - > 7 files changed, 472 insertions(+), 1186 deletions(-) > create mode 100644 src/vdagent/clipboard.c > create mode 100644 src/vdagent/clipboard.h > > -- > 2.14.3 > > _______________________________________________ > Spice-devel mailing list > Spiceemail@example.com > https://lists.freedesktop.org/mailman/listinfo/spice-devel
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spicefirstname.lastname@example.org https://lists.freedesktop.org/mailman/listinfo/spice-devel